Bleeding edge - UnusedFunctionParametersCheck: report precise line

Author: janedbal

conf/bleedingEdge.neon

@@ -4,3 +4,4 @@ parameters:
 		checkParameterCastableToNumberFunctions: true
 		skipCheckGenericClasses!: []
 		stricterFunctionMap: true
+		reportPreciseLineForUnusedFunctionParameter: true

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

conf/parametersSchema.neon

@@ -31,6 +31,7 @@ parametersSchema:
 		checkParameterCastableToNumberFunctions: bool(),
 		skipCheckGenericClasses: listOf(string()),
 		stricterFunctionMap: bool()
+		reportPreciseLineForUnusedFunctionParameter: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAdvancedIsset: bool()

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,

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<Node\Expr\Closure>
@@ -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',

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<IdentifierRuleError>
 	 */
 	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)) {

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'], []);

tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php

@@ -14,19 +14,19 @@ 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
 	{
 		$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.',