Fixing MaxLineLength Rule (#22)

Author: floriankraemer

data/MaxLineLengthLongUseStatementsClass.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App;
+
+use Some\Very\Long\Namespace\That\Definitely\Exceeds\Eighty\Characters\SomeVeryLongClassName;
+use Another\Very\Long\Namespace\That\Also\Exceeds\The\Maximum\Line\Length\Limit\AnotherClass;
+use YetAnother\Very\Long\Namespace\Path\That\Goes\On\And\On\Until\It\Exceeds\EightyCharacters;
+
+class MaxLineLengthLongUseStatementsClass
+{
+    public function shortMethod(): void
+    {
+        $variable = 'short';
+    }
+
+    public function methodWithVeryLongLineInsideThatDefinitelyExceedsTheEightyCharacterMaximumLineLengthLimit(): void
+    {
+        $thisVariableNameIsAlsoExtremelyLongAndWillDefinitelyExceedTheMaximumLineLengthLimitOf80Characters = true;
+    }
+}
+

src/CleanCode/MaxLineLengthRule.php

@@ -46,6 +46,12 @@ class MaxLineLengthRule implements Rule
      */
     private array $processedLines = [];
 
+    /**
+     * @var array<string, array<int, bool>>
+     * Cache of which lines contain use statements per file
+     */
+    private array $useStatementLines = [];
+
     /**
      * @param string[] $excludePatterns
      */
@@ -76,14 +82,24 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        // Skip use statements if configured to ignore them
-        if ($this->ignoreUseStatements && $node instanceof Use_) {
-            return [];
-        }
-
         $filePath = $scope->getFile();
         $lineNumber = $node->getLine();
 
+        // 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 [];
+            }
+        }
+
+        // Skip if this line is a use statement and we're ignoring them
+        if ($this->ignoreUseStatements && $this->isUseStatementLine($filePath, $lineNumber)) {
+            return [];
+        }
+
         // Skip if we've already processed this line
         if ($this->isLineProcessed($filePath, $lineNumber)) {
             return [];
@@ -137,6 +153,20 @@ private function markLineAsProcessed(string $filePath, int $lineNumber): void
         $this->processedLines[$filePath][$lineNumber] = true;
     }
 
+    private function isUseStatementLine(string $filePath, int $lineNumber): bool
+    {
+        return isset($this->useStatementLines[$filePath][$lineNumber]);
+    }
+
+    private function markLineAsUseStatement(string $filePath, int $lineNumber): void
+    {
+        if (!isset($this->useStatementLines[$filePath])) {
+            $this->useStatementLines[$filePath] = [];
+        }
+
+        $this->useStatementLines[$filePath][$lineNumber] = true;
+    }
+
     private function getLineLength(string $filePath, int $lineNumber): int
     {
         // Cache file line lengths to avoid reading the same file multiple times

tests/TestCases/CleanCode/MaxLineLengthRuleDetectUseLongLinesTest.php

@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Phauthentic\PHPStanRules\Tests\TestCases\CleanCode;
+
+use Phauthentic\PHPStanRules\CleanCode\MaxLineLengthRule;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * 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<MaxLineLengthRule>
+ */
+class MaxLineLengthRuleDetectUseLongLinesTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        // Do NOT ignore use statements (3rd parameter is false/default)
+        return new MaxLineLengthRule(80, [], false);
+    }
+
+    /**
+     * Test that long use statements ARE detected when ignoreUseStatements is false
+     */
+    public function testLongUseStatementsAreDetectedWhenNotIgnored(): void
+    {
+        // Lines 5, 6, 7 have use statements that exceed 80 characters - should be detected
+        // 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 5 exceeds the maximum length of 80 characters (found 93 characters).',
+                5,
+            ],
+            [
+                'Line 6 exceeds the maximum length of 80 characters (found 93 characters).',
+                6,
+            ],
+            [
+                'Line 7 exceeds the maximum length of 80 characters (found 94 characters).',
+                7,
+            ],
+            [
+                '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,
+            ],
+        ]);
+    }
+}
+

tests/TestCases/CleanCode/MaxLineLengthRuleIgnoreUseLongLinesTest.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Phauthentic\PHPStanRules\Tests\TestCases\CleanCode;
+
+use Phauthentic\PHPStanRules\CleanCode\MaxLineLengthRule;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * Regression test for ignoreUseStatements feature
+ *
+ * This test ensures that when use statements exceed the line length limit,
+ * they are properly ignored when ignoreUseStatements is true.
+ *
+ * @extends RuleTestCase<MaxLineLengthRule>
+ */
+class MaxLineLengthRuleIgnoreUseLongLinesTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        // Ignore use statements (3rd parameter is true)
+        return new MaxLineLengthRule(80, [], true);
+    }
+
+    /**
+     * Test that long use statements are ignored when ignoreUseStatements is true,
+     * but other long lines are still detected.
+     *
+     * This is a regression test for a bug where use statements were not properly
+     * ignored because the check only looked at the node type, not the line itself.
+     */
+    public function testLongUseStatementsAreIgnoredButOtherLongLinesAreDetected(): 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,
+            ],
+        ]);
+    }
+}
+