[TypeDeclaration] Adds AddClosureParamTypeFromArgRector #6258
Conversation
TomasVotruba
changed the title
| $type = match (true) { | ||
| $node instanceof MethodCall => $node->var, | ||
| $node instanceof StaticCall => $node->class, | ||
| default => null, | ||
| }; |
There was a problem hiding this comment.
In this way we loose type of $node, that's why the if (! ($node->name ?? null) instanceof Identifier) { was needed.
Instead, the instanceof should be used to always improve knowledge about varaible types.
| } else { | ||
| $args = array_filter($callLike->getArgs(), static function (Arg $arg) use ( | ||
| $addParamTypeForFunctionLikeWithinCallLikeArgFromArgDeclaration | ||
| ): bool { | ||
| if ($arg->name === null) { | ||
| return false; | ||
| } | ||
|
|
||
| return $arg->name->name === $addParamTypeForFunctionLikeWithinCallLikeArgFromArgDeclaration->getCallLikePosition(); | ||
| }); |
There was a problem hiding this comment.
I've removed the string named args feature, as those are skipped now anyway. I'd limit this rule to work only with standard args in future as well, as area of use case is pretty narrow.
| @@ -69,11 +67,12 @@ public function getNodeTypes(): array | |||
| } | |||
|
|
|||
| /** | |||
| * @param CallLike $node | |||
| * @param MethodCall|StaticCall $node | |||
There was a problem hiding this comment.
It's always better to have exact same types as in getNodeTypes(). The CallLike is to generic and it gives the refactor() method types that are never passed.
| private function processFunctionLike( | ||
| CallLike $callLike, | ||
| private function processCallLike( | ||
| MethodCall|StaticCall $callLike, |
There was a problem hiding this comment.
Result of well defined types in refactor() docblock 👍
| $argType = $this->nodeTypeResolver->getType($arg->value); | ||
| if (! $argType instanceof ConstantStringType) { | ||
| return; |
There was a problem hiding this comment.
PHPStan can help us with class name detection 👍
| SomeClass::someCall(SomeClass::class, function (\SomeNamespace\SomeClass $object) { | ||
| SimpleContainer::someCall(\Rector\Tests\TypeDeclaration\Rector\FunctionLike\AddClosureParamTypeFromArgClassStringRector\Source\SomeType::class, function (\Rector\Tests\TypeDeclaration\Rector\FunctionLike\AddClosureParamTypeFromArgClassStringRector\Source\SomeType $object) { |
There was a problem hiding this comment.
By defalut, non-existing class are seen as strings. By both PHPStan and Rector.
That's why it's better to use existing class in tests, so we know it's an object and not a dummy string.
| /** | ||
| * @see \Rector\Tests\TypeDeclaration\Rector\FunctionLike\AddClosureParamTypeFromArgRector\AddClosureParamTypeFromArgRectorTest | ||
| */ | ||
| final class AddClosureParamTypeFromArgClassStringRector extends AbstractRector implements ConfigurableRectorInterface |
There was a problem hiding this comment.
We can separate string and object type now, so both rule now do the same. I've merged them together.
TomasVotruba
force-pushed
the
feature/argument-type-closure
branch
from
e35731d to
3b5c524
Compare
TomasVotruba
force-pushed
the
feature/argument-type-closure
branch
from
3b5c524 to
a934e09
Compare
| // stmts aware type | ||
| RenameParamToMatchTypeRector::class => [ | ||
| __DIR__ . '/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php', | ||
| ], | ||
| AddMethodCallBasedStrictParamTypeRector::class => [ | ||
| __DIR__ . '/rules/DeadCode/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php', | ||
| ], |
There was a problem hiding this comment.
this is not needed, I tested latest main branch without this skip and it is working ok, I will create PR to remove it.
There was a problem hiding this comment.
see #6262, tested with rm -rf vendor && composer update && bin/rector, nothing change on rules directory
There was a problem hiding this comment.
I had some local glitches with PHPStan cache.
Patches get stuck sometimes:)
Thanks for restoring 👌
There was a problem hiding this comment.
You may need to force remove it
php -r 'shell_exec("rm -rf " . sys_get_temp_dir() . "/rector_cached_files");';
php -r 'shell_exec("rm -rf " . sys_get_temp_dir() . "/cache/PHPStan");';
php -r 'shell_exec("rm -rf " . sys_get_temp_dir() . "/cache/nette.configurator");';
Rebase and merge of original PR #6198 by @peterfox 🙏
Closes #6198