RequireConstructorPropertyPromotion: Potential bug with different variable name to property name

[mcampbell508]

trafficstars

Just found this project for things like RequireConstructorPropertyPromotion, which look really cool.

I ran it on some files in a project and may have either uncovered a bug or something that should maybe be covered by another sniff, first.

Problem

class ParamsWithDifferentNameToProperty
{
	private TypeOne $paramOne;

	private TypeTwo $paramTwo;

	public function __construct(TypeOne $paramOne, TypeTwo $paramTwoWithDifferentName)
	{
		$this->paramTwo = $paramTwoWithDifferentName;
		$this->paramOne = $paramOne;
	}
}

Expected (If not the responsibility of another rule / fixer)

class ParamsWithDifferentNameToProperty
{
-	private TypeOne $paramOne;

-	private TypeTwo $paramTwo;

-	public function __construct(TypeOne $paramOne, TypeTwo $paramTwoWithDifferentName)
+	public function __construct(private TypeOne $paramOne, private TypeTwo $paramTwo)
	{
-               $this->paramOne = $paramOne;	
-		$this->paramTwo = $paramTwoWithDifferentName;
	}
}

Actual

class ParamsWithDifferentNameToProperty
{
-	private TypeOne $paramOne;

	private TypeTwo $paramTwo;

-	public function __construct(TypeOne $paramOne, TypeTwo $paramTwoWithDifferentName)
+	public function __construct(private TypeOne $paramOne, TypeTwo $paramTwoWithDifferentName)
	{
-               $this->paramOne = $paramOne;
		$this->paramTwo = $paramTwoWithDifferentName;
	}
}

I guess this is quite tricky as there is a clear rename of the argument variable above, and it would only be possible / would have to handle instances where the variable is used elsewhere within the constructor.

Again, this may be the responsibility of an existing or new sniff/fixer that does the rename if possible and then this rule is able to work as it does already, without modification.

There is also opportunity to add a config option which allows this rename, potentially, defaulting to false, maybe. I.e something like:

• allowArgumentToMatchPropertyRename: Allow constructor argument variables to be renamed to match their assigned property type name. By default, it is disabled.

and then this config added as a new feature... Just an idea.

[enl]

Automatically renaming constructor parameter names is dangerous thing in some frameworks, e.g. Magento relies on parameter name in their DI framework.