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

[TypeDeclaration] optionally only add types for hard coded return values in ReturnTypeFromStrictScalarReturnExprRector #5364

Merged

Conversation

RobertMe
Copy link
Contributor

Supersedes #5361, relates to rectorphp/rector#8362

@RobertMe
Copy link
Contributor Author

Build failure seems to be unrelated (/incidental hickup by composer while downloading a package?)

@RobertMe
Copy link
Contributor Author

Ran it on my actual work project. And well, it works 😃

Did it in two stages, as desired. So first with the hard_coded_only option enabled which did precisely what I wanted it to do (only add return types to methods with hardcoded return true; / return 10; / return 'some string';). Followed by a second run without the option set which added return type hints for the more "complex" returns (based on strict method calls, string concats, ...), on top of it. And to validate everything still properly worked reverted the changes to Rector (the rule and analyzer) and ran it again, which didn't make any additional changes.

@TomasVotruba
Copy link
Member

@RobertMe Thanks for fast work. I drifted my attention away over holidays and only got to see this one. Could you rebase the PR?

cc @samsonasik Could you review this one?

@RobertMe RobertMe force-pushed the return-type-from-scalar-hardcoded branch from b196f84 to fb2d6ae Compare December 29, 2023 20:22
@RobertMe
Copy link
Contributor Author

@TomasVotruba , sure, rebased.

I don't mind it took a bit longer, it's the holiday season anyway and everybody needs some rest 😄

@RobertMe RobertMe force-pushed the return-type-from-scalar-hardcoded branch from fb2d6ae to 594536b Compare December 30, 2023 11:48
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.

Looking good 👍

@samsonasik
Copy link
Member

Thank you @RobertMe

…ues in `ReturnTypeFromStrictScalarReturnExprRector`
}
CODE_SAMPLE
,
[
ReturnTypeFromStrictScalarReturnExprRector::HARD_CODED_ONLY => true,
Copy link
Member

Choose a reason for hiding this comment

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

This need false per default value for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yip, and the example output was wrong as well 🙈 Noticed that myself too when once more going over your comments

@RobertMe RobertMe force-pushed the return-type-from-scalar-hardcoded branch from 594536b to b7262ea Compare December 30, 2023 11:55
@samsonasik samsonasik enabled auto-merge (squash) December 30, 2023 11:56
@samsonasik samsonasik merged commit e044d50 into rectorphp:main Dec 30, 2023
40 checks passed
@RobertMe
Copy link
Contributor Author

Thank you @RobertMe

Thank you @samsonasik for the review and merging 😃. You were even quicker with merging then I was going through the comments (and then finding the "issue" with the config example and having to fix it once more).

@TomasVotruba
Copy link
Member

Thank you @RobertMe 👏

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