[CodeQuality] Handle return throw and return null in void mock callbacks#713
Merged
Conversation
…VoidMethodMockCallbackRector void callbacks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a mocked void method's
willReturnCallback()closure mixesreturn null;with areturn throw new X;,RemoveReturnFromVoidMethodMockCallbackRectorbailed out (areturn throwis not a "pure" value), so it stripped nothing and typed nothing.TypeWillReturnCallableArrowFunctionRectorthen added: voidon its own, leaving the value returns in place — producing code that fatals:Note
return throw new X;is also illegal in avoidfunction, so stripping onlyreturn null;is not enough — thereturnmust be dropped from the throw too.This makes the rule normalize the whole void closure instead of bailing:
return null;(and other pure returns) →return;return throw new X;→throw new X;: voidImpure, non-throw value returns still bail safely (behavior unchanged).
Before → After
->method('persist') - ->willReturnCallback(function ($entity) use ($matcher) { + ->willReturnCallback(function ($entity) use ($matcher): void { if (1 === $matcher->numberOfInvocations()) { - return null; + return; } if (2 === $matcher->numberOfInvocations()) { - return throw new \RuntimeException('boom'); + throw new \RuntimeException('boom'); } });