Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add checks to disallow use of empty() #1219

Open
westonruter opened this issue May 16, 2024 · 3 comments · May be fixed by #1786
Open

Add checks to disallow use of empty() #1219

westonruter opened this issue May 16, 2024 · 3 comments · May be fixed by #1786
Assignees
@ShyamGadde
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

westonruter commented May 16, 2024 •
edited
Loading

Feature Description

The use of empty() and isset( $var ) can mask code problems (e.g. typos). And they rarely need to be used, for example:

cf. comment by @felixarntz in #1091 (comment)

We should consider adding sniffs to warn against their use. (Granted, PHPStan should catch the problematic uses of isset() and empty().)
@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure labels May 16, 2024
@swissspidy
Copy link
Member

https://github.com/phpstan/phpstan-strict-rules Is your friend.

Works like a charm

@westonruter
Copy link
Member Author

But I said we were done with PHPStan PRs 😂

@westonruter westonruter mentioned this issue May 23, 2024
Merged
13 tasks
@westonruter westonruter changed the title Add sniffs to disallow use of isset() and empty() Add checks to disallow use of isset() and empty() May 23, 2024
@westonruter westonruter added this to the performance-lab 3.2.0 milestone May 23, 2024
@westonruter westonruter mentioned this issue Jun 3, 2024
Merged
This was referenced Jul 24, 2024
Merged
Merged
@westonruter westonruter mentioned this issue Jul 30, 2024
Open
@westonruter westonruter removed this from the performance-lab n.e.x.t milestone Aug 13, 2024
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Aug 13, 2024
@westonruter westonruter added this to the performance-lab n.e.x.t milestone Aug 13, 2024
@westonruter westonruter mentioned this issue Aug 21, 2024
Merged
@felixarntz
Copy link
Member

Per #1489 (comment), let's change this issue to be only about empty() - or, if we want to include isset(), it should only be prevented on variable checks like isset( $something ), but not on e.g. array key or object property checks like isset( $arr['something'] ).

@westonruter westonruter changed the title Add checks to disallow use of isset() and empty() Add checks to disallow use of isset( $var ) and empty() Aug 21, 2024
@westonruter westonruter removed this from the performance-lab n.e.x.t milestone Sep 20, 2024
@westonruter westonruter changed the title Add checks to disallow use of isset( $var ) and empty() Add checks to disallow use of empty() Nov 13, 2024
@westonruter westonruter moved this from Not Started/Backlog 📆 to To Do 🔧 in WP Performance 2024 Nov 13, 2024
@westonruter westonruter mentioned this issue Nov 13, 2024
Merged
@westonruter westonruter mentioned this issue Nov 20, 2024
Merged
@ShyamGadde ShyamGadde linked a pull request Jan 9, 2025 that will close this issue
Open
@westonruter westonruter moved this from To Do 🔧 to In Progress 🚧 in WP Performance 2024 Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ShyamGadde ShyamGadde

Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature
Projects
WP Performance 2024
Status: In Progress 🚧
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add custom PHPCS sniff to warn against usage of empty() ShyamGadde/wp-performance
4 participants
@westonruter @swissspidy @felixarntz @ShyamGadde