Autofix: fix removal + multierror (#10)

* Autofix: fix removal + multierror

* Fix plural

Author: janedbal

src/RuleTestCase.php

@@ -9,6 +9,7 @@
 use function array_filter;
 use function array_map;
 use function array_values;
+use function count;
 use function explode;
 use function file_get_contents;
 use function file_put_contents;
@@ -50,8 +51,10 @@ protected function analyzeFiles(
                 $this->autofix($file, array_values($fileErrors));
             }
 
+            $plural = count($files) > 1 ? 's' : '';
+            $were = count($files) > 1 ? 'were' : 'was';
             $filesStr = implode(', ', $files);
-            self::fail("Files {$filesStr} were autofixed. This setup should never remain in the codebase.");
+            self::fail("File{$plural} {$filesStr} {$were} autofixed. This setup should never remain in the codebase.");
         }
 
         foreach ($files as $file) {
@@ -136,24 +139,30 @@ private function autofix(
                 throw new LogicException('Error without line number: ' . $analyserError->getMessage());
             }
 
-            $errorsByLines[$line] = $analyserError;
+            $errorsByLines[$line][] = $analyserError;
         }
 
         $fileLines = $this->getFileLines($file);
 
         foreach ($fileLines as $line => &$row) {
-            if (!isset($errorsByLines[$line + 1])) {
-                continue;
-            }
-
-            $errorCommentPattern = '~ ?//.*$~';
-            $errorMessage = $errorsByLines[$line + 1]->getMessage();
-            $errorComment = ' // error: ' . $errorMessage;
-
-            if (preg_match($errorCommentPattern, $row) === 1) {
-                $row = preg_replace($errorCommentPattern, $errorComment, $row);
-            } else {
-                $row .= $errorComment;
+            $errorCommentPattern = '~ ?// error:.*$~';
+
+            if (isset($errorsByLines[$line + 1])) {
+                // Line has errors - add or update error comments
+                $errorComments = '';
+
+                foreach ($errorsByLines[$line + 1] as $error) {
+                    $errorComments .= ' // error: ' . $error->getMessage();
+                }
+
+                if (preg_match($errorCommentPattern, $row) === 1) {
+                    $row = preg_replace($errorCommentPattern, $errorComments, $row);
+                } else {
+                    $row .= $errorComments;
+                }
+            } elseif (preg_match($errorCommentPattern, $row) === 1) {
+                // Line has no error but has an error comment - remove it
+                $row = preg_replace($errorCommentPattern, '', $row);
             }
         }
 

tests/Rule/Data/DisallowDivisionByLiteralZeroRule/autofix.expected.php

@@ -0,0 +1,16 @@
+<?php declare(strict_types = 1);
+
+namespace DisallowDivisionByLiteralZeroRule;
+
+function testDivisionAutofix(): void
+{
+    1 / 0; // error: Division by literal zero is not allowed
+    1 / 0; // error: Division by literal zero is not allowed
+    1 / 0; // error: Division by literal zero is not allowed
+    1 / 1;
+    1 / 1;
+
+    ($a / 0) + (5 / 0); // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+    ($a / 0) + (5 / 0); // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+    ($a / 0) + (5 / 0); // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+}

tests/Rule/Data/DisallowDivisionByLiteralZeroRule/autofix.php

@@ -0,0 +1,16 @@
+<?php declare(strict_types = 1);
+
+namespace DisallowDivisionByLiteralZeroRule;
+
+function testDivisionAutofix(): void
+{
+    1 / 0; // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+    1 / 0; // error: Division by literal zero is not allowed
+    1 / 0;
+    1 / 1; // error: Division by literal zero is not allowed
+    1 / 1; // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+
+    ($a / 0) + (5 / 0); // error: Division by literal zero is not allowed // error: Division by literal zero is not allowed
+    ($a / 0) + (5 / 0); // error: Division by literal zero is not allowed
+    ($a / 0) + (5 / 0);
+}

tests/RuleTestCaseTest.php

@@ -3,6 +3,7 @@
 namespace ShipMonkTests\PHPStanDev;
 
 use PHPStan\Rules\Rule;
+use PHPUnit\Framework\AssertionFailedError;
 use ShipMonk\PHPStanDev\RuleTestCase;
 use ShipMonkTests\PHPStanDev\Rule\Data\DisallowDivisionByLiteralZeroRule\DisallowDivisionByLiteralZeroRule;
 
@@ -35,4 +36,21 @@ public function testMultipleErrorsOnSameLine(): void
         $this->analyzeFiles([$testFile]);
     }
 
+    public function testAutofix(): void
+    {
+        $expectedFile = __DIR__ . '/Rule/Data/DisallowDivisionByLiteralZeroRule/autofix.expected.php';
+        $testFile = __DIR__ . '/Rule/Data/DisallowDivisionByLiteralZeroRule/autofix.php';
+        $tmpFile = sys_get_temp_dir() . '/autofix.php';
+        copy($testFile, $tmpFile);
+
+        try {
+            $this->analyzeFiles([$tmpFile], true);
+            self::fail('Autofix should have thrown an exception');
+        } catch (AssertionFailedError $e) { // @phpstan-ignore catch.internalClass
+            self::assertStringContainsString('autofixed', $e->getMessage());
+        }
+
+        self::assertFileEquals($expectedFile, $tmpFile);
+    }
+
 }