Skip to content
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

[DeadCode][TypeDeclaration] Handle Nullable Param on RemoveUnusedPrivateMethodParameterRector+RemoveUnreachableStatementRector+AddArrayParamDocTypeRector #1216

Merged
merged 7 commits into from Nov 12, 2021

Conversation

samsonasik
Copy link
Member

Given the following code:

use PhpParser\Node\Stmt\ClassLike;

class Fixture
{
    public function run(?ClassLike $classLike): bool
    {
        if (! $classLike instanceof ClassLike) {
            return false;
        }

        return $this->callPrivateMethod($classLike, true);
    }

    private function callPrivateMethod(ClassLike $classLike, bool $value)
    {
        return $classLike->getMethods() !== [];
    }
}

It produce addition docblock:

+    /**
+     * @param \ClassLike|null $classLike
+     */

while removing unused parameter in private method. The Param itself is invalid as there is no \ClassLike class, but PhpParser\Node\Stmt\ClassLike which imported in use statement.

Applied rules:

Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector;
Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector;
Rector\TypeDeclaration\Rector\ClassMethod\AddArrayParamDocTypeRector;

…Rector+RemoveUnreachableStatementRector+AddArrayParamDocTypeRector
@samsonasik
Copy link
Member Author

Fixed 🎉

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

Comment on lines 146 to 148
if ($type instanceof ObjectType && ! $type instanceof FullyQualifiedObjectType) {
return new MixedType();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would falsy transform all short object types to mixed. That's incorrect.

Instead we should find out, why the short class name is resolved. It will affect more rules.
IMO it will be related somehow to NameResolver.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with namespacedName attribute in a Name node check 9e49b53 🎉

@samsonasik
Copy link
Member Author

samsonasik commented Nov 12, 2021

oh, that's seems broke sabbelasichon/typo3-rector package test now https://github.com/rectorphp/rector-src/runs/4189871248?check_suite_focus=true#step:7:24

@TomasVotruba
Copy link
Member

This looks better! 👍

It broke the typo3 package probalby, not sure why. Could you check it?

@samsonasik
Copy link
Member Author

I will try.

@samsonasik
Copy link
Member Author

samsonasik commented Nov 12, 2021

Fixed 🎉 by move the namespacedName check under NullableType for verify its type 9304060

@samsonasik samsonasik force-pushed the handle-nullable-param-namespaced branch from 650f605 to 9304060 Compare November 12, 2021 15:06
@samsonasik samsonasik force-pushed the handle-nullable-param-namespaced branch from 650f605 to f0c47c0 Compare November 12, 2021 15:07
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@TomasVotruba TomasVotruba merged commit f775b6e into main Nov 12, 2021
@TomasVotruba TomasVotruba deleted the handle-nullable-param-namespaced branch November 12, 2021 19:10
@TomasVotruba
Copy link
Member

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants