Rector: [Question] Is there a safe type mode?

[orklah]

Question

Psalm, with its tool to change code, has a --safe-types mode who exclude phpdoc types from its type inference engine.

This is really useful because we're sure to avoid any mistakes due to wrong phpdoc somewhere.

Does Rector have something similar?

Thanks!

[TomasVotruba]

Hi, where exactly would you use such feature here?

[orklah]

Hi,

Well, I want to avoid this at all cost:
https://getrector.org/demo/d5d420f2-ce40-4542-b44d-0fafa9940461

The second fix by rector will provoke a TypeError in strict_types and an implicit cast if not in strict_types.

Rector is adding the type hint "int" because another function was declaring it was its return type in phpdoc.

I don't have enough trust in our phpdoc and/or tests to risk this on our work codebase

Psalm has a --safe-types option that allows ignoring phpdoc when editing such code and will only add types when they're certain (for example, after a cast or if we return the result of a function that has a type hint too). It's a very reassuring feature and it allow modifying code almost blindly. I was wondering if it was possible or already implemented in Rector

[TomasVotruba]

I see.

You could add such option as parameter into particular *TypeInferer services.
If on, it could allow exclude services implementing e.g. PhpDocTypeIntefererInterface

[orklah]

If I understand right, your solution would be to exclude every service that rely on phpdoc?

Is there a solution to modify/duplicate a service to make it ignore phpdoc instead?

[TomasVotruba]

Just add the interface to *TypeInteferer ones.

[TomasVotruba]

E.g. add it to this one: https://github.com/rectorphp/rector/blob/master/rules/type-declaration/src/TypeInferer/ParamTypeInferer/FunctionLikeDocParamTypeInferer.php

And filter the interface based on parameter here: https://github.com/rectorphp/rector/blob/a2e11ba1ff4b8d0c316e39b56ac7a868844f279b/rules/type-declaration/src/TypeInferer/ParamTypeInferer.php#L29-L36

[orklah]

Ok, i'll try to understand all that, I'm not too familiar with Rector's internal yet. Thanks!

[orklah]

I tried to tweak a little, I managed to exclude ReturnTagReturnTypeInferer from ReturnTypeInferer. It worked but it's not enough. The int type also come from the ReturnedNodesReturnTypeInferer. However, if I try to exclude this one, Rector stop doing anything for this service so this is clearly essential.

I tried to search inside but it seems to depend directly from phpstan. I'm not sure there's a way to totally exclude unsafe sources of types unless it is implemented directly into phpstan...

[TomasVotruba]

You might need to decouple ReturnedNodesReturnTypeInferer into 2 services. One working with doc, the other with PHP code only.

unless it is implemented directly into phpstan...

Agreed, you might need to add this feature to PHPStan first, as we depend on it in type resolving.

[orklah]

I'm pretty sure it's a big task in PHPStan. Each source must be tracked, any union must take this into account... I don't think I'll be able to make it, and even so, this could be a big overhead and something more to be maintained. My personnal use case is not THAT important!

Unless you want to keep this open, I think we can close this issue.

Thanks for your advices!

[TomasVotruba]

Closing as answered

[TomasVotruba]

@orklah This might be useful to you https://github.com/rectorphp/rector/pull/3535 :wink:

Experimental, but needs testing. Would you mind trying it?

[orklah]

Oh! that sounds interesting! I'll try that!