Use InvalidScalarArgument only when we can be sure PHP attempts coercion

vimeo · Dec 19, 2021 · 87a7a88 · 87a7a88
1 parent 2e32a18
commit 87a7a88
Show file tree

Hide file tree

Showing 13 changed files with 28 additions and 23 deletions.
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;
@@ -299,7 +300,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;
@@ -621,7 +622,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;
                 }
             }
         }
@@ -632,7 +634,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/tests/AnnotationTest.php b/tests/AnnotationTest.php
@@ -1864,7 +1864,7 @@ function foo(&...$s) : void {}
 
                     $a = 1;
                     foo($a);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'spreadOperatorByRefAnnotationBadCall2' => [
                 '<?php
@@ -1873,7 +1873,7 @@ function foo(&...$s) : void {}
 
                     $b = 2;
                     foo($b);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'spreadOperatorByRefAnnotationBadCall3' => [
                 '<?php
@@ -1882,7 +1882,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
@@ -1868,7 +1868,7 @@ function takesArray(array $arr) : void {}
                     $a[] = 2;
 
                     takesArray($a);',
-                'error_message' => 'InvalidScalarArgument',
+                'error_message' => 'InvalidArgument',
             ],
             'listUsedAsArrayWrongListType' => [
                 '<?php
@@ -1880,7 +1880,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
@@ -1268,7 +1268,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