PHP 8.0: NewNestedStaticAccess: add support for class constant dereferencing by elazar · Pull Request #1262 · PHPCompatibility/PHPCompatibility

@jrfnl

I only have a few small remarks, nothing major.

  1. Each error message should have a unique error code so error messages can be selectively ignored. As things are, they currently both have the same error code.
    You can either just adjust the error code for the new error message or adjust both to made the distinction between the two clear.
    As the next release will be a major, we don't have to worry too much about a change in the original error code being a BC break.

Done in f4e4e84.

  1. As this sniff now has checks related to different PHP versions, it would be a good idea to add a "no-violation on PHP 7.0" check to the testNestedStaticAccess() method.

Done in bdc6c09.

  1. Aside from the existing "false positive" prevention checks, is there any other code pattern you can think of which we need to guard against specifically for the new class constant dereferencing ? I.e. are there new false positive code patterns which could be added in the test case file which aren't currently accounted for ? (if the answer is "no", that's perfectly fine, I just want to make sure this has been considered)

None that I can think of, no.

Non-blocking:

  1. The error message phrasing is inconsistent - cannot be dereferenced in PHP 7.4 or earlier vs was not supported in PHP 5.6 or earlier.
    Not a blocker, but might be an idea to make the phasing more consistent.

Done in bb66569.

  1. Code-scouting: I realize the original testNestedStaticAccess() test does not have a proper method description, but with an eye on the documentation clean up, it would be nice if the newly added test method has a proper description and if you feel like adding a proper description to the testNestedStaticAccess() that would be awesome too! smile

Done in d5c7e51.