Skip to content

Commit

Permalink
MatchExpressionRule - use ConstantConditionRuleHelper to remove some …
Browse files Browse the repository at this point in the history
…duplicate errors
  • Loading branch information
ondrejmirtes committed Jan 12, 2023
1 parent 6e6037d commit 7380ed0
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 12 deletions.
14 changes: 13 additions & 1 deletion src/Rules/Comparison/MatchExpressionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class MatchExpressionRule implements Rule
{

public function __construct(
private ConstantConditionRuleHelper $constantConditionRuleHelper,
private bool $checkAlwaysTrueStrictComparison,
private bool $disableUnreachable,
private bool $reportAlwaysTrueInLastCondition,
Expand All @@ -41,6 +42,7 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
$matchCondition = $node->getCondition();
$matchConditionType = $scope->getType($matchCondition);
$nextArmIsDead = false;
$errors = [];
$armsCount = count($node->getArms());
Expand All @@ -67,6 +69,17 @@ public function processNode(Node $node, Scope $scope): array
continue;
}

if ($armConditionResult->getValue()) {
$nextArmIsDead = true;
}

if ($matchConditionType instanceof ConstantBooleanType) {
$armConditionStandaloneResult = $this->constantConditionRuleHelper->getBooleanType($armConditionScope, $armCondition->getCondition());
if (!$armConditionStandaloneResult instanceof ConstantBooleanType) {
continue;
}
}

$armLine = $armCondition->getLine();
if (!$armConditionResult->getValue()) {
$errors[] = RuleErrorBuilder::message(sprintf(
Expand All @@ -75,7 +88,6 @@ public function processNode(Node $node, Scope $scope): array
$armConditionScope->getType($armCondition->getCondition())->describe(VerbosityLevel::value()),
))->line($armLine)->build();
} else {
$nextArmIsDead = true;
if ($this->checkAlwaysTrueStrictComparison) {
if ($i === $armsCount - 1 && !$this->reportAlwaysTrueInLastCondition) {
continue;
Expand Down
25 changes: 16 additions & 9 deletions tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,22 @@ class MatchExpressionRuleTest extends RuleTestCase

protected function getRule(): Rule
{
return new MatchExpressionRule(true, $this->disableUnreachable, $this->reportAlwaysTrueInLastCondition);
return new MatchExpressionRule(
new ConstantConditionRuleHelper(
new ImpossibleCheckTypeHelper(
$this->createReflectionProvider(),
$this->getTypeSpecifier(),
[],
$this->treatPhpDocTypesAsCertain,
true,
),
$this->treatPhpDocTypesAsCertain,
true,
),
true,
$this->disableUnreachable,
$this->reportAlwaysTrueInLastCondition,
);
}

protected function shouldTreatPhpDocTypesAsCertain(): bool
Expand Down Expand Up @@ -92,14 +107,6 @@ public function testRule(): void
'Match expression does not handle remaining values: 1|2|3',
78,
],
[
'Match arm comparison between true and false is always false.',
86,
],
[
'Match arm comparison between true and false is always false.',
92,
],
[
'Match expression does not handle remaining value: true',
90,
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Rules/Comparison/data/match-expr.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ public function doFoo(int $i): void
public function doBar(\Exception $e): void
{
match (true) {
$e instanceof \InvalidArgumentException, $e instanceof \InvalidArgumentException => true,
$e instanceof \InvalidArgumentException, $e instanceof \InvalidArgumentException => true, // reported by ImpossibleInstanceOfRule
default => null,
};

match (true) {
$e instanceof \InvalidArgumentException => true,
$e instanceof \InvalidArgumentException => true,
$e instanceof \InvalidArgumentException => true, // reported by ImpossibleInstanceOfRule
};
}

Expand Down

0 comments on commit 7380ed0

Please sign in to comment.