From 1da98a11f1f9e1390ab3f587cd67cea2a02c8ae9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 10 Oct 2011 12:34:28 -0700 Subject: [PATCH] Correctly parse git diffs with an empty file at the end Summary: A minor bug in the parser prevented it from handling git diffs where an empty file appears at the end of the diff. Since git appends an extra newline, we failed to jump into the '$line === null' block. Test Plan: Ran unit tests. Generated a revision which only deleted an empty file. Reviewers: jungejason, nh, tuomaspelkonen, aran Reviewed By: nh CC: aran, nh, epriestley Differential Revision: 998 --- src/parser/diff/ArcanistDiffParser.php | 6 +++++- src/parser/diff/__tests__/ArcanistDiffParserTestCase.php | 6 ++++++ .../__tests__/data/basic-missing-both-newlines-plus.udiff | 0 .../__tests__/data/basic-missing-new-newline-plus.udiff | 0 .../__tests__/data/basic-missing-old-newline-plus.udiff | 0 src/parser/diff/__tests__/data/git-commit.gitdiff | 0 src/parser/diff/__tests__/data/git-copy-plus.gitdiff | 0 src/parser/diff/__tests__/data/git-copy.gitdiff | 0 src/parser/diff/__tests__/data/git-delete-file.gitdiff | 0 src/parser/diff/__tests__/data/git-empty-files.gitdiff | 6 ++++++ .../diff/__tests__/data/git-ignore-whitespace-only.gitdiff | 0 src/parser/diff/__tests__/data/git-merge-header.gitdiff | 0 src/parser/diff/__tests__/data/git-move-edit.gitdiff | 0 src/parser/diff/__tests__/data/git-move-plus.gitdiff | 0 src/parser/diff/__tests__/data/git-move.gitdiff | 0 src/parser/diff/__tests__/data/svn-binary-diff.svndiff | 0 src/parser/diff/__tests__/data/svn-property-delete.svndiff | 0 src/parser/diff/__tests__/data/svn-property-merge.svndiff | 0 src/parser/diff/__tests__/data/svn-property-modify.svndiff | 0 19 files changed, 17 insertions(+), 1 deletion(-) mode change 100755 => 100644 src/parser/diff/__tests__/data/basic-missing-both-newlines-plus.udiff mode change 100755 => 100644 src/parser/diff/__tests__/data/basic-missing-new-newline-plus.udiff mode change 100755 => 100644 src/parser/diff/__tests__/data/basic-missing-old-newline-plus.udiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-commit.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-copy-plus.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-copy.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-delete-file.gitdiff create mode 100644 src/parser/diff/__tests__/data/git-empty-files.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-ignore-whitespace-only.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-merge-header.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-move-edit.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-move-plus.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/git-move.gitdiff mode change 100755 => 100644 src/parser/diff/__tests__/data/svn-binary-diff.svndiff mode change 100755 => 100644 src/parser/diff/__tests__/data/svn-property-delete.svndiff mode change 100755 => 100644 src/parser/diff/__tests__/data/svn-property-merge.svndiff mode change 100755 => 100644 src/parser/diff/__tests__/data/svn-property-modify.svndiff diff --git a/src/parser/diff/ArcanistDiffParser.php b/src/parser/diff/ArcanistDiffParser.php index 28a7dd83..dfd8fda9 100644 --- a/src/parser/diff/ArcanistDiffParser.php +++ b/src/parser/diff/ArcanistDiffParser.php @@ -569,7 +569,11 @@ class ArcanistDiffParser { // $this->didFailParse("Expected 'index af23f...a98bc' header line."); } else { - $line = $this->nextLine(); + // NOTE: In the git case, where this patch is the last change in the + // file, we may have a final terminal newline. Skip over it so that + // we'll hit the '$line === null' block below. This is covered by the + // 'git-empty-file.gitdiff' test case. + $line = $this->nextNonemptyLine(); } } diff --git a/src/parser/diff/__tests__/ArcanistDiffParserTestCase.php b/src/parser/diff/__tests__/ArcanistDiffParserTestCase.php index b69ba362..7e1fdc31 100644 --- a/src/parser/diff/__tests__/ArcanistDiffParserTestCase.php +++ b/src/parser/diff/__tests__/ArcanistDiffParserTestCase.php @@ -412,6 +412,12 @@ EOTEXT $change->getNewProperties() ); break; + case 'git-empty-files.gitdiff': + $this->assertEqual(2, count($changes)); + while ($change = array_shift($changes)) { + $this->assertEqual(0, count($change->getHunks())); + } + break; case 'git-commit.gitdiff': $this->assertEqual(1, count($changes)); $change = reset($changes); diff --git a/src/parser/diff/__tests__/data/basic-missing-both-newlines-plus.udiff b/src/parser/diff/__tests__/data/basic-missing-both-newlines-plus.udiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/basic-missing-new-newline-plus.udiff b/src/parser/diff/__tests__/data/basic-missing-new-newline-plus.udiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/basic-missing-old-newline-plus.udiff b/src/parser/diff/__tests__/data/basic-missing-old-newline-plus.udiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-commit.gitdiff b/src/parser/diff/__tests__/data/git-commit.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-copy-plus.gitdiff b/src/parser/diff/__tests__/data/git-copy-plus.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-copy.gitdiff b/src/parser/diff/__tests__/data/git-copy.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-delete-file.gitdiff b/src/parser/diff/__tests__/data/git-delete-file.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-empty-files.gitdiff b/src/parser/diff/__tests__/data/git-empty-files.gitdiff new file mode 100644 index 00000000..52cd5c32 --- /dev/null +++ b/src/parser/diff/__tests__/data/git-empty-files.gitdiff @@ -0,0 +1,6 @@ +diff --git a/empty b/empty +new file mode 100644 +index 0000000..e69de29 +diff --git a/empty2 b/empty2 +new file mode 100644 +index 0000000..e69de29 diff --git a/src/parser/diff/__tests__/data/git-ignore-whitespace-only.gitdiff b/src/parser/diff/__tests__/data/git-ignore-whitespace-only.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-merge-header.gitdiff b/src/parser/diff/__tests__/data/git-merge-header.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-move-edit.gitdiff b/src/parser/diff/__tests__/data/git-move-edit.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-move-plus.gitdiff b/src/parser/diff/__tests__/data/git-move-plus.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/git-move.gitdiff b/src/parser/diff/__tests__/data/git-move.gitdiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/svn-binary-diff.svndiff b/src/parser/diff/__tests__/data/svn-binary-diff.svndiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/svn-property-delete.svndiff b/src/parser/diff/__tests__/data/svn-property-delete.svndiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/svn-property-merge.svndiff b/src/parser/diff/__tests__/data/svn-property-merge.svndiff old mode 100755 new mode 100644 diff --git a/src/parser/diff/__tests__/data/svn-property-modify.svndiff b/src/parser/diff/__tests__/data/svn-property-modify.svndiff old mode 100755 new mode 100644