fix error with empty index by including raw information in diff

Author: Patrick-Beuks

Diff for: src/Gitonomy/Git/Commit.php

@@ -62,7 +62,7 @@ public function setData(array $data)
      */
     public function getDiff()
     {
-        $args = ['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index', $this->revision];
+        $args = ['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index', $this->revision];
 
         $diff = Diff::parse($this->repository->run('diff-tree', $args));
         $diff->setRepository($this->repository);

Diff for: src/Gitonomy/Git/Diff/File.php

@@ -278,6 +278,10 @@ public function getOldBlob()
             throw new \LogicException('Can\'t return old Blob on a creation');
         }
 
+        if($this->oldIndex === '') {
+            throw new \RuntimeException('Index is missing to return Blob object.');
+        }
+
         return $this->repository->getBlob($this->oldIndex);
     }
 
@@ -291,6 +295,10 @@ public function getNewBlob()
             throw new \LogicException('Can\'t return new Blob on a deletion');
         }
 
+        if($this->newIndex === '') {
+            throw new \RuntimeException('Index is missing to return Blob object.');
+        }
+
         return $this->repository->getBlob($this->newIndex);
     }
 }

Diff for: src/Gitonomy/Git/Parser/DiffParser.php

@@ -22,14 +22,31 @@ class DiffParser extends ParserBase
     protected function doParse()
     {
         $this->files = [];
+        $indexes = [];
+        // Diff contains raw information
+        if (str_starts_with($this->content, ':')) {
+            while($this->expects(':')) {
+                $this->consumeRegexp('/\d{6} \d{6} /');
+                $oldIndex = $this->consumeShortHash();
+                $this->consume(' ');
+                $newIndex = $this->consumeShortHash();
+                $this->consumeTo("\n");
+                $this->consumeNewLine();
+                $indexes[] = [$oldIndex, $newIndex];
+            }
+            $this->consumeNewLine();
+        } elseif (!$this->isFinished()) {
+            trigger_error('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', E_USER_DEPRECATED);
+        }
 
+        $fileIndex = 0;
         while (!$this->isFinished()) {
             // 1. title
             $vars = $this->consumeRegexp("/diff --git \"?(a\/.*?)\"? \"?(b\/.*?)\"?\n/");
             $oldName = $vars[1];
             $newName = $vars[2];
-            $oldIndex = null;
-            $newIndex = null;
+            $oldIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][0] : null;
+            $newIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][1] : null;
             $oldMode = null;
             $newMode = null;
 

Diff for: src/Gitonomy/Git/Repository.php

@@ -416,7 +416,7 @@ public function getDiff($revisions)
             $revisions = new RevisionList($this, $revisions);
         }
 
-        $args = array_merge(['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray());
+        $args = array_merge(['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray());
 
         $diff = Diff::parse($this->run('diff', $args));
         $diff->setRepository($this);

Diff for: src/Gitonomy/Git/WorkingCopy.php

@@ -50,15 +50,15 @@ public function getUntrackedFiles()
 
     public function getDiffPending()
     {
-        $diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index']));
+        $diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index']));
         $diff->setRepository($this->repository);
 
         return $diff;
     }
 
     public function getDiffStaged()
     {
-        $diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index', '--staged']));
+        $diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index', '--staged']));
         $diff->setRepository($this->repository);
 
         return $diff;

Diff for: tests/Gitonomy/Git/Tests/DiffTest.php

@@ -13,6 +13,9 @@
 namespace Gitonomy\Git\Tests;
 
 use Gitonomy\Git\Diff\Diff;
+use Gitonomy\Git\Diff\File;
+use Gitonomy\Git\Repository;
+use RuntimeException;
 
 class DiffTest extends AbstractTest
 {
@@ -150,4 +153,110 @@ public function testWorksWithUmlauts($repository)
         $files = $repository->getCommit(self::FILE_WITH_UMLAUTS_COMMIT)->getDiff()->getFiles();
         $this->assertSame('file_with_umlauts_\303\244\303\266\303\274', $files[0]->getNewName());
     }
+
+    public function testDeleteFileWithoutRaw()
+    {
+        $deprecationCalled = false;
+        $self = $this;
+        set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void {
+            $deprecationCalled = true;
+            $self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr);
+        }, E_USER_DEPRECATED);
+
+        $diff = Diff::parse(<<<DIFF
+        diff --git a/test b/test
+        deleted file mode 100644
+        index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000
+
+        DIFF);
+        $firstFile = $diff->getFiles()[0];
+
+        restore_exception_handler();
+
+        $this->assertTrue($deprecationCalled);
+        $this->assertFalse($firstFile->isCreation());
+        $this->assertTrue($firstFile->isDeletion());
+        $this->assertFalse($firstFile->isChangeMode());
+        $this->assertSame('e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', $firstFile->getOldIndex());
+        $this->assertNull($firstFile->getNewIndex());
+    }
+
+    public function testModeChangeFileWithoutRaw()
+    {
+        $deprecationCalled = false;
+        $self = $this;
+        set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void {
+            $deprecationCalled = true;
+            $self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr);
+        }, E_USER_DEPRECATED);
+
+        $diff = Diff::parse(<<<DIFF
+        diff --git a/a.out b/a.out
+        old mode 100755
+        new mode 100644
+
+        DIFF);
+        $firstFile = $diff->getFiles()[0];
+
+        restore_exception_handler();
+
+        $this->assertTrue($deprecationCalled);
+        $this->assertFalse($firstFile->isCreation());
+        $this->assertFalse($firstFile->isDeletion());
+        $this->assertTrue($firstFile->isChangeMode());
+        $this->assertSame('', $firstFile->getOldIndex());
+        $this->assertSame('', $firstFile->getNewIndex());
+    }
+
+    public function testModeChangeFileWithRaw()
+    {
+        $deprecationCalled = false;
+        $self = $this;
+        set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void {
+            $deprecationCalled = true;
+        }, E_USER_DEPRECATED);
+
+        $diff = Diff::parse(<<<DIFF
+        :100755 100644 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 M      a.out
+
+        diff --git a/a.out b/a.out
+        old mode 100755
+        new mode 100644
+
+        DIFF);
+        $firstFile = $diff->getFiles()[0];
+
+        restore_exception_handler();
+
+        $this->assertFalse($deprecationCalled);
+        $this->assertFalse($firstFile->isCreation());
+        $this->assertFalse($firstFile->isDeletion());
+        $this->assertTrue($firstFile->isChangeMode());
+        $this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getOldIndex());
+        $this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getNewIndex());
+    }
+
+    public function testThrowErrorOnBlobGetWithoutIndex()
+    {
+        $repository = $this->getMockBuilder(Repository::class)->disableOriginalConstructor()->getMock();
+        $file = new File("oldName", "newName", "100755", "100644", "", '', false);
+        $file->setRepository($repository);
+
+        try {
+            $file->getOldBlob();
+        } catch(\RuntimeException $exception) {
+            $this->assertSame('Index is missing to return Blob object.', $exception->getMessage());
+        }
+        try {
+            $file->getNewBlob();
+        } catch(\RuntimeException $exception) {
+            $this->assertSame('Index is missing to return Blob object.', $exception->getMessage());
+        }
+
+        $this->assertFalse($file->isCreation());
+        $this->assertFalse($file->isDeletion());
+        $this->assertTrue($file->isChangeMode());
+        $this->assertSame('', $file->getOldIndex());
+        $this->assertSame('', $file->getNewIndex());
+    }
 }