Add PhanSuspiciousNamedArgumentVariadicInternal

Warn about code such as `sprintf('foo=%s', foo: 'value')` Fixes phan#4284
TysonAndre · Dec 11, 2020 · a8ee551 · a8ee551
1 parent 1664d17
commit a8ee551
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 5 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -11,6 +11,8 @@ New features (Analysis):
 + Mention if PhanUndeclaredFunction is potentially caused by the target php version being too old. (#4230)
 + Support a `non-null-mixed` type and change the way analysis involving nullability is checked for `mixed` (phpdoc and real). (#4278, #4276)
 + Improve real type inference for conditionals on literal types (#4288)
++ Emit `PhanSuspiciousNamedArgumentVariadicInternal` when using named arguments with variadic parameters of internal functions that are
+  not among the few reflection functions known to support named arguments. (#4284)
 
 Nov 27 2020, Phan 3.2.6
 -----------------------

diff --git a/internal/Issue-Types-Caught-by-Phan.md b/internal/Issue-Types-Caught-by-Phan.md
@@ -2211,6 +2211,14 @@ Passing named argument to a variadic parameter ${PARAMETER} of the same name in
 
 e.g. [this issue](https://github.com/phan/phan/tree/3.2.2/tests/php80_files/expected/029_named_variadic.php.expected#L2) is emitted when analyzing [this PHP file](https://github.com/phan/phan/tree/3.2.2/tests/php80_files/src/029_named_variadic.php#L5).
 
+## PhanSuspiciousNamedArgumentVariadicInternal
+
+```
+Passing named argument {CODE} to the variadic parameter of the internal function {METHOD}. Except for a few internal methods that call methods/constructors dynamically, this is usually not supported by internal functions.
+```
+
+e.g. [this issue](https://github.com/phan/phan/tree/master/tests/php80_files/expected/036_named_variadic.php.expected#L4) is emitted when analyzing [this PHP file](https://github.com/phan/phan/tree/master/tests/php80_files/src/036_named_variadic.php#L6).
+
 ## PhanUndeclaredNamedArgument
 
 ```

diff --git a/src/Phan/Analysis/ArgumentType.php b/src/Phan/Analysis/ArgumentType.php
@@ -469,11 +469,19 @@ private static function analyzeParameterListForCallback(
                         break;
                     }
                 }
-
-                if (!isset($parameter) || (!$found && !$parameter->isVariadic())) {
+                if (!isset($parameter)) {
                     self::emitUndeclaredNamedArgument($code_base, $context, $method, $argument);
                     continue;
                 }
+
+                if (!$found) {
+                    if (!$parameter->isVariadic()) {
+                        self::emitUndeclaredNamedArgument($code_base, $context, $method, $argument);
+                    } elseif ($method->isPHPInternal()) {
+                        self::emitSuspiciousNamedArgumentVariadicInternal($code_base, $context, $method, $argument);
+                    }
+                    continue;
+                }
                 if (!\is_array($positions_used)) {
                     $positions_used = \array_slice($arg_nodes, 0, $original_i);
                 }
@@ -613,10 +621,18 @@ private static function analyzeParameterList(
                     }
                 }
 
-                if (!isset($parameter) || (!$found && !$parameter->isVariadic())) {
+                if (!isset($parameter)) {
                     self::emitUndeclaredNamedArgument($code_base, $context, $method, $argument);
                     continue;
                 }
+                if (!$found) {
+                    if (!$parameter->isVariadic()) {
+                        self::emitUndeclaredNamedArgument($code_base, $context, $method, $argument);
+                    } elseif ($method->isPHPInternal()) {
+                        self::emitSuspiciousNamedArgumentVariadicInternal($code_base, $context, $method, $argument);
+                    }
+                    continue;
+                }
                 if (!\is_array($positions_used)) {
                     $positions_used = \array_slice($node->children, 0, $original_i);
                 }
@@ -802,6 +818,43 @@ private static function emitUndeclaredNamedArgument(
         }
     }
 
+    /**
+     * Warn about using named arguments with internal functions,
+     * ignoring known exceptions such as call_user_func, ReflectionMethod->invoke, etc.
+     * @param FunctionInterface $method an internal function
+     * @param Node $argument a node of kind ast\AST_NAMED_ARG
+     */
+    private static function emitSuspiciousNamedArgumentVariadicInternal(
+        CodeBase $code_base,
+        Context $context,
+        FunctionInterface $method,
+        Node $argument
+    ): void {
+        $fqsen = $method instanceof Method ? $method->getRealDefiningFQSEN() : $method->getFQSEN();
+        if (!\in_array($fqsen->__toString(), [
+            '\call_user_func',
+            '\ReflectionMethod::invoke',
+            '\ReflectionMethod::newInstance',
+            '\ReflectionFunction::invoke',
+            '\ReflectionFunction::newInstance',
+            '\ReflectionFunctionAbstract::invoke',
+            '\ReflectionFunctionAbstract::newInstance',
+            '\Closure::call',
+            '\Closure::__invoke',
+        ], true)) {
+            Issue::maybeEmitWithParameters(
+                $code_base,
+                $context,
+                Issue::SuspiciousNamedArgumentVariadicInternal,
+                $argument->lineno,
+                [
+                    ASTReverter::toShortString($argument),
+                    $method->getRepresentationForIssue(true),
+                ]
+            );
+        }
+    }
+
     /**
      * @param array<int,mixed> $positions_used
      */

diff --git a/src/Phan/Issue.php b/src/Phan/Issue.php
@@ -368,6 +368,7 @@ class Issue
     public const MissingNamedArgument                                        = 'PhanMissingNamedArgument';
     public const MissingNamedArgumentInternal                                = 'PhanMissingNamedArgumentInternal';
     public const SuspiciousNamedArgumentForVariadic                          = 'PhanSuspiciousNamedArgumentForVariadic';
+    public const SuspiciousNamedArgumentVariadicInternal                     = 'PhanSuspiciousNamedArgumentVariadicInternal';
 
     // Issue::CATEGORY_NOOP
     public const NoopArray                     = 'PhanNoopArray';
@@ -3443,6 +3444,14 @@ private static function generateIssueMap(): array
                 self::REMEDIATION_B,
                 7061
             ),
+            new Issue(
+                self::SuspiciousNamedArgumentVariadicInternal,
+                self::CATEGORY_PARAMETER,
+                self::SEVERITY_NORMAL,
+                'Passing named argument {CODE} to the variadic parameter of the internal function {METHOD}. Except for a few internal methods that call methods/constructors dynamically, this is usually not supported by internal functions.',
+                self::REMEDIATION_B,
+                7062
+            ),
 
             // Issue::CATEGORY_NOOP
             new Issue(
@@ -4819,15 +4828,15 @@ private static function generateIssueMap(): array
             new Issue(
                 self::CompatibleScalarTypePHP56,
                 self::CATEGORY_COMPATIBLE,
-                self::SEVERITY_NORMAL,
+                self::SEVERITY_CRITICAL,
                 "In PHP 5.6, scalar types such as {TYPE} in type signatures are treated like class names",
                 self::REMEDIATION_B,
                 3024
             ),
             new Issue(
                 self::CompatibleAnyReturnTypePHP56,
                 self::CATEGORY_COMPATIBLE,
-                self::SEVERITY_NORMAL,
+                self::SEVERITY_CRITICAL,
                 "In PHP 5.6, return types ({TYPE}) are not supported",
                 self::REMEDIATION_B,
                 3025

diff --git a/tests/php80_files/expected/036_named_variadic.php.expected b/tests/php80_files/expected/036_named_variadic.php.expected
@@ -0,0 +1,5 @@
+%s:5 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (arg: 123)
+%s:6 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (foo: 'test')
+%s:6 PhanPluginUseReturnValueInternalKnown Expected to use the return value of the internal function/method \sprintf
+%s:6 PhanSuspiciousNamedArgumentVariadicInternal Passing named argument foo: 'test' to the variadic parameter of the internal function \sprintf(string $format, ...$values). Except for a few internal methods that call methods/constructors dynamically, this is usually not supported by internal functions.
+%s:8 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (arg: 123)
diff --git a/tests/php80_files/src/035_non_null_mixed.php b/tests/php80_files/src/035_non_null_mixed.php
@@ -0,0 +1,4 @@
+<?php
+function decode_or_default(string $json) {
+    return json_decode($json) ?? 'default';
+}
diff --git a/tests/php80_files/src/036_named_variadic.php b/tests/php80_files/src/036_named_variadic.php
@@ -0,0 +1,8 @@
+<?php
+function test36(int $arg) {
+    var_export($arg);
+}
+call_user_func('test36', arg: 123);
+sprintf("foo=%s\n", foo: 'test');
+$c = Closure::fromCallable('test36');
+$c->call(arg: 123);