diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index dac88304..9a168f34 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -101,6 +101,10 @@ // avoid call on nullable object AddInstanceofAssertForNullableInstanceRector::class, + + // enable next after testing + // \Rector\PHPUnit\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableArgumentRector::class, + AssertArrayCastedObjectToAssertSameRector::class, /** diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/AddInstanceofAssertForNullableArgumentRectorTest.php b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/AddInstanceofAssertForNullableArgumentRectorTest.php new file mode 100644 index 00000000..dace023c --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/AddInstanceofAssertForNullableArgumentRectorTest.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/AddInstanceofAssertForNullableArgumentRector/Fixture/passing_nullable_arg.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Fixture/passing_nullable_arg.php.inc new file mode 100644 index 00000000..7d1d9fe4 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Fixture/passing_nullable_arg.php.inc @@ -0,0 +1,68 @@ +getSomeObject(); + $this->process($someObject); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } + + private function process(SomeClassUsedInTests $someObject): void + { + // non nullable required + } +} + +?> +----- +getSomeObject(); + $this->assertInstanceOf(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableArgumentRector\Source\SomeClassUsedInTests::class, $someObject); + $this->process($someObject); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } + + private function process(SomeClassUsedInTests $someObject): void + { + // non nullable required + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Fixture/skip_non_nullable_type.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Fixture/skip_non_nullable_type.php.inc new file mode 100644 index 00000000..ca556600 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Fixture/skip_non_nullable_type.php.inc @@ -0,0 +1,27 @@ +getSomeObject(); + + $this->process($someObject); + } + + private function getSomeObject(): SomeClassUsedInTests + { + return new SomeClassUsedInTests(); + } + + private function process(SomeClassUsedInTests $someObject) + { + } +} diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Source/SomeClassUsedInTests.php b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Source/SomeClassUsedInTests.php new file mode 100644 index 00000000..9455363e --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector/Source/SomeClassUsedInTests.php @@ -0,0 +1,18 @@ +rule(AddInstanceofAssertForNullableArgumentRector::class); +}; diff --git a/rules/CodeQuality/NodeFactory/AssertMethodCallFactory.php b/rules/CodeQuality/NodeFactory/AssertMethodCallFactory.php new file mode 100644 index 00000000..7d054938 --- /dev/null +++ b/rules/CodeQuality/NodeFactory/AssertMethodCallFactory.php @@ -0,0 +1,28 @@ +getObjectType()), 'class')), + new Arg(new Variable($variableNameToType->getVariableName())), + ]; + + $methodCall = new MethodCall(new Variable('this'), 'assertInstanceOf', $args); + + return new Expression($methodCall); + } +} diff --git a/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector.php b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector.php new file mode 100644 index 00000000..0eef1610 --- /dev/null +++ b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableArgumentRector.php @@ -0,0 +1,220 @@ +getSomeObject(); + + $this->process($someObject); + } + + private function getSomeObject(): ?SomeClass + { + if (mt_rand(0, 1)) { + return new SomeClass(); + } + + return null; + } + + private function process(SomeClass $someObject): void + { + // non-nullable use here + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $someObject = $this->getSomeObject(); + $this->assertInstanceOf(SomeClass::class, $someObject); + + $this->process($someObject); + } + + private function getSomeObject(): ?SomeClass + { + if (mt_rand(0, 1)) { + return new SomeClass(); + } + + return null; + } + + private function process(SomeClass $someObject): void + { + // non-nullable use here + } +} +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; + } + + $hasChanged = false; + $variableNameToTypeCollection = $this->nullableObjectAssignCollector->collect($node); + + $next = 0; + foreach ($node->stmts as $key => $stmt) { + // has callable on nullable variable of already collected name? + $matchedNullableVariableNameToType = $this->matchedNullableArgumentNameToType( + $stmt, + $variableNameToTypeCollection + ); + + if (! $matchedNullableVariableNameToType instanceof VariableNameToType) { + continue; + } + + // adding type here + popping the variable name out + $assertInstanceOfExpression = $this->assertMethodCallFactory->createAssertInstanceOf( + $matchedNullableVariableNameToType + ); + array_splice($node->stmts, $key + $next, 0, [$assertInstanceOfExpression]); + + // remove variable name from nullable ones + $hasChanged = true; + + // from now on, the variable is not nullable, remove to avoid double asserts + $variableNameToTypeCollection->remove($matchedNullableVariableNameToType); + + ++$next; + } + + if (! $hasChanged) { + return null; + } + + return $node; + } + + private function matchedNullableArgumentNameToType( + Stmt $stmt, + VariableNameToTypeCollection $variableNameToTypeCollection + ): ?VariableNameToType { + $matchedNullableVariableNameToType = null; + + $this->traverseNodesWithCallable($stmt, function (Node $node) use ( + $variableNameToTypeCollection, + &$matchedNullableVariableNameToType + ): ?int { + if (! $node instanceof MethodCall) { + return null; + } + + if ($node->isFirstClassCallable()) { + return null; + } + + $classMethodParameterTypes = $this->methodCallParameterTypeResolver->resolve($node); + + foreach ($node->getArgs() as $position => $arg) { + if (! $arg->value instanceof Variable) { + continue; + } + + $variableType = $this->getType($arg->value); + if (! SimpleTypeAnalyzer::isNullableType($variableType)) { + return null; + } + + // should not happen + if (! isset($classMethodParameterTypes[$position])) { + return null; + } + + $variableName = $this->getName($arg->value); + if (! is_string($variableName)) { + return null; + } + + $matchedNullableVariableNameToType = $variableNameToTypeCollection->matchByVariableName($variableName); + + // is object type required? + if (! $classMethodParameterTypes[$position] instanceof ObjectType) { + return null; + } + + return NodeVisitor::STOP_TRAVERSAL; + } + + return null; + }); + + return $matchedNullableVariableNameToType; + } +} diff --git a/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php index f24e41c7..ea7473c9 100644 --- a/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php @@ -5,19 +5,15 @@ namespace Rector\PHPUnit\CodeQuality\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Arg; -use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Foreach_; -use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; -use PHPStan\Type\UnionType; +use PhpParser\NodeVisitor; use Rector\PHPUnit\CodeQuality\NodeAnalyser\NullableObjectAssignCollector; +use Rector\PHPUnit\CodeQuality\NodeFactory\AssertMethodCallFactory; +use Rector\PHPUnit\CodeQuality\TypeAnalyzer\SimpleTypeAnalyzer; use Rector\PHPUnit\CodeQuality\ValueObject\VariableNameToType; use Rector\PHPUnit\CodeQuality\ValueObject\VariableNameToTypeCollection; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; @@ -33,6 +29,7 @@ final class AddInstanceofAssertForNullableInstanceRector extends AbstractRector public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly NullableObjectAssignCollector $nullableObjectAssignCollector, + private readonly AssertMethodCallFactory $assertMethodCallFactory, ) { } @@ -130,7 +127,9 @@ public function refactor(Node $node): ?Node } // adding type here + popping the variable name out - $assertInstanceOfExpression = $this->createAssertInstanceOf($matchedNullableVariableNameToType); + $assertInstanceOfExpression = $this->assertMethodCallFactory->createAssertInstanceOf( + $matchedNullableVariableNameToType + ); array_splice($node->stmts, $key + $next, 0, [$assertInstanceOfExpression]); // remove variable name from nullable ones @@ -149,31 +148,6 @@ public function refactor(Node $node): ?Node return $node; } - private function isNullableType(Type $type): bool - { - if (! $type instanceof UnionType) { - return false; - } - - if (! TypeCombinator::containsNull($type)) { - return false; - } - - return count($type->getTypes()) === 2; - } - - private function createAssertInstanceOf(VariableNameToType $variableNameToType): Expression - { - $args = [ - new Arg(new ClassConstFetch(new FullyQualified($variableNameToType->getObjectType()), 'class')), - new Arg(new Variable($variableNameToType->getVariableName())), - ]; - - $methodCall = new MethodCall(new Variable('this'), 'assertInstanceOf', $args); - - return new Expression($methodCall); - } - private function matchedNullableVariableNameToType( Stmt $stmt, VariableNameToTypeCollection $variableNameToTypeCollection @@ -183,7 +157,7 @@ private function matchedNullableVariableNameToType( $this->traverseNodesWithCallable($stmt, function (Node $node) use ( $variableNameToTypeCollection, &$matchedNullableVariableNameToType - ): null { + ): ?int { if (! $node instanceof MethodCall) { return null; } @@ -193,7 +167,7 @@ private function matchedNullableVariableNameToType( } $variableType = $this->getType($node->var); - if (! $this->isNullableType($variableType)) { + if (! SimpleTypeAnalyzer::isNullableType($variableType)) { return null; } @@ -205,7 +179,7 @@ private function matchedNullableVariableNameToType( $matchedNullableVariableNameToType = $variableNameToTypeCollection->matchByVariableName($variableName); // is the variable we're interested in? - return null; + return NodeVisitor::STOP_TRAVERSAL; }); return $matchedNullableVariableNameToType; diff --git a/rules/CodeQuality/TypeAnalyzer/MethodCallParameterTypeResolver.php b/rules/CodeQuality/TypeAnalyzer/MethodCallParameterTypeResolver.php new file mode 100644 index 00000000..667c932a --- /dev/null +++ b/rules/CodeQuality/TypeAnalyzer/MethodCallParameterTypeResolver.php @@ -0,0 +1,43 @@ +getArgs() === []) { + return null; + } + + $methodReflection = $this->reflectionResolver->resolveMethodReflectionFromMethodCall($methodCall); + if (! $methodReflection instanceof MethodReflection) { + return null; + } + + $extendedParametersAcceptor = ParametersAcceptorSelector::combineAcceptors($methodReflection->getVariants()); + + $parameterTypesByPosition = []; + foreach ($extendedParametersAcceptor->getParameters() as $extendedParameterReflection) { + $parameterTypesByPosition[] = $extendedParameterReflection->getType(); + } + + return $parameterTypesByPosition; + } +} diff --git a/rules/CodeQuality/TypeAnalyzer/SimpleTypeAnalyzer.php b/rules/CodeQuality/TypeAnalyzer/SimpleTypeAnalyzer.php new file mode 100644 index 00000000..f8588ee0 --- /dev/null +++ b/rules/CodeQuality/TypeAnalyzer/SimpleTypeAnalyzer.php @@ -0,0 +1,25 @@ +getTypes()) === 2; + } +}