Thank you for this PR.

What would the benefit of this change be?

I fear this will create needless merge conflicts in the future. I think we should wait a few more years before doing so. Like we did for the short array syntax.

As much as I'd love to use CPP for the whole codebase, we will still need to merge changes from lower branches. This change is a source of potential merge conflicts, which is why I'm leaning towards rejecting it.

I agree with @nicolas-grekas and @derrabus here. This change brings no value to users of the framework (as using CPP versus writing the unsugared code is purely an implementation detail of the class and does not impact its usage) while having a maintenance cost as long as we cannot apply it on all maintained branches.
So this is a -1 from me

As explained, having this syntactic sugar only on some branches is likely to trigger headaches when merging branches up.
I'm going to close this PR and use it as reference for future PRs, until we can use it on all active branches (i.e. once everything is PHP 8+).
Thanks for raising the topic, Tomas.

Thanks for feedback.

I fear this will create needless merge conflicts in the future. I think we should wait a few more years before doing so. Like we did for the short array syntax.

The merge conflicts can be removed as this process is automated. Basically you con contribute old ctor approach a new properties in 2 different PRs and they both will be normalized to promoted ones.

What would the benefit of this change be?

Promoted properties themselves narrow potential bugs to 1/3, as definition is written only once. The rest of benefits can be found in the RFC.

Another benefit is consistency with contributed code. As minimum PHP versoin suggest PHP 8 code is accepted. In some period of time it will be confusing what language version should be respected where.

Let me know when the right time comes :) this a matter of one day to setup with @rectorphp.

The "merge conflicts" referred to here is when we fix a bug in branch 4.4 and have to merge it up to 5.3, 5.4 and 6.0.

Promoted properties themselves narrow potential bugs to 1/3, as definition is written only once

That problem is already solved using static code checkers.

Another benefit is consistency with contributed code.

That is an argument against this PR. We dont want different styles for contributing to 5.4 compared to 6.0. That is what @nicolas-grekas loosely referred to.

Let me know when the right time comes

When we eventually switch to promoted properties, we will definitely automate it.

This was a very short but important discussion. Thank you again Tomas for raising it.

Thus, migrating to CPP involves migrating to typed properties first (as removing the typehints for constructor arguments is not an option). And that change is a lot more risky (as can be seen in this PR where you only migrated 2 properties to CPP, with one of them being broken)

If you update rector to only apply the CPP migration to properties that are already typed (which it should do, otherwise the migration is unsafe), it will not migrate anything in Symfony.

The merge conflicts can be removed as this process is automated.

They can't because we will create a codebase that significantly differs from the still maintained 4.4 and 5.2/5.3/5.4 branches. 5.4 will live on for years because it's LTS. Merging from 5.4 to 6.0 and up will become harder this way. CPP is purely syntactic sugar and that's imho not enough to accept such a maintenance burden.

this a matter of one day to setup with @rectorphp.

This is certainly a task that can be automated. I've done this in one of my projects with PhpStorm and I really liked the result.

However, we need to be careful on Symfony's codebase. Just blindly converting all constructors to CPP will also sneak in typed properties that this codebase barely uses at the moment. Unlike CPP alone, typed properties actually do have side effects, so we need to be careful when introducing them to an existing codebase.

Either way: typed properties have to be the first step before we can think of CPP.

We dont want different styles for contributing to 5.4 compared to 6.0.

I would disagree here. I would allow CPP in contributions to a 6.x branch if…

• … CPP is used in an entirely new class
• … the constructor of an existing class is changed significantly during a change