diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e57a11e --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +/vendor/ +/.php-cs-fixer.cache +/.phpunit.result.cache +/composer.lock diff --git a/extension-mocks.neon b/extension-mocks.neon new file mode 100644 index 0000000..a172105 --- /dev/null +++ b/extension-mocks.neon @@ -0,0 +1,3 @@ +rules: + - Ibexa\PHPStan\Rules\RequireMockObjectInPropertyTypeRule + - Ibexa\PHPStan\Rules\RequireConcreteTypeForMockReturnRule diff --git a/phpstan.neon b/phpstan.neon index ab3d432..c6a905c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,4 +3,6 @@ parameters: paths: - rules - tests + excludePaths: + - tests/rules/Fixtures/* checkMissingCallableSignature: true diff --git a/rules/NoConfigResolverParametersInConstructorRule.php b/rules/NoConfigResolverParametersInConstructorRule.php index 6bd3213..94fa6d8 100644 --- a/rules/NoConfigResolverParametersInConstructorRule.php +++ b/rules/NoConfigResolverParametersInConstructorRule.php @@ -60,7 +60,7 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder ::message('Referring to ConfigResolver parameters in constructor is not allowed due to potential scope change.') - ->identifier('Ibexa.NoConfigResolverParametersInConstructor') + ->identifier('Ibexa.noConfigResolverParametersInConstructor') ->nonIgnorable() ->build(), ]; diff --git a/rules/RequireClosureReturnTypeRule.php b/rules/RequireClosureReturnTypeRule.php index 5a9975e..ace2ca5 100644 --- a/rules/RequireClosureReturnTypeRule.php +++ b/rules/RequireClosureReturnTypeRule.php @@ -23,6 +23,9 @@ public function getNodeType(): string return Node\Expr::class; } + /** + * @return list<\PHPStan\Rules\IdentifierRuleError> + */ public function processNode(Node $node, Scope $scope): array { if (!$node instanceof Node\Expr\Closure && !$node instanceof Node\Expr\ArrowFunction) { @@ -35,7 +38,7 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message( sprintf('%s is missing a return type declaration', $nodeType) - )->build(), + )->identifier('Ibexa.requireClosureReturnType')->build(), ]; } diff --git a/rules/RequireConcreteTypeForMockReturnRule.php b/rules/RequireConcreteTypeForMockReturnRule.php new file mode 100644 index 0000000..30332a0 --- /dev/null +++ b/rules/RequireConcreteTypeForMockReturnRule.php @@ -0,0 +1,170 @@ + + */ +final readonly class RequireConcreteTypeForMockReturnRule implements Rule +{ + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * @return list<\PHPStan\Rules\IdentifierRuleError> + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->returnType === null || $node->stmts === null) { + return []; + } + + if (!$this->returnsMock($node)) { + return []; + } + + if (!$this->typeNodeIsMockObjectOnly($node->returnType)) { + return []; + } + + return [ + RuleErrorBuilder::message('Method returns a mock and declares only MockObject as return type. Use an intersection with a concrete type.') + ->identifier('Ibexa.requireConcreteTypeForMockReturn') + ->build(), + ]; + } + + private function returnsMock(ClassMethod $node): bool + { + $mockVariables = []; + foreach ($node->getStmts() ?? [] as $stmt) { + if ($stmt instanceof Node\Stmt\Expression && $stmt->expr instanceof Node\Expr\Assign) { + $assign = $stmt->expr; + if ($assign->var instanceof Variable && is_string($assign->var->name)) { + if ($assign->expr instanceof MethodCall && $this->isCreateMockCall($assign->expr)) { + $mockVariables[$assign->var->name] = true; + } + + if ($assign->expr instanceof StaticCall && $this->isCreateMockCall($assign->expr)) { + $mockVariables[$assign->var->name] = true; + } + } + } + + if (!$stmt instanceof Node\Stmt\Return_ || $stmt->expr === null) { + continue; + } + + $expr = $stmt->expr; + if ($expr instanceof MethodCall && $this->isCreateMockCall($expr)) { + return true; + } + + if ($expr instanceof StaticCall && $this->isCreateMockCall($expr)) { + return true; + } + + if ($expr instanceof Variable && is_string($expr->name) && isset($mockVariables[$expr->name])) { + return true; + } + } + + return false; + } + + /** + * @param \PhpParser\Node\Expr\MethodCall|\PhpParser\Node\Expr\StaticCall $call + */ + private function isCreateMockCall(Node $call): bool + { + if (!$call->name instanceof Node\Identifier) { + return false; + } + + if ($call->name->toString() !== 'createMock') { + return false; + } + + if ($call instanceof MethodCall) { + return $call->var instanceof Variable && $call->var->name === 'this'; + } + + return true; + } + + private function typeNodeIsMockObjectOnly(Node $type): bool + { + if ($type instanceof NullableType) { + return $this->typeNodeIsMockObjectOnly($type->type); + } + + if ($type instanceof IntersectionType) { + $hasMockObject = false; + foreach ($type->types as $innerType) { + if ($this->isMockObjectType($innerType)) { + $hasMockObject = true; + continue; + } + + return false; + } + + return $hasMockObject; + } + + if ($type instanceof UnionType) { + $hasMockObject = false; + foreach ($type->types as $innerType) { + if ($innerType instanceof Name && $innerType->getLast() === 'null') { + continue; + } + + if ($this->isMockObjectType($innerType)) { + $hasMockObject = true; + continue; + } + + return false; + } + + return $hasMockObject; + } + + return $this->isMockObjectType($type); + } + + private function isMockObjectType(Node $type): bool + { + if ($type instanceof Identifier) { + return $type->toString() === 'MockObject'; + } + + if ($type instanceof Name) { + return $type->getLast() === 'MockObject'; + } + + return false; + } +} diff --git a/rules/RequireMockObjectInPropertyTypeRule.php b/rules/RequireMockObjectInPropertyTypeRule.php new file mode 100644 index 0000000..15d7fd9 --- /dev/null +++ b/rules/RequireMockObjectInPropertyTypeRule.php @@ -0,0 +1,92 @@ + + */ +final readonly class RequireMockObjectInPropertyTypeRule implements Rule +{ + public function getNodeType(): string + { + return Property::class; + } + + /** + * @return list<\PHPStan\Rules\IdentifierRuleError> + */ + public function processNode(Node $node, Scope $scope): array + { + if ($node->type === null) { + return []; + } + + if (!$this->docCommentIncludesMockObject($node)) { + return []; + } + + if ($this->typeNodeIncludesMockObject($node->type)) { + return []; + } + + return [ + RuleErrorBuilder::message('Property typed as MockObject only in PHPDoc. Use intersection type with MockObject.') + ->identifier('Ibexa.requireMockObjectPropertyType') + ->build(), + ]; + } + + private function typeNodeIncludesMockObject(Node $type): bool + { + if ($type instanceof NullableType) { + return $this->typeNodeIncludesMockObject($type->type); + } + + if ($type instanceof UnionType || $type instanceof IntersectionType) { + foreach ($type->types as $innerType) { + if ($this->typeNodeIncludesMockObject($innerType)) { + return true; + } + } + + return false; + } + + if ($type instanceof Identifier) { + return $type->toString() === 'MockObject'; + } + + if ($type instanceof Name) { + return $type->getLast() === 'MockObject'; + } + + return false; + } + + private function docCommentIncludesMockObject(Property $property): bool + { + $docComment = $property->getDocComment(); + if ($docComment === null) { + return false; + } + + return preg_match('/@var\\s+[^\\n]*MockObject/', $docComment->getText()) === 1; + } +} diff --git a/tests/rules/Fixtures/RequireConcreteTypeForMockReturnFixture.php b/tests/rules/Fixtures/RequireConcreteTypeForMockReturnFixture.php new file mode 100644 index 0000000..784402b --- /dev/null +++ b/tests/rules/Fixtures/RequireConcreteTypeForMockReturnFixture.php @@ -0,0 +1,39 @@ +createMock(Foo::class); + + return $foo; + } + + private function createFooOk(): Foo&MockObject + { + return $this->createMock(Foo::class); + } + + private function createMockObjectOnly(): MockObject + { + return $this->createMock(Foo::class); + } +} + +final class Foo +{ +} + +interface MockObject +{ +} diff --git a/tests/rules/Fixtures/RequireMockObjectInPropertyTypeFixture.php b/tests/rules/Fixtures/RequireMockObjectInPropertyTypeFixture.php new file mode 100644 index 0000000..3708f8b --- /dev/null +++ b/tests/rules/Fixtures/RequireMockObjectInPropertyTypeFixture.php @@ -0,0 +1,25 @@ + + */ +final class RequireConcreteTypeForMockReturnRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new RequireConcreteTypeForMockReturnRule(); + } + + public function testRule(): void + { + $this->analyse( + [__DIR__ . '/Fixtures/RequireConcreteTypeForMockReturnFixture.php'], + [ + [ + 'Method returns a mock and declares only MockObject as return type. Use an intersection with a concrete type.', + 27, + ], + ] + ); + } +} diff --git a/tests/rules/RequireMockObjectInPropertyTypeRuleTest.php b/tests/rules/RequireMockObjectInPropertyTypeRuleTest.php new file mode 100644 index 0000000..55e97cb --- /dev/null +++ b/tests/rules/RequireMockObjectInPropertyTypeRuleTest.php @@ -0,0 +1,37 @@ + + */ +final class RequireMockObjectInPropertyTypeRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new RequireMockObjectInPropertyTypeRule(); + } + + public function testRule(): void + { + $this->analyse( + [__DIR__ . '/Fixtures/RequireMockObjectInPropertyTypeFixture.php'], + [ + [ + 'Property typed as MockObject only in PHPDoc. Use intersection type with MockObject.', + 16, + ], + ] + ); + } +}