PHP 8.3: New sniff to detect dynamic class constant fetch #1974

afilina · 2025-10-30T15:17:51Z

Part of this effort: #1589

Adding a sniff for new syntax to dynamically fetch class constants C::{$name}. Existing sniffs were not fit for this purpose, and adding things there would have made them unnecessarily hard to maintain.

Implemented in a similar fashion to other new syntax sniffs.

Will flag any use of this syntax before it was introduced in PHP 8.3,
Message: Dynamic class constant fetch is not available in PHP 8.2 or earlier.
Using Found error code.
Added complex expressions in the curly brackets, including one with :: to ensure sniff doesn't get confused.
Assumed 10.0.0 target release.
Out of scope: checking for corresponding closing curly bracket. This seemed unnecessary, as it will do nothing but lint in exchange for extra overhead.

Edit: we ended up checking the corresponding closing bracket. It was needed to weed out some false positives.

afilina · 2025-10-30T15:18:27Z

@jrfnl Please review when you are able. Thanks.

afilina · 2025-10-31T00:19:39Z

Checklist per our discussions:

Show message on opening curly bracket
Add more test cases & split false positives into 2 groups, including parse error example
Strict comparisons instead of !
Inline variables in favor of comments
Line 6 not actually tested

The three groups of tests are: - Explicitly targeted by the sniff and found on unsupported version - Same above, but not found on supported versions - Cases that are not the sniff's target, but using similar tokens or syntax that might throw false positives Added a parse error test case, since PHPCS can be used as a plugin, and so will run on code still being typed (live coding).

afilina · 2025-10-31T01:15:39Z

@jrfnl Ready for a second pass. I likely won't be around tomorrow evening so I don't mind comments in writing, unless there's something complex I need to understand. I will focus on incorporating your feedback to the other sniff PRs.

I added a comment in the test fixtures for each line. Hopefully it can help with documenting test guidelines (putting words on things so you have less work).

Refs - RFC: https://wiki.php.net/rfc/dynamic_class_constant_fetch

afilina · 2025-10-31T13:26:01Z

@jrfnl I added XML docs per the proposed contributing guide #1976

afilina · 2025-11-13T14:36:15Z

@jrfnl Would you mind giving this another look and merging if all is good? I'm hesitant to work on new sniffs until I understand what to aim for.

jrfnl

Hi @afilina I really like where this is going. It's close, but it's not there yet.

The tests as they currently are, are not correctly testing what needs testing. When the testVersion changes have been made for which I left inline suggestions, you will see that with the current sniff code, the sniff throws a false positive.
This will need to be fixed.

Hope this review helps.

PHPCompatibility/Docs/Syntax/NewDynamicClassConstantFetchStandard.xml

PHPCompatibility/Tests/Syntax/NewDynamicClassConstantFetchUnitTest.inc

PHPCompatibility/Tests/Syntax/NewDynamicClassConstantFetchUnitTest.php

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

jrfnl · 2025-11-18T03:02:55Z

However, I'm now getting cs errors on other sniffs. Is that because I need to rebase?

There has been a new PHPCSDevCS release, which is causing those. PR #1983 is open to handle that update. Rebasing will only fix the issues you are seeing after PR #1983 has been merged. (which is why that PR has the "high priority" label as until it is merged, other PRs are blocked due to CI failing)

In the mean time, you can ignore those CS errors as long as they are not for the new files in this PR.

jrfnl · 2025-11-18T03:18:39Z

I addressed all your comments.

@afilina The tests are failing now as predicted in my base review comment. To fix this, a change is needed to the sniff logic.

The tests as they currently are, are not correctly testing what needs testing. When the testVersion changes have been made for which I left inline suggestions, you will see that with the current sniff code, the sniff throws a false positive.
This will need to be fixed.

As things are, the sniff does not distinguish between the following:

Foo::{$bar}(); // Supported since PHP 5,4
Foo::{$bar}; // Supported since PHP 8.3

https://3v4l.org/5a2r8

Also looks like the PHP 5.4 change which added support for Foo::{$bar}() isn't handled by PHPCompatibility yet, but that's outside the scope of this PR (but may be a straight-forward one to address once this PR has been merged ?).

Maybe we should open an issue to investigate the details of the PHP 5.4 change separately ? (I haven't found the relevant RFC yet, but only took a quick glance at the PHP 5.4 RFCs).

afilina · 2025-11-18T15:58:41Z

With CI cancelling the actual tests due to CS issues, how would I know that my PR is ready to be merged, other than running the unit tests locally? I want to avoid manual review back-and-forth.

Edit: feedback on the error message

Operator ! prohibited; use === FALSE instead

I remember we spoke about a sniff for prohibiting uppercase booleans. Not sure it's fixable since a sniff doesn't look at the config of other sniffs for its message, but just wanted to point out that this small issue exists.

jrfnl · 2025-11-18T16:08:57Z

With CI cancelling the actual tests due to CS issues, how would I know that my PR is ready to be merged ... ?

CI doesn't cancel the test runs due to the CS issues. The tests run just fine as they are in a separate workflow, but they are failing: https://github.com/PHPCompatibility/PHPCompatibility/actions/runs/19445175583

... other than running the unit tests locally

I would always recommend running the tests locally (on a single PHP version) for any sniff related commit anyhow.

CI will verify it against every supported combination, but it is rare for the tests for PHPCompatibility to fail on a single combination. In most cases, like here, it will fail for every single combination of PHP + PHPCS + Utils as there is something wrong in the sniff logic.

afilina · 2025-11-18T16:21:07Z

@jrfnl Hmm, I may have misread the CI results then. It seemed like the test tasks appeared as cancelled seconds after the CS failed.

Edit: I also wanted to apologize for not running the tests before asking for another review. I was caught in merging the suggestions and didn't consider this at the end.

jrfnl · 2025-11-18T16:32:21Z

@afilina While trying to fall asleep last night, my brain came up with some additional code samples to run the tests with.
I've checked with 3v4l and they appear to all be PHP 8.3+, as in: these are part of the new syntax being detected by this sniff.

Foo::{$bar}->prop;
Foo::{$bar}->method();
Foo::{$bar}?->prop;
Foo::{$bar}?->method();
Foo::{$bar}::CONSTANT_NAME;

Still probably wouldn't go amiss to include the additional tests, if only to document that that syntax is now supported and that PHPCompatibility will flag it.

PHPCompatibility/Sniffs/Syntax/NewDynamicClassConstantFetchSniff.php

jrfnl · 2025-11-18T16:51:35Z

PHPCompatibility/Sniffs/Syntax/NewDynamicClassConstantFetchSniff.php

            return;
        }

+        $closingBracket = $phpcsFile->findNext(\T_CLOSE_CURLY_BRACKET, $nextNonEmpty + 1);


There is a better way to do this.

As things are, the findNext() call may return false during live coding Foo::{$bar (and yes, this test will need to be added as a separate parse error test). This false would now be turned into 1 via the $closingBracket + 1 on the next line, which is a bug...

Additionally, what about: Foo::{{$bar->name}()} ? Now, the sniff would find the wrong curly as it doesn't take the nested curlies into account.

The good news is that brackets will create an extra detail entry in the token stream, which means you can find the close curly like this:

$opener = $nextNonEmpty; if (isset($tokens[$opener]['bracket_closer']) === false) { // Live coding or parse error. Bow out. // To think over: or should this throw an error ? return; } $closer = $tokens[$opener]['bracket_closer'];

This is why arrays are so hard to work with :)
Thanks for the suggestion.

or should this throw an error ?

Normally we skip parse errors. And since it prevents us from definitively detect the sniff's target issue, we shouldn't report anything. That's how I came to expect PHPCompatibility to work.

This is why arrays are so hard to work with :)

Not sure I understand that jibe. The problem was with an incorrect findNext(). Not with an array which actually contained useful information.

Going back to Foo::{{$bar->name}()}, this is another parse error, but it looks fine to the sniff, which would basically see Foo::{...} with no following parenthesis. So not only do we need to match the right closer, we need to make sure that there is no opener immediately following an opener. I have to admit I'm a bit lost as to how far I need to dig into this entire category of parse errors.

Edit: made an attempt in the commit below. After {, PHP expects an expression. We don't have a simple way to say "everything between this and that pointer is a valid expression". So I just limit to { so we don't go overboard with this sniff.

Going back to Foo::{{$bar->name}()}, this is another parse error, but it looks fine to the sniff, which would basically see Foo::{...} with no following parenthesis. So not only do we need to match the right closer, we need to make sure that there is no opener immediately following an opener.

I used the Foo::{{$bar->name}()} example just to highlight the logic error in the token walking.

Unless there are currently explicit rules in PHP about what is allowed between the curlies for the new syntax, I'm fine with not checking between the curlies.

I have to admit I'm a bit lost as to how far I need to dig into this entire category of parse errors.

It's always hard to determine where to draw the line. As a rule of thumb: when your own fantasy of syntaxes to take into account runs out ;-) I know that is not a hard or solid guideline, but sometimes it's the best we can do.

Let me know if there are any other blockers on this PR.

…sts to their own test files

…move test down ... to comply with expected order of the test methods.

* Add missing line numbers to data provider * Add one extra test case

* Fix tag order in test docblocks. * Use proper punctuation in inline comments in sniff. * Use FQN class refs in docblocks.

jrfnl

@afilina Thank you for all your work on this.

I've now merged the current develop branch in to allow for this PR to get a passing build.
I've also made a few more tweaks, most of which were discussed in review comments already. I've pushed those in separate commits so you can see what I've done.

I'm happy for this PR to be merged now and will include it in the 10.0.0-alpha2 release.

afilina added 2 commits October 30, 2025 10:37

Add minimal sniff and test for NewDynamicClassConstantFetch C::{$name}

6a48c88

Add remaining test cases for NewDynamicClassConstantFetch

75c2a07

jrfnl added this to the 10.0.0-alpha2 milestone Oct 30, 2025

jrfnl added Type: enhancement PR: ready for review labels Oct 30, 2025

jrfnl changed the title ~~Dynamic class constant fetch~~ PHP 8.3 | New sniff to detect dynamic class constant fetch Oct 30, 2025

afilina added 4 commits October 30, 2025 20:20

Add coverage for last fixture

b50bd94

Remove single-use variables; add strict comparisons

f2ed0ec

Add error on the opening curly brace token

0780a12

jrfnl mentioned this pull request Oct 31, 2025

Account for changes introduced in PHP 8.3 #1589

Open

35 tasks

PHP 8.3 | NewDynamicClassConstantFetch: add XML documentation

7c3cee9

Refs - RFC: https://wiki.php.net/rfc/dynamic_class_constant_fetch

afilina changed the title ~~PHP 8.3 | New sniff to detect dynamic class constant fetch~~ PHP 8.3: New sniff to detect dynamic class constant fetch Nov 14, 2025

jrfnl requested changes Nov 17, 2025

View reviewed changes

jrfnl modified the milestones: 10.0.0-alpha2, 10.0.0-alpha3 Nov 17, 2025

jrfnl added Status: awaiting feedback and removed PR: ready for review labels Nov 17, 2025

afilina and others added 7 commits November 17, 2025 16:14

Normalize indentation

46bab38

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Add missing semicolon to avoid parse error on the wrong line

72ef6cb

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

More specific return type hint

1b2350e

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Earlier versions that don't support the syntax should throw errors

2686269

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Extend the no-violation check to the whole file

4cfe30f

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Standardize comment above testNoFalsePositives

c691a41

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

More specific return type hint

a3bbc5c

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Adjust sniff for newly introduced CS rules

13c0f5c

Fix deprecation since PHPCS 4.0

067f366

afilina added 2 commits November 18, 2025 11:34

Detect false positive with a static method called dynamically

22b21ae

Add more test cases for 8.3+ behavior (fail on 8.2)

49e4cc7

jrfnl reviewed Nov 18, 2025

View reviewed changes

PHPCompatibility/Sniffs/Syntax/NewDynamicClassConstantFetchSniff.php Outdated Show resolved Hide resolved

afilina added 2 commits November 18, 2025 11:44

Revert to Tokens::$emptyTokens as we still need to support older PHPCS

a693745

Revert to Tokens::$emptyTokens as we still need to support older PHPCS

7044ac2

jrfnl reviewed Nov 18, 2025

View reviewed changes

Match correct closing bracket while skipping {{}} parse error

ce2aa13

jrfnl mentioned this pull request Nov 19, 2025

[Project] Add XML documentation for each sniff #1285

Open

jrfnl added 6 commits November 25, 2025 11:30

Merge branch 'develop' into afilina_dynamic-class-constant-fetch

bcfc88d

Syntax/NewDynamicClassConstantFetch: add extra parse error test

c3754ee

Syntax/NewDynamicClassConstantFetch: move pre-existing parse error te…

f622772

…sts to their own test files

Syntax/NewDynamicClassConstantFetch: remove unused data provider and …

c0fcfbe

…move test down ... to comply with expected order of the test methods.

Syntax/NewDynamicClassConstantFetch: make sure all test cases are tested

3af44d3

* Add missing line numbers to data provider * Add one extra test case

Syntax/NewDynamicClassConstantFetch: minor documentation touch up

3fc92bd

* Fix tag order in test docblocks. * Use proper punctuation in inline comments in sniff. * Use FQN class refs in docblocks.

jrfnl modified the milestones: 10.0.0-alpha3, 10.0.0-alpha2 Nov 25, 2025

jrfnl approved these changes Nov 25, 2025

View reviewed changes

jrfnl merged commit 23bd186 into PHPCompatibility:develop Nov 25, 2025
66 checks passed

jrfnl added PHP: 8.3 and removed Status: awaiting feedback labels Nov 25, 2025

afilina deleted the dynamic-class-constant-fetch branch November 25, 2025 20:27

afilina mentioned this pull request Dec 4, 2025

PHP 8.5: Soft deprecate __sleep / __wakeup #1972

Open

Uh oh!

PHP 8.3: New sniff to detect dynamic class constant fetch #1974

PHP 8.3: New sniff to detect dynamic class constant fetch #1974

Uh oh!

Conversation

afilina commented Oct 30, 2025 • edited by jrfnl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afilina commented Oct 30, 2025

Uh oh!

afilina commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afilina commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afilina commented Oct 31, 2025

Uh oh!

afilina commented Nov 13, 2025

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrfnl commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrfnl commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afilina commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrfnl commented Nov 18, 2025

Uh oh!

afilina commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrfnl commented Nov 18, 2025

Uh oh!

Uh oh!

jrfnl Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

afilina Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

afilina Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

jrfnl Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

afilina Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrfnl Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

afilina Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

afilina commented Oct 30, 2025 •

edited by jrfnl

Loading

afilina commented Oct 31, 2025 •

edited

Loading

afilina commented Oct 31, 2025 •

edited

Loading

jrfnl commented Nov 18, 2025 •

edited

Loading

jrfnl commented Nov 18, 2025 •

edited

Loading

afilina commented Nov 18, 2025 •

edited

Loading

afilina commented Nov 18, 2025 •

edited

Loading

afilina Nov 18, 2025 •

edited

Loading