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

[Rector] Update comment usage for TypedPropertyFromAssignsRector #61

Merged

Conversation

samsonasik
Copy link
Member

Per discussion on codeigniter4/shield#560 (comment), I updated comment for TypedPropertyFromAssignsRector

@samsonasik samsonasik merged commit f5ea3a2 into develop Dec 16, 2022
@kenjis kenjis deleted the update-comment-usage-typed-property-from-assigns branch December 19, 2022 01:43
/**
* The INLINE_PUBLIC value is default to false to avoid BC break, if you use for libraries and want to preserve BC break, you don't need to configure it, as it included in LevelSetList::UP_TO_PHP_74
* Set to true for projects that allow BC break
*/
TypedPropertyFromAssignsRector::INLINE_PUBLIC => true,
Copy link
Member

Choose a reason for hiding this comment

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

I have applied this setting to an App, but many files that extend framework classes were broken because not all properties are typed.

Realistically, it seems to be difficult to use this setting with true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's true is usefull when need to be applied for whole project.

Probably next improvement to verify vendor/ location like in ParentClassMethodTypeOverrideGuard

https://github.com/rectorphp/rector-src/blob/8354587b777450cb9f2e3bd48cf750d81b78cf05/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php#L61-L84

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

Successfully merging this pull request may close these issues.

2 participants