Skip to content

Conversation

@eliashaeussler
Copy link
Contributor

@eliashaeussler eliashaeussler commented Dec 9, 2022

@samsonasik
Copy link
Member

samsonasik commented Dec 9, 2022

Could you try to provide a patch for it? It seems you can check on ClassConstManipulator by remove the following check:

$class = $this->betterNodeFinder->findParentType($classConst, Class_::class);
if (! $class instanceof Class_) {
return false;
}

-        $class = $this->betterNodeFinder->findParentType($classConst, Class_::class);
-        if (! $class instanceof Class_) {
-            return false;
-        }

since it already check on next $classReflection->getAncestors() which fallback to return false in the end.

@eliashaeussler
Copy link
Contributor Author

@samsonasik Thanks for the hint, I'm already on it and will provide a fix soon.

@samsonasik
Copy link
Member

After fix applied, please rename PR title to something like:

[DeadCode] Skip Class Constant used in Enum on RemoveUnusedPrivateClassConstantRector

@eliashaeussler eliashaeussler changed the title Add failing test fixture for RemoveUnusedPrivateClassConstantRector [DeadCode] Skip Class Constant used in Enum on RemoveUnusedPrivateCla… Dec 9, 2022
Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@TomasVotruba TomasVotruba merged commit b23e1fb into rectorphp:main Dec 9, 2022
@TomasVotruba
Copy link
Member

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect behavior of RemoveUnusedPrivateClassConstantRector

3 participants