ForbidIdenticalClassComparisonRule: fix false positive with TemplateM…

…ixedType (#115)
shipmonk-rnd · May 24, 2023 · 20c7c73 · 20c7c73
1 parent 9ffe7b7
commit 20c7c73
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 17 deletions.
diff --git a/src/Rule/ForbidIdenticalClassComparisonRule.php b/src/Rule/ForbidIdenticalClassComparisonRule.php
@@ -11,10 +11,9 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Rules\Rule;
-use PHPStan\Type\CallableType;
-use PHPStan\Type\MixedType;
 use PHPStan\Type\ObjectType;
-use PHPStan\Type\ObjectWithoutClassType;
+use PHPStan\Type\Type;
+use PHPStan\Type\TypeUtils;
 use PHPStan\Type\VerbosityLevel;
 use function count;
 
@@ -25,11 +24,6 @@ class ForbidIdenticalClassComparisonRule implements Rule
 {
 
     private const DEFAULT_BLACKLIST = [DateTimeInterface::class];
-    private const IGNORED_TYPES = [
-        MixedType::class, // mixed is "maybe" accepted by any (denied) class
-        ObjectWithoutClassType::class, // object is "maybe" accepted by any (denied) class
-        CallableType::class, // any non-final class descendant can have __invoke method causing it to be "maybe" accepted by any (denied) class
-    ];
 
     /**
      * @var array<int, class-string<object>>
@@ -80,20 +74,14 @@ public function processNode(Node $node, Scope $scope): array
             return []; // always-true or always-false, already reported by native PHPStan (like $a === $a)
         }
 
-        foreach (self::IGNORED_TYPES as $ignoredType) {
-            if ($leftType instanceof $ignoredType || $rightType instanceof $ignoredType) {
-                return [];
-            }
-        }
-
         $errors = [];
 
         foreach ($this->blacklist as $className) {
             $forbiddenObjectType = new ObjectType($className);
 
             if (
-                !$forbiddenObjectType->accepts($leftType, $scope->isDeclareStrictTypes())->no()
-                && !$forbiddenObjectType->accepts($rightType, $scope->isDeclareStrictTypes())->no()
+                $this->containsClass($leftType, $className)
+                && $this->containsClass($rightType, $className)
             ) {
                 $errors[] = "Using {$node->getOperatorSigil()} with {$forbiddenObjectType->describe(VerbosityLevel::typeOnly())} is denied";
             }
@@ -102,4 +90,19 @@ public function processNode(Node $node, Scope $scope): array
         return $errors;
     }
 
+    private function containsClass(Type $type, string $className): bool
+    {
+        $benevolentType = TypeUtils::toBenevolentUnion($type);
+
+        foreach ($benevolentType->getObjectClassNames() as $classNameInType) {
+            $classInType = new ObjectType($classNameInType);
+
+            if ($classInType->isInstanceOf($className)->yes()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
 }
diff --git a/tests/Rule/data/ForbidIdenticalClassComparisonRule/code.php b/tests/Rule/data/ForbidIdenticalClassComparisonRule/code.php
@@ -23,16 +23,30 @@ public function testNonObject(?DateTimeImmutable $a, string $b): void
         }
     }
 
+    /**
+     * @param TItem|null $mixedTemplate1
+     * @param TItem|null $mixedTemplate2
+     * @param callable(DateTimeImmutable): void $callable1
+     * @param callable(DateTimeImmutable): void $callable2
+     *
+     * @template TItem
+     */
     public function testProblematicTypes(
         DateTimeImmutable $a,
         mixed $b,
         object $c,
-        callable $d
+        callable $d,
+        mixed $mixedTemplate1,
+        mixed $mixedTemplate2,
+        callable $callable1,
+        callable $callable2
     ): void
     {
         $a === $b;
         $a === $c;
         $a === $d;
+        $mixedTemplate1 === $mixedTemplate2;
+        $callable1 === $callable2;
     }
 
     public function testRegular(DateTimeImmutable $a, DateTimeImmutable $b): void