diff --git a/src/applications/differential/storage/DifferentialHunk.php b/src/applications/differential/storage/DifferentialHunk.php index 922db530de..2850bd9f4f 100644 --- a/src/applications/differential/storage/DifferentialHunk.php +++ b/src/applications/differential/storage/DifferentialHunk.php @@ -10,6 +10,8 @@ abstract class DifferentialHunk extends DifferentialDAO protected $newLen; private $changeset; + private $structuredLines; + private $structuredFiles = array(); const FLAG_LINES_ADDED = 1; const FLAG_LINES_REMOVED = 2; @@ -35,6 +37,102 @@ abstract class DifferentialHunk extends DifferentialDAO return implode('', $this->makeContent($include = '-+')); } + public function getStructuredOldFile() { + return $this->getStructuredFile('-'); + } + + public function getStructuredNewFile() { + return $this->getStructuredFile('+'); + } + + private function getStructuredFile($kind) { + if ($kind !== '+' && $kind !== '-') { + throw new Exception( + pht( + 'Structured file kind should be "+" or "-", got "%s".', + $kind)); + } + + if (!isset($this->structuredFiles[$kind])) { + if ($kind == '+') { + $number = $this->newOffset; + } else { + $number = $this->oldOffset; + } + + $lines = $this->getStructuredLines(); + + // NOTE: We keep the "\ No newline at end of file" line if it appears + // after a line which is not excluded. For example, if we're constructing + // the "+" side of the diff, we want to ignore this one since it's + // relevant only to the "-" side of the diff: + // + // - x + // \ No newline at end of file + // + x + // + // ...but we want to keep this one: + // + // - x + // + x + // \ No newline at end of file + + $file = array(); + $keep = true; + foreach ($lines as $line) { + switch ($line['type']) { + case ' ': + case $kind: + $file[$number++] = $line; + $keep = true; + break; + case '\\': + if ($keep) { + // Strip the actual newline off the line's text. + $text = $file[$number - 1]['text']; + $text = rtrim($text, "\r\n"); + $file[$number - 1]['text'] = $text; + + $file[$number++] = $line; + $keep = false; + } + break; + default: + $keep = false; + break; + } + } + + $this->structuredFiles[$kind] = $file; + } + + return $this->structuredFiles[$kind]; + } + + private function getStructuredLines() { + if ($this->structuredLines === null) { + $lines = phutil_split_lines($this->getChanges()); + + $structured = array(); + foreach ($lines as $line) { + if (empty($line[0])) { + // TODO: Can we just get rid of this? + continue; + } + + $structured[] = array( + 'type' => $line[0], + 'text' => substr($line, 1), + ); + } + + $this->structuredLines = $structured; + } + + return $this->structuredLines; + } + + public function getContentWithMask($mask) { $include = array(); @@ -59,21 +157,6 @@ abstract class DifferentialHunk extends DifferentialDAO $results = array(); $lines = explode("\n", $this->getChanges()); - // NOTE: To determine whether the recomposed file should have a trailing - // newline, we look for a "\ No newline at end of file" line which appears - // after a line which we don't exclude. For example, if we're constructing - // the "new" side of a diff (excluding "-"), we want to ignore this one: - // - // - x - // \ No newline at end of file - // + x - // - // ...since it's talking about the "old" side of the diff, but interpret - // this as meaning we should omit the newline: - // - // - x - // + x - // \ No newline at end of file $n = (strpos($include, '+') !== false ? $this->newOffset : diff --git a/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php b/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php index acb23709f7..2fcd7a9427 100644 --- a/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php +++ b/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php @@ -34,4 +34,320 @@ final class DifferentialHunkTestCase extends ArcanistPhutilTestCase { } + public function testMakeStructuredChanges1() { + $hunk = $this->loadHunk('fruit1.diff'); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "apple\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "cherry\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "apple\n", + ), + 2 => array( + 'type' => '+', + 'text' => "banana\n", + ), + 3 => array( + 'type' => '+', + 'text' => "plum\n", + ), + 4 => array( + 'type' => ' ', + 'text' => "cherry\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + public function testMakeStructuredChanges2() { + $hunk = $this->loadHunk('fruit2.diff'); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "apple\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "banana\n", + ), + 3 => array( + 'type' => '-', + 'text' => "plum\n", + ), + 4 => array( + 'type' => ' ', + 'text' => "cherry\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "apple\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "banana\n", + ), + 3 => array( + 'type' => ' ', + 'text' => "cherry\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + public function testMakeStructuredNewlineAdded() { + $hunk = $this->loadHunk('trailing_newline_added.diff'); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => '-', + 'text' => 'quack', + ), + 4 => array( + 'type' => '\\', + 'text' => " No newline at end of file\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => '+', + 'text' => "quack\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + public function testMakeStructuredNewlineRemoved() { + $hunk = $this->loadHunk('trailing_newline_removed.diff'); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => '-', + 'text' => "quack\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 1 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => '+', + 'text' => 'quack', + ), + 4 => array( + 'type' => '\\', + 'text' => " No newline at end of file\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + public function testMakeStructuredNewlineAbsent() { + $hunk = $this->loadHunk('trailing_newline_absent.diff'); + + $this->assertEqual( + array( + 1 => array( + 'type' => '-', + 'text' => "quack\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => ' ', + 'text' => 'quack', + ), + 4 => array( + 'type' => '\\', + 'text' => " No newline at end of file\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 1 => array( + 'type' => '+', + 'text' => "meow\n", + ), + 2 => array( + 'type' => ' ', + 'text' => "quack\n", + ), + 3 => array( + 'type' => ' ', + 'text' => 'quack', + ), + 4 => array( + 'type' => '\\', + 'text' => " No newline at end of file\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + public function testMakeStructuredOffset() { + $hunk = $this->loadHunk('offset.diff'); + + $this->assertEqual( + array( + 76 => array( + 'type' => ' ', + 'text' => " \$bits .= '0';\n", + ), + 77 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 78 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 79 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 80 => array( + 'type' => '-', + 'text' => " \$this->bits = \$bits;\n", + ), + 81 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 82 => array( + 'type' => ' ', + 'text' => " return \$this->bits;\n", + ), + ), + $hunk->getStructuredOldFile()); + + $this->assertEqual( + array( + 76 => array( + 'type' => ' ', + 'text' => " \$bits .= '0';\n", + ), + 77 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 78 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 79 => array( + 'type' => '+', + 'text' => " break;\n", + ), + 80 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 81 => array( + 'type' => '+', + 'text' => " \$this->bits = \$bytes;\n", + ), + 82 => array( + 'type' => ' ', + 'text' => " }\n", + ), + 83 => array( + 'type' => ' ', + 'text' => " return \$this->bits;\n", + ), + ), + $hunk->getStructuredNewFile()); + } + + private function loadHunk($name) { + $root = dirname(__FILE__).'/hunk/'; + $data = Filesystem::readFile($root.$name); + + $parser = new ArcanistDiffParser(); + $changes = $parser->parseDiff($data); + + $viewer = PhabricatorUser::getOmnipotentUser(); + $diff = DifferentialDiff::newFromRawChanges($viewer, $changes); + + $changesets = $diff->getChangesets(); + if (count($changesets) !== 1) { + throw new Exception( + pht( + 'Expected exactly one changeset from "%s".', + $name)); + } + $changeset = head($changesets); + + $hunks = $changeset->getHunks(); + if (count($hunks) !== 1) { + throw new Exception( + pht( + 'Expected exactly one hunk from "%s".', + $name)); + } + $hunk = head($hunks); + + return $hunk; + } + + } diff --git a/src/applications/differential/storage/__tests__/hunk/fruit1.diff b/src/applications/differential/storage/__tests__/hunk/fruit1.diff new file mode 100644 index 0000000000..d1d3ad7253 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/fruit1.diff @@ -0,0 +1,9 @@ +diff --git a/fruit b/fruit +index 29b651e..ef05da5 100644 +--- a/fruit ++++ b/fruit +@@ -1,2 +1,4 @@ + apple ++banana ++plum + cherry diff --git a/src/applications/differential/storage/__tests__/hunk/fruit2.diff b/src/applications/differential/storage/__tests__/hunk/fruit2.diff new file mode 100644 index 0000000000..7c387d74d5 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/fruit2.diff @@ -0,0 +1,9 @@ +diff --git a/fruit b/fruit +index ef05da5..fde8dcd 100644 +--- a/fruit ++++ b/fruit +@@ -1,4 +1,3 @@ + apple + banana +-plum + cherry diff --git a/src/applications/differential/storage/__tests__/hunk/offset.diff b/src/applications/differential/storage/__tests__/hunk/offset.diff new file mode 100644 index 0000000000..1108ea87d0 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/offset.diff @@ -0,0 +1,16 @@ +diff --git a/src/ip/PhutilIPAddress.php b/src/ip/PhutilIPAddress.php +index 47c8f2e..0e216d2 100644 +--- a/src/ip/PhutilIPAddress.php ++++ b/src/ip/PhutilIPAddress.php +@@ -76,9 +76,10 @@ final class PhutilIPAddress extends Phobject { + $bits .= '0'; + } + } ++ break; + } + +- $this->bits = $bits; ++ $this->bits = $bytes; + } + + return $this->bits; diff --git a/src/applications/differential/storage/__tests__/hunk/trailing_newline_absent.diff b/src/applications/differential/storage/__tests__/hunk/trailing_newline_absent.diff new file mode 100644 index 0000000000..5dcb06cce9 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/trailing_newline_absent.diff @@ -0,0 +1,10 @@ +diff --git a/newliney b/newliney +index 82ab3ce..ba1dfb1 100644 +--- a/newliney ++++ b/newliney +@@ -1,3 +1,3 @@ +-quack ++meow + quack + quack +\ No newline at end of file diff --git a/src/applications/differential/storage/__tests__/hunk/trailing_newline_added.diff b/src/applications/differential/storage/__tests__/hunk/trailing_newline_added.diff new file mode 100644 index 0000000000..c36ddb33d2 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/trailing_newline_added.diff @@ -0,0 +1,10 @@ +diff --git a/newliney b/newliney +index 82ab3ce..8f44286 100644 +--- a/newliney ++++ b/newliney +@@ -1,3 +1,3 @@ + quack + quack +-quack +\ No newline at end of file ++quack diff --git a/src/applications/differential/storage/__tests__/hunk/trailing_newline_removed.diff b/src/applications/differential/storage/__tests__/hunk/trailing_newline_removed.diff new file mode 100644 index 0000000000..efbec5efc2 --- /dev/null +++ b/src/applications/differential/storage/__tests__/hunk/trailing_newline_removed.diff @@ -0,0 +1,10 @@ +diff --git a/newliney b/newliney +index 8f44286..82ab3ce 100644 +--- a/newliney ++++ b/newliney +@@ -1,3 +1,3 @@ + quack + quack +-quack ++quack +\ No newline at end of file