Skip to content

Commit cd037c5

Browse files
authored
Detect private promoted properties whose only reads are self-writes via new self() (#5599)
1 parent e94cf05 commit cd037c5

26 files changed

Lines changed: 355 additions & 43 deletions

src/Reflection/Type/IntersectionTypeUnresolvedPropertyPrototypeReflection.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ final class IntersectionTypeUnresolvedPropertyPrototypeReflection implements Unr
1818
* @param UnresolvedPropertyPrototypeReflection[] $propertyPrototypes
1919
*/
2020
public function __construct(
21-
private string $propertyName,
2221
private array $propertyPrototypes,
2322
)
2423
{
@@ -30,7 +29,7 @@ public function doNotResolveTemplateTypeMapToBounds(): UnresolvedPropertyPrototy
3029
return $this->cachedDoNotResolveTemplateTypeMapToBounds;
3130
}
3231

33-
return $this->cachedDoNotResolveTemplateTypeMapToBounds = new self($this->propertyName, array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->doNotResolveTemplateTypeMapToBounds(), $this->propertyPrototypes));
32+
return $this->cachedDoNotResolveTemplateTypeMapToBounds = new self(array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->doNotResolveTemplateTypeMapToBounds(), $this->propertyPrototypes));
3433
}
3534

3635
public function getNakedProperty(): ExtendedPropertyReflection
@@ -50,7 +49,7 @@ public function getTransformedProperty(): ExtendedPropertyReflection
5049

5150
public function withFechedOnType(Type $type): UnresolvedPropertyPrototypeReflection
5251
{
53-
return new self($this->propertyName, array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->withFechedOnType($type), $this->propertyPrototypes));
52+
return new self(array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->withFechedOnType($type), $this->propertyPrototypes));
5453
}
5554

5655
}

src/Reflection/Type/UnionTypeUnresolvedPropertyPrototypeReflection.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ final class UnionTypeUnresolvedPropertyPrototypeReflection implements Unresolved
1818
* @param UnresolvedPropertyPrototypeReflection[] $propertyPrototypes
1919
*/
2020
public function __construct(
21-
private string $propertyName,
2221
private array $propertyPrototypes,
2322
)
2423
{
@@ -29,7 +28,7 @@ public function doNotResolveTemplateTypeMapToBounds(): UnresolvedPropertyPrototy
2928
if ($this->cachedDoNotResolveTemplateTypeMapToBounds !== null) {
3029
return $this->cachedDoNotResolveTemplateTypeMapToBounds;
3130
}
32-
return $this->cachedDoNotResolveTemplateTypeMapToBounds = new self($this->propertyName, array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->doNotResolveTemplateTypeMapToBounds(), $this->propertyPrototypes));
31+
return $this->cachedDoNotResolveTemplateTypeMapToBounds = new self(array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->doNotResolveTemplateTypeMapToBounds(), $this->propertyPrototypes));
3332
}
3433

3534
public function getNakedProperty(): ExtendedPropertyReflection
@@ -50,7 +49,7 @@ public function getTransformedProperty(): ExtendedPropertyReflection
5049

5150
public function withFechedOnType(Type $type): UnresolvedPropertyPrototypeReflection
5251
{
53-
return new self($this->propertyName, array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->withFechedOnType($type), $this->propertyPrototypes));
52+
return new self(array_map(static fn (UnresolvedPropertyPrototypeReflection $prototype): UnresolvedPropertyPrototypeReflection => $prototype->withFechedOnType($type), $this->propertyPrototypes));
5453
}
5554

5655
}

src/Rules/Comparison/ImpossibleCheckTypeHelper.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,10 @@
4444
final class ImpossibleCheckTypeHelper
4545
{
4646

47-
/**
48-
* @param string[] $universalObjectCratesClasses
49-
*/
5047
public function __construct(
5148
private ReflectionProvider $reflectionProvider,
5249
private TypeSpecifier $typeSpecifier,
5350
#[AutowiredParameter]
54-
private array $universalObjectCratesClasses,
55-
#[AutowiredParameter]
5651
private bool $treatPhpDocTypesAsCertain,
5752
)
5853
{
@@ -417,7 +412,6 @@ public function doNotTreatPhpDocTypesAsCertain(): self
417412
return new self(
418413
$this->reflectionProvider,
419414
$this->typeSpecifier,
420-
$this->universalObjectCratesClasses,
421415
false,
422416
);
423417
}

src/Rules/DeadCode/UnusedPrivatePropertyRule.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
use PHPStan\DependencyInjection\AutowiredParameter;
88
use PHPStan\DependencyInjection\RegisteredRule;
99
use PHPStan\Node\ClassPropertiesNode;
10+
use PHPStan\Node\ClassPropertyNode;
1011
use PHPStan\Node\Property\PropertyRead;
12+
use PHPStan\Reflection\MethodReflection;
1113
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
1214
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
1315
use PHPStan\Rules\Rule;
@@ -120,6 +122,7 @@ public function processNode(Node $node, Scope $scope): array
120122
'node' => $property,
121123
'onlyReadable' => $property->isReadable() && !$property->isWritable(),
122124
'onlyWritable' => $property->isWritable() && !$property->isReadable(),
125+
'hasTrueRead' => $alwaysRead,
123126
];
124127
}
125128

@@ -194,6 +197,7 @@ public function processNode(Node $node, Scope $scope): array
194197
if (!$classType->isSuperTypeOf($fetchedOnType)->no()) {
195198
if ($usage instanceof PropertyRead) {
196199
$properties[$propertyName]['read'] = true;
200+
$properties[$propertyName]['hasTrueRead'] = true;
197201
} else {
198202
$properties[$propertyName]['written'] = true;
199203
}
@@ -204,6 +208,7 @@ public function processNode(Node $node, Scope $scope): array
204208
if (!$classType->isSuperTypeOf($fetchedOnType)->no()) {
205209
if ($usage instanceof PropertyRead) {
206210
$properties[$propertyName]['read'] = true;
211+
$properties[$propertyName]['hasTrueRead'] = true;
207212
} else {
208213
$properties[$propertyName]['written'] = true;
209214
}
@@ -213,12 +218,25 @@ public function processNode(Node $node, Scope $scope): array
213218

214219
if ($usage instanceof PropertyRead) {
215220
$properties[$propertyName]['read'] = true;
221+
if (!$this->isPropertySelfWrite($usageScope, $propertyName, $propertyNode, $classReflection->getName())) {
222+
$properties[$propertyName]['hasTrueRead'] = true;
223+
}
216224
} else {
217225
$properties[$propertyName]['written'] = true;
218226
}
219227
}
220228
}
221229

230+
foreach ($properties as $propertyName => $data) {
231+
if (!$data['read'] || $data['hasTrueRead']) {
232+
continue;
233+
}
234+
if (!$data['node']->isPromoted()) {
235+
continue;
236+
}
237+
$properties[$propertyName]['read'] = false;
238+
}
239+
222240
[$uninitializedProperties] = $node->getUninitializedProperties($scope, []);
223241

224242
$errors = [];
@@ -270,4 +288,42 @@ public function processNode(Node $node, Scope $scope): array
270288
return $errors;
271289
}
272290

291+
private function isPropertySelfWrite(
292+
Scope $usageScope,
293+
string $propertyName,
294+
ClassPropertyNode $propertyNode,
295+
string $className,
296+
): bool
297+
{
298+
if (!$propertyNode->isPromoted()) {
299+
return false;
300+
}
301+
302+
$callStack = $usageScope->getFunctionCallStackWithParameters();
303+
if ($callStack === []) {
304+
return false;
305+
}
306+
307+
$lastCall = $callStack[count($callStack) - 1];
308+
[$calleeReflection, $parameterReflection] = $lastCall;
309+
310+
if (!$calleeReflection instanceof MethodReflection) {
311+
return false;
312+
}
313+
314+
if ($calleeReflection->getName() !== '__construct') {
315+
return false;
316+
}
317+
318+
if ($calleeReflection->getDeclaringClass()->getName() !== $className) {
319+
return false;
320+
}
321+
322+
if ($parameterReflection === null) {
323+
return false;
324+
}
325+
326+
return $parameterReflection->getName() === $propertyName;
327+
}
328+
273329
}

src/Type/IntersectionType.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ public function getUnresolvedPropertyPrototype(string $propertyName, ClassMember
615615
return $propertyPrototypes[0];
616616
}
617617

618-
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
618+
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
619619
}
620620

621621
public function hasInstanceProperty(string $propertyName): TrinaryLogic
@@ -648,7 +648,7 @@ public function getUnresolvedInstancePropertyPrototype(string $propertyName, Cla
648648
return $propertyPrototypes[0];
649649
}
650650

651-
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
651+
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
652652
}
653653

654654
public function hasStaticProperty(string $propertyName): TrinaryLogic
@@ -681,7 +681,7 @@ public function getUnresolvedStaticPropertyPrototype(string $propertyName, Class
681681
return $propertyPrototypes[0];
682682
}
683683

684-
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
684+
return new IntersectionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
685685
}
686686

687687
public function canCallMethods(): TrinaryLogic

src/Type/ObjectType.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ public function getUnresolvedPropertyPrototype(string $propertyName, ClassMember
214214
return $properties[0];
215215
}
216216

217-
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyName, $properties);
217+
return new UnionTypeUnresolvedPropertyPrototypeReflection($properties);
218218
}
219219
}
220220
}
@@ -320,7 +320,7 @@ public function getUnresolvedInstancePropertyPrototype(string $propertyName, Cla
320320
return $properties[0];
321321
}
322322

323-
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyName, $properties);
323+
return new UnionTypeUnresolvedPropertyPrototypeReflection($properties);
324324
}
325325
}
326326
}

src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,10 @@ final class TypeSpecifyingFunctionsDynamicReturnTypeExtension implements Dynamic
2525

2626
private ?ImpossibleCheckTypeHelper $helper = null;
2727

28-
/**
29-
* @param string[] $universalObjectCratesClasses
30-
*/
3128
public function __construct(
3229
private ReflectionProvider $reflectionProvider,
3330
#[AutowiredParameter]
3431
private bool $treatPhpDocTypesAsCertain,
35-
#[AutowiredParameter]
36-
private array $universalObjectCratesClasses,
3732
)
3833
{
3934
}
@@ -76,7 +71,7 @@ public function getTypeFromFunctionCall(
7671

7772
private function getHelper(): ImpossibleCheckTypeHelper
7873
{
79-
return $this->helper ??= new ImpossibleCheckTypeHelper($this->reflectionProvider, $this->typeSpecifier, $this->universalObjectCratesClasses, $this->treatPhpDocTypesAsCertain);
74+
return $this->helper ??= new ImpossibleCheckTypeHelper($this->reflectionProvider, $this->typeSpecifier, $this->treatPhpDocTypesAsCertain);
8075
}
8176

8277
}

src/Type/UnionType.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ public function getUnresolvedPropertyPrototype(string $propertyName, ClassMember
521521
return $propertyPrototypes[0];
522522
}
523523

524-
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
524+
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
525525
}
526526

527527
public function hasInstanceProperty(string $propertyName): TrinaryLogic
@@ -554,7 +554,7 @@ public function getUnresolvedInstancePropertyPrototype(string $propertyName, Cla
554554
return $propertyPrototypes[0];
555555
}
556556

557-
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
557+
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
558558
}
559559

560560
public function hasStaticProperty(string $propertyName): TrinaryLogic
@@ -587,7 +587,7 @@ public function getUnresolvedStaticPropertyPrototype(string $propertyName, Class
587587
return $propertyPrototypes[0];
588588
}
589589

590-
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyName, $propertyPrototypes);
590+
return new UnionTypeUnresolvedPropertyPrototypeReflection($propertyPrototypes);
591591
}
592592

593593
public function canCallMethods(): TrinaryLogic

tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ protected function getRule(): Rule
2323
new ImpossibleCheckTypeHelper(
2424
self::createReflectionProvider(),
2525
$this->getTypeSpecifier(),
26-
[],
2726
$this->treatPhpDocTypesAsCertain,
2827
),
2928
$this->treatPhpDocTypesAsCertain,

tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ protected function getRule(): Rule
2323
new ImpossibleCheckTypeHelper(
2424
self::createReflectionProvider(),
2525
$this->getTypeSpecifier(),
26-
[],
2726
$this->treatPhpDocTypesAsCertain,
2827
),
2928
$this->treatPhpDocTypesAsCertain,

0 commit comments

Comments
 (0)