Skip to content

Commit ce1c164

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MenQLNmxZKVgpFuzRRTBGW
1 parent 175e3fb commit ce1c164

3 files changed

Lines changed: 143 additions & 2 deletions

File tree

‎build/PHPStan/Build/InlineCallCollector.php‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,21 @@
2929
use function in_array;
3030
use function is_string;
3131
use function json_decode;
32+
use function realpath;
33+
use function str_starts_with;
3234
use function strtolower;
3335
use function substr;
36+
use const DIRECTORY_SEPARATOR;
3437

3538
/**
3639
* Source-level inliner for the phar build (compiler's PrepareCommand): for
3740
* every method call whose receiver type resolves to a single class and whose
3841
* callee body is one `return <expr>;`, records a textual replacement of the
3942
* call with that expression ($this and parameters substituted), plus the
4043
* non-public properties the expression reads (they become public for the
41-
* rewrite to run — see InlineEditsApplier in the compiler).
44+
* rewrite to run — see InlineEditsApplier in the compiler). Properties of
45+
* php-parser and phpdoc-parser are never made public
46+
* (PROTECTED_PACKAGE_DIRECTORIES): a body reading one is left as a call.
4247
*
4348
* Callees are ones no subclass can override — final class, final or private
4449
* method — or, closed-world, non-final ones nothing in the scanned code base
@@ -53,6 +58,18 @@ final class InlineCallCollector implements Collector
5358

5459
private const MAX_EXPR_NODES = 30;
5560

61+
/**
62+
* Packages the phar ships under their own, unprefixed namespaces and that
63+
* projects install on their own too — the copy loaded at run time may be
64+
* the project's, whose properties are still non-public. Their properties
65+
* are never made public; better-reflection is PHPStan's own fork.
66+
* Relative to the repository root; InlineEditsApplier keeps the same list.
67+
*/
68+
private const PROTECTED_PACKAGE_DIRECTORIES = [
69+
'vendor/nikic/php-parser',
70+
'vendor/phpstan/phpdoc-parser',
71+
];
72+
5673
/** @var array<string, array<string, Stmt\ClassMethod|null>> */
5774
private array $methodNodes = [];
5875

@@ -215,6 +232,9 @@ public function processNode(Node $node, Scope $scope): ?array
215232
continue;
216233
}
217234
foreach ($this->publicizeTargets($propertyDeclaringClass->getName(), $propertyName) as $target) {
235+
if ($target['file'] !== null && $this->isInProtectedPackage($target['file'])) {
236+
return null;
237+
}
218238
$publicize[] = $target;
219239
}
220240
}
@@ -511,6 +531,29 @@ private function publicizeTargets(string $className, string $propertyName): arra
511531
return $targets;
512532
}
513533

534+
/** @var list<string>|null */
535+
private ?array $protectedDirectories = null;
536+
537+
private function isInProtectedPackage(string $file): bool
538+
{
539+
if ($this->protectedDirectories === null) {
540+
$this->protectedDirectories = [];
541+
foreach (self::PROTECTED_PACKAGE_DIRECTORIES as $directory) {
542+
$directory = dirname(__DIR__, 3) . '/' . $directory;
543+
$realDirectory = realpath($directory);
544+
$this->protectedDirectories[] = ($realDirectory === false ? $directory : $realDirectory) . DIRECTORY_SEPARATOR;
545+
}
546+
}
547+
$realFile = realpath($file);
548+
foreach ($this->protectedDirectories as $directory) {
549+
if (str_starts_with($realFile === false ? $file : $realFile, $directory)) {
550+
return true;
551+
}
552+
}
553+
554+
return false;
555+
}
556+
514557
/**
515558
* The code the phar holds and runs, as far as inlining reaches into it:
516559
* PHPStan itself and the three vendor packages on its hot paths (the

‎compiler/src/InlineEditsApplier.php‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,28 @@
66
use function array_keys;
77
use function array_reverse;
88
use function count;
9+
use function dirname;
910
use function file_get_contents;
1011
use function file_put_contents;
1112
use function json_decode;
1213
use function preg_quote;
1314
use function preg_replace_callback;
15+
use function realpath;
1416
use function sprintf;
17+
use function str_starts_with;
1518
use function substr;
1619
use function usort;
20+
use const DIRECTORY_SEPARATOR;
1721
use const JSON_THROW_ON_ERROR;
1822

1923
/**
2024
* Applies the call-site edits InlineCallCollector gathered (build/inline.neon)
2125
* to the sources in place, then makes the non-public properties the inlined
2226
* bodies read public — a property read moved from its class into a caller
23-
* needs to be accessible there.
27+
* needs to be accessible there. Never a property of php-parser or
28+
* phpdoc-parser (PROTECTED_PACKAGE_DIRECTORIES): the collector does not
29+
* record those, and an edit that would need one is refused here rather than
30+
* applied.
2431
*
2532
* Overlapping edits (a call inside an argument of another inlined call)
2633
* resolve outermost-wins: the outer replacement was printed from the
@@ -30,6 +37,37 @@
3037
final class InlineEditsApplier
3138
{
3239

40+
/**
41+
* Same list as InlineCallCollector::PROTECTED_PACKAGE_DIRECTORIES —
42+
* packages the phar ships unprefixed and projects install on their own
43+
* too, so the copy loaded at run time may be one with the properties
44+
* still non-public.
45+
*/
46+
private const PROTECTED_PACKAGE_DIRECTORIES = [
47+
'vendor/nikic/php-parser',
48+
'vendor/phpstan/phpdoc-parser',
49+
];
50+
51+
/** @var list<string> */
52+
private array $protectedDirectories = [];
53+
54+
/**
55+
* @param list<string>|null $protectedDirectories defaults to PROTECTED_PACKAGE_DIRECTORIES under the repository root
56+
*/
57+
public function __construct(?array $protectedDirectories = null)
58+
{
59+
if ($protectedDirectories === null) {
60+
$protectedDirectories = [];
61+
foreach (self::PROTECTED_PACKAGE_DIRECTORIES as $directory) {
62+
$protectedDirectories[] = dirname(__DIR__, 2) . '/' . $directory;
63+
}
64+
}
65+
foreach ($protectedDirectories as $directory) {
66+
$realDirectory = realpath($directory);
67+
$this->protectedDirectories[] = ($realDirectory === false ? $directory : $realDirectory) . DIRECTORY_SEPARATOR;
68+
}
69+
}
70+
3371
/**
3472
* @return array{edits: int, files: int, properties: int}
3573
*/
@@ -67,6 +105,9 @@ public function apply(string $editsJsonFile): array
67105
if ($property['file'] === null) {
68106
throw new ShouldNotHappenException(sprintf('No file for %s::$%s', $property['class'], $property['property']));
69107
}
108+
if ($this->isInProtectedPackage($property['file'])) {
109+
throw new ShouldNotHappenException(sprintf('Refusing to make %s::$%s public, it is declared in a protected package (%s)', $property['class'], $property['property'], $property['file']));
110+
}
70111
$publicize[$property['file']][$property['property']] = true;
71112
}
72113
}
@@ -107,4 +148,16 @@ public function apply(string $editsJsonFile): array
107148
return ['edits' => $applied, 'files' => count($byFile), 'properties' => $properties];
108149
}
109150

151+
private function isInProtectedPackage(string $file): bool
152+
{
153+
$realFile = realpath($file);
154+
foreach ($this->protectedDirectories as $directory) {
155+
if (str_starts_with($realFile === false ? $file : $realFile, $directory)) {
156+
return true;
157+
}
158+
}
159+
160+
return false;
161+
}
162+
110163
}

‎compiler/tests/InlineEditsApplierTest.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
namespace PHPStan\Compiler;
44

5+
use PHPStan\ShouldNotHappenException;
56
use PHPUnit\Framework\TestCase;
67
use function file_get_contents;
78
use function file_put_contents;
89
use function json_encode;
10+
use function mkdir;
11+
use function rmdir;
912
use function strlen;
1013
use function strpos;
1114
use function sys_get_temp_dir;
@@ -66,4 +69,46 @@ public function testApply(): void
6669
unlink($editsFile);
6770
}
6871

72+
public function testRefusesToPublicizeProtectedPackageProperty(): void
73+
{
74+
$vendor = tempnam(sys_get_temp_dir(), 'vendor');
75+
unlink($vendor);
76+
mkdir($vendor . '/acme/lib', 0777, true);
77+
$caller = tempnam(sys_get_temp_dir(), 'caller');
78+
$callee = $vendor . '/acme/lib/Foo.php';
79+
$source = "<?php\n\$a = \$foo->getBar();\n";
80+
file_put_contents($caller, $source);
81+
$calleeSource = "<?php\nclass Foo {\n\tprivate int \$bar = 1;\n}\n";
82+
file_put_contents($callee, $calleeSource);
83+
84+
$start = strpos($source, '$foo->getBar()');
85+
$edits = [
86+
[
87+
'file' => $caller,
88+
'start' => $start,
89+
'end' => $start + strlen('$foo->getBar()') - 1,
90+
'replacement' => '$foo->bar',
91+
'callee' => 'Foo::getBar',
92+
'publicize' => [['class' => 'Foo', 'property' => 'bar', 'file' => $callee]],
93+
],
94+
];
95+
$editsFile = tempnam(sys_get_temp_dir(), 'edits');
96+
file_put_contents($editsFile, json_encode($edits, JSON_THROW_ON_ERROR));
97+
98+
try {
99+
(new InlineEditsApplier([$vendor . '/acme']))->apply($editsFile);
100+
self::fail('Expected the property to be refused');
101+
} catch (ShouldNotHappenException $e) {
102+
self::assertStringContainsString('Refusing to make Foo::$bar public, it is declared in a protected package', $e->getMessage());
103+
} finally {
104+
self::assertSame($calleeSource, file_get_contents($callee));
105+
unlink($caller);
106+
unlink($callee);
107+
unlink($editsFile);
108+
rmdir($vendor . '/acme/lib');
109+
rmdir($vendor . '/acme');
110+
rmdir($vendor);
111+
}
112+
}
113+
69114
}

0 commit comments

Comments
 (0)