Skip to content

Commit 5ad702b

Browse files
janedbaljanedbal
authored
ForbidUnusedClosureParametersRule (#313)
1 parent fb2cfff commit 5ad702b

File tree

5 files changed

+265
-0
lines changed

5 files changed

+265
-0
lines changed

‎README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,14 @@ match ($foo) { // unused match result
621621
```
622622

623623

624+
### forbidUnusedClosureParameters
625+
- Reports unused parameters in closures and arrow functions
626+
- Only reports trailing unused parameters (parameters that are unused and all parameters after them are also unused)
627+
```php
628+
fn (int $key, Item $item, string $unused) => $item->ok(); // unused parameter $unused is reported, $key not
629+
```
630+
631+
624632
### requirePreviousExceptionPass
625633
- Detects forgotten exception pass-as-previous when re-throwing
626634
- Checks if caught exception can be passed as argument to the call (including constructor call) in `throw` node inside the catch block

‎rules.neon

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ parameters:
8181
enabled: %shipmonkRules.enableAllRules%
8282
forbidUnusedMatchResult:
8383
enabled: %shipmonkRules.enableAllRules%
84+
forbidUnusedClosureParameters:
85+
enabled: %shipmonkRules.enableAllRules%
8486
requirePreviousExceptionPass:
8587
enabled: %shipmonkRules.enableAllRules%
8688
reportEvenIfExceptionIsNotAcceptableByRethrownOne: true
@@ -203,6 +205,9 @@ parametersSchema:
203205
forbidUnusedMatchResult: structure([
204206
enabled: bool()
205207
])
208+
forbidUnusedClosureParameters: structure([
209+
enabled: bool()
210+
])
206211
requirePreviousExceptionPass: structure([
207212
enabled: bool()
208213
reportEvenIfExceptionIsNotAcceptableByRethrownOne: bool()
@@ -279,6 +284,8 @@ conditionalTags:
279284
phpstan.rules.rule: %shipmonkRules.forbidUnusedException.enabled%
280285
ShipMonk\PHPStan\Rule\ForbidUnusedMatchResultRule:
281286
phpstan.rules.rule: %shipmonkRules.forbidUnusedMatchResult.enabled%
287+
ShipMonk\PHPStan\Rule\ForbidUnusedClosureParametersRule:
288+
phpstan.rules.rule: %shipmonkRules.forbidUnusedClosureParameters.enabled%
282289
ShipMonk\PHPStan\Rule\ForbidReturnInConstructorRule:
283290
phpstan.rules.rule: %shipmonkRules.uselessPrivatePropertyDefaultValue.enabled%
284291
ShipMonk\PHPStan\Rule\RequirePreviousExceptionPassRule:
@@ -394,6 +401,8 @@ services:
394401
class: ShipMonk\PHPStan\Rule\ForbidUnusedExceptionRule
395402
-
396403
class: ShipMonk\PHPStan\Rule\ForbidUnusedMatchResultRule
404+
-
405+
class: ShipMonk\PHPStan\Rule\ForbidUnusedClosureParametersRule
397406
-
398407
class: ShipMonk\PHPStan\Rule\ForbidReturnInConstructorRule
399408
-

‎src/Rule/ForbidUnusedClosureParametersRule.php

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule;
4+
5+
use LogicException;
6+
use PhpParser\Node;
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\ArrowFunction;
9+
use PhpParser\Node\Expr\Closure;
10+
use PhpParser\Node\Expr\Variable;
11+
use PhpParser\Node\Param;
12+
use PhpParser\NodeTraverser;
13+
use PhpParser\NodeVisitorAbstract;
14+
use PHPStan\Analyser\Scope;
15+
use PHPStan\Rules\IdentifierRuleError;
16+
use PHPStan\Rules\Rule;
17+
use PHPStan\Rules\RuleErrorBuilder;
18+
use function array_map;
19+
use function array_slice;
20+
use function array_values;
21+
use function count;
22+
use function is_string;
23+
24+
/**
25+
* @implements Rule<Expr>
26+
*/
27+
final class ForbidUnusedClosureParametersRule implements Rule
28+
{
29+
30+
public function getNodeType(): string
31+
{
32+
return Expr::class;
33+
}
34+
35+
/**
36+
* @param Expr $node
37+
* @return list<IdentifierRuleError>
38+
*/
39+
public function processNode(
40+
Node $node,
41+
Scope $scope
42+
): array
43+
{
44+
if ($node instanceof ArrowFunction) {
45+
$referencedVariables = $this->getReferencedVariables([$node->expr]);
46+
47+
} elseif ($node instanceof Closure) {
48+
$referencedVariables = $this->getReferencedVariables($node->stmts);
49+
50+
} else {
51+
return [];
52+
}
53+
54+
$errors = [];
55+
$trailingUnusedParameterNames = $this->getTrailingUnusedParameterNames(array_values($node->params), $referencedVariables);
56+
57+
$functionType = $node instanceof ArrowFunction ? 'Arrow function' : 'Closure';
58+
foreach ($trailingUnusedParameterNames as $parameterName) {
59+
$errors[] = RuleErrorBuilder::message("{$functionType} parameter \${$parameterName} is unused")
60+
->identifier('shipmonk.unusedParameter')
61+
->build();
62+
}
63+
64+
return $errors;
65+
}
66+
67+
/**
68+
* @param array<Node> $nodes
69+
* @return array<string, true>
70+
*/
71+
private function getReferencedVariables(array $nodes): array
72+
{
73+
$visitor = new class extends NodeVisitorAbstract {
74+
75+
/**
76+
* @var array<string, true>
77+
*/
78+
private array $usedVariables = [];
79+
80+
public function enterNode(Node $node): ?Node
81+
{
82+
if ($node instanceof Variable && is_string($node->name)) {
83+
$this->usedVariables[$node->name] = true;
84+
}
85+
86+
return null;
87+
}
88+
89+
/**
90+
* @return array<string, true>
91+
*/
92+
public function getUsedVariables(): array
93+
{
94+
return $this->usedVariables;
95+
}
96+
97+
};
98+
99+
$traverser = new NodeTraverser();
100+
$traverser->addVisitor($visitor);
101+
$traverser->traverse($nodes);
102+
103+
return $visitor->getUsedVariables();
104+
}
105+
106+
/**
107+
* @param list<Param> $params
108+
* @param array<string, true> $referencedVariables
109+
* @return list<string>
110+
*/
111+
private function getTrailingUnusedParameterNames(
112+
array $params,
113+
array $referencedVariables
114+
): array
115+
{
116+
for ($i = count($params) - 1; $i >= 0; $i--) {
117+
$param = $params[$i]; // @phpstan-ignore offsetAccess.notFound
118+
119+
if (!$param->var instanceof Variable) {
120+
continue;
121+
}
122+
123+
if (!is_string($param->var->name)) {
124+
continue;
125+
}
126+
127+
$parameterName = $param->var->name;
128+
129+
if (isset($referencedVariables[$parameterName])) {
130+
return $this->extractParamNames(array_slice($params, $i + 1));
131+
}
132+
}
133+
134+
return $this->extractParamNames($params);
135+
}
136+
137+
/**
138+
* @param array<Param> $params
139+
* @return list<string>
140+
*/
141+
private function extractParamNames(array $params): array
142+
{
143+
return array_values(array_map(static function (Param $param): string {
144+
if (!$param->var instanceof Variable) {
145+
throw new LogicException('Param variable must be a variable');
146+
}
147+
if (!is_string($param->var->name)) {
148+
throw new LogicException('Param variable name must be a string');
149+
}
150+
return $param->var->name;
151+
}, $params));
152+
}
153+
154+
}

‎tests/Rule/ForbidUnusedClosureParametersRuleTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule;
4+
5+
use PHPStan\Rules\Rule;
6+
use ShipMonk\PHPStan\RuleTestCase;
7+
8+
/**
9+
* @extends RuleTestCase<ForbidUnusedClosureParametersRule>
10+
*/
11+
class ForbidUnusedClosureParametersRuleTest extends RuleTestCase
12+
{
13+
14+
protected function getRule(): Rule
15+
{
16+
return new ForbidUnusedClosureParametersRule();
17+
}
18+
19+
public function testClass(): void
20+
{
21+
$this->analyseFile(__DIR__ . '/data/ForbidUnusedClosureParametersRule/code.php');
22+
}
23+
24+
}

‎tests/Rule/data/ForbidUnusedClosureParametersRule/code.php

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule\Data\ForbidUnusedClosureParametersRule;
4+
5+
class ClosureUnusedArgumentTestClass
6+
{
7+
8+
public function testUnusedParam(): void
9+
{
10+
$narrowAisleOccurrenceIds = [1, 2, 3];
11+
12+
// Arrow functions - This should trigger an error - $occurrenceIds is not used (trailing unused param)
13+
$callback = fn (array $occurrenceIds) => $narrowAisleOccurrenceIds; // error: Arrow function parameter $occurrenceIds is unused
14+
15+
// This should not trigger an error - both parameters are used
16+
$usedCallback = fn (array $ids, string $type) => str_contains($type, 'narrow') ? $ids : [];
17+
18+
// This should not trigger an error - single parameter is used
19+
$singleUsedCallback = fn (array $data) => count($data);
20+
21+
// This should trigger an error - only $unused is trailing unused (but $used is used)
22+
$mixedCallback = fn (array $used, string $unused) => count($used); // error: Arrow function parameter $unused is unused
23+
24+
// This should NOT trigger an error - $key is unused but needed for $receivingItem position
25+
$positionCallback = fn (int $key, \stdClass $receivingItem) => $receivingItem->id === 123;
26+
27+
// This should trigger an error - multiple trailing unused parameters
28+
$multipleUnusedCallback = fn (array $used, string $first, int $second) => count($used); // error: Arrow function parameter $first is unused // error: Arrow function parameter $second is unused
29+
30+
// This should trigger an error - all parameters are unused (all trailing)
31+
$allUnusedCallback = fn (array $first, string $second, int $third) => 'constant'; // error: Arrow function parameter $first is unused // error: Arrow function parameter $second is unused // error: Arrow function parameter $third is unused
32+
33+
// This should not trigger an error - no parameters
34+
$noParamsCallback = fn () => 'constant';
35+
36+
// This should not trigger an error - unused in middle, but used at end
37+
$unusedInMiddleCallback = fn (int $unused1, string $unused2, array $used) => count($used);
38+
39+
// Regular closures - This should trigger an error - $occurrenceIds is not used (trailing unused param)
40+
$closureCallback = function (array $occurrenceIds) { // error: Closure parameter $occurrenceIds is unused
41+
return $narrowAisleOccurrenceIds;
42+
};
43+
44+
// This should not trigger an error - both parameters are used
45+
$usedClosure = function (array $ids, string $type) {
46+
return str_contains($type, 'narrow') ? $ids : [];
47+
};
48+
49+
// This should trigger an error - only $unused is trailing unused
50+
$mixedClosure = function (array $used, string $unused) { // error: Closure parameter $unused is unused
51+
return count($used);
52+
};
53+
54+
// This should NOT trigger an error - $key is unused but needed for $receivingItem position
55+
$positionClosure = function (int $key, \stdClass $receivingItem) {
56+
return $receivingItem->id === 123;
57+
};
58+
59+
// This should trigger an error - multiple trailing unused parameters
60+
$multipleUnusedClosure = function (array $used, string $first, int $second) { // error: Closure parameter $first is unused // error: Closure parameter $second is unused
61+
return count($used);
62+
};
63+
64+
// This should not trigger an error - unused in middle, but used at end
65+
$unusedInMiddleClosure = function (int $unused1, string $unused2, array $used) {
66+
return count($used);
67+
};
68+
}
69+
70+
}

0 commit comments

Comments
 (0)