Do not make php-parser and phpdoc-parser properties public when inlining getters in the phar build

build/PHPStan/Build/InlineCallCollector.php

@@ -29,16 +29,21 @@
 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
  * every method call whose receiver type resolves to a single class and whose
  * callee body is one `return <expr>;`, 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<string, array<string, Stmt\ClassMethod|null>> */
 	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<string>|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

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<string> */
+	private array $protectedDirectories = [];
+
+	/**
+	 * @param list<string>|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;
+	}
+
 }

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 = "<?php\n\$a = \$foo->getBar();\n";
+		file_put_contents($caller, $source);
+		$calleeSource = "<?php\nclass Foo {\n\tprivate int \$bar = 1;\n}\n";
+		file_put_contents($callee, $calleeSource);
+
+		$start = strpos($source, '$foo->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);
+		}
+	}
+
 }