diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index f625cf36e3..84f234836b 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -147,6 +147,10 @@ jobs: cd e2e/bug-15102 composer install ../../bin/phpstan analyze + - script: | + cd e2e/bug-15102c + composer install + ../../bin/phpstan analyze - script: | cd e2e/bug-14724 composer install diff --git a/e2e/bug-15102c/.gitignore b/e2e/bug-15102c/.gitignore new file mode 100644 index 0000000000..3a9875b460 --- /dev/null +++ b/e2e/bug-15102c/.gitignore @@ -0,0 +1,2 @@ +/vendor/ +composer.lock diff --git a/e2e/bug-15102c/bootstrap.php b/e2e/bug-15102c/bootstrap.php new file mode 100644 index 0000000000..172119dbd4 --- /dev/null +++ b/e2e/bug-15102c/bootstrap.php @@ -0,0 +1,16 @@ +doFoo(); +}; diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocator.php index cf49e56013..ba554f1950 100644 --- a/src/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/AutoloadFunctionsSourceLocator.php @@ -10,15 +10,20 @@ use PHPStan\BetterReflection\SourceLocator\Type\SourceLocator; use function class_exists; use function function_exists; -use function get_included_files; -use function in_array; use function interface_exists; +use function opcache_invalidate; use function PHPStan\autoloadFunctions; use function PHPStan\autoloadFunctionsPrependedToComposer; use function restore_error_handler; use function set_error_handler; use function trait_exists; +/** + * Consults the autoload functions that bootstrap files registered - spl_autoload_register() + * callbacks that are not Composer's class loader. Asked for a class, such an autoloader either + * reads a file (which the file-read trap detects, so the class is located in it statically) or + * defines the class without one, through class_alias() or eval(). + */ final class AutoloadFunctionsSourceLocator implements SourceLocator { @@ -55,17 +60,36 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): return null; } - if (function_exists($className)) { - if ($this->wouldReIncludeALoadedFile($autoloadFunctions, $className)) { - return null; - } + $locatedFiles = $this->probeAutoloadFunctions($autoloadFunctions, $className); - // The trap intercepts file reads, not execution, so the probe ran the autoloaders for - // real. One that defines the class without reading a file - class_alias(), eval() - - // has already done its work, and calling it again would redeclare what it defined. - if (class_exists($className, false) || interface_exists($className, false) || trait_exists($className, false)) { - return $this->locateWithoutAutoloading($reflector, $identifier); - } + // An autoloader can define the class without reading any file - class_alias() with an + // already-loaded target, or eval(). The trap intercepts file reads, not execution, so + // such an autoloader has already done its work during the probe. + if (class_exists($className, false) || interface_exists($className, false) || trait_exists($className, false)) { + return $this->locateWithoutAutoloading($reflector, $identifier); + } + + if ($locatedFiles === []) { + return null; + } + + // The autoloaders asked for these files - locate the class in them statically, without + // executing anything. + $reflection = $this->autoloadSourceLocator->locateIdentifierInFiles($reflector, $identifier, $locatedFiles); + if ($reflection !== null) { + return $reflection; + } + + // The located files do not declare the class under this name. Running the autoloaders + // for real can still resolve it: class_alias() whose target Composer has to autoload + // first asks for the *target's* file, which never declares the alias name - Laravel's + // Redirect alias reads the file of Illuminate\Support\Facades\Redirect. But a real run + // is only safe when it cannot redeclare anything: a catch-all autoloader can resolve a + // class name to the file of an already-loaded function of the same name and fatally + // include it a second time, which is what + // https://github.com/phpstan/phpstan/issues/14988 reported. + if ($this->autoloadSourceLocator->wouldIncludingFilesRedeclareSymbols($locatedFiles)) { + return null; } foreach ($autoloadFunctions as $autoloadFunction) { @@ -91,22 +115,16 @@ private function locateWithoutAutoloading(Reflector $reflector, Identifier $iden } /** - * Whether running these autoloaders for $className would include a file that is loaded already. - * - * A function of this name exists, so an autoloader that maps names to paths - a catch-all one - * like PHP_CodeSniffer's, falling back to Composer's findFile() - can resolve this *class* name - * to the *function's* own file. Including that file a second time fatally redeclares the - * function, which is what https://github.com/phpstan/phpstan/issues/14988 reported. - * - * Probing under the file-read trap answers which file the autoloaders would read without - * executing it, so only that case is declined. Declining on the name alone would also block - * class names that merely coincide with a function - classes and functions live in separate - * symbol spaces, and Laravel's facade aliases (Cache, File, Str, ...) collide with the global - * helpers cache(), file() and str(). See https://github.com/phpstan/phpstan/issues/15102 + * Runs the autoload functions under the file-read trap and reports which files they asked + * for. No file content is executed - the trap serves empty data - so the probe is free of + * the side effects that make running bootstrap autoloaders for real hazardous. Mirrors + * spl_autoload_call() by stopping at the first autoloader that defines the name or asks + * for a file. * * @param array $autoloadFunctions + * @return string[] */ - private function wouldReIncludeALoadedFile(array $autoloadFunctions, string $className): bool + private function probeAutoloadFunctions(array $autoloadFunctions, string $className): array { set_error_handler(static fn (): bool => true); @@ -136,21 +154,17 @@ static function () use ($autoloadFunctions, $className): array { restore_error_handler(); } - if ($locatedFiles === []) { - return false; + if (!function_exists('opcache_invalidate')) { + return $locatedFiles; } - // PHP canonicalises the path before it reaches a stream wrapper - a `/./` segment, a - // symlinked directory or an include-path-relative name all arrive resolved - so the - // trapped paths compare directly against get_included_files(). - $includedFiles = get_included_files(); + // The pseudo-include may have cached the trap's empty content; running the autoloaders + // for real afterwards has to compile the actual file. foreach ($locatedFiles as $locatedFile) { - if (in_array($locatedFile, $includedFiles, true)) { - return true; - } + opcache_invalidate($locatedFile, true); } - return false; + return $locatedFiles; } #[Override] diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php index 3c43f17c7c..21f489eb59 100644 --- a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php @@ -168,6 +168,62 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): return null; } + /** + * Whether including these files for real would redeclare a class or function that is + * already defined in this process - which is fatal, so the caller must not execute them. + * The check is against the files' statically parsed symbols, not get_included_files(): + * a file-read-trap probe pseudo-includes every file it traps (the include succeeds with + * empty content and registers in get_included_files() while defining nothing), so the + * include list overreports what is genuinely loaded. + * + * @param string[] $files + */ + public function wouldIncludingFilesRedeclareSymbols(array $files): bool + { + foreach ($files as $file) { + if (!is_file($file)) { + continue; + } + + $result = $this->fileNodesFetcher->fetchNodes($file); + foreach (array_keys($result->getClassNodes()) as $className) { + if (class_exists($className, false) || interface_exists($className, false) || trait_exists($className, false)) { + return true; + } + } + + foreach (array_keys($result->getFunctionNodes()) as $functionName) { + if (function_exists($functionName)) { + return true; + } + } + } + + return false; + } + + /** + * Locates the identifier in the given files without invoking any autoloader - used by + * AutoloadFunctionsSourceLocator with the files its file-read-trap probe recorded. + * + * @param string[] $files + */ + public function locateIdentifierInFiles(Reflector $reflector, Identifier $identifier, array $files): ?Reflection + { + foreach ($files as $file) { + if (!is_file($file)) { + continue; + } + + $reflection = $this->findReflection($reflector, $file, $identifier, null); + if ($reflection !== null) { + return $reflection; + } + } + + return null; + } + private function findReflection(Reflector $reflector, string $file, Identifier $identifier, ?int $startLine): ?Reflection { $result = $this->fileNodesFetcher->fetchNodes($file);