-
Notifications
You must be signed in to change notification settings - Fork 574
Skip class name case check for type hints using explicit use ... as aliases
#5671
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
Changes from all commits
e25f35a
89fadb0
d6625e9
3030d4a
cba9669
61b22b4
fa1050e
af672cf
86daf13
da7c907
7c69c26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace PHPStan\Parser; | ||
|
|
||
| use Override; | ||
| use PhpParser\Node; | ||
| use PhpParser\Node\Name; | ||
| use PhpParser\Node\Stmt\GroupUse; | ||
| use PhpParser\Node\Stmt\Use_; | ||
| use PhpParser\NodeVisitorAbstract; | ||
| use PHPStan\DependencyInjection\AutowiredService; | ||
| use function count; | ||
| use function strtolower; | ||
|
|
||
| #[AutowiredService] | ||
| final class UseAliasVisitor extends NodeVisitorAbstract | ||
| { | ||
|
|
||
| public const ATTRIBUTE_NAME = 'isExplicitUseAlias'; | ||
|
|
||
| /** @var array<string, string> alias name (original case) keyed by lowercase alias name */ | ||
| private array $explicitAliases = []; | ||
|
|
||
| #[Override] | ||
| public function enterNode(Node $node): ?Node | ||
| { | ||
| if ($node instanceof Node\Stmt\Namespace_) { | ||
| $this->explicitAliases = []; | ||
| } | ||
|
|
||
| if ($node instanceof Use_ && $node->type === Use_::TYPE_NORMAL) { | ||
| foreach ($node->uses as $use) { | ||
| if ($use->alias === null) { | ||
| continue; | ||
| } | ||
|
|
||
| $this->explicitAliases[strtolower($use->alias->name)] = $use->alias->name; | ||
| } | ||
| } | ||
|
|
||
| if ($node instanceof GroupUse) { | ||
| foreach ($node->uses as $use) { | ||
| if ($use->type !== Use_::TYPE_NORMAL && $node->type !== Use_::TYPE_NORMAL) { | ||
| continue; | ||
| } | ||
| if ($use->alias === null) { | ||
| continue; | ||
| } | ||
|
|
||
| $this->explicitAliases[strtolower($use->alias->name)] = $use->alias->name; | ||
| } | ||
| } | ||
|
|
||
| if ($node instanceof Name) { | ||
| $originalName = $node->getAttribute('originalName'); | ||
| if ($originalName instanceof Name) { | ||
| $originalParts = $originalName->getParts(); | ||
| if (count($originalParts) === 1) { | ||
| $lowerOriginal = strtolower($originalParts[0]); | ||
| if ( | ||
| isset($this->explicitAliases[$lowerOriginal]) | ||
| && $this->explicitAliases[$lowerOriginal] === $originalParts[0] | ||
| ) { | ||
| $node->setAttribute(self::ATTRIBUTE_NAME, true); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| use PHPStan\DependencyInjection\AutowiredParameter; | ||
| use PHPStan\DependencyInjection\AutowiredService; | ||
| use PHPStan\Parser\UseAliasVisitor; | ||
| use PHPStan\Reflection\ReflectionProvider; | ||
| use function sprintf; | ||
| use function strtolower; | ||
|
|
@@ -38,7 +39,10 @@ public function checkClassNames(array $pairs): array | |
| } | ||
| $realClassName = $classReflection->getName(); | ||
| if (strtolower($realClassName) !== strtolower($className)) { | ||
| continue; // skip class alias | ||
| continue; // skip class_alias() where the alias is a completely different name | ||
| } | ||
|
Comment on lines
41
to
43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should/Could this if be removed too ?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's a summary of what was done to address the review: The The Changes made:
All 12,076 tests pass and |
||
| if ($pair->getNode()->getAttribute(UseAliasVisitor::ATTRIBUTE_NAME) === true) { | ||
| continue; | ||
| } | ||
| if ($realClassName === $className) { | ||
| continue; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617Classes; | ||
|
|
||
| class MyClass {} | ||
|
|
||
| interface MyInterface {} | ||
|
|
||
| namespace Bug14617Classes\Consumer; | ||
|
|
||
| use Bug14617Classes\MyClass as myclass; | ||
| use Bug14617Classes\MyInterface as myinterface; | ||
|
|
||
| class Foo extends myclass implements myinterface { | ||
| public myclass $prop; | ||
| } | ||
|
|
||
| function test(mixed $x): void { | ||
| if ($x instanceof myclass) { | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617Closure; | ||
|
|
||
| class MyClass {} | ||
|
|
||
| namespace Bug14617Closure\Consumer; | ||
|
|
||
| use Bug14617Closure\MyClass as myclass; | ||
|
|
||
| $callback = function (myclass $a): myclass { | ||
| return $a; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617GroupUse; | ||
|
|
||
| class MyClass {} | ||
| class AnotherClass {} | ||
|
|
||
| namespace Bug14617GroupUse\Consumer; | ||
|
|
||
| use Bug14617GroupUse\{MyClass as myclass, AnotherClass as anotherclass}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add another test for
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushed. Added test coverage for
No implementation change was needed because function name case checks compare the imported FQN ( |
||
|
|
||
| $callback = function (myclass $a): anotherclass { | ||
| return new anotherclass(); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617UseFunctionAlias; | ||
|
|
||
| function myFunction(): void {} | ||
| function anotherFunction(): void {} | ||
|
|
||
| namespace Bug14617UseFunctionAlias\Consumer; | ||
|
|
||
| use function Bug14617UseFunctionAlias\myFunction as myfunction; | ||
| use function Bug14617UseFunctionAlias\{anotherFunction as ANOTHERFUNCTION}; | ||
|
|
||
| myfunction(); | ||
| ANOTHERFUNCTION(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace ClassAliasCaseSensitivity; | ||
|
|
||
| $callback = function (\ReturnTypes\FooAlias $a): \ReturnTypes\FooAlias { | ||
| return $a; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617GroupUseMethod; | ||
|
|
||
| class MyClass {} | ||
| class AnotherClass {} | ||
|
|
||
| namespace Bug14617GroupUseMethod\Consumer; | ||
|
|
||
| use Bug14617GroupUseMethod\{MyClass as myclass, AnotherClass as anotherclass}; | ||
|
|
||
| class Foo { | ||
| public function bar(myclass $a): anotherclass { | ||
| return new anotherclass(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617; | ||
|
|
||
| class MyClass {} | ||
|
|
||
| namespace Bug14617\Consumer; | ||
|
|
||
| use Bug14617\MyClass as myclass; | ||
|
|
||
| function test(): myclass { | ||
| return new myclass(); | ||
| } | ||
|
|
||
| class Foo { | ||
| public function bar(myclass $a): myclass { | ||
| return $a; | ||
| } | ||
|
|
||
| public function nullable(?myclass $a): ?myclass { | ||
| return $a; | ||
| } | ||
|
|
||
| public function union(myclass|string $a): myclass|int { | ||
| return $a; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617UseFunctionAliasNs; | ||
|
|
||
| function myFunction(): void {} | ||
|
|
||
| namespace Bug14617UseFunctionAliasNs\Consumer; | ||
|
|
||
| use function Bug14617UseFunctionAliasNs\myFunction as myfunction; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?php declare(strict_types = 1); | ||
|
|
||
| namespace Bug14617UseFunctionGroupAliasNs; | ||
|
|
||
| function myFunction(): void {} | ||
| function anotherFunction(): void {} | ||
|
|
||
| namespace Bug14617UseFunctionGroupAliasNs\Consumer; | ||
|
|
||
| use function Bug14617UseFunctionGroupAliasNs\{myFunction as myfunction, anotherFunction as ANOTHERFUNCTION}; |
Uh oh!
There was an error while loading. Please reload this page.