diff --git a/config.xsd b/config.xsd
index 46840b0a3a9..9e5d5a24aff 100644
--- a/config.xsd
+++ b/config.xsd
@@ -403,6 +403,7 @@
+
diff --git a/docs/running_psalm/issues/ReferenceReusedFromConfusingScope.md b/docs/running_psalm/issues/ReferenceReusedFromConfusingScope.md
new file mode 100644
index 00000000000..b280e4948e2
--- /dev/null
+++ b/docs/running_psalm/issues/ReferenceReusedFromConfusingScope.md
@@ -0,0 +1,39 @@
+# ReferenceReusedFromConfusingScope
+
+Emitted when a reference assigned in a potentially confusing scope is reused later.
+Since PHP doesn't scope loops and ifs the same way most other languages do, a common issue is the reuse of a variable
+declared in such a scope. Usually it doesn't matter, because a reassignment to an already-defined variable will just
+reassign it, but if the already-defined variable is a reference it will change the value of the referred to variable.
+
+```php
+taint_flow_graph->addSource($source);
- $expr_type->parent_nodes = [
- $source->id => $source,
- ];
+ $expr_type->parent_nodes[$source->id] = $source;
}
/**
diff --git a/src/Psalm/Context.php b/src/Psalm/Context.php
index 78b116ba034..73b559173d9 100644
--- a/src/Psalm/Context.php
+++ b/src/Psalm/Context.php
@@ -14,6 +14,7 @@
use Psalm\Type\Atomic\DependentType;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Union;
+use RuntimeException;
use function array_keys;
use function array_search;
@@ -41,6 +42,54 @@ class Context
*/
public $vars_possibly_in_scope = [];
+ /**
+ * Keeps track of how many times a var_in_scope has been referenced. May not be set for all $vars_in_scope.
+ *
+ * @var array>
+ */
+ public $referenced_counts = [];
+
+ /**
+ * Maps references to referenced variables for the current scope.
+ * With `$b = &$a`, this will contain `['$b' => '$a']`.
+ *
+ * All keys and values in this array are guaranteed to be set in $vars_in_scope.
+ *
+ * To check if a variable was passed or returned by reference, or
+ * references an object property or array item, see Union::$by_ref.
+ *
+ * @var array
+ */
+ public $references_in_scope = [];
+
+ /**
+ * Set of references to variables in another scope. These references will be marked as used if they are assigned to.
+ *
+ * @var array
+ */
+ public $references_to_external_scope = [];
+
+ /**
+ * A set of globals that are referenced somewhere.
+ * Ideally this shouldn't be needed and GlobalAnalyzer should add an edge to the
+ * DataFlowGraph pointing from the global to its use in another scope, but since that's
+ * difficult this is used as a workaround to always mark referenced globals as used.
+ *
+ * @internal May be removed if GlobalAnalyzer is improved.
+ *
+ * @var array
+ */
+ public $referenced_globals = [];
+
+ /**
+ * A set of references that might still be in scope from a scope likely to cause confusion. This applies
+ * to references set inside a loop or if statement, since it's easy to forget about PHP's weird scope
+ * rules, and assinging to a reference will change the referenced variable rather than shadowing it.
+ *
+ * @var array
+ */
+ public $references_possibly_from_confusing_scope = [];
+
/**
* Whether or not we're inside the conditional of an if/where etc.
*
@@ -375,11 +424,6 @@ class Context
*/
public $has_returned = false;
- /**
- * @var array
- */
- public $vars_from_global = [];
-
public function __construct(?string $self = null)
{
$this->self = $self;
@@ -458,6 +502,28 @@ public function update(
}
}
+ /**
+ * Updates the list of possible references from a confusing scope,
+ * such as a reference created in an if and then later reused.
+ */
+ public function updateReferencesPossiblyFromConfusingScope(
+ Context $confusing_scope_context,
+ StatementsAnalyzer $statements_analyzer
+ ): void {
+ foreach ($confusing_scope_context->references_in_scope + $confusing_scope_context->references_to_external_scope
+ as $reference_id => $_
+ ) {
+ if (!isset($this->references_in_scope[$reference_id])
+ && !isset($this->references_to_external_scope[$reference_id])
+ && $reference_location = $statements_analyzer->getFirstAppearance($reference_id)
+ ) {
+ $this->references_possibly_from_confusing_scope[$reference_id] = $reference_location;
+ }
+ }
+ $this->references_possibly_from_confusing_scope +=
+ $confusing_scope_context->references_possibly_from_confusing_scope;
+ }
+
/**
* @param array $new_vars_in_scope
*
@@ -505,19 +571,39 @@ public static function getNewOrUpdatedVarIds(Context $original_context, Context
return $redefined_var_ids;
}
- public function remove(string $remove_var_id): void
+ public function remove(string $remove_var_id, bool $removeDescendents = true): void
{
- unset(
- $this->referenced_var_ids[$remove_var_id],
- $this->vars_possibly_in_scope[$remove_var_id]
- );
-
if (isset($this->vars_in_scope[$remove_var_id])) {
$existing_type = $this->vars_in_scope[$remove_var_id];
unset($this->vars_in_scope[$remove_var_id]);
- $this->removeDescendents($remove_var_id, $existing_type);
+ if ($removeDescendents) {
+ $this->removeDescendents($remove_var_id, $existing_type);
+ }
+ }
+ $this->removePossibleReference($remove_var_id);
+ unset($this->vars_possibly_in_scope[$remove_var_id]);
+ }
+
+ /**
+ * Remove a variable from the context which might be a reference to another variable.
+ * Leaves the variable as possibly-in-scope, unlike remove().
+ */
+ public function removePossibleReference(string $remove_var_id): void
+ {
+ if (isset($this->references_in_scope[$remove_var_id])) {
+ $reference_count = &$this->referenced_counts[$this->references_in_scope[$remove_var_id]];
+ if ($reference_count < 1) {
+ throw new RuntimeException("Incorrect referenced count found");
+ }
+ --$reference_count;
}
+ unset(
+ $this->vars_in_scope[$remove_var_id],
+ $this->referenced_var_ids[$remove_var_id],
+ $this->references_in_scope[$remove_var_id],
+ $this->references_to_external_scope[$remove_var_id],
+ );
}
/**
@@ -667,7 +753,7 @@ public function removeDescendents(
foreach ($this->vars_in_scope as $var_id => $type) {
if (preg_match('/' . preg_quote($remove_var_id, '/') . '[\]\[\-]/', $var_id)) {
- unset($this->vars_in_scope[$var_id]);
+ $this->remove($var_id, false);
}
foreach ($type->getAtomicTypes() as $atomic_type) {
@@ -698,7 +784,7 @@ public function removeMutableObjectVars(bool $methods_only = false): void
}
foreach ($vars_to_remove as $var_id) {
- unset($this->vars_in_scope[$var_id], $this->vars_possibly_in_scope[$var_id]);
+ $this->remove($var_id, false);
}
$clauses_to_keep = [];
diff --git a/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php b/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
index 1b36c9becf3..02c95ada4a4 100644
--- a/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
@@ -160,6 +160,7 @@ public static function analyzeExpression(
if ($use->byRef) {
$use_context->vars_in_scope[$use_var_id]->by_ref = true;
+ $use_context->references_to_external_scope[$use_var_id] = true;
}
$use_context->vars_possibly_in_scope[$use_var_id] = true;
diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
index f371bab0ec3..f6a452bba0b 100644
--- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -763,7 +763,7 @@ public function analyze(
foreach ($context->vars_in_scope as $var => $_) {
if (strpos($var, '$this->') !== 0 && $var !== '$this') {
- unset($context->vars_in_scope[$var]);
+ $context->removePossibleReference($var);
}
}
@@ -846,10 +846,6 @@ private function checkParamReferences(
throw new UnexpectedValueException('$position should not be false here');
}
- if ($storage->params[$position]->by_ref) {
- continue;
- }
-
if ($storage->params[$position]->promoted_property) {
continue;
}
@@ -976,6 +972,7 @@ private function processParams(
$project_analyzer = $statements_analyzer->getProjectAnalyzer();
foreach ($params as $offset => $function_param) {
+ $function_param_id = '$' . $function_param->name;
$signature_type = $function_param->signature_type;
$signature_type_location = $function_param->signature_type_location;
@@ -1064,7 +1061,7 @@ private function processParams(
&& !$function_param->type->isBool())
) {
$param_assignment = DataFlowNode::getForAssignment(
- '$' . $function_param->name,
+ $function_param_id,
$function_param->location
);
@@ -1082,16 +1079,6 @@ private function processParams(
$statements_analyzer->data_flow_graph->addPath($type_source, $param_assignment, 'param');
}
- if ($function_param->by_ref
- && $codebase->find_unused_variables
- ) {
- $statements_analyzer->data_flow_graph->addPath(
- $param_assignment,
- new DataFlowNode('variable-use', 'variable use', null),
- 'variable-use'
- );
- }
-
if ($storage->variadic) {
$this->param_nodes += [$param_assignment->id => $param_assignment];
}
@@ -1100,18 +1087,19 @@ private function processParams(
}
}
- $context->vars_in_scope['$' . $function_param->name] = $var_type;
- $context->vars_possibly_in_scope['$' . $function_param->name] = true;
+ $context->vars_in_scope[$function_param_id] = $var_type;
+ $context->vars_possibly_in_scope[$function_param_id] = true;
if ($function_param->by_ref) {
- $context->vars_in_scope['$' . $function_param->name]->by_ref = true;
+ $context->vars_in_scope[$function_param_id]->by_ref = true;
+ $context->references_to_external_scope[$function_param_id] = true;
}
$parser_param = $this->function->getParams()[$offset] ?? null;
if ($function_param->location) {
$statements_analyzer->registerVariable(
- '$' . $function_param->name,
+ $function_param_id,
$function_param->location,
null
);
@@ -1147,7 +1135,7 @@ private function processParams(
IssueBuffer::maybeAdd(
new MismatchingDocblockParamType(
- 'Parameter $' . $function_param->name . ' has wrong type \'' . $param_type .
+ 'Parameter ' . $function_param_id . ' has wrong type \'' . $param_type .
'\', should be \'' . $signature_type . '\'',
$function_param->type_location
),
@@ -1248,13 +1236,6 @@ private function processParams(
}
}
- if ($function_param->by_ref) {
- // register by ref params as having been used, to avoid false positives
- // @todo change the assignment analysis *just* for byref params
- // so that we don't have to do this
- $context->hasVariable('$' . $function_param->name);
- }
-
foreach ($function_param->attributes as $attribute) {
AttributeAnalyzer::analyze(
$this,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
index f03f3d0a0b5..31b465a10e6 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/ForeachAnalyzer.php
@@ -298,6 +298,15 @@ public static function analyze(
$value_type->by_ref = true;
}
+ if ($stmt->byRef
+ && $stmt->valueVar instanceof PhpParser\Node\Expr\Variable
+ && is_string($stmt->valueVar->name)
+ ) {
+ // When assigning as reference, it removes any previous
+ // reference, so it's no longer from a previous confusing scope
+ unset($foreach_context->references_possibly_from_confusing_scope['$' . $stmt->valueVar->name]);
+ }
+
AssignmentAnalyzer::analyze(
$statements_analyzer,
$stmt->valueVar,
@@ -311,6 +320,14 @@ public static function analyze(
: []
);
+ if ($stmt->byRef
+ && $stmt->valueVar instanceof PhpParser\Node\Expr\Variable
+ && is_string($stmt->valueVar->name)
+ ) {
+ // TODO support references with destructuring
+ $foreach_context->references_to_external_scope['$' . $stmt->valueVar->name] = true;
+ }
+
foreach ($var_comments as $var_comment) {
if (!$var_comment->var_id || !$var_comment->type) {
continue;
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
index 5944847fdff..dba64ad4ba6 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
@@ -97,7 +97,7 @@ public static function analyze(
if (preg_match('/' . preg_quote($changed_var_id, '/') . '[\]\[\-]/', $var_id)
&& !array_key_exists($var_id, $changed_var_ids)
) {
- unset($else_context->vars_in_scope[$var_id]);
+ $else_context->removePossibleReference($var_id);
}
}
}
@@ -239,6 +239,12 @@ public static function analyze(
$outer_context->mergeExceptions($else_context);
}
+ // Track references set in the else to make sure they aren't reused later
+ $outer_context->updateReferencesPossiblyFromConfusingScope(
+ $else_context,
+ $statements_analyzer
+ );
+
return null;
}
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
index bac5b65fd84..9fb78a41cc7 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
@@ -255,7 +255,7 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl
&& !array_key_exists($var_id, $newly_reconciled_var_ids)
&& !array_key_exists($var_id, $cond_referenced_var_ids)
) {
- unset($elseif_context->vars_in_scope[$var_id]);
+ $elseif_context->removePossibleReference($var_id);
}
}
}
@@ -430,6 +430,12 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl
$if_scope->negated_clauses = [];
}
+ // Track references set in the elseif to make sure they aren't reused later
+ $outer_context->updateReferencesPossiblyFromConfusingScope(
+ $elseif_context,
+ $statements_analyzer
+ );
+
return null;
}
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
index f5e7e79f5fc..7af37ad3c35 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
@@ -252,6 +252,12 @@ public static function analyze(
$outer_context->mergeExceptions($if_context);
}
+ // Track references set in the if to make sure they aren't reused later
+ $outer_context->updateReferencesPossiblyFromConfusingScope(
+ $if_context,
+ $statements_analyzer
+ );
+
return null;
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
index 221ab6ca49d..f12036fefa7 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
@@ -297,7 +297,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
&& !array_key_exists($var_id, $changed_var_ids)
&& !array_key_exists($var_id, $cond_referenced_var_ids)
) {
- unset($if_context->vars_in_scope[$var_id]);
+ $if_context->removePossibleReference($var_id);
}
}
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
index a5ed096112a..099a581f4a4 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
@@ -321,7 +321,7 @@ public static function analyze(
// remove vars that were defined in the foreach
foreach ($vars_to_remove as $var_id) {
- unset($inner_context->vars_in_scope[$var_id]);
+ $inner_context->removePossibleReference($var_id);
}
$inner_context->clauses = $pre_loop_context->clauses;
@@ -364,7 +364,7 @@ public static function analyze(
if (isset($pre_loop_context->vars_in_scope[$var_id])) {
$inner_context->vars_in_scope[$var_id] = clone $pre_loop_context->vars_in_scope[$var_id];
} else {
- unset($inner_context->vars_in_scope[$var_id]);
+ $inner_context->removePossibleReference($var_id);
}
}
}
@@ -459,7 +459,7 @@ public static function analyze(
if (!$does_always_break) {
foreach ($loop_scope->loop_parent_context->vars_in_scope as $var_id => $type) {
if (!isset($inner_context->vars_in_scope[$var_id])) {
- unset($loop_scope->loop_parent_context->vars_in_scope[$var_id]);
+ $loop_scope->loop_parent_context->removePossibleReference($var_id);
continue;
}
@@ -561,6 +561,12 @@ public static function analyze(
$inner_context = $inner_do_context;
}
+ // Track references set in the loop to make sure they aren't reused later
+ $loop_scope->loop_parent_context->updateReferencesPossiblyFromConfusingScope(
+ $inner_context,
+ $statements_analyzer
+ );
+
return null;
}
@@ -638,7 +644,7 @@ private static function applyPreConditionToLoopContext(
if (ExpressionAnalyzer::analyze($statements_analyzer, $pre_condition, $loop_context) === false) {
$loop_context->inside_conditional = $was_inside_conditional;
-
+
return [];
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
index 7652589699b..b4d7c3dace1 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/ArrayAssignmentAnalyzer.php
@@ -4,6 +4,7 @@
use InvalidArgumentException;
use PhpParser;
+use PhpParser\Node\Expr\Variable;
use Psalm\CodeLocation;
use Psalm\Codebase;
use Psalm\Context;
@@ -46,6 +47,7 @@
use function in_array;
use function is_string;
use function preg_match;
+use function reset;
use function strlen;
/**
@@ -690,6 +692,10 @@ private static function analyzeNestedArrayAssignment(
$child_stmt = null;
+ if (!empty($child_stmts)) {
+ $root_var = reset($child_stmts)->var;
+ }
+
// First go from the root element up, and go as far as we can to figure out what
// array types there are
while ($child_stmts) {
@@ -816,6 +822,23 @@ private static function analyzeNestedArrayAssignment(
$parent_var_id = $array_var_id;
}
+ if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
+ && $root_var_id !== null
+ && isset($context->references_to_external_scope[$root_var_id])
+ && isset($root_var) && $root_var instanceof Variable && is_string($root_var->name)
+ && $root_var_id === '$' . $root_var->name
+ ) {
+ // Array is a reference to an external scope, mark it as used
+ $statements_analyzer->data_flow_graph->addPath(
+ DataFlowNode::getForAssignment(
+ $root_var_id,
+ new CodeLocation($statements_analyzer->getSource(), $root_var)
+ ),
+ new DataFlowNode('variable-use', 'variable use', null),
+ 'variable-use'
+ );
+ }
+
if ($root_var_id
&& $full_var_id
&& $child_stmt
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php
index 12a09c8d7b7..46449085797 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php
@@ -1494,7 +1494,7 @@ private static function analyzeSetCall(
$statements_analyzer
);
- unset($context->vars_in_scope[$var_id]);
+ $context->removePossibleReference($var_id);
}
$old_data_provider = $statements_analyzer->node_data;
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
index 6cb610b2b1e..cca39e58457 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -49,6 +49,7 @@
use Psalm\Issue\PossiblyNullArrayAccess;
use Psalm\Issue\PossiblyUndefinedArrayOffset;
use Psalm\Issue\ReferenceConstraintViolation;
+use Psalm\Issue\ReferenceReusedFromConfusingScope;
use Psalm\Issue\UnnecessaryVarAnnotation;
use Psalm\IssueBuffer;
use Psalm\Node\Expr\BinaryOp\VirtualBitwiseAnd;
@@ -77,6 +78,7 @@
use Psalm\Type\Atomic\TNonEmptyList;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Union;
+use RuntimeException;
use UnexpectedValueException;
use function array_filter;
@@ -847,20 +849,7 @@ public static function analyzeAssignmentRef(
PhpParser\Node\Expr\AssignRef $stmt,
Context $context
): bool {
- $assignment_type = self::analyze(
- $statements_analyzer,
- $stmt->var,
- $stmt->expr,
- null,
- $context,
- $stmt->getDocComment()
- );
-
- if ($assignment_type === false) {
- return false;
- }
-
- $assignment_type->by_ref = true;
+ ExpressionAnalyzer::analyze($statements_analyzer, $stmt->expr, $context, false, null, false, true);
$lhs_var_id = ExpressionIdentifier::getArrayVarId(
$stmt->var,
@@ -874,47 +863,86 @@ public static function analyzeAssignmentRef(
$statements_analyzer
);
- if ($lhs_var_id) {
- $context->vars_in_scope[$lhs_var_id] = $assignment_type;
- $context->hasVariable($lhs_var_id);
- $context->byref_constraints[$lhs_var_id] = new ReferenceConstraint();
-
- if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
- foreach ($context->vars_in_scope[$lhs_var_id]->parent_nodes as $parent_node) {
- $statements_analyzer->data_flow_graph->addPath(
- $parent_node,
- new DataFlowNode('variable-use', 'variable use', null),
- 'variable-use'
- );
- }
+ $doc_comment = $stmt->getDocComment();
+ if ($doc_comment) {
+ try {
+ $var_comments = CommentAnalyzer::getTypeFromComment(
+ $doc_comment,
+ $statements_analyzer->getSource(),
+ $statements_analyzer->getAliases(),
+ );
+ } catch (IncorrectDocblockException $e) {
+ IssueBuffer::maybeAdd(
+ new MissingDocblockType(
+ $e->getMessage(),
+ new CodeLocation($statements_analyzer->getSource(), $stmt)
+ )
+ );
+ } catch (DocblockParseException $e) {
+ IssueBuffer::maybeAdd(
+ new InvalidDocblock(
+ $e->getMessage(),
+ new CodeLocation($statements_analyzer->getSource(), $stmt)
+ )
+ );
}
- }
-
- if ($rhs_var_id) {
- if (!isset($context->vars_in_scope[$rhs_var_id])) {
- $context->vars_in_scope[$rhs_var_id] = Type::getMixed();
+ if (!empty($var_comments)) {
+ IssueBuffer::maybeAdd(
+ new InvalidDocblock(
+ "Docblock type cannot be used for reference assignment",
+ new CodeLocation($statements_analyzer->getSource(), $stmt)
+ )
+ );
}
-
- $context->byref_constraints[$rhs_var_id] = new ReferenceConstraint();
}
- if ($statements_analyzer->data_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];
-
- $data_flow_graph = $statements_analyzer->data_flow_graph;
-
- $lhs_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var);
+ if ($lhs_var_id === null || $rhs_var_id === null) {
+ return false;
+ }
- $lhs_node = DataFlowNode::getForAssignment($lhs_var_id, $lhs_location);
+ if (isset($context->references_in_scope[$lhs_var_id])) {
+ // Decrement old referenced variable's reference count
+ $reference_count = &$context->referenced_counts[$context->references_in_scope[$lhs_var_id]];
+ if ($reference_count < 1) {
+ throw new RuntimeException("Incorrect referenced count found");
+ }
+ --$reference_count;
- foreach ($rhs_type->parent_nodes as $byref_destination_node) {
- $data_flow_graph->addPath($lhs_node, $byref_destination_node, '=');
+ // Remove old reference parent node so previously referenced variable usage doesn't count as reference usage
+ $old_type = $context->vars_in_scope[$lhs_var_id];
+ foreach ($old_type->parent_nodes as $old_parent_node_id => $_) {
+ if (strpos($old_parent_node_id, "$lhs_var_id-") === 0) {
+ unset($old_type->parent_nodes[$old_parent_node_id]);
+ }
}
}
+ // When assigning an existing reference as a reference it removes the
+ // old reference, so it's no longer potentially from a confusing scope.
+ unset($context->references_possibly_from_confusing_scope[$lhs_var_id]);
+
+ $context->vars_in_scope[$lhs_var_id] = &$context->vars_in_scope[$rhs_var_id];
+ $context->hasVariable($lhs_var_id);
+ $context->references_in_scope[$lhs_var_id] = $rhs_var_id;
+ $context->referenced_counts[$rhs_var_id] = ($context->referenced_counts[$rhs_var_id] ?? 0) + 1;
+ if (strpos($rhs_var_id, '[') !== false) {
+ // Reference to array item, we always consider array items to be an external scope for references
+ // TODO handle differently so it's detected as unused if the array is unused?
+ $context->references_to_external_scope[$lhs_var_id] = true;
+ }
+ if (strpos($rhs_var_id, '->') !== false) {
+ // Reference to object property, we always consider object properties to be an external scope for references
+ // TODO handle differently so it's detected as unused if the object is unused?
+ $context->references_to_external_scope[$lhs_var_id] = true;
+ }
+
+ $lhs_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var);
+ $statements_analyzer->registerVariableAssignment(
+ $lhs_var_id,
+ $lhs_location
+ );
+
+ $lhs_node = DataFlowNode::getForAssignment($lhs_var_id, $lhs_location);
+ $context->vars_in_scope[$lhs_var_id]->parent_nodes[$lhs_node->id] = $lhs_node;
return true;
}
@@ -1595,6 +1623,7 @@ private static function analyzeAssignmentToVariable(
): void {
if (is_string($assign_var->name)) {
if ($var_id) {
+ $original_type = $context->vars_in_scope[$var_id] ?? null;
$context->vars_in_scope[$var_id] = $assign_value_type;
$context->vars_possibly_in_scope[$var_id] = true;
@@ -1631,30 +1660,50 @@ private static function analyzeAssignmentToVariable(
$assign_value_type->by_ref = true;
}
- if ($assign_value_type->by_ref) {
- if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
- && $assign_value_type->parent_nodes
+ if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph
+ && $assign_value_type->parent_nodes
+ ) {
+ if (isset($context->references_to_external_scope[$var_id])
+ || isset($context->references_in_scope[$var_id])
+ || isset($context->referenced_counts[$var_id]) && $context->referenced_counts[$var_id] > 0
) {
$location = new CodeLocation($statements_analyzer, $assign_var);
-
- $byref_node = DataFlowNode::getForAssignment($var_id, $location);
-
- foreach ($assign_value_type->parent_nodes as $parent_node) {
+ $assignment_node = DataFlowNode::getForAssignment($var_id, $location);
+ $parent_nodes = $assign_value_type->parent_nodes;
+ if ($original_type !== null) {
+ $parent_nodes += $original_type->parent_nodes;
+ }
+ foreach ($parent_nodes as $parent_node) {
$statements_analyzer->data_flow_graph->addPath(
$parent_node,
- new DataFlowNode('variable-use', 'variable use', null),
- 'variable-use'
+ $assignment_node,
+ '&=' // Normal assignment to reference/referenced variable
);
+ }
+ if (isset($context->references_to_external_scope[$var_id])) {
+ // Mark reference to an external scope as used when a value is assigned to it
$statements_analyzer->data_flow_graph->addPath(
- $byref_node,
- $parent_node,
- 'byref-assignment'
+ $assignment_node,
+ new DataFlowNode('variable-use', 'variable use', null),
+ 'variable-use'
);
}
}
}
+ if (isset($context->references_possibly_from_confusing_scope[$var_id])) {
+ IssueBuffer::maybeAdd(
+ new ReferenceReusedFromConfusingScope(
+ "$var_id is possibly a reference defined at"
+ . " {$context->references_possibly_from_confusing_scope[$var_id]->getShortSummary()}."
+ . " Reusing this variable may cause the referenced value to change.",
+ new CodeLocation($statements_analyzer, $assign_var)
+ ),
+ $statements_analyzer->getSuppressedIssues()
+ );
+ }
+
if ($assign_value_type->getId() === 'bool'
&& ($assign_value instanceof PhpParser\Node\Expr\BinaryOp
|| ($assign_value instanceof PhpParser\Node\Expr\BooleanNot
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
index 941d88f7088..e40ce54eaf6 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
@@ -228,8 +228,7 @@ public static function collectSpecialInformation(
if ($type->initialized) {
$local_vars_in_scope[$var_id] = $context->vars_in_scope[$var_id];
- unset($context->vars_in_scope[$var_id]);
- unset($context->vars_possibly_in_scope[$var_id]);
+ $context->remove($var_id, false);
}
} elseif ($var_id !== '$this') {
$local_vars_in_scope[$var_id] = $context->vars_in_scope[$var_id];
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
index ee9f642c186..ff04c3f4596 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
@@ -74,6 +74,7 @@ class AtomicPropertyFetchAnalyzer
{
/**
* @param array $invalid_fetch_types $invalid_fetch_types
+ * @psalm-suppress ComplexMethod
*/
public static function analyze(
StatementsAnalyzer $statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
index 3ecde22a7f9..331c818c709 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/VariableFetchAnalyzer.php
@@ -49,6 +49,8 @@ class VariableFetchAnalyzer
/**
* @param bool $from_global - when used in a global keyword
+ * @param bool $assigned_to_reference This is set to true when the expression being analyzed
+ * here is being assigned to another variable by reference.
*/
public static function analyze(
StatementsAnalyzer $statements_analyzer,
@@ -57,7 +59,8 @@ public static function analyze(
bool $passed_by_reference = false,
?Union $by_ref_type = null,
bool $array_assignment = false,
- bool $from_global = false
+ bool $from_global = false,
+ bool $assigned_to_reference = false
): bool {
$project_analyzer = $statements_analyzer->getFileAnalyzer()->project_analyzer;
$codebase = $statements_analyzer->getCodebase();
@@ -220,11 +223,16 @@ public static function analyze(
if (!isset($context->vars_possibly_in_scope[$var_name])
|| !$statements_analyzer->getFirstAppearance($var_name)
) {
- if ($array_assignment) {
- // if we're in an array assignment, let's assign the variable
- // because PHP allows it
-
- $context->vars_in_scope[$var_name] = Type::getArray();
+ if ($array_assignment || $assigned_to_reference) {
+ if ($array_assignment) {
+ // if we're in an array assignment, let's assign the variable because PHP allows it
+ $stmt_type = Type::getArray();
+ } else {
+ // If a variable is assigned by reference to a variable that
+ // does not exist, they are automatically initialized as `null`
+ $stmt_type = Type::getNull();
+ }
+ $context->vars_in_scope[$var_name] = $stmt_type;
$context->vars_possibly_in_scope[$var_name] = true;
// it might have been defined first in another if/else branch
@@ -235,6 +243,16 @@ public static function analyze(
$context->branch_point
);
}
+ $statements_analyzer->node_data->setType($stmt, $stmt_type);
+
+ if ($assigned_to_reference) {
+ // Since this variable was created by being assigned to as a reference (ie for
+ // `$a = &$b` this variable is $b), we need to analyze it as an assignment to null.
+ AssignmentAnalyzer::analyze($statements_analyzer, $stmt, null, $stmt_type, $context, null);
+
+ // Stop here, we don't want it to be considered possibly undefined like the array case.
+ return true;
+ }
} elseif (!$context->inside_isset
|| $statements_analyzer->getSource() instanceof FunctionLikeAnalyzer
) {
diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
index 552a3ecaedf..69454eb0078 100644
--- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
@@ -64,13 +64,18 @@
*/
class ExpressionAnalyzer
{
+ /**
+ * @param bool $assigned_to_reference This is set to true when the expression being analyzed
+ * here is being assigned to another variable by reference.
+ */
public static function analyze(
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Expr $stmt,
Context $context,
bool $array_assignment = false,
?Context $global_context = null,
- bool $from_stmt = false
+ bool $from_stmt = false,
+ bool $assigned_to_reference = false
): bool {
$codebase = $statements_analyzer->getCodebase();
@@ -80,7 +85,8 @@ public static function analyze(
$context,
$array_assignment,
$global_context,
- $from_stmt
+ $from_stmt,
+ $assigned_to_reference
) === false
) {
return false;
@@ -138,13 +144,18 @@ public static function analyze(
return true;
}
+ /**
+ * @param bool $assigned_to_reference This is set to true when the expression being analyzed
+ * here is being assigned to another variable by reference.
+ */
private static function handleExpression(
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Expr $stmt,
Context $context,
bool $array_assignment,
?Context $global_context,
- bool $from_stmt
+ bool $from_stmt,
+ bool $assigned_to_reference = false
): bool {
if ($stmt instanceof PhpParser\Node\Expr\Variable) {
return VariableFetchAnalyzer::analyze(
@@ -153,7 +164,9 @@ private static function handleExpression(
$context,
false,
null,
- $array_assignment
+ $array_assignment,
+ false,
+ $assigned_to_reference
);
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php
index 1781f242d72..3ef2635c8ff 100644
--- a/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php
@@ -8,10 +8,12 @@
use Psalm\Internal\Analyzer\FunctionLikeAnalyzer;
use Psalm\Internal\Analyzer\Statements\Expression\Fetch\VariableFetchAnalyzer;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
+use Psalm\Internal\Codebase\VariableUseGraph;
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\ReferenceConstraint;
use Psalm\Issue\InvalidGlobal;
use Psalm\IssueBuffer;
+use RuntimeException;
use function is_string;
@@ -61,6 +63,7 @@ public static function analyze(
$context->byref_constraints[$var_id] = new ReferenceConstraint();
}
+
$assignment_node = DataFlowNode::getForAssignment(
$var_id,
new CodeLocation($statements_analyzer, $var)
@@ -68,7 +71,17 @@ public static function analyze(
$context->vars_in_scope[$var_id]->parent_nodes = [
$assignment_node->id => $assignment_node,
];
- $context->vars_from_global[$var_id] = true;
+ $context->references_to_external_scope[$var_id] = true;
+
+ if (isset($context->references_in_scope[$var_id])) {
+ // Global shadows existing reference
+ $reference_count = &$context->referenced_counts[$context->references_in_scope[$var_id]];
+ if ($reference_count < 1) {
+ throw new RuntimeException("Incorrect referenced count found");
+ }
+ --$reference_count;
+ unset($context->references_in_scope[$var_id]);
+ }
$statements_analyzer->registerVariable(
$var_id,
new CodeLocation($statements_analyzer, $var),
@@ -79,6 +92,20 @@ public static function analyze(
$var,
$var_id
);
+
+ if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) {
+ if ($global_context !== null && $global_context->hasVariable($var_id)) {
+ // TODO get global CodeLocation so that global variable can be marked as unused if it is
+ // unused inside the function. Marking it as a referenced global causes it to be marked
+ // used if it's used as a global in any function, even if it's unused in that function.
+ $global_context->referenced_globals[$var_id] = true;
+ // $statements_analyzer->data_flow_graph->addPath(
+ // $assignment_node,
+ // $global_node,
+ // 'byref-assignment'
+ // );
+ }
+ }
}
}
}
diff --git a/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
index b11ce758a63..d580f6c9938 100644
--- a/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/UnsetAnalyzer.php
@@ -47,6 +47,7 @@ public static function analyze(
if ($var_id) {
$context->remove($var_id);
+ unset($context->references_possibly_from_confusing_scope[$var_id]);
}
if ($var instanceof PhpParser\Node\Expr\ArrayDimFetch && $var->dim) {
diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php
index 10ea738065a..3eb0c6b95d0 100644
--- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php
@@ -131,9 +131,9 @@ class StatementsAnalyzer extends SourceAnalyzer
private $unused_var_locations = [];
/**
- * @var ?array
+ * @var array
*/
- public $byref_uses;
+ public $byref_uses = [];
/**
* @var ParsedDocblock|null
@@ -214,7 +214,6 @@ public function analyze(
&& $codebase->find_unused_variables
&& $context->check_variables
) {
- //var_dump($this->data_flow_graph);
$this->checkUnreferencedVars($stmts, $context);
}
@@ -790,7 +789,7 @@ public function checkUnreferencedVars(array $stmts, Context $context): void
$assignment_node = DataFlowNode::getForAssignment($var_id, $original_location);
if (!isset($this->byref_uses[$var_id])
- && !isset($context->vars_from_global[$var_id])
+ && !isset($context->referenced_globals[$var_id])
&& !VariableFetchAnalyzer::isSuperGlobal($var_id)
&& $this->data_flow_graph instanceof VariableUseGraph
&& !$this->data_flow_graph->isVariableUsed($assignment_node)
@@ -955,7 +954,7 @@ public function getFunctionAnalyzers(): array
}
/**
- * @param array $byref_uses
+ * @param array $byref_uses
*/
public function setByRefUses(array $byref_uses): void
{
diff --git a/src/Psalm/Internal/Diff/FileStatementsDiffer.php b/src/Psalm/Internal/Diff/FileStatementsDiffer.php
index 9580d7f634e..d5decb928d9 100644
--- a/src/Psalm/Internal/Diff/FileStatementsDiffer.php
+++ b/src/Psalm/Internal/Diff/FileStatementsDiffer.php
@@ -34,8 +34,7 @@ function (
PhpParser\Node\Stmt $a,
PhpParser\Node\Stmt $b,
string $a_code,
- string $b_code,
- bool &$body_change = false
+ string $b_code
): bool {
if (get_class($a) !== get_class($b)) {
return false;
diff --git a/src/Psalm/Internal/Diff/NamespaceStatementsDiffer.php b/src/Psalm/Internal/Diff/NamespaceStatementsDiffer.php
index 5ac1e8a9dcb..77eec387528 100644
--- a/src/Psalm/Internal/Diff/NamespaceStatementsDiffer.php
+++ b/src/Psalm/Internal/Diff/NamespaceStatementsDiffer.php
@@ -34,8 +34,7 @@ function (
PhpParser\Node\Stmt $a,
PhpParser\Node\Stmt $b,
string $a_code,
- string $b_code,
- bool &$body_change = false
+ string $b_code
): bool {
if (get_class($a) !== get_class($b)) {
return false;
diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
index baf3479524c..120aade190f 100644
--- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
+++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
@@ -393,7 +393,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
null
);
@@ -406,7 +405,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
(int) substr($assertion, 13)
);
@@ -532,7 +530,6 @@ private static function reconcileIsset(
/**
* @param string[] $suppressed_issues
- * @param 0|1|2 $failed_reconciliation
*/
private static function reconcileNonEmptyCountable(
Union $existing_var_type,
@@ -540,7 +537,6 @@ private static function reconcileNonEmptyCountable(
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues,
- int &$failed_reconciliation,
bool $is_equality,
?int $min_count
): Union {
diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
index 72b5a11488d..28fbf4b8572 100644
--- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
+++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
@@ -323,7 +323,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
null
);
@@ -342,7 +341,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
(int) substr($assertion, 13)
);
@@ -467,7 +465,6 @@ private static function reconcileNonEmptyCountable(
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues,
- int &$failed_reconciliation,
bool $is_equality,
?int $min_count
): Union {
diff --git a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php
index 939388c3429..81b64f9f434 100644
--- a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php
+++ b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php
@@ -213,8 +213,7 @@ private static function handleAtomicStandin(
$add_lower_bound,
$bound_equality_classlike,
$depth,
- $was_single,
- $had_template
+ $was_single
);
}
}
@@ -901,7 +900,6 @@ public static function handleTemplateParamClassStandin(
?string $bound_equality_classlike,
int $depth,
bool $was_single,
- bool &$had_template
): array {
if ($atomic_type->defining_class === $calling_class) {
return [$atomic_type];
diff --git a/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php b/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php
new file mode 100644
index 00000000000..31332b98bc0
--- /dev/null
+++ b/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php
@@ -0,0 +1,9 @@
+ [
' [
' [
' [
' [
+ 'SKIPPED-removeUnusedVarAssignByRefToSubsequentlyUsedVariable' => [
' [
+ ' [
',error_levels?:string[]}>
+ */
+ public function providerValidCodeParse(): iterable
+ {
+ return [
+ 'updateReferencedTypes' => [
+ ' [
+ '$a===' => '2',
+ '$b===' => '4',
+ '$c===' => '4',
+ ],
+ ],
+ 'SKIPPED-referenceToArrayItemChangesArrayType' => [
+ ' */
+ $arr = [];
+
+ assert(isset($arr[0]));
+ $int = &$arr[0];
+ $int = (string) $int;
+ ',
+ 'assertions' => [
+ '$arr' => 'list',
+ ],
+ ],
+ 'referenceToReference' => [
+ ' [
+ '$a===' => '2',
+ '$b===' => '3',
+ '$c===' => '2',
+ '$d===' => '3',
+ ],
+ ],
+ 'referenceToSubsequentCatch' => [
+ ' [
+ ' [
+ ' [
+ '$a===' => 'null',
+ '$b===' => 'null',
+ ],
+ ],
+ 'referenceShadowedByGlobal' => [
+ ' [
+ '$a' => 'string',
+ ],
+ ],
+ 'unsetPreventsReferenceConfusion' => [
+ ' [
+ ' [
+ '
+ */
+ public function providerInvalidCodeParse(): iterable
+ {
+ return [
+ 'referenceReuseForeachValue' => [
+ ' */
+ $arr = [];
+
+ foreach ($arr as &$var) {
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInForeach' => [
+ ' */
+ $arr = [];
+
+ foreach ($arr as $val) {
+ $var = &$val;
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInFor' => [
+ ' */
+ $arr = [];
+
+ for ($i = 0; $i < count($arr); ++$i) {
+ $var = &$arr[$i];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInIf' => [
+ ' */
+ $arr = [];
+
+ if (isset($arr[0])) {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInElseif' => [
+ ' */
+ $arr = [];
+
+ if (random_int(0, 1)) {
+ } elseif (isset($arr[0])) {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInElse' => [
+ ' */
+ $arr = [];
+
+ if (!isset($arr[0])) {
+ } else {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeeplyNested' => [
+ '>> */
+ $arr = [];
+
+ for ($i = 0; $i < count($arr); ++$i) {
+ foreach ($arr[$i] as $inner_arr) {
+ if (isset($inner_arr[0])) {
+ $var = &$inner_arr[0];
+ $var += 1;
+ }
+ }
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referencesIgnoreVarAnnotation' => [
+ ' 'InvalidDocblock - src' . DIRECTORY_SEPARATOR . 'somefile.php:4:21 - Docblock type cannot be used for reference assignment',
+ ],
+ 'SKIPPED-referenceToTypedArrayConstrainsAssignment' => [
+ ' */
+ public array $arr = [];
+
+ public function __construct()
+ {
+ assert(isset($this->arr[0]));
+ $int = &$this->arr[0];
+ $int = (string) $int;
+ }
+ }
+ ',
+ 'error_message' => 'ReferenceConstraintViolation',
+ ],
+ 'SKIPPED-referenceToTypedArrayConstrainsAssignmentWithNullReferenceInitialization' => [
+ ' */
+ public array $arr = [];
+
+ public function __construct()
+ {
+ $int = &$this->arr[0]; // If $this->arr[0] isn\'t set, this will set it to null.
+ }
+ }
+ ',
+ 'error_message' => 'PossiblyInvalidPropertyAssignmentValue',
+ ],
+ 'unsetOnlyPreventsReferenceConfusionAfterCall' => [
+ ' 'ReferenceReusedFromConfusingScope',
+ ],
+ 'assignmentAsReferenceOnlyPreventsReferenceConfusionAfterAssignment' => [
+ ' 'ReferenceReusedFromConfusingScope',
+ ],
+ ];
+ }
+}
diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php
index 055be038db8..2c48e2eef52 100644
--- a/tests/UnusedVariableTest.php
+++ b/tests/UnusedVariableTest.php
@@ -1522,7 +1522,10 @@ function foo(array $arr) : void {
}
}
- /** @param mixed $p */
+ /**
+ * @param mixed $p
+ * @psalm-suppress UnusedParam
+ */
function takes_ref(&$p): void {}'
],
'passedByRefArrayOffset' => [
@@ -1952,6 +1955,23 @@ function foo(array &$arr): void {
$arr[0] = $b;
}'
],
+ 'byRefDeeplyNestedArrayParam' => [
+ '> $arr */
+ function foo(array &$arr): void {
+ $b = 5;
+ $arr[0][0] = $b;
+ }'
+ ],
+ 'nestedReferencesToByRefParam' => [
+ '> $arr */
+ function foo(array &$arr): void {
+ $a = &$arr[0];
+ $b = &$a[0];
+ $b = 5;
+ }'
+ ],
'byRefNestedArrayInForeach' => [
' [
+ ' [
+ ' [
+ '$b===' => '2',
+ '$a===' => '2',
+ ],
+ ],
+ 'referenceUsedAfterVariableReassignment' => [
+ ' [
+ ' [
+ ' 'MixedArgumentTypeCoercion - src' . DIRECTORY_SEPARATOR . 'somefile.php:12:44 - Argument 1 of takesArrayOfString expects array, parent type array{mixed} provided. Consider improving the type at src' . DIRECTORY_SEPARATOR . 'somefile.php:10:41'
],
+ 'referenceReassignmentUnusedVariable' => [
+ ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3:21 - $c',
+ ],
+ 'referenceAssignmentIsNotUsed' => [
+ ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - $a',
+ ],
+ 'unusedReferenceToPreviouslyUsedVariable' => [
+ ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:4:21 - $b',
+ ],
+ 'SKIPPED-unusedReferenceToSubsequentlyUsedVariable' => [ // Not easy to do the way it's currently set up
+ ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3:21 - $b',
+ ],
+ 'unusedReferenceInForeach' => [
+ ' 'UnusedForeachValue',
+ ],
+ 'SKIPPED-unusedReferenceInDestructuredForeach' => [
+ ' 'UnusedForeachValue',
+ ],
+ 'unusedReturnByReference' => [
+ ' 'UnusedVariable',
+ ],
+ 'unusedPassByReference' => [
+ ' 'UnusedParam',
+ ],
+ 'SKIPPED-unusedGlobalVariable' => [
+ ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - $a',
+ ],
+ 'unusedUndeclaredGlobalVariable' => [
+ ' 'UnusedVariable',
+ ],
];
}
}