diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index fac692020a3..4be574fcbc1 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -606,10 +606,27 @@ jobs: OUTPUT=$(../bashunit -a exit_code "0" "../../bin/phpstan analyse -vv") echo "$OUTPUT" ../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT" + - script: | + cd e2e/result-cache-dots-in-path + # scanFiles/bootstrapFiles/stubFiles built out of %rootDir% with a '..' segment in the + # middle: the placeholder keeps NeonAdapter from normalizing them when the Neon file is + # read, so nothing may compare that spelling against the normalized one. + ../../bin/phpstan analyse + OUTPUT=$(../bashunit -a exit_code "0" "../../bin/phpstan analyse -vv") + echo "$OUTPUT" + ../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT" - script: | cd e2e/result-cache-package-update composer install ../../bin/phpstan analyse + # Nothing changed at all: the cache must be reused without even reaching the + # changed-packages fallback. Composer spells install_path in installed.php with a '..' + # segment ('vendor/composer/../test/logger'), so a meta that is not canonicalized the + # same way it is restored reports composerInstalled as changed on every single run. + OUTPUT=$(../bashunit -a exit_code "0" "../../bin/phpstan analyse -vv") + echo "$OUTPUT" + ../bashunit -a not_contains 'Composer metadata changed' "$OUTPUT" + ../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT" # Update a single Composer package by bumping its version with a patch and letting Composer # reinstall it, which changes its recorded reference in vendor/composer/installed.php. The # cache must be invalidated only for files depending on that package (src/Foo.php uses diff --git a/e2e/result-cache-dots-in-path/.gitignore b/e2e/result-cache-dots-in-path/.gitignore new file mode 100644 index 00000000000..ceeb05b4108 --- /dev/null +++ b/e2e/result-cache-dots-in-path/.gitignore @@ -0,0 +1 @@ +/tmp diff --git a/e2e/result-cache-dots-in-path/bootstrap.php b/e2e/result-cache-dots-in-path/bootstrap.php new file mode 100644 index 00000000000..3c6b2651747 --- /dev/null +++ b/e2e/result-cache-dots-in-path/bootstrap.php @@ -0,0 +1 @@ +greet('PHPStan'); + } + +} diff --git a/e2e/result-cache-dots-in-path/stubs/ScannedGreeter.stub b/e2e/result-cache-dots-in-path/stubs/ScannedGreeter.stub new file mode 100644 index 00000000000..b0efc4f00ab --- /dev/null +++ b/e2e/result-cache-dots-in-path/stubs/ScannedGreeter.stub @@ -0,0 +1,11 @@ + */ private array $fileHashes = []; @@ -1588,7 +1587,12 @@ private function getMeta(array $allAnalysedFiles, ?array $projectConfigArray): a ksort($projectConfigArray); } - return [ + // The meta is stored relativized and comes back absolutized, which also normalizes every path + // in it ('a/b/../c' becomes 'a/c'). Not every path here is normalized to begin with - Composer + // records install_path as 'vendor/composer/../foo/bar' in installed.php - so the freshly computed + // meta is put through the very same transformation. Otherwise the restored meta could never equal + // it and the cache would be discarded on every single run. + return $this->getPathTransformer()->absolutizeMeta([ 'cacheVersion' => self::CACHE_VERSION, 'phpstanVersion' => ComposerHelper::getPhpStanVersion(), 'metaExtensions' => $this->getMetaFromPhpStanExtensions(), @@ -1605,9 +1609,9 @@ private function getMeta(array $allAnalysedFiles, ?array $projectConfigArray): a // extensions may only run after the bootstrapFiles. This entry catches a changed // list in any config file, including the included ones that are not part of the // projectConfig entry above. - 'configStubFiles' => array_map(fn (string $stubFile): string => $this->fileHelper->normalizePath($stubFile), $this->configStubFiles), + 'configStubFiles' => $this->configStubFiles, 'level' => $this->usedLevel, - ]; + ]); } private function getFileHash(string $path): string diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index 5f89864098e..84bfb5c3cfe 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -181,7 +181,7 @@ public static function begin( $autoloadFunctionsBefore = spl_autoload_functions(); if ($autoloadFile !== null) { - $autoloadFile = $currentWorkingDirectoryFileHelper->absolutizePath($autoloadFile); + $autoloadFile = $currentWorkingDirectoryFileHelper->normalizePath($currentWorkingDirectoryFileHelper->absolutizePath($autoloadFile)); if (!is_file($autoloadFile)) { $errorOutput->writeLineFormatted(sprintf('Autoload file "%s" not found.', $autoloadFile)); throw new InceptionNotSuccessfulException(); @@ -209,7 +209,7 @@ public static function begin( } } } else { - $projectConfigFile = $currentWorkingDirectoryFileHelper->absolutizePath($projectConfigFile); + $projectConfigFile = $currentWorkingDirectoryFileHelper->normalizePath($currentWorkingDirectoryFileHelper->absolutizePath($projectConfigFile)); } if ($generateBaselineFile !== null) { diff --git a/src/DependencyInjection/NormalizePathParametersExtension.php b/src/DependencyInjection/NormalizePathParametersExtension.php new file mode 100644 index 00000000000..c154584dd48 --- /dev/null +++ b/src/DependencyInjection/NormalizePathParametersExtension.php @@ -0,0 +1,135 @@ +getContainerBuilder(); + $fileHelper = new FileHelper($builder->parameters['currentWorkingDirectory']); + + foreach ($this->getExpandRelativePaths() as $configKey) { + if (preg_match_all('~\[([^\]]*)\]~', $configKey, $matches) === 0) { + continue; + } + + $segments = $matches[1]; + if (array_shift($segments) !== 'parameters') { + continue; + } + + $parameters = $builder->parameters; + $this->normalizeAtPath($parameters, $segments, $fileHelper); + $builder->parameters = $parameters; + } + } + + /** + * @return list + */ + private function getExpandRelativePaths(): array + { + $configKeys = []; + foreach ($this->compiler->getExtensions(ExpandRelativePathExtension::class) as $extension) { + foreach ($extension->getConfig() as $configKey) { + if (!is_string($configKey)) { + continue; + } + + $configKeys[] = $configKey; + } + } + + return $configKeys; + } + + /** + * @param mixed[] $value + * @param list $segments an empty segment stands for "every element of this list" + */ + private function normalizeAtPath(array &$value, array $segments, FileHelper $fileHelper): void + { + $segment = array_shift($segments); + if ($segment === null) { + return; + } + + if ($segment === '') { + foreach ($value as $key => $item) { + $value[$key] = $this->normalizeItem($item, $segments, $fileHelper); + } + + return; + } + + if (!array_key_exists($segment, $value)) { + return; + } + + $value[$segment] = $this->normalizeItem($value[$segment], $segments, $fileHelper); + } + + /** + * @param list $segments + */ + private function normalizeItem(mixed $item, array $segments, FileHelper $fileHelper): mixed + { + if ($segments !== []) { + if (is_array($item)) { + $this->normalizeAtPath($item, $segments, $fileHelper); + } + + return $item; + } + + if ($item instanceof OptionalPath) { + return new OptionalPath($this->normalizePath($item->path, $fileHelper)); + } + + if (!is_string($item)) { + return $item; + } + + return $this->normalizePath($item, $fileHelper); + } + + private function normalizePath(string $path, FileHelper $fileHelper): string + { + // A path that is still relative here is either an fnmatch pattern or came from a placeholder that + // expanded to a relative value; there is no config file left to resolve it against, and + // normalizePath() would silently drop its leading '..' segments. absolutizePath() returning the + // path unchanged is what "already absolute" means to the rest of PHPStan, including `scheme://` URLs. + if ($fileHelper->absolutizePath($path) !== $path) { + return $path; + } + + return $fileHelper->normalizePath($path); + } + +} diff --git a/tests/PHPStan/Analyser/ResultCache/ResultCachePathTransformerTest.php b/tests/PHPStan/Analyser/ResultCache/ResultCachePathTransformerTest.php new file mode 100644 index 00000000000..2f7985b30a0 --- /dev/null +++ b/tests/PHPStan/Analyser/ResultCache/ResultCachePathTransformerTest.php @@ -0,0 +1,82 @@ +createTransformer(); + $path = '/project/vendor/composer/../nette/neon/src/Neon.php'; + + $this->assertSame( + '/project/vendor/nette/neon/src/Neon.php', + $transformer->absolutizePath($transformer->relativizePath($path)), + ); + } + + /** + * Composer records install_path in vendor/composer/installed.php as `vendor/composer/../foo/bar`, + * so the meta the result cache stores is not normalized to begin with. Restoring it collapses those + * `..` segments, which is why ResultCacheManager::getMeta() puts the freshly computed meta through + * the very same transformation - otherwise the restored meta could never equal the computed one and + * the cache would be discarded on every run. + */ + public function testAbsolutizeMetaIsTheCanonicalFormOfTheRoundTrip(): void + { + $transformer = $this->createTransformer(); + $meta = [ + 'analysedPaths' => ['/project/src/../src'], + 'scannedFiles' => ['/project/vendor/phpstan/phpstan/../../../vendor/autoload.php' => 'hash'], + 'executedFilesHashes' => ['/project/vendor/../vendor/autoload.php' => 'hash'], + 'composerLocks' => ['/project/./composer.lock' => 'hash'], + 'composerInstalled' => [ + '/project/vendor/composer/installed.php' => [ + 'versions' => [ + 'nette/neon' => ['install_path' => '/project/vendor/composer/../nette/neon'], + ], + ], + ], + ]; + + $canonical = $transformer->absolutizeMeta($meta); + + $this->assertSame([ + 'analysedPaths' => ['/project/src'], + 'scannedFiles' => ['/project/vendor/autoload.php' => 'hash'], + 'executedFilesHashes' => ['/project/vendor/autoload.php' => 'hash'], + 'composerLocks' => ['/project/composer.lock' => 'hash'], + 'composerInstalled' => [ + '/project/vendor/composer/installed.php' => [ + 'versions' => [ + 'nette/neon' => ['install_path' => '/project/vendor/nette/neon'], + ], + ], + ], + ], $canonical); + + $this->assertSame($canonical, $transformer->absolutizeMeta($transformer->relativizeMeta($meta))); + $this->assertSame($canonical, $transformer->absolutizeMeta($canonical)); + } + +} diff --git a/tests/PHPStan/Command/CommandHelperTest.php b/tests/PHPStan/Command/CommandHelperTest.php index a9c2c8414ee..779ac8f6a1f 100644 --- a/tests/PHPStan/Command/CommandHelperTest.php +++ b/tests/PHPStan/Command/CommandHelperTest.php @@ -283,6 +283,52 @@ public static function dataParameters(): array ], ], ], + [ + // paths built out of a %placeholder% are not expanded when the Neon file is read, so + // they reach the container with their '..' segments intact unless they get normalized + // after the expansion - https://github.com/phpstan/phpstan/issues/15125 + __DIR__ . '/relative-paths/placeholder-dots.neon', + [ + 'bootstrapFiles' => [ + realpath(__DIR__ . '/../../../stubs/runtime/ReflectionUnionType.php'), + realpath(__DIR__ . '/../../../stubs/runtime/ReflectionAttribute.php'), + realpath(__DIR__ . '/../../../stubs/runtime/Attribute85.php'), + realpath(__DIR__ . '/../../../stubs/runtime/ReflectionIntersectionType.php'), + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'here.php', + ], + 'scanFiles' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'here.php', + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'test' . DIRECTORY_SEPARATOR . 'there.php', + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'up.php', + ], + 'scanDirectories' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'src', + ], + 'paths' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'src', + ], + 'excludePaths' => [ + 'analyseAndScan' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'test', + ], + 'analyse' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'src', + ], + ], + 'ignoreErrors' => [ + [ + 'message' => '#aaa#', + 'path' => __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'src' . DIRECTORY_SEPARATOR . 'aaa.php', + ], + [ + 'message' => '#bbb#', + 'paths' => [ + __DIR__ . DIRECTORY_SEPARATOR . 'relative-paths' . DIRECTORY_SEPARATOR . 'src' . DIRECTORY_SEPARATOR . 'bbb.php', + ], + ], + ], + ], + ], ]; } diff --git a/tests/PHPStan/Command/relative-paths/placeholder-dots.neon b/tests/PHPStan/Command/relative-paths/placeholder-dots.neon new file mode 100644 index 00000000000..8f1727a1089 --- /dev/null +++ b/tests/PHPStan/Command/relative-paths/placeholder-dots.neon @@ -0,0 +1,25 @@ +parameters: + bootstrapFiles: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../here.php + scanFiles: + - %rootDir%/tests/PHPStan/Command/relative-paths/../relative-paths/here.php + - %rootDir%/tests/PHPStan/Command/relative-paths/test/../test/there.php + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../up.php + scanDirectories: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../src + paths: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../src + excludePaths: + analyse: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../src + analyseAndScan: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../test + reportUnmatchedIgnoredErrors: false + ignoreErrors: + - + message: '#aaa#' + path: %rootDir%/tests/PHPStan/Command/relative-paths/nested/../src/aaa.php + - + message: '#bbb#' + paths: + - %rootDir%/tests/PHPStan/Command/relative-paths/nested/../src/bbb.php