WP/AlternativeFunctions: add tests for namespaced names tests and fix handling of class functions/constants/properties

[rodrigoprimo]

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

[rodrigoprimo]

@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.

[jrfnl]

Hi @rodrigoprimo Thanks for working on this. Happy to see this merged.

Open tickets related to follow-up work on this sniff:

• #2603 for handling namespaced name variations for constants/function calls passed as the filename to file_get_contents(), which the dedicated logic for handling calls to file_get_contents() then recognizes as logic files and bows out for.
• #2602 for improving/more selectively bowing out when STDIN/STDOUT/STDERR is seen in a parameter for select function calls.

While looking at the sniff to see if everything that needs to be covered by tests for the changed tokenization in PHPCS 4.x is covered, I uncovered one more situation which may need looking in to and/or needs tests: namespaced variations for STDIN/STDOUT/STDERR.

Example: https://3v4l.org/gvZhG

#2602 is open to look whether the sniff makes correct exceptions for those input/output streams, but in the mean time, the sniff should handle namespaced variants of these correctly.
And based on the current code, I expect that the sniff already does so, with only one exception: an FQN reference to the global PHP native constants is not handled correctly, i.e. \STDOUT.
As far as I can see this can be fixed with only a tiny tweak to the code, so I'd rather see that handled now instead of it being left for #2602.

@rodrigoprimo What do you think ? And was there a reason not to include namespace variants of STDIN/STDOUT/STDERR in this PR ?

[rodrigoprimo]

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).

[rodrigoprimo]

Looking into the failing quick test build.

[jrfnl]

Looking into the failing quick test build.

I'm not seeing any failures ?

[jrfnl]

Thanks @rodrigoprimo ! LGTM

[rodrigoprimo]

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.

[rodrigoprimo]

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.