[Php80] Add MixedTypeRector

[samsonasik]

Fixes rectorphp/rector#7301

[samsonasik]

All checks have passed 🎉 @TomasVotruba it is ready for review.

For summary:

• Applied mixed type when:

  • no param type
  • param doc is mixed

• Handle virtual Node when combined with other rules https://github.com/rectorphp/rector-src/pull/2701/files#diff-42a5a059b66868f3ede960ea9a50a3591de610668457d76063ac2f6dc653dcc3 , ref

  • [Php80] Add MixedTypeRector  #2701 (comment)
  • [Php80] Add MixedTypeRector  #2701 (comment)

For Return Type, I think that require different PR as that will need various handling, eg: actually return void.

[samsonasik]

it seems when parent method has mixed type already, and current doesn't has type doc, it should be transformed to mixed.

[samsonasik]

@TomasVotruba so, I think the use case like this should be skipped:

class VendorParam
{
    /**
     * @param int $param
     */
    public function run($param)
    {}
}

class UseParam extends VendorParam
{
    public function run($param)
    {}
}

Next, when parent method int param doc be typed, has child widen with mixed should not be applied while it is working ok, same type is better.

There are other use case like typed already that should be skipped as well:

class A
{
    public function run(int $a)
    {}
}

class B extends A
{
    public function run($a)
    {}
}

as same type in child should be preferred, so there should be different rule or phpstan that handle that

https://phpstan.org/r/c1ffffc4-e76a-4367-adf7-26608665d9b1

[samsonasik]

implemented with add MakeClassMethodParamMixedTypedGuard

[TomasVotruba]

@TomasVotruba so, I think the use case like this should be skipped:

Agreed. I think only the doc-typed mixed should be included, so the intention is already defined by user.
Implicit mixed types should be skipped, as the absence of doc-type does not mean mixed type. Maybe it's just missing.

public function run($value)
{
}

-/**
- * @param mixed $value
- */
-public function run($value)
+public function run(mixed $value)
 {
 }

[samsonasik]

The issue with only check mixed doc type is when we use the following code in code base:

interface SomeInterface 
{ 
    /** @param mixed $data */
    public function save($data);
}

class Memory extends SomeInterface
{
    public function save($data) {}
}

That will only change interface, and that will cause error something like:

 ------------------------------------------------------------------------
  packages/Caching/ValueObject/Storage/MemoryCacheStorage.php:37
 ------------------------------------------------------------------------
  - '#Method Rector\\Caching\\ValueObject\\Storage\\MemoryCacheStorage\:\:save\(\) has parameter \$data with no type specified#'

see https://github.com/rectorphp/rector-src/runs/7473011787?check_suite_focus=true#step:5:16 , that's why I created MakeClassMethodParamMixedTypedGuard to cover that.

[samsonasik]

Ok, let's only make typed with use doc @param mixed

[TomasVotruba]

I think these cases should be skipped, as imlicit mixed doc can be overriden by explicit type in of childs:

Then it could crash:
https://3v4l.org/DGidN

[samsonasik]

@TomasVotruba It seems we need to ensure it only apply change when:

• no parent method
• no child method

that will also ensure no manual add mixed type when parent has @param mixed and child not, or child using @inheritDoc, what do you think?

[TomasVotruba]

That makes sense 👍

[samsonasik]

for ClassMethod, I think the mixed type can be applied for final class.

[samsonasik]

or not, as final class can have parent

[samsonasik]

implemented check has child or parent method 🎉

[samsonasik]

All checks have passed 🎉 @TomasVotruba I think it is ready.

[samsonasik]

rebased.

[TomasVotruba]

Thanks 👍