diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 7227e369b3..76dd0d8904 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -4,3 +4,4 @@ parameters: checkParameterCastableToNumberFunctions: true skipCheckGenericClasses!: [] stricterFunctionMap: true + reportPreciseLineForUnusedFunctionParameter: true diff --git a/conf/config.neon b/conf/config.neon index 40e57d400d..d77e889531 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -25,6 +25,7 @@ parameters: checkParameterCastableToNumberFunctions: false skipCheckGenericClasses: [] stricterFunctionMap: false + reportPreciseLineForUnusedFunctionParameter: false fileExtensions: - php checkAdvancedIsset: false @@ -1043,6 +1044,8 @@ services: - class: PHPStan\Rules\UnusedFunctionParametersCheck + arguments: + reportExactLine: %featureToggles.reportPreciseLineForUnusedFunctionParameter% - class: PHPStan\Rules\TooWideTypehints\TooWideParameterOutTypeCheck diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 3dbe4e87ec..f7328f7863 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -31,6 +31,7 @@ parametersSchema: checkParameterCastableToNumberFunctions: bool(), skipCheckGenericClasses: listOf(string()), stricterFunctionMap: bool() + reportPreciseLineForUnusedFunctionParameter: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Classes/UnusedConstructorParametersRule.php b/src/Rules/Classes/UnusedConstructorParametersRule.php index e42a60d5f7..8b38392470 100644 --- a/src/Rules/Classes/UnusedConstructorParametersRule.php +++ b/src/Rules/Classes/UnusedConstructorParametersRule.php @@ -15,7 +15,6 @@ use function array_map; use function array_values; use function count; -use function is_string; use function sprintf; use function strtolower; @@ -56,11 +55,11 @@ public function processNode(Node $node, Scope $scope): array return $this->check->getUnusedParameters( $scope, - array_map(static function (Param $parameter): string { - if (!$parameter->var instanceof Variable || !is_string($parameter->var->name)) { + array_map(static function (Param $parameter): Variable { + if (!$parameter->var instanceof Variable) { throw new ShouldNotHappenException(); } - return $parameter->var->name; + return $parameter->var; }, array_values(array_filter($originalNode->params, static fn (Param $parameter): bool => $parameter->flags === 0))), $originalNode->stmts, $message, diff --git a/src/Rules/Functions/UnusedClosureUsesRule.php b/src/Rules/Functions/UnusedClosureUsesRule.php index c019d69240..ed9639e41f 100644 --- a/src/Rules/Functions/UnusedClosureUsesRule.php +++ b/src/Rules/Functions/UnusedClosureUsesRule.php @@ -6,10 +6,8 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\UnusedFunctionParametersCheck; -use PHPStan\ShouldNotHappenException; use function array_map; use function count; -use function is_string; /** * @implements Rule @@ -34,12 +32,7 @@ public function processNode(Node $node, Scope $scope): array return $this->check->getUnusedParameters( $scope, - array_map(static function (Node\ClosureUse $use): string { - if (!is_string($use->var->name)) { - throw new ShouldNotHappenException(); - } - return $use->var->name; - }, $node->uses), + array_map(static fn (Node\ClosureUse $use): Node\Expr\Variable => $use->var, $node->uses), $node->stmts, 'Anonymous function has an unused use $%s.', 'closure.unusedUse', diff --git a/src/Rules/UnusedFunctionParametersCheck.php b/src/Rules/UnusedFunctionParametersCheck.php index 4fbe76d20d..628041a032 100644 --- a/src/Rules/UnusedFunctionParametersCheck.php +++ b/src/Rules/UnusedFunctionParametersCheck.php @@ -3,11 +3,13 @@ namespace PHPStan\Rules; use PhpParser\Node; +use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\Scope; use PHPStan\Reflection\ReflectionProvider; +use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantStringType; -use function array_fill_keys; -use function array_keys; +use function array_combine; +use function array_map; use function array_merge; use function is_array; use function is_string; @@ -16,25 +18,34 @@ final class UnusedFunctionParametersCheck { - public function __construct(private ReflectionProvider $reflectionProvider) + public function __construct( + private ReflectionProvider $reflectionProvider, + private bool $reportExactLine, + ) { } /** - * @param string[] $parameterNames + * @param Variable[] $parameterVars * @param Node[] $statements * @param 'constructor.unusedParameter'|'closure.unusedUse' $identifier * @return list */ public function getUnusedParameters( Scope $scope, - array $parameterNames, + array $parameterVars, array $statements, string $unusedParameterMessage, string $identifier, ): array { - $unusedParameters = array_fill_keys($parameterNames, true); + $parameterNames = array_map(static function (Variable $variable): string { + if (!is_string($variable->name)) { + throw new ShouldNotHappenException(); + } + return $variable->name; + }, $parameterVars); + $unusedParameters = array_combine($parameterNames, $parameterVars); foreach ($this->getUsedVariables($scope, $statements) as $variableName) { if (!isset($unusedParameters[$variableName])) { continue; @@ -43,10 +54,12 @@ public function getUnusedParameters( unset($unusedParameters[$variableName]); } $errors = []; - foreach (array_keys($unusedParameters) as $name) { - $errors[] = RuleErrorBuilder::message( - sprintf($unusedParameterMessage, $name), - )->identifier($identifier)->build(); + foreach ($unusedParameters as $name => $variable) { + $errorBuilder = RuleErrorBuilder::message(sprintf($unusedParameterMessage, $name))->identifier($identifier); + if ($this->reportExactLine) { + $errorBuilder->line($variable->getStartLine()); + } + $errors[] = $errorBuilder->build(); } return $errors; @@ -66,7 +79,7 @@ private function getUsedVariables(Scope $scope, $node): array return $scope->getDefinedVariables(); } } - if ($node instanceof Node\Expr\Variable && is_string($node->name) && $node->name !== 'this') { + if ($node instanceof Variable && is_string($node->name) && $node->name !== 'this') { return [$node->name]; } if ($node instanceof Node\ClosureUse && is_string($node->var->name)) { diff --git a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php index b6530920df..beb402c267 100644 --- a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php +++ b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php @@ -12,15 +12,19 @@ class UnusedConstructorParametersRuleTest extends RuleTestCase { + private bool $reportExactLine = true; + protected function getRule(): Rule { return new UnusedConstructorParametersRule(new UnusedFunctionParametersCheck( $this->createReflectionProvider(), + $this->reportExactLine, )); } - public function testUnusedConstructorParameters(): void + public function testUnusedConstructorParametersNoExactLine(): void { + $this->reportExactLine = false; $this->analyse([__DIR__ . '/data/unused-constructor-parameters.php'], [ [ 'Constructor of class UnusedConstructorParameters\Foo has an unused parameter $unusedParameter.', @@ -33,6 +37,20 @@ public function testUnusedConstructorParameters(): void ]); } + public function testUnusedConstructorParameters(): void + { + $this->analyse([__DIR__ . '/data/unused-constructor-parameters.php'], [ + [ + 'Constructor of class UnusedConstructorParameters\Foo has an unused parameter $unusedParameter.', + 19, + ], + [ + 'Constructor of class UnusedConstructorParameters\Foo has an unused parameter $anotherUnusedParameter.', + 20, + ], + ]); + } + public function testPromotedProperties(): void { $this->analyse([__DIR__ . '/data/unused-constructor-parameters-promoted-properties.php'], []); diff --git a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php index 0d033268d8..38a555afda 100644 --- a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php @@ -14,7 +14,7 @@ class UnusedClosureUsesRuleTest extends RuleTestCase protected function getRule(): Rule { - return new UnusedClosureUsesRule(new UnusedFunctionParametersCheck($this->createReflectionProvider())); + return new UnusedClosureUsesRule(new UnusedFunctionParametersCheck($this->createReflectionProvider(), true)); } public function testUnusedClosureUses(): void @@ -22,11 +22,11 @@ public function testUnusedClosureUses(): void $this->analyse([__DIR__ . '/data/unused-closure-uses.php'], [ [ 'Anonymous function has an unused use $unused.', - 3, + 6, ], [ 'Anonymous function has an unused use $anotherUnused.', - 3, + 7, ], [ 'Anonymous function has an unused use $usedInClosureUse.',