Normalize %placeholder%-expanded path parameters and canonicalize the result cache meta - #6295
Open
phpstan-bot wants to merge 1 commit into
Open
Normalize %placeholder%-expanded path parameters and canonicalize the result cache meta#6295phpstan-bot wants to merge 1 commit into
%placeholder%-expanded path parameters and canonicalize the result cache meta#6295phpstan-bot wants to merge 1 commit into
Conversation
…he result cache meta - Add `NormalizePathParametersExtension`, a compiler extension that runs `FileHelper::normalizePath()` over every parameter listed in the `expandRelativePaths` section once Nette has expanded the `%placeholder%`s. `NeonAdapter` skips values containing a `%`, so a `%rootDir%/../../../vendor/autoload.php` entry used to reach the container with its `..` segments intact. - Canonicalize the freshly computed result cache meta in `ResultCacheManager::getMeta()` by putting it through `ResultCachePathTransformer::absolutizeMeta()`, the exact transformation the stored meta goes through on restore. Paths PHPStan does not control are not normalized to begin with - Composer records `install_path` as `vendor/composer/../foo/bar` - so `composerInstalled` used to differ on every single run. Bump `CACHE_VERSION` to `v15-canonicalPaths` and drop the now-redundant ad-hoc `configStubFiles` normalization. - Normalize `--autoload-file` and `--configuration` in `CommandHelper::begin()`, the two siblings that only absolutized while `--generate-baseline`, `--tmp-file`, `--instead-of` and the analysed paths already normalized. - Probed and found already correct: `FileExcluder`, `IgnoredErrorHelper`, `FileFinder`, `DefaultStubFilesProvider` and `NodeDependencies` all normalize defensively on their own, and `ReflectionClass::getFileName()` is resolved by PHP even for Composer's classmap entries.
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
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 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
A
scanFilesentry written as%rootDir%/../../../vendor/autoload.phpdiscarded the result cache onevery run (
Result cache not used because the metadata do not match: scannedFiles, composerInstalled),while the equivalent placeholder-free
vendor/autoload.phpentry worked fine.NeonAdapterabsolutizes and normalizes the path entries listed in theexpandRelativePathssection,but skips any value containing a
%- those are still unexpanded strings at load time. Such a paththerefore reaches the container with its
..segments intact, while the result cache stores its metadatarelativized and restores it absolutized, which collapses those segments. Restored never equalled
recomputed, so the cache was thrown away every run.
The fix normalizes the path parameters right after Nette expands the placeholders, and additionally
canonicalizes the freshly computed result cache meta with the very same transformation the stored meta
goes through - which also fixes a second, unreported instance of the same round trip being lossy.
Changes
src/DependencyInjection/NormalizePathParametersExtension.php(new): a#[ContainerExtension]that walks the parameters named by the
expandRelativePathssection and runsFileHelper::normalizePath()over them.loadConfiguration()is the first point where theplaceholders are already expanded. It handles the
[]"every list element" segments of those configkeys (
[parameters][ignoreErrors][][paths][]), unwraps and rewrapsOptionalPath, and deliberatelyleaves still-relative values alone - an fnmatch pattern or a placeholder that expanded to a relative
value has no config file left to resolve against, and
normalizePath()would silently drop itsleading
..segments.src/Analyser/ResultCache/ResultCacheManager.php:getMeta()now returns$this->getPathTransformer()->absolutizeMeta(...), so the computed meta is in the same canonical formas the restored one.
CACHE_VERSIONbumped tov15-canonicalPaths. The ad-hocconfigStubFilesnormalization is dropped - it was a one-off instance of what the canonicalization now does for every
path-bearing meta key.
src/Command/CommandHelper.php:--autoload-fileand--configurationare now normalized aswell as absolutized, matching
--generate-baseline,--tmp-file,--instead-ofand the analysedpaths, which already did both.
FileExcluder,IgnoredErrorHelper,FileFinder,DefaultStubFilesProviderandNodeDependencieseach normalizedefensively before comparing (the new extension makes those defences redundant rather than
load-bearing);
allConfigFilesis normalized inContainerFactory::detectDuplicateIncludedFiles();and
ReflectionClass::getFileName()is resolved by PHP itself, so Composer's classmap entries(
__DIR__ . '/../..' . '/src/Foo.php') do not leak..intoprojectExtensionFiles.resultCachePath(%tmpDir%/resultCache.php) is not inexpandRelativePathsand stays untouched:adding it there would also change how a relative
resultCachePathis resolved, which is abehaviour change unrelated to this bug. It is only ever opened, never compared.
Root cause
The pattern is "the same file spelled two ways": one code path produces a lexically non-canonical
path (
a/b/../c) and another produces the canonical one (a/c), and the two are then compared asstrings. Two places were affected:
Config paths containing a
%placeholder%.NeonAdapter::process()guards itsnormalizePath(absolutizePath($val))call with!str_contains($val, '%'), because the placeholderis only expanded later by the DI compiler. Every
expandRelativePathskey was affected -scanFiles,bootstrapFiles,stubFiles,paths,excludePaths,ignoreErrorspaths,tmpDir,and the rest - not just the
scanFilesentry from the report.Paths PHPStan does not author.
ResultCachePathTransformer::absolutizePath()normalizes(it must -
relativizePath()emits../..prefixes that have to collapse against the anchor), butthe values PHPStan recomputes each run are whatever the source hands it. Composer's
vendor/composer/installed.phprecordsinstall_pathas__DIR__ . '/../foo/bar', socomposerInstalleddiffered on every run in every project with at least one installed package.That difference was silently absorbed by the changed-packages fallback, which is why it never showed
up as a discarded cache - it only printed "Composer metadata changed but no package versions changed"
on each run and re-resolved the package set for nothing.
Test
tests/PHPStan/Command/CommandHelperTest.phpgets adataParameters()case backed by the newtests/PHPStan/Command/relative-paths/placeholder-dots.neon, which spellsbootstrapFiles,scanFiles,scanDirectories,paths, bothexcludePathskeys and bothignoreErrorspath formsthrough
%rootDir%/.../nested/../..., and asserts every resulting container parameter is normalized.Verified to fail without the new extension (
.../relative-paths/nested/../here.phpvs.../relative-paths/here.php).tests/PHPStan/Analyser/ResultCache/ResultCachePathTransformerTest.php(new) pins the invariant thecanonicalization relies on: the relativize/absolutize round trip collapses
..segments, andabsolutizeMeta()is that round trip's fixed point foranalysedPaths,scannedFiles,executedFilesHashes,composerLocksand theinstall_pathvalues insidecomposerInstalled.e2e/result-cache-dots-in-path(new e2e fixture + step ine2e-tests.yml) reproduces the report:scanFiles,bootstrapFilesand a load-bearingstubFilesentry all written through a..segment.Without the fixes the second run prints
Result cache not used because the metadata do not match: scannedFiles, executedFilesHashes; withthem it restores 0 files.
result-cache-package-updatee2e step gains an unchanged-run assertion thatComposer metadata changedis not printed. Verified to fail before thegetMeta()change.Fixes phpstan/phpstan#15125