search

rectorphp / rector-phpunit

Rector upgrade rules for PHPUnit
http://getrector.com
MIT License
61 stars 46 forks source link

Incorrect implementation of WithConsecutiveRector #311

Closed f-albert closed 2 months ago

f-albert commented 4 months ago

The implementation of WithConsecutiveRector seems to be incorrect. Before: ->withConsecutive( [1, 2], [3, 4] ); After: ->willReturnCallback(function () use ($matcher) { return match ($matcher->numberOfInvocations()) { 1 => [1, 2], 2 => [3, 4] };

The function withConsecutive checks if a function is called with specific parameter. The replacement function willReturnCallback only gets a function which determines the return value but doesn't checks the parameter. Furthermore the function returns the parameter.

I will create a PR with a (hopefully correct) solution.

TomasVotruba commented 4 months ago

There is also option to use willReturnMap() that handles some simpler cases.

f-albert commented 4 months ago

There is also option to use willReturnMap() that handles some simpler cases.

I don't think so: withConsecutive checks if the parameter of a call matches the expected ones. If one parameter is wrong, the test will fail. willReturnMap only chooses the right return value based on the parameters. if no parameter list matches the call parameters, the return value will be null

f-albert commented 3 months ago

Is it possible to remove this rule first so that other projects do not have the same problem? The PR still needs some time.

samsonasik commented 2 months ago

implemented at https://github.com/rectorphp/rector-phpunit/pull/313