Skip to content

RequiredPhpVersionVisitor: report named arguments as requiring PHP 8.0#5890

Merged
staabm merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-pcs33g5
Jun 17, 2026
Merged

RequiredPhpVersionVisitor: report named arguments as requiring PHP 8.0#5890
staabm merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-pcs33g5

Conversation

@phpstan-bot

@phpstan-bot phpstan-bot commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

RequiredPhpVersionVisitor detects the minimum PHP version a test fixture needs to be parsed, so CI can skip it on older PHP versions instead of hitting a hard parse error. It did not recognize named arguments, which require PHP 8.0. This PR teaches it to report named arguments, following up on the same work done for other version-gated syntax (phpstan/phpstan#14816, phpstan/phpstan#14824, phpstan/phpstan#14831, #5855).

Changes

  • build/PHPStan/Build/RequiredPhpVersionVisitor.php: report named arguments as requiring PHP 8.0 when a Node\Arg has a non-null name. A single check on the Arg node covers all call contexts — function calls, method calls, nullsafe method calls, static calls, new expressions and attributes — instead of enumerating each call type.
  • tests/PHPStan/Build/RequiredPhpVersionCommentTest.php: add testDetectedVersion cases for named arguments in function calls, method calls, nullsafe method calls, static calls, new, and attributes, plus a negative case asserting positional-only arguments require nothing.
  • Added // lint >= 8.0 comments to the 25 fixtures that the new detection correctly flagged as using named arguments without advertising the version.
  • For the two nsrt fixtures (bug-4510.php, bug-5262.php), which are consumed by TypeInferenceTestCase that requires the // lint comment to immediately follow <?php, the comment is placed on its own first line and declare() moved to the next line. CallCallablesRuleTest::testBug4510 asserts errors on specific lines of bug-4510.php, so its expected line numbers are bumped by one.

Root cause

The visitor enumerated which syntactic features require which PHP version but had no entry for named arguments. Rather than adding the check to each of the four call-expression branches it already handled (and still missing new and attributes), the fix queries the Node\Arg node directly: a named argument is exactly an Arg whose name is set, regardless of the enclosing construct, so one check sweeps the entire call-argument axis.

Test

  • RequiredPhpVersionCommentTest::testDetectedVersion gains cases proving named arguments are reported as PHP 8.0 in function/method/nullsafe-method/static calls, new, and attributes, and that positional-only arguments report nothing. These fail without the visitor change.
  • RequiredPhpVersionCommentTest::testFixtureHasRequiredLintComment (which runs over every data/nsrt fixture) now passes after the flagged fixtures received their // lint >= 8.0 comments.
  • Full make tests and make phpstan pass.

Refs phpstan/phpstan#14835

- Detect named arguments via a single `Node\Arg` check with a non-null `name`,
  which covers every call context at once: function calls, method calls,
  nullsafe method calls, static calls, `new` expressions and attributes.
- Add `testDetectedVersion` cases for named arguments across all those contexts
  plus a negative case for positional-only arguments.
- Add `// lint >= 8.0` comments to the fixtures the new detection flagged so
  they are skipped on older PHP versions in CI. For the two `nsrt` fixtures
  (consumed by TypeInferenceTestCase, which requires the comment to immediately
  follow `<?php`), the comment is placed on its own first line and the
  `declare()` moved down; the line-sensitive expectations in
  CallCallablesRuleTest::testBug4510 are bumped by one line accordingly.
@staabm staabm merged commit 1c80634 into phpstan:2.2.x Jun 17, 2026
388 of 389 checks passed
@staabm staabm deleted the create-pull-request/patch-pcs33g5 branch June 17, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants