Skip to content

Commit

Permalink
LastConditionVisitor: condition followed by throw is marked as last
Browse files Browse the repository at this point in the history
  • Loading branch information
JanTvrdik authored and ondrejmirtes committed Sep 13, 2023
1 parent 78c6477 commit fc7c028
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 12 deletions.
41 changes: 40 additions & 1 deletion src/Parser/LastConditionVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ public function enterNode(Node $node): ?Node
if ($node instanceof Node\Stmt\If_ && $node->elseifs !== []) {
$lastElseIf = count($node->elseifs) - 1;

$elseIsMissingOrThrowing = $node->else === null
|| (count($node->else->stmts) === 1 && $node->else->stmts[0] instanceof Node\Stmt\Throw_);

foreach ($node->elseifs as $i => $elseif) {
$isLast = $i === $lastElseIf && $node->else === null;
$isLast = $i === $lastElseIf && $elseIsMissingOrThrowing;
$elseif->cond->setAttribute(self::ATTRIBUTE_NAME, $isLast);
}
}
Expand All @@ -38,6 +41,42 @@ public function enterNode(Node $node): ?Node
}
}

if (
$node instanceof Node\Stmt\Function_
|| $node instanceof Node\Stmt\ClassMethod
|| $node instanceof Node\Stmt\If_
|| $node instanceof Node\Stmt\ElseIf_
|| $node instanceof Node\Stmt\Else_
|| $node instanceof Node\Stmt\Case_
|| $node instanceof Node\Stmt\Catch_
|| $node instanceof Node\Stmt\Do_
|| $node instanceof Node\Stmt\Finally_
|| $node instanceof Node\Stmt\For_
|| $node instanceof Node\Stmt\Foreach_
|| $node instanceof Node\Stmt\Namespace_
|| $node instanceof Node\Stmt\TryCatch
|| $node instanceof Node\Stmt\While_
) {
$statements = $node->stmts ?? [];
$statementCount = count($statements);

if ($statementCount < 2) {
return null;
}

if (!$statements[$statementCount - 1] instanceof Node\Stmt\Throw_) {
return null;
}

if (!$statements[$statementCount - 2] instanceof Node\Stmt\If_ || $statements[$statementCount - 2]->else !== null) {
return null;
}

$if = $statements[$statementCount - 2];
$cond = count($if->elseifs) > 0 ? $if->elseifs[count($if->elseifs) - 1]->cond : $if->cond;
$cond->setAttribute(self::ATTRIBUTE_NAME, true);
}

return null;
}

Expand Down
4 changes: 4 additions & 0 deletions tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,10 @@ public function dataReportAlwaysTrueInLastCondition(): iterable
'Instanceof between Exception and Exception will always evaluate to true.',
21,
],
[
'Instanceof between DateTime and DateTime will always evaluate to true.',
34,
],
]];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,17 @@ public function doBar(\Exception $e)
}
}

public function cloneDateTime(\DateTimeInterface $dateTime): \DateTimeImmutable
{
if ($dateTime instanceof \DateTimeImmutable) {
return $dateTime;
}

if ($dateTime instanceof \DateTime) {
return \DateTimeImmutable::createFromMutable($dateTime);
}

throw new \LogicException('Unknown class of DateTimeInterface implementation.');
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -945,19 +945,9 @@ public function testEnumTips(): void

$this->checkAlwaysTrueStrictComparison = true;
$this->analyse([__DIR__ . '/data/strict-comparison-enum-tips.php'], [
[
'Strict comparison using === between $this(StrictComparisonEnumTips\SomeEnum)&StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
15,
"• Remove remaining cases below this one and this error will disappear too.\n• Use match expression instead. PHPStan will report unhandled enum cases.",
],
[
'Strict comparison using === between $this(StrictComparisonEnumTips\SomeEnum)&StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
29,
'Use match expression instead. PHPStan will report unhandled enum cases.',
],
[
'Strict comparison using === between StrictComparisonEnumTips\SomeEnum::Two and StrictComparisonEnumTips\SomeEnum::Two will always evaluate to true.',
50,
52,
'Remove remaining cases below this one and this error will disappear too.',
],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ enum SomeEnum

public function exhaustiveWithSafetyCheck(): int
{
// not reported by this rule at all
if ($this === self::One) {
return -1;
} elseif ($this === self::Two) {
Expand All @@ -22,6 +23,7 @@ public function exhaustiveWithSafetyCheck(): int

public function exhaustiveWithSafetyCheck2(): int
{
// not reported by this rule at all
if ($this === self::One) {
return -1;
}
Expand Down

0 comments on commit fc7c028

Please sign in to comment.