revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 08:42:41 +01:00
Code Issues Releases Wiki Activity

Don't skip even lines in copied code detector

Browse source 
Summary: PHP 3 basics: `each()` advances internal pointer so calling `next()` is wrong.

Test Plan: New test.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4011
This commit is contained in:
vrana 2012-11-21 12:30:38 -08:00
parent 254678d41d
commit 4d7b441834
4 changed files with 71 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__phutil_library_map__.php
View file

@ -245,6 +245,7 @@ phutil_register_library_map(array(
'DifferentialDiffCreateController' => 'applications/differential/controller/DifferentialDiffCreateController.php',
'DifferentialDiffProperty' => 'applications/differential/storage/DifferentialDiffProperty.php',
'DifferentialDiffTableOfContentsView' => 'applications/differential/view/DifferentialDiffTableOfContentsView.php',
'DifferentialDiffTestCase' => 'applications/differential/storage/__tests__/DifferentialDiffTestCase.php',
'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php',
'DifferentialException' => 'applications/differential/exception/DifferentialException.php',
'DifferentialExceptionMail' => 'applications/differential/mail/DifferentialExceptionMail.php',
@ -1502,6 +1503,7 @@ phutil_register_library_map(array(
'DifferentialDiffCreateController' => 'DifferentialController',
'DifferentialDiffProperty' => 'DifferentialDAO',
'DifferentialDiffTableOfContentsView' => 'AphrontView',
'DifferentialDiffTestCase' => 'ArcanistPhutilTestCase',
'DifferentialDiffViewController' => 'DifferentialController',
'DifferentialException' => 'Exception',
'DifferentialExceptionMail' => 'DifferentialMail',

2
src/applications/differential/storage/DifferentialDiff.php
View file

@ -204,7 +204,7 @@ final class DifferentialDiff extends DifferentialDAO {
$copies = array();
foreach ($changeset->getHunks() as $hunk) {
$added = array_map('trim', $hunk->getAddedLines());
for (reset($added); list($line, $code) = each($added); next($added)) {
for (reset($added); list($line, $code) = each($added); ) {
if (isset($map[$code])) { // We found a long matching line.
$best_length = 0;
foreach ($map[$code] as $val) { // Explore all candidates.

18
src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php Normal file
View file

@ -0,0 +1,18 @@
<?php
final class DifferentialDiffTestCase extends ArcanistPhutilTestCase {
public function testDetectCopiedCode() {
$root = dirname(__FILE__).'/diff/';
$parser = new ArcanistDiffParser();
$diff = DifferentialDiff::newFromRawChanges(
$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));
}
}

50
src/applications/differential/storage/__tests__/diff/lint_engine.diff Normal file
View file

@ -0,0 +1,50 @@
diff --git a/ArcanistLintEngine.php b/ArcanistLintEngine.php
index 41e1153..255b049 100644
--- a/ArcanistLintEngine.php
+++ b/ArcanistLintEngine.php
@@ -165,22 +165,6 @@ abstract class ArcanistLintEngine {
$stopped = array();
$linters = $this->buildLinters();
- if (!$linters) {
- throw new ArcanistNoEffectException("No linters to run.");
- }
-
- $have_paths = false;
- foreach ($linters as $linter) {
- if ($linter->getPaths()) {
- $have_paths = true;
- break;
- }
- }
-
- if (!$have_paths) {
- throw new ArcanistNoEffectException("No paths are lintable.");
- }
-
$exceptions = array();
foreach ($linters as $linter_name => $linter) {
try {
@@ -251,6 +235,22 @@ abstract class ArcanistLintEngine {
}
}
+ if (!$linters) {
+ throw new ArcanistNoEffectException("No linters to run.");
+ }
+
+ $have_paths = false;
+ foreach ($linters as $linter) {
+ if ($linter->getPaths()) {
+ $have_paths = true;
+ break;
+ }
+ }
+
+ if (!$have_paths) {
+ throw new ArcanistNoEffectException("No paths are lintable.");
+ }
+
if ($exceptions) {
throw new PhutilAggregateException('Some linters failed:', $exceptions);
}