Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ parameters:
checkParameterCastableToNumberFunctions: true
skipCheckGenericClasses!: []
stricterFunctionMap: true
reportPreciseLineForUnusedFunctionParameter: true
3 changes: 3 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ parameters:
checkParameterCastableToNumberFunctions: false
skipCheckGenericClasses: []
stricterFunctionMap: false
reportPreciseLineForUnusedFunctionParameter: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -1043,6 +1044,8 @@ services:

-
class: PHPStan\Rules\UnusedFunctionParametersCheck
arguments:
reportExactLine: %featureToggles.reportPreciseLineForUnusedFunctionParameter%

-
class: PHPStan\Rules\TooWideTypehints\TooWideParameterOutTypeCheck
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ parametersSchema:
checkParameterCastableToNumberFunctions: bool(),
skipCheckGenericClasses: listOf(string()),
stricterFunctionMap: bool()
reportPreciseLineForUnusedFunctionParameter: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
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