From e9be1fa05a3f38d8e6e1f2ad8220abf53c88bf59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Kr=C3=A4mer?= Date: Thu, 4 Dec 2025 15:45:01 +0100 Subject: [PATCH] Add Max Line Length Rule enhancements and tests - Introduced new options for ignoring specific line types (use statements, namespace declarations, docblocks) in the Max Line Length Rule. - Updated the rule implementation to handle these new options effectively. - Added multiple test cases to ensure proper functionality of the new features, including backward compatibility for the ignoreUseStatements parameter. - Created tests to validate detection and ignoring of long lines in docblocks and namespaces based on the new configuration options. --- data/MaxLineLengthLongDocBlockClass.php | 24 +++ data/MaxLineLengthLongNamespaceClass.php | 17 +++ docs/rules/Max-Line-Length-Rule.md | 58 +++++++- src/CleanCode/MaxLineLengthRule.php | 138 ++++++++++++++++-- .../MaxLineLengthRuleArrayApiTest.php | 44 ++++++ ...ineLengthRuleBackwardCompatibilityTest.php | 44 ++++++ ...eLengthRuleDetectDocBlockLongLinesTest.php | 58 ++++++++ ...LengthRuleDetectNamespaceLongLinesTest.php | 48 ++++++ ...eLengthRuleIgnoreDocBlockLongLinesTest.php | 44 ++++++ ...LengthRuleIgnoreNamespaceLongLinesTest.php | 44 ++++++ 10 files changed, 502 insertions(+), 17 deletions(-) create mode 100644 data/MaxLineLengthLongDocBlockClass.php create mode 100644 data/MaxLineLengthLongNamespaceClass.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreDocBlockLongLinesTest.php create mode 100644 tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreNamespaceLongLinesTest.php diff --git a/data/MaxLineLengthLongDocBlockClass.php b/data/MaxLineLengthLongDocBlockClass.php new file mode 100644 index 0000000..2942433 --- /dev/null +++ b/data/MaxLineLengthLongDocBlockClass.php @@ -0,0 +1,24 @@ + */ @@ -36,6 +40,10 @@ class MaxLineLengthRule implements Rule private bool $ignoreUseStatements; + private bool $ignoreNamespaceDeclaration; + + private bool $ignoreDocBlocks; + /** * @var array> */ @@ -52,14 +60,35 @@ class MaxLineLengthRule implements Rule */ private array $useStatementLines = []; + /** + * @var array> + * Cache of which lines contain namespace statements per file + */ + private array $namespaceLines = []; + + /** + * @var array> + * Cache of which lines contain docblock comments per file + */ + private array $docBlockLines = []; + /** * @param string[] $excludePatterns + * @param array $ignoreLineTypes Array of line types to ignore (e.g., ['useStatements' => true, 'namespaceDeclaration' => true, 'docBlocks' => true]) */ - public function __construct(int $maxLineLength, array $excludePatterns = [], bool $ignoreUseStatements = false) - { + public function __construct( + int $maxLineLength, + array $excludePatterns = [], + bool $ignoreUseStatements = false, + array $ignoreLineTypes = [] + ) { $this->maxLineLength = $maxLineLength; $this->excludePatterns = $excludePatterns; - $this->ignoreUseStatements = $ignoreUseStatements; + + // BC: ignoreUseStatements parameter takes precedence over array when both are set + $this->ignoreUseStatements = $ignoreUseStatements ?: ($ignoreLineTypes['useStatements'] ?? false); + $this->ignoreNamespaceDeclaration = $ignoreLineTypes['namespaceDeclaration'] ?? false; + $this->ignoreDocBlocks = $ignoreLineTypes['docBlocks'] ?? false; } public function getNodeType(): string @@ -77,13 +106,18 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { + // Skip PHPStan-specific nodes that don't represent actual PHP code + if ($node instanceof FileNode) { + return []; + } + // Skip if file should be excluded if ($this->shouldExcludeFile($scope)) { return []; } $filePath = $scope->getFile(); - $lineNumber = $node->getLine(); + $lineNumber = $node->getStartLine(); // Track use statement lines for this file if ($node instanceof Use_) { @@ -95,11 +129,65 @@ public function processNode(Node $node, Scope $scope): array } } + // Track namespace statement lines for this file + if ($node instanceof Namespace_) { + // 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 []; + } + } + + // Handle docblock lines for this node + $errors = []; + $docComment = $node->getDocComment(); + 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++) { + // Skip if we've already processed this line + if ($this->isLineProcessed($filePath, $line)) { + continue; + } + + $lineLength = $this->getLineLength($filePath, $line); + if ($lineLength > $this->maxLineLength) { + $this->markLineAsProcessed($filePath, $line); + $errors[] = RuleErrorBuilder::message($this->buildErrorMessage($line, $lineLength)) + ->identifier(self::IDENTIFIER) + ->line($line) + ->build(); + } + } + } + } + // Skip if this line is a use statement and we're ignoring them if ($this->ignoreUseStatements && $this->isUseStatementLine($filePath, $lineNumber)) { return []; } + // Skip if this line is a namespace and we're ignoring them + if ($this->ignoreNamespaceDeclaration && $this->isNamespaceLine($filePath, $lineNumber)) { + return []; + } + + // Skip if this line is a docblock and we're ignoring them + if ($this->ignoreDocBlocks && $this->isDocBlockLine($filePath, $lineNumber)) { + return []; + } + // Skip if we've already processed this line if ($this->isLineProcessed($filePath, $lineNumber)) { return []; @@ -111,15 +199,13 @@ public function processNode(Node $node, Scope $scope): array if ($lineLength > $this->maxLineLength) { $this->markLineAsProcessed($filePath, $lineNumber); - return [ - RuleErrorBuilder::message($this->buildErrorMessage($lineNumber, $lineLength)) - ->identifier(self::IDENTIFIER) - ->line($lineNumber) - ->build() - ]; + $errors[] = RuleErrorBuilder::message($this->buildErrorMessage($lineNumber, $lineLength)) + ->identifier(self::IDENTIFIER) + ->line($lineNumber) + ->build(); } - return []; + return $errors; } private function shouldExcludeFile(Scope $scope): bool @@ -167,6 +253,34 @@ private function markLineAsUseStatement(string $filePath, int $lineNumber): void $this->useStatementLines[$filePath][$lineNumber] = true; } + private function isNamespaceLine(string $filePath, int $lineNumber): bool + { + return isset($this->namespaceLines[$filePath][$lineNumber]); + } + + private function markLineAsNamespace(string $filePath, int $lineNumber): void + { + if (!isset($this->namespaceLines[$filePath])) { + $this->namespaceLines[$filePath] = []; + } + + $this->namespaceLines[$filePath][$lineNumber] = true; + } + + private function isDocBlockLine(string $filePath, int $lineNumber): bool + { + return isset($this->docBlockLines[$filePath][$lineNumber]); + } + + private function markLineAsDocBlock(string $filePath, int $lineNumber): void + { + if (!isset($this->docBlockLines[$filePath])) { + $this->docBlockLines[$filePath] = []; + } + + $this->docBlockLines[$filePath][$lineNumber] = true; + } + private function getLineLength(string $filePath, int $lineNumber): int { // Cache file line lengths to avoid reading the same file multiple times diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php new file mode 100644 index 0000000..d1ef9e5 --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleArrayApiTest.php @@ -0,0 +1,44 @@ + + */ +class MaxLineLengthRuleArrayApiTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Use the new array API to ignore use statements via the array + return new MaxLineLengthRule(80, [], false, ['useStatements' => true]); + } + + /** + * Test that the new array API works for useStatements + */ + public function testArrayApiForUseStatements(): void + { + // Lines 5, 6, 7 have use statements that exceed 80 characters - should be ignored + // Line 16 has a method signature that exceeds 80 characters - should be detected + // Line 18 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongUseStatementsClass.php'], [ + [ + 'Line 16 exceeds the maximum length of 80 characters (found 117 characters).', + 16, + ], + [ + 'Line 18 exceeds the maximum length of 80 characters (found 114 characters).', + 18, + ], + ]); + } +} + diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php new file mode 100644 index 0000000..b074834 --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleBackwardCompatibilityTest.php @@ -0,0 +1,44 @@ + + */ +class MaxLineLengthRuleBackwardCompatibilityTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Use the old API: 3rd parameter for ignoreUseStatements + return new MaxLineLengthRule(80, [], true); + } + + /** + * Test that the old ignoreUseStatements parameter still works (backward compatibility) + */ + public function testBackwardCompatibilityWithOldIgnoreUseStatementsParameter(): void + { + // Lines 5, 6, 7 have use statements that exceed 80 characters - should be ignored (BC) + // Line 16 has a method signature that exceeds 80 characters - should be detected + // Line 18 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongUseStatementsClass.php'], [ + [ + 'Line 16 exceeds the maximum length of 80 characters (found 117 characters).', + 16, + ], + [ + 'Line 18 exceeds the maximum length of 80 characters (found 114 characters).', + 18, + ], + ]); + } +} + diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php new file mode 100644 index 0000000..2429d2b --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectDocBlockLongLinesTest.php @@ -0,0 +1,58 @@ + + */ +class MaxLineLengthRuleDetectDocBlockLongLinesTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Do NOT ignore docblocks (docBlocks is false/default) + return new MaxLineLengthRule(80, [], false, ['docBlocks' => false]); + } + + /** + * Test that long docblock lines ARE detected when ignoreDocBlocks is false + */ + public function testLongDocBlockLinesAreDetectedWhenNotIgnored(): void + { + // Line 6 has a long docblock line - should be detected + // Line 7 has a long docblock line - should be detected + // Line 17 has a long docblock line - should be detected + // Line 19 has a method signature that exceeds 80 characters - should be detected + // Line 21 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongDocBlockClass.php'], [ + [ + 'Line 6 exceeds the maximum length of 80 characters (found 111 characters).', + 6, + ], + [ + 'Line 7 exceeds the maximum length of 80 characters (found 113 characters).', + 7, + ], + [ + 'Line 17 exceeds the maximum length of 80 characters (found 121 characters).', + 17, + ], + [ + 'Line 19 exceeds the maximum length of 80 characters (found 117 characters).', + 19, + ], + [ + 'Line 21 exceeds the maximum length of 80 characters (found 114 characters).', + 21, + ], + ]); + } +} + diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php new file mode 100644 index 0000000..ebe1a2d --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleDetectNamespaceLongLinesTest.php @@ -0,0 +1,48 @@ + + */ +class MaxLineLengthRuleDetectNamespaceLongLinesTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Do NOT ignore namespaces (namespaceDeclaration is false/default) + return new MaxLineLengthRule(80, [], false, ['namespaceDeclaration' => false]); + } + + /** + * Test that long namespace declarations ARE detected when ignoreNamespaces is false + */ + public function testLongNamespacesAreDetectedWhenNotIgnored(): void + { + // Line 3 has namespace that exceeds 80 characters - should be detected + // Line 12 has a method signature that exceeds 80 characters - should be detected + // Line 14 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongNamespaceClass.php'], [ + [ + 'Line 3 exceeds the maximum length of 80 characters (found 101 characters).', + 3, + ], + [ + 'Line 12 exceeds the maximum length of 80 characters (found 117 characters).', + 12, + ], + [ + 'Line 14 exceeds the maximum length of 80 characters (found 114 characters).', + 14, + ], + ]); + } +} + diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreDocBlockLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreDocBlockLongLinesTest.php new file mode 100644 index 0000000..a3dca79 --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreDocBlockLongLinesTest.php @@ -0,0 +1,44 @@ + + */ +class MaxLineLengthRuleIgnoreDocBlockLongLinesTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Ignore docblocks (docBlocks is true) + return new MaxLineLengthRule(80, [], false, ['docBlocks' => true]); + } + + /** + * Test that long docblock lines are ignored when ignoreDocBlocks is true, + * but other long lines are still detected. + */ + public function testLongDocBlockLinesAreIgnoredButOtherLongLinesAreDetected(): void + { + // Lines 6, 7, 17 have long docblock lines - should be ignored + // Line 19 has a method signature that exceeds 80 characters - should be detected + // Line 21 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongDocBlockClass.php'], [ + [ + 'Line 19 exceeds the maximum length of 80 characters (found 117 characters).', + 19, + ], + [ + 'Line 21 exceeds the maximum length of 80 characters (found 114 characters).', + 21, + ], + ]); + } +} diff --git a/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreNamespaceLongLinesTest.php b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreNamespaceLongLinesTest.php new file mode 100644 index 0000000..16f695e --- /dev/null +++ b/tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreNamespaceLongLinesTest.php @@ -0,0 +1,44 @@ + + */ +class MaxLineLengthRuleIgnoreNamespaceLongLinesTest extends RuleTestCase +{ + protected function getRule(): Rule + { + // Ignore namespaces (namespaceDeclaration is true) + return new MaxLineLengthRule(80, [], false, ['namespaceDeclaration' => true]); + } + + /** + * Test that long namespace declarations are ignored when ignoreNamespaces is true, + * but other long lines are still detected. + */ + public function testLongNamespacesAreIgnoredButOtherLongLinesAreDetected(): void + { + // Line 3 has namespace that exceeds 80 characters - should be ignored + // Line 12 has a method signature that exceeds 80 characters - should be detected + // Line 14 has a variable assignment that exceeds 80 characters - should be detected + $this->analyse([__DIR__ . '/../../../data/MaxLineLengthLongNamespaceClass.php'], [ + [ + 'Line 12 exceeds the maximum length of 80 characters (found 117 characters).', + 12, + ], + [ + 'Line 14 exceeds the maximum length of 80 characters (found 114 characters).', + 14, + ], + ]); + } +}