diff --git a/README.md b/README.md index 504d4ee..13fdaeb 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,10 @@ Additional rules for PHPStan, mostly focused on Clean Code and architecture conventions. +The rules help with enforcing certain method signatures, return types and dependency constraints in your codebase. + +All controllers in your application should be `readonly`, no method should have more than 3 arguments, and no class should have more than 2 nested control structures. + ## Usage ```bash @@ -12,13 +16,16 @@ composer require phauthentic/phpstan-rules --dev See [Rules documentation](docs/Rules.md) for a list of available rules and configuration examples. -**Available Rules:** -- [Control Structure Nesting Rule](docs/Rules.md#control-structure-nesting-rule) -- [Too Many Arguments Rule](docs/Rules.md#too-many-arguments-rule) -- [Readonly Class Rule](docs/Rules.md#readonly-class-rule) -- [Dependency Constraints Rule](docs/Rules.md#dependency-constraints-rule) -- [Final Class Rule](docs/Rules.md#final-class-rule) -- [Namespace Class Pattern Rule](docs/Rules.md#namespace-class-pattern-rule) +- Architecture Rules: + - [Dependency Constraints Rule](docs/Rules.md#dependency-constraints-rule) + - [Readonly Class Rule](docs/Rules.md#readonly-class-rule) + - [Final Class Rule](docs/Rules.md#final-class-rule) + - [Namespace Class Pattern Rule](docs/Rules.md#namespace-class-pattern-rule) + - [Method Signature Must Match Rule](docs/Rules.md#method-signature-must-match-rule) + - [Method Must Return Type Rule](docs/Rules.md#method-must-return-type-rule) +- Clean Code Rules: + - [Control Structure Nesting Rule](docs/Rules.md#control-structure-nesting-rule) + - [Too Many Arguments Rule](docs/Rules.md#too-many-arguments-rule) ### Using Regex in Rules diff --git a/data/MethodMustReturnType/ReturnTypeTestClass.php b/data/MethodMustReturnType/ReturnTypeTestClass.php new file mode 100644 index 0000000..19c3112 --- /dev/null +++ b/data/MethodMustReturnType/ReturnTypeTestClass.php @@ -0,0 +1,12 @@ + */ class ClassMustBeFinalRule implements Rule @@ -20,19 +24,13 @@ class ClassMustBeFinalRule implements Rule private const IDENTIFIER = 'phauthentic.architecture.classMustBeFinal'; - /** - * @var array An array of regex patterns to match against class names. - * e.g., ['#^App\\Domain\\.*#', '#^App\\Service\\.*#'] - */ - protected array $patterns; - /** * @param array $patterns An array of regex patterns to match against class names. * Each pattern should be a valid PCRE regex. */ - public function __construct(array $patterns) - { - $this->patterns = $patterns; + public function __construct( + protected array $patterns + ) { } public function getNodeType(): string @@ -55,14 +53,17 @@ public function processNode(Node $node, Scope $scope): array foreach ($this->patterns as $pattern) { if (preg_match($pattern, $fullClassName) && !$node->isFinal()) { - return [ - RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $fullClassName)) - ->identifier(self::IDENTIFIER) - ->build(), - ]; + return [$this->buildRuleError($fullClassName)]; } } return []; } + + private function buildRuleError(string $fullClassName) + { + return RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $fullClassName)) + ->identifier(self::IDENTIFIER) + ->build(); + } } diff --git a/src/Architecture/ClassMustBeReadonlyRule.php b/src/Architecture/ClassMustBeReadonlyRule.php index 697ae43..59f6756 100644 --- a/src/Architecture/ClassMustBeReadonlyRule.php +++ b/src/Architecture/ClassMustBeReadonlyRule.php @@ -11,6 +11,8 @@ use PHPStan\Rules\RuleErrorBuilder; /** + * Specification: + * - A matching class must be declared as readonly. * @implements Rule */ class ClassMustBeReadonlyRule implements Rule diff --git a/src/Architecture/ClassnameMustMatchPatternRule.php b/src/Architecture/ClassnameMustMatchPatternRule.php index e86c3e7..c001f2a 100644 --- a/src/Architecture/ClassnameMustMatchPatternRule.php +++ b/src/Architecture/ClassnameMustMatchPatternRule.php @@ -13,6 +13,7 @@ use PHPStan\ShouldNotHappenException; /** + * Specification: * PHPStan rule to ensure that classes inside namespaces matching a given regex * must have names matching at least one of the provided patterns. * @@ -24,17 +25,12 @@ class ClassnameMustMatchPatternRule implements Rule private const IDENTIFIER = 'phauthentic.architecture.classnameMustMatchPattern'; - /** - * @var array{namespace: string, classPatterns: string[]}[] - */ - private array $namespaceClassPatterns; - /** * @param array{namespace: string, classPatterns: string[]}[] $namespaceClassPatterns */ - public function __construct(array $namespaceClassPatterns) - { - $this->namespaceClassPatterns = $namespaceClassPatterns; + public function __construct( + protected array $namespaceClassPatterns + ) { } public function getNodeType(): string @@ -124,8 +120,8 @@ public function buildRuleError( ): array { $fqcn = $namespaceName ? $namespaceName . '\\' . $className : $className; $errors[] = RuleErrorBuilder::message( - $this->buildErrorMessage($fqcn, $namespaceName, $classPatterns) - ) + $this->buildErrorMessage($fqcn, $namespaceName, $classPatterns) + ) ->line($stmt->getLine()) ->identifier(self::IDENTIFIER) ->build(); diff --git a/src/Architecture/DependencyConstraintsRule.php b/src/Architecture/DependencyConstraintsRule.php index 982c698..a7c73c0 100644 --- a/src/Architecture/DependencyConstraintsRule.php +++ b/src/Architecture/DependencyConstraintsRule.php @@ -18,6 +18,10 @@ * This rule checks the `use` statements in your PHP code and ensures that * certain namespaces do not depend on other namespaces as specified in the * configuration. + * + * Specification: + * - A class in a namespace matching a given regex is not allowed to depend on any namespace defined by a set of + * other regexes. */ class DependencyConstraintsRule implements Rule { diff --git a/src/Architecture/MethodMustReturnTypeRule.php b/src/Architecture/MethodMustReturnTypeRule.php new file mode 100644 index 0000000..a3b1a75 --- /dev/null +++ b/src/Architecture/MethodMustReturnTypeRule.php @@ -0,0 +1,252 @@ + $returnTypePatterns + */ + public function __construct( + protected array $returnTypePatterns + ) { + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + * @param Scope $scope + * @return array + */ + public function processNode(Node $node, Scope $scope): array + { + $errors = []; + $className = $node->name ? $node->name->toString() : ''; + + foreach ($node->getMethods() as $method) { + $methodName = $method->name->toString(); + $fullName = $className . '::' . $methodName; + + foreach ($this->returnTypePatterns as $patternConfig) { + if (!preg_match($patternConfig['pattern'], $fullName)) { + continue; + } + + $returnTypeNode = $method->getReturnType(); + $returnType = $this->getTypeAsString($returnTypeNode); + + // Check for void + if ($this->shouldErrorOnVoid($patternConfig, $returnType)) { + $errors[] = $this->buildVoidError($fullName, $method->getLine()); + continue; + } + + // Check for missing return type + if ($this->shouldErrorOnMissingReturnType($returnType)) { + $errors[] = $this->buildMissingReturnTypeError($fullName, $patternConfig['type'], $method->getLine()); + continue; + } + + // Check for nullable + $isNullable = $this->isNullableType($returnTypeNode); + + if ($this->shouldErrorOnNullability($patternConfig, $isNullable)) { + $errors[] = $this->buildNullabilityError($fullName, $patternConfig['nullable'], $method->getLine()); + } + + // Check for type + if ($patternConfig['type'] === 'object') { + if ($returnTypeNode instanceof Name) { + $objectType = $returnTypeNode->toString(); + if ($this->shouldErrorOnObjectTypePattern($patternConfig, $objectType)) { + $errors[] = $this->buildObjectTypePatternError($fullName, $patternConfig['objectTypePattern'], $objectType, $method->getLine()); + } + } else { + $errors[] = $this->buildObjectTypeError($fullName, $method->getLine()); + } + } elseif ($this->shouldErrorOnTypeMismatch($returnType, $patternConfig['type'])) { + $errors[] = $this->buildTypeMismatchError($fullName, $patternConfig['type'], $returnType, $method->getLine()); + } + } + } + + return $errors; + } + + private function shouldErrorOnVoid(array $patternConfig, ?string $returnType): bool + { + return $patternConfig['void'] && $returnType !== 'void'; + } + + private function buildVoidError(string $fullName, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_VOID, + $fullName + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function shouldErrorOnMissingReturnType(?string $returnType): bool + { + return $returnType === null; + } + + private function buildMissingReturnTypeError(string $fullName, string $expectedType, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_MISSING_RETURN_TYPE, + $fullName, + $expectedType + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function shouldErrorOnNullability(array $patternConfig, bool $isNullable): bool + { + return $patternConfig['nullable'] !== $isNullable; + } + + private function buildNullabilityError(string $fullName, bool $expectedNullable, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_NULLABILITY, + $fullName, + $expectedNullable ? 'nullable' : 'non-nullable' + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function shouldErrorOnObjectTypePattern(array $patternConfig, string $objectType): bool + { + return $patternConfig['objectTypePattern'] !== null && + !preg_match($patternConfig['objectTypePattern'], $objectType); + } + + private function buildObjectTypePatternError(string $fullName, string $pattern, string $objectType, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_OBJECT_TYPE_PATTERN, + $fullName, + $pattern, + $objectType + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function buildObjectTypeError(string $fullName, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_OBJECT_TYPE, + $fullName + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function shouldErrorOnTypeMismatch(?string $returnType, string $expectedType): bool + { + return $returnType !== $expectedType && !$this->isNullableMatch($returnType, $expectedType); + } + + private function buildTypeMismatchError(string $fullName, string $expectedType, ?string $actualType, int $line) + { + return RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_TYPE_MISMATCH, + $fullName, + $expectedType, + $actualType + ) + ) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + + private function getTypeAsString(mixed $type): ?string + { + $nullableInner = null; + if ($type instanceof NullableType) { + $nullableInner = $this->getTypeAsString($type->type); + } + + return match (true) { + $type === null => null, + $type instanceof Identifier => $type->name, + $type instanceof Name => $type->toString(), + $type instanceof NullableType => $nullableInner !== null ? '?' . $nullableInner : null, + default => null, + }; + } + + private function isNullableType($type): bool + { + return $type instanceof NullableType; + } + + private function isNullableMatch(string $actual, string $expected): bool + { + // Handles cases like '?int' vs 'int' + if (ltrim($actual, '?') === $expected) { + return true; + } + return false; + } +} diff --git a/src/Architecture/MethodSignatureMustMatchRule.php b/src/Architecture/MethodSignatureMustMatchRule.php new file mode 100644 index 0000000..9437023 --- /dev/null +++ b/src/Architecture/MethodSignatureMustMatchRule.php @@ -0,0 +1,249 @@ + + * }> $signaturePatterns + */ + public function __construct( + protected array $signaturePatterns + ) { + } + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + * @param Scope $scope + * @return array + */ + public function processNode(Node $node, Scope $scope): array + { + $errors = []; + $className = $node->name ? $node->name->toString() : ''; + + foreach ($node->getMethods() as $method) { + $methodName = $method->name->toString(); + $fullName = $className . '::' . $methodName; + + foreach ($this->signaturePatterns as $patternConfig) { + if (!preg_match($patternConfig['pattern'], $fullName)) { + continue; + } + + $paramCount = count($method->params); + + $minParamErrors = $this->checkMinParameters( + patternConfig: $patternConfig, + paramCount: $paramCount, + fullName: $fullName, + method: $method + ); + + $maxParamErrors = $this->checkMaxParameters( + patternConfig: $patternConfig, + paramCount: $paramCount, + fullName: $fullName, + method: $method + ); + + foreach ([$minParamErrors, $maxParamErrors] as $paramErrors) { + foreach ($paramErrors as $error) { + $errors[] = $error; + } + } + + // Check parameter types and patterns + if (!empty($patternConfig['signature'])) { + foreach ($patternConfig['signature'] as $i => $expected) { + if (!isset($method->params[$i])) { + $errors[] = RuleErrorBuilder::message( + message: sprintf( + self::ERROR_MESSAGE_MISSING_PARAMETER, + $fullName, + $i + 1, + $expected['type'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->getLine()) + ->build(); + + continue; + } + + $param = $method->params[$i]; + $paramType = $param->type ? $this->getTypeAsString($param->type) : null; + + if ($paramType !== $expected['type']) { + $errors[] = RuleErrorBuilder::message( + message: sprintf( + self::ERROR_MESSAGE_WRONG_TYPE, + $fullName, + $i + 1, + $expected['type'], + $paramType ?? 'none' + ) + ) + ->identifier(identifier: self::IDENTIFIER) + ->line(line: $param->getLine()) + ->build(); + } + + if ($this->isInvalidParameterName(expected: $expected, param: $param)) { + $errors[] = RuleErrorBuilder::message( + message: sprintf( + self::ERROR_MESSAGE_NAME_PATTERN, + $fullName, + $i + 1, + $param->var->name, + $expected['pattern'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($param->getLine()) + ->build(); + } + } + } + } + } + + return $errors; + } + + /** + * @param array $patternConfig + * @param int $paramCount + * @param string $fullName + * @param ClassMethod $method + * @return array + */ + private function checkMinParameters( + array $patternConfig, + int $paramCount, + string $fullName, + ClassMethod $method + ): array { + if ( + $patternConfig['minParameters'] !== null && + $paramCount < $patternConfig['minParameters'] + ) { + return [ + RuleErrorBuilder::message( + message: sprintf( + self::ERROR_MESSAGE_MIN_PARAMETERS, + $fullName, + $paramCount, + $patternConfig['minParameters'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->getLine()) + ->build() + ]; + } + + return []; + } + + /** + * @param array $patternConfig + * @param int $paramCount + * @param string $fullName + * @param ClassMethod $method + * @return array + */ + private function checkMaxParameters( + array $patternConfig, + int $paramCount, + string $fullName, + ClassMethod $method + ): array { + if ( + $patternConfig['maxParameters'] !== null && + $paramCount > $patternConfig['maxParameters'] + ) { + return [ + RuleErrorBuilder::message( + message: sprintf( + self::ERROR_MESSAGE_MAX_PARAMETERS, + $fullName, + $paramCount, + $patternConfig['maxParameters'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->getLine()) + ->build() + ]; + } + return []; + } + + /** + * Checks if the parameter name does not match the expected pattern. + * + * @param array $expected + * @param \PhpParser\Node\Param $param + * @return bool + */ + private function isInvalidParameterName(array $expected, $param): bool + { + return isset($expected['pattern']) + && $expected['pattern'] !== null + && $param->var->name !== null + && !preg_match($expected['pattern'], (string)$param->var->name); + } + + private function getTypeAsString(mixed $type): ?string + { + return match (true) { + $type === null => null, + $type instanceof Identifier => $type->name, + $type instanceof Name => $type->toString(), + $type instanceof NullableType => + ($inner = $this->getTypeAsString($type->type)) !== null ? '?' . $inner : null, + default => null, + }; + } +} diff --git a/src/Architecture/MethodsReturningBoolMustFollowNamingConventionRule.php b/src/Architecture/MethodsReturningBoolMustFollowNamingConventionRule.php new file mode 100644 index 0000000..d1b501e --- /dev/null +++ b/src/Architecture/MethodsReturningBoolMustFollowNamingConventionRule.php @@ -0,0 +1,106 @@ + + */ +class MethodsReturningBoolMustFollowNamingConventionRule implements Rule +{ + private const ERROR_MESSAGE = 'Method %s::%s() returns boolean but does not follow naming convention (regex: %s).'; + private const IDENTIFIER = 'phauthentic.architecture.methodsReturningBoolMustFollowNamingConvention'; + + public function __construct( + protected string $regex = '/^(is|has|can|should|was|will)[A-Z_]/' + ) { + } + + public function getNodeType(): string + { + return ClassMethod::class; + } + + /** + * Skip constructors, destructors, and magic methods + */ + private function isSkippableMethod(Node $node): bool + { + return $node->name->toString() === '__construct' || + $node->name->toString() === '__destruct' || + strpos($node->name->toString(), '__') === 0; + } + + private function hasReturnType(Node $node): bool + { + return $node->returnType !== null; + } + + private function hasBooleanReturnType(Node $node, Scope $scope): bool + { + $classReflection = $scope->getClassReflection(); + if ($classReflection === null) { + return false; + } + + if (!$classReflection->hasMethod($node->name->toString())) { + return false; + } + + $returnType = $classReflection->getMethod($node->name->toString(), $scope) + ->getVariants()[0] + ->getReturnType(); + + if (!$returnType instanceof BooleanType) { + return false; + } + + return true; + } + + /** + * @param ClassMethod $node + * @param Scope $scope + * @return list<\PHPStan\Rules\RuleError> + */ + public function processNode(Node $node, Scope $scope): array + { + if ( + $this->isSkippableMethod($node) || + !$this->hasReturnType($node) || + !$this->hasBooleanReturnType($node, $scope) + ) { + return []; + } + + $methodName = $node->name->toString(); + if (!preg_match($this->regex, $methodName)) { + return [ + RuleErrorBuilder::message(sprintf( + self::ERROR_MESSAGE, + $scope->getClassReflection()->getName(), + $methodName, + $this->regex + )) + ->identifier(self::IDENTIFIER) + ->line($node->getLine()) + ->build() + ]; + } + + return []; + } +} diff --git a/tests/TestCases/Architecture/MethodMustReturnTypeRuleTest.php b/tests/TestCases/Architecture/MethodMustReturnTypeRuleTest.php new file mode 100644 index 0000000..6cb91f9 --- /dev/null +++ b/tests/TestCases/Architecture/MethodMustReturnTypeRuleTest.php @@ -0,0 +1,71 @@ + + */ +class MethodMustReturnTypeRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new MethodMustReturnTypeRule([ + [ + 'pattern' => '/^ReturnTypeTestClass::mustReturnInt$/', + 'type' => 'int', + 'nullable' => false, + 'void' => false, + 'objectTypePattern' => null, + ], + [ + 'pattern' => '/^ReturnTypeTestClass::mustReturnNullableString$/', + 'type' => 'string', + 'nullable' => true, + 'void' => false, + 'objectTypePattern' => null, + ], + [ + 'pattern' => '/^ReturnTypeTestClass::mustReturnVoid$/', + 'type' => 'void', + 'nullable' => false, + 'void' => true, + 'objectTypePattern' => null, + ], + [ + 'pattern' => '/^ReturnTypeTestClass::mustReturnSpecificObject$/', + 'type' => 'object', + 'nullable' => false, + 'void' => false, + 'objectTypePattern' => '/^SomeObject$/', + ], + ]); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/../../../data/MethodMustReturnType/ReturnTypeTestClass.php'], [ + [ + 'Method ReturnTypeTestClass::mustReturnInt must have return type int, void given.', + 5, + ], + [ + 'Method ReturnTypeTestClass::mustReturnNullableString return type nullability does not match: expected nullable.', + 6, + ], + [ + 'Method ReturnTypeTestClass::mustReturnVoid must have a void return type.', + 7, + ], + [ + 'Method ReturnTypeTestClass::mustReturnSpecificObject must return an object matching pattern /^SomeObject$/, OtherObject given.', + 8, + ], + ]); + } +} diff --git a/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php new file mode 100644 index 0000000..0f3867a --- /dev/null +++ b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php @@ -0,0 +1,44 @@ + + */ +class MethodSignatureMustMatchRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new MethodSignatureMustMatchRule([ + [ + 'pattern' => '/^TestClass::testMethod$/', + 'minParameters' => 2, + 'maxParameters' => 3, + 'signature' => [ + ['type' => 'int', 'pattern' => '/^a/'], + ['type' => 'string', 'pattern' => '/^b/'], + ], + ], + ]); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/../../../data/MethodSignatureMustMatch/TestClass.php'], [ + [ + 'Method TestClass::testMethod has 1 parameters, but at least 2 required.', + 5, + ], + [ + 'Method TestClass::testMethod is missing parameter #2 of type string.', + 5, + ], + ]); + } +} diff --git a/tests/TestCases/Architecture/MethodsReturningBooleMustFollowNamingConventionRuleTest.php b/tests/TestCases/Architecture/MethodsReturningBooleMustFollowNamingConventionRuleTest.php new file mode 100644 index 0000000..7dc36c3 --- /dev/null +++ b/tests/TestCases/Architecture/MethodsReturningBooleMustFollowNamingConventionRuleTest.php @@ -0,0 +1,30 @@ + + */ +class MethodsReturningBooleMustFollowNamingConventionRuleTest extends RuleTestCase +{ + protected function getRule(): MethodsReturningBoolMustFollowNamingConventionRule + { + return new MethodsReturningBoolMustFollowNamingConventionRule(); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/../../../data/MethodsReturningBooleMustFollowNamingConvention/TestClass.php'], [ + [ + 'Method TestClass::flag() returns boolean but does not follow naming convention (regex: /^(is|has|can|should|was|will)[A-Z_]/).', + 5, + ], + ]); + } +}