Merge pull request #7188 from vimeo/muglug-use-invalidscalarargument-…

…less
vimeo · Jan 3, 2022 · ab60d31 · ab60d31
2 parents 966b6ae + 1e115da
commit ab60d31
Show file tree

Hide file tree

Showing 15 changed files with 39 additions and 25 deletions.
diff --git a/docs/running_psalm/issues/InvalidScalarArgument.md b/docs/running_psalm/issues/InvalidScalarArgument.md
@@ -1,6 +1,10 @@
 # InvalidScalarArgument
 
-Emitted when a scalar value is passed to a method that expected another scalar type
+Emitted when a scalar value is passed to a method that expected another scalar type.
+
+This is only emitted in situations where Psalm can be sure that PHP tries to coerce one scalar type to another.
+
+In all other cases `InvalidArgument` is emitted.
 
 ```php
 <?php

diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
@@ -983,7 +983,7 @@ public static function verifyType(
                     ),
                     $statements_analyzer->getSuppressedIssues()
                 );
-            } else {
+            } elseif ($cased_method_id !== 'echo' && $cased_method_id !== 'print') {
                 IssueBuffer::maybeAdd(
                     new ArgumentTypeCoercion(
                         'Argument ' . ($argument_offset + 1) . $method_identifier . ' expects ' . $param_type->getId() .

diff --git a/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php b/src/Psalm/Internal/Type/Comparator/ScalarTypeComparator.php
@@ -27,6 +27,7 @@
 use Psalm\Type\Atomic\TNamedObject;
 use Psalm\Type\Atomic\TNonEmptyLowercaseString;
 use Psalm\Type\Atomic\TNonEmptyNonspecificLiteralString;
+use Psalm\Type\Atomic\TNonEmptyScalar;
 use Psalm\Type\Atomic\TNonEmptyString;
 use Psalm\Type\Atomic\TNonFalsyString;
 use Psalm\Type\Atomic\TNonspecificLiteralInt;
@@ -297,7 +298,7 @@ public static function isContainedBy(
             if ($atomic_comparison_result) {
                 $atomic_comparison_result->type_coerced = true;
                 $atomic_comparison_result->type_coerced_from_mixed = true;
-                $atomic_comparison_result->scalar_type_match_found = true;
+                $atomic_comparison_result->scalar_type_match_found = !$container_type_part->from_docblock;
             }
 
             return false;
@@ -619,7 +620,8 @@ public static function isContainedBy(
         if ($input_type_part instanceof TNumeric) {
             if ($container_type_part->isNumericType()) {
                 if ($atomic_comparison_result) {
-                    $atomic_comparison_result->scalar_type_match_found = true;
+                    $atomic_comparison_result->type_coerced = true;
+                    $atomic_comparison_result->scalar_type_match_found = !$container_type_part->from_docblock;
                 }
             }
         }
@@ -630,7 +632,10 @@ public static function isContainedBy(
                 && !$container_type_part instanceof TLiteralFloat
             ) {
                 if ($atomic_comparison_result) {
-                    $atomic_comparison_result->scalar_type_match_found = true;
+                    $atomic_comparison_result->type_coerced
+                        = $atomic_comparison_result->type_coerced_from_scalar
+                        = ($input_type_part instanceof TScalar || $input_type_part instanceof TNonEmptyScalar);
+                    $atomic_comparison_result->scalar_type_match_found = !$container_type_part->from_docblock;
                 }
             }
         }

diff --git a/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php b/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php
@@ -7,7 +7,12 @@
 
 class TypeComparisonResult
 {
-    /** @var ?bool */
+    /**
+     * This is used to trigger `InvalidScalarArgument` in situations where we know PHP
+     * will try to coerce one scalar type to another.
+     *
+     * @var ?bool
+     */
     public $scalar_type_match_found;
 
     /** @var ?bool */

diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php
@@ -1702,7 +1702,7 @@ function foo(&...$s) : void {}
 
                     $a = 1;
                     foo($a);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'spreadOperatorByRefAnnotationBadCall2' => [
                 '<?php
@@ -1711,7 +1711,7 @@ function foo(&...$s) : void {}
 
                     $b = 2;
                     foo($b);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'spreadOperatorByRefAnnotationBadCall3' => [
                 '<?php
@@ -1720,7 +1720,7 @@ function foo(&...$s) : void {}
 
                     $c = 3;
                     foo($c);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'identifyReturnType' => [
                 '<?php

diff --git a/tests/ArgTest.php b/tests/ArgTest.php
@@ -160,9 +160,9 @@ function takesInt(int $i) : void {}
                     function foo($b) : void {}
                     foo(null);',
             ],
-            'allowArrayIntScalarForArrayStringWithScalarIgnored' => [
+            'allowArrayIntScalarForArrayStringWithArgumentTypeCoercionIgnored' => [
                 '<?php
-                    /** @param array<int|string> $arr */
+                    /** @param array<array-key> $arr */
                     function foo(array $arr) : void {
                     }
 
@@ -171,10 +171,10 @@ function bar() : array {
                       return [];
                     }
 
-                    /** @psalm-suppress InvalidScalarArgument */
+                    /** @psalm-suppress ArgumentTypeCoercion */
                     foo(bar());',
             ],
-            'allowArrayScalarForArrayStringWithScalarIgnored' => [
+            'allowArrayScalarForArrayStringWithArgumentTypeCoercionIgnored' => [
                 '<?php declare(strict_types=1);
                     /** @param array<string> $arr */
                     function foo(array $arr) : void {}
@@ -184,7 +184,7 @@ function bar() : array {
                         return [];
                     }
 
-                    /** @psalm-suppress InvalidScalarArgument */
+                    /** @psalm-suppress ArgumentTypeCoercion */
                     foo(bar());',
             ],
             'unpackObjectlikeListArgs' => [

diff --git a/tests/ArrayAssignmentTest.php b/tests/ArrayAssignmentTest.php
@@ -1887,7 +1887,7 @@ function takesArray(array $arr) : void {}
                     $a[] = 2;
 
                     takesArray($a);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'listUsedAsArrayWrongListType' => [
                 '<?php
@@ -1899,7 +1899,7 @@ function takesArray(array $arr) : void {}
                     $a[] = 2;
 
                     takesArray($a);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'nonEmptyAssignmentToListElementChangeType' => [
                 '<?php

diff --git a/tests/CallableTest.php b/tests/CallableTest.php
@@ -1390,7 +1390,7 @@ function takesCallableReturningString(callable $c) : void {
                     function foo(string $c) : void {
                         takesCallableReturningString([$c, "bar"]);
                     }',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'inexistantCallableinCallableString' => [
                 '<?php

diff --git a/tests/DocblockInheritanceTest.php b/tests/DocblockInheritanceTest.php
@@ -173,7 +173,7 @@ public function boo(array $arr) : void {}
                     }
 
                     (new X())->boo([1, 2]);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
         ];
     }

diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php
@@ -1941,7 +1941,7 @@ function a($b): int
                     }
 
                     a(["a" => "hello"]);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'objectLikeKeyChecksAgainstDifferentTKeyedArray' => [
                 '<?php

diff --git a/tests/IncludeTest.php b/tests/IncludeTest.php
@@ -722,7 +722,7 @@ function fooFoo($bar): void {
                 'files_to_check' => [
                     getcwd() . DIRECTORY_SEPARATOR . 'file2.php',
                 ],
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'namespacedRequireFunction' => [
                 'files' => [

diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php
@@ -813,7 +813,7 @@ class Child extends ParentClass {}
                     $child = new Child();
 
                     $child->setString("five");',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'unionAnnotationInvalidArg' => [
                 '<?php
@@ -829,7 +829,7 @@ class Child extends ParentClass {}
                     $child = new Child();
 
                     $b = $child->setBool("hello", 5);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'validAnnotationWithInvalidVariadicCall' => [
                 '<?php

diff --git a/tests/Template/ClassTemplateExtendsTest.php b/tests/Template/ClassTemplateExtendsTest.php
@@ -4626,7 +4626,7 @@ public function getID()
                     class AppUser extends User {}
 
                     $au = new AppUser("string");',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'extendsTwiceDifferentNameBrokenChain' => [
                 '<?php

diff --git a/tests/Template/ClassTemplateTest.php b/tests/Template/ClassTemplateTest.php
@@ -3822,7 +3822,7 @@ function takesIntCollection(Collection $c): void {}
 
                     takesStringCollection($collection);
                     takesIntCollection($collection);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'argumentExpectsFleshOutTIndexedAccess' => [
                 '<?php

diff --git a/tests/Template/FunctionTemplateTest.php b/tests/Template/FunctionTemplateTest.php
@@ -1811,7 +1811,7 @@ function(callable $s) {
                     function takesReturnTCallable(callable $s) {}
 
                     takesReturnTCallable($b);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'multipleArgConstraintWithMoreRestrictiveFirstArg' => [
                 '<?php