diff --git a/composer.json b/composer.json index ea05b0d6..6587a799 100644 --- a/composer.json +++ b/composer.json @@ -12,11 +12,12 @@ "phpstan/phpstan": "^2.1.8", "phpecs/phpecs": "^2.0", "phpstan/extension-installer": "^1.4", - "symplify/vendor-patches": "^11.3", + "symplify/vendor-patches": "^11.4", "tracy/tracy": "^2.10", "tomasvotruba/class-leak": "^1.2", "rector/swiss-knife": "^1.0", - "phpstan/phpstan-webmozart-assert": "^2.0" + "phpstan/phpstan-webmozart-assert": "^2.0", + "rector/type-perfect": "^2.0" }, "autoload": { "psr-4": { @@ -47,17 +48,15 @@ "rector": "vendor/bin/rector process --ansi" }, "extra": { - "enable-patching": true, - "branch-alias": { - "dev-main": "0.11-dev" - } + "enable-patching": true }, "conflict": { - "rector/rector": "<0.11" + "rector/rector": "<2.0" }, "minimum-stability": "dev", "prefer-stable": true, "config": { + "sort-packages": true, "allow-plugins": { "cweagans/composer-patches": true, "rector/extension-installer": true, diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index 3eea0839..b1c9f15b 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -9,6 +9,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\SingleMockPropertyTypeRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\TestWithToDataProviderRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector; +use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\DataProviderArrayItemsNewLinedRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\RemoveEmptyTestMethodRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\ReplaceTestAnnotationWithPrefixedFunctionRector; @@ -75,6 +76,9 @@ AssertEmptyNullableObjectToAssertInstanceofRector::class, AssertCountWithZeroToAssertEmptyRector::class, + // avoid call on nullable object + AddInstanceofAssertForNullableInstanceRector::class, + /** * Improve direct testing of your code, without mock creep. Make it simple, clear and easy to maintain: * diff --git a/phpstan.neon b/phpstan.neon index 70ebfee0..655b005b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -19,13 +19,12 @@ parameters: - */Fixture/* - */Expected/* -# to be enabled later once rector upgraded to use phpstan v2 -# # see https://github.com/rectorphp/type-perfect/ -# type_perfect: -# no_mixed: true -# null_over_false: true -# narrow_param: true -# narrow_return: true + # see https://github.com/rectorphp/type-perfect/ + type_perfect: + no_mixed: true + null_over_false: true + narrow_param: true + narrow_return: true ignoreErrors: # phpstan false positive diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/AddInstanceofAssertForNullableInstanceRectorTest.php b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/AddInstanceofAssertForNullableInstanceRectorTest.php new file mode 100644 index 00000000..b6f0e61d --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/AddInstanceofAssertForNullableInstanceRectorTest.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/AddInstanceofAssertForNullableInstanceRector/Fixture/add_instance_only_once.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/add_instance_only_once.php.inc new file mode 100644 index 00000000..7922a205 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/add_instance_only_once.php.inc @@ -0,0 +1,68 @@ +getSomeObject(); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + + // we know the value here, no need to add instanceof + $value = $someObject->getSomeMethod(); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } +} + +?> +----- +getSomeObject(); + $this->assertInstanceof(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector\Source\SomeClassUsedInTests::class, $someObject); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + + // we know the value here, no need to add instanceof + $value = $someObject->getSomeMethod(); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/call_on_nullable.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/call_on_nullable.php.inc new file mode 100644 index 00000000..487bb889 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/call_on_nullable.php.inc @@ -0,0 +1,62 @@ +getSomeObject(); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } +} + +?> +----- +getSomeObject(); + $this->assertInstanceof(\Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector\Source\SomeClassUsedInTests::class, $someObject); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + } + + private function getSomeObject(): ?SomeClassUsedInTests + { + if (mt_rand(0, 1)) { + return new SomeClassUsedInTests(); + } + + return null; + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_int_nullabe.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_int_nullabe.php.inc new file mode 100644 index 00000000..40939b09 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_int_nullabe.php.inc @@ -0,0 +1,27 @@ +getValue(); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + } + + private function getValue(): ?int + { + if (mt_rand(0, 1)) { + return 1000; + } + + return null; + } +} diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_non_nullable_type.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_non_nullable_type.php.inc new file mode 100644 index 00000000..7f0ad0f1 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/skip_non_nullable_type.php.inc @@ -0,0 +1,24 @@ +getSomeObject(); + $value = $someObject->getSomeMethod(); + + $this->assertSame(123, $value); + } + + private function getSomeObject(): SomeClassUsedInTests + { + return new SomeClassUsedInTests(); + } +} diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Source/SomeClassUsedInTests.php b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Source/SomeClassUsedInTests.php new file mode 100644 index 00000000..0c1bd934 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Source/SomeClassUsedInTests.php @@ -0,0 +1,13 @@ +rule(AddInstanceofAssertForNullableInstanceRector::class); +}; diff --git a/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/skip_sole_type.php.inc b/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/skip_sole_type.php.inc index a67e6d31..2f76011e 100644 --- a/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/skip_sole_type.php.inc +++ b/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/skip_sole_type.php.inc @@ -6,7 +6,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\SingleMockPropertyTypeRector\Source\AnyBook; -class SkipSoleType extends TestCase +final class SkipSoleType extends TestCase { private MockObject $anyBook; } diff --git a/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/union_type_mock.php.inc b/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/union_type_mock.php.inc index 31c61ad5..60287bf5 100644 --- a/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/union_type_mock.php.inc +++ b/rules-tests/CodeQuality/Rector/Class_/SingleMockPropertyTypeRector/Fixture/union_type_mock.php.inc @@ -6,7 +6,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\SingleMockPropertyTypeRector\Source\AnyBook; -class UnionTypeMock extends TestCase +final class UnionTypeMock extends TestCase { private MockObject|AnyBook $anyBook; } @@ -21,7 +21,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\SingleMockPropertyTypeRector\Source\AnyBook; -class UnionTypeMock extends TestCase +final class UnionTypeMock extends TestCase { private MockObject $anyBook; } diff --git a/rules/CodeQuality/NodeAnalyser/NullableObjectAssignCollector.php b/rules/CodeQuality/NodeAnalyser/NullableObjectAssignCollector.php new file mode 100644 index 00000000..681220e6 --- /dev/null +++ b/rules/CodeQuality/NodeAnalyser/NullableObjectAssignCollector.php @@ -0,0 +1,78 @@ +getSomething(); + */ +final readonly class NullableObjectAssignCollector +{ + public function __construct( + private NodeNameResolver $nodeNameResolver, + private NodeTypeResolver $nodeTypeResolver, + ) { + } + + public function collect(ClassMethod|Foreach_ $stmtsAware): VariableNameToTypeCollection + { + $variableNamesToType = []; + + // first round to collect assigns + foreach ((array) $stmtsAware->stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $variableNameToType = $this->collectFromAssign($stmt->expr); + if (! $variableNameToType instanceof VariableNameToType) { + continue; + } + + $variableNamesToType[] = $variableNameToType; + } + + return new VariableNameToTypeCollection($variableNamesToType); + } + + private function collectFromAssign(Assign $assign): ?VariableNameToType + { + if (! $assign->expr instanceof MethodCall) { + return null; + } + + if (! $assign->var instanceof Variable) { + return null; + } + + $variableType = $this->nodeTypeResolver->getType($assign); + + $bareVariableType = TypeCombinator::removeNull($variableType); + if (! $bareVariableType instanceof ObjectType) { + return null; + } + + $variableName = $this->nodeNameResolver->getName($assign->var); + return new VariableNameToType($variableName, $bareVariableType->getClassName()); + } +} diff --git a/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php new file mode 100644 index 00000000..f4c0bb13 --- /dev/null +++ b/rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php @@ -0,0 +1,207 @@ +getSomeObject(); + + $value = $someObject->getSomeMethod(); + } + + private function getSomeObject(): ?SomeClass + { + if (mt_rand(0, 1)) { + return new SomeClass(); + } + + return null; + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $someObject = $this->getSomeObject(); + $this->assertInstanceof(SomeClass::class, $someObject); + + $value = $someObject->getSomeMethod(); + } + + private function getSomeObject(): ?SomeClass + { + if (mt_rand(0, 1)) { + return new SomeClass(); + } + + return null; + } +} +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 === [] || $node->stmts === null || count($node->stmts) < 2) { + return null; + } + + $hasChanged = false; + $variableNameToTypeCollection = $this->nullableObjectAssignCollector->collect($node); + + foreach ($node->stmts as $key => $stmt) { + // has callable on nullable variable of already collected name? + $matchedNullableVariableNameToType = $this->matchedNullableVariableNameToType( + $stmt, + $variableNameToTypeCollection + ); + if (! $matchedNullableVariableNameToType instanceof VariableNameToType) { + continue; + } + + // adding type here + popping the variable name out + $assertInstanceofExpression = $this->createAssertInstanceof($matchedNullableVariableNameToType); + array_splice($node->stmts, $key, 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); + } + + if (! $hasChanged) { + return null; + } + + 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 + ): ?VariableNameToType { + $matchedNullableVariableNameToType = null; + + $this->traverseNodesWithCallable($stmt, function (Node $node) use ( + $variableNameToTypeCollection, + &$matchedNullableVariableNameToType + ): null { + if (! $node instanceof MethodCall) { + return null; + } + + if (! $node->var instanceof Variable) { + return null; + } + + $variableType = $this->getType($node->var); + if (! $this->isNullableType($variableType)) { + return null; + } + + $matchedNullableVariableNameToType = $variableNameToTypeCollection->matchByVariableName( + $this->getName($node->var) + ); + + // is the variable we're interested in? + return null; + }); + + return $matchedNullableVariableNameToType; + } +} diff --git a/rules/CodeQuality/ValueObject/VariableNameToType.php b/rules/CodeQuality/ValueObject/VariableNameToType.php new file mode 100644 index 00000000..33582056 --- /dev/null +++ b/rules/CodeQuality/ValueObject/VariableNameToType.php @@ -0,0 +1,24 @@ +variableName; + } + + public function getObjectType(): string + { + return $this->objectType; + } +} diff --git a/rules/CodeQuality/ValueObject/VariableNameToTypeCollection.php b/rules/CodeQuality/ValueObject/VariableNameToTypeCollection.php new file mode 100644 index 00000000..baf6da60 --- /dev/null +++ b/rules/CodeQuality/ValueObject/VariableNameToTypeCollection.php @@ -0,0 +1,41 @@ +variableNameToType as $variableNameToType) { + if ($variableNameToType->getVariableName() !== $variableName) { + continue; + } + + return $variableNameToType; + } + + return null; + } + + public function remove(VariableNameToType $matchedNullableVariableNameToType): void + { + foreach ($this->variableNameToType as $key => $variableNamesToType) { + if ($matchedNullableVariableNameToType !== $variableNamesToType) { + continue; + } + + unset($this->variableNameToType[$key]); + break; + } + } +}