From f7813ed82595563da4fa34e48923dcf85efbc7e7 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 26 Jan 2026 14:14:08 +0100 Subject: [PATCH] [code-quality] Add BareCreateMockAssignToDirectUseRector --- config/sets/phpunit-code-quality.php | 2 + ...eCreateMockAssignToDirectUseRectorTest.php | 28 +++ .../multiple_uses_as_no_connection.php.inc | 51 ++++ .../Fixture/skip_used_as_mock.php.inc | 22 ++ .../Fixture/some_class.php.inc | 41 +++ .../Source/AnotherClass.php | 9 + .../config/configured_rule.php | 9 + .../Fixture/skip_comment_inline.php.inc | 4 +- .../Fixture/skip_no_array.php.inc | 2 +- .../Fixture/skip_non_phpunit.php.inc | 2 +- .../Fixture/skip_test_method.php.inc | 2 +- .../Fixture/some_class.php.inc | 4 +- .../NodeAnalyser/AssignedMocksCollector.php | 123 +++++++++ .../DoctrineEntityDocumentAnalyser.php | 15 +- .../CodeQuality/NodeFinder/VariableFinder.php | 32 +++ .../BareCreateMockAssignToDirectUseRector.php | 238 ++++++++++++++++++ ...ityDocumentCreateMockToDirectNewRector.php | 121 ++++----- 17 files changed, 624 insertions(+), 81 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/BareCreateMockAssignToDirectUseRectorTest.php create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/multiple_uses_as_no_connection.php.inc create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/skip_used_as_mock.php.inc create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/some_class.php.inc create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Source/AnotherClass.php create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/config/configured_rule.php create mode 100644 rules/CodeQuality/NodeAnalyser/AssignedMocksCollector.php create mode 100644 rules/CodeQuality/NodeFinder/VariableFinder.php create mode 100644 rules/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector.php diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index d9fd8c33..1be3f326 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -14,6 +14,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\TypeWillReturnCallableArrowFunctionRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector; +use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\DataProviderArrayItemsNewLinedRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\EntityDocumentCreateMockToDirectNewRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\RemoveEmptyTestMethodRector; @@ -132,5 +133,6 @@ GetMockBuilderGetMockToCreateMockRector::class, EntityDocumentCreateMockToDirectNewRector::class, ReplaceAtMethodWithDesiredMatcherRector::class, + BareCreateMockAssignToDirectUseRector::class, ]); }; diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/BareCreateMockAssignToDirectUseRectorTest.php b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/BareCreateMockAssignToDirectUseRectorTest.php new file mode 100644 index 00000000..86b1abfd --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/BareCreateMockAssignToDirectUseRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/multiple_uses_as_no_connection.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/multiple_uses_as_no_connection.php.inc new file mode 100644 index 00000000..2734ca02 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/multiple_uses_as_no_connection.php.inc @@ -0,0 +1,51 @@ +createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class); + + $this->useMock($someMock); + $this->useMockAgain($someMock); + } + + private function useMock($someMock) + { + } + + private function useMockAgain($someMock) + { + } +} + +?> +----- +useMock($this->createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class)); + $this->useMockAgain($this->createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class)); + } + + private function useMock($someMock) + { + } + + private function useMockAgain($someMock) + { + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/skip_used_as_mock.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/skip_used_as_mock.php.inc new file mode 100644 index 00000000..f516808e --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/skip_used_as_mock.php.inc @@ -0,0 +1,22 @@ +createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class); + + $someMock->expects($this->atLeastOnce()) + ->method('some'); + + $this->useMock($someMock); + } + + private function useMock($someMock) + { + } +} diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/some_class.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/some_class.php.inc new file mode 100644 index 00000000..ac3a6016 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Fixture/some_class.php.inc @@ -0,0 +1,41 @@ +createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class); + + $this->useMock($someMock); + } + + private function useMock($someMock) + { + } +} + +?> +----- +useMock($this->createMock(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector\Source\AnotherClass::class)); + } + + private function useMock($someMock) + { + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Source/AnotherClass.php b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Source/AnotherClass.php new file mode 100644 index 00000000..ae754107 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector/Source/AnotherClass.php @@ -0,0 +1,9 @@ +withRules([BareCreateMockAssignToDirectUseRector::class]); diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/DataProviderArrayItemsNewLinedRector/Fixture/skip_comment_inline.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/DataProviderArrayItemsNewLinedRector/Fixture/skip_comment_inline.php.inc index 5edd93dd..0f2ae9b4 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/DataProviderArrayItemsNewLinedRector/Fixture/skip_comment_inline.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/DataProviderArrayItemsNewLinedRector/Fixture/skip_comment_inline.php.inc @@ -1,6 +1,6 @@ + */ + public function collect(ClassMethod|Foreach_ $stmtsAware): array + { + if ($stmtsAware->stmts === null) { + return []; + } + + $mockedVariablesToTypes = []; + + foreach ($stmtsAware->stmts as $stmt) { + // find assign mock + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + + $firstArg = $this->matchCreateMockArgAssignedToVariable($assign); + if (! $firstArg instanceof Arg) { + continue; + } + + $mockedClass = $this->valueResolver->getValue($firstArg->value); + if (! is_string($mockedClass)) { + continue; + } + + if (! $this->reflectionProvider->hasClass($mockedClass)) { + continue; + } + + $mockClassReflection = $this->reflectionProvider->getClass($mockedClass); + // these cannot be instantiated + if ($mockClassReflection->isAbstract()) { + continue; + } + + if ($mockClassReflection->isInterface()) { + continue; + } + + $mockedVariableName = $this->nodeNameResolver->getName($assign->var); + $mockedVariablesToTypes[$mockedVariableName] = $mockedClass; + } + + return $mockedVariablesToTypes; + } + + /** + * @return array + */ + public function collectEntityClasses(ClassMethod $classMethod): array + { + $variableNamesToClassNames = $this->collect($classMethod); + + $variableNamesToEntityClasses = []; + + foreach ($variableNamesToClassNames as $variableName => $className) { + if (! $this->doctrineEntityDocumentAnalyser->isEntityClass($className)) { + continue; + } + + $variableNamesToEntityClasses[$variableName] = $className; + } + + return $variableNamesToEntityClasses; + } + + public function matchCreateMockArgAssignedToVariable(Assign $assign): ?Arg + { + if (! $assign->var instanceof Variable) { + return null; + } + + if (! $assign->expr instanceof MethodCall) { + return null; + } + + $methodCall = $assign->expr; + if (! $this->nodeNameResolver->isName($methodCall->name, 'createMock')) { + return null; + } + + if ($methodCall->isFirstClassCallable()) { + return null; + } + + return $methodCall->getArgs()[0]; + } +} diff --git a/rules/CodeQuality/NodeAnalyser/DoctrineEntityDocumentAnalyser.php b/rules/CodeQuality/NodeAnalyser/DoctrineEntityDocumentAnalyser.php index 0c9b37ad..2da29616 100644 --- a/rules/CodeQuality/NodeAnalyser/DoctrineEntityDocumentAnalyser.php +++ b/rules/CodeQuality/NodeAnalyser/DoctrineEntityDocumentAnalyser.php @@ -5,7 +5,7 @@ namespace Rector\PHPUnit\CodeQuality\NodeAnalyser; use PHPStan\PhpDoc\ResolvedPhpDocBlock; -use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; final readonly class DoctrineEntityDocumentAnalyser { @@ -14,8 +14,19 @@ */ private const array ENTITY_DOCBLOCK_MARKERS = ['@Document', '@ORM\\Document', '@Entity', '@ORM\\Entity']; - public function isEntityClass(ClassReflection $classReflection): bool + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + + public function isEntityClass(string $className): bool { + if (! $this->reflectionProvider->hasClass($className)) { + return false; + } + + $classReflection = $this->reflectionProvider->getClass($className); + $resolvedPhpDocBlock = $classReflection->getResolvedPhpDoc(); if (! $resolvedPhpDocBlock instanceof ResolvedPhpDocBlock) { return false; diff --git a/rules/CodeQuality/NodeFinder/VariableFinder.php b/rules/CodeQuality/NodeFinder/VariableFinder.php new file mode 100644 index 00000000..01d038c2 --- /dev/null +++ b/rules/CodeQuality/NodeFinder/VariableFinder.php @@ -0,0 +1,32 @@ +betterNodeFinder->findInstancesOfScoped([$node], Variable::class); + + return array_filter( + $variables, + fn (Variable $variable): bool => $this->nodeNameResolver->isName($variable, $variableName) + ); + } +} diff --git a/rules/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector.php b/rules/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector.php new file mode 100644 index 00000000..67cae9a8 --- /dev/null +++ b/rules/CodeQuality/Rector/ClassMethod/BareCreateMockAssignToDirectUseRector.php @@ -0,0 +1,238 @@ +createMock(SomeClass::class); + $this->process($someObject); + } + + + private function process(SomeClass $someObject): void + { + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $this->process($this->createMock(SomeClass::class)); + } + + private function process(SomeClass $someObject): void + { + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class, Foreach_::class]; + } + + /** + * @param ClassMethod|Foreach_ $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } + + if ($node->stmts === null || count($node->stmts) < 2) { + return null; + } + + $mockedClassesToVariableNames = $this->assignedMocksCollector->collect($node); + if ($mockedClassesToVariableNames === []) { + return null; + } + + $hasChanged = false; + + foreach (array_keys($mockedClassesToVariableNames) as $variableName) { + // variable cannot be part of any method call + if ($this->isVariableUsedAsPartOfMethodCall($node, $variableName)) { + continue; + } + + if ($this->isUsedMoreOftenThanInCallLikeArgs($node, $variableName)) { + continue; + } + + // 1. remove initial assign + $variablesToMethodCalls = []; + + foreach ($node->stmts as $key => $stmt) { + if ($stmt instanceof Expression && $stmt->expr instanceof Assign) { + $assign = $stmt->expr; + + $instanceArg = $this->assignedMocksCollector->matchCreateMockArgAssignedToVariable($assign); + if (! $instanceArg instanceof Arg) { + continue; + } + + if (! $assign->var instanceof Variable) { + continue; + } + + if (! $this->isName($assign->var, $variableName)) { + continue; + } + + // 1. remove assign + unset($node->stmts[$key]); + $hasChanged = true; + + $variablesToMethodCalls[$variableName] = $assign->expr; + continue; + } + + // 2. replace variable with call-like args of new instance + /** @var CallLike[] $callLikes */ + $callLikes = $this->findCallLikes($stmt); + + foreach ($callLikes as $callLike) { + foreach ($callLike->getArgs() as $arg) { + if (! $arg->value instanceof Variable) { + continue; + } + + if (! $this->isName($arg->value, $variableName)) { + continue; + } + + if (! isset($variablesToMethodCalls[$variableName])) { + continue; + } + + // 2. replace variable with call-like args + $arg->value = $variablesToMethodCalls[$variableName]; + } + } + } + } + + if (! $hasChanged) { + return null; + } + + return $node; + } + + private function isVariableUsedAsPartOfMethodCall(ClassMethod|Foreach_ $stmtsAware, string $variableName): bool + { + /** @var MethodCall[] $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped([$stmtsAware], [MethodCall::class]); + + foreach ($methodCalls as $methodCall) { + if ($this->isName($methodCall->var, $variableName)) { + return true; + } + } + + return false; + } + + private function isUsedMoreOftenThanInCallLikeArgs(ClassMethod|Foreach_ $stmtsAware, string $variableName): bool + { + // get use count + $foundVariables = $this->variableFinder->find($stmtsAware, $variableName); + + // found method call, static call or new arg-only usage + $callLikeVariableUseCount = 0; + + /** @var CallLike[] $callLikes */ + $callLikes = $this->findCallLikes($stmtsAware); + + foreach ($callLikes as $callLike) { + foreach ($callLike->getArgs() as $arg) { + if (! $arg->value instanceof Variable) { + continue; + } + + if (! $this->isName($arg->value, $variableName)) { + continue; + } + + ++$callLikeVariableUseCount; + } + } + + // not suitable for direct replacing + return (count($foundVariables) - 1) > ($callLikeVariableUseCount); + } + + /** + * @return CallLike[] + */ + private function findCallLikes(ClassMethod|Foreach_|Stmt $node): array + { + $callLikes = $this->betterNodeFinder->findInstancesOfScoped( + [$node], + [MethodCall::class, StaticCall::class, New_::class] + ); + + return array_filter($callLikes, fn (CallLike $callLike): bool => ! $callLike->isFirstClassCallable()); + } +} diff --git a/rules/CodeQuality/Rector/ClassMethod/EntityDocumentCreateMockToDirectNewRector.php b/rules/CodeQuality/Rector/ClassMethod/EntityDocumentCreateMockToDirectNewRector.php index cb728bfa..67fc5a9f 100644 --- a/rules/CodeQuality/Rector/ClassMethod/EntityDocumentCreateMockToDirectNewRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/EntityDocumentCreateMockToDirectNewRector.php @@ -19,8 +19,7 @@ use PhpParser\NodeFinder; use PHPStan\Reflection\ReflectionProvider; use Rector\Exception\ShouldNotHappenException; -use Rector\PhpParser\Node\Value\ValueResolver; -use Rector\PHPUnit\CodeQuality\NodeAnalyser\DoctrineEntityDocumentAnalyser; +use Rector\PHPUnit\CodeQuality\NodeAnalyser\AssignedMocksCollector; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -31,9 +30,8 @@ final class EntityDocumentCreateMockToDirectNewRector extends AbstractRector { public function __construct( - private readonly ValueResolver $valueResolver, private readonly ReflectionProvider $reflectionProvider, - private readonly DoctrineEntityDocumentAnalyser $doctrineEntityDocumentAnalyser, + private readonly AssignedMocksCollector $assignedMocksCollector, ) { } @@ -90,12 +88,12 @@ public function refactor(Node $node): ?ClassMethod return null; } - $mockedVariablesToTypes = $this->collectMockedVariableToTypeAndRefactorAssign($node); - if ($mockedVariablesToTypes === []) { + $mockedVariablesToEntityClassNames = $this->assignedMocksCollector->collectEntityClasses($node); + if ($mockedVariablesToEntityClassNames === []) { return null; } - foreach ($mockedVariablesToTypes as $mockedVariableName => $mockedClass) { + foreach ($mockedVariablesToEntityClassNames as $mockedVariableName => $mockedClass) { foreach ($node->stmts as $key => $stmt) { if (! $stmt instanceof Expression) { continue; @@ -133,11 +131,12 @@ public function refactor(Node $node): ?ClassMethod ]); } } - } + $this->replaceCreateMockWithDirectNew($node, $mockedVariablesToEntityClassNames); + // 3. replace value without "mock" in name - $mockedVariableNames = array_keys($mockedVariablesToTypes); + $mockedVariableNames = array_keys($mockedVariablesToEntityClassNames); $this->traverseNodesWithCallable($node, function (Node $node) use ($mockedVariableNames): ?Variable { if (! $node instanceof Variable) { @@ -158,69 +157,6 @@ public function refactor(Node $node): ?ClassMethod return $node; } - /** - * @return array - */ - private function collectMockedVariableToTypeAndRefactorAssign(ClassMethod $classMethod): array - { - if ($classMethod->stmts === null) { - return []; - } - - $mockedVariablesToTypes = []; - - foreach ($classMethod->stmts as $stmt) { - // find assign mock - if (! $stmt instanceof Expression) { - continue; - } - - $stmtExpr = $stmt->expr; - if (! $stmtExpr instanceof Assign) { - continue; - } - - /** @var Assign $assign */ - $assign = $stmtExpr; - if (! $assign->expr instanceof MethodCall) { - continue; - } - - $methodCall = $assign->expr; - if (! $this->isName($methodCall->name, 'createMock')) { - continue; - } - - $firstArg = $methodCall->getArgs()[0]; - - $mockedClass = $this->valueResolver->getValue($firstArg->value); - if (! is_string($mockedClass)) { - continue; - } - - if (! $this->reflectionProvider->hasClass($mockedClass)) { - continue; - } - - $mockClassReflection = $this->reflectionProvider->getClass($mockedClass); - if ($mockClassReflection->isAbstract()) { - continue; - } - - if (! $this->doctrineEntityDocumentAnalyser->isEntityClass($mockClassReflection)) { - continue; - } - - // ready to replace :) - $assign->expr = new New_(new FullyQualified($mockedClass)); - - $mockedVariableName = $this->getName($assign->var); - $mockedVariablesToTypes[$mockedVariableName] = $mockedClass; - } - - return $mockedVariablesToTypes; - } - private function resolveMethodCallFirstArgValue(MethodCall $methodCall, string $methodName): string|Expr|null { $nodeFinder = new NodeFinder(); @@ -290,4 +226,45 @@ private function findMethodCallOnVariableNamed(MethodCall $methodCall, string $d return $foundMethodCall; } + + /** + * @param array $mockedVariablesToTypes + */ + private function replaceCreateMockWithDirectNew(ClassMethod $classMethod, array $mockedVariablesToTypes): void + { + $mockedVariableNames = array_keys($mockedVariablesToTypes); + + // replace mock assigns with direct new + foreach ((array) $classMethod->stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + if (! $assign->expr instanceof MethodCall) { + continue; + } + + if (! $this->isName($assign->expr->name, 'createMock')) { + continue; + } + + if (! $assign->var instanceof Variable) { + continue; + } + + if (! $this->isNames($assign->var, $mockedVariableNames)) { + continue; + } + + $mockedVariableName = $this->getName($assign->var); + + $mockedClassName = $mockedVariablesToTypes[$mockedVariableName]; + $assign->expr = new New_(new FullyQualified($mockedClassName)); + } + } }