Skip to content

Commit 3c1d809

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Report constant-offset assignments that are redundant
`$a = [1, 2]; $a[0] = 1;` assigns a value the offset already holds, the same finding the rule already reports for whole variables. The check walks the constant dimensions of the target down the array type in both type flavours; append, dynamic keys, by-reference items and PhpDoc-only certainty are left alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018CZ79ZBiMt4KWgU3uy4Y4j
1 parent d18e287 commit 3c1d809

4 files changed

Lines changed: 147 additions & 14 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
259259
}
260260
}
261261

262-
$redundantType = $expr instanceof Assign && $expr->var instanceof Variable && is_string($expr->var->name)
263-
? self::redundant($assignedExprResult, $expr->var->name)
262+
$redundantType = $expr instanceof Assign
263+
? self::redundant($assignedExprResult, $expr->var, $storage)
264264
: null;
265265
$variableFlow = VariableFlow::sequence(
266266
VariableFlowBuilder::targetRead($expr->var, $storage, false),
@@ -2500,21 +2500,45 @@ private static function hasArrayReference(Expr\Array_ $array): bool
25002500
return false;
25012501
}
25022502

2503-
private static function redundant(ExpressionResult $rhs, string $name): ?Type
2503+
private static function redundant(ExpressionResult $rhs, Expr $target, ExpressionResultStorage $storage): ?Type
25042504
{
2505-
$scope = $rhs->getScope();
2506-
if (!$scope->hasVariableType($name)->yes()) {
2507-
return null;
2505+
$dimensions = [];
2506+
while ($target instanceof ArrayDimFetch) {
2507+
if ($target->dim === null) {
2508+
return null;
2509+
}
2510+
$dimResult = $storage->findExpressionResult($target->dim);
2511+
if ($dimResult === null || count($dimResult->getType()->toArrayKey()->getConstantScalarValues()) !== 1) {
2512+
return null;
2513+
}
2514+
$dimensions[] = $dimResult;
2515+
$target = $target->var;
25082516
}
2509-
$values = $scope->getVariableType($name)->getFiniteTypes();
2510-
if (count($values) !== 1 || !$values[0]->equals($rhs->getType())) {
2517+
if (!$target instanceof Variable || !is_string($target->name)) {
25112518
return null;
25122519
}
2520+
$name = $target->name;
2521+
$scope = $rhs->getScope();
25132522
$nativeScope = $scope->doNotTreatPhpDocTypesAsCertain();
2514-
if (!$nativeScope->hasVariableType($name)->yes()) {
2523+
if (!$scope->hasVariableType($name)->yes() || !$nativeScope->hasVariableType($name)->yes()) {
2524+
return null;
2525+
}
2526+
$type = $scope->getVariableType($name);
2527+
$nativeType = $nativeScope->getVariableType($name);
2528+
foreach (array_reverse($dimensions) as $dimension) {
2529+
$offset = $dimension->getType()->toArrayKey();
2530+
$nativeOffset = $dimension->getNativeType()->toArrayKey();
2531+
if (!$type->isArray()->yes() || !$nativeType->isArray()->yes() || !$type->hasOffsetValueType($offset)->yes() || !$nativeType->hasOffsetValueType($nativeOffset)->yes()) {
2532+
return null;
2533+
}
2534+
$type = $type->getOffsetValueType($offset);
2535+
$nativeType = $nativeType->getOffsetValueType($nativeOffset);
2536+
}
2537+
$values = $type->getFiniteTypes();
2538+
if (count($values) !== 1 || !$values[0]->equals($rhs->getType())) {
25152539
return null;
25162540
}
2517-
$nativeValues = $nativeScope->getVariableType($name)->getFiniteTypes();
2541+
$nativeValues = $nativeType->getFiniteTypes();
25182542
return count($nativeValues) === 1 && $nativeValues[0]->equals($rhs->getNativeType()) ? $rhs->getType() : null;
25192543
}
25202544

‎src/Rules/DeadCode/UnusedVariableRule.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,19 @@ public function processNode(Node $node, Scope $scope): array
7171
}
7272

7373
$redundantType = $node->getRedundantType($write);
74-
if ($redundantType !== null && $node->isUsed($write)) {
74+
if ($redundantType !== null && ($write->isOffsetWrite() || $node->isUsed($write))) {
75+
$target = $write->getNode();
76+
if (!$target instanceof Node\Expr) {
77+
throw new ShouldNotHappenException();
78+
}
79+
$description = $write->isOffsetWrite() ? 'Offset ' . $this->exprPrinter->printExpr($target) : 'Variable $' . $name;
7580
$errors[] = RuleErrorBuilder::message(sprintf(
76-
'Variable $%s is assigned value %s but it already has that value.',
77-
$name,
81+
'%s is assigned value %s but it already has that value.',
82+
$description,
7883
$redundantType->describe(VerbosityLevel::value()),
7984
))
8085
->identifier('assign.redundant')
81-
->line($write->getNode()->getStartLine())
86+
->line($target->getStartLine())
8287
->build();
8388
continue;
8489
}

‎tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,23 @@ public function testOffsets(): void
441441
]);
442442
}
443443

444+
public function testRedundantOffsets(): void
445+
{
446+
$redundant = [];
447+
foreach ($this->gatherAnalyserErrors([__DIR__ . '/data/unused-variable-redundant-offsets.php']) as $error) {
448+
if ($error->getIdentifier() !== 'assign.redundant') {
449+
continue;
450+
}
451+
$redundant[] = [$error->getMessage(), $error->getLine()];
452+
}
453+
$this->assertSame([
454+
['Offset $a[0] is assigned value 1 but it already has that value.', 8],
455+
['Offset $a[0] is assigned value 1 but it already has that value.', 14],
456+
['Offset $a[\'x\'][\'y\'] is assigned value 1 but it already has that value.', 21],
457+
['Offset $a[\'0\'] is assigned value 1 but it already has that value.', 28],
458+
], $redundant);
459+
}
460+
444461
public function testDynamicOffsetOverwritten(): void
445462
{
446463
$errors = $this->gatherAnalyserErrors([__DIR__ . '/data/unused-variable-offset-overwrite.php']);

‎tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace UnusedVariableRedundantOffsets;
4+
5+
function unused(): void
6+
{
7+
$a = [1, 2];
8+
$a[0] = 1;
9+
}
10+
11+
function used(): array
12+
{
13+
$a = [1, 2];
14+
$a[0] = 1;
15+
return $a;
16+
}
17+
18+
function nested(): array
19+
{
20+
$a = ['x' => ['y' => 1]];
21+
$a['x']['y'] = 1;
22+
return $a;
23+
}
24+
25+
function coercedKey(): array
26+
{
27+
$a = [1];
28+
$a['0'] = 1;
29+
return $a;
30+
}
31+
32+
function differentValue(): array
33+
{
34+
$a = [1, 2];
35+
$a[0] = 3;
36+
return $a;
37+
}
38+
39+
function newOffset(): array
40+
{
41+
$a = [1];
42+
$a[1] = 1;
43+
return $a;
44+
}
45+
46+
/** @param array{0?: 1} $a */
47+
function optionalOffset(array $a): array
48+
{
49+
$a[0] = 1;
50+
return $a;
51+
}
52+
53+
/** @param array{0: 1} $a */
54+
function phpDocOnly(array $a): array
55+
{
56+
$a[0] = 1;
57+
return $a;
58+
}
59+
60+
function append(): array
61+
{
62+
$a = [1];
63+
$a[] = 1;
64+
return $a;
65+
}
66+
67+
function dynamicKey(int $i): array
68+
{
69+
$a = [1, 2];
70+
$a[$i] = 1;
71+
return $a;
72+
}
73+
74+
function reference(): array
75+
{
76+
$x = 1;
77+
$a = [&$x];
78+
$a[0] = 1;
79+
return $a;
80+
}
81+
82+
function stringOffset(): string
83+
{
84+
$a = '12';
85+
$a[0] = '1';
86+
return $a;
87+
}

0 commit comments

Comments
 (0)