Show TypeResult reasons in StrictComparisonOfDifferentTypesRule

Author: ondrejmirtes

conf/config.neon

@@ -582,6 +582,9 @@ services:
 		arguments:
 			cacheFilePath: %resultCachePath%
 
+	-
+		class: PHPStan\Analyser\RicherScopeGetTypeHelper
+
 	-
 		class: PHPStan\Cache\Cache
 		arguments:

src/Analyser/DirectInternalScopeFactory.php

@@ -34,6 +34,7 @@ public function __construct(
 		private PropertyReflectionFinder $propertyReflectionFinder,
 		private Parser $parser,
 		private NodeScopeResolver $nodeScopeResolver,
+		private RicherScopeGetTypeHelper $richerScopeGetTypeHelper,
 		private PhpVersion $phpVersion,
 		private bool $explicitMixedInUnknownGenericNew,
 		private bool $explicitMixedForGlobalVariables,
@@ -85,6 +86,7 @@ public function create(
 			$this->propertyReflectionFinder,
 			$this->parser,
 			$this->nodeScopeResolver,
+			$this->richerScopeGetTypeHelper,
 			$this->constantResolver,
 			$context,
 			$this->phpVersion,

src/Analyser/LazyInternalScopeFactory.php

@@ -79,6 +79,7 @@ public function create(
 			$this->container->getByType(PropertyReflectionFinder::class),
 			$this->container->getService('currentPhpVersionSimpleParser'),
 			$this->container->getByType(NodeScopeResolver::class),
+			$this->container->getByType(RicherScopeGetTypeHelper::class),
 			$this->container->getByType(ConstantResolver::class),
 			$context,
 			$this->container->getByType(PhpVersion::class),

src/Analyser/MutatingScope.php

@@ -204,6 +204,7 @@ public function __construct(
 		private PropertyReflectionFinder $propertyReflectionFinder,
 		private Parser $parser,
 		private NodeScopeResolver $nodeScopeResolver,
+		private RicherScopeGetTypeHelper $richerScopeGetTypeHelper,
 		private ConstantResolver $constantResolver,
 		private ScopeContext $context,
 		private PhpVersion $phpVersion,
@@ -924,46 +925,11 @@ private function resolveType(string $exprString, Expr $node): Type
 		}
 
 		if ($node instanceof Expr\BinaryOp\Identical) {
-			if (
-				$node->left instanceof Variable
-				&& is_string($node->left->name)
-				&& $node->right instanceof Variable
-				&& is_string($node->right->name)
-				&& $node->left->name === $node->right->name
-			) {
-				return new ConstantBooleanType(true);
-			}
-
-			$leftType = $this->getType($node->left);
-			$rightType = $this->getType($node->right);
-
-			if (
-				(
-					$node->left instanceof Node\Expr\PropertyFetch
-					|| $node->left instanceof Node\Expr\StaticPropertyFetch
-				)
-				&& $rightType->isNull()->yes()
-				&& !$this->hasPropertyNativeType($node->left)
-			) {
-				return new BooleanType();
-			}
-
-			if (
-				(
-					$node->right instanceof Node\Expr\PropertyFetch
-					|| $node->right instanceof Node\Expr\StaticPropertyFetch
-				)
-				&& $leftType->isNull()->yes()
-				&& !$this->hasPropertyNativeType($node->right)
-			) {
-				return new BooleanType();
-			}
-
-			return $this->initializerExprTypeResolver->resolveIdenticalType($leftType, $rightType)->type;
+			return $this->richerScopeGetTypeHelper->getIdenticalResult($this, $node)->type;
 		}
 
 		if ($node instanceof Expr\BinaryOp\NotIdentical) {
-			return $this->getType(new Expr\BooleanNot(new BinaryOp\Identical($node->left, $node->right)));
+			return $this->richerScopeGetTypeHelper->getNotIdenticalResult($this, $node)->type;
 		}
 
 		if ($node instanceof Expr\Instanceof_) {
@@ -2654,7 +2620,7 @@ private function promoteNativeTypes(): self
 	/**
 	 * @param Node\Expr\PropertyFetch|Node\Expr\StaticPropertyFetch $propertyFetch
 	 */
-	private function hasPropertyNativeType($propertyFetch): bool
+	public function hasPropertyNativeType($propertyFetch): bool
 	{
 		$propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($propertyFetch, $this);
 		if ($propertyReflection === null) {

src/Analyser/RicherScopeGetTypeHelper.php

@@ -0,0 +1,78 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Analyser;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\BinaryOp\Identical;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Reflection\InitializerExprTypeResolver;
+use PHPStan\Type\BooleanType;
+use PHPStan\Type\Constant\ConstantBooleanType;
+use PHPStan\Type\TypeResult;
+use function is_string;
+
+final class RicherScopeGetTypeHelper
+{
+
+	public function __construct(private InitializerExprTypeResolver $initializerExprTypeResolver)
+	{
+	}
+
+	/**
+	 * @return TypeResult<BooleanType>
+	 */
+	public function getIdenticalResult(Scope $scope, Identical $expr): TypeResult
+	{
+		if (
+			$expr->left instanceof Variable
+			&& is_string($expr->left->name)
+			&& $expr->right instanceof Variable
+			&& is_string($expr->right->name)
+			&& $expr->left->name === $expr->right->name
+		) {
+			return new TypeResult(new ConstantBooleanType(true), []);
+		}
+
+		$leftType = $scope->getType($expr->left);
+		$rightType = $scope->getType($expr->right);
+
+		if (
+			(
+				$expr->left instanceof Node\Expr\PropertyFetch
+				|| $expr->left instanceof Node\Expr\StaticPropertyFetch
+			)
+			&& $rightType->isNull()->yes()
+			&& !$scope->hasPropertyNativeType($expr->left)
+		) {
+			return new TypeResult(new BooleanType(), []);
+		}
+
+		if (
+			(
+				$expr->right instanceof Node\Expr\PropertyFetch
+				|| $expr->right instanceof Node\Expr\StaticPropertyFetch
+			)
+			&& $leftType->isNull()->yes()
+			&& !$scope->hasPropertyNativeType($expr->right)
+		) {
+			return new TypeResult(new BooleanType(), []);
+		}
+
+		return $this->initializerExprTypeResolver->resolveIdenticalType($leftType, $rightType);
+	}
+
+	/**
+	 * @return TypeResult<BooleanType>
+	 */
+	public function getNotIdenticalResult(Scope $scope, Node\Expr\BinaryOp\NotIdentical $expr): TypeResult
+	{
+		$identicalResult = $this->getIdenticalResult($scope, new Identical($expr->left, $expr->right));
+		$identicalType = $identicalResult->type;
+		if ($identicalType instanceof ConstantBooleanType) {
+			return new TypeResult(new ConstantBooleanType(!$identicalType->getValue()), $identicalResult->reasons);
+		}
+
+		return new TypeResult(new BooleanType(), []);
+	}
+
+}

src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php

@@ -3,13 +3,15 @@
 namespace PHPStan\Rules\Comparison;
 
 use PhpParser\Node;
+use PHPStan\Analyser\RicherScopeGetTypeHelper;
 use PHPStan\Analyser\Scope;
 use PHPStan\Parser\LastConditionVisitor;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\VerbosityLevel;
+use function count;
 use function sprintf;
 
 /**
@@ -19,6 +21,7 @@ final class StrictComparisonOfDifferentTypesRule implements Rule
 {
 
 	public function __construct(
+		private RicherScopeGetTypeHelper $richerScopeGetTypeHelper,
 		private bool $checkAlwaysTrueStrictComparison,
 		private bool $treatPhpDocTypesAsCertain,
 		private bool $reportAlwaysTrueInLastCondition,
@@ -34,19 +37,28 @@ public function getNodeType(): string
 
 	public function processNode(Node $node, Scope $scope): array
 	{
-		if (!$node instanceof Node\Expr\BinaryOp\Identical && !$node instanceof Node\Expr\BinaryOp\NotIdentical) {
+		if ($node instanceof Node\Expr\BinaryOp\Identical) {
+			$nodeTypeResult = $this->richerScopeGetTypeHelper->getIdenticalResult($this->treatPhpDocTypesAsCertain ? $scope : $scope->doNotTreatPhpDocTypesAsCertain(), $node);
+		} elseif ($node instanceof Node\Expr\BinaryOp\NotIdentical) {
+			$nodeTypeResult = $this->richerScopeGetTypeHelper->getNotIdenticalResult($this->treatPhpDocTypesAsCertain ? $scope : $scope->doNotTreatPhpDocTypesAsCertain(), $node);
+		} else {
 			return [];
 		}
 
-		$nodeType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node) : $scope->getNativeType($node);
+		$nodeType = $nodeTypeResult->type;
 		if (!$nodeType instanceof ConstantBooleanType) {
 			return [];
 		}
 
 		$leftType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->left) : $scope->getNativeType($node->left);
 		$rightType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->right) : $scope->getNativeType($node->right);
 
-		$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder {
+		$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $nodeTypeResult): RuleErrorBuilder {
+			$reasons = $nodeTypeResult->reasons;
+			if (count($reasons) > 0) {
+				return $ruleErrorBuilder->acceptsReasonsTip($reasons);
+			}
+
 			if (!$this->treatPhpDocTypesAsCertain) {
 				return $ruleErrorBuilder;
 			}

src/Testing/PHPStanTestCase.php

@@ -7,6 +7,7 @@
 use PHPStan\Analyser\Error;
 use PHPStan\Analyser\MutatingScope;
 use PHPStan\Analyser\NodeScopeResolver;
+use PHPStan\Analyser\RicherScopeGetTypeHelper;
 use PHPStan\Analyser\ScopeFactory;
 use PHPStan\Analyser\TypeSpecifier;
 use PHPStan\BetterReflection\Reflector\ClassReflector;
@@ -168,25 +169,28 @@ public static function createScopeFactory(ReflectionProvider $reflectionProvider
 		$reflectionProviderProvider = new DirectReflectionProviderProvider($reflectionProvider);
 		$constantResolver = new ConstantResolver($reflectionProviderProvider, $dynamicConstantNames);
 
+		$initializerExprTypeResolver = new InitializerExprTypeResolver(
+			$constantResolver,
+			$reflectionProviderProvider,
+			$container->getByType(PhpVersion::class),
+			$container->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class),
+			new OversizedArrayBuilder(),
+			$container->getParameter('usePathConstantsAsConstantString'),
+		);
+
 		return new ScopeFactory(
 			new DirectInternalScopeFactory(
 				MutatingScope::class,
 				$reflectionProvider,
-				new InitializerExprTypeResolver(
-					$constantResolver,
-					$reflectionProviderProvider,
-					$container->getByType(PhpVersion::class),
-					$container->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class),
-					new OversizedArrayBuilder(),
-					$container->getParameter('usePathConstantsAsConstantString'),
-				),
+				$initializerExprTypeResolver,
 				$container->getByType(DynamicReturnTypeExtensionRegistryProvider::class),
 				$container->getByType(ExpressionTypeResolverExtensionRegistryProvider::class),
 				$container->getByType(ExprPrinter::class),
 				$typeSpecifier,
 				new PropertyReflectionFinder(),
 				self::getParser(),
 				$container->getByType(NodeScopeResolver::class),
+				new RicherScopeGetTypeHelper($initializerExprTypeResolver),
 				$container->getByType(PhpVersion::class),
 				$container->getParameter('featureToggles')['explicitMixedInUnknownGenericNew'],
 				$container->getParameter('featureToggles')['explicitMixedForGlobalVariables'],

src/Type/MixedType.php

@@ -159,7 +159,18 @@ public function isSuperTypeOfWithReason(Type $type): IsSuperTypeOfResult
 			return IsSuperTypeOfResult::createMaybe();
 		}
 
-		return $this->subtractedType->isSuperTypeOfWithReason($type)->negate();
+		$result = $this->subtractedType->isSuperTypeOfWithReason($type)->negate();
+		if ($result->no()) {
+			return IsSuperTypeOfResult::createNo([
+				sprintf(
+					'Type %s has already been eliminated from %s.',
+					$this->subtractedType->describe(VerbosityLevel::precise()),
+					$this->describe(VerbosityLevel::typeOnly()),
+				),
+			]);
+		}
+
+		return $result;
 	}
 
 	public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $unionValues = true): Type

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

@@ -2,6 +2,7 @@
 
 namespace PHPStan\Rules\Comparison;
 
+use PHPStan\Analyser\RicherScopeGetTypeHelper;
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 use const PHP_INT_SIZE;
@@ -22,6 +23,7 @@ class StrictComparisonOfDifferentTypesRuleTest extends RuleTestCase
 	protected function getRule(): Rule
 	{
 		return new StrictComparisonOfDifferentTypesRule(
+			self::getContainer()->getByType(RicherScopeGetTypeHelper::class),
 			$this->checkAlwaysTrueStrictComparison,
 			$this->treatPhpDocTypesAsCertain,
 			$this->reportAlwaysTrueInLastCondition,
@@ -216,10 +218,12 @@ public function testStrictComparison(): void
 				[
 					'Strict comparison using === between mixed and \'foo\' will always evaluate to false.',
 					808,
+					'Type 1|string has already been eliminated from mixed.',
 				],
 				[
 					'Strict comparison using !== between mixed and 1 will always evaluate to true.',
 					812,
+					'Type 1|string has already been eliminated from mixed.',
 				],
 				[
 					'Strict comparison using === between \'foo\' and \'foo\' will always evaluate to true.',
@@ -275,6 +279,16 @@ public function testStrictComparison(): void
 					1014,
 					$tipText,
 				],
+				[
+					'Strict comparison using === between mixed and null will always evaluate to false.',
+					1030,
+					'Type null has already been eliminated from mixed.',
+				],
+				[
+					'Strict comparison using !== between mixed and null will always evaluate to true.',
+					1034,
+					'Type null has already been eliminated from mixed.',
+				],
 			],
 		);
 	}
@@ -419,6 +433,7 @@ public function testStrictComparisonWithoutAlwaysTrue(): void
 				[
 					'Strict comparison using === between mixed and \'foo\' will always evaluate to false.',
 					808,
+					'Type 1|string has already been eliminated from mixed.',
 				],
 				[
 					'Strict comparison using === between NAN and NAN will always evaluate to false.',
@@ -433,6 +448,11 @@ public function testStrictComparisonWithoutAlwaysTrue(): void
 					1014,
 					$tipText,
 				],
+				[
+					'Strict comparison using === between mixed and null will always evaluate to false.',
+					1030,
+					'Type null has already been eliminated from mixed.',
+				],
 			],
 		);
 	}

tests/PHPStan/Rules/Comparison/data/strict-comparison.php

@@ -1017,3 +1017,23 @@ public function doFoo($a): void
 	}
 
 }
+
+class SubtractedMixedAgainstNull
+{
+
+	public function doFoo($m): void
+	{
+		if ($m === null) {
+			return;
+		}
+
+		if ($m === null) {
+
+		}
+
+		if ($m !== null) {
+
+		}
+	}
+
+}