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

[Downgrade PHP 7.2] Parameter type widening #4754

Closed
leoloso opened this issue Dec 2, 2020 · 4 comments · Fixed by #4818
Closed

[Downgrade PHP 7.2] Parameter type widening #4754

leoloso opened this issue Dec 2, 2020 · 4 comments · Fixed by #4818
Labels

Comments

@leoloso
Copy link
Contributor

leoloso commented Dec 2, 2020

Feature Request

Downgrade new feature from PHP 7.2 to its equivalent PHP 7.1 code. Description.

Because the child class parameter type can be more generic than the parent class (check argument contravariance), we can't simply copy the same parameter type from parent to child, since it could generate an error from invoking the method in the child with a more generic type.

A possible solution is to remove the type on the parent:

interface A
{
-    public function test(array $input);
+    /**
+     * @param array $input
+     */
+    public function test($input);
}

class B implements A
{
    public function test($input){} // type omitted for $input
}

Watch out: then the type must also be removed everywhere, and maybe this is difficult to achieve. Because the change to be done does not happen on the child class, but on the parent class, the Rector rule should check if a method on the parent class has been overloaded removing the parameter type on any child class and, if so, remove the parameter in ALL methods from all extending classes!

For instance, this class must also be modified, even though it didn't introduce any change in the method signature:

class C implements A
{
-    public function test(array $input){}
+    public function test($input){}
}

Is it too messy? Is there a better approach?

@leoloso leoloso added the feature label Dec 2, 2020
@leoloso leoloso changed the title [Downgrade PHP 7.2] Parameter type widening ¶ [Downgrade PHP 7.2] Parameter type widening Dec 2, 2020
@leoloso
Copy link
Contributor Author

leoloso commented Dec 3, 2020

The same strategy has been implemented for ParamTypeDeclarationRector. I'll use this rule as source

@leoloso
Copy link
Contributor Author

leoloso commented Dec 3, 2020

The strategy is: for every method from a class and interface, check if the same method in child/implementing classes has some parameter with a different typehint. If so, the parameter typehint must be removed everywhere: both in the parent class/interface, and in ALL the child/implementing classes (not just the one with a different signature).

@leoloso
Copy link
Contributor Author

leoloso commented Dec 3, 2020

There is a big issue: the downgrade script for Rector is currently downgrading package by package, limiting the input files to those of a certain package. Hence, we won't be able to modify signatures from other packages than the current one being downgraded.

2 solutions:

  1. Do not downgrade package by package, but the whole project together
  2. Directly remove all parameter types, ALWAYS, even if there's no need to modify the code (ugly, but it will work)

@leoloso
Copy link
Contributor Author

leoloso commented Dec 3, 2020

Argument types were introduced to PHP 7.0: https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.scalar-type-declarations

Hence, we could implement and execute that rule for the Rector downgrade script

TomasVotruba added a commit that referenced this issue Aug 10, 2023
rectorphp/rector-src@f4b71a5 [TypeDeclaration] Use native type detection instead of docblock on AssignToPropertyTypeInferer for TypedPropertyFromAssignsRector (#4754)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant