From 5e8e183667cfd2641fce7771e0116e9761f785e9 Mon Sep 17 00:00:00 2001 From: Brown Date: Mon, 28 Sep 2020 00:45:02 -0400 Subject: [PATCH] Add improvements from unused variable checks --- .../Analyzer/FunctionLikeAnalyzer.php | 2 +- .../Statements/Block/LoopAnalyzer.php | 37 +- .../Analyzer/Statements/BreakAnalyzer.php | 5 + .../Analyzer/Statements/ContinueAnalyzer.php | 4 + .../Statements/Expression/ArrayAnalyzer.php | 2 +- .../Assignment/ArrayAssignmentAnalyzer.php | 86 +++-- .../InstancePropertyAssignmentAnalyzer.php | 2 +- .../Expression/AssignmentAnalyzer.php | 328 ++++++++++++++---- .../BinaryOp/NonComparisonOpAnalyzer.php | 59 +++- .../Expression/BinaryOpAnalyzer.php | 63 +++- .../Expression/Call/ArgumentsAnalyzer.php | 19 - .../Call/ClassTemplateParamCollector.php | 4 - .../Expression/Call/FunctionCallAnalyzer.php | 52 ++- .../Call/Method/AtomicMethodCallAnalyzer.php | 12 +- .../Method/MethodCallReturnTypeFetcher.php | 4 +- .../Expression/Call/NewAnalyzer.php | 2 +- .../Expression/Call/StaticCallAnalyzer.php | 2 +- .../Statements/Expression/CastAnalyzer.php | 6 +- .../Expression/EncapsulatedStringAnalyzer.php | 2 +- .../Expression/Fetch/ArrayFetchAnalyzer.php | 2 +- .../Fetch/InstancePropertyFetchAnalyzer.php | 4 +- .../Fetch/VariableFetchAnalyzer.php | 6 +- .../Expression/IncDecExpressionAnalyzer.php | 6 +- .../Statements/Expression/YieldAnalyzer.php | 4 + .../Analyzer/Statements/ReturnAnalyzer.php | 12 +- .../FilterVarReturnTypeProvider.php | 2 +- src/Psalm/Type.php | 4 +- src/Psalm/Type/Union.php | 4 +- tests/Loop/WhileTest.php | 31 ++ 29 files changed, 584 insertions(+), 182 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php index 22b433995..b8540a2ba 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php @@ -1199,7 +1199,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer $function_param->location, null ); - $var_type->parent_nodes = [$type_source]; + $var_type->parent_nodes = [$type_source->id => $type_source]; } $context->vars_in_scope['$' . $function_param->name] = $var_type; diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php index 960627fb2..3d38c3c56 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php @@ -57,7 +57,7 @@ class LoopAnalyzer $assignment_depth = 0; - $asserted_var_ids = []; + $always_assigned_before_loop_body_vars = []; $pre_condition_clauses = []; @@ -81,7 +81,7 @@ class LoopAnalyzer ); } } else { - $asserted_var_ids = Context::getNewOrUpdatedVarIds( + $always_assigned_before_loop_body_vars = Context::getNewOrUpdatedVarIds( $loop_scope->loop_parent_context, $loop_scope->loop_context ); @@ -174,16 +174,13 @@ class LoopAnalyzer if (!$is_do) { foreach ($pre_conditions as $condition_offset => $pre_condition) { - $asserted_var_ids = array_merge( - self::applyPreConditionToLoopContext( - $statements_analyzer, - $pre_condition, - $pre_condition_clauses[$condition_offset], - $loop_scope->loop_context, - $loop_scope->loop_parent_context, - $is_do - ), - $asserted_var_ids + self::applyPreConditionToLoopContext( + $statements_analyzer, + $pre_condition, + $pre_condition_clauses[$condition_offset], + $loop_scope->loop_context, + $loop_scope->loop_parent_context, + $is_do ); } } @@ -215,7 +212,7 @@ class LoopAnalyzer $inner_do_context = clone $inner_context; foreach ($pre_conditions as $condition_offset => $pre_condition) { - $asserted_var_ids = array_merge( + $always_assigned_before_loop_body_vars = array_merge( self::applyPreConditionToLoopContext( $statements_analyzer, $pre_condition, @@ -224,12 +221,12 @@ class LoopAnalyzer $loop_scope->loop_parent_context, $is_do ), - $asserted_var_ids + $always_assigned_before_loop_body_vars ); } } - $asserted_var_ids = array_unique($asserted_var_ids); + $always_assigned_before_loop_body_vars = array_unique($always_assigned_before_loop_body_vars); foreach ($post_expressions as $post_expression) { if (ExpressionAnalyzer::analyze($statements_analyzer, $post_expression, $inner_context) === false) { @@ -260,7 +257,7 @@ class LoopAnalyzer // but union the types with what's in the loop scope foreach ($inner_context->vars_in_scope as $var_id => $type) { - if (in_array($var_id, $asserted_var_ids, true)) { + if (in_array($var_id, $always_assigned_before_loop_body_vars, true)) { // set the vars to whatever the while/foreach loop expects them to be if (!isset($pre_loop_context->vars_in_scope[$var_id]) || !$type->equals($pre_loop_context->vars_in_scope[$var_id]) @@ -375,7 +372,7 @@ class LoopAnalyzer } } - foreach ($asserted_var_ids as $var_id) { + foreach ($always_assigned_before_loop_body_vars as $var_id) { if ((!isset($inner_context->vars_in_scope[$var_id]) || $inner_context->vars_in_scope[$var_id]->getId() !== $pre_loop_context->vars_in_scope[$var_id]->getId() @@ -693,7 +690,7 @@ class LoopAnalyzer $new_referenced_var_ids = $loop_context->referenced_var_ids; $loop_context->referenced_var_ids = array_merge($pre_referenced_var_ids, $new_referenced_var_ids); - $asserted_var_ids = Context::getNewOrUpdatedVarIds($outer_context, $loop_context); + $always_assigned_before_loop_body_vars = Context::getNewOrUpdatedVarIds($outer_context, $loop_context); $loop_context->clauses = Algebra::simplifyCNF( array_merge($outer_context->clauses, $pre_condition_clauses) @@ -741,7 +738,7 @@ class LoopAnalyzer return []; } - foreach ($asserted_var_ids as $var_id) { + foreach ($always_assigned_before_loop_body_vars as $var_id) { $loop_context->clauses = Context::filterClauses( $var_id, $loop_context->clauses, @@ -750,7 +747,7 @@ class LoopAnalyzer ); } - return $asserted_var_ids; + return $always_assigned_before_loop_body_vars; } /** diff --git a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php index addd1bc42..b7fc3a254 100644 --- a/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/BreakAnalyzer.php @@ -42,6 +42,11 @@ class BreakAnalyzer } else { foreach ($redefined_vars as $var => $type) { if ($type->hasMixed()) { + if (isset($loop_scope->possibly_redefined_loop_parent_vars[$var])) { + $type->parent_nodes + += $loop_scope->possibly_redefined_loop_parent_vars[$var]->parent_nodes; + } + $loop_scope->possibly_redefined_loop_parent_vars[$var] = $type; } elseif (isset($loop_scope->possibly_redefined_loop_parent_vars[$var])) { $loop_scope->possibly_redefined_loop_parent_vars[$var] = Type::combineUnionTypes( diff --git a/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php index a131182aa..086f2e824 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ContinueAnalyzer.php @@ -71,6 +71,10 @@ class ContinueAnalyzer foreach ($redefined_vars as $var => $type) { if ($type->hasMixed()) { + if (isset($loop_scope->possibly_redefined_loop_vars[$var])) { + $type->parent_nodes += $loop_scope->possibly_redefined_loop_vars[$var]->parent_nodes; + } + $loop_scope->possibly_redefined_loop_vars[$var] = $type; } elseif (isset($loop_scope->possibly_redefined_loop_vars[$var])) { $loop_scope->possibly_redefined_loop_vars[$var] = Type::combineUnionTypes( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php index c9a2cfcf5..4e247d560 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ArrayAnalyzer.php @@ -226,7 +226,7 @@ class ArrayAnalyzer ); } - $parent_taint_nodes = array_merge($parent_taint_nodes, [$new_parent_node]); + $parent_taint_nodes[$new_parent_node->id] = $new_parent_node; } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php index 008abd941..59cb17416 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php @@ -285,14 +285,20 @@ class ArrayAssignmentAnalyzer $child_stmt_type = $assignment_type; $statements_analyzer->node_data->setType($child_stmt, $assignment_type); - self::taintArrayAssignment( - $statements_analyzer, - $child_stmt->var, - $array_type, - $assignment_type, - $array_var_id, - $dim_value !== null ? [$dim_value] : [] - ); + if ($statements_analyzer->control_flow_graph) { + self::taintArrayAssignment( + $statements_analyzer, + $child_stmt, + $array_type, + $assignment_type, + ExpressionIdentifier::getArrayVarId( + $child_stmt->var, + $statements_analyzer->getFQCLN(), + $statements_analyzer + ), + $dim_value !== null ? [$dim_value] : [] + ); + } } $current_type = $child_stmt_type; @@ -400,10 +406,11 @@ class ArrayAssignmentAnalyzer array_pop($var_id_additions); - $array_var_id = null; + $parent_array_var_id = null; if ($root_var_id) { $array_var_id = $root_var_id . implode('', $var_id_additions); + $parent_array_var_id = $root_var_id . implode('', \array_slice($var_id_additions, 0, -1)); $context->vars_in_scope[$array_var_id] = clone $child_stmt_type; $context->possibly_assigned_var_ids[$array_var_id] = true; } @@ -411,10 +418,10 @@ class ArrayAssignmentAnalyzer if ($statements_analyzer->control_flow_graph) { self::taintArrayAssignment( $statements_analyzer, - $child_stmt->var, + $child_stmt, $statements_analyzer->node_data->getType($child_stmt->var) ?: Type::getMixed(), $new_child_type, - $array_var_id, + $parent_array_var_id, $key_values ); } @@ -565,6 +572,8 @@ class ArrayAssignmentAnalyzer $array_atomic_type = clone $atomic_root_types['array']; $new_child_type = new Type\Union([$array_atomic_type]); + + $new_child_type->parent_nodes = $root_type->parent_nodes; } } elseif ($array_atomic_type instanceof TList) { $array_atomic_type = new TNonEmptyList( @@ -767,44 +776,47 @@ class ArrayAssignmentAnalyzer */ private static function taintArrayAssignment( StatementsAnalyzer $statements_analyzer, - PhpParser\Node\Expr $stmt, + PhpParser\Node\Expr\ArrayDimFetch $expr, Type\Union $stmt_type, Type\Union $child_stmt_type, - ?string $array_var_id, + ?string $var_var_id, array $key_values ) : void { if ($statements_analyzer->control_flow_graph - && $child_stmt_type->parent_nodes && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) ) { - $var_location = new \Psalm\CodeLocation($statements_analyzer->getSource(), $stmt); + if (!$stmt_type->parent_nodes) { + $var_location = new \Psalm\CodeLocation($statements_analyzer->getSource(), $expr->var); - $new_parent_node = \Psalm\Internal\ControlFlow\ControlFlowNode::getForAssignment( - $array_var_id ?: 'array-assignment', - $var_location - ); + $parent_node = \Psalm\Internal\ControlFlow\ControlFlowNode::getForAssignment( + $var_var_id ?: 'assignment', + $var_location + ); - $statements_analyzer->control_flow_graph->addNode($new_parent_node); + $statements_analyzer->control_flow_graph->addNode($parent_node); - foreach ($child_stmt_type->parent_nodes as $parent_node) { - if ($key_values) { - foreach ($key_values as $key_value) { - $statements_analyzer->control_flow_graph->addPath( - $parent_node, - $new_parent_node, - 'array-assignment-\'' . $key_value->value . '\'' - ); - } - } else { - $statements_analyzer->control_flow_graph->addPath( - $parent_node, - $new_parent_node, - 'array-assignment' - ); - } + $stmt_type->parent_nodes = [$parent_node->id => $parent_node]; } - $stmt_type->parent_nodes[] = $new_parent_node; + foreach ($stmt_type->parent_nodes as $parent_node) { + foreach ($child_stmt_type->parent_nodes as $child_parent_node) { + if ($key_values) { + foreach ($key_values as $key_value) { + $statements_analyzer->control_flow_graph->addPath( + $child_parent_node, + $parent_node, + 'array-assignment-\'' . $key_value->value . '\'' + ); + } + } else { + $statements_analyzer->control_flow_graph->addPath( + $child_parent_node, + $parent_node, + 'array-assignment' + ); + } + } + } } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index 7fdf17225..3aafb9efb 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php @@ -1210,7 +1210,7 @@ class InstancePropertyAssignmentAnalyzer } } - $stmt_var_type->parent_nodes = [$var_node]; + $stmt_var_type->parent_nodes = [$var_node->id => $var_node]; $context->vars_in_scope[$var_id] = $stmt_var_type; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php index 6586ef973..8dd856bcf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php @@ -4,6 +4,8 @@ namespace Psalm\Internal\Analyzer\Statements\Expression; use PhpParser; use Psalm\Internal\Analyzer\CommentAnalyzer; use Psalm\Internal\Analyzer\Statements\Block\ForeachAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\Fetch\ArrayFetchAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\Fetch\VariableFetchAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Assignment\ArrayAssignmentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Assignment\InstancePropertyAssignmentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Assignment\StaticPropertyAssignmentAnalyzer; @@ -197,12 +199,20 @@ class AssignmentAnalyzer } } + $parent_nodes = $temp_assign_value_type->parent_nodes ?? []; + $assign_value_type = $comment_type; - $assign_value_type->parent_nodes = $temp_assign_value_type->parent_nodes ?? []; + $assign_value_type->parent_nodes = $parent_nodes; } elseif (!$assign_value_type) { - $assign_value_type = $assign_value - ? ($statements_analyzer->node_data->getType($assign_value) ?: Type::getMixed()) - : Type::getMixed(); + if ($assign_value) { + $assign_value_type = $statements_analyzer->node_data->getType($assign_value); + } + + if ($assign_value_type) { + $assign_value_type = clone $assign_value_type; + } else { + $assign_value_type = Type::getMixed(); + } } if ($array_var_id && isset($context->vars_in_scope[$array_var_id])) { @@ -439,6 +449,14 @@ class AssignmentAnalyzer continue; } + $offset_value = null; + + if (!$assign_var_item->key) { + $offset_value = $offset; + } elseif ($assign_var_item->key instanceof PhpParser\Node\Scalar\String_) { + $offset_value = $assign_var_item->key->value; + } + $list_var_id = ExpressionIdentifier::getArrayVarId( $var, $statements_analyzer->getFQCLN(), @@ -454,10 +472,12 @@ class AssignmentAnalyzer && !$assign_var_item->key ) { // if object-like has int offsets - if (isset($assign_value_atomic_type->properties[$offset])) { - $offset_type = $assign_value_atomic_type->properties[(string)$offset]; + if ($offset_value !== null + && isset($assign_value_atomic_type->properties[$offset_value]) + ) { + $value_type = $assign_value_atomic_type->properties[$offset_value]; - if ($offset_type->possibly_undefined) { + if ($value_type->possibly_undefined) { if (IssueBuffer::accepts( new PossiblyUndefinedArrayOffset( 'Possibly undefined array key', @@ -468,15 +488,39 @@ class AssignmentAnalyzer // fall through } - $offset_type = clone $offset_type; - $offset_type->possibly_undefined = false; + $value_type = clone $value_type; + $value_type->possibly_undefined = false; + } + + if ($statements_analyzer->control_flow_graph + && $assign_value + ) { + $assign_value_id = ExpressionIdentifier::getArrayVarId( + $assign_value, + $statements_analyzer->getFQCLN(), + $statements_analyzer + ); + + $keyed_array_var_id = null; + + if ($assign_value_id) { + $keyed_array_var_id = $assign_value_id . '[\'' . $offset_value . '\']'; + } + + ArrayFetchAnalyzer::taintArrayFetch( + $statements_analyzer, + $assign_value, + $keyed_array_var_id, + $value_type, + Type::getString((string) $offset_value) + ); } self::analyze( $statements_analyzer, $var, null, - $offset_type, + $value_type, $context, $doc_comment ); @@ -574,13 +618,15 @@ class AssignmentAnalyzer ]); } + $array_value_type = $assign_value_atomic_type instanceof Type\Atomic\TArray + ? clone $assign_value_atomic_type->type_params[1] + : Type::getMixed(); + self::analyze( $statements_analyzer, $var, null, - $assign_value_atomic_type instanceof Type\Atomic\TArray - ? clone $assign_value_atomic_type->type_params[1] - : Type::getMixed(), + $array_value_type, $context, $doc_comment ); @@ -623,10 +669,32 @@ class AssignmentAnalyzer if ($assign_value_atomic_type instanceof Type\Atomic\TArray) { $new_assign_type = clone $assign_value_atomic_type->type_params[1]; + if ($statements_analyzer->control_flow_graph + && $assign_value + ) { + ArrayFetchAnalyzer::taintArrayFetch( + $statements_analyzer, + $assign_value, + null, + $new_assign_type, + Type::getArrayKey() + ); + } + $can_be_empty = !$assign_value_atomic_type instanceof Type\Atomic\TNonEmptyArray; } elseif ($assign_value_atomic_type instanceof Type\Atomic\TList) { $new_assign_type = clone $assign_value_atomic_type->type_param; + if ($statements_analyzer->control_flow_graph && $assign_value) { + ArrayFetchAnalyzer::taintArrayFetch( + $statements_analyzer, + $assign_value, + null, + $new_assign_type, + Type::getArrayKey() + ); + } + $can_be_empty = !$assign_value_atomic_type instanceof Type\Atomic\TNonEmptyList; } elseif ($assign_value_atomic_type instanceof Type\Atomic\TKeyedArray) { if ($assign_var_item->key @@ -652,6 +720,16 @@ class AssignmentAnalyzer } } + if ($statements_analyzer->control_flow_graph && $assign_value && $new_assign_type) { + ArrayFetchAnalyzer::taintArrayFetch( + $statements_analyzer, + $assign_value, + null, + $new_assign_type, + Type::getArrayKey() + ); + } + $can_be_empty = !$assign_value_atomic_type->sealed; } elseif ($assign_value_atomic_type->hasArrayAccessInterface($codebase)) { ForeachAnalyzer::getKeyValueParamsForTraversableObject( @@ -717,6 +795,45 @@ class AssignmentAnalyzer ) { $context->vars_in_scope[$list_var_id]->addType(new Type\Atomic\TNull); } + + if ($statements_analyzer->control_flow_graph) { + $control_flow_graph = $statements_analyzer->control_flow_graph; + + $var_location = new CodeLocation($statements_analyzer->getSource(), $var); + + if (!$context->vars_in_scope[$list_var_id]->parent_nodes) { + $assignment_node = ControlFlowNode::getForAssignment( + $list_var_id, + $var_location + ); + + $context->vars_in_scope[$list_var_id]->parent_nodes = [ + $assignment_node->id => $assignment_node + ]; + } else { + if (\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $context->vars_in_scope[$list_var_id]->parent_nodes = []; + } else { + $new_parent_node = ControlFlowNode::getForAssignment($list_var_id, $var_location); + + $statements_analyzer->control_flow_graph->addNode($new_parent_node); + + foreach ($context->vars_in_scope[$list_var_id]->parent_nodes as $parent_node) { + $control_flow_graph->addPath( + $parent_node, + $new_parent_node, + '=', + [], + $removed_taints + ); + } + + $context->vars_in_scope[$list_var_id]->parent_nodes = [ + $new_parent_node->id => $new_parent_node + ]; + } + } + } } } } @@ -896,7 +1013,9 @@ class AssignmentAnalyzer $control_flow_graph->addPath($parent_node, $new_parent_node, '=', [], $removed_taints); } - $context->vars_in_scope[$var_id]->parent_nodes = [$new_parent_node]; + $context->vars_in_scope[$var_id]->parent_nodes = [ + $new_parent_node->id => $new_parent_node + ]; } } } @@ -1058,6 +1177,14 @@ class AssignmentAnalyzer $statements_analyzer->node_data->setType($stmt, $fake_coalesce_type); } + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->var, + $stmt->expr, + 'coalesce' + ); + return true; } @@ -1165,26 +1292,51 @@ class AssignmentAnalyzer ); if ($stmt->var instanceof PhpParser\Node\Expr\ArrayDimFetch) { + $result_type = $result_type ?: Type::getMixed($context->inside_loop); + ArrayAssignmentAnalyzer::analyze( $statements_analyzer, $stmt->var, $context, $stmt->expr, - $result_type ?: Type::getMixed($context->inside_loop) + $result_type ); } elseif ($result_type && $array_var_id) { + $result_type = clone $result_type; $context->vars_in_scope[$array_var_id] = $result_type; - $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$array_var_id]); } - } elseif ($stmt instanceof PhpParser\Node\Expr\AssignOp\Div - && $stmt_var_type - && $stmt_expr_type - && $stmt_var_type->hasDefinitelyNumericType() - && $stmt_expr_type->hasDefinitelyNumericType() - && $array_var_id - ) { - $context->vars_in_scope[$array_var_id] = Type::combineUnionTypes(Type::getFloat(), Type::getInt()); - $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$array_var_id]); + + if ($result_type) { + $statements_analyzer->node_data->setType($stmt, $result_type); + } + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->var, + $stmt->expr, + 'nondivop' + ); + } elseif ($stmt instanceof PhpParser\Node\Expr\AssignOp\Div) { + if ($stmt_var_type + && $stmt_expr_type + && $stmt_var_type->hasDefinitelyNumericType() + && $stmt_expr_type->hasDefinitelyNumericType() + && $array_var_id + ) { + $context->vars_in_scope[$array_var_id] = Type::combineUnionTypes(Type::getFloat(), Type::getInt()); + $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$array_var_id]); + } else { + $statements_analyzer->node_data->setType($stmt, Type::getMixed()); + } + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->var, + $stmt->expr, + 'div' + ); } elseif ($stmt instanceof PhpParser\Node\Expr\AssignOp\Concat) { BinaryOp\ConcatAnalyzer::analyze( $statements_analyzer, @@ -1197,42 +1349,20 @@ class AssignmentAnalyzer if ($result_type && $array_var_id) { $context->vars_in_scope[$array_var_id] = $result_type; $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$array_var_id]); - - if ($statements_analyzer->control_flow_graph - && !\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues()) - ) { - $stmt_left_type = $statements_analyzer->node_data->getType($stmt->var); - $stmt_right_type = $statements_analyzer->node_data->getType($stmt->expr); - - $var_location = new CodeLocation($statements_analyzer, $stmt); - - $new_parent_node = ControlFlowNode::getForAssignment($array_var_id, $var_location); - $statements_analyzer->control_flow_graph->addNode($new_parent_node); - - $result_type->parent_nodes = [$new_parent_node]; - - if ($stmt_left_type && $stmt_left_type->parent_nodes) { - foreach ($stmt_left_type->parent_nodes as $parent_node) { - $statements_analyzer->control_flow_graph->addPath($parent_node, $new_parent_node, 'concat'); - } - } - - if ($stmt_right_type && $stmt_right_type->parent_nodes) { - foreach ($stmt_right_type->parent_nodes as $parent_node) { - $statements_analyzer->control_flow_graph->addPath($parent_node, $new_parent_node, 'concat'); - } - } - } } - } elseif ($stmt_var_type - && $stmt_expr_type - && ($stmt_var_type->hasInt() || $stmt_expr_type->hasInt()) - && ($stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseOr - || $stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseXor - || $stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseAnd - || $stmt instanceof PhpParser\Node\Expr\AssignOp\ShiftLeft - || $stmt instanceof PhpParser\Node\Expr\AssignOp\ShiftRight - ) + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->var, + $stmt->expr, + 'concatop' + ); + } elseif ($stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseOr + || $stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseXor + || $stmt instanceof PhpParser\Node\Expr\AssignOp\BitwiseAnd + || $stmt instanceof PhpParser\Node\Expr\AssignOp\ShiftLeft + || $stmt instanceof PhpParser\Node\Expr\AssignOp\ShiftRight ) { BinaryOp\NonDivArithmeticOpAnalyzer::analyze( $statements_analyzer, @@ -1245,8 +1375,44 @@ class AssignmentAnalyzer ); if ($result_type && $array_var_id) { - $context->vars_in_scope[$array_var_id] = $result_type; - $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$array_var_id]); + $context->vars_in_scope[$array_var_id] = clone $result_type; + $statements_analyzer->node_data->setType($stmt, $context->vars_in_scope[$array_var_id]); + } + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->var, + $stmt->expr, + 'bitwiseop' + ); + } + + if ($statements_analyzer->control_flow_graph + && $array_var_id + && isset($context->vars_in_scope[$array_var_id]) + && ($stmt_type = $statements_analyzer->node_data->getType($stmt)) + ) { + $control_flow_graph = $statements_analyzer->control_flow_graph; + + if ($stmt_type->parent_nodes) { + if (\in_array('TaintedInput', $statements_analyzer->getSuppressedIssues())) { + $stmt_type->parent_nodes = []; + } else { + $var_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); + + $new_parent_node = ControlFlowNode::getForAssignment($array_var_id, $var_location); + + $control_flow_graph->addNode($new_parent_node); + + foreach ($stmt_type->parent_nodes as $parent_node) { + $control_flow_graph->addPath($parent_node, $new_parent_node, '='); + } + + $context->vars_in_scope[$array_var_id]->parent_nodes = [ + $new_parent_node->id => $new_parent_node + ]; + } } } @@ -1278,7 +1444,13 @@ class AssignmentAnalyzer } } - if ($stmt->var instanceof PhpParser\Node\Expr\ArrayDimFetch) { + if (!($stmt instanceof PhpParser\Node\Expr\AssignOp\Plus + || $stmt instanceof PhpParser\Node\Expr\AssignOp\Minus + || $stmt instanceof PhpParser\Node\Expr\AssignOp\Mod + || $stmt instanceof PhpParser\Node\Expr\AssignOp\Mul + || $stmt instanceof PhpParser\Node\Expr\AssignOp\Pow) + && $stmt->var instanceof PhpParser\Node\Expr\ArrayDimFetch + ) { ArrayAssignmentAnalyzer::analyze( $statements_analyzer, $stmt->var, @@ -1336,6 +1508,24 @@ class AssignmentAnalyzer $context->vars_in_scope[$rhs_var_id] = Type::getMixed(); } + if ($statements_analyzer->control_flow_graph + && $lhs_var_id + && $rhs_var_id + && isset($context->vars_in_scope[$rhs_var_id]) + ) { + $rhs_type = $context->vars_in_scope[$rhs_var_id]; + + $control_flow_graph = $statements_analyzer->control_flow_graph; + + $lhs_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var); + + $lhs_node = ControlFlowNode::getForAssignment($lhs_var_id, $lhs_location); + + foreach ($rhs_type->parent_nodes as $byref_destination_node) { + $control_flow_graph->addPath($lhs_node, $byref_destination_node, '='); + } + } + return true; } @@ -1424,8 +1614,14 @@ class AssignmentAnalyzer $statements_analyzer ); + $by_ref_out_type = clone $by_ref_out_type; + + if ($existing_type->parent_nodes) { + $by_ref_out_type->parent_nodes += $existing_type->parent_nodes; + } + if ($existing_type->getId() !== 'array') { - $context->vars_in_scope[$var_id] = clone $by_ref_out_type; + $context->vars_in_scope[$var_id] = $by_ref_out_type; if (!($stmt_type = $statements_analyzer->node_data->getType($stmt)) || $stmt_type->isEmpty() @@ -1441,7 +1637,9 @@ class AssignmentAnalyzer $context->vars_in_scope[$var_id] = $by_ref_out_type; - if (!($stmt_type = $statements_analyzer->node_data->getType($stmt)) || $stmt_type->isEmpty()) { + $stmt_type = $statements_analyzer->node_data->getType($stmt); + + if (!$stmt_type || $stmt_type->isEmpty()) { $statements_analyzer->node_data->setType($stmt, clone $by_ref_type); } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/NonComparisonOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/NonComparisonOpAnalyzer.php index e0256cdc2..0406018f4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/NonComparisonOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/NonComparisonOpAnalyzer.php @@ -3,6 +3,7 @@ namespace Psalm\Internal\Analyzer\Statements\Expression\BinaryOp; use PhpParser; use Psalm\Internal\Analyzer\StatementsAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\BinaryOpAnalyzer; use Psalm\Context; use Psalm\Type; use Psalm\Type\Atomic\TFloat; @@ -62,10 +63,20 @@ class NonComparisonOpAnalyzer $context ); - if ($result_type) { - $statements_analyzer->node_data->setType($stmt, $result_type); + if (!$result_type) { + $result_type = new Type\Union([new Type\Atomic\TInt(), new Type\Atomic\TFloat()]); } + $statements_analyzer->node_data->setType($stmt, $result_type); + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'nondivop' + ); + return; } @@ -74,6 +85,14 @@ class NonComparisonOpAnalyzer $statements_analyzer->node_data->setType($stmt, Type::getInt()); } + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'xor' + ); + return; } @@ -82,6 +101,14 @@ class NonComparisonOpAnalyzer $statements_analyzer->node_data->setType($stmt, Type::getBool()); } + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'xor' + ); + return; } @@ -96,14 +123,22 @@ class NonComparisonOpAnalyzer $context ); - if ($result_type) { - if ($result_type->hasInt()) { - $result_type->addType(new TFloat); - } - - $statements_analyzer->node_data->setType($stmt, $result_type); + if (!$result_type) { + $result_type = new Type\Union([new Type\Atomic\TInt(), new Type\Atomic\TFloat()]); + } elseif ($result_type->hasInt()) { + $result_type->addType(new TFloat); } + $statements_analyzer->node_data->setType($stmt, $result_type); + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'div' + ); + return; } @@ -117,6 +152,14 @@ class NonComparisonOpAnalyzer $result_type, $context ); + + BinaryOpAnalyzer::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'or' + ); } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php index de6e71947..005f3aec5 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOpAnalyzer.php @@ -26,7 +26,7 @@ class BinaryOpAnalyzer bool $from_stmt = false ) : bool { if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Concat && $nesting > 100) { - $statements_analyzer->node_data->setType($stmt, Type::getBool()); + $statements_analyzer->node_data->setType($stmt, Type::getString()); // ignore deeply-nested string concatenation return true; @@ -116,7 +116,9 @@ class BinaryOpAnalyzer $new_parent_node = ControlFlowNode::getForAssignment('concat', $var_location); $statements_analyzer->control_flow_graph->addNode($new_parent_node); - $stmt_type->parent_nodes = [$new_parent_node]; + $stmt_type->parent_nodes = [ + $new_parent_node->id => $new_parent_node + ]; if ($stmt_left_type && $stmt_left_type->parent_nodes) { foreach ($stmt_left_type->parent_nodes as $parent_node) { @@ -139,6 +141,14 @@ class BinaryOpAnalyzer if ($stmt instanceof PhpParser\Node\Expr\BinaryOp\Spaceship) { $statements_analyzer->node_data->setType($stmt, Type::getInt()); + self::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + '<=>' + ); + return true; } @@ -278,6 +288,14 @@ class BinaryOpAnalyzer ); } + self::addControlFlow( + $statements_analyzer, + $stmt, + $stmt->left, + $stmt->right, + 'comparison' + ); + return true; } @@ -290,6 +308,47 @@ class BinaryOpAnalyzer return true; } + public static function addControlFlow( + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr $stmt, + PhpParser\Node\Expr $left, + PhpParser\Node\Expr $right, + string $type = 'binaryop' + ) : void { + if ($stmt->getLine() === -1) { + throw new \UnexpectedValueException('bad'); + } + $result_type = $statements_analyzer->node_data->getType($stmt); + + if ($statements_analyzer->control_flow_graph + && $result_type + ) { + $stmt_left_type = $statements_analyzer->node_data->getType($left); + $stmt_right_type = $statements_analyzer->node_data->getType($right); + + $var_location = new CodeLocation($statements_analyzer, $stmt); + + $new_parent_node = ControlFlowNode::getForAssignment($type, $var_location); + $statements_analyzer->control_flow_graph->addNode($new_parent_node); + + $result_type->parent_nodes = [ + $new_parent_node->id => $new_parent_node + ]; + + if ($stmt_left_type && $stmt_left_type->parent_nodes) { + foreach ($stmt_left_type->parent_nodes as $parent_node) { + $statements_analyzer->control_flow_graph->addPath($parent_node, $new_parent_node, $type); + } + } + + if ($stmt_right_type && $stmt_right_type->parent_nodes) { + foreach ($stmt_right_type->parent_nodes as $parent_node) { + $statements_analyzer->control_flow_graph->addPath($parent_node, $new_parent_node, $type); + } + } + } + } + private static function checkForImpureEqualityComparison( StatementsAnalyzer $statements_analyzer, PhpParser\Node\Expr\BinaryOp\Equal $stmt, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php index a576d7173..24576578f 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php @@ -639,25 +639,6 @@ class ArgumentsAnalyzer $arg_value_type = $statements_analyzer->node_data->getType($arg->value); - if ($method_id === 'compact' - && $arg_value_type - && $arg_value_type->isSingleStringLiteral() - ) { - $literal = $arg_value_type->getSingleStringLiteral(); - - if (!$context->hasVariable('$' . $literal->value, $statements_analyzer)) { - if (IssueBuffer::accepts( - new UndefinedVariable( - 'Cannot find referenced variable $' . $literal->value, - new CodeLocation($statements_analyzer->getSource(), $arg->value) - ), - $statements_analyzer->getSuppressedIssues() - )) { - // fall through - } - } - } - if (ArgumentAnalyzer::checkArgumentMatches( $statements_analyzer, $cased_method_id, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php index f21c913ae..9a2028288 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ClassTemplateParamCollector.php @@ -104,10 +104,8 @@ class ClassTemplateParamCollector } } - $i = 0; foreach ($template_types as $type_name => $_) { if (isset($class_template_params[$type_name])) { - $i++; continue; } @@ -190,8 +188,6 @@ class ClassTemplateParamCollector $class_storage->name => [Type::getMixed()] ]; } - - $i++; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index bdb26767f..027265a64 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -1076,7 +1076,7 @@ class FunctionCallAnalyzer extends CallAnalyzer $statements_analyzer->control_flow_graph->addNode($function_call_node); - $stmt_type->parent_nodes[] = $function_call_node; + $stmt_type->parent_nodes[$function_call_node->id] = $function_call_node; if ($function_storage->return_source_params) { $removed_taints = $function_storage->removed_taints; @@ -1464,6 +1464,56 @@ class FunctionCallAnalyzer extends CallAnalyzer $context->check_consts = false; } elseif ($function_name->parts === ['extract']) { $context->check_variables = false; + + foreach ($context->vars_in_scope as $var_id => $_) { + if ($var_id === '$this' || strpos($var_id, '[') || strpos($var_id, '>')) { + continue; + } + + $mixed_type = Type::getMixed(); + $mixed_type->parent_nodes = $context->vars_in_scope[$var_id]->parent_nodes; + + $context->vars_in_scope[$var_id] = $mixed_type; + $context->assigned_var_ids[$var_id] = true; + $context->possibly_assigned_var_ids[$var_id] = true; + } + } elseif ($function_name->parts === ['compact']) { + $all_args_string_literals = true; + $new_items = []; + + foreach ($stmt->args as $arg) { + $arg_type = $statements_analyzer->node_data->getType($arg->value); + + if (!$arg_type || !$arg_type->isSingleStringLiteral()) { + $all_args_string_literals = false; + break; + } + + $var_name = $arg_type->getSingleStringLiteral()->value; + + $new_items[] = new PhpParser\Node\Expr\ArrayItem( + new PhpParser\Node\Expr\Variable($var_name, $arg->value->getAttributes()), + new PhpParser\Node\Scalar\String_($var_name, $arg->value->getAttributes()), + false, + $arg->getAttributes() + ); + } + + if ($all_args_string_literals) { + $arr = new PhpParser\Node\Expr\Array_($new_items, $stmt->getAttributes()); + $old_node_data = $statements_analyzer->node_data; + $statements_analyzer->node_data = clone $statements_analyzer->node_data; + + ExpressionAnalyzer::analyze($statements_analyzer, $arr, $context); + + $arr_type = $statements_analyzer->node_data->getType($arr); + + $statements_analyzer->node_data = $old_node_data; + + if ($arr_type) { + $statements_analyzer->node_data->setType($stmt, $arr_type); + } + } } elseif (strtolower($function_name->parts[0]) === 'var_dump' || strtolower($function_name->parts[0]) === 'shell_exec') { if (IssueBuffer::accepts( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php index f69fe52cf..874be3d21 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/AtomicMethodCallAnalyzer.php @@ -221,6 +221,14 @@ class AtomicMethodCallAnalyzer extends CallAnalyzer ); } + ArgumentsAnalyzer::analyze( + $statements_analyzer, + $stmt->args, + null, + null, + $context + ); + $result->return_type = Type::getMixed(); return; } @@ -304,10 +312,12 @@ class AtomicMethodCallAnalyzer extends CallAnalyzer && $lhs_type_part instanceof Type\Atomic\TGenericObject && $class_storage->template_types ) { + $template_type_keys = \array_keys($class_storage->template_types); + foreach ($class_storage->templatedMixins as $mixin) { $param_position = \array_search( $mixin->param_name, - \array_keys($class_storage->template_types) + $template_type_keys ); if ($param_position !== false diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php index 45382dc2e..258ca5a69 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php @@ -246,7 +246,7 @@ class MethodCallReturnTypeFetcher $statements_analyzer->control_flow_graph->addNode($method_call_node); $return_type_candidate->parent_nodes = [ - $method_call_node + $method_call_node->id => $method_call_node ]; if ($method_storage->specialize_call) { @@ -278,7 +278,7 @@ class MethodCallReturnTypeFetcher } } - $stmt_var_type->parent_nodes = [$var_node]; + $stmt_var_type->parent_nodes = [$var_node->id => $var_node]; $context->vars_in_scope[$var_id] = $stmt_var_type; } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 44b368993..c44e2e765 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -673,7 +673,7 @@ class NewAnalyzer extends \Psalm\Internal\Analyzer\Statements\Expression\CallAna $statements_analyzer->control_flow_graph->addNode($method_source); - $stmt_type->parent_nodes = [$method_source]; + $stmt_type->parent_nodes = [$method_source->id => $method_source]; } } else { ArgumentsAnalyzer::analyze( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php index 7b2c2ace8..2a96cdcd4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticCallAnalyzer.php @@ -1452,7 +1452,7 @@ class StaticCallAnalyzer extends CallAnalyzer $statements_analyzer->control_flow_graph->addNode($method_source); - $return_type_candidate->parent_nodes = [$method_source]; + $return_type_candidate->parent_nodes = [$method_source->id => $method_source]; if ($method_storage && $method_storage->taint_source_types) { $method_node = TaintSource::getForMethodReturn( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php index 0a9486cfd..8f8dd8530 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php @@ -211,7 +211,7 @@ class CastAnalyzer if ($atomic_type instanceof TString) { $valid_strings[] = $atomic_type; if ($statements_analyzer->control_flow_graph) { - $parent_nodes = array_merge($parent_nodes, $stmt_type->parent_nodes ?: []); + $parent_nodes = $parent_nodes + $stmt_type->parent_nodes; } continue; @@ -230,7 +230,7 @@ class CastAnalyzer ) { $castable_types[] = new TString(); if ($statements_analyzer->control_flow_graph) { - $parent_nodes = array_merge($parent_nodes, $stmt_type->parent_nodes ?: []); + $parent_nodes = $parent_nodes + $stmt_type->parent_nodes; } continue; @@ -276,7 +276,7 @@ class CastAnalyzer ); if ($statements_analyzer->control_flow_graph) { - $parent_nodes = array_merge($return_type->parent_nodes ?: [], $parent_nodes); + $parent_nodes = array_merge($return_type->parent_nodes, $parent_nodes); } $castable_types = array_merge( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php index a9e0140e3..17cfae952 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/EncapsulatedStringAnalyzer.php @@ -41,7 +41,7 @@ class EncapsulatedStringAnalyzer $new_parent_node = ControlFlowNode::getForAssignment('concat', $var_location); $statements_analyzer->control_flow_graph->addNode($new_parent_node); - $stmt_type->parent_nodes[] = $new_parent_node; + $stmt_type->parent_nodes[$new_parent_node->id] = $new_parent_node; if ($casted_part_type->parent_nodes) { foreach ($casted_part_type->parent_nodes as $parent_node) { diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php index cd15a5481..744ba68f8 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ArrayFetchAnalyzer.php @@ -344,7 +344,7 @@ class ArrayFetchAnalyzer ); } - $stmt_type->parent_nodes = [$new_parent_node]; + $stmt_type->parent_nodes = [$new_parent_node->id => $new_parent_node]; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php index aed28a4f9..9f5c9c0aa 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php @@ -1252,7 +1252,7 @@ class InstancePropertyFetchAnalyzer } } - $type->parent_nodes = [$property_node]; + $type->parent_nodes = [$property_node->id => $property_node]; } } else { $code_location = new CodeLocation($statements_analyzer, $stmt->name); @@ -1281,7 +1281,7 @@ class InstancePropertyFetchAnalyzer $control_flow_graph->addPath($property_node, $localized_property_node, 'property-fetch'); } - $type->parent_nodes[] = $localized_property_node; + $type->parent_nodes[$localized_property_node->id] = $localized_property_node; } } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php index adeaccf07..9787f8469 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php @@ -132,7 +132,9 @@ class VariableFetchAnalyzer $context->vars_possibly_in_scope[$var_name] = true; $statements_analyzer->node_data->setType($stmt, Type::getMixed()); } else { - $statements_analyzer->node_data->setType($stmt, clone $context->vars_in_scope[$var_name]); + $stmt_type = clone $context->vars_in_scope[$var_name]; + + $statements_analyzer->node_data->setType($stmt, $stmt_type); } } else { $statements_analyzer->node_data->setType($stmt, Type::getMixed()); @@ -414,7 +416,7 @@ class VariableFetchAnalyzer $statements_analyzer->control_flow_graph->addSource($server_taint_source); $type->parent_nodes = [ - $server_taint_source + $server_taint_source->id => $server_taint_source ]; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php index 339837825..fa429b309 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/IncDecExpressionAnalyzer.php @@ -100,11 +100,13 @@ class IncDecExpressionAnalyzer $operation = $stmt instanceof PostInc || $stmt instanceof PreInc ? new PhpParser\Node\Expr\BinaryOp\Plus( $stmt->var, - $fake_right_expr + $fake_right_expr, + $stmt->var->getAttributes() ) : new PhpParser\Node\Expr\BinaryOp\Minus( $stmt->var, - $fake_right_expr + $fake_right_expr, + $stmt->var->getAttributes() ); $fake_assignment = new PhpParser\Node\Expr\Assign( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/YieldAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/YieldAnalyzer.php index 181296679..2c134c352 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/YieldAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/YieldAnalyzer.php @@ -104,6 +104,10 @@ class YieldAnalyzer } } + if (isset($context->vars_in_scope[$var_comment->var_id])) { + $comment_type->parent_nodes = $context->vars_in_scope[$var_comment->var_id]->parent_nodes; + } + $context->vars_in_scope[$var_comment->var_id] = $comment_type; } } diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 208e69cc9..101f5cb93 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -115,6 +115,10 @@ class ReturnAnalyzer continue; } + if (isset($context->vars_in_scope[$var_comment->var_id])) { + $comment_type->parent_nodes = $context->vars_in_scope[$var_comment->var_id]->parent_nodes; + } + $context->vars_in_scope[$var_comment->var_id] = $comment_type; } } @@ -136,11 +140,17 @@ class ReturnAnalyzer return false; } + $stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr); + if ($var_comment_type) { $stmt_type = $var_comment_type; + if ($stmt_expr_type && $stmt_expr_type->parent_nodes) { + $stmt_type->parent_nodes = $stmt_expr_type->parent_nodes; + } + $statements_analyzer->node_data->setType($stmt, $var_comment_type); - } elseif ($stmt_expr_type = $statements_analyzer->node_data->getType($stmt->expr)) { + } elseif ($stmt_expr_type) { $stmt_type = $stmt_expr_type; if ($stmt_type->isNever()) { diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php index 1896d0d9e..89f403496 100644 --- a/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/FilterVarReturnTypeProvider.php @@ -146,7 +146,7 @@ class FilterVarReturnTypeProvider implements \Psalm\Plugin\Hook\FunctionReturnTy 'arg' ); - $filter_type->parent_nodes = [$function_return_sink]; + $filter_type->parent_nodes = [$function_return_sink->id => $function_return_sink]; } return $filter_type; diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php index 4a6f88f01..def384949 100644 --- a/src/Psalm/Type.php +++ b/src/Psalm/Type.php @@ -490,9 +490,7 @@ abstract class Type } if ($type_1->parent_nodes || $type_2->parent_nodes) { - $combined_type->parent_nodes = \array_unique( - array_merge($type_1->parent_nodes ?: [], $type_2->parent_nodes ?: []) - ); + $combined_type->parent_nodes = $type_1->parent_nodes + $type_2->parent_nodes; } return $combined_type; diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php index 73a233788..581c9552b 100644 --- a/src/Psalm/Type/Union.php +++ b/src/Psalm/Type/Union.php @@ -161,9 +161,9 @@ class Union implements TypeNode private $id; /** - * @var ?array<\Psalm\Internal\ControlFlow\ControlFlowNode> + * @var array */ - public $parent_nodes; + public $parent_nodes = []; /** * @var bool diff --git a/tests/Loop/WhileTest.php b/tests/Loop/WhileTest.php index 40ce70dfa..13b1e33ee 100644 --- a/tests/Loop/WhileTest.php +++ b/tests/Loop/WhileTest.php @@ -464,6 +464,37 @@ class WhileTest extends \Psalm\Tests\TestCase } }' ], + 'nonEmptyListIterationChangeVarWithContinue' => [ + ' $arr */ + function foo(array $arr) : void { + while (array_shift($arr)) { + if ($arr && $arr[0] === "a") {} + + if (rand(0, 1)) { + $arr = array_merge($arr, ["a"]); + continue; + } + + echo "here"; + } + }' + ], + 'nonEmptyListIterationChangeVarWithoutContinue' => [ + ' $arr */ + function foo(array $arr) : void { + while (array_shift($arr)) { + if ($arr && $arr[0] === "a") {} + + if (rand(0, 1)) { + $arr = array_merge($arr, ["a"]); + } + + echo "here"; + } + }' + ], ]; }