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

ReturnTypeFromStrictNewArrayRector: missing array return type - part 2 #8185

Closed
staabm opened this issue Sep 6, 2023 · 5 comments · Fixed by rectorphp/rector-src#4928
Closed
Labels

Comments

@staabm
Copy link
Contributor

staabm commented Sep 6, 2023

Bug Report

similar to #8183 I would expect rector to add a array return type in

https://getrector.com/demo/aed924b0-d4e6-4522-a0f2-d8a5034ca416

only difference is the added block

        if (count($offers) == 0) {
            return [];
        }
@staabm staabm added the bug label Sep 6, 2023
@samsonasik
Copy link
Member

ReturnTypeFromStrictNewArrayRector seems designed for only returns once instead of multiple returns and only return variable of assigned to array;

$returns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($node, Return_::class);
if (\count($returns) !== 1) {
return null;
}
if ($this->isVariableOverriddenWithNonArray($node, $variable)) {
return null;
}

@staabm
Copy link
Contributor Author

staabm commented Sep 6, 2023

if we would widen the scope to 1 dynamic array return and n-constant array/null-returns, we could support the initial snippet and additionally things like https://getrector.com/demo/c74af6a5-bc6a-479d-8469-cb3a2e56dd5a

@samsonasik
Copy link
Member

samsonasik commented Sep 6, 2023

For union nullable, I think that can use ReturnUnionTypeRector https://getrector.com/demo/8154c1f5-8db4-4a8e-949a-b38b31042c9c

@samsonasik
Copy link
Member

@staabm ReturnTypeFromStrictNewArrayRector seems only cover Variable, while for possible return array, It is on ReturnTypeFromReturnDirectArrayRector, the ReturnTypeFromReturnDirectArrayRector doesn't support variable return tho, so it is possible improvement for fallback support of the 2 rules.

@staabm
Copy link
Contributor Author

staabm commented Sep 6, 2023

I don't mind which of the rules gets able to resolve my initial snippet :-) - getting support the mentioned cases would be awesome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants