diff --git a/README.md b/README.md index 13fdaeb..2169eb5 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ Additional rules for PHPStan, mostly focused on Clean Code and architecture conv 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. +For example, you can configure the rules so that all controllers in your application must be `readonly`, no method should have more than 3 arguments, and no class should have more than 2 nested control structures. ## Usage @@ -21,6 +21,8 @@ See [Rules documentation](docs/Rules.md) for a list of available rules and confi - [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) + - [Catch Exception of Type Not Allowed Rule](docs/Rules.md#catch-exception-of-type-not-allowed-rule) + - [Methods Returning Bool Must Follow Naming Convention Rule](docs/Rules.md#methods-returning-bool-must-follow-naming-convention-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: diff --git a/data/MethodSignatureMustMatch/TestClass.php b/data/MethodSignatureMustMatch/TestClass.php index e74ac3c..a02fefb 100644 --- a/data/MethodSignatureMustMatch/TestClass.php +++ b/data/MethodSignatureMustMatch/TestClass.php @@ -5,4 +5,12 @@ class TestClass public function testMethod(int $a) { } + + public function testMethodNoType($x, string $y) + { + } + + public function testMethodWithWrongType(int $x, int $y) + { + } } diff --git a/docs/Rules.md b/docs/Rules.md index ce4c647..1ca9a13 100644 --- a/docs/Rules.md +++ b/docs/Rules.md @@ -2,49 +2,49 @@ Add them to your `phpstan.neon` configuration file under the section `services`. -## Control Structure Nesting Rule +## Control Structure Nesting Rule {#control-structure-nesting-rule} Ensures that the nesting level of `if` and `try-catch` statements does not exceed a specified limit. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\CleanCode\ControlStructureNestingRule + class: Phauthentic\PHPStanRules\CleanCode\ControlStructureNestingRule arguments: maxNestingLevel: 2 tags: - phpstan.rules.rule ``` -## Too Many Arguments Rule +## Too Many Arguments Rule {#too-many-arguments-rule} Checks that methods do not have more than a specified number of arguments. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\CleanCode\TooManyArgumentsRule + class: Phauthentic\PHPStanRules\CleanCode\TooManyArgumentsRule arguments: maxArguments: 3 tags: - phpstan.rules.rule ``` -## Readonly Class Rule +## Readonly Class Rule {#readonly-class-rule} Ensures that classes matching specified patterns are declared as `readonly`. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\ReadonlyClassRule + class: Phauthentic\PHPStanRules\Architecture\ClassMustBeReadonlyRule arguments: patterns: ['/^App\\Controller\\/'] tags: - phpstan.rules.rule ``` -## Dependency Constraints Rule +## Dependency Constraints Rule {#dependency-constraints-rule} Enforces dependency constraints between namespaces by checking `use` statements. @@ -55,7 +55,7 @@ In the example below nothing from `App\Domain` can depend on anything from `App\ **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\DependencyConstraintsRule + class: Phauthentic\PHPStanRules\Architecture\DependencyConstraintsRule arguments: forbiddenDependencies: [ '/^App\\Domain(?:\\\w+)*$/': ['/^App\\Controller\\/'] @@ -64,28 +64,28 @@ In the example below nothing from `App\Domain` can depend on anything from `App\ - phpstan.rules.rule ``` -## Final Class Rule +## Final Class Rule {#final-class-rule} Ensures that classes matching specified patterns are declared as `final`. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\FinalClassRule + class: Phauthentic\PHPStanRules\Architecture\ClassMustBeFinalRule arguments: patterns: ['/^App\\Service\\/'] tags: - phpstan.rules.rule ``` -## Namespace Class Pattern Rule +## Namespace Class Pattern Rule {#namespace-class-pattern-rule} Ensures that classes inside namespaces matching a given regex must have names matching at least one of the provided patterns. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\NamespaceClassPatternRule + class: Phauthentic\PHPStanRules\Architecture\ClassnameMustMatchPatternRule arguments: namespaceClassPatterns: [ [ @@ -99,28 +99,42 @@ Ensures that classes inside namespaces matching a given regex must have names ma - phpstan.rules.rule ``` -## Catch Exception of Type Not Allowed Rule +## Catch Exception of Type Not Allowed Rule {#catch-exception-of-type-not-allowed-rule} Ensures that specific exception types are not caught in catch blocks. This is useful for preventing the catching of overly broad exception types like `Exception`, `Error`, or `Throwable`. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\CatchExceptionOfTypeNotAllowedRule + class: Phauthentic\PHPStanRules\Architecture\CatchExceptionOfTypeNotAllowedRule arguments: forbiddenExceptionTypes: ['Exception', 'Error', 'Throwable'] tags: - phpstan.rules.rule ``` -## Method Signature Must Match Rule +## Methods Returning Bool Must Follow Naming Convention Rule {#methods-returning-bool-must-follow-naming-convention-rule} + +Ensures that methods returning boolean values follow a specific naming convention. By default, boolean methods should start with `is`, `has`, `can`, `should`, `was`, or `will`. + +**Configuration Example:** +```neon + - + class: Phauthentic\PHPStanRules\Architecture\MethodsReturningBoolMustFollowNamingConventionRule + arguments: + regex: '/^(is|has|can|should|was|will)[A-Z_]/' + tags: + - phpstan.rules.rule +``` + +## Method Signature Must Match Rule {#method-signature-must-match-rule} Ensures that methods matching a class and method name pattern have a specific signature, including parameter types, names, and count. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\MethodSignatureMustMatchRule + class: Phauthentic\PHPStanRules\Architecture\MethodSignatureMustMatchRule arguments: signaturePatterns: - @@ -140,15 +154,16 @@ Ensures that methods matching a class and method name pattern have a specific si - `pattern`: Regex for `ClassName::methodName`. - `minParameters`/`maxParameters`: Minimum/maximum number of parameters. - `signature`: List of expected parameter types and (optionally) name patterns. +- `visibilityScope`: Optional visibility scope (e.g., `public`, `protected`, `private`). -## Method Must Return Type Rule +## Method Must Return Type Rule {#method-must-return-type-rule} Ensures that methods matching a class and method name pattern have a specific return type, nullability, or are void. **Configuration Example:** ```neon - - class: Phauthentic\PhpstanRules\Architecture\MethodMustReturnTypeRule + class: Phauthentic\PHPStanRules\Architecture\MethodMustReturnTypeRule arguments: returnTypePatterns: - @@ -162,7 +177,7 @@ Ensures that methods matching a class and method name pattern have a specific re type: 'object' nullable: true void: false - objectTypePattern: '/^App\\\\Entity\\\\User$/' + objectTypePattern: '/^App\\Entity\\User$/' - pattern: '/^MyClass::reset$/' type: 'void' diff --git a/phpstan.neon b/phpstan.neon index b530b7a..25b6441 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,49 +4,3 @@ parameters: - src parallel: maximumNumberOfProcesses: 8 - -services: - - - class: Phauthentic\PHPStanRules\CleanCode\ControlStructureNestingRule - arguments: - maxNestingLevel: 2 - tags: - - phpstan.rules.rule - - - class: Phauthentic\PHPStanRules\CleanCode\TooManyArgumentsRule - arguments: - maxArguments: 3 - tags: - - phpstan.rules.rule - - - class: Phauthentic\PHPStanRules\Architecture\ClassMustBeReadonlyRule - arguments: - patterns: ['/^App\\Controller\\/'] - tags: - - phpstan.rules.rule - - - class: Phauthentic\PHPStanRules\Architecture\DependencyConstraintsRule - arguments: - forbiddenDependencies: [ - '/^App\\Domain(?:\\\w+)*$/': ['/^App\\Controller\\/'] - ] - tags: - - phpstan.rules.rule - - - class: Phauthentic\PHPStanRules\Architecture\ClassMustBeFinalRule - arguments: - patterns: ['/^App\\Service\\/'] - tags: - - phpstan.rules.rule - - - class: Phauthentic\PHPStanRules\Architecture\ClassnameMustMatchPatternRule - - arguments: - namespaceClassPatterns: [ - [ - namespace: '/^App\\Service$/', - classPatterns: ['/Class$/'] - ] - ] - tags: - - phpstan.rules.rule \ No newline at end of file diff --git a/src/Architecture/MethodMustReturnTypeRule.php b/src/Architecture/MethodMustReturnTypeRule.php index a3b1a75..1b552ca 100644 --- a/src/Architecture/MethodMustReturnTypeRule.php +++ b/src/Architecture/MethodMustReturnTypeRule.php @@ -15,6 +15,7 @@ /** * Specification: + * * - Checks if the classname plus method name matches a given regex pattern. * - Checks if the method returns the expected type or object, is nullable or void. * - Check if the types of the parameters match the expected types. diff --git a/src/Architecture/MethodSignatureMustMatchRule.php b/src/Architecture/MethodSignatureMustMatchRule.php index 9437023..6d0e7e2 100644 --- a/src/Architecture/MethodSignatureMustMatchRule.php +++ b/src/Architecture/MethodSignatureMustMatchRule.php @@ -17,8 +17,10 @@ /** * Specification: * - Checks if the classname plus method name matches a given regex pattern. - * - Check the min and max number of parameters. - * - Check if the types of the parameters match the expected types. + * - Checks the min and max number of parameters. + * - Checks if the types of the parameters match the expected types. + * - Checks if the parameter names match the expected patterns. + * - Checks if the method has the required visibility scope if specified (public, protected, private). */ class MethodSignatureMustMatchRule implements Rule { @@ -29,6 +31,7 @@ class MethodSignatureMustMatchRule implements Rule private const ERROR_MESSAGE_NAME_PATTERN = 'Method %s parameter #%d name "%s" does not match pattern %s.'; private const ERROR_MESSAGE_MIN_PARAMETERS = 'Method %s has %d parameters, but at least %d required.'; private const ERROR_MESSAGE_MAX_PARAMETERS = 'Method %s has %d parameters, but at most %d allowed.'; + private const ERROR_MESSAGE_VISIBILITY_SCOPE = 'Method %s must be %s.'; /** * @param array + * }>, + * visibilityScope?: string|null * }> $signaturePatterns */ public function __construct( @@ -95,62 +99,112 @@ public function processNode(Node $node, Scope $scope): array // 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(); + $validationResult = $this->validateParameter($expected, $method, $i, $fullName); + if ($validationResult !== null) { + $errors[] = $validationResult; } } } + + if (!$this->isValidVisibilityScope($patternConfig, $method)) { + $errors[] = RuleErrorBuilder::message( + sprintf(self::ERROR_MESSAGE_VISIBILITY_SCOPE, $fullName, $patternConfig['visibilityScope']) + ) + ->identifier(self::IDENTIFIER) + ->line($method->getLine()) + ->build(); + } } } return $errors; } + private function validateParameter(array $expected, $method, int $i, string $fullName): ?\PHPStan\Rules\RuleError + { + $validationCase = $this->determineValidationCase($expected, $method, $i); + + return match ($validationCase) { + 'missing_parameter' => RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_MISSING_PARAMETER, + $fullName, + $i + 1, + $expected['type'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->getLine()) + ->build(), + + 'wrong_type' => RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_WRONG_TYPE, + $fullName, + $i + 1, + $expected['type'], + $this->getTypeAsString($method->params[$i]->type) ?? 'none' + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->params[$i]->getLine()) + ->build(), + + 'invalid_name' => RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_NAME_PATTERN, + $fullName, + $i + 1, + $method->params[$i]->var->name, + $expected['pattern'] + ) + ) + ->identifier(self::IDENTIFIER) + ->line($method->params[$i]->getLine()) + ->build(), + + default => null, + }; + } + + private function determineValidationCase(array $expected, $method, int $i): string + { + if (!isset($method->params[$i])) { + return 'missing_parameter'; + } + + $param = $method->params[$i]; + + // Check type if specified in configuration + if (isset($expected['type']) && $expected['type'] !== null) { + $paramType = $param->type ? $this->getTypeAsString($param->type) : null; + if ($paramType !== $expected['type']) { + return 'wrong_type'; + } + } + + // Check name pattern + if ($this->isInvalidParameterName(expected: $expected, param: $param)) { + return 'invalid_name'; + } + + return 'valid'; + } + + private function isValidVisibilityScope(array $patternConfig, $method): bool + { + if (!isset($patternConfig['visibilityScope']) || $patternConfig['visibilityScope'] === null) { + return true; + } + + return match ($patternConfig['visibilityScope']) { + 'public' => $method->isPublic(), + 'protected' => $method->isProtected(), + 'private' => $method->isPrivate(), + default => true, + }; + } + /** * @param array $patternConfig * @param int $paramCount @@ -164,10 +218,7 @@ private function checkMinParameters( string $fullName, ClassMethod $method ): array { - if ( - $patternConfig['minParameters'] !== null && - $paramCount < $patternConfig['minParameters'] - ) { + if ($this->isBelowMinParameters($patternConfig, $paramCount)) { return [ RuleErrorBuilder::message( message: sprintf( @@ -186,6 +237,19 @@ private function checkMinParameters( return []; } + /** + * Checks if the parameter count is below the minimum required. + * + * @param array $patternConfig + * @param int $paramCount + * @return bool + */ + private function isBelowMinParameters(array $patternConfig, int $paramCount): bool + { + return $patternConfig['minParameters'] !== null + && $paramCount < $patternConfig['minParameters']; + } + /** * @param array $patternConfig * @param int $paramCount @@ -199,10 +263,7 @@ private function checkMaxParameters( string $fullName, ClassMethod $method ): array { - if ( - $patternConfig['maxParameters'] !== null && - $paramCount > $patternConfig['maxParameters'] - ) { + if ($this->isAboveMaxParameters($patternConfig, $paramCount)) { return [ RuleErrorBuilder::message( message: sprintf( @@ -220,6 +281,19 @@ private function checkMaxParameters( return []; } + /** + * Checks if the parameter count is above the maximum allowed. + * + * @param array $patternConfig + * @param int $paramCount + * @return bool + */ + private function isAboveMaxParameters(array $patternConfig, int $paramCount): bool + { + return $patternConfig['maxParameters'] !== null + && $paramCount > $patternConfig['maxParameters']; + } + /** * Checks if the parameter name does not match the expected pattern. * diff --git a/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php index 0f3867a..3605760 100644 --- a/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php +++ b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleTest.php @@ -24,6 +24,27 @@ protected function getRule(): Rule ['type' => 'int', 'pattern' => '/^a/'], ['type' => 'string', 'pattern' => '/^b/'], ], + 'visibilityScope' => 'private', + ], + [ + 'pattern' => '/^TestClass::testMethodNoType$/', + 'minParameters' => 1, + 'maxParameters' => 2, + 'signature' => [ + ['pattern' => '/^x$/'], // No type specified + ['type' => 'string', 'pattern' => '/^y$/'], // Type specified + ], + 'visibilityScope' => 'public', + ], + [ + 'pattern' => '/^TestClass::testMethodWithWrongType$/', + 'minParameters' => 2, + 'maxParameters' => 2, + 'signature' => [ + ['type' => 'string', 'pattern' => '/^x$/'], // Type specified but wrong + ['pattern' => '/^y$/'], // No type specified, should be ignored + ], + 'visibilityScope' => 'public', ], ]); } @@ -31,6 +52,7 @@ protected function getRule(): Rule public function testRule(): void { $this->analyse([__DIR__ . '/../../../data/MethodSignatureMustMatch/TestClass.php'], [ + // Errors for testMethod (type checking enabled) [ 'Method TestClass::testMethod has 1 parameters, but at least 2 required.', 5, @@ -39,6 +61,23 @@ public function testRule(): void 'Method TestClass::testMethod is missing parameter #2 of type string.', 5, ], + [ + 'Method TestClass::testMethod must be private.', + 5, + ], + // No errors for testMethodNoType since: + // - First parameter has no type specified, so type checking is skipped + // - Second parameter has correct type (string) and name matches pattern + // - Parameter count is within limits (1-2) + // - Visibility is correct (public) + + // Error for testMethodWithWrongType: + // - First parameter has wrong type (int instead of string) + // - Second parameter has no type specified, so type checking is skipped + [ + 'Method TestClass::testMethodWithWrongType parameter #1 should be of type string, int given.', + 13, + ], ]); } }