diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -1347,12 +1347,10 @@ $copies = array(); foreach ($changeset->getHunks() as $hunk) { $added = $hunk->getStructuredNewFile(); + $atype = array(); foreach ($added as $line => $info) { - if ($info['type'] != '+') { - unset($added[$line]); - continue; - } + $atype[$line] = $info['type']; $added[$line] = trim($info['text']); } @@ -1365,6 +1363,12 @@ continue; } + if ($atype[$line] !== '+') { + // This line hasn't been changed in the new file, so don't try + // to figure out where it came from. + continue; + } + if (empty($map[$code])) { // This line was too short to trigger copy/move detection. continue; diff --git a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php --- a/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php +++ b/src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php @@ -3,17 +3,36 @@ final class DifferentialDiffTestCase extends ArcanistPhutilTestCase { public function testDetectCopiedCode() { + $copies = $this->detectCopiesIn('lint_engine.diff'); + + $this->assertEqual( + array_combine(range(237, 252), range(167, 182)), + ipull($copies, 1)); + } + + public function testDetectCopiedOverlaidCode() { + $copies = $this->detectCopiesIn('copy_overlay.diff'); + + $this->assertEqual( + array( + 7 => 22, + 8 => 23, + 9 => 24, + 10 => 25, + 11 => 26, + 12 => 27, + ), + ipull($copies, 1)); + } + + private function detectCopiesIn($file) { $root = dirname(__FILE__).'/diff/'; $parser = new ArcanistDiffParser(); $diff = DifferentialDiff::newFromRawChanges( PhabricatorUser::getOmnipotentUser(), - $parser->parseDiff(Filesystem::readFile($root.'lint_engine.diff'))); - $copies = idx(head($diff->getChangesets())->getMetadata(), 'copy:lines'); - - $this->assertEqual( - array_combine(range(237, 252), range(167, 182)), - ipull($copies, 1)); + $parser->parseDiff(Filesystem::readFile($root.$file))); + return idx(head($diff->getChangesets())->getMetadata(), 'copy:lines'); } public function testDetectSlowCopiedCode() { diff --git a/src/applications/differential/storage/__tests__/diff/copy_overlay.diff b/src/applications/differential/storage/__tests__/diff/copy_overlay.diff new file mode 100644 --- /dev/null +++ b/src/applications/differential/storage/__tests__/diff/copy_overlay.diff @@ -0,0 +1,41 @@ +diff --git a/test.c b/test.c +index 441f670..5a0aa05 100644 +--- a/test.c ++++ b/test.c +@@ -1,33 +1,28 @@ + // NOTE: Lines must be more than 30 characters long to activate copy/move + // detection. + + if (100000000000000000000000000000) + { + // A + // B + } +-else if (200000000000000000000000000000) ++else if (500000000000000000000000000000) + { +- // C +- // D ++ // I ++ // J + } + else if (300000000000000000000000000000) + { + // E + // F + } + else if (400000000000000000000000000000) + { + // G + // H + } +-else if (500000000000000000000000000000) +-{ +- // I +- // J +-} + else if (600000000000000000000000000000) + { + // K + // L + }