Skip to content
/ rector-src Public

[CodeQuality] Skip if else return on ExplicitReturnNullRector#5755

Merged
samsonasik merged 13 commits intomainfrom
skip-if-else
Mar 22, 2024
Merged

[CodeQuality] Skip if else return on ExplicitReturnNullRector#5755
samsonasik merged 13 commits intomainfrom
skip-if-else

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik samsonasik commented Mar 22, 2024

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024

Fixed 🎉

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024

Try catch seems can always return inside If https://getrector.com/demo/25fe6ca5-f447-4ada-83c7-6a9c8d4bf692

We may need to use existing SilentVoidResolver for it.

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024 •
edited
Loading

I added another fixture in if else with try catch fixture 67e848e

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024

The logic for it should like in SilentVoidResolver

rector-src/rules/TypeDeclaration/TypeInferer/SilentVoidResolver.php

Lines 66 to 78 in b4eb883

// has switch with always return
if ($stmt instanceof Switch_ && $this->isSwitchWithAlwaysReturn($stmt)) {
return false;
}
// is part of try/catch
if ($stmt instanceof TryCatch && $this->isTryCatchAlwaysReturn($stmt)) {
return false;
}
if ($stmt instanceof Throw_) {
return false;
}

the solution is either make that detection there, with if else check addition, and utilize ReturnTypeInferer to verify final type has Void type or check here more.

I will look more...

@samsonasik samsonasik self-assigned this Mar 22, 2024
@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024

implemented 🎉 , I moved the functionality to SilentVoidResolver::hasSilentVoid() so it centralized for future improvement 👍

@samsonasik
Copy link
Copy Markdown
Member Author

samsonasik commented Mar 22, 2024

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 32bf5c7 into main Mar 22, 2024
@samsonasik samsonasik deleted the skip-if-else branch March 22, 2024 16:51
@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Mar 22, 2024

Thank you 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@samsonasik samsonasik

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@samsonasik @TomasVotruba @actions-user