Skip to content

Commit 0d1fef1

Browse files
ondrejmirtesondrejmirtes
committed
Private property with @property-read is still writable inside the class
1 parent b69bb4a commit 0d1fef1

File tree

8 files changed

+99
-36
lines changed

8 files changed

+99
-36
lines changed

‎src/Reflection/ClassReflection.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PhpParser\Node\Identifier;
99
use PhpParser\Node\Name\FullyQualified;
1010
use PHPStan\Analyser\ArgumentsNormalizer;
11+
use PHPStan\Analyser\OutOfClassScope;
1112
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass;
1213
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum;
1314
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnumBackedCase;
@@ -505,7 +506,7 @@ public function hasInstanceProperty(string $propertyName): bool
505506
}
506507

507508
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
508-
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
509+
$property = $this->phpClassReflectionExtension->getNativeProperty($this, $propertyName);
509510
if (!$property->isStatic()) {
510511
return $this->hasInstancePropertyCache[$propertyName] = true;
511512
}
@@ -537,7 +538,7 @@ public function hasStaticProperty(string $propertyName): bool
537538
}
538539

539540
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
540-
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
541+
$property = $this->phpClassReflectionExtension->getNativeProperty($this, $propertyName);
541542
if ($property->isStatic()) {
542543
return $this->hasStaticPropertyCache[$propertyName] = true;
543544
}
@@ -733,7 +734,7 @@ public function getProperty(string $propertyName, ClassMemberAccessAnswerer $sco
733734
}
734735

735736
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
736-
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
737+
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName, $scope);
737738
if ($scope->canReadProperty($property)) {
738739
return $this->properties[$key] = $property;
739740
}
@@ -756,7 +757,7 @@ public function getProperty(string $propertyName, ClassMemberAccessAnswerer $sco
756757

757758
// For BC purpose
758759
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
759-
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
760+
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName, $scope);
760761

761762
return $this->properties[$key] = $property;
762763
}
@@ -787,7 +788,7 @@ public function getInstanceProperty(string $propertyName, ClassMemberAccessAnswe
787788
}
788789

789790
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
790-
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
791+
$property = $this->phpClassReflectionExtension->getProperty($this, $propertyName, $scope);
791792
if (!$property->isStatic()) {
792793
if ($scope->canReadProperty($property)) {
793794
return $this->instanceProperties[$key] = $property;
@@ -837,7 +838,7 @@ public function getStaticProperty(string $propertyName): ExtendedPropertyReflect
837838
}
838839

839840
if ($this->phpClassReflectionExtension->hasProperty($this, $propertyName)) {
840-
$nakedProperty = $this->phpClassReflectionExtension->getProperty($this, $propertyName);
841+
$nakedProperty = $this->phpClassReflectionExtension->getProperty($this, $propertyName, new OutOfClassScope());
841842
if ($nakedProperty->isStatic()) {
842843
$property = $this->wrapExtendedProperty($propertyName, $nakedProperty);
843844
if ($property->isStatic()) {

‎src/Reflection/Php/PhpClassReflectionExtension.php‎

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PhpParser\Node\Stmt\Declare_;
99
use PhpParser\Node\Stmt\Namespace_;
1010
use PHPStan\Analyser\NodeScopeResolver;
11+
use PHPStan\Analyser\OutOfClassScope;
1112
use PHPStan\Analyser\ScopeContext;
1213
use PHPStan\Analyser\ScopeFactory;
1314
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
@@ -21,6 +22,7 @@
2122
use PHPStan\Reflection\Annotations\AnnotationsPropertiesClassReflectionExtension;
2223
use PHPStan\Reflection\Assertions;
2324
use PHPStan\Reflection\AttributeReflectionFactory;
25+
use PHPStan\Reflection\ClassMemberAccessAnswerer;
2426
use PHPStan\Reflection\ClassReflection;
2527
use PHPStan\Reflection\Deprecation\DeprecationProvider;
2628
use PHPStan\Reflection\ExtendedFunctionVariant;
@@ -155,19 +157,23 @@ public function hasProperty(ClassReflection $classReflection, string $propertyNa
155157
return $classReflection->getNativeReflection()->hasProperty($propertyName);
156158
}
157159

158-
public function getProperty(ClassReflection $classReflection, string $propertyName): PhpPropertyReflection
160+
public function getProperty(ClassReflection $classReflection, string $propertyName, ClassMemberAccessAnswerer $scope): PhpPropertyReflection
159161
{
160-
if (!isset($this->propertiesIncludingAnnotations[$classReflection->getCacheKey()][$propertyName])) {
161-
$this->propertiesIncludingAnnotations[$classReflection->getCacheKey()][$propertyName] = $this->createProperty($classReflection, $propertyName, true);
162+
$cacheKey = $classReflection->getCacheKey();
163+
if ($scope->isInClass()) {
164+
$cacheKey = sprintf('%s-%s', $cacheKey, $scope->getClassReflection()->getCacheKey());
165+
}
166+
if (!isset($this->propertiesIncludingAnnotations[$cacheKey][$propertyName])) {
167+
$this->propertiesIncludingAnnotations[$cacheKey][$propertyName] = $this->createProperty($classReflection, $propertyName, $scope, true);
162168
}
163169

164-
return $this->propertiesIncludingAnnotations[$classReflection->getCacheKey()][$propertyName];
170+
return $this->propertiesIncludingAnnotations[$cacheKey][$propertyName];
165171
}
166172

167173
public function getNativeProperty(ClassReflection $classReflection, string $propertyName): PhpPropertyReflection
168174
{
169175
if (!isset($this->nativeProperties[$classReflection->getCacheKey()][$propertyName])) {
170-
$property = $this->createProperty($classReflection, $propertyName, false);
176+
$property = $this->createProperty($classReflection, $propertyName, new OutOfClassScope(), false);
171177
$this->nativeProperties[$classReflection->getCacheKey()][$propertyName] = $property;
172178
}
173179

@@ -177,6 +183,7 @@ public function getNativeProperty(ClassReflection $classReflection, string $prop
177183
private function createProperty(
178184
ClassReflection $classReflection,
179185
string $propertyName,
186+
ClassMemberAccessAnswerer $scope,
180187
bool $includingAnnotations,
181188
): PhpPropertyReflection
182189
{
@@ -213,7 +220,7 @@ private function createProperty(
213220
$types[] = $value;
214221
}
215222

216-
return new PhpPropertyReflection($declaringClassReflection, null, null, TypeCombinator::union(...$types), $classReflection->getNativeReflection()->getProperty($propertyName), null, null, null, false, false, false, false, [], false, true, false);
223+
return new PhpPropertyReflection($declaringClassReflection, null, null, TypeCombinator::union(...$types), $classReflection->getNativeReflection()->getProperty($propertyName), null, null, null, false, false, false, false, [], false, true, false, false, true);
217224
}
218225
}
219226

@@ -393,12 +400,34 @@ private function createProperty(
393400
}
394401
}
395402

403+
$nativeProperty = new PhpPropertyReflection(
404+
$declaringClassReflection,
405+
$declaringTrait,
406+
$nativeType,
407+
$phpDocType,
408+
$propertyReflection,
409+
$getHook,
410+
$setHook,
411+
$deprecatedDescription,
412+
$isDeprecated,
413+
$isInternal,
414+
$isReadOnlyByPhpDoc,
415+
$isAllowedPrivateMutation,
416+
$this->attributeReflectionFactory->fromNativeReflection($propertyReflection->getAttributes(), InitializerExprContext::fromClass($declaringClassReflection->getName(), $declaringClassReflection->getFileName())),
417+
$isFinal,
418+
true,
419+
true,
420+
$propertyReflection->isPrivate(),
421+
$propertyReflection->isPublic(),
422+
);
423+
396424
if (
397425
$includingAnnotations
398426
&& !$declaringClassReflection->isEnum()
399427
&& !$propertyReflection->isStatic()
400428
&& ($classReflection->allowsDynamicProperties() || !$propertyReflection->isPrivate())
401429
&& $this->annotationsPropertiesClassReflectionExtension->hasProperty($classReflection, $propertyName)
430+
&& (!$scope->canReadProperty($nativeProperty) || $nativeProperty->isPublic())
402431
) {
403432
$hierarchyDistances = $classReflection->getClassHierarchyDistances();
404433
$annotationProperty = $this->annotationsPropertiesClassReflectionExtension->getProperty($classReflection, $propertyName);
@@ -415,7 +444,9 @@ private function createProperty(
415444
throw new ShouldNotHappenException();
416445
}
417446

418-
if ($hierarchyDistances[$annotationProperty->getDeclaringClass()->getName()] <= $hierarchyDistances[$distanceDeclaringClass]) {
447+
if (
448+
$hierarchyDistances[$annotationProperty->getDeclaringClass()->getName()] <= $hierarchyDistances[$distanceDeclaringClass]
449+
) {
419450
return new PhpPropertyReflection(
420451
$annotationProperty->getDeclaringClass(),
421452
$declaringTrait,
@@ -433,28 +464,13 @@ private function createProperty(
433464
$isFinal,
434465
$annotationProperty->isReadable(),
435466
$annotationProperty->isWritable(),
467+
false,
468+
true,
436469
);
437470
}
438471
}
439472

440-
return new PhpPropertyReflection(
441-
$declaringClassReflection,
442-
$declaringTrait,
443-
$nativeType,
444-
$phpDocType,
445-
$propertyReflection,
446-
$getHook,
447-
$setHook,
448-
$deprecatedDescription,
449-
$isDeprecated,
450-
$isInternal,
451-
$isReadOnlyByPhpDoc,
452-
$isAllowedPrivateMutation,
453-
$this->attributeReflectionFactory->fromNativeReflection($propertyReflection->getAttributes(), InitializerExprContext::fromClass($declaringClassReflection->getName(), $declaringClassReflection->getFileName())),
454-
$isFinal,
455-
true,
456-
true,
457-
);
473+
return $nativeProperty;
458474
}
459475

460476
public function hasMethod(ClassReflection $classReflection, string $methodName): bool

‎src/Reflection/Php/PhpPropertyReflection.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public function __construct(
4747
private bool $isFinal,
4848
private bool $readable,
4949
private bool $writable,
50+
private bool $private,
51+
private bool $public,
5052
)
5153
{
5254
}
@@ -83,12 +85,12 @@ public function isStatic(): bool
8385

8486
public function isPrivate(): bool
8587
{
86-
return $this->reflection->isPrivate();
88+
return $this->private;
8789
}
8890

8991
public function isPublic(): bool
9092
{
91-
return $this->reflection->isPublic();
93+
return $this->public;
9294
}
9395

9496
public function isReadOnly(): bool

‎tests/PHPStan/Analyser/nsrt/bug-1216.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class Baz extends Foo
3434

3535
public function __construct()
3636
{
37-
assertType('string', $this->foo);
37+
assertType('int', $this->foo);
3838
assertType('string', $this->bar);
3939
assertType('string', $this->untypedBar);
4040
}

‎tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,21 @@ public function testTypesAssignedToProperties(): void
126126
public function testBug1216(): void
127127
{
128128
$this->analyse([__DIR__ . '/data/bug-1216.php'], [
129+
[
130+
'Property Bug1216PropertyTest\Foo::$foo (int) does not accept string.',
131+
36,
132+
],
129133
[
130134
'Property Bug1216PropertyTest\Baz::$untypedBar (string) does not accept int.',
131135
38,
132136
],
137+
[
138+
'Property Bug1216PropertyTest\Baz::$untypedBar (string) does not accept int.',
139+
46,
140+
],
133141
[
134142
'Property Bug1216PropertyTest\Dummy::$foo (Exception) does not accept stdClass.',
135-
62,
143+
68,
136144
],
137145
]);
138146
}

‎tests/PHPStan/Rules/Properties/WritingToReadOnlyPropertiesRuleTest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,11 @@ public function testBug11241(): void
137137
]);
138138
}
139139

140+
#[RequiresPhp('>= 8.0')]
141+
public function testBug13530(): void
142+
{
143+
$this->checkThisOnly = false;
144+
$this->analyse([__DIR__ . '/data/bug-13530.php'], []);
145+
}
146+
140147
}

‎tests/PHPStan/Rules/Properties/data/bug-1216.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,19 @@ class Baz extends Foo
3333

3434
public function __construct()
3535
{
36-
$this->foo = 'foo'; // OK
36+
$this->foo = 'foo'; // error because property is protected so in this scope "int" type is used
3737
$this->bar = 'bar'; // OK
3838
$this->untypedBar = 123; // error
3939
}
4040

4141
}
4242

43+
function (Baz $baz): void {
44+
$baz->foo = 'foo'; // OK
45+
$baz->bar = 'bar'; // OK
46+
$baz->untypedBar = 123; // error
47+
};
48+
4349
trait DecoratorTrait
4450
{
4551

‎tests/PHPStan/Rules/Properties/data/bug-13530.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug13530;
4+
5+
/** @property-read string $url */
6+
class HelloWorld
7+
{
8+
9+
private string $url;
10+
11+
public function __construct(string $url) {
12+
$this->url = $url;
13+
}
14+
15+
public function __get(string $key): mixed {
16+
if ($key === 'url') {
17+
return $this->url;
18+
}
19+
20+
return NULL;
21+
}
22+
23+
}

0 commit comments

Comments
 (0)