Report constant-string arguments to #[JetBrains\PhpStorm\FileReference] parameters that do not resolve to an existing path#6062
Conversation
…ce]` parameters that do not resolve to an existing path - Extract the include/require path resolution (current working directory + include_path + script directory) from `RequireFileExistsRule` into a new autowired `PHPStan\File\FileExistenceChecker` service, adding a `pathExists()` variant that also accepts directories. - In `FunctionCallParametersCheck`, validate every constant-string argument passed to a parameter annotated with `#[JetBrains\PhpStorm\FileReference]` and report `argument.fileReference` when the path does not resolve to an existing file or directory. This runs for every call type that funnels through the check: function, method, static-method and constructor (including promoted properties) calls. - Honor the attribute's `$basePath` constructor argument as an additional search root, resolved against the analysed file's directory when relative. - Non-constant string arguments are never flagged (no constant strings -> no check), so there are no false positives on dynamic paths. - Closures/arrow functions are not covered: their parameters use `NativeParameterReflection`, which does not carry attribute reflections. Left out of scope on purpose.
…ure toggle The check for constant-string arguments passed to #[JetBrains\PhpStorm\FileReference] parameters is now only enabled in bleeding edge via the new %featureToggles.checkFileReferences% toggle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| @@ -47,12 +43,11 @@ final class RequireFileExistsRule implements Rule | |||
| ]; | |||
There was a problem hiding this comment.
memo to me:
in case the above file-existence functions also get the #[FileReference] attributes in phpstorm stubs, this might mean in the future we need to adjust this rule
https://youtrack.jetbrains.com/issue/WI-85516/FileReference-and-builtin-functions-like-isfile
There was a problem hiding this comment.
Done. Here's what I did to address the reviewer's memo.
The concern
staabm left a memo warning that if phpstorm-stubs eventually add #[JetBrains\PhpStorm\FileReference] to file-existence-testing built-ins (is_file, file_exists, …) — as tracked in WI-85516 — the new check would start reporting argument.fileReference false positives: it's nonsensical to complain that a path "does not exist" for a function whose entire purpose is to test whether it exists.
I confirmed this is a real, reachable regression by redeclaring is_file with the attribute and running the analyser under bleeding edge — it did emit Path "..." does not exist. before the fix.
The fix (one logical commit)
src/File/FileExistenceChecker.php— moved the list of existence-testing functions (previously a private const inRequireFileExistsRule) here as a sharedpublic const FILE_EXISTENCE_FUNCTIONS, with a doc comment linking WI-85516. Both the include/require guard and the new call-site check now share one source of truth.src/Rules/Keywords/RequireFileExistsRule.php— references the shared constant instead of its own copy.src/Rules/FunctionCallParametersCheck.php—checkFileReferences()now skips the whole call when it resolves to one of those functions (isFileExistenceTestingCall()), so annotated existence-testers never trigger the check.tests/PHPStan/Rules/data/file-reference.php— added a global-namespaceis_file()redeclared with#[FileReference]and called with a missing path; theCallToFunctionParametersRuleTestexpectations stay at 3 errors, proving the new call is not reported.
Verification
- Confirmed the test fails without the guard (a 4th error appears at the new line) and passes with it.
make tests: 17611 passed (97 skipped), no failures.make phpstan: No errors.
Committed as c4b753983 and pushed to create-pull-request/patch-q6sl023.
One note for the reviewer: this covers the false-positive risk proactively. The separate memo about dropping the in-fixture FileReference attribute re-declaration once phpstorm-stubs ship it remains genuinely future work (it depends on those stubs existing), so I left it untouched.
| namespace JetBrains\PhpStorm { | ||
|
|
||
| #[\Attribute(\Attribute::TARGET_PARAMETER)] | ||
| class FileReference | ||
| { | ||
|
|
||
| public function __construct(string $basePath = '') | ||
| { | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
I think we can cleanup the re-declaration of this attribute, after phpstorm-stubs ship the declaration themselfes
There was a problem hiding this comment.
Pushed. Here's a summary of how I addressed the review.
Review comment
I think we can cleanup the re-declaration of this attribute, after phpstorm-stubs ship the declaration themselves
Investigation
I checked whether the local JetBrains\PhpStorm\FileReference declaration in the test fixtures can be dropped in favor of phpstorm-stubs' own declaration. It cannot yet, for three concrete reasons I verified:
-
PHPStan doesn't consume the declaration phpstorm-stubs ships. phpstorm-stubs does ship
meta/attributes/FileReference.php, but that lives in the IDE-onlymeta/attributes/directory. PHPStan resolves stub classes only via the autoloadedPhpStormStubsMap, which excludes every meta attribute. Proof:#[\JetBrains\PhpStorm\Pure](same directory) reportsattribute.notFound.FileReferenceonly resolves in this repo today because the fixture's declaration got indexed into composer's classmap — a dev-only artifact. -
A PHPStan
stubFilecan't rescue it either. Stub files only augment reflection for classes that exist via another source; they don't register brand-new classes. I confirmed with a probe: a class defined solely in a registered.stubstill reportsclass.notFound. -
The declaration is functionally required.
getFileReferenceBaseDirectories()readsgetArgumentTypes()['basePath']. Mapping the positional argument in#[FileReference('file-reference-base')]to the namebasePathneeds the constructor reflection. Remove the class and the base-path positive case regresses into a false positive.
So removal is blocked on an external change (phpstorm-stubs registering FileReference in PhpStormStubsMap), not on anything in this PR.
Change made
Since the cleanup isn't possible yet, I captured the constraint durably in both fixtures — replacing the blank line under the namespace brace (net-zero lines, so no test expected-line-numbers changed):
namespace JetBrains\PhpStorm {
// Declared locally: PHPStan does not autoload phpstorm-stubs' meta/attributes/FileReference.php (it is absent from PhpStormStubsMap). Drop this once the attribute is registered there.
#[\Attribute(\Attribute::TARGET_PARAMETER)]This tells a future maintainer exactly when and why the redeclaration can go — mirroring the reviewer's own "memo to me" style earlier in the PR.
Verification
- Targeted rule + require + integration tests: pass.
make phpstan: No errors.- Rebased onto the newly-pushed upstream commit (the
is_fileexemption); no conflict, re-ran tests green, pushed tocreate-pull-request/patch-q6sl023.
Functions like is_file()/file_exists() legitimately receive paths that may not exist, so reporting argument.fileReference for them would be a false positive once phpstorm-stubs annotate them with #[FileReference] (https://youtrack.jetbrains.com/issue/WI-85516). The list of such functions, previously private to RequireFileExistsRule, now lives on FileExistenceChecker and is shared by both the include/require guard and the call-site check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fixtures PHPStan cannot resolve JetBrains\PhpStorm\FileReference from phpstorm-stubs: the attribute ships only in the IDE-only meta/attributes/ directory, which is not part of the autoloaded PhpStormStubsMap, and PHPStan stub files cannot register a brand-new class. The local declaration is also functionally required so the constructor is reflectable and the positional $basePath argument maps to its name. Note the removal condition so the fixture declaration can be dropped once the attribute is registered in PhpStormStubsMap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| ]; | ||
| } | ||
|
|
||
| public function testFileReferenceAttribute(): void |
There was a problem hiding this comment.
AnalyserIntegrationTest is reserved for crash bugs - remove this test
There was a problem hiding this comment.
Done. I addressed the review:
Change
Removed testFileReferenceAttribute from AnalyserIntegrationTest — the reviewer noted this test class is reserved for crash bugs, not feature coverage.
Specifically:
- Deleted the test method from
tests/PHPStan/Analyser/AnalyserIntegrationTest.php. - Deleted its data fixture
tests/PHPStan/Analyser/data/file-reference-attribute.php. - Removed that fixture's entry from
build/collision-detector.json(it was only excluded there because it redeclared theFileReferenceattribute).
The feature remains fully covered by the four rule tests (CallToFunctionParametersRuleTest, CallMethodsRuleTest, CallStaticMethodsRuleTest, InstantiationRuleTest), which each set checkFileReferences: true and pull the real FileExistenceChecker from the container — so the autowired-dependency wiring is still exercised.
Verification
- File-reference rule tests: pass (3 tests).
AnalyserIntegrationTest: pass (184 tests) after removal.make phpstan: No errors.
Committed as aab2b2840 and pushed to create-pull-request/patch-q6sl023.
AnalyserIntegrationTest is reserved for crash bugs. The feature is already covered end-to-end by the rule tests (CallToFunctionParametersRuleTest, CallMethodsRuleTest, CallStaticMethodsRuleTest, InstantiationRuleTest). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
PhpStorm 2026.2 added
#[JetBrains\PhpStorm\FileReference], an attribute that marks a string parameter as a file or directory path. This PR teaches PHPStan to validate constant-string arguments passed to such parameters: if the string does not resolve to an existing file or directory, an error is reported — reusing the same path-resolution logic that already backsrequire/includechecking.Changes
src/File/FileExistenceChecker.php: extracts the path-resolution logic (base directories → current working directory →include_path→ analysed script directory) out ofRequireFileExistsRule. ExposesfileExists()(file only, used by the require/include rule) andpathExists()(file or directory, used by the new check). Relative base directories are absolutized against the analysed file's directory.src/Rules/Keywords/RequireFileExistsRule.php: now delegates itsdoesFileExist()toFileExistenceCheckerinstead of duplicating the directory walk.src/Rules/FunctionCallParametersCheck.php: after mapping arguments to parameters, checks each constant-string argument passed to a parameter carrying#[JetBrains\PhpStorm\FileReference]and reportsargument.fileReferencewhen the path does not exist. Because every call rule (CallToFunctionParametersRule,CallMethodsRule,CallStaticMethodsRule,InstantiationRule,CallCallablesRule) funnels through this check, the feature covers function, method, static-method and constructor calls (including promoted constructor properties) in one place. The attribute's$basePathargument is honored as an extra search root.FunctionCallParametersCheck/RequireFileExistsRulemanually to pass the new dependency; regeneratedvendor/attributes.phpso the new autowired service is registered; added the two duplicate attribute-definition data files to the name-collision exclude list.Root cause
This is a feature request, not a bug. The reporter asked to reuse "the machinery around
RequireFileExistsRule". The shared piece is how a possibly-relative path is resolved and existence-checked; it was previously private toRequireFileExistsRule. Pulling it intoFileExistenceCheckerlets both the include/require rule and the new call-site check share one implementation. Placing the call-site check insideFunctionCallParametersCheck— where argument-to-parameter mapping (named args, variadics, reordering) is already solved and where the analogous#[\Deprecated]/allowed-constants attribute checks already live — means every parallel call construct is handled by a single implementation rather than four near-duplicate rules.Parallel cases considered
#[FileReference]on a promoted__constructparameter is checked, verified by a dedicated test case.$basePathargument (fixed): relative base paths resolve against the analysed file's directory; covered by a test.pathExists()accepts directories; covered by a test.NativeParameterReflection, which does not implementExtendedParameterReflectionand carries no attribute reflections, so the attribute is invisible at the call site. Threading attributes throughNativeParameterReflectionand the closure-type resolver is a larger, independent change and was intentionally left out.Test
tests/PHPStan/Rules/data/file-reference.php— a shared fixture declaring theJetBrains\PhpStorm\FileReferenceattribute and exercising function, method, static, constructor and promoted-property calls with existing paths, missing paths, a directory, a non-constant path (must not be flagged) and a$basePath-relative path. Asserted fromCallToFunctionParametersRuleTest,CallMethodsRuleTest,CallStaticMethodsRuleTestandInstantiationRuleTest(each rule reports only errors for its own node kind).tests/PHPStan/Analyser/AnalyserIntegrationTest::testFileReferenceAttribute— end-to-end through the real DI container, proving the new autowired dependency is injected in production.RequireFileExistsRuletests continue to pass against the refactoredFileExistenceChecker.Fixes phpstan/phpstan#14971