Merge pull request #3747 from TysonAndre/unknown-method-plugin

Fix false positives in UnknownClassElementAccesPlugin

.phan/plugins/README.md

@@ -227,7 +227,8 @@ has the given value.
 
 ### UnknownClassElementAccessPlugin.php
 
-This plugin checks for accesses to unknown class elements that can't be type checked.
+This plugin checks for accesses to unknown class elements that can't be type checked (which may hide potential runtime errors such as having too few parameters).
+To reduce false positives, this will suppress warnings if at least one recursive analysis could infer class/interface types for the object.
 
 - **PhanPluginUnknownObjectMethodCall**: `Phan could not infer any class/interface types for the object of the method call {CODE} - inferred a type of {TYPE}`
 

.phan/plugins/UnknownClassElementAccessPlugin.php

@@ -5,7 +5,13 @@
 use ast\Node;
 use Phan\AST\ASTReverter;
 use Phan\AST\UnionTypeVisitor;
+use Phan\CodeBase;
+use Phan\Issue;
+use Phan\Language\Context;
+use Phan\Language\FileRef;
+use Phan\Language\UnionType;
 use Phan\PluginV3;
+use Phan\PluginV3\FinalizeProcessCapability;
 use Phan\PluginV3\PluginAwarePostAnalysisVisitor;
 use Phan\PluginV3\PostAnalyzeNodeCapability;
 
@@ -34,8 +40,21 @@
  * add them to the corresponding section of README.md
  */
 class UnknownClassElementAccessPlugin extends PluginV3 implements
-    PostAnalyzeNodeCapability
+    PostAnalyzeNodeCapability,
+    FinalizeProcessCapability
 {
+    public const UnknownObjectMethodCall = 'PhanPluginUnknownObjectMethodCall';
+    /**
+     * @var array<string,list<array{0:Context,1:string, 2:UnionType}>>
+     * Map from file name+line+node hash to the union type to a closure to emit the issue
+     */
+    private static $deferred_unknown_method_issues = [];
+
+    /**
+     * @var array<string,true>
+     * Set of file name+line+node hashes where the union type is known.
+     */
+    private static $known_method_set = [];
 
     /**
      * @return class-string - name of PluginAwarePostAnalysisVisitor subclass
@@ -44,6 +63,54 @@ public static function getPostAnalyzeNodeVisitorClassName(): string
     {
         return UnknownClassElementAccessVisitor::class;
     }
+
+    private static function generateKey(FileRef $context, int $lineno, string $node_string): string
+    {
+        // Sadly, the node can either be from the parse phase or any analysis phase, so we can't use spl_object_id.
+        return $context->getFile() . ':' . $lineno . ':' . sha1($node_string);
+    }
+
+    /**
+     * Emit an issue if the object of the method call isn't found later/earlier
+     */
+    public static function deferEmittingMethodIssue(Context $context, Node $node, UnionType $union_type): void
+    {
+        $node_string = ASTReverter::toShortString($node);
+        $key = self::generateKey($context, $node->lineno, $node_string);
+        if (isset(self::$known_method_set[$key])) {
+            return;
+        }
+        self::$deferred_unknown_method_issues[$key][] = [(clone $context)->withLineNumberStart($node->lineno), $node_string, $union_type];
+    }
+
+    /**
+     * Prevent this plugin from warning about $node_string at this file and line
+     */
+    public static function blacklistMethodIssue(Context $context, Node $node): void
+    {
+        $node_string = ASTReverter::toShortString($node);
+        $key = self::generateKey($context, $node->lineno, $node_string);
+        self::$known_method_set[$key] = true;
+        unset(self::$deferred_unknown_method_issues[$key]);
+    }
+
+    public function finalizeProcess(CodeBase $code_base): void
+    {
+        foreach (self::$deferred_unknown_method_issues as $issues) {
+            foreach ($issues as [$context, $node_string, $union_type]) {
+                $this->emitIssue(
+                    $code_base,
+                    $context,
+                    self::UnknownObjectMethodCall,
+                    'Phan could not infer any class/interface types for the object of the method call {CODE} - inferred a type of {TYPE}',
+                    [
+                        $node_string,
+                        $union_type->isEmpty() ? '(empty union type)' : $union_type
+                    ]
+                );
+            }
+        }
+    }
 }
 
 /**
@@ -67,19 +134,14 @@ public function visitMethodCall(Node $node): void
         }
         foreach ($union_type->getTypeSet() as $type) {
             if ($type->isObjectWithKnownFQSEN()) {
+                UnknownClassElementAccessPlugin::blacklistMethodIssue($this->context, $node);
                 return;
             }
         }
-        $this->emitPluginIssue(
-            $this->code_base,
-            $this->context,
-            'PhanPluginUnknownObjectMethodCall',
-            'Phan could not infer any class/interface types for the object of the method call {CODE} - inferred a type of {TYPE}',
-            [
-                ASTReverter::toShortString($node),
-                $union_type->isEmpty() ? '(empty union type)' : $union_type
-            ]
-        );
+        if (Issue::shouldSuppressIssue($this->code_base, $this->context, UnknownClassElementAccessPlugin::UnknownObjectMethodCall, $node->lineno, [])) {
+            return;
+        }
+        UnknownClassElementAccessPlugin::deferEmittingMethodIssue($this->context, $node, $union_type);
     }
 }
 

NEWS.md

@@ -18,7 +18,7 @@ New features(Analysis):
 + Infer key and element types from `iterator_to_array()`
 
 Plugins:
-+ Add `UnknownClassElementAccessPlugin` to warn about cases where Phan can't infer which class a method is being called on.
++ Add `UnknownClassElementAccessPlugin` to warn about cases where Phan can't infer which class an instance method is being called on.
 
 Bug fixes:
 + Fix bug causing phan to fail to properly recursively analyze parameters of inherited methods (#3740)

tests/files/src/0687_suspicious_string.php

@@ -7,7 +7,7 @@ public function test() {
         echo $this;
     }
 }
-class Stringable {
+class Stringable {  // in a namespace, doesn't conflict with php8 stringable.
     public function test() {
         echo "$this\n";
         echo $this;

tests/plugin_test/.phan/config.php

@@ -141,5 +141,6 @@
         'StrictComparisonPlugin',
         'LoopVariableReusePlugin',
         'RedundantAssignmentPlugin',
+        'UnknownClassElementAccessPlugin',
     ],
 ];

tests/plugin_test/expected/060_strict_method_check.php.expected

@@ -3,3 +3,4 @@ src/060_strict_method_check.php:13 PhanPossiblyNonClassMethodCall Call to method
 src/060_strict_method_check.php:16 PhanPossiblyNonClassMethodCall Call to method count on type ?\ArrayObject that could be a non-object
 src/060_strict_method_check.php:18 PhanPossiblyNonClassMethodCall Call to method count on type \ArrayObject|array that could be a non-object
 src/060_strict_method_check.php:19 PhanNonClassMethodCall Call to method count on non-class type bool
+src/060_strict_method_check.php:19 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $e->count() - inferred a type of bool

tests/plugin_test/expected/068_template_typeof.php.expected

@@ -1,4 +1,6 @@
 src/068_template_typeof.php:8 PhanUndeclaredClosureScope Reference to undeclared class \TTemplateType in @phan-closure-scope
+src/068_template_typeof.php:9 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $this->method() - inferred a type of (empty union type)
 src/068_template_typeof.php:9 PhanUndeclaredThis Variable $this is undeclared
 src/068_template_typeof.php:10 PhanNonClassMethodCall Call to method method on non-class type class-string<TTemplateType>
+src/068_template_typeof.php:10 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $x->method() - inferred a type of class-string<TTemplateType>
 src/068_template_typeof.php:11 PhanTypeMismatchReturn Returning type class-string<TTemplateType> but Closure($x) is declared to return TTemplateType

tests/plugin_test/expected/096_suspicious_name_order.php.expected

@@ -3,6 +3,9 @@ src/096_suspicious_name_order.php:13 PhanPluginSuspiciousParamOrder Suspicious o
 src/096_suspicious_name_order.php:14 PhanPluginSuspiciousParamOrder Suspicious order for arguments named second and third and first - These are being passed to parameters #1 ($first) and #2 ($second) and #3 ($third) of \test_order($first, $second, $third) defined at src/096_suspicious_name_order.php:3
 src/096_suspicious_name_order.php:15 PhanPluginSuspiciousParamOrder Suspicious order for arguments named second and first - These are being passed to parameters #1 ($first) and #2 ($second) of \test_order($first, $second, $third) defined at src/096_suspicious_name_order.php:3
 src/096_suspicious_name_order.php:16 PhanPluginSuspiciousParamOrder Suspicious order for arguments named getThird and getFirst - These are being passed to parameters #1 ($first) and #3 ($third) of \test_order($first, $second, $third) defined at src/096_suspicious_name_order.php:3
+src/096_suspicious_name_order.php:16 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $o->getFirst() - inferred a type of object
+src/096_suspicious_name_order.php:16 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $o->getSecond() - inferred a type of object
+src/096_suspicious_name_order.php:16 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $o->getThird() - inferred a type of object
 src/096_suspicious_name_order.php:26 PhanUnreferencedFunction Possibly zero references to function \test_no_false_positive()
 src/096_suspicious_name_order.php:31 PhanPluginSuspiciousParamOrder Suspicious order for arguments named secondValue and objFirst - These are being passed to parameters #1 (\stdClass $first) and #2 (bool $second) of \some_check(\stdClass $first, bool $second) defined at src/096_suspicious_name_order.php:19
 src/096_suspicious_name_order.php:31 PhanTypeMismatchArgumentReal Argument 1 ($first) is false but \some_check() takes \stdClass defined at src/096_suspicious_name_order.php:19

tests/plugin_test/expected/165_dont_warn_unknown.php.expected

@@ -0,0 +1,2 @@
+src/165_dont_warn_unknown.php:2 PhanUnreferencedFunction Possibly zero references to function \test165()
+src/165_dont_warn_unknown.php:4 PhanPluginUnknownObjectMethodCall Phan could not infer any class/interface types for the object of the method call $x->count() - inferred a type of (empty union type)

tests/plugin_test/src/165_dont_warn_unknown.php

@@ -0,0 +1,10 @@
+<?php
+function test165($x) {
+    // Should emit PhanPluginUnknownObjectMethodCall
+    $x->count();
+}
+function test165b($x) {
+    // Should not emit PhanPluginUnknownObjectMethodCall because a possible type was inferred in at least one recursive call
+    $x->count();
+}
+test165b(new ArrayObject());