Report editor-mode (--tmp-file) analysis under the --instead-of path so Scope::getFile() returns the real path#6079
Open
phpstan-bot wants to merge 1 commit into
Conversation
…ath so `Scope::getFile()` returns the real path - `ScopeContext` now carries both the reported file and the analysis file (the temp file whose content is read); `FileAnalyser::analyseFile()` takes a `$reportedFile` so the temp file is parsed but the Scope, errors, collected data and line-ignores are all reported under the real `--instead-of` path. `Scope::getFile()` / `getFileDescription()` therefore match a normal run, and location-aware rules (Larastan's `NoEnvCallsOutsideOfConfigRule`, `RequireFileExistsRule`, `__FILE__`/`__DIR__`) behave the same in editor mode. - Add `EditorModeFileHelper`, which maps a reported path back to the temp file. Internal machinery that must read/reflect/cache the file by its Scope path now uses the analysis file: `NodeScopeResolver` AST class reflection, `BetterReflectionProvider` anonymous classes, `MutatingScope`'s `__COMPILER_HALT_OFFSET__` check, and `FileTypeMapper` PHPDoc name-scope resolution. This keeps reflection/PHPDoc reading the edited content and keyed by the temp file (no cross-run cache poisoning). - Thread `--tmp-file`/`--instead-of` into `Analyser::analyse()` and update `WorkerRunner` so both the single-process and parallel paths report the real path. - `ResultCacheManager` merge of errors, locally-ignored errors and collected data no longer remaps the cache key through `fileReplacements`, because these fresh results are now keyed by the real path; content reads (file hash, exported-nodes diff) still resolve to the temp file.
Member
|
This looks very complicated, let it settle for a while. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In editor mode (
--tmp-file/--instead-of) PHPStan analysed the temp file under the temp file's own path, soScope::getFile()returned the temp path (typically/tmp/xxxx.php) duringRule::processNode(). The path was only swapped back to the real file after analysis, when results were formatted. Any rule whose logic depends on the analysed file's location behaved differently in editor mode than in a normal run — e.g. Larastan'sNoEnvCallsOutsideOfConfigRulefired on everyenv()call inconfig/*.phpbecause the temp path never starts with<project>/config.This change makes the analysis observe the file at its real (
--instead-of) path: the temp file's content is still read/parsed/reflected, but the Scope, errors, collected data and line-ignores are all reported under the real path, soScope::getFile()(and anything derived from it) matches a normal analysis run.Changes
src/Analyser/ScopeContext.php— carries bothfile(reported path) andanalysisFile(temp file being read); addsgetAnalysisFile().create()gains an optional second argument (backward compatible).src/Analyser/FileAnalyser.php—analyseFile()gains a?string $reportedFileargument. The temp file is parsed/read, but theScopeContext,FileAnalyserCallback, processed-files list and parse/reflection error paths are reported under$reportedFile.src/Analyser/Analyser.php/src/Command/AnalyserRunner.php/src/Parallel/WorkerRunner.php— thread--tmp-file/--instead-ofthrough so both the single-process and parallel worker paths report the real path.src/Analyser/EditorModeFileHelper.php(new) — maps a reported path back to the temp file whose content is analysed (identity outside editor mode).src/Analyser/MutatingScope.php—getAnalysisFile();__COMPILER_HALT_OFFSET__detection parses the analysis file.src/Analyser/NodeScopeResolver.php— AST-based class reflection (getCurrentClassReflection/createAstClassReflection) matches and reads the analysis file.src/Reflection/BetterReflection/BetterReflectionProvider.php— anonymous-class reflection reads/identifies via the analysis file.src/Type/FileTypeMapper.php—getResolvedPhpDoc()/getNameScope()resolve the PHPDoc name-scope from the analysis file, so PHPDoc rules (including third-party ones) that passScope::getFile()still read the editeduse/namespace context.src/Analyser/ResultCache/ResultCacheManager.php— merging of errors, locally-ignored errors and collected data no longer remaps the cache key throughfileReplacements(these fresh results are now keyed by the real path). File-hash and exported-node reads still resolve to the temp file so cache invalidation stays correct.Root cause
The file identity used during analysis was the temp file, with a post-analysis path swap normalizing the output. Rules run before that swap, so they saw the temp path. The fix inverts this for everything rules observe: the real path becomes the identity for the Scope, errors, collected data and line-ignores, while a single helper (
EditorModeFileHelper) redirects the handful of internal consumers that genuinely need the temp file's content — reflection matching/reading (NodeScopeResolver,BetterReflectionProvider), PHPDoc name-scope resolution (FileTypeMapper) and the__halt_compilercheck (MutatingScope). The temp file stays the cache identity for reflection/PHPDoc so persistent caches are not poisoned across runs.Analogous location-derived behaviors fixed by the same change (each now matching a normal run instead of leaking the temp path):
Scope::getFileDescription(),RequireFileExistsRule(dirname($scope->getFile())), and the__FILE__/__DIR__magic constants (InitializerExprContext).Test
tests/PHPStan/Analyser/EditorModeGetFileTest.php— runsFileAnalyser::analyseFile()on a temp file with areportedFile, asserting a rule sees the real path via bothScope::getFile()andScope::getFileDescription(), and that the reported error path is the real path; a second case confirms non-editor-mode behavior is unchanged. Verified failing before the fix (reported the temp path) and passing after.e2e/editor-modescenario (reflection of the edited file across a dependency, plus the result-cache restore path) continues to pass, including the assertion that the temp path never appears in output.Fixes phpstan/phpstan#14982