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.

Sign up for GitHub

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

Jump to bottom

[AutoImport] Do not add cast on valid Integer type on Name Node on auto import enabled #6236

Merged
merged 8 commits into from
Aug 16, 2024
Merged

[AutoImport] Do not add cast on valid Integer type on Name Node on auto import enabled #6236

merged 8 commits into from
Aug 16, 2024

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik marked this pull request as draft August 16, 2024 16:20
@samsonasik samsonasik force-pushed the do-not-add-valid-int-cast-from-name branch from 91a6c1e to 7bed825 Compare August 16, 2024 16:25
@samsonasik
Copy link
Member Author

@carlos-granados @TomasVotruba I added ClassConstFetchTypeResolver which verify its $node->class is a Name with namespacedName attribute to cover it

a0bef12

I will add more test for direct Name usage when possible, eg: on Some\StaticClass::execute() to cast not casted toint while Some\StaticClass already imported to verify that namespacedName attribute is needed on NameTypeResolver

@samsonasik
Copy link
Member Author

I added failing fixture for from StaticCall 6a28fe4 which got cast int because of it read a Name

rector-src/src/NodeTypeResolver/NodeTypeResolver/StaticCallMethodCallTypeResolver.php

Line 72 in 3ed476b

$callerType = $this->nodeTypeResolver->getType($node->class);

which has namespacedName which should pull it from it.

@samsonasik samsonasik marked this pull request as ready for review August 16, 2024 17:05
@samsonasik
Copy link
Member Author

Fixed 🎉

samsonasik
samsonasik commented Aug 16, 2024
View reviewed changes
src/NodeTypeResolver/NodeTypeResolver/NameTypeResolver.php
Comment on lines +40 to +43
// not instanceof FullyQualified means it is a Name
if (! $node instanceof FullyQualified && $node->hasAttribute(AttributeKey::NAMESPACED_NAME)) {
return $this->resolve(new FullyQualified($node->getAttribute(AttributeKey::NAMESPACED_NAME)));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

this is really needed to patch StaticCall that imported fixture https://github.com/rectorphp/rector-src/pull/6236/files#diff-ec6cd31df51e19b0199b350a318339fbc3e2b1aa740593511a466849e7fc5289 as a proof that namespacedName attribute is needed on NameTypeResolver 👍

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 174adf1 into main Aug 16, 2024
35 checks passed
@samsonasik samsonasik deleted the do-not-add-valid-int-cast-from-name branch August 16, 2024 17:11
@samsonasik samsonasik mentioned this pull request Aug 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect behavior of ConsoleExecuteReturnIntRector
1 participant
@samsonasik