From 071015835830220bbda6ae25ab74f8a4bd387077 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 10 Sep 2026 14:30:29 +0200 Subject: [PATCH] Do not make php-parser and phpdoc-parser properties public when inlining getters in the phar build The phar ships both packages under their own, unprefixed namespaces, and projects install them on their own too, so the copy loaded at run time may be one whose properties are still non-public. The collector skips a call site whenever the inlined body would need such a property made public, and the applier refuses such a target instead of stamping it. better-reflection is PHPStan's own fork and keeps being inlined; call sites inside the two packages and their getters reading public members are still inlined. Inlined call sites 9,005 -> 8,426, properties made public 851 -> 823 (the 28 php-parser and phpdoc-parser ones gone), rewritten files 1,039 -> 976. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MenQLNmxZKVgpFuzRRTBGW --- build/PHPStan/Build/InlineCallCollector.php | 45 ++++++++++++++++- compiler/src/InlineEditsApplier.php | 55 ++++++++++++++++++++- compiler/tests/InlineEditsApplierTest.php | 45 +++++++++++++++++ 3 files changed, 143 insertions(+), 2 deletions(-) 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); + } + } + }