Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnusedFunctionParametersCheck: report precise line #3743

Merged
merged 4 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,8 @@ services:

-
class: PHPStan\Rules\UnusedFunctionParametersCheck
arguments:
reportExactLine: %featureToggles.bleedingEdge%
janedbal marked this conversation as resolved.
Show resolved Hide resolved

-
class: PHPStan\Rules\TooWideTypehints\TooWideParameterOutTypeCheck
Expand Down
7 changes: 3 additions & 4 deletions src/Rules/Classes/UnusedConstructorParametersRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
9 changes: 1 addition & 8 deletions src/Rules/Functions/UnusedClosureUsesRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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',
Expand Down
35 changes: 24 additions & 11 deletions src/Rules/UnusedFunctionParametersCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand All @@ -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'], []);
Expand Down
6 changes: 3 additions & 3 deletions tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
Loading