Classes/NewReadonlyProperties: start using PHPCSUtils 1.1.0 getDeclared*() #1841
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A while ago, someone reported a performance issue with the
Generic.PHP.LowerCaseType
sniff, which was due to the sniff examining all variables, instead of only examining OO properties.Along the same lines, a similar performance issue was previously reported in #1477 and an initial fix was put in place via #1577.
PHPCSUtils 1.1.0 now introduces the new
ObjectDeclarations::getDeclaredProperties
andObjectDeclarations::getDeclaredMethods()
methods which can retrieve a list of all properties/methods declared in an OO structure.These PHPCSUtils methods are highly optimized and will cache the results, which means that if multiple sniffs use these methods, the results will be nearly instantaneously returned on subsequent uses.
This commits starts using these methods in this sniff, which should further improve performance.
Note: the one downside of using the new methods, is that the names of the declared properties and methods in an OO construct have to be unique. As that's already a requirement in PHP anyway, I'm not concerned about the "real world" impact of this, though it does mean we need to be careful with this when creating test case files for sniffs which use these methods.
For that reason, there are some minor adjustments in the test case file for this sniff.
Some non-scientific performance impact estimation:
Using the test file which was shared with me - https://github.com/Automattic/HyperDB/blob/trunk/db.php - and running the below on PHP 8.4:
... yielded the following difference in runtime:
Baseline - Timing when run without this commit:
Timing when run with this commit:
While it doesn't make that much difference on the total runtime with just one file, you can see the sniff runtime has gone down significantly - from 0.1542 secs to 0.002 secs, which should have a significant positive impact when scanning complete codebases.