ImpossibleInstanceOfRule - respect reportAlwaysTrueInLastCondition

Author: ondrejmirtes

conf/config.level4.neon

@@ -37,6 +37,7 @@ services:
 		arguments:
 			checkAlwaysTrueInstanceof: %checkAlwaysTrueInstanceof%
 			treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
+			reportAlwaysTrueInLastCondition: %reportAlwaysTrueInLastCondition%
 		tags:
 			- phpstan.rules.rule
 

src/Rules/Classes/ImpossibleInstanceOfRule.php

@@ -24,6 +24,7 @@ class ImpossibleInstanceOfRule implements Rule
 	public function __construct(
 		private bool $checkAlwaysTrueInstanceof,
 		private bool $treatPhpDocTypesAsCertain,
+		private bool $reportAlwaysTrueInLastCondition,
 	)
 	{
 	}
@@ -84,19 +85,22 @@ public function processNode(Node $node, Scope $scope): array
 				)))->build(),
 			];
 		} elseif ($this->checkAlwaysTrueInstanceof) {
-			if ($node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME) === true) {
+			$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
+			if ($isLast === true && !$this->reportAlwaysTrueInLastCondition) {
 				return [];
 			}
 
 			$exprType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->expr) : $scope->getNativeType($node->expr);
+			$errorBuilder = $addTip(RuleErrorBuilder::message(sprintf(
+				'Instanceof between %s and %s will always evaluate to true.',
+				$exprType->describe(VerbosityLevel::typeOnly()),
+				$classType->describe(VerbosityLevel::getRecommendedLevelByType($classType)),
+			)));
+			if ($isLast === false && !$this->reportAlwaysTrueInLastCondition) {
+				$errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.');
+			}
 
-			return [
-				$addTip(RuleErrorBuilder::message(sprintf(
-					'Instanceof between %s and %s will always evaluate to true.',
-					$exprType->describe(VerbosityLevel::typeOnly()),
-					$classType->describe(VerbosityLevel::getRecommendedLevelByType($classType)),
-				)))->build(),
-			];
+			return [$errorBuilder->build()];
 		}
 
 		return [];

tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

@@ -16,9 +16,11 @@ class ImpossibleInstanceOfRuleTest extends RuleTestCase
 
 	private bool $treatPhpDocTypesAsCertain;
 
+	private bool $reportAlwaysTrueInLastCondition = false;
+
 	protected function getRule(): Rule
 	{
-		return new ImpossibleInstanceOfRule($this->checkAlwaysTrueInstanceOf, $this->treatPhpDocTypesAsCertain);
+		return new ImpossibleInstanceOfRule($this->checkAlwaysTrueInstanceOf, $this->treatPhpDocTypesAsCertain, $this->reportAlwaysTrueInLastCondition);
 	}
 
 	protected function shouldTreatPhpDocTypesAsCertain(): bool
@@ -378,10 +380,12 @@ public function testBug8042(): void
 			[
 				'Instanceof between Bug8042\B and Bug8042\B will always evaluate to true.',
 				18,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 			[
 				'Instanceof between Bug8042\B and Bug8042\B will always evaluate to true.',
 				26,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 		]);
 	}
@@ -417,6 +421,7 @@ public function testUnreachableIfBranches(): void
 			[
 				'Instanceof between stdClass and stdClass will always evaluate to true.',
 				37,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 		]);
 	}
@@ -433,10 +438,12 @@ public function testIfBranchesDoNotReportPhpDoc(): void
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				26,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				36,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 		]);
 	}
@@ -454,10 +461,12 @@ public function testIfBranchesReportPhpDoc(): void
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				26,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				36,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
@@ -467,10 +476,12 @@ public function testIfBranchesReportPhpDoc(): void
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				52,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 			[
 				'Instanceof between UnreachableIfBranchesNotPhpDoc\Foo and UnreachableIfBranchesNotPhpDoc\Foo will always evaluate to true.',
 				62,
+				'Remove remaining cases below this one and this error will disappear too.',
 			],
 		]);
 	}
@@ -541,4 +552,37 @@ public function testBug4689(): void
 		$this->analyse([__DIR__ . '/data/bug-4689.php'], []);
 	}
 
+	public function dataReportAlwaysTrueInLastCondition(): iterable
+	{
+		yield [false, [
+			[
+				'Instanceof between Exception and Exception will always evaluate to true.',
+				21,
+				'Remove remaining cases below this one and this error will disappear too.',
+			],
+		]];
+		yield [true, [
+			[
+				'Instanceof between Exception and Exception will always evaluate to true.',
+				12,
+			],
+			[
+				'Instanceof between Exception and Exception will always evaluate to true.',
+				21,
+			],
+		]];
+	}
+
+	/**
+	 * @dataProvider dataReportAlwaysTrueInLastCondition
+	 * @param list<array{0: string, 1: int, 2?: string}> $expectedErrors
+	 */
+	public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLastCondition, array $expectedErrors): void
+	{
+		$this->checkAlwaysTrueInstanceOf = true;
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->reportAlwaysTrueInLastCondition = $reportAlwaysTrueInLastCondition;
+		$this->analyse([__DIR__ . '/data/impossible-instanceof-report-always-true-last-condition.php'], $expectedErrors);
+	}
+
 }

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

@@ -0,0 +1,28 @@
+<?php
+
+namespace ImpossibleInstanceOfReportAlwaysTrueLastCondition;
+
+class Foo
+{
+
+	public function doFoo(\Exception $e)
+	{
+		if (rand(0, 1)) {
+
+		} elseif ($e instanceof \Exception) {
+
+		}
+	}
+
+	public function doBar(\Exception $e)
+	{
+		if (rand(0, 1)) {
+
+		} elseif ($e instanceof \Exception) {
+
+		} else {
+
+		}
+	}
+
+}