mirror of
https://github.com/danog/psalm.git
synced 2024-11-30 04:39:00 +01:00
Detect mismatching param names effectively
This commit is contained in:
parent
50cc3a8afa
commit
6085e42fc1
@ -74,6 +74,7 @@
|
||||
<xs:attribute name="sealAllMethods" type="xs:boolean" default="false" />
|
||||
<xs:attribute name="runTaintAnalysis" type="xs:boolean" default="false" />
|
||||
<xs:attribute name="usePhpStormMetaPath" type="xs:boolean" default="true" />
|
||||
<xs:attribute name="allowInternalNamedParamCalls" type="xs:boolean" default="true" />
|
||||
</xs:complexType>
|
||||
|
||||
<xs:complexType name="ProjectFilesType">
|
||||
@ -299,6 +300,7 @@
|
||||
<xs:element name="NullReference" type="IssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="OverriddenMethodAccess" type="IssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="OverriddenPropertyAccess" type="PropertyIssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="ParamNameMismatch" type="IssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="ParadoxicalCondition" type="IssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="ParentNotFound" type="IssueHandlerType" minOccurs="0" />
|
||||
<xs:element name="PossibleRawObjectIteration" type="IssueHandlerType" minOccurs="0" />
|
||||
|
87
docs/running_psalm/issues/ParamNameMismatch.md
Normal file
87
docs/running_psalm/issues/ParamNameMismatch.md
Normal file
@ -0,0 +1,87 @@
|
||||
# ParamNameMismatch
|
||||
|
||||
Emitted when method overrides a parent method but renames a param.
|
||||
|
||||
```php
|
||||
<?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 {}
|
||||
}
|
||||
```
|
||||
|
||||
## Why is this bad?
|
||||
|
||||
PHP 8 introduces [named parameters](https://wiki.php.net/rfc/named_params) which allow developers to call methods with explicitly-named parameters;
|
||||
|
||||
```php
|
||||
<?php
|
||||
|
||||
function callFoo(A $a) {
|
||||
$a->foo(str: "hello");
|
||||
}
|
||||
```
|
||||
|
||||
In the first example passing `new AChild()` to `callFoo()` results in a fatal error, as AChild's definition of the method `foo()` doesn't have a parameter named `$str`.
|
||||
|
||||
## How to fix
|
||||
|
||||
You can change the child method param name to match:
|
||||
|
||||
```php
|
||||
<?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 {}
|
||||
}
|
||||
```
|
||||
|
||||
## Workarounds
|
||||
|
||||
### @no-named-params
|
||||
|
||||
Alternatively you can ignore this issue by adding a `@no-named-params` annotation to the parent method:
|
||||
|
||||
```php
|
||||
<?php
|
||||
|
||||
class A {
|
||||
/** @no-named-params */
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {}
|
||||
}
|
||||
```
|
||||
|
||||
Any method with this annotation will be prevented (by Psalm) from being called with named parameters, so the original issue does not matter.
|
||||
|
||||
### Config allowInternalNamedParamCalls="false"
|
||||
|
||||
You can also set a config flag that tells Psalm to prohibit any named parameter calls on `@internal` classes or methods.
|
||||
|
||||
With that config value, this is now allowed:
|
||||
|
||||
```php
|
||||
<?php
|
||||
|
||||
/**
|
||||
* @internal
|
||||
*/
|
||||
class A {
|
||||
public function foo(string $str, bool $b = false) : void {}
|
||||
}
|
||||
|
||||
class AChild extends A {
|
||||
public function foo(string $string, bool $b = false) : void {}
|
||||
}
|
||||
```
|
@ -520,6 +520,11 @@ class Config
|
||||
*/
|
||||
public $before_analyze_file = [];
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
public $allow_internal_named_param_calls = true;
|
||||
|
||||
/**
|
||||
* Static methods to be called after functionlike checks have completed
|
||||
*
|
||||
@ -818,6 +823,7 @@ class Config
|
||||
'sealAllMethods' => 'seal_all_methods',
|
||||
'runTaintAnalysis' => 'run_taint_analysis',
|
||||
'usePhpStormMetaPath' => 'use_phpstorm_meta_path',
|
||||
'allowInternalNamedParamCalls' => 'allow_internal_named_param_calls',
|
||||
];
|
||||
|
||||
foreach ($booleanAttributes as $xmlName => $internalName) {
|
||||
|
@ -756,6 +756,10 @@ class CommentAnalyzer
|
||||
$info->external_mutation_free = true;
|
||||
}
|
||||
|
||||
if (isset($parsed_docblock->tags['no-named-params'])) {
|
||||
$info->no_named_params = true;
|
||||
}
|
||||
|
||||
$info->ignore_nullable_return = isset($parsed_docblock->tags['psalm-ignore-nullable-return']);
|
||||
$info->ignore_falsable_return = isset($parsed_docblock->tags['psalm-ignore-falsable-return']);
|
||||
|
||||
|
@ -11,6 +11,7 @@ use Psalm\Issue\ImplementedReturnTypeMismatch;
|
||||
use Psalm\Issue\MethodSignatureMismatch;
|
||||
use Psalm\Issue\MoreSpecificImplementedParamType;
|
||||
use Psalm\Issue\LessSpecificImplementedReturnType;
|
||||
use Psalm\Issue\ParamNameMismatch;
|
||||
use Psalm\Issue\OverriddenMethodAccess;
|
||||
use Psalm\Issue\TraitMethodSignatureMismatch;
|
||||
use Psalm\IssueBuffer;
|
||||
@ -344,6 +345,32 @@ class MethodComparator
|
||||
}
|
||||
}
|
||||
|
||||
if ($guide_param->name !== $implementer_param->name
|
||||
&& $guide_method_storage->allow_named_param_calls
|
||||
&& count($implementer_method_storage->params) > 1
|
||||
&& $guide_classlike_storage->user_defined
|
||||
&& $implementer_classlike_storage->user_defined
|
||||
) {
|
||||
$config = \Psalm\Config::getInstance();
|
||||
|
||||
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 ($guide_classlike_storage->user_defined
|
||||
&& $implementer_param->signature_type
|
||||
) {
|
||||
|
@ -2278,6 +2278,14 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse
|
||||
$storage->internal = $docblock_info->psalm_internal ?? '';
|
||||
}
|
||||
|
||||
if (($storage->internal || ($class_storage && $class_storage->internal))
|
||||
&& !$this->config->allow_internal_named_param_calls
|
||||
) {
|
||||
$storage->allow_named_param_calls = false;
|
||||
} elseif ($docblock_info->no_named_params) {
|
||||
$storage->allow_named_param_calls = false;
|
||||
}
|
||||
|
||||
if ($docblock_info->variadic) {
|
||||
$storage->variadic = true;
|
||||
}
|
||||
|
@ -459,4 +459,9 @@ class StatementsProvider
|
||||
|
||||
return $stmts;
|
||||
}
|
||||
|
||||
public static function clearLexer() : void
|
||||
{
|
||||
self::$lexer = null;
|
||||
}
|
||||
}
|
||||
|
@ -188,4 +188,9 @@ class FunctionDocblockComment
|
||||
* @var bool
|
||||
*/
|
||||
public $external_mutation_free = false;
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
public $no_named_params = false;
|
||||
}
|
||||
|
8
src/Psalm/Issue/ParamNameMismatch.php
Normal file
8
src/Psalm/Issue/ParamNameMismatch.php
Normal file
@ -0,0 +1,8 @@
|
||||
<?php
|
||||
namespace Psalm\Issue;
|
||||
|
||||
class ParamNameMismatch extends CodeIssue
|
||||
{
|
||||
const ERROR_LEVEL = 7;
|
||||
const SHORTCODE = 230;
|
||||
}
|
@ -199,6 +199,11 @@ abstract class FunctionLikeStorage
|
||||
*/
|
||||
public $return_source_params = [];
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
public $allow_named_param_calls = true;
|
||||
|
||||
public function __toString()
|
||||
{
|
||||
return $this->getSignature(false);
|
||||
|
@ -869,7 +869,7 @@ class MethodSignatureTest extends TestCase
|
||||
}',
|
||||
'error_message' => 'Method B::fooFoo has fewer parameters than parent method A::fooFoo',
|
||||
],
|
||||
'differentArguments' => [
|
||||
'differentArgumentTypes' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function fooFoo(int $a, bool $b): void {
|
||||
@ -878,13 +878,28 @@ class MethodSignatureTest extends TestCase
|
||||
}
|
||||
|
||||
class B extends A {
|
||||
public function fooFoo(bool $b, int $a): void {
|
||||
public function fooFoo(int $a, int $b): void {
|
||||
|
||||
}
|
||||
}',
|
||||
'error_message' => 'Argument 1 of B::fooFoo has wrong type \'bool\', expecting \'int\' as defined ' .
|
||||
'error_message' => 'Argument 2 of B::fooFoo has wrong type \'int\', expecting \'bool\' as defined ' .
|
||||
'by A::fooFoo',
|
||||
],
|
||||
'differentArgumentNames' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function fooFoo(int $a, bool $b): void {
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
class B extends A {
|
||||
public function fooFoo(int $a, bool $c): void {
|
||||
|
||||
}
|
||||
}',
|
||||
'error_message' => 'ParamNameMismatch',
|
||||
],
|
||||
'nonNullableSubclassParam' => [
|
||||
'<?php
|
||||
class A {
|
||||
|
@ -2739,7 +2739,10 @@ class ClassTemplateTest extends TestCase
|
||||
* @return T
|
||||
*/
|
||||
function unwrap(array $containers) {
|
||||
return array_map(fn($container) => $container->get(), $containers)[0];
|
||||
return array_map(
|
||||
fn($container) => $container->get(),
|
||||
$containers
|
||||
)[0];
|
||||
}
|
||||
|
||||
/**
|
||||
@ -2750,7 +2753,10 @@ class ClassTemplateTest extends TestCase
|
||||
|
||||
if (is_string($ret)) {}
|
||||
if (is_int($ret)) {}
|
||||
}'
|
||||
}',
|
||||
[],
|
||||
[],
|
||||
'7.4'
|
||||
],
|
||||
'templateWithLateResolvedType' => [
|
||||
'<?php
|
||||
|
@ -1332,7 +1332,10 @@ class FunctionTemplateTest extends TestCase
|
||||
*/
|
||||
function foo(Closure $fn, $arg): void {
|
||||
$a = partial($fn, $arg);
|
||||
}'
|
||||
}',
|
||||
[],
|
||||
[],
|
||||
'7.4'
|
||||
],
|
||||
];
|
||||
}
|
||||
|
@ -62,6 +62,8 @@ class TestCase extends BaseTestCase
|
||||
|
||||
FileAnalyzer::clearCache();
|
||||
|
||||
\Psalm\Internal\Provider\StatementsProvider::clearLexer();
|
||||
|
||||
$this->file_provider = new \Psalm\Tests\Internal\Provider\FakeFileProvider();
|
||||
|
||||
$config = $this->makeConfig();
|
||||
@ -76,6 +78,8 @@ class TestCase extends BaseTestCase
|
||||
$providers
|
||||
);
|
||||
|
||||
|
||||
|
||||
$this->project_analyzer->setPhpVersion('7.3');
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user