From 1819a2d880489968c08dc9ab21a28cd776a0b8c5 Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sat, 26 Nov 2022 11:26:36 -0500 Subject: [PATCH] Add diagnostic message when shape fields are missing (#8762) * Add a simple diagnostic for missing array shape fields * Fix dumb mistakes * Allow nested shape extra keys to prompt warning too --- .../Analyzer/FunctionLike/ReturnTypeAnalyzer.php | 9 ++++++++- .../Statements/Expression/Call/ArgumentAnalyzer.php | 11 +++++++++-- .../Internal/Analyzer/Statements/ReturnAnalyzer.php | 8 +++++++- .../Internal/Type/Comparator/KeyedArrayComparator.php | 9 +++++++++ .../Internal/Type/Comparator/TypeComparisonResult.php | 3 +++ .../Internal/Type/Comparator/UnionTypeComparator.php | 10 ++++++++++ tests/ArgTest.php | 4 +++- 7 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php index ba279ff711a..bc3ab40eecb 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLike/ReturnTypeAnalyzer.php @@ -56,6 +56,7 @@ use function array_filter; use function array_values; use function count; +use function implode; use function in_array; use function strpos; use function strtolower; @@ -568,7 +569,13 @@ public static function verifyReturnType( . $declared_return_type->getId() . '\' for ' . $cased_method_id . ' is incorrect, got \'' - . $inferred_return_type->getId() . '\'', + . $inferred_return_type->getId() + . '\'' + . ($union_comparison_results->missing_shape_fields + ? ' which is different due to additional array shape fields (' + . implode(', ', $union_comparison_results->missing_shape_fields) + . ')' + : ''), $return_type_location ), $suppressed_issues, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php index f439a643932..22dd08950f6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php @@ -64,6 +64,7 @@ use function count; use function explode; +use function implode; use function in_array; use function ord; use function preg_split; @@ -1044,8 +1045,14 @@ public static function verifyType( } else { IssueBuffer::maybeAdd( new InvalidArgument( - 'Argument ' . ($argument_offset + 1) . $method_identifier . ' expects ' . $param_type->getId() . - ', but ' . $type . ' provided', + 'Argument ' . ($argument_offset + 1) . $method_identifier . ' expects ' . $param_type->getId() + . ', but ' . $type + . ($union_comparison_results->missing_shape_fields + ? ' with additional array shape fields (' + . implode(', ', $union_comparison_results->missing_shape_fields) + . ') was' + : '') + . ' provided', $arg_location, $cased_method_id ), diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 1e76b6676bc..618031bd768 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -45,6 +45,7 @@ use function count; use function explode; +use function implode; use function reset; use function strtolower; @@ -480,7 +481,12 @@ public static function analyze( new InvalidReturnStatement( 'The inferred type \'' . $inferred_type->getId() . '\' does not match the declared return ' - . 'type \'' . $local_return_type->getId() . '\' for ' . $cased_method_id, + . 'type \'' . $local_return_type->getId() . '\' for ' . $cased_method_id + . ($union_comparison_results->missing_shape_fields + ? ' due to additional array shape fields (' + . implode(', ', $union_comparison_results->missing_shape_fields) + . ')' + : ''), new CodeLocation($source, $stmt->expr) ), $statements_analyzer->getSuppressedIssues() diff --git a/src/Psalm/Internal/Type/Comparator/KeyedArrayComparator.php b/src/Psalm/Internal/Type/Comparator/KeyedArrayComparator.php index a42bcb20a34..f3f86ead906 100644 --- a/src/Psalm/Internal/Type/Comparator/KeyedArrayComparator.php +++ b/src/Psalm/Internal/Type/Comparator/KeyedArrayComparator.php @@ -9,6 +9,7 @@ use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Atomic\TObjectWithProperties; +use function array_keys; use function is_string; /** @@ -82,6 +83,11 @@ public static function isContainedBy( ) { $atomic_comparison_result->type_coerced = true; } + + if ($property_type_comparison->missing_shape_fields) { + $atomic_comparison_result->missing_shape_fields + = $property_type_comparison->missing_shape_fields; + } } $all_types_contain = false; @@ -95,6 +101,9 @@ public static function isContainedBy( } } if ($container_sealed && $input_properties) { + if ($atomic_comparison_result) { + $atomic_comparison_result->missing_shape_fields = array_keys($input_properties); + } return false; } return $all_types_contain; diff --git a/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php b/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php index 05bdb46c9f8..0edffdbdc51 100644 --- a/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php +++ b/src/Psalm/Internal/Type/Comparator/TypeComparisonResult.php @@ -48,4 +48,7 @@ class TypeComparisonResult /** @var ?Atomic */ public $replacement_atomic_type; + + /** @var ?non-empty-list */ + public $missing_shape_fields; } diff --git a/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php b/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php index b7fd6e43535..c93c4b2ced0 100644 --- a/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php +++ b/src/Psalm/Internal/Type/Comparator/UnionTypeComparator.php @@ -100,6 +100,8 @@ public static function isContainedBy( $some_type_coerced = false; $some_type_coerced_from_mixed = false; + $some_missing_shape_fields = null; + if ($input_type_part instanceof TArrayKey && ($container_type->hasInt() && $container_type->hasString()) ) { @@ -272,6 +274,10 @@ public static function isContainedBy( } else { $all_type_coerced_from_as_mixed = true; } + + if ($atomic_comparison_result->missing_shape_fields) { + $some_missing_shape_fields = $atomic_comparison_result->missing_shape_fields; + } } if ($is_atomic_contained_by) { @@ -331,6 +337,10 @@ public static function isContainedBy( if (!$scalar_type_match_found) { $union_comparison_result->scalar_type_match_found = false; } + + if ($some_missing_shape_fields && !$some_type_coerced && !$scalar_type_match_found) { + $union_comparison_result->missing_shape_fields = $some_missing_shape_fields; + } } return false; diff --git a/tests/ArgTest.php b/tests/ArgTest.php index 488715120a9..ec48b4c8c55 100644 --- a/tests/ArgTest.php +++ b/tests/ArgTest.php @@ -5,6 +5,8 @@ use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait; use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait; +use const DIRECTORY_SEPARATOR; + class ArgTest extends TestCase { use InvalidCodeAnalysisTestTrait; @@ -792,7 +794,7 @@ function a(array $a): string { $sealedExtraKeys = ["test" => "str", "somethingElse" => "test"]; a($sealedExtraKeys); ', - 'error_message' => 'InvalidArgument', + 'error_message' => 'InvalidArgument - src' . DIRECTORY_SEPARATOR . 'somefile.php:8:23 - Argument 1 of a expects array{test: string}, but array{somethingElse: \'test\', test: \'str\'} with additional array shape fields (somethingElse) was provided', ], 'callbackArgsCountMismatch' => [ 'code' => '