From 7a0c47c6ca4a0393a90d5af6d7b14cef70b61bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Mon, 8 Dec 2025 13:05:37 +0100 Subject: [PATCH] Making it possible to require methods in a class --- .../RequiredMethodTestClass.php | 38 +++++ .../rules/Method-Signature-Must-Match-Rule.md | 29 +++- .../MethodSignatureMustMatchRule.php | 147 +++++++++++++++++- src/CleanCode/MaxLineLengthRule.php | 14 +- ...thodSignatureMustMatchRuleRequiredTest.php | 50 ++++++ .../MaxLineLengthRuleArrayApiTest.php | 3 +- ...ineLengthRuleBackwardCompatibilityTest.php | 3 +- ...eLengthRuleDetectDocBlockLongLinesTest.php | 3 +- ...LengthRuleDetectNamespaceLongLinesTest.php | 3 +- ...axLineLengthRuleDetectUseLongLinesTest.php | 5 +- ...axLineLengthRuleIgnoreUseLongLinesTest.php | 1 - 11 files changed, 275 insertions(+), 21 deletions(-) create mode 100644 data/MethodSignatureMustMatch/RequiredMethodTestClass.php create mode 100644 tests/TestCases/Architecture/MethodSignatureMustMatchRuleRequiredTest.php diff --git a/data/MethodSignatureMustMatch/RequiredMethodTestClass.php b/data/MethodSignatureMustMatch/RequiredMethodTestClass.php new file mode 100644 index 0000000..29627ba --- /dev/null +++ b/data/MethodSignatureMustMatch/RequiredMethodTestClass.php @@ -0,0 +1,38 @@ +, - * visibilityScope?: string|null + * visibilityScope?: string|null, + * required?: bool|null * }> $signaturePatterns */ public function __construct( @@ -65,6 +68,12 @@ public function processNode(Node $node, Scope $scope): array $errors = []; $className = $node->name ? $node->name->toString() : ''; + // Check for required methods first + $requiredMethodErrors = $this->checkRequiredMethods($node, $className); + foreach ($requiredMethodErrors as $error) { + $errors[] = $error; + } + foreach ($node->getMethods() as $method) { $methodName = $method->name->toString(); $fullName = $className . '::' . $methodName; @@ -320,4 +329,140 @@ private function getTypeAsString(mixed $type): ?string default => null, }; } + + /** + * Extract class name pattern and method name from a regex pattern. + * Expected pattern format: '/^ClassName::methodName$/' or '/ClassName::methodName$/' + * + * @param string $pattern + * @return array|null Array with 'classPattern' and 'methodName', or null if parsing fails + */ + private function extractClassAndMethodFromPattern(string $pattern): ?array + { + // Remove pattern delimiters and anchors + $cleaned = preg_replace('/^\/\^?/', '', $pattern); + $cleaned = preg_replace('/\$?\/$/', '', $cleaned); + + if ($cleaned === null || !str_contains($cleaned, '::')) { + return null; + } + + $parts = explode('::', $cleaned, 2); + if (count($parts) !== 2) { + return null; + } + + return [ + 'classPattern' => $parts[0], + 'methodName' => $parts[1], + ]; + } + + /** + * Check if a class name matches a pattern extracted from regex. + * + * @param string $className + * @param string $classPattern + * @return bool + */ + private function classMatchesPattern(string $className, string $classPattern): bool + { + // Build a regex from the class pattern + $regex = '/^' . $classPattern . '$/'; + return preg_match($regex, $className) === 1; + } + + /** + * Format the expected method signature for error messages. + * + * @param array $patternConfig + * @return string + */ + private function formatSignatureForError(array $patternConfig): string + { + $parts = []; + + // Add visibility scope if specified + if (isset($patternConfig['visibilityScope']) && $patternConfig['visibilityScope'] !== null) { + $parts[] = $patternConfig['visibilityScope']; + } + + $parts[] = 'function'; + + // Extract method name from pattern + $extracted = $this->extractClassAndMethodFromPattern($patternConfig['pattern']); + if ($extracted !== null) { + $parts[] = $extracted['methodName']; + } + + // Build parameters + $params = []; + if (!empty($patternConfig['signature'])) { + foreach ($patternConfig['signature'] as $i => $sig) { + $paramParts = []; + if (isset($sig['type']) && $sig['type'] !== null) { + $paramParts[] = $sig['type']; + } + $paramParts[] = '$param' . ($i + 1); + $params[] = implode(' ', $paramParts); + } + } + + return implode(' ', $parts) . '(' . implode(', ', $params) . ')'; + } + + /** + * Check if required methods are implemented in the class. + * + * @param Class_ $node + * @param string $className + * @return array + */ + private function checkRequiredMethods(Class_ $node, string $className): array + { + $errors = []; + + // Get list of implemented methods + $implementedMethods = []; + foreach ($node->getMethods() as $method) { + $implementedMethods[] = $method->name->toString(); + } + + // Check each pattern with required flag + foreach ($this->signaturePatterns as $patternConfig) { + // Skip if not required + if (!isset($patternConfig['required']) || $patternConfig['required'] !== true) { + continue; + } + + // Extract class and method patterns + $extracted = $this->extractClassAndMethodFromPattern($patternConfig['pattern']); + if ($extracted === null) { + continue; + } + + // Check if class matches the pattern + if (!$this->classMatchesPattern($className, $extracted['classPattern'])) { + continue; + } + + // Check if method is implemented + if (!in_array($extracted['methodName'], $implementedMethods, true)) { + $signature = $this->formatSignatureForError($patternConfig); + $errors[] = RuleErrorBuilder::message( + sprintf( + self::ERROR_MESSAGE_REQUIRED_METHOD, + $className, + $extracted['methodName'], + $signature + ) + ) + ->identifier(self::IDENTIFIER) + ->line($node->getLine()) + ->build(); + } + } + + return $errors; + } } diff --git a/src/CleanCode/MaxLineLengthRule.php b/src/CleanCode/MaxLineLengthRule.php index 6b0521f..15cd530 100644 --- a/src/CleanCode/MaxLineLengthRule.php +++ b/src/CleanCode/MaxLineLengthRule.php @@ -84,7 +84,7 @@ public function __construct( ) { $this->maxLineLength = $maxLineLength; $this->excludePatterns = $excludePatterns; - + // BC: ignoreUseStatements parameter takes precedence over array when both are set $this->ignoreUseStatements = $ignoreUseStatements ?: ($ignoreLineTypes['useStatements'] ?? false); $this->ignoreNamespaceDeclaration = $ignoreLineTypes['namespaceDeclaration'] ?? false; @@ -110,7 +110,7 @@ public function processNode(Node $node, Scope $scope): array if ($node instanceof FileNode) { return []; } - + // Skip if file should be excluded if ($this->shouldExcludeFile($scope)) { return []; @@ -122,7 +122,7 @@ public function processNode(Node $node, Scope $scope): array // Track use statement lines for this file if ($node instanceof Use_) { $this->markLineAsUseStatement($filePath, $lineNumber); - + // If ignoring use statements, skip processing this node if ($this->ignoreUseStatements) { return []; @@ -134,7 +134,7 @@ public function processNode(Node $node, Scope $scope): array // Only mark the start line where the namespace declaration appears $namespaceLine = $node->getStartLine(); $this->markLineAsNamespace($filePath, $namespaceLine); - + // If ignoring namespaces and this is the namespace declaration line, skip it if ($this->ignoreNamespaceDeclaration && $lineNumber === $namespaceLine) { return []; @@ -147,12 +147,12 @@ public function processNode(Node $node, Scope $scope): array if ($docComment !== null) { $startLine = $docComment->getStartLine(); $endLine = $docComment->getEndLine(); - + // Mark all docblock lines for ($line = $startLine; $line <= $endLine; $line++) { $this->markLineAsDocBlock($filePath, $line); } - + // If not ignoring docblocks, check each line in the docblock if (!$this->ignoreDocBlocks) { for ($line = $startLine; $line <= $endLine; $line++) { @@ -160,7 +160,7 @@ public function processNode(Node $node, Scope $scope): array if ($this->isLineProcessed($filePath, $line)) { continue; } - + $lineLength = $this->getLineLength($filePath, $line); if ($lineLength > $this->maxLineLength) { $this->markLineAsProcessed($filePath, $line); diff --git a/tests/TestCases/Architecture/MethodSignatureMustMatchRuleRequiredTest.php b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleRequiredTest.php new file mode 100644 index 0000000..bd33e69 --- /dev/null +++ b/tests/TestCases/Architecture/MethodSignatureMustMatchRuleRequiredTest.php @@ -0,0 +1,50 @@ + + */ +class MethodSignatureMustMatchRuleRequiredTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new MethodSignatureMustMatchRule([ + [ + 'pattern' => '/^.*TestController::execute$/', + 'minParameters' => 1, + 'maxParameters' => 1, + 'signature' => [ + ['type' => 'int', 'pattern' => '/^id$/'], + ], + 'visibilityScope' => 'public', + 'required' => true, + ], + ]); + } + + public function testRequiredMethodRule(): void + { + $this->analyse([__DIR__ . '/../../../data/MethodSignatureMustMatch/RequiredMethodTestClass.php'], [ + // MyTestController is missing the required execute method + [ + 'Class MyTestController must implement method execute with signature: public function execute(int $param1).', + 8, + ], + // AnotherTestController implements the method correctly - no error expected + + // YetAnotherTestController is missing the required execute method + [ + 'Class YetAnotherTestController must implement method execute with signature: public function execute(int $param1).', + 24, + ], + // NotAController doesn't match the pattern - no error expected + ]); + } +} diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php index d1ef9e5..6109d1d 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php @@ -10,7 +10,7 @@ /** * Test the new array API for ignore options - * + * * @extends RuleTestCase */ class MaxLineLengthRuleArrayApiTest extends RuleTestCase @@ -41,4 +41,3 @@ public function testArrayApiForUseStatements(): void ]); } } - diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php index b074834..5a44993 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php @@ -10,7 +10,7 @@ /** * Test backward compatibility: ignoreUseStatements parameter still works - * + * * @extends RuleTestCase */ class MaxLineLengthRuleBackwardCompatibilityTest extends RuleTestCase @@ -41,4 +41,3 @@ public function testBackwardCompatibilityWithOldIgnoreUseStatementsParameter(): ]); } } - diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php index 2429d2b..ada5ff6 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php @@ -10,7 +10,7 @@ /** * Test that long docblock lines ARE detected when ignoreDocBlocks is false - * + * * @extends RuleTestCase */ class MaxLineLengthRuleDetectDocBlockLongLinesTest extends RuleTestCase @@ -55,4 +55,3 @@ public function testLongDocBlockLinesAreDetectedWhenNotIgnored(): void ]); } } - diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php index ebe1a2d..aa6a2a9 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php @@ -10,7 +10,7 @@ /** * Test that long namespace declarations ARE detected when ignoreNamespaces is false - * + * * @extends RuleTestCase */ class MaxLineLengthRuleDetectNamespaceLongLinesTest extends RuleTestCase @@ -45,4 +45,3 @@ public function testLongNamespacesAreDetectedWhenNotIgnored(): void ]); } } - diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectUseLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectUseLongLinesTest.php index cf8d129..e1f3b0e 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectUseLongLinesTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectUseLongLinesTest.php @@ -10,10 +10,10 @@ /** * Test that long use statements ARE detected when ignoreUseStatements is false - * + * * This complements the regression test to ensure the fix doesn't break * the default behavior of detecting long use statements. - * + * * @extends RuleTestCase */ class MaxLineLengthRuleDetectUseLongLinesTest extends RuleTestCase @@ -56,4 +56,3 @@ public function testLongUseStatementsAreDetectedWhenNotIgnored(): void ]); } } - diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreUseLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreUseLongLinesTest.php index 82475f1..fd63b3b 100644 --- a/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreUseLongLinesTest.php +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreUseLongLinesTest.php @@ -48,4 +48,3 @@ public function testLongUseStatementsAreIgnoredButOtherLongLinesAreDetected(): v ]); } } -