Introduce LastConditionVisitor to prevent annoying last-condition errors

Author: staabm

conf/config.neon

@@ -488,6 +488,11 @@ services:
 		tags:
 			- phpstan.parser.richParserNodeVisitor
 
+	-
+		class: PHPStan\Parser\LastConditionVisitor
+		tags:
+			- phpstan.parser.richParserNodeVisitor
+
 	-
 		class: PhpParser\NodeVisitor\NodeConnectingVisitor
 

src/Parser/LastConditionVisitor.php

@@ -0,0 +1,33 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Parser;
+
+use PhpParser\Node;
+use PhpParser\NodeVisitorAbstract;
+use function count;
+
+class LastConditionVisitor extends NodeVisitorAbstract
+{
+
+	public const ATTRIBUTE_NAME = 'isLastCondition';
+
+	public function enterNode(Node $node): ?Node
+	{
+		if ($node instanceof Node\Stmt\If_ && $node->elseifs !== []) {
+			$lastElseIf = count($node->elseifs) - 1;
+
+			$node->elseifs[$lastElseIf]->cond->setAttribute(self::ATTRIBUTE_NAME, true);
+		}
+
+		if ($node instanceof Node\Expr\Match_ && $node->arms !== []) {
+			$lastArm = count($node->arms) - 1;
+
+			if ($node->arms[$lastArm]->conds !== null && $node->arms[$lastArm]->conds !== []) {
+				$node->arms[$lastArm]->conds[0]->setAttribute(self::ATTRIBUTE_NAME, true);
+			}
+		}
+
+		return null;
+	}
+
+}

src/Rules/Classes/ImpossibleInstanceOfRule.php

@@ -4,6 +4,7 @@
 
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
+use PHPStan\Parser\LastConditionVisitor;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\Constant\ConstantBooleanType;
@@ -81,6 +82,10 @@ public function processNode(Node $node, Scope $scope): array
 				)))->build(),
 			];
 		} elseif ($this->checkAlwaysTrueInstanceof) {
+			if ($node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME) === true) {
+				return [];
+			}
+
 			return [
 				$addTip(RuleErrorBuilder::message(sprintf(
 					'Instanceof between %s and %s will always evaluate to true.',

src/Rules/Comparison/ConstantConditionRuleHelper.php

@@ -6,6 +6,7 @@
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\MethodCall;
 use PHPStan\Analyser\Scope;
+use PHPStan\Parser\LastConditionVisitor;
 use PHPStan\Type\BooleanType;
 
 class ConstantConditionRuleHelper

src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php

@@ -4,6 +4,7 @@
 
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
+use PHPStan\Parser\LastConditionVisitor;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\Constant\ConstantBooleanType;
@@ -49,6 +50,10 @@ public function processNode(Node $node, Scope $scope): array
 				))->build(),
 			];
 		} elseif ($this->checkAlwaysTrueStrictComparison) {
+			if ($node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME) === true) {
+				return [];
+			}
+
 			return [
 				RuleErrorBuilder::message(sprintf(
 					'Strict comparison using %s between %s and %s will always evaluate to true.',

tests/PHPStan/Analyser/NodeScopeResolverTest.php

@@ -1149,6 +1149,9 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8467b.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8442.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/PDOStatement.php');
+		if (PHP_VERSION_ID >= 80000) {
+			yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-8485.php');
+		}
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/discussion-8447.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7805.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-82.php');

tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

@@ -4,6 +4,7 @@
 
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
+use const PHP_VERSION_ID;
 
 /**
  * @extends RuleTestCase<ImpossibleInstanceOfRule>
@@ -368,4 +369,24 @@ public function testBug5333(): void
 		]);
 	}
 
+	public function testBug8042(): void
+	{
+		if (PHP_VERSION_ID < 80000) {
+			$this->markTestSkipped('This test needs PHP 8.0');
+		}
+
+		$this->checkAlwaysTrueInstanceOf = true;
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-8042.php'], [
+			[
+				'Instanceof between Bug8042\B and Bug8042\B will always evaluate to true.',
+				18
+			],
+			[
+				'Instanceof between Bug8042\B and Bug8042\B will always evaluate to true.',
+				26
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Classes/data/bug-8042.php

@@ -0,0 +1,29 @@
+<?php // lint >= 8.0
+
+namespace Bug8042;
+
+class A {}
+class B {}
+
+function test(A|B $ab): string {
+	return match (true) {
+		$ab instanceof A => 'a',
+		$ab instanceof B => 'b',
+	};
+}
+
+function test2(A|B $ab): string {
+	return match (true) {
+		$ab instanceof A => 'a',
+		$ab instanceof B => 'b',
+		rand(0, 1) => 'never'
+	};
+}
+
+function test3(A|B $ab): string {
+	return match (true) {
+		$ab instanceof A => 'a',
+		$ab instanceof B => 'b',
+		default => 'never'
+	};
+}

tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php

@@ -140,15 +140,15 @@ public function testBug8485(): void
 		$this->analyse([__DIR__ . '/data/bug-8485.php'], [
 			[
 				'If condition is always true.',
-				19,
+				21,
 			],
 			[
 				'If condition is always false.',
-				24,
+				26,
 			],
 			[
 				'If condition is always false.',
-				29,
+				31,
 			],
 		]);
 	}

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

@@ -182,10 +182,6 @@ public function testStrictComparison(): void
 					'Strict comparison using === between int<10, max> and \'foo\' will always evaluate to false.',
 					635,
 				],
-				[
-					'Strict comparison using === between \'foofoofoofoofoofoof…\' and \'foofoofoofoofoofoof…\' will always evaluate to true.',
-					654,
-				],
 				[
 					'Strict comparison using === between string|null and 1 will always evaluate to false.',
 					685,
@@ -250,6 +246,10 @@ public function testStrictComparison(): void
 					'Strict comparison using !== between NAN and NAN will always evaluate to true.',
 					983,
 				],
+				[
+					'Strict comparison using === between \'foofoofoofoofoofoof…\' and \'foofoofoofoofoofoof…\' will always evaluate to true.',
+					996,
+				],
 			],
 		);
 	}
@@ -617,23 +617,27 @@ public function testBug8485(): void
 		$this->analyse([__DIR__ . '/data/bug-8485.php'], [
 			[
 				'Strict comparison using === between Bug8485\E::c and Bug8485\E::c will always evaluate to true.',
-				17,
+				19,
 			],
 			[
 				'Strict comparison using === between Bug8485\F::c and Bug8485\E::c will always evaluate to false.',
-				22,
+				24,
 			],
 			[
 				'Strict comparison using === between Bug8485\F::c and Bug8485\E::c will always evaluate to false.',
-				27,
+				29,
 			],
 			[
 				'Strict comparison using === between Bug8485\F and Bug8485\E will always evaluate to false.',
-				35,
+				36,
 			],
 			[
 				'Strict comparison using === between Bug8485\F and Bug8485\E::c will always evaluate to false.',
-				40,
+				41,
+			],
+			[
+				'Strict comparison using === between Bug8485\FooEnum::C and Bug8485\FooEnum::C will always evaluate to true.',
+				74,
 			],
 		]);
 	}