diff --git a/build/PHPStan/Build/InlineCallCollector.php b/build/PHPStan/Build/InlineCallCollector.php index 7c068325c1..1c883ea281 100644 --- a/build/PHPStan/Build/InlineCallCollector.php +++ b/build/PHPStan/Build/InlineCallCollector.php @@ -29,8 +29,11 @@ use function in_array; use function is_string; use function json_decode; +use function realpath; +use function str_starts_with; use function strtolower; use function substr; +use const DIRECTORY_SEPARATOR; /** * Source-level inliner for the phar build (compiler's PrepareCommand): for @@ -38,7 +41,9 @@ * callee body is one `return ;`, records a textual replacement of the * call with that expression ($this and parameters substituted), plus the * non-public properties the expression reads (they become public for the - * rewrite to run — see InlineEditsApplier in the compiler). + * rewrite to run — see InlineEditsApplier in the compiler). Properties of + * php-parser and phpdoc-parser are never made public + * (PROTECTED_PACKAGE_DIRECTORIES): a body reading one is left as a call. * * Callees are ones no subclass can override — final class, final or private * method — or, closed-world, non-final ones nothing in the scanned code base @@ -53,6 +58,18 @@ final class InlineCallCollector implements Collector private const MAX_EXPR_NODES = 30; + /** + * Packages the phar ships under their own, unprefixed namespaces and that + * projects install on their own too — the copy loaded at run time may be + * the project's, whose properties are still non-public. Their properties + * are never made public; better-reflection is PHPStan's own fork. + * Relative to the repository root; InlineEditsApplier keeps the same list. + */ + private const PROTECTED_PACKAGE_DIRECTORIES = [ + 'vendor/nikic/php-parser', + 'vendor/phpstan/phpdoc-parser', + ]; + /** @var array> */ private array $methodNodes = []; @@ -215,6 +232,9 @@ public function processNode(Node $node, Scope $scope): ?array continue; } foreach ($this->publicizeTargets($propertyDeclaringClass->getName(), $propertyName) as $target) { + if ($target['file'] !== null && $this->isInProtectedPackage($target['file'])) { + return null; + } $publicize[] = $target; } } @@ -511,6 +531,29 @@ private function publicizeTargets(string $className, string $propertyName): arra return $targets; } + /** @var list|null */ + private ?array $protectedDirectories = null; + + private function isInProtectedPackage(string $file): bool + { + if ($this->protectedDirectories === null) { + $this->protectedDirectories = []; + foreach (self::PROTECTED_PACKAGE_DIRECTORIES as $directory) { + $directory = dirname(__DIR__, 3) . '/' . $directory; + $realDirectory = realpath($directory); + $this->protectedDirectories[] = ($realDirectory === false ? $directory : $realDirectory) . DIRECTORY_SEPARATOR; + } + } + $realFile = realpath($file); + foreach ($this->protectedDirectories as $directory) { + if (str_starts_with($realFile === false ? $file : $realFile, $directory)) { + return true; + } + } + + return false; + } + /** * The code the phar holds and runs, as far as inlining reaches into it: * PHPStan itself and the three vendor packages on its hot paths (the diff --git a/compiler/src/InlineEditsApplier.php b/compiler/src/InlineEditsApplier.php index 2815b1d593..a727e6484c 100644 --- a/compiler/src/InlineEditsApplier.php +++ b/compiler/src/InlineEditsApplier.php @@ -6,21 +6,28 @@ use function array_keys; use function array_reverse; use function count; +use function dirname; use function file_get_contents; use function file_put_contents; use function json_decode; use function preg_quote; use function preg_replace_callback; +use function realpath; use function sprintf; +use function str_starts_with; use function substr; use function usort; +use const DIRECTORY_SEPARATOR; use const JSON_THROW_ON_ERROR; /** * Applies the call-site edits InlineCallCollector gathered (build/inline.neon) * to the sources in place, then makes the non-public properties the inlined * bodies read public — a property read moved from its class into a caller - * needs to be accessible there. + * needs to be accessible there. Never a property of php-parser or + * phpdoc-parser (PROTECTED_PACKAGE_DIRECTORIES): the collector does not + * record those, and an edit that would need one is refused here rather than + * applied. * * Overlapping edits (a call inside an argument of another inlined call) * resolve outermost-wins: the outer replacement was printed from the @@ -30,6 +37,37 @@ final class InlineEditsApplier { + /** + * Same list as InlineCallCollector::PROTECTED_PACKAGE_DIRECTORIES — + * packages the phar ships unprefixed and projects install on their own + * too, so the copy loaded at run time may be one with the properties + * still non-public. + */ + private const PROTECTED_PACKAGE_DIRECTORIES = [ + 'vendor/nikic/php-parser', + 'vendor/phpstan/phpdoc-parser', + ]; + + /** @var list */ + private array $protectedDirectories = []; + + /** + * @param list|null $protectedDirectories defaults to PROTECTED_PACKAGE_DIRECTORIES under the repository root + */ + public function __construct(?array $protectedDirectories = null) + { + if ($protectedDirectories === null) { + $protectedDirectories = []; + foreach (self::PROTECTED_PACKAGE_DIRECTORIES as $directory) { + $protectedDirectories[] = dirname(__DIR__, 2) . '/' . $directory; + } + } + foreach ($protectedDirectories as $directory) { + $realDirectory = realpath($directory); + $this->protectedDirectories[] = ($realDirectory === false ? $directory : $realDirectory) . DIRECTORY_SEPARATOR; + } + } + /** * @return array{edits: int, files: int, properties: int} */ @@ -67,6 +105,9 @@ public function apply(string $editsJsonFile): array if ($property['file'] === null) { throw new ShouldNotHappenException(sprintf('No file for %s::$%s', $property['class'], $property['property'])); } + if ($this->isInProtectedPackage($property['file'])) { + throw new ShouldNotHappenException(sprintf('Refusing to make %s::$%s public, it is declared in a protected package (%s)', $property['class'], $property['property'], $property['file'])); + } $publicize[$property['file']][$property['property']] = true; } } @@ -107,4 +148,16 @@ public function apply(string $editsJsonFile): array return ['edits' => $applied, 'files' => count($byFile), 'properties' => $properties]; } + private function isInProtectedPackage(string $file): bool + { + $realFile = realpath($file); + foreach ($this->protectedDirectories as $directory) { + if (str_starts_with($realFile === false ? $file : $realFile, $directory)) { + return true; + } + } + + return false; + } + } diff --git a/compiler/tests/InlineEditsApplierTest.php b/compiler/tests/InlineEditsApplierTest.php index 2418ce3131..baddcb9353 100644 --- a/compiler/tests/InlineEditsApplierTest.php +++ b/compiler/tests/InlineEditsApplierTest.php @@ -2,10 +2,13 @@ namespace PHPStan\Compiler; +use PHPStan\ShouldNotHappenException; use PHPUnit\Framework\TestCase; use function file_get_contents; use function file_put_contents; use function json_encode; +use function mkdir; +use function rmdir; use function strlen; use function strpos; use function sys_get_temp_dir; @@ -66,4 +69,46 @@ public function testApply(): void unlink($editsFile); } + public function testRefusesToPublicizeProtectedPackageProperty(): void + { + $vendor = tempnam(sys_get_temp_dir(), 'vendor'); + unlink($vendor); + mkdir($vendor . '/acme/lib', 0777, true); + $caller = tempnam(sys_get_temp_dir(), 'caller'); + $callee = $vendor . '/acme/lib/Foo.php'; + $source = "getBar();\n"; + file_put_contents($caller, $source); + $calleeSource = "getBar()'); + $edits = [ + [ + 'file' => $caller, + 'start' => $start, + 'end' => $start + strlen('$foo->getBar()') - 1, + 'replacement' => '$foo->bar', + 'callee' => 'Foo::getBar', + 'publicize' => [['class' => 'Foo', 'property' => 'bar', 'file' => $callee]], + ], + ]; + $editsFile = tempnam(sys_get_temp_dir(), 'edits'); + file_put_contents($editsFile, json_encode($edits, JSON_THROW_ON_ERROR)); + + try { + (new InlineEditsApplier([$vendor . '/acme']))->apply($editsFile); + self::fail('Expected the property to be refused'); + } catch (ShouldNotHappenException $e) { + self::assertStringContainsString('Refusing to make Foo::$bar public, it is declared in a protected package', $e->getMessage()); + } finally { + self::assertSame($calleeSource, file_get_contents($callee)); + unlink($caller); + unlink($callee); + unlink($editsFile); + rmdir($vendor . '/acme/lib'); + rmdir($vendor . '/acme'); + rmdir($vendor); + } + } + }