fix(ValueObject): FileDiff first line number is below as expected (#6746)

neilime · web-flow · commit b17648099c31 · 2025-02-20T10:52:25.000+01:00
Signed-off-by: Emilien Escalle &lt;emilien.escalle@escemi.com&gt;
diff --git a/e2e/applied-polyfill-php80/expected-output.diff b/e2e/applied-polyfill-php80/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) src/SomeStartWith.php:3
+1) src/SomeStartWith.php:4
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/applied-rule-change-docblock/expected-output.diff b/e2e/applied-rule-change-docblock/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/RenameDocblock.php:0
+1) src/RenameDocblock.php:1
 
     ---------- begin diff ----------
 @@ @@
@@ -19,7 +19,7 @@ Applied rules:
  * RenameClassRector
 
 
-2) src/UselessVarTag.php:1
+2) src/UselessVarTag.php:2
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/applied-rule-removed-node-with-cache/expected-output.diff b/e2e/applied-rule-removed-node-with-cache/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/AlwaysTrue.php:3
+1) src/AlwaysTrue.php:4
 
     ---------- begin diff ----------
 @@ @@
@@ -20,7 +20,7 @@ Applied rules:
  * RemoveAlwaysTrueIfConditionRector
 
 
-2) src/DeadConstructor.php:1
+2) src/DeadConstructor.php:2
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/applied-rule-removed-node/expected-output.diff b/e2e/applied-rule-removed-node/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/AlwaysTrue.php:3
+1) src/AlwaysTrue.php:4
 
     ---------- begin diff ----------
 @@ @@
@@ -20,7 +20,7 @@ Applied rules:
  * RemoveAlwaysTrueIfConditionRector
 
 
-2) src/DeadConstructor.php:1
+2) src/DeadConstructor.php:2
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/applied-rule-return-array-nodes/expected-output.diff b/e2e/applied-rule-return-array-nodes/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/MultiRules.php:5
+1) src/MultiRules.php:6
 
     ---------- begin diff ----------
 @@ @@
@@ -25,7 +25,7 @@ Applied rules:
  * RemoveAlwaysElseRector
 
 
-2) src/RemoveAlwaysElse.php:5
+2) src/RemoveAlwaysElse.php:6
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/different-path-over-skip-config/expected-output.diff b/e2e/different-path-over-skip-config/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) src/models/DeadConstructor.php:3
+1) src/models/DeadConstructor.php:4
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/no-parallel-reflection-resolver/expected-output.diff b/e2e/no-parallel-reflection-resolver/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/NamespacedSomeClassFound.php:5
+1) src/NamespacedSomeClassFound.php:6
 
     ---------- begin diff ----------
 @@ @@
@@ -16,7 +16,7 @@ Applied rules:
  * RemoveUnusedPrivatePropertyRector
 
 
-2) src/SomeClassNotFound.php:3
+2) src/SomeClassNotFound.php:4
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/only-option-quote-double-equalnone/expected-output.diff b/e2e/only-option-quote-double-equalnone/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) ../only-option/src/MultiRules.php:9
+1) ../only-option/src/MultiRules.php:10
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/only-option-quote-single-bsdouble/expected-output.diff b/e2e/only-option-quote-single-bsdouble/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) ../only-option/src/MultiRules.php:9
+1) ../only-option/src/MultiRules.php:10
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/only-option-quote-single-equalnone/expected-output.diff b/e2e/only-option-quote-single-equalnone/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) ../only-option/src/MultiRules.php:9
+1) ../only-option/src/MultiRules.php:10
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/only-option-quote-single/expected-output.diff b/e2e/only-option-quote-single/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) ../only-option/src/MultiRules.php:9
+1) ../only-option/src/MultiRules.php:10
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/only-option/expected-output.diff b/e2e/only-option/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) src/MultiRules.php:9
+1) src/MultiRules.php:10
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/parallel-custom-config/expected-output.diff b/e2e/parallel-custom-config/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) src/SomeClass.php:6
+1) src/SomeClass.php:7
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/parallel-reflection-resolver/expected-output.diff b/e2e/parallel-reflection-resolver/expected-output.diff
@@ -1,7 +1,7 @@
 2 files with changes
 ====================
 
-1) src/NamespacedSomeClassFound.php:5
+1) src/NamespacedSomeClassFound.php:6
 
     ---------- begin diff ----------
 @@ @@
@@ -16,7 +16,7 @@ Applied rules:
  * RemoveUnusedPrivatePropertyRector
 
 
-2) src/SomeClassNotFound.php:3
+2) src/SomeClassNotFound.php:4
 
     ---------- begin diff ----------
 @@ @@
diff --git a/e2e/print-new-node/expected-output.diff b/e2e/print-new-node/expected-output.diff
@@ -1,7 +1,7 @@
 1 file with changes
 ===================
 
-1) src/ExtendingTestClass.php:0
+1) src/ExtendingTestClass.php:1
 
     ---------- begin diff ----------
 @@ @@
diff --git a/rules/Php53/Rector/Ternary/TernaryToElvisRector.php b/rules/Php53/Rector/Ternary/TernaryToElvisRector.php
@@ -70,6 +70,11 @@ public function refactor(Node $node): ?Node
         return $node;
     }
 
+    public function provideMinPhpVersion(): int
+    {
+        return PhpVersionFeature::ELVIS_OPERATOR;
+    }
+
     private function isParenthesized(Expr $ifExpr, Expr $elseExpr): bool
     {
         $tokens = $this->file->getOldTokens();
@@ -95,9 +100,4 @@ private function isParenthesized(Expr $ifExpr, Expr $elseExpr): bool
 
         return false;
     }
-
-    public function provideMinPhpVersion(): int
-    {
-        return PhpVersionFeature::ELVIS_OPERATOR;
-    }
 }
diff --git a/src/ValueObject/Reporting/FileDiff.php b/src/ValueObject/Reporting/FileDiff.php
@@ -15,9 +15,10 @@
 {
     /**
      * @var string
-     * @se https://regex101.com/r/AUPIX4/1
+     * @see https://en.wikipedia.org/wiki/Diff#Unified_format
+     * @see https://regex101.com/r/AUPIX4/1
      */
-    private const FIRST_LINE_REGEX = '#@@(.*?)(?<' . self::FIRST_LINE_KEY . '>\d+)(.*?)@@#';
+    private const DIFF_HUNK_HEADER_REGEX = '#@@(.*?)(?<' . self::FIRST_LINE_KEY . '>\d+)(.*?)@@#';
 
     /**
      * @var string
@@ -93,14 +94,14 @@ public function getRectorClasses(): array
 
     public function getFirstLineNumber(): ?int
     {
-        $match = Strings::match($this->diff, self::FIRST_LINE_REGEX);
+        $match = Strings::match($this->diff, self::DIFF_HUNK_HEADER_REGEX);
 
         // probably some error in diff
         if (! isset($match[self::FIRST_LINE_KEY])) {
             return null;
         }
 
-        return (int) $match[self::FIRST_LINE_KEY] - 1;
+        return (int) $match[self::FIRST_LINE_KEY];
     }
 
     /**
diff --git a/tests/ValueObject/Reporting/FileDiffTest.php b/tests/ValueObject/Reporting/FileDiffTest.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\ValueObject\Reporting;
+
+use PHPUnit\Framework\TestCase;
+use Rector\ValueObject\Reporting\FileDiff;
+
+final class FileDiffTest extends TestCase
+{
+    public function testGetFirstLineNumberShouldReturnFirstLineNumberRegardingHunk(): void
+    {
+        $fileDiff = new FileDiff(
+            'some/file.php',
+            '--- Original\n+++ New\n@@ -38,5 +39,6 @@\nreturn true;\n}\n',
+            'diff console formatted'
+        );
+        $this->assertSame(38, $fileDiff->getFirstLineNumber());
+    }
+
+    public function testGetFirstLineNumberShouldBeNullWhenHunkIsInvalid(): void
+    {
+        $fileDiff = new FileDiff(
+            'some/file.php',
+            '--- Original\n+++ New\n@@@@\nreturn true;\n}\n',
+            'diff console formatted'
+        );
+        $this->assertNull($fileDiff->getFirstLineNumber());
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,11 @@ public function refactor(Node $node): ?Node`
`70`	`70`	`return $node;`
`71`	`71`	`}`
`72`	`72`
	`73`	`+ public function provideMinPhpVersion(): int`
	`74`	`+ {`
	`75`	`+ return PhpVersionFeature::ELVIS_OPERATOR;`
	`76`	`+ }`
	`77`	`+`
`73`	`78`	`private function isParenthesized(Expr $ifExpr, Expr $elseExpr): bool`
`74`	`79`	`{`
`75`	`80`	`$tokens = $this->file->getOldTokens();`
`@@ -95,9 +100,4 @@ private function isParenthesized(Expr $ifExpr, Expr $elseExpr): bool`
`95`	`100`
`96`	`101`	`return false;`
`97`	`102`	`}`
`98`		`-`
`99`		`- public function provideMinPhpVersion(): int`
`100`		`- {`
`101`		`- return PhpVersionFeature::ELVIS_OPERATOR;`
`102`		`- }`
`103`	`103`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,9 +15,10 @@`
`15`	`15`	`{`
`16`	`16`	`/**`
`17`	`17`	`* @var string`
`18`		`- * @se https://regex101.com/r/AUPIX4/1`
	`18`	`+ * @see https://en.wikipedia.org/wiki/Diff#Unified_format`
	`19`	`+ * @see https://regex101.com/r/AUPIX4/1`
`19`	`20`	`*/`
`20`		`- private const FIRST_LINE_REGEX = '#@@(.?)(?<' . self::FIRST_LINE_KEY . '>\d+)(.?)@@#';`
	`21`	`+ private const DIFF_HUNK_HEADER_REGEX = '#@@(.?)(?<' . self::FIRST_LINE_KEY . '>\d+)(.?)@@#';`
`21`	`22`
`22`	`23`	`/**`
`23`	`24`	`* @var string`
`@@ -93,14 +94,14 @@ public function getRectorClasses(): array`
`93`	`94`
`94`	`95`	`public function getFirstLineNumber(): ?int`
`95`	`96`	`{`
`96`		`- $match = Strings::match($this->diff, self::FIRST_LINE_REGEX);`
	`97`	`+ $match = Strings::match($this->diff, self::DIFF_HUNK_HEADER_REGEX);`
`97`	`98`
`98`	`99`	`// probably some error in diff`
`99`	`100`	`if (! isset($match[self::FIRST_LINE_KEY])) {`
`100`	`101`	`return null;`
`101`	`102`	`}`
`102`	`103`
`103`		`- return (int) $match[self::FIRST_LINE_KEY] - 1;`
	`104`	`+ return (int) $match[self::FIRST_LINE_KEY];`
`104`	`105`	`}`
`105`	`106`
`106`	`107`	`/**`