add support for extract to set variables for keyed arrays and respect…

… EXTR_SKIP
vimeo · Jan 14, 2024 · 02467fb · 02467fb
1 parent 079bfb8
commit 02467fb
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 4 deletions.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
@@ -35,6 +35,8 @@
 use Psalm\Type\Atomic\TDependentGetType;
 use Psalm\Type\Atomic\TFloat;
 use Psalm\Type\Atomic\TInt;
+use Psalm\Type\Atomic\TKeyedArray;
+use Psalm\Type\Atomic\TList;
 use Psalm\Type\Atomic\TLowercaseString;
 use Psalm\Type\Atomic\TMixed;
 use Psalm\Type\Atomic\TNamedObject;
@@ -47,12 +49,18 @@
 use Psalm\Type\Union;
 
 use function array_map;
+use function count;
 use function extension_loaded;
 use function in_array;
+use function is_numeric;
 use function is_string;
+use function preg_match;
 use function strpos;
 use function strtolower;
 
+use const EXTR_OVERWRITE;
+use const EXTR_SKIP;
+
 /**
  * @internal
  */
@@ -261,13 +269,98 @@ public static function handle(
         }
 
         if ($function_id === 'extract') {
+            $flag_value = false;
+            if (!isset($stmt->args[1])) {
+                $flag_value = EXTR_OVERWRITE;
+            } elseif (isset($stmt->args[1]->value)
+                && $stmt->args[1]->value instanceof PhpParser\Node\Expr
+                && ($flags_type = $statements_analyzer->node_data->getType($stmt->args[1]->value))
+                && $flags_type->hasLiteralInt() && count($flags_type->getAtomicTypes()) === 1) {
+                $flag_type_value = $flags_type->getSingleIntLiteral()->value;
+                if ($flag_type_value === EXTR_SKIP) {
+                    $flag_value = EXTR_SKIP;
+                } elseif ($flag_type_value === EXTR_OVERWRITE) {
+                    $flag_value = EXTR_OVERWRITE;
+                }
+                // @todo add support for other flags
+            }
+
+            $is_unsealed = true;
+            $validated_var_ids = [];
+            if ($flag_value !== false && isset($stmt->args[0]->value)
+                && $stmt->args[0]->value instanceof PhpParser\Node\Expr
+                && ($array_type_union = $statements_analyzer->node_data->getType($stmt->args[0]->value))
+                && $array_type_union->isSingle()
+            ) {
+                foreach ($array_type_union->getAtomicTypes() as $array_type) {
+                    if ($array_type instanceof TList) {
+                        $array_type = $array_type->getKeyedArray();
+                    }
+
+                    if ($array_type instanceof TKeyedArray) {
+                        foreach ($array_type->properties as $key => $type) {
+                            // variables must start with letters or underscore
+                            if ($key === '' || is_numeric($key) || preg_match('/^[A-Za-z_]/', $key) !== 1) {
+                                continue;
+                            }
+
+                            $var_id = '$' . $key;
+                            $validated_var_ids[] = $var_id;
+
+                            if (isset($context->vars_in_scope[$var_id]) && $flag_value === EXTR_SKIP) {
+                                continue;
+                            }
+
+                            if (!isset($context->vars_in_scope[$var_id]) && $type->possibly_undefined === true) {
+                                $context->possibly_assigned_var_ids[$var_id] = true;
+                            } elseif (isset($context->vars_in_scope[$var_id])
+                                && $type->possibly_undefined === true
+                                && $flag_value === EXTR_OVERWRITE) {
+                                $type = Type::combineUnionTypes(
+                                    $context->vars_in_scope[$var_id],
+                                    $type,
+                                    $codebase,
+                                    false,
+                                    true,
+                                    500,
+                                    false,
+                                );
+                            }
+
+                            $context->vars_in_scope[$var_id] = $type;
+                            $context->assigned_var_ids[$var_id] = (int) $stmt->getAttribute('startFilePos');
+                        }
+
+                        if (!isset($array_type->fallback_params)) {
+                            $is_unsealed = false;
+                        }
+                    }
+                }
+            }
+
+            if ($flag_value === EXTR_OVERWRITE && $is_unsealed === false) {
+                return;
+            }
+
+            if ($flag_value === EXTR_SKIP && $is_unsealed === false) {
+                return;
+            }
+
             $context->check_variables = false;
 
+            if ($flag_value === EXTR_SKIP) {
+                return;
+            }
+
             foreach ($context->vars_in_scope as $var_id => $_) {
                 if ($var_id === '$this' || strpos($var_id, '[') || strpos($var_id, '>')) {
                     continue;
                 }
 
+                if (in_array($var_id, $validated_var_ids, true)) {
+                    continue;
+                }
+
                 $mixed_type = new Union([new TMixed()], [
                     'parent_nodes' => $context->vars_in_scope[$var_id]->parent_nodes,
                 ]);

diff --git a/tests/FunctionCallTest.php b/tests/FunctionCallTest.php
@@ -513,19 +513,41 @@ function foo($s): array {
             ],
             'extractVarCheck' => [
                 'code' => '<?php
+                    /**
+                     * @psalm-suppress InvalidReturnType
+                     * @return array{a: 15, ...}
+                     */
+                    function getUnsealedArray() {}
                     function takesString(string $str): void {}
 
-                    $foo = null;
-                    $a = ["$foo" => "bar"];
+                    $foo = "foo";
+                    $a = getUnsealedArray();
                     extract($a);
                     takesString($foo);',
                 'assertions' => [],
                 'ignored_issues' => [
-                    'MixedAssignment',
-                    'MixedArrayAccess',
                     'MixedArgument',
                 ],
             ],
+            'extractVarCheckValid' => [
+                'code' => '<?php
+                    function takesInt(int $i): void {}
+
+                    $foo = "foo";
+                    $a = [$foo => 15];
+                    extract($a);
+                    takesInt($foo);',
+            ],
+            'extractSkipExtr' => [
+                'code' => '<?php
+                    $a = 1;
+
+                    extract(["a" => "x", "b" => "y"], EXTR_SKIP);',
+                'assertions' => [
+                    '$a===' => '1',
+                    '$b===' => '\'y\'',
+                ],
+            ],
             'compact' => [
                 'code' => '<?php
                     /**
@@ -3100,6 +3122,16 @@ function foo(never $_): void
                 'ignored_issues' => [],
                 'php_version' => '8.1',
             ],
+            'extractVarCheckInvalid' => [
+                'code' => '<?php
+                    function takesInt(int $i): void {}
+
+                    $foo = "123hello";
+                    $a = [$foo => 15];
+                    extract($a);
+                    takesInt($foo);',
+                'error_message' => 'InvalidScalarArgument',
+            ],
         ];
     }