mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Provide better parsing primitives for hunks
Summary: Ref T1266. This prepares to fix case (2) on T1266 by improving the robustness of hunk parsing. In particular, the copy detection code abuses this API because it isn't currently expressive or flexible enough. Make it more flexible and cover it exhaustively. I'll move callsites to the new stuff in upcoming revisions. Test Plan: Unit tests. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T1266 Differential Revision: https://secure.phabricator.com/D12144
This commit is contained in:
parent
dcaafd6159
commit
74a4c2cf0b
8 changed files with 478 additions and 15 deletions
|
@ -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 :
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -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;
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
Loading…
Reference in a new issue