Skip to content

Commit

Permalink
[CodeQuality] Add NarrowSingleWillReturnCallbackRector (#374)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba authored Sep 29, 2024
1 parent daac9f0 commit ea5a3c1
Show file tree
Hide file tree
Showing 8 changed files with 347 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertCompareOnCountableWithMethodToAssertCountRector\Fixture;

use Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\AssertCompareOnCountableWithMethodToAssertCountRector\Source\Collection;

final class Count extends \PHPUnit\Framework\TestCase
{
public function test()
{
$collection = new Collection();
$this->assertSame(5, $collection->count());
$this->assertEquals(5, $collection->count());
\PHPUnit\Framework\TestCase::assertSame(5, $collection->count());
self::assertSame(5, $collection->count());
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\Fixture;

use PHPUnit\Framework\TestCase;

final class SingleMatch extends TestCase
{
public function test()
{
$matcher = $this->exactly(3);

$someServiceMock = $this->createMock(SomeMockedClass::class);
$someServiceMock->expects($matcher)
->method('prepare')
->willReturnCallback(function (...$parameters) use ($matcher) {
match ($matcher->getInvocationCount()) {
1 => $this->assertSame([1], $parameters),
};
});
}
}

?>
-----
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\Fixture;

use PHPUnit\Framework\TestCase;

final class SingleMatch extends TestCase
{
public function test()
{
$matcher = $this->exactly(3);

$someServiceMock = $this->createMock(SomeMockedClass::class);
$someServiceMock->expects($matcher)
->method('prepare')
->with([1]);
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\Fixture;

use PHPUnit\Framework\TestCase;

final class SkipMultiValue extends TestCase
{
public function test()
{
$matcher = $this->exactly(3);

$someServiceMock = $this->createMock(SomeMockedClass::class);
$someServiceMock->expects($matcher)
->method('prepare')
->willReturnCallback(function (...$parameters) use ($matcher) {
match ($matcher->getInvocationCount()) {
1 => $this->assertSame([1], $parameters),
2 => $this->assertSame([1], $parameters),
};
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\Fixture;

use PHPUnit\Framework\TestCase;

final class SkipNonOne extends TestCase
{
public function test()
{
$matcher = $this->exactly(3);

$someServiceMock = $this->createMock(SomeMockedClass::class);
$someServiceMock->expects($matcher)
->method('prepare')
->willReturnCallback(function (...$parameters) use ($matcher) {
match ($matcher->getInvocationCount()) {
2 => $this->assertSame([1], $parameters),
};
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class NarrowSingleWillReturnCallbackRectorTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\Source;

class SomeMockedClass
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(NarrowSingleWillReturnCallbackRector::class);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
<?php

declare(strict_types=1);

namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\Match_;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Stmt\Expression;
use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\NarrowSingleWillReturnCallbackRector\NarrowSingleWillReturnCallbackRectorTest
*/
final class NarrowSingleWillReturnCallbackRector extends AbstractRector
{
public function __construct(
private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Narrow single-value match willReturnCallback() to with() and willReturn() call',
[
new CodeSample(
<<<'CODE_SAMPLE'
use PHPUnit\Framework\TestCase;
final class SomeTest extends TestCase
{
public function run()
{
$matcher = $this->exactly(1);
$this->personServiceMock->expects($matcher)
->willReturnCallback(function (...$parameters) use ($matcher) {
match ($matcher->getInvocationCount()) {
1 => $this->assertSame([1], $parameters),
};
});
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
use PHPUnit\Framework\TestCase;
final class SomeTest extends TestCase
{
public function run()
{
$matcher = $this->exactly(1);
$this->personServiceMock->expects($matcher)
->with([1], $parameters);
}
}
CODE_SAMPLE
),
]
);
}

/**
* @return array<class-string<MethodCall>>
*/
public function getNodeTypes(): array
{
return [MethodCall::class];
}

/**
* @param MethodCall $node
*/
public function refactor(Node $node): MethodCall|null
{
if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
return null;
}

if (! $this->isName($node->name, 'willReturnCallback')) {
return null;
}

if ($node->isFirstClassCallable()) {
return null;
}

if (count($node->getArgs()) !== 1) {
return null;
}

$firstArg = $node->getArgs()[0];

$match = $this->matchClosureSingleStmtMatch($firstArg->value);
if (! $match instanceof Match_) {
return null;
}

$matchArmBody = $this->matchSingleMatchArmBodyWithConditionOne($match);
if (! $matchArmBody instanceof MethodCall) {
return null;
}

// we look for $this->assertSame(...)
if (! $this->isLocalMethodCall($matchArmBody, 'assertSame')) {
return null;
}

$expectedArg = $matchArmBody->getArgs()[0];

$node->name = new Identifier('with');
$node->args = [new Arg($expectedArg->value)];

return $node;
}

private function matchClosureSingleStmtMatch(Expr $expr): null|Match_
{
if (! $expr instanceof Closure) {
return null;
}

// handle easy path of single stmt first
if (count($expr->stmts) !== 1) {
return null;
}

$onlyStmts = $expr->stmts[0];
if (! $onlyStmts instanceof Expression) {
return null;
}

if (! $onlyStmts->expr instanceof Match_) {
return null;
}

return $onlyStmts->expr;
}

private function isLocalMethodCall(Expr $expr, string $methodName): bool
{
if (! $expr instanceof MethodCall) {
return false;
}

if (! $this->isName($expr->var, 'this')) {
return false;
}

return $this->isName($expr->name, $methodName);
}

private function matchSingleMatchArmBodyWithConditionOne(Match_ $match): ?Expr
{
// has more options
if (count($match->arms) !== 1) {
return null;
}

$onlyArm = $match->arms[0];
if ($onlyArm->conds === null || count($onlyArm->conds) !== 1) {
return null;
}

$onlyArmCond = $onlyArm->conds[0];
if (! $this->isNumberOne($onlyArmCond)) {
return null;
}

return $onlyArm->body;
}

private function isNumberOne(Expr $expr): bool
{
if (! $expr instanceof LNumber) {
return false;
}

return $expr->value === 1;
}
}

0 comments on commit ea5a3c1

Please sign in to comment.