Skip to content

Report constant-string arguments to #[JetBrains\PhpStorm\FileReference] parameters that do not resolve to an existing path#6062

Open
phpstan-bot wants to merge 7 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-q6sl023
Open

Report constant-string arguments to #[JetBrains\PhpStorm\FileReference] parameters that do not resolve to an existing path#6062
phpstan-bot wants to merge 7 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-q6sl023

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

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 backs require/include checking.

Changes

  • New service src/File/FileExistenceChecker.php: extracts the path-resolution logic (base directories → current working directory → include_path → analysed script directory) out of RequireFileExistsRule. Exposes fileExists() (file only, used by the require/include rule) and pathExists() (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 its doesFileExist() to FileExistenceChecker instead 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 reports argument.fileReference when 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 $basePath argument is honored as an extra search root.
  • Updated the tests that construct FunctionCallParametersCheck / RequireFileExistsRule manually to pass the new dependency; regenerated vendor/attributes.php so 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 to RequireFileExistsRule. Pulling it into FileExistenceChecker lets both the include/require rule and the new call-site check share one implementation. Placing the call-site check inside FunctionCallParametersCheck — 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

  • Call kinds (fixed): function, method, static method, and constructor calls all report correctly — one shared implementation, one test per kind.
  • Promoted constructor properties (fixed): #[FileReference] on a promoted __construct parameter is checked, verified by a dedicated test case.
  • $basePath argument (fixed): relative base paths resolve against the analysed file's directory; covered by a test.
  • Directories (fixed): the attribute marks a file or directory path, so pathExists() accepts directories; covered by a test.
  • Closures / arrow functions (out of scope): their parameters are represented by NativeParameterReflection, which does not implement ExtendedParameterReflection and carries no attribute reflections, so the attribute is invisible at the call site. Threading attributes through NativeParameterReflection and 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 the JetBrains\PhpStorm\FileReference attribute 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 from CallToFunctionParametersRuleTest, CallMethodsRuleTest, CallStaticMethodsRuleTest and InstantiationRuleTest (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.
  • Existing RequireFileExistsRule tests continue to pass against the refactored FileExistenceChecker.

Fixes phpstan/phpstan#14971

…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.
Comment thread src/Rules/FunctionCallParametersCheck.php Outdated
…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
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in RequireFileExistsRule) here as a shared public 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.phpcheckFileReferences() 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-namespace is_file() redeclared with #[FileReference] and called with a missing path; the CallToFunctionParametersRuleTest expectations 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.

Comment on lines +3 to +15
namespace JetBrains\PhpStorm {

#[\Attribute(\Attribute::TARGET_PARAMETER)]
class FileReference
{

public function __construct(string $basePath = '')
{
}

}

}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can cleanup the re-declaration of this attribute, after phpstorm-stubs ship the declaration themselfes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. PHPStan doesn't consume the declaration phpstorm-stubs ships. phpstorm-stubs does ship meta/attributes/FileReference.php, but that lives in the IDE-only meta/attributes/ directory. PHPStan resolves stub classes only via the autoloaded PhpStormStubsMap, which excludes every meta attribute. Proof: #[\JetBrains\PhpStorm\Pure] (same directory) reports attribute.notFound. FileReference only resolves in this repo today because the fixture's declaration got indexed into composer's classmap — a dev-only artifact.

  2. A PHPStan stubFile can'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 .stub still reports class.notFound.

  3. The declaration is functionally required. getFileReferenceBaseDirectories() reads getArgumentTypes()['basePath']. Mapping the positional argument in #[FileReference('file-reference-base')] to the name basePath needs 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_file exemption); no conflict, re-ran tests green, pushed to create-pull-request/patch-q6sl023.

phpstan-bot and others added 3 commits July 18, 2026 18:01
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

@staabm staabm Jul 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AnalyserIntegrationTest is reserved for crash bugs - remove this test

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the FileReference attribute).

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.

phpstan-bot and others added 2 commits July 18, 2026 18:12
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>
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.

Support #[JetBrains\PhpStorm\FileReference]

2 participants