refactor: PHP 8.0 constructor promotion#6924
refactor: PHP 8.0 constructor promotion#6924kenjis wants to merge 12 commits intocodeigniter4:4.3from
Conversation
Applied rules: * StrEndsWithRector (https://wiki.php.net/rfc/add_str_starts_with_and_ends_with_functions) * StrStartsWithRector (https://wiki.php.net/rfc/add_str_starts_with_and_ends_with_functions) * StrContainsRector (https://externals.io/message/108562 php/php-src#5179)
…anual tweaks This may contains breaking changes.
|
This is a breaking chage: |
|
Yes, that seems need to be skipped |
| $this->autoloader = $autoloader; | ||
| public function __construct( | ||
| protected Autoloader $autoloader | ||
| ) { |
There was a problem hiding this comment.
For example, if a dev extends FileLocator and it has protected $autoloader;,
this change is a breaking change.
https://3v4l.org/NhqDb
There was a problem hiding this comment.
There are possible options:
- make a note about property re-definition possible breaking change without add type
- wait to push to 4.4 only
There was a problem hiding this comment.
Another way;: make Rector extension to help user migrating to new definition.
There was a problem hiding this comment.
Removing type in the constructor makes no errors (I don't know why),
https://3v4l.org/2A85L
but I don't want to remove already declared types in constructors.
There was a problem hiding this comment.
Better way: make a patch to rector to:
- skip non-typed property for protected/public property, except define
INLINE_PUBLICconfig that allow bc break on non-final class, same like TypedPropertyRector way
That's just quick guessing, probably there is a better way.
There was a problem hiding this comment.
@kenjis here possibly the solution, I create PR to make it configurable INLINE_PUBLIC default false:
There was a problem hiding this comment.
This seems the best approach to me. Glad your patch was already accepted!
|
@kenjis some rules like |
|
If we drop PHP 7.4 in v4.4, I would like to type missing public/protected properties with constructor promotion. |
|
Yes, I agree 👍 |
Needs #6923
Description
See #6921
Use constructor promotion.
This may contain breaking changes.
Checklist: