Merge pull request #32 from mll-lab/handle-potential-first-class-callable

Handle FuncCall nodes that represent first class callables

Author: shish

phpstan-safe-rule.neon

@@ -1,4 +1,8 @@
 services:
+  -
+    class: TheCodingMachine\Safe\PHPStan\Rules\UseSafeCallablesRule
+    tags:
+      - phpstan.rules.rule
   -
     class: TheCodingMachine\Safe\PHPStan\Rules\UseSafeFunctionsRule
     tags:

phpstan.neon

@@ -4,7 +4,9 @@ parameters:
         - src
         - tests
     excludePaths:
-        - tests/Rules/data
+        - tests/Rules/UseSafeCallablesRule
+        - tests/Rules/UseSafeClassesRule
+        - tests/Rules/UseSafeFunctionsRule
     ignoreErrors:
         -
             message: '#^Implementing PHPStan\\Rules\\IdentifierRuleError is not covered by backward compatibility promise\. The interface might change in a minor PHPStan version\.$#'

src/Rules/UseSafeCallablesRule.php

@@ -0,0 +1,44 @@
+<?php
+
+namespace TheCodingMachine\Safe\PHPStan\Rules;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Node\FunctionCallableNode;
+use TheCodingMachine\Safe\PHPStan\Rules\Error\SafeFunctionRuleError;
+use TheCodingMachine\Safe\PHPStan\Utils\FunctionListLoader;
+
+/**
+ * This rule checks that no "unsafe" functions are used in code.
+ *
+ * @implements Rule<FunctionCallableNode>
+ */
+class UseSafeCallablesRule implements Rule
+{
+    /**
+     * @see JSON_THROW_ON_ERROR
+     */
+    const JSON_THROW_ON_ERROR = 4194304;
+
+    public function getNodeType(): string
+    {
+        return FunctionCallableNode::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $nodeName = $node->getName();
+        if (!$nodeName instanceof Node\Name) {
+            return [];
+        }
+        $functionName = $nodeName->toString();
+        $unsafeFunctions = FunctionListLoader::getFunctionList();
+
+        if (isset($unsafeFunctions[$functionName])) {
+            return [new SafeFunctionRuleError($nodeName, $node->getStartLine())];
+        }
+
+        return [];
+    }
+}

src/Rules/UseSafeFunctionsRule.php

@@ -1,6 +1,5 @@
 <?php
 
-
 namespace TheCodingMachine\Safe\PHPStan\Rules;
 
 use PhpParser\Node;
@@ -19,17 +18,23 @@
  */
 class UseSafeFunctionsRule implements Rule
 {
+    /**
+     * @see JSON_THROW_ON_ERROR
+     */
+    const JSON_THROW_ON_ERROR = 4194304;
+
     public function getNodeType(): string
     {
         return Node\Expr\FuncCall::class;
     }
 
     public function processNode(Node $node, Scope $scope): array
     {
-        if (!$node->name instanceof Node\Name) {
+        $nodeName = $node->name;
+        if (!$nodeName instanceof Node\Name) {
             return [];
         }
-        $functionName = $node->name->toString();
+        $functionName = $nodeName->toString();
         $unsafeFunctions = FunctionListLoader::getFunctionList();
 
         if (isset($unsafeFunctions[$functionName])) {
@@ -58,7 +63,7 @@ public function processNode(Node $node, Scope $scope): array
                 return [];
             }
 
-            return [new SafeFunctionRuleError($node->name, $node->getStartLine())];
+            return [new SafeFunctionRuleError($nodeName, $node->getStartLine())];
         }
 
         return [];
@@ -87,11 +92,16 @@ private function argValueIncludeJSONTHROWONERROR(?Arg $arg): bool
             return true;
         }
 
-        return in_array(true, array_map(function ($element) {
-            // JSON_THROW_ON_ERROR == 4194304
-            return ($element & 4194304) == 4194304;
-        }, array_filter($options, function ($element) {
-            return is_int($element);
-        })), true);
+        $intOptions = array_filter($options, function (mixed $option): bool {
+            return is_int($option);
+        });
+
+        foreach ($intOptions as $option) {
+            if (($option & self::JSON_THROW_ON_ERROR) === self::JSON_THROW_ON_ERROR) {
+                return true;
+            }
+        }
+
+        return false;
     }
 }

src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php

@@ -56,22 +56,17 @@ private function getPreliminarilyResolvedTypeFromFunctionCall(
         Scope $scope
     ): Type {
         $argumentPosition = $this->functions[$functionReflection->getName()];
-        $defaultReturnType = ParametersAcceptorSelector::selectFromArgs(
-            $scope,
-            $functionCall->getArgs(),
-            $functionReflection->getVariants()
-        )
+
+        $args = $functionCall->getArgs();
+        $variants = $functionReflection->getVariants();
+        $defaultReturnType = ParametersAcceptorSelector::selectFromArgs($scope, $args, $variants)
             ->getReturnType();
-        
-        if (count($functionCall->args) <= $argumentPosition) {
-            return $defaultReturnType;
-        }
 
-        $subjectArgument = $functionCall->args[$argumentPosition];
-        if (!$subjectArgument instanceof Arg) {
+        if (count($args) <= $argumentPosition) {
             return $defaultReturnType;
         }
-        
+
+        $subjectArgument = $args[$argumentPosition];
         $subjectArgumentType = $scope->getType($subjectArgument->value);
         $mixedType = new MixedType();
         if ($subjectArgumentType->isSuperTypeOf($mixedType)->yes()) {

tests/Rules/UseSafeCallablesRule/expr.php

@@ -0,0 +1,4 @@
+<?php
+
+$json_encode_name = 'json_encode';
+$encode_from_expression = $json_encode_name(...);

tests/Rules/UseSafeCallablesRule/native_safe.php

@@ -0,0 +1,3 @@
+<?php
+
+var_dump(...);

tests/Rules/UseSafeCallablesRule/unsafe.php

@@ -0,0 +1,3 @@
+<?php
+
+json_encode(...);

tests/Rules/UseSafeCallablesRule/use_safe.php

@@ -0,0 +1,5 @@
+<?php
+
+use function Safe\json_encode;
+
+json_encode(...);

tests/Rules/UseSafeCallablesRuleTest.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace TheCodingMachine\Safe\PHPStan\Rules;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @template-extends RuleTestCase<UseSafeCallablesRule>
+ */
+class UseSafeCallablesRuleTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        return new UseSafeCallablesRule();
+    }
+
+    public function testUnsafe(): void
+    {
+        $this->analyse([__DIR__ . '/UseSafeCallablesRule/unsafe.php'], [
+            [
+                "Function json_encode is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\json_encode;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.",
+                3,
+            ],
+        ]);
+    }
+
+    public function testUseSafe(): void
+    {
+        $this->analyse([__DIR__ . '/UseSafeCallablesRule/use_safe.php'], []);
+    }
+
+    public function testNativeSafe(): void
+    {
+        $this->analyse([__DIR__ . '/UseSafeCallablesRule/native_safe.php'], []);
+    }
+
+    public function testExpr(): void
+    {
+        $this->analyse([__DIR__ . '/UseSafeCallablesRule/expr.php'], []);
+    }
+}

tests/Rules/data/datetime.php renamed to tests/Rules/UseSafeClassesRule/datetime.php

@@ -17,7 +17,7 @@ protected function getRule(): Rule
 
     public function testDateTime(): void
     {
-        $this->analyse([__DIR__ . '/data/datetime.php'], [
+        $this->analyse([__DIR__ . '/UseSafeClassesRule/datetime.php'], [
             [
                 "Class DateTime is unsafe to use. Its methods can return FALSE instead of throwing an exception. Please add 'use Safe\DateTime;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.",
                 3,

tests/Rules/UseSafeClassesRuleTest.php

@@ -0,0 +1,4 @@
+<?php
+
+$fopen = 'fopen';
+$fopen('foobar', 'r');

tests/Rules/UseSafeFunctionsRule/expr.php

@@ -1,5 +1,7 @@
 <?php
 
+var_dump();
+
 // Test various combinations of flags
 json_decode("{}", true, 512, JSON_THROW_ON_ERROR);
 json_decode("{}", true, 512, JSON_INVALID_UTF8_IGNORE | JSON_THROW_ON_ERROR);
@@ -12,3 +14,16 @@
 
 // Test named arguments instead of positional
 json_decode("{}", flags: JSON_THROW_ON_ERROR);
+
+// Various combinations of flags
+json_encode([], JSON_THROW_ON_ERROR, 512);
+json_encode([], JSON_FORCE_OBJECT | JSON_THROW_ON_ERROR, 512);
+json_encode([], JSON_FORCE_OBJECT | JSON_INVALID_UTF8_IGNORE | JSON_THROW_ON_ERROR, 512);
+
+// Test raw integers too
+json_encode([], 4194304, 512);
+json_encode([], 16 | 4194304, 512);
+json_encode([], 16 | 1048576 | 4194304, 512);
+
+// Named arguments
+json_encode([], flags: JSON_THROW_ON_ERROR);

tests/Rules/data/safe_json_decode.php renamed to tests/Rules/UseSafeFunctionsRule/native_safe.php

@@ -0,0 +1,5 @@
+<?php
+
+fopen('foobar', 'r');
+json_decode("{}", flags: JSON_INVALID_UTF8_IGNORE);
+json_encode([], flags: JSON_FORCE_OBJECT);

tests/Rules/UseSafeFunctionsRule/unsafe.php

@@ -1,7 +1,11 @@
 <?php
+
+use function Safe\fopen;
 use function Safe\preg_replace;
 
+fopen('foobar', 'r');
+
 $x = preg_replace('/foo/', 'bar', 'baz');
 $y = stripos($x, 'foo');
 
-$x = preg_replace(['/foo/'], ['bar'], ['baz']);
+$x = Safe\preg_replace(['/foo/'], ['bar'], ['baz']);

tests/Rules/data/safe_pregreplace.php renamed to tests/Rules/UseSafeFunctionsRule/use_safe.php

@@ -15,33 +15,36 @@ protected function getRule(): Rule
         return new UseSafeFunctionsRule();
     }
 
-    public function testCatch(): void
+    public function testUnsafe(): void
     {
-        $this->analyse([__DIR__ . '/data/fopen.php'], [
+        $this->analyse([__DIR__ . '/UseSafeFunctionsRule/unsafe.php'], [
             [
                 "Function fopen is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\fopen;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.",
+                3,
+            ],
+            [
+                "Function json_decode is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\json_decode;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.",
                 4,
             ],
+            [
+                "Function json_encode is unsafe to use. It can return FALSE instead of throwing an exception. Please add 'use function Safe\\json_encode;' at the beginning of the file to use the variant provided by the 'thecodingmachine/safe' library.",
+                5,
+            ],
         ]);
     }
 
-    public function testNoCatchSafe(): void
-    {
-        $this->analyse([__DIR__ . '/data/safe_fopen.php'], []);
-    }
-
-    public function testExprCall(): void
+    public function testUseSafe(): void
     {
-        $this->analyse([__DIR__ . '/data/undirect_call.php'], []);
+        $this->analyse([__DIR__ . '/UseSafeFunctionsRule/use_safe.php'], []);
     }
 
-    public function testJSONDecodeNoCatchSafe(): void
+    public function testNativeSafe(): void
     {
-        $this->analyse([__DIR__ . '/data/safe_json_decode.php'], []);
+        $this->analyse([__DIR__ . '/UseSafeFunctionsRule/native_safe.php'], []);
     }
 
-    public function testJSONEncodeNoCatchSafe(): void
+    public function testExpr(): void
     {
-        $this->analyse([__DIR__ . '/data/safe_json_encode.php'], []);
+        $this->analyse([__DIR__ . '/UseSafeFunctionsRule/expr.php'], []);
     }
 }