OnMixedRules: report even static fetch/call (#143)

shipmonk-rnd · Aug 31, 2023 · 092b785 · 092b785
1 parent 69dd3c9
commit 092b785
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 10 deletions.
diff --git a/src/Rule/ForbidFetchOnMixedRule.php b/src/Rule/ForbidFetchOnMixedRule.php
@@ -2,17 +2,21 @@
 
 namespace ShipMonk\PHPStan\Rule;
 
+use LogicException;
 use PhpParser\Node;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Identifier;
 use PhpParser\PrettyPrinter\Standard;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
 use PHPStan\Type\MixedType;
+use function get_class;
 use function sprintf;
 
 /**
- * @implements Rule<PropertyFetch>
+ * @implements Rule<Node>
  */
 class ForbidFetchOnMixedRule implements Rule
 {
@@ -29,11 +33,10 @@ public function __construct(Standard $printer, bool $checkExplicitMixed)
 
     public function getNodeType(): string
     {
-        return PropertyFetch::class;
+        return Node::class;
     }
 
     /**
-     * @param PropertyFetch $node
      * @return list<string> errors
      */
     public function processNode(Node $node, Scope $scope): array
@@ -42,7 +45,27 @@ public function processNode(Node $node, Scope $scope): array
             return []; // already checked by native PHPStan
         }
 
-        $caller = $node->var;
+        if ($node instanceof PropertyFetch || $node instanceof StaticPropertyFetch) {
+            return $this->processFetch($node, $scope);
+        }
+
+        return [];
+    }
+
+    /**
+     * @param PropertyFetch|StaticPropertyFetch $node
+     * @return list<string>
+     */
+    private function processFetch(Node $node, Scope $scope): array
+    {
+        $caller = $node instanceof PropertyFetch
+            ? $node->var
+            : $node->class;
+
+        if (!$caller instanceof Expr) {
+            return [];
+        }
+
         $callerType = $scope->getType($caller);
 
         if ($callerType instanceof MixedType) {
@@ -53,7 +76,8 @@ public function processNode(Node $node, Scope $scope): array
 
             return [
                 sprintf(
-                    'Property fetch ->%s is prohibited on unknown type (%s)',
+                    'Property fetch %s%s is prohibited on unknown type (%s)',
+                    $this->getFetchToken($node),
                     $property,
                     $this->printer->prettyPrintExpr($caller),
                 ),
@@ -63,4 +87,21 @@ public function processNode(Node $node, Scope $scope): array
         return [];
     }
 
+    /**
+     * @param PropertyFetch|StaticPropertyFetch $node
+     */
+    private function getFetchToken(Node $node): string
+    {
+        switch (get_class($node)) {
+            case StaticPropertyFetch::class:
+                return '::';
+
+            case PropertyFetch::class:
+                return '->';
+
+            default:
+                throw new LogicException('Unexpected node given: ' . get_class($node));
+        }
+    }
+
 }
diff --git a/src/Rule/ForbidMethodCallOnMixedRule.php b/src/Rule/ForbidMethodCallOnMixedRule.php
@@ -2,17 +2,22 @@
 
 namespace ShipMonk\PHPStan\Rule;
 
+use LogicException;
 use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\CallLike;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Identifier;
 use PhpParser\PrettyPrinter\Standard;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
 use PHPStan\Type\MixedType;
+use function get_class;
 use function sprintf;
 
 /**
- * @implements Rule<MethodCall>
+ * @implements Rule<CallLike>
  */
 class ForbidMethodCallOnMixedRule implements Rule
 {
@@ -29,11 +34,11 @@ public function __construct(Standard $printer, bool $checkExplicitMixed)
 
     public function getNodeType(): string
     {
-        return MethodCall::class;
+        return CallLike::class;
     }
 
     /**
-     * @param MethodCall $node
+     * @param CallLike $node
      * @return list<string> errors
      */
     public function processNode(Node $node, Scope $scope): array
@@ -42,7 +47,26 @@ public function processNode(Node $node, Scope $scope): array
             return []; // already checked by native PHPStan
         }
 
-        $caller = $node->var;
+        // NullsafeMethodCall not present due to https://github.com/phpstan/phpstan/issues/9830
+        if ($node instanceof MethodCall || $node instanceof StaticCall) {
+            return $this->checkCall($node, $scope);
+        }
+
+        return [];
+    }
+
+    /**
+     * @param MethodCall|StaticCall $node
+     * @return list<string>
+     */
+    private function checkCall(CallLike $node, Scope $scope): array
+    {
+        $caller = $node instanceof StaticCall ? $node->class : $node->var;
+
+        if (!$caller instanceof Expr) {
+            return [];
+        }
+
         $callerType = $scope->getType($caller);
 
         if ($callerType instanceof MixedType) {
@@ -51,7 +75,8 @@ public function processNode(Node $node, Scope $scope): array
 
             return [
                 sprintf(
-                    'Method call ->%s() is prohibited on unknown type (%s)',
+                    'Method call %s%s() is prohibited on unknown type (%s)',
+                    $this->getCallToken($node),
                     $method,
                     $this->printer->prettyPrintExpr($caller),
                 ),
@@ -61,4 +86,21 @@ public function processNode(Node $node, Scope $scope): array
         return [];
     }
 
+    /**
+     * @param MethodCall|StaticCall $node
+     */
+    private function getCallToken(CallLike $node): string
+    {
+        switch (get_class($node)) {
+            case StaticCall::class:
+                return '::';
+
+            case MethodCall::class:
+                return '->';
+
+            default:
+                throw new LogicException('Unexpected node given: ' . get_class($node));
+        }
+    }
+
 }
diff --git a/tests/Rule/data/ForbidFetchOnMixedRuleTest/code.php b/tests/Rule/data/ForbidFetchOnMixedRuleTest/code.php
@@ -4,9 +4,19 @@
 
 use ReflectionClass;
 
+class Foo {
+    public ?int $property = null;
+    public static ?int $staticProperty = null;
+}
+
 $fn = function (mixed $mixed, $unknown, array $array, ReflectionClass $reflection) {
+    (new Foo)->property;
+    Foo::$staticProperty;
+
     $mixed->fetch1; // error: Property fetch ->fetch1 is prohibited on unknown type ($mixed)
+    $mixed::$fetch1; // error: Property fetch ::$fetch1 is prohibited on unknown type ($mixed)
     $unknown->fetch2; // error: Property fetch ->fetch2 is prohibited on unknown type ($unknown)
+    $unknown::$fetch2; // error: Property fetch ::$fetch2 is prohibited on unknown type ($unknown)
     $array[0]->fetch3; // error: Property fetch ->fetch3 is prohibited on unknown type ($array[0])
     $reflection->newInstance()->fetch4;
 };
diff --git a/tests/Rule/data/ForbidMethodCallOnMixedRule/code.php b/tests/Rule/data/ForbidMethodCallOnMixedRule/code.php
@@ -4,9 +4,25 @@
 
 use ReflectionClass;
 
+class Foo {
+    public function method() {}
+    public static function staticMethod() {}
+}
+
 $fn = function (mixed $mixed, $unknown, array $array, ReflectionClass $reflection) {
+
+    $foo = new Foo();
+    $foo->method();
+    $foo?->method();
+    Foo::staticMethod();
+    $foo::staticMethod();
+
     $mixed->call1(); // error: Method call ->call1() is prohibited on unknown type ($mixed)
+    $mixed?->call1(); // error: Method call ->call1() is prohibited on unknown type ($mixed)
+    $mixed::call1(); // error: Method call ::call1() is prohibited on unknown type ($mixed)
     $unknown->call2(); // error: Method call ->call2() is prohibited on unknown type ($unknown)
+    $unknown?->call2(); // error: Method call ->call2() is prohibited on unknown type ($unknown)
+    $unknown::call2(); // error: Method call ::call2() is prohibited on unknown type ($unknown)
     $array[0]->call3(); // error: Method call ->call3() is prohibited on unknown type ($array[0])
     $reflection->newInstance()->call4();
 };