Skip to content

Commit fc7c028

Browse files
JanTvrdikondrejmirtes
authored andcommitted
LastConditionVisitor: condition followed by throw is marked as last
1 parent 78c6477 commit fc7c028

5 files changed

+60
-12
lines changed

Diff for: src/Parser/LastConditionVisitor.php

+40-1
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ public function enterNode(Node $node): ?Node
1717
if ($node instanceof Node\Stmt\If_ && $node->elseifs !== []) {
1818
$lastElseIf = count($node->elseifs) - 1;
1919

20+
$elseIsMissingOrThrowing = $node->else === null
21+
|| (count($node->else->stmts) === 1 && $node->else->stmts[0] instanceof Node\Stmt\Throw_);
22+
2023
foreach ($node->elseifs as $i => $elseif) {
21-
$isLast = $i === $lastElseIf && $node->else === null;
24+
$isLast = $i === $lastElseIf && $elseIsMissingOrThrowing;
2225
$elseif->cond->setAttribute(self::ATTRIBUTE_NAME, $isLast);
2326
}
2427
}
@@ -38,6 +41,42 @@ public function enterNode(Node $node): ?Node
3841
}
3942
}
4043

44+
if (
45+
$node instanceof Node\Stmt\Function_
46+
|| $node instanceof Node\Stmt\ClassMethod
47+
|| $node instanceof Node\Stmt\If_
48+
|| $node instanceof Node\Stmt\ElseIf_
49+
|| $node instanceof Node\Stmt\Else_
50+
|| $node instanceof Node\Stmt\Case_
51+
|| $node instanceof Node\Stmt\Catch_
52+
|| $node instanceof Node\Stmt\Do_
53+
|| $node instanceof Node\Stmt\Finally_
54+
|| $node instanceof Node\Stmt\For_
55+
|| $node instanceof Node\Stmt\Foreach_
56+
|| $node instanceof Node\Stmt\Namespace_
57+
|| $node instanceof Node\Stmt\TryCatch
58+
|| $node instanceof Node\Stmt\While_
59+
) {
60+
$statements = $node->stmts ?? [];
61+
$statementCount = count($statements);
62+
63+
if ($statementCount < 2) {
64+
return null;
65+
}
66+
67+
if (!$statements[$statementCount - 1] instanceof Node\Stmt\Throw_) {
68+
return null;
69+
}
70+
71+
if (!$statements[$statementCount - 2] instanceof Node\Stmt\If_ || $statements[$statementCount - 2]->else !== null) {
72+
return null;
73+
}
74+
75+
$if = $statements[$statementCount - 2];
76+
$cond = count($if->elseifs) > 0 ? $if->elseifs[count($if->elseifs) - 1]->cond : $if->cond;
77+
$cond->setAttribute(self::ATTRIBUTE_NAME, true);
78+
}
79+
4180
return null;
4281
}
4382

Diff for: tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

+4
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,10 @@ public function dataReportAlwaysTrueInLastCondition(): iterable
570570
'Instanceof between Exception and Exception will always evaluate to true.',
571571
21,
572572
],
573+
[
574+
'Instanceof between DateTime and DateTime will always evaluate to true.',
575+
34,
576+
],
573577
]];
574578
}
575579

Diff for: tests/PHPStan/Rules/Classes/data/impossible-instanceof-report-always-true-last-condition.php

+13
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,17 @@ public function doBar(\Exception $e)
2525
}
2626
}
2727

28+
public function cloneDateTime(\DateTimeInterface $dateTime): \DateTimeImmutable
29+
{
30+
if ($dateTime instanceof \DateTimeImmutable) {
31+
return $dateTime;
32+
}
33+
34+
if ($dateTime instanceof \DateTime) {
35+
return \DateTimeImmutable::createFromMutable($dateTime);
36+
}
37+
38+
throw new \LogicException('Unknown class of DateTimeInterface implementation.');
39+
}
40+
2841
}

Diff for: tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

+1-11
Original file line numberDiff line numberDiff line change
@@ -945,19 +945,9 @@ public function testEnumTips(): void
945945

946946
$this->checkAlwaysTrueStrictComparison = true;
947947
$this->analyse([__DIR__ . '/data/strict-comparison-enum-tips.php'], [
948-
[
949-
'Strict comparison using === between $this(StrictComparisonEnumTips\SomeEnum)&StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
950-
15,
951-
"• Remove remaining cases below this one and this error will disappear too.\n• Use match expression instead. PHPStan will report unhandled enum cases.",
952-
],
953-
[
954-
'Strict comparison using === between $this(StrictComparisonEnumTips\SomeEnum)&StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
955-
29,
956-
'Use match expression instead. PHPStan will report unhandled enum cases.',
957-
],
958948
[
959949
'Strict comparison using === between StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
960-
50,
950+
52,
961951
'Remove remaining cases below this one and this error will disappear too.',
962952
],
963953
]);

Diff for: tests/PHPStan/Rules/Comparison/data/strict-comparison-enum-tips.php

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ enum SomeEnum
1010

1111
public function exhaustiveWithSafetyCheck(): int
1212
{
13+
// not reported by this rule at all
1314
if ($this === self::One) {
1415
return -1;
1516
} elseif ($this === self::Two) {
@@ -22,6 +23,7 @@ public function exhaustiveWithSafetyCheck(): int
2223

2324
public function exhaustiveWithSafetyCheck2(): int
2425
{
26+
// not reported by this rule at all
2527
if ($this === self::One) {
2628
return -1;
2729
}

0 commit comments

Comments
 (0)