WordPress
WP/AlternativeFunctions: add tests for namespaced names tests and fix handling of class functions/constants/properties
Description
WP/AlternativeFunctions: add tests for namespaced names
Add tests for namespaced names, including some that currently are not flagged by the sniff but will be flagged once #2603 is fully addressed. As discussed in that issue, it will be easier to address this part once support for PHPCS 3.x is dropped.
WP/AlternativeFunctions: improve regex to distinguish global WP constants/functions from non-WP class-based ones
This commit changes two regexes used to identify global WP constants and functions to prevent the sniff from incorrectly identifying class constants/methods with the same names as WordPress globals as being WordPress globals. This is done by adding a negative lookbehind to ensure the searched strings are not preceded by an object operator, null-safe object operator, or scope resolution operator.
WP/AlternativeFunctions: fix handling of FQN references to global stream constants
This commit fixes the handling of fully qualified name (FQN) references to the global PHP stream constants \STDIN, \STDOUT, and \STDERR by normalizing the passed parameter before checking it against the allowed list.
Tests have been added to cover all namespace forms of references to the global PHP stream constants.
Suggested changelog entry
Fixed: WordPress.WP.AlternativeFunctions: prevents false negatives when class functions/constants/properties use the same name as select global WP constants/functions.
Fixed: WordPress.WP.AlternativeFunctions: prevents false positives when fully qualified references to the global PHP stream constants \STDIN, \STDOUT, and \STDERR are used.
Related issues/external references
Partially addresses #2603
@jrfnl, I updated the namespaced names tests for this PR based on what we discussed earlier this week and also left a new reply in one of the discussions above. This PR is ready for review when you are avaiable.
Thanks for your review, @jrfnl!
@rodrigoprimo What do you think ? And was there a reason not to include namespace variants of STDIN/STDOUT/STDERR in this PR ?
That makes sense to me. I added a new commit that fixes the handling of FQN references to STDIN/STDOUT/STDERR, including tests that cover namespaced variations. I also updated the PR description to include this new commit and a suggested changelog entry.
I don't recall if there was a reason not to include tests with namespace variants of STDIN/STDOUT/STDERR in this PR. It was probably something that I missed.
Once you finish reviewing this PR, I can squash the commits before another maintainer reviews it. I'm thinking this PR should contain three commits (the ones mentioned in the PR description).
I'm not seeing any failures ?
That is interesting. The "Basic QA Tests" and the "Quick Tests" actions that ran on my fork failed, and they were showing to me here as a red X next to the last commit. That is what I was referring to. But now I don't see it anymore. Instead, I see that they passed with a link to the actions in this repository. So I guess this can be ignored.
Once you finish reviewing this PR, I can squash the commits before another maintainer reviews it. I'm thinking this PR should contain three commits (the ones mentioned in the PR description).
I went ahead, squashed the commits, and force-pushed without changes.