From b419c299e30110b22391c67376610cbd8067a17f Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sun, 19 Dec 2021 11:17:25 +0000 Subject: [PATCH 1/3] Use InvalidScalarArgument only when we can be sure PHP attempts coercion --- .../Statements/Expression/Call/ArgumentAnalyzer.php | 2 +- .../Internal/Type/Comparator/ScalarTypeComparator.php | 11 ++++++++--- tests/AnnotationTest.php | 6 +++--- tests/ArgTest.php | 10 +++++----- tests/ArrayAssignmentTest.php | 4 ++-- tests/CallableTest.php | 2 +- tests/DocblockInheritanceTest.php | 2 +- tests/FunctionCallTest.php | 2 +- tests/IncludeTest.php | 2 +- tests/MagicMethodAnnotationTest.php | 4 ++-- tests/Template/ClassTemplateExtendsTest.php | 2 +- tests/Template/ClassTemplateTest.php | 2 +- tests/Template/FunctionTemplateTest.php | 2 +- 13 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index 84b777c7e20..ee64255efe8 100644 --- 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 index 12b1dd951da..a8fdce09fc3 100644 --- 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/tests/AnnotationTest.php b/tests/AnnotationTest.php index 9c65da93b99..c72d1edc367 100644 --- 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' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'spreadOperatorByRefAnnotationBadCall3' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'identifyReturnType' => [ ' [ + 'allowArrayIntScalarForArrayStringWithArgumentTypeCoercionIgnored' => [ ' $arr */ + /** @param array $arr */ function foo(array $arr) : void { } @@ -171,10 +171,10 @@ function bar() : array { return []; } - /** @psalm-suppress InvalidScalarArgument */ + /** @psalm-suppress ArgumentTypeCoercion */ foo(bar());', ], - 'allowArrayScalarForArrayStringWithScalarIgnored' => [ + 'allowArrayScalarForArrayStringWithArgumentTypeCoercionIgnored' => [ ' $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 index 5a7c7f6c1ec..e465b392981 100644 --- 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' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'nonEmptyAssignmentToListElementChangeType' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'inexistantCallableinCallableString' => [ 'boo([1, 2]);', - 'error_message' => 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], ]; } diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php index 4835ce61e91..ba66e993c9c 100644 --- 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' => [ ' [ getcwd() . DIRECTORY_SEPARATOR . 'file2.php', ], - 'error_message' => 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'namespacedRequireFunction' => [ 'files' => [ diff --git a/tests/MagicMethodAnnotationTest.php b/tests/MagicMethodAnnotationTest.php index f961664c170..11c5fdf53c9 100644 --- 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' => [ 'setBool("hello", 5);', - 'error_message' => 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'validAnnotationWithInvalidVariadicCall' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'extendsTwiceDifferentNameBrokenChain' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'argumentExpectsFleshOutTIndexedAccess' => [ ' 'InvalidScalarArgument', + 'error_message' => 'InvalidArgument', ], 'multipleArgConstraintWithMoreRestrictiveFirstArg' => [ ' Date: Sun, 19 Dec 2021 13:27:31 +0000 Subject: [PATCH 2/3] Add better documentation --- docs/running_psalm/issues/InvalidScalarArgument.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/running_psalm/issues/InvalidScalarArgument.md b/docs/running_psalm/issues/InvalidScalarArgument.md index bc0f97200f9..5a8713736cf 100644 --- 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 Date: Sun, 19 Dec 2021 17:59:48 +0000 Subject: [PATCH 3/3] Add better docs to TypeComparisonResult --- .../Internal/Type/Comparator/TypeComparisonResult.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php b/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php index 5fbe29af824..bdfafc95626 100644 --- 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 */