diff --git a/UPGRADING.md b/UPGRADING.md
index f278fe8b219..8dafd844e3b 100644
--- a/UPGRADING.md
+++ b/UPGRADING.md
@@ -152,6 +152,8 @@
- [BC] Atomic::__toString() is now final
- [BC] Atomic::__toString() now returns a more detailed version of the type (it calls getId() under the hood)
- [BC] Atomic::getId() has now a first param $exact. Calling the method with false will return a less detailed version of the type in some cases (similarly to what __toString used to return)
+- [BC] To remove a variable from the context, Context::remove(). Calling
+ `unset($context->vars_in_scope[$var_id])` can cause problems when using references.
## Removed
- [BC] Property `Psalm\Codebase::$php_major_version` was removed, use
@@ -194,3 +196,4 @@
- [BC] Method `Psalm\Issue\CodeIssue::getMessage()` was removed
- [BC] Method `Psalm\DocComment::parse()` was removed
- [BC] Class `Psalm\Type\Atomic\THtmlEscapedString` has been removed
+ - [BC] Property `Psalm\Context::$vars_from_global` has been renamed to `$referenced_globals`
diff --git a/config.xsd b/config.xsd
index 634b3de26a1..e03e0b62369 100644
--- a/config.xsd
+++ b/config.xsd
@@ -413,6 +413,7 @@
+
diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
index a57d76b9e4f..a06d6349e7e 100644
--- a/docs/running_psalm/issues.md
+++ b/docs/running_psalm/issues.md
@@ -216,6 +216,7 @@
- [RedundantIdentityWithTrue](issues/RedundantIdentityWithTrue.md)
- [RedundantPropertyInitializationCheck](issues/RedundantPropertyInitializationCheck.md)
- [ReferenceConstraintViolation](issues/ReferenceConstraintViolation.md)
+ - [ReferenceReusedFromConfusingScope](issues/ReferenceReusedFromConfusingScope.md)
- [ReservedWord](issues/ReservedWord.md)
- [StringIncrement](issues/StringIncrement.md)
- [TaintedCallable](issues/TaintedCallable.md)
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 eba6e9a445c..b1346e55e87 100644
--- a/src/Psalm/Context.php
+++ b/src/Psalm/Context.php
@@ -13,9 +13,12 @@
use Psalm\Type\Atomic\DependentType;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Union;
+use RuntimeException;
use function array_keys;
use function array_search;
+use function array_shift;
+use function assert;
use function count;
use function in_array;
use function is_int;
@@ -40,6 +43,49 @@ 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.
+ *
+ * @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.
*
@@ -362,11 +408,6 @@ class Context
*/
public $has_returned = false;
- /**
- * @var array
- */
- public $vars_from_global = [];
-
/**
* @var array
*/
@@ -449,6 +490,28 @@ public function update(
}
}
+ /**
+ * Updates the list of possible references from a confusing scope,
+ * such as a reference created in an if that might later be reused.
+ */
+ public function updateReferencesPossiblyFromConfusingScope(
+ Context $confusing_scope_context,
+ StatementsAnalyzer $statements_analyzer
+ ): void {
+ $references = $confusing_scope_context->references_in_scope
+ + $confusing_scope_context->references_to_external_scope;
+ foreach ($references 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
*
@@ -496,19 +559,60 @@ 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, or
+ * referenced by another variable. Leaves the variable as possibly-in-scope, unlike remove().
+ */
+ public function removePossibleReference(string $remove_var_id): void
+ {
+ if (isset($this->referenced_counts[$remove_var_id]) && $this->referenced_counts[$remove_var_id] > 0) {
+ // If a referenced variable goes out of scope, we need to update the references.
+ // All of the references to this variable are still references to the same value,
+ // so we pick the first one and make the rest of the references point to it.
+ $references = [];
+ foreach ($this->references_in_scope as $reference => $referenced) {
+ if ($referenced === $remove_var_id) {
+ $references[] = $reference;
+ unset($this->references_in_scope[$reference]);
+ }
+ }
+ assert(!empty($references));
+ $first_reference = array_shift($references);
+ if (!empty($references)) {
+ $this->referenced_counts[$first_reference] = count($references);
+ foreach ($references as $reference) {
+ $this->references_in_scope[$reference] = $first_reference;
+ }
+ }
+ }
+ 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->referenced_counts[$remove_var_id],
+ $this->references_in_scope[$remove_var_id],
+ $this->references_to_external_scope[$remove_var_id],
+ );
}
/**
@@ -645,7 +749,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) {
@@ -676,7 +780,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 aabb0f43f1b..cbed8f768f3 100644
--- a/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
@@ -159,6 +159,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 d329e7f8cc9..8e767dd32b1 100644
--- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -771,7 +771,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);
}
}
@@ -854,10 +854,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;
}
@@ -984,6 +980,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;
@@ -1087,7 +1084,7 @@ private function processParams(
&& !$function_param->type->isBool())
) {
$param_assignment = DataFlowNode::getForAssignment(
- '$' . $function_param->name,
+ $function_param_id,
$function_param->location
);
@@ -1105,16 +1102,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];
}
@@ -1123,18 +1110,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
);
@@ -1170,7 +1158,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
),
@@ -1271,13 +1259,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/DoAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
index aeeb964f90a..2298b6cd41f 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/DoAnalyzer.php
@@ -129,6 +129,7 @@ function (Clause $c) use ($mixed_var_ids): bool {
$negated_while_types,
[],
$inner_loop_context->vars_in_scope,
+ $inner_loop_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
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/IfConditionalAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
index 29a94d5c78e..7c672ca138d 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfConditionalAnalyzer.php
@@ -54,6 +54,7 @@ public static function analyze(
$if_scope->negated_types,
[],
$outer_context->vars_in_scope,
+ $outer_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
index ad7ba5c7dcc..4d878054555 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseAnalyzer.php
@@ -78,6 +78,7 @@ public static function analyze(
$else_types,
[],
$else_context->vars_in_scope,
+ $else_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
@@ -97,7 +98,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);
}
}
}
@@ -243,6 +244,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 0a5ec037aea..9cda6b597b5 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/ElseIfAnalyzer.php
@@ -227,6 +227,7 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl
$reconcilable_elseif_types,
$active_elseif_types,
$elseif_context->vars_in_scope,
+ $elseif_context->references_in_scope,
$newly_reconciled_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
@@ -255,7 +256,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);
}
}
}
@@ -360,6 +361,7 @@ function (Clause $c) use ($assigned_in_conditional_var_ids, $elseif_cond_id): Cl
$negated_elseif_types,
[],
$pre_conditional_context->vars_in_scope,
+ $pre_conditional_context->references_in_scope,
$newly_reconciled_var_ids,
[],
$statements_analyzer,
@@ -434,6 +436,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 51b8e80dd3d..fda3b851617 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElse/IfAnalyzer.php
@@ -254,6 +254,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;
}
@@ -280,6 +286,7 @@ private static function handleMicDrop(
$if_scope->negated_types,
[],
$post_if_context->vars_in_scope,
+ $post_if_context->references_in_scope,
$newly_reconciled_var_ids,
[],
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
index a3bf7c90045..316b91a8961 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php
@@ -267,6 +267,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$reconcilable_if_types,
$active_if_types,
$if_context->vars_in_scope,
+ $if_context->references_in_scope,
$changed_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
@@ -297,7 +298,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);
}
}
}
@@ -328,6 +329,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_object_id): Clause {
$if_scope->negated_types,
[],
$temp_else_context->vars_in_scope,
+ $temp_else_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
index 13683b2ac7a..bc11d8644c4 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
@@ -317,7 +317,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;
@@ -360,7 +360,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);
}
}
}
@@ -453,7 +453,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;
}
@@ -503,6 +503,7 @@ public static function analyze(
$negated_pre_condition_types,
[],
$inner_context->vars_in_scope,
+ $inner_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
@@ -555,6 +556,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;
}
@@ -630,7 +637,7 @@ private static function applyPreConditionToLoopContext(
if (ExpressionAnalyzer::analyze($statements_analyzer, $pre_condition, $loop_context) === false) {
$loop_context->inside_conditional = $was_inside_conditional;
-
+
return [];
}
@@ -660,6 +667,7 @@ private static function applyPreConditionToLoopContext(
$reconcilable_while_types,
$active_while_types,
$loop_context->vars_in_scope,
+ $loop_context->references_in_scope,
$changed_var_ids,
$new_referenced_var_ids,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php
index 570881470bb..6d286879b7c 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchAnalyzer.php
@@ -159,6 +159,7 @@ public static function analyze(
$reconcilable_if_types,
[],
$original_context->vars_in_scope,
+ $original_context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php
index 078fc3cd75a..49177b5b414 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Block/SwitchCaseAnalyzer.php
@@ -405,6 +405,7 @@ public static function analyze(
$reconcilable_if_types,
[],
$case_context->vars_in_scope,
+ $case_context->references_in_scope,
$changed_var_ids,
$case->cond && $switch_var_id ? [$switch_var_id => true] : [],
$statements_analyzer,
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 976fafa45ce..797de49243d 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssignmentAnalyzer.php
@@ -3,6 +3,9 @@
namespace Psalm\Internal\Analyzer\Statements\Expression;
use PhpParser;
+use PhpParser\Node\Expr\ArrayDimFetch;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
use Psalm\CodeLocation;
use Psalm\CodeLocation\DocblockTypeLocation;
use Psalm\Codebase;
@@ -49,6 +52,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;
@@ -78,6 +82,7 @@
use Psalm\Type\Atomic\TNonEmptyList;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Union;
+use RuntimeException;
use UnexpectedValueException;
use function array_filter;
@@ -852,20 +857,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, null, true);
$lhs_var_id = ExpressionIdentifier::getArrayVarId(
$stmt->var,
@@ -879,48 +871,114 @@ 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 (!empty($var_comments)) {
+ IssueBuffer::maybeAdd(
+ new InvalidDocblock(
+ "Docblock type cannot be used for reference assignment",
+ 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 ($lhs_var_id === null || $rhs_var_id === null) {
+ return false;
+ }
+
+ 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;
- $context->byref_constraints[$rhs_var_id] = new ReferenceConstraint();
+ // 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;
}
- 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);
+ $statements_analyzer->registerVariableAssignment(
+ $lhs_var_id,
+ $lhs_location
+ );
- $lhs_location = new CodeLocation($statements_analyzer->getSource(), $stmt->var);
+ $lhs_node = DataFlowNode::getForAssignment($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;
- foreach ($rhs_type->parent_nodes as $byref_destination_node) {
- $data_flow_graph->addPath($lhs_node, $byref_destination_node, '=');
+ if (($stmt->var instanceof ArrayDimFetch || $stmt->var instanceof PropertyFetch)
+ && $stmt->var->var instanceof Variable
+ && is_string($stmt->var->var->name)
+ ) {
+ // If left-hand-side is an array offset or object property, add an edge to the root
+ // variable, so if the root variable is used, the offset/property is also marked as used.
+ $root_var_id = "$" . $stmt->var->var->name;
+ foreach ($context->vars_in_scope[$root_var_id]->parent_nodes as $root_parent) {
+ $statements_analyzer->data_flow_graph->addPath(
+ $lhs_node,
+ $root_parent,
+ $stmt->var instanceof ArrayDimFetch
+ ? "offset-assignment-as-reference"
+ : "property-assignment-as-reference"
+ );
}
}
+ if ($stmt->var instanceof ArrayDimFetch) {
+ // Analyze offset so that variables in the offset get marked as used
+ $was_inside_general_use = $context->inside_general_use;
+ $context->inside_general_use = true;
+ ExpressionAnalyzer::analyze($statements_analyzer, $stmt->var->dim, $context);
+ $context->inside_general_use = $was_inside_general_use;
+ }
+
return true;
}
@@ -1600,6 +1658,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;
@@ -1636,30 +1695,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/BinaryOp/AndAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php
index 98ca259fb2b..7e3937c38ac 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/AndAnalyzer.php
@@ -138,6 +138,7 @@ public static function analyze(
$left_type_assertions,
$active_left_assertions,
$context->vars_in_scope,
+ $context->references_in_scope,
$changed_var_ids,
$left_referenced_var_ids,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php
index cf71761e339..f5ef20d7a8f 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/OrAnalyzer.php
@@ -223,6 +223,7 @@ public static function analyze(
$negated_type_assertions,
$active_negated_type_assertions,
$right_context->vars_in_scope,
+ $right_context->references_in_scope,
$changed_var_ids,
$left_referenced_var_ids,
$statements_analyzer,
@@ -314,6 +315,7 @@ public static function analyze(
$right_type_assertions,
$active_right_type_assertions,
$right_context->vars_in_scope,
+ $right_context->references_in_scope,
$right_changed_var_ids,
$right_referenced_var_ids,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
index bb07f369b3d..ce2198ac666 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
@@ -945,6 +945,7 @@ private static function processAssertFunctionEffects(
$assert_type_assertions,
$assert_type_assertions,
$context->vars_in_scope,
+ $context->references_in_scope,
$changed_var_ids,
array_map(
fn($_): bool => true,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
index 06d6f10c17b..4358c47f17c 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NamedFunctionCallHandler.php
@@ -399,6 +399,7 @@ public static function handle(
$assertions,
$assertions,
$context->vars_in_scope,
+ $context->references_in_scope,
$changed_vars,
$referenced_var_ids,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
index df54b95d64f..c90c44fe83e 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/CallAnalyzer.php
@@ -227,8 +227,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];
@@ -886,6 +885,7 @@ public static function applyAssertionsToContext(
$type_assertions,
$type_assertions,
$context->vars_in_scope,
+ $context->references_in_scope,
$changed_var_ids,
$asserted_keys,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
index b0ffb87d273..39bfe8d2f8b 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/Expression/MatchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php
index 2e2ee100d94..d131d7b2737 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/MatchAnalyzer.php
@@ -253,6 +253,7 @@ public static function analyze(
$reconcilable_types,
[],
$context->vars_in_scope,
+ $context->references_in_scope,
$changed_var_ids,
[],
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
index 39905239d37..2bd6307fda8 100644
--- a/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/Expression/TernaryAnalyzer.php
@@ -161,6 +161,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause {
$reconcilable_if_types,
$active_if_types,
$if_context->vars_in_scope,
+ $if_context->references_in_scope,
$changed_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
@@ -199,6 +200,7 @@ function (Clause $c) use ($mixed_var_ids, $cond_id): Clause {
$negated_if_types,
$negated_if_types,
$t_else_context->vars_in_scope,
+ $t_else_context->references_in_scope,
$changed_var_ids,
$cond_referenced_var_ids,
$statements_analyzer,
diff --git a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
index a1a4483b717..a33d2e3c973 100644
--- a/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
@@ -65,6 +65,10 @@
*/
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,
@@ -72,7 +76,8 @@ public static function analyze(
bool $array_assignment = false,
?Context $global_context = null,
bool $from_stmt = false,
- ?TemplateResult $template_result = null
+ ?TemplateResult $template_result = null,
+ bool $assigned_to_reference = false
): bool {
$codebase = $statements_analyzer->getCodebase();
@@ -83,7 +88,8 @@ public static function analyze(
$array_assignment,
$global_context,
$from_stmt,
- $template_result
+ $template_result,
+ $assigned_to_reference,
) === false) {
return false;
}
@@ -140,6 +146,10 @@ 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,
@@ -147,7 +157,8 @@ private static function handleExpression(
bool $array_assignment,
?Context $global_context,
bool $from_stmt,
- ?TemplateResult $template_result = null
+ ?TemplateResult $template_result = null,
+ bool $assigned_to_reference = false
): bool {
if ($stmt instanceof PhpParser\Node\Expr\Variable) {
return VariableFetchAnalyzer::analyze(
@@ -156,7 +167,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..a04e72d9740 100644
--- a/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/Statements/GlobalAnalyzer.php
@@ -12,6 +12,7 @@
use Psalm\Internal\ReferenceConstraint;
use Psalm\Issue\InvalidGlobal;
use Psalm\IssueBuffer;
+use RuntimeException;
use function is_string;
@@ -61,6 +62,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 +70,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 +91,10 @@ public static function analyze(
$var,
$var_id
);
+
+ if ($global_context !== null && $global_context->hasVariable($var_id)) {
+ $global_context->referenced_globals[$var_id] = true;
+ }
}
}
}
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 952d8c5ac3f..2881e5273bf 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/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
index f26ef15cbf1..fd07c16459c 100644
--- a/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
+++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/ArrayFilterReturnTypeProvider.php
@@ -217,6 +217,7 @@ static function ($keyed_type) use ($statements_source, $context) {
$assertions,
$assertions,
['$inner_type' => $inner_type],
+ [],
$changed_var_ids,
['$inner_type' => true],
$statements_source,
@@ -296,6 +297,7 @@ static function ($keyed_type) use ($statements_source, $context) {
$assertions,
$assertions,
['$inner_type' => $inner_type],
+ [],
$changed_var_ids,
['$inner_type' => true],
$statements_source,
diff --git a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
index b2e1753500e..7ce33e72eaf 100644
--- a/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
+++ b/src/Psalm/Internal/Type/SimpleAssertionReconciler.php
@@ -241,8 +241,7 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
- $is_equality
+ $is_equality,
);
}
@@ -254,8 +253,7 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
- $is_equality
+ $is_equality,
);
}
@@ -434,7 +432,7 @@ public static function reconcile(
$code_location,
$suppressed_issues,
$failed_reconciliation,
- $is_equality
+ $is_equality,
);
}
@@ -560,7 +558,6 @@ private static function reconcileIsset(
/**
* @param string[] $suppressed_issues
- * @param Reconciler::RECONCILIATION_* $failed_reconciliation
*/
private static function reconcileNonEmptyCountable(
Assertion $assertion,
@@ -569,7 +566,6 @@ private static function reconcileNonEmptyCountable(
bool $negated,
?CodeLocation $code_location,
array $suppressed_issues,
- int &$failed_reconciliation,
bool $is_equality
): Union {
$old_var_type_string = $existing_var_type->getId();
diff --git a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
index 548ec187b79..fc8d6b62c98 100644
--- a/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
+++ b/src/Psalm/Internal/Type/SimpleNegatedAssertionReconciler.php
@@ -204,7 +204,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
null
);
@@ -218,7 +217,6 @@ public static function reconcile(
$negated,
$code_location,
$suppressed_issues,
- $failed_reconciliation,
$is_equality,
$assertion->count
);
@@ -510,7 +508,6 @@ private static function reconcileNotNonEmptyCountable(
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 c3e336edc4e..9d86881d75f 100644
--- a/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php
+++ b/src/Psalm/Internal/Type/TemplateStandinTypeReplacer.php
@@ -214,7 +214,6 @@ private static function handleAtomicStandin(
$bound_equality_classlike,
$depth,
$was_single,
- $had_template
);
}
}
@@ -895,8 +894,7 @@ public static function handleTemplateParamClassStandin(
bool $add_lower_bound,
?string $bound_equality_classlike,
int $depth,
- bool $was_single,
- bool &$had_template
+ bool $was_single
): array {
if ($atomic_type->defining_class === $calling_class) {
return [$atomic_type];
diff --git a/src/Psalm/Internal/Type/TypeExpander.php b/src/Psalm/Internal/Type/TypeExpander.php
index 5901ced6e19..97c1a8854a8 100644
--- a/src/Psalm/Internal/Type/TypeExpander.php
+++ b/src/Psalm/Internal/Type/TypeExpander.php
@@ -125,6 +125,8 @@ public static function expandUnion(
* @param string|TNamedObject|TTemplateParam|null $static_class_type
*
* @return non-empty-list
+ *
+ * @psalm-suppress ComplexMethod
*/
public static function expandAtomic(
Codebase $codebase,
diff --git a/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php b/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php
new file mode 100644
index 00000000000..6c88cf67b26
--- /dev/null
+++ b/src/Psalm/Issue/ReferenceReusedFromConfusingScope.php
@@ -0,0 +1,9 @@
+>> $new_types
* @param array>> $active_new_types - types we can complain about
* @param array $existing_types
+ * @param array $existing_references Maps keys of $existing_types that are references to other
+ * keys of $existing_types that they are references to.
* @param array $changed_var_ids
* @param array $referenced_var_ids
* @param array> $template_type_map
@@ -98,6 +100,7 @@ public static function reconcileKeyedTypes(
array $new_types,
array $active_new_types,
array $existing_types,
+ array $existing_references,
array &$changed_var_ids,
array $referenced_var_ids,
StatementsAnalyzer $statements_analyzer,
@@ -110,6 +113,32 @@ public static function reconcileKeyedTypes(
return $existing_types;
}
+ $reference_map = [];
+ if (!empty($existing_references)) {
+ // PHP behaves oddly when passing an array containing references: https://bugs.php.net/bug.php?id=20993
+ // To work around the issue, if there are any references, we have to recreate the array and fix the
+ // references so they're properly scoped and won't affect the caller. Starting with a new array is
+ // required for some unclear reason, just cloning elements of the existing array doesn't work properly.
+ $old_existing_types = $existing_types;
+ $existing_types = [];
+
+ $cloned_referenceds = [];
+ foreach ($existing_references as $reference => $referenced) {
+ if (!isset($cloned_referenceds[$referenced])) {
+ $existing_types[$referenced] = clone $old_existing_types[$referenced];
+ $cloned_referenceds[$referenced] = true;
+ }
+ $existing_types[$reference] = &$existing_types[$referenced];
+ }
+ $existing_types += $old_existing_types;
+
+ // Build a map from reference/referenced variables to other variables with the same reference
+ foreach ($existing_references as $reference => $referenced) {
+ $reference_map[$reference][] = $referenced;
+ $reference_map[$referenced][] = $reference;
+ }
+ }
+
$suppressed_issues = $statements_analyzer->getSuppressedIssues();
$old_new_types = $new_types;
@@ -214,6 +243,7 @@ public static function reconcileKeyedTypes(
$data,
$data,
$existing_types,
+ $existing_references,
$changed_var_ids,
$referenced_var_ids,
$statements_analyzer,
@@ -285,11 +315,11 @@ public static function reconcileKeyedTypes(
$type_changed = !$before_adjustment || !$result_type->equals($before_adjustment);
+ $key_parts = self::breakUpPathIntoParts($key);
if ($type_changed || $failed_reconciliation) {
$changed_var_ids[$key] = true;
if (substr($key, -1) === ']' && !$has_inverted_isset && !$has_empty && !$is_equality) {
- $key_parts = self::breakUpPathIntoParts($key);
self::adjustTKeyedArrayType(
$key_parts,
$existing_types,
@@ -321,6 +351,16 @@ public static function reconcileKeyedTypes(
if (!$has_object_array_access) {
$existing_types[$key] = $result_type;
}
+
+ if (!$did_type_exist && isset($existing_types[$key]) && isset($reference_map[$key_parts[0]])) {
+ // If key is new, create references for other variables that reference the root variable.
+ $reference_key_parts = $key_parts;
+ foreach ($reference_map[$key_parts[0]] as $reference) {
+ $reference_key_parts[0] = $reference;
+ $reference_key = implode("", $reference_key_parts);
+ $existing_types[$reference_key] = &$existing_types[$key];
+ }
+ }
}
return $existing_types;
diff --git a/src/Psalm/Type/Union.php b/src/Psalm/Type/Union.php
index dd949d61f8c..a5c550bbf15 100644
--- a/src/Psalm/Type/Union.php
+++ b/src/Psalm/Type/Union.php
@@ -190,7 +190,9 @@ class Union implements TypeNode
private $literal_float_types = [];
/**
- * Whether or not the type was passed by reference
+ * True if the type was passed or returned by reference, or if the type refers to an object's
+ * property or an item in an array. Note that this is not true for locally created references
+ * that don't refer to properties or array items (see Context::$references_in_scope).
*
* @var bool
*/
diff --git a/tests/AnnotationTest.php b/tests/AnnotationTest.php
index 8d287fd7724..c485f07b43a 100644
--- a/tests/AnnotationTest.php
+++ b/tests/AnnotationTest.php
@@ -537,11 +537,20 @@ function bar(string ...$_s) : void {}
],
'spreadOperatorByRefAnnotation' => [
'code' => ' [
'code' => ' [
'code' => ' [
'code' => ' [ // See #7254
+ 'referencesDontBreakAssertions' => [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' [
'code' => ' [ // See #7254
+ 'applyAssertionsOnPropertiesFromReferences' => [
'code' => ' [ // See #7254
+ 'applyAssertionsOnPropertiesToReferencesWithConditionalOperator' => [
'code' => ' 'PossiblyNullArgument',
],
- 'SKIPPED-forgetAssertionAfterRelevantNonMutationFreeCallOnReference' => [ // See #7254
+ 'forgetAssertionAfterRelevantNonMutationFreeCallOnReference' => [
'code' => ' 'PossiblyNullArgument',
],
- 'SKIPPED-forgetAssertionAfterReferenceModification' => [ // See #7254
+ 'forgetAssertionAfterReferenceModification' => [
'code' => ' 'PossiblyNullArgument',
+ 'error_message' => 'NullArgument',
],
'assertionOnMagicPropertyWithoutMutationFreeGet' => [
'code' => ' true,
],
- 'removeUnusedVarAssignByRef' => [
+ 'SKIPPED-removeUnusedVarAssignByRefToSubsequentlyUsedVariable' => [
'input' => ' ' '7.1',
@@ -356,6 +355,20 @@ function foo() : void {
'safe_types' => true,
],
+ 'removeUnusedVarAssignByRef' => [
+ 'input' => ' ' '7.1',
+ 'issues_to_fix' => ['UnusedVariable'],
+ 'safe_types' => true,
+ ],
+
'removeUnusedVarAssignByRefPartial' => [
'input' => ' ' '7.1',
@@ -384,7 +397,7 @@ function foo() : void {
'output' => ' '7.1',
diff --git a/tests/ReferenceConstraintTest.php b/tests/ReferenceConstraintTest.php
index 892ec3997db..2cdec8413a4 100644
--- a/tests/ReferenceConstraintTest.php
+++ b/tests/ReferenceConstraintTest.php
@@ -299,6 +299,38 @@ function main(bool $a, string $b, string $c): void {
}',
'error_message' => 'InvalidPassByReference',
],
+ 'SKIPPED-referenceToTypedArrayConstrainsAssignment' => [
+ 'code' => ' */
+ public array $arr = [];
+
+ public function __construct()
+ {
+ assert(isset($this->arr[0]));
+ $int = &$this->arr[0];
+ $int = (string) $int;
+ }
+ }
+ ',
+ 'error_message' => 'ReferenceConstraintViolation',
+ ],
+ 'SKIPPED-referenceToTypedArrayConstrainsAssignmentWithNullReferenceInitialization' => [
+ 'code' => ' */
+ 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',
+ ],
];
}
}
diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php
new file mode 100644
index 00000000000..13ae8ec17f3
--- /dev/null
+++ b/tests/ReferenceTest.php
@@ -0,0 +1,373 @@
+,ignored_issues?:list,php_version?:string}>
+ */
+ public function providerValidCodeParse(): iterable
+ {
+ return [
+ 'updateReferencedTypes' => [
+ 'code' => ' [
+ '$a===' => '2',
+ '$b===' => '4',
+ '$c===' => '4',
+ ],
+ ],
+ 'changingReferencedVariableChangesReference' => [
+ 'code' => ' [
+ '$a===' => '2',
+ '$b===' => '2',
+ ],
+ ],
+ 'unsetReference' => [
+ 'code' => ' [
+ '$a===' => '2',
+ '$b===' => '3',
+ ],
+ ],
+ 'recursiveReference' => [
+ 'code' => ' [
+ '$a===' => '2',
+ '$b===' => '2',
+ ],
+ ],
+ 'SKIPPED-referenceToArrayItemChangesArrayType' => [
+ 'code' => ' */
+ $arr = [];
+
+ assert(isset($arr[0]));
+ $int = &$arr[0];
+ $int = (string) $int;
+ ',
+ 'assertions' => [
+ '$arr' => 'list',
+ ],
+ ],
+ 'referenceToReference' => [
+ 'code' => ' [
+ '$a===' => '2',
+ '$b===' => '3',
+ '$c===' => '2',
+ '$d===' => '3',
+ ],
+ ],
+ 'referenceToSubsequentCatch' => [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' [
+ '$a===' => 'null',
+ '$b===' => 'null',
+ ],
+ ],
+ 'referenceShadowedByGlobal' => [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => 'bar;
+
+ $foo->bar = "bar";
+ ',
+ 'assertions' => [
+ '$bar===' => "'bar'",
+ ],
+ ],
+ 'referenceReassignedInLoop' => [
+ 'code' => ' $keys */
+ function &ensure_array(array &$what, array $keys): array
+ {
+ $arr = & $what;
+ while ($key = array_shift($keys)) {
+ if (!isset($arr[$key]) || !is_array($arr[$key])) {
+ $arr[$key] = array();
+ }
+ $arr = & $arr[$key];
+ }
+ return $arr;
+ }
+ ',
+ ],
+ ];
+ }
+
+ /**
+ * @return iterable,php_version?:string}>
+ */
+ public function providerInvalidCodeParse(): iterable
+ {
+ return [
+ 'referenceReuseForeachValue' => [
+ 'code' => ' */
+ $arr = [];
+
+ foreach ($arr as &$var) {
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInForeach' => [
+ 'code' => ' */
+ $arr = [];
+
+ foreach ($arr as $val) {
+ $var = &$val;
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInFor' => [
+ 'code' => ' */
+ $arr = [];
+
+ for ($i = 0; $i < count($arr); ++$i) {
+ $var = &$arr[$i];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInIf' => [
+ 'code' => ' */
+ $arr = [];
+
+ if (isset($arr[0])) {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInElseif' => [
+ 'code' => ' */
+ $arr = [];
+
+ if (random_int(0, 1)) {
+ } elseif (isset($arr[0])) {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeclaredInElse' => [
+ 'code' => ' */
+ $arr = [];
+
+ if (!isset($arr[0])) {
+ } else {
+ $var = &$arr[0];
+ $var += 1;
+ }
+
+ $var = "foo";
+ ',
+ 'error_message' => 'ReferenceReusedFromConfusingScope',
+ ],
+ 'referenceReuseDeeplyNested' => [
+ 'code' => '>> */
+ $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' => [
+ 'code' => ' 'InvalidDocblock - src' . DIRECTORY_SEPARATOR . 'somefile.php:4:21 - Docblock type cannot be used for reference assignment',
+ ],
+ 'unsetOnlyPreventsReferenceConfusionAfterCall' => [
+ 'code' => ' 'ReferenceReusedFromConfusingScope',
+ ],
+ 'assignmentAsReferenceOnlyPreventsReferenceConfusionAfterAssignment' => [
+ 'code' => ' 'ReferenceReusedFromConfusingScope',
+ ],
+ ];
+ }
+}
diff --git a/tests/TypeReconciliation/ConditionalTest.php b/tests/TypeReconciliation/ConditionalTest.php
index 2ab475b5fd5..733743da3e7 100644
--- a/tests/TypeReconciliation/ConditionalTest.php
+++ b/tests/TypeReconciliation/ConditionalTest.php
@@ -3241,6 +3241,27 @@ function c(string $c): void {
',
'error_message' => 'RedundantCondition',
],
+ 'impossibleConditionWithReference' => [
+ 'code' => ' 'TypeDoesNotContainType',
+ ],
+ 'redundantConditionWithReference' => [
+ 'code' => ' 'RedundantCondition',
+ ],
];
}
}
diff --git a/tests/UnusedVariableTest.php b/tests/UnusedVariableTest.php
index dc7fb38cac7..2e5ce623a71 100644
--- a/tests/UnusedVariableTest.php
+++ b/tests/UnusedVariableTest.php
@@ -1525,7 +1525,10 @@ function foo(array $arr) : void {
}
}
- /** @param mixed $p */
+ /**
+ * @param mixed $p
+ * @psalm-suppress UnusedParam
+ */
function takes_ref(&$p): void {}'
],
'passedByRefArrayOffset' => [
@@ -1955,6 +1958,23 @@ function foo(array &$arr): void {
$arr[0] = $b;
}'
],
+ 'byRefDeeplyNestedArrayParam' => [
+ 'code' => '> $arr */
+ function foo(array &$arr): void {
+ $b = 5;
+ $arr[0][0] = $b;
+ }'
+ ],
+ 'nestedReferencesToByRefParam' => [
+ 'code' => '> $arr */
+ function foo(array &$arr): void {
+ $a = &$arr[0];
+ $b = &$a[0];
+ $b = 5;
+ }'
+ ],
'byRefNestedArrayInForeach' => [
'code' => ' 10) {
break;
} else {}
- }
+ }
}',
],
+ 'referenceUseUsesReferencedVariable' => [
+ 'code' => ' [
+ 'code' => ' [
+ '$b===' => '2',
+ '$a===' => '2',
+ ],
+ ],
+ 'referenceUsedAfterVariableReassignment' => [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' [
+ 'code' => ' */
+ $arr = [1];
+ $arr[1] = &$arr[0];
+
+ takesArray($arr);
+
+ function takesArray(array $_arr): void {}
+ ',
+ ],
+ 'arrayWithVariableOffsetAssignedToReferenceUsesVariableOffset' => [
+ 'code' => ' */
+ $arr = [1];
+ $int = 1;
+ $arr[$int] = &$arr[0];
+
+ takesArray($arr);
+
+ function takesArray(array $_arr): void {}
+ ',
+ ],
];
}
@@ -3433,6 +3517,98 @@ function takesArray($a) : void {
}',
'error_message' => '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' => [
+ 'code' => ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3:21 - $c',
+ ],
+ 'referenceAssignmentIsNotUsed' => [
+ 'code' => ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - $a',
+ ],
+ 'unusedReferenceToPreviouslyUsedVariable' => [
+ 'code' => ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:4:21 - $b',
+ ],
+ 'SKIPPED-unusedReferenceToSubsequentlyUsedVariable' => [ // Not easy to do the way it's currently set up
+ 'code' => ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:3:21 - $b',
+ ],
+ 'unusedReferenceInForeach' => [
+ 'code' => ' 'UnusedForeachValue',
+ ],
+ 'SKIPPED-unusedReferenceInDestructuredForeach' => [
+ 'code' => ' 'UnusedForeachValue',
+ ],
+ 'unusedReturnByReference' => [
+ 'code' => ' 'UnusedVariable',
+ ],
+ 'unusedPassByReference' => [
+ 'code' => ' 'UnusedParam',
+ ],
+ 'SKIPPED-unusedGlobalVariable' => [
+ 'code' => ' 'UnusedVariable - src' . DIRECTORY_SEPARATOR . 'somefile.php:2:21 - $a',
+ ],
+ 'unusedUndeclaredGlobalVariable' => [
+ 'code' => ' 'UnusedVariable',
+ ],
];
}
}