mirror of
https://github.com/danog/psalm.git
synced 2024-11-26 20:34:47 +01:00
Add fixer for mismatching param names
This commit is contained in:
parent
669a843cb0
commit
dbcf154036
@ -669,6 +669,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
|
||||
MethodComparator::compare(
|
||||
$codebase,
|
||||
null,
|
||||
$implementer_classlike_storage ?: $storage,
|
||||
$interface_storage,
|
||||
$implementer_method_storage,
|
||||
@ -938,6 +939,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
|
||||
MethodComparator::compare(
|
||||
$codebase,
|
||||
null,
|
||||
$storage,
|
||||
$parent_storage,
|
||||
$pseudo_method_storage,
|
||||
@ -1839,6 +1841,7 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
|
||||
MethodComparator::compare(
|
||||
$codebase,
|
||||
$stmt,
|
||||
$class_storage,
|
||||
$declaring_storage,
|
||||
$implementer_method_storage,
|
||||
|
@ -298,6 +298,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
|
||||
if (!isset($appearing_class_storage->class_implements[strtolower($overridden_fq_class_name)])) {
|
||||
MethodComparator::compare(
|
||||
$codebase,
|
||||
\count($overridden_method_ids) === 1 ? $this->function : null,
|
||||
$declaring_class_storage,
|
||||
$parent_storage,
|
||||
$storage,
|
||||
|
@ -1,9 +1,11 @@
|
||||
<?php
|
||||
namespace Psalm\Internal\Analyzer;
|
||||
|
||||
use PhpParser\Node\Stmt\ClassMethod;
|
||||
use Psalm\Codebase;
|
||||
use Psalm\CodeLocation;
|
||||
use Psalm\Internal\MethodIdentifier;
|
||||
use Psalm\Internal\PhpVisitor\ParamReplacementVisitor;
|
||||
use Psalm\Internal\Type\Comparator\TypeComparisonResult;
|
||||
use Psalm\Internal\Type\Comparator\UnionTypeComparator;
|
||||
use Psalm\Issue\ImplementedParamTypeMismatch;
|
||||
@ -42,6 +44,7 @@ class MethodComparator
|
||||
*/
|
||||
public static function compare(
|
||||
Codebase $codebase,
|
||||
?ClassMethod $stmt,
|
||||
ClassLikeStorage $implementer_classlike_storage,
|
||||
ClassLikeStorage $guide_classlike_storage,
|
||||
MethodStorage $implementer_method_storage,
|
||||
@ -50,8 +53,8 @@ class MethodComparator
|
||||
int $implementer_visibility,
|
||||
CodeLocation $code_location,
|
||||
array $suppressed_issues,
|
||||
$prevent_abstract_override = true,
|
||||
$prevent_method_signature_mismatch = true
|
||||
bool $prevent_abstract_override = true,
|
||||
bool $prevent_method_signature_mismatch = true
|
||||
) {
|
||||
$implementer_declaring_method_id = $codebase->methods->getDeclaringMethodId(
|
||||
new MethodIdentifier(
|
||||
@ -140,6 +143,7 @@ class MethodComparator
|
||||
|
||||
self::compareMethodParams(
|
||||
$codebase,
|
||||
$stmt,
|
||||
$implementer_classlike_storage,
|
||||
$guide_classlike_storage,
|
||||
$implementer_called_class_name,
|
||||
@ -289,6 +293,7 @@ class MethodComparator
|
||||
*/
|
||||
private static function compareMethodParams(
|
||||
Codebase $codebase,
|
||||
?ClassMethod $stmt,
|
||||
ClassLikeStorage $implementer_classlike_storage,
|
||||
ClassLikeStorage $guide_classlike_storage,
|
||||
string $implementer_called_class_name,
|
||||
@ -387,6 +392,7 @@ class MethodComparator
|
||||
&& count($implementer_method_storage->params) > 1
|
||||
&& $guide_classlike_storage->user_defined
|
||||
&& $implementer_classlike_storage->user_defined
|
||||
&& $implementer_classlike_storage->location
|
||||
) {
|
||||
$config = \Psalm\Config::getInstance();
|
||||
|
||||
@ -395,21 +401,43 @@ class MethodComparator
|
||||
&& !$config->isInProjectDirs($guide_classlike_storage->location->file_path)
|
||||
)
|
||||
) {
|
||||
if (IssueBuffer::accepts(
|
||||
new ParamNameMismatch(
|
||||
'Argument ' . ($i + 1) . ' of ' . $cased_implementer_method_id . ' has wrong name $'
|
||||
. $implementer_param->name . ', expecting $'
|
||||
. $guide_param->name . ' as defined by '
|
||||
. $cased_guide_method_id,
|
||||
$implementer_param->location
|
||||
&& $config->isInProjectDirs(
|
||||
$implementer_param->location->file_path
|
||||
)
|
||||
? $implementer_param->location
|
||||
: $code_location
|
||||
)
|
||||
)) {
|
||||
// fall through
|
||||
if ($codebase->alter_code) {
|
||||
$project_analyzer = \Psalm\Internal\Analyzer\ProjectAnalyzer::getInstance();
|
||||
|
||||
if ($stmt && isset($project_analyzer->getIssuesToFix()['ParamNameMismatch'])) {
|
||||
$param_replacer = new ParamReplacementVisitor(
|
||||
$implementer_param->name,
|
||||
$guide_param->name
|
||||
);
|
||||
|
||||
$traverser = new \PhpParser\NodeTraverser();
|
||||
$traverser->addVisitor($param_replacer);
|
||||
$traverser->traverse([$stmt]);
|
||||
|
||||
if ($replacements = $param_replacer->getReplacements()) {
|
||||
\Psalm\Internal\FileManipulation\FileManipulationBuffer::add(
|
||||
$implementer_classlike_storage->location->file_path,
|
||||
$replacements
|
||||
);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new ParamNameMismatch(
|
||||
'Argument ' . ($i + 1) . ' of ' . $cased_implementer_method_id . ' has wrong name $'
|
||||
. $implementer_param->name . ', expecting $'
|
||||
. $guide_param->name . ' as defined by '
|
||||
. $cased_guide_method_id,
|
||||
$implementer_param->location
|
||||
&& $config->isInProjectDirs(
|
||||
$implementer_param->location->file_path
|
||||
)
|
||||
? $implementer_param->location
|
||||
: $code_location
|
||||
)
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -23,6 +23,7 @@ use Psalm\Issue\MissingClosureReturnType;
|
||||
use Psalm\Issue\MissingParamType;
|
||||
use Psalm\Issue\MissingPropertyType;
|
||||
use Psalm\Issue\MissingReturnType;
|
||||
use Psalm\Issue\ParamNameMismatch;
|
||||
use Psalm\Issue\PossiblyUndefinedGlobalVariable;
|
||||
use Psalm\Issue\PossiblyUndefinedVariable;
|
||||
use Psalm\Issue\PossiblyUnusedMethod;
|
||||
@ -217,6 +218,7 @@ class ProjectAnalyzer
|
||||
MissingParamType::class,
|
||||
MissingPropertyType::class,
|
||||
MissingReturnType::class,
|
||||
ParamNameMismatch::class,
|
||||
PossiblyUndefinedGlobalVariable::class,
|
||||
PossiblyUndefinedVariable::class,
|
||||
PossiblyUnusedMethod::class,
|
||||
|
78
src/Psalm/Internal/PhpVisitor/ParamReplacementVisitor.php
Normal file
78
src/Psalm/Internal/PhpVisitor/ParamReplacementVisitor.php
Normal file
@ -0,0 +1,78 @@
|
||||
<?php
|
||||
namespace Psalm\Internal\PhpVisitor;
|
||||
|
||||
use PhpParser;
|
||||
use Psalm\FileManipulation;
|
||||
|
||||
/**
|
||||
* @internal
|
||||
*/
|
||||
class ParamReplacementVisitor extends PhpParser\NodeVisitorAbstract implements PhpParser\NodeVisitor
|
||||
{
|
||||
/** @var string */
|
||||
private $old_name;
|
||||
|
||||
/** @var string */
|
||||
private $new_name;
|
||||
|
||||
/** @var list<FileManipulation> */
|
||||
private $replacements = [];
|
||||
|
||||
/** @var bool */
|
||||
private $new_name_replaced = false;
|
||||
|
||||
/** @var bool */
|
||||
private $new_new_name_used = false;
|
||||
|
||||
public function __construct(string $old_name, string $new_name)
|
||||
{
|
||||
$this->old_name = $old_name;
|
||||
$this->new_name = $new_name;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param PhpParser\Node $node
|
||||
*
|
||||
* @return null|int
|
||||
*/
|
||||
public function enterNode(PhpParser\Node $node)
|
||||
{
|
||||
if ($node instanceof PhpParser\Node\Expr\Variable) {
|
||||
if ($node->name === $this->old_name) {
|
||||
$this->replacements[] = new FileManipulation(
|
||||
(int) $node->getAttribute('startFilePos') + 1,
|
||||
(int) $node->getAttribute('endFilePos') + 1,
|
||||
$this->new_name
|
||||
);
|
||||
} elseif ($node->name === $this->new_name) {
|
||||
if ($this->new_new_name_used) {
|
||||
$this->replacements = [];
|
||||
return PhpParser\NodeTraverser::STOP_TRAVERSAL;
|
||||
}
|
||||
|
||||
$this->replacements[] = new FileManipulation(
|
||||
(int) $node->getAttribute('startFilePos') + 1,
|
||||
(int) $node->getAttribute('endFilePos') + 1,
|
||||
$this->new_name . '_new'
|
||||
);
|
||||
|
||||
$this->new_name_replaced = true;
|
||||
} elseif ($node->name === $this->new_name . '_new') {
|
||||
if ($this->new_name_replaced) {
|
||||
$this->replacements = [];
|
||||
return PhpParser\NodeTraverser::STOP_TRAVERSAL;
|
||||
}
|
||||
|
||||
$this->new_new_name_used = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<FileManipulation>
|
||||
*/
|
||||
public function getReplacements()
|
||||
{
|
||||
return $this->replacements;
|
||||
}
|
||||
}
|
97
tests/FileManipulation/ParamNameMismatchTest.php
Normal file
97
tests/FileManipulation/ParamNameMismatchTest.php
Normal file
@ -0,0 +1,97 @@
|
||||
<?php
|
||||
namespace Psalm\Tests\FileManipulation;
|
||||
|
||||
class ParamNameMismatchTest extends FileManipulationTest
|
||||
{
|
||||
/**
|
||||
* @return array<string,array{string,string,string,string[],bool}>
|
||||
*/
|
||||
public function providerValidCodeParse()
|
||||
{
|
||||
return [
|
||||
'fixMismatchingParamName74' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {
|
||||
$str = $string;
|
||||
$string = $string === "hello" ? "foo" : "bar";
|
||||
echo $string;
|
||||
echo $str;
|
||||
}
|
||||
}',
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $str, bool $b = false) : void {
|
||||
$str_new = $str;
|
||||
$str = $str === "hello" ? "foo" : "bar";
|
||||
echo $str;
|
||||
echo $str_new;
|
||||
}
|
||||
}',
|
||||
'7.1',
|
||||
['ParamNameMismatch'],
|
||||
true,
|
||||
],
|
||||
'fixIfNewNameAlreadyExists74' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {
|
||||
$str_new = $string;
|
||||
}
|
||||
}',
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $str, bool $b = false) : void {
|
||||
$str_new = $str;
|
||||
}
|
||||
}',
|
||||
'7.1',
|
||||
['ParamNameMismatch'],
|
||||
true,
|
||||
],
|
||||
'noFixIfNewNameAndOldNameAlreadyExists74' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {
|
||||
$str = $string;
|
||||
$str_new = $string;
|
||||
}
|
||||
}',
|
||||
'<?php
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {
|
||||
$str = $string;
|
||||
$str_new = $string;
|
||||
}
|
||||
}',
|
||||
'7.1',
|
||||
['ParamNameMismatch'],
|
||||
true,
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user