Skip to content

Commit 8b27943

Browse files
committed
Calling to a constructor with promoted properties has side effects
1 parent 2132cc0 commit 8b27943

10 files changed

+52
-18
lines changed

Diff for: src/Analyser/MutatingScope.php

+2
Original file line numberDiff line numberDiff line change
@@ -2969,6 +2969,7 @@ public function enterClassMethod(
29692969
array $parameterOutTypes = [],
29702970
array $immediatelyInvokedCallableParameters = [],
29712971
array $phpDocClosureThisTypeParameters = [],
2972+
bool $isConstructor = false,
29722973
): self
29732974
{
29742975
if (!$this->isInClass()) {
@@ -2999,6 +3000,7 @@ public function enterClassMethod(
29993000
array_map(fn (Type $type): Type => $this->transformStaticType(TemplateTypeHelper::toArgument($type)), $parameterOutTypes),
30003001
$immediatelyInvokedCallableParameters,
30013002
array_map(fn (Type $type): Type => $this->transformStaticType(TemplateTypeHelper::toArgument($type)), $phpDocClosureThisTypeParameters),
3003+
$isConstructor,
30023004
),
30033005
!$classMethod->isStatic(),
30043006
);

Diff for: src/Analyser/NodeScopeResolver.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,9 @@ private function processStmtNode(
614614
$nodeCallback($stmt->returnType, $scope);
615615
}
616616

617+
$isFromTrait = $stmt->getAttribute('originalTraitMethodName') === '__construct';
618+
$isConstructor = $isFromTrait || $stmt->name->toLowerString() === '__construct';
619+
617620
$methodScope = $scope->enterClassMethod(
618621
$stmt,
619622
$templateTypeMap,
@@ -632,14 +635,14 @@ private function processStmtNode(
632635
$phpDocParameterOutTypes,
633636
$phpDocImmediatelyInvokedCallableParameters,
634637
$phpDocClosureThisTypeParameters,
638+
$isConstructor,
635639
);
636640

637641
if (!$scope->isInClass()) {
638642
throw new ShouldNotHappenException();
639643
}
640644

641-
$isFromTrait = $stmt->getAttribute('originalTraitMethodName') === '__construct';
642-
if ($isFromTrait || $stmt->name->toLowerString() === '__construct') {
645+
if ($isConstructor) {
643646
foreach ($stmt->params as $param) {
644647
if ($param->flags === 0) {
645648
continue;

Diff for: src/Reflection/Php/PhpMethodFromParserNodeReflection.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,14 @@ public function __construct(
6060
array $parameterOutTypes,
6161
array $immediatelyInvokedCallableParameters,
6262
array $phpDocClosureThisTypeParameters,
63+
private bool $isConstructor,
6364
)
6465
{
6566
$name = strtolower($classMethod->name->name);
66-
if (in_array($name, ['__construct', '__destruct', '__unset', '__wakeup', '__clone'], true)) {
67+
if ($this->isConstructor) {
68+
$realReturnType = new VoidType();
69+
}
70+
if (in_array($name, ['__destruct', '__unset', '__wakeup', '__clone'], true)) {
6771
$realReturnType = new VoidType();
6872
}
6973
if ($name === '__tostring') {
@@ -175,6 +179,11 @@ public function isAbstract(): TrinaryLogic
175179
return TrinaryLogic::createFromBoolean($this->getClassMethod()->isAbstract());
176180
}
177181

182+
public function isConstructor(): bool
183+
{
184+
return $this->isConstructor;
185+
}
186+
178187
public function hasSideEffects(): TrinaryLogic
179188
{
180189
if (

Diff for: src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use PHPStan\Collectors\Collector;
88
use PHPStan\Node\MethodReturnStatementsNode;
99
use function count;
10-
use function strtolower;
1110

1211
/**
1312
* @implements Collector<MethodReturnStatementsNode, string>
@@ -23,7 +22,7 @@ public function getNodeType(): string
2322
public function processNode(Node $node, Scope $scope)
2423
{
2524
$method = $node->getMethodReflection();
26-
if (strtolower($method->getName()) !== '__construct') {
25+
if (!$method->isConstructor()) {
2726
return null;
2827
}
2928

Diff for: src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php

+1-5
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,7 @@ public function processNode(Node $node, Scope $scope)
4949
return null;
5050
}
5151

52-
$declaringClass = $method->getDeclaringClass();
53-
if (
54-
$declaringClass->hasConstructor()
55-
&& $declaringClass->getConstructor()->getName() === $method->getName()
56-
) {
52+
if ($method->isConstructor()) {
5753
return null;
5854
}
5955

Diff for: src/Rules/Pure/FunctionPurityCheck.php

+1-8
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,11 @@ public function check(
4040
array $impurePoints,
4141
array $throwPoints,
4242
array $statements,
43+
bool $isConstructor,
4344
): array
4445
{
4546
$errors = [];
4647
$isPure = $functionReflection->isPure();
47-
$isConstructor = false;
48-
if (
49-
$functionReflection instanceof ExtendedMethodReflection
50-
&& $functionReflection->getDeclaringClass()->hasConstructor()
51-
&& $functionReflection->getDeclaringClass()->getConstructor()->getName() === $functionReflection->getName()
52-
) {
53-
$isConstructor = true;
54-
}
5548

5649
if ($isPure->yes()) {
5750
foreach ($parameters as $parameter) {

Diff for: src/Rules/Pure/PureFunctionRule.php

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function processNode(Node $node, Scope $scope): array
3636
$node->getImpurePoints(),
3737
$node->getStatementResult()->getThrowPoints(),
3838
$node->getStatements(),
39+
false,
3940
);
4041
}
4142

Diff for: src/Rules/Pure/PureMethodRule.php

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function processNode(Node $node, Scope $scope): array
3636
$node->getImpurePoints(),
3737
$node->getStatementResult()->getThrowPoints(),
3838
$node->getStatements(),
39+
$method->isConstructor(),
3940
);
4041
}
4142

Diff for: tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php

+8
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ public function testBug11011(): void
7272
]);
7373
}
7474

75+
public function testBug12379(): void
76+
{
77+
if (PHP_VERSION_ID < 80000) {
78+
$this->markTestSkipped('Test requires PHP 8.1.');
79+
}
80+
$this->analyse([__DIR__ . '/data/bug-12379.php'], []);
81+
}
82+
7583
protected function getCollectors(): array
7684
{
7785
return [

Diff for: tests/PHPStan/Rules/DeadCode/data/bug-12379.php

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug12379;
4+
5+
class HelloWorld
6+
{
7+
use myTrait{
8+
myTrait::__construct as private __myTraitConstruct;
9+
}
10+
11+
public function __construct(
12+
int $entityManager
13+
){
14+
$this->__myTraitConstruct($entityManager);
15+
}
16+
}
17+
18+
trait myTrait{
19+
public function __construct(
20+
private readonly int $entityManager
21+
){}
22+
}

0 commit comments

Comments
 (0)