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
vimeo · Nov 26, 2022 · 1819a2d · 1819a2d
1 parent d1921b1
commit 1819a2d
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -48,4 +48,7 @@ class TypeComparisonResult
 
     /** @var ?Atomic */
     public $replacement_atomic_type;
+
+    /** @var ?non-empty-list<int|string> */
+    public $missing_shape_fields;
 }
diff --git 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
@@ -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' => '<?php