Skip to content

Commit

Permalink
Fix a similar lint rendering issue when trimming identical lines out …
Browse files Browse the repository at this point in the history
…of patches

Summary: Ref T9846. See PHI48. See D18538 for a similar fix. We can contract the suffix lines too much if, e.g, a newline after another newline is removed. Prevent contraction to fewer than 0 lines.

Test Plan: Added a failing test, made it pass.

Reviewers: chad

Reviewed By: chad

Subscribers: alexmv

Maniphest Tasks: T9846

Differential Revision: https://secure.phabricator.com/D18541
  • Loading branch information
epriestley committed Sep 6, 2017
1 parent 7371277 commit cbc785d
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 7 deletions.
11 changes: 4 additions & 7 deletions src/lint/renderer/ArcanistConsoleLintRenderer.php
Expand Up @@ -171,13 +171,10 @@ protected function renderContext(
$old_impact--;
$new_impact--;

if ($old_impact < 0 || $new_impact < 0) {
throw new Exception(
pht(
'Modified suffix line range has become negative '.
'(old = %d, new = %d).',
$old_impact,
$new_impact));
// We can end up here if a patch removes a line which occurs after
// another identical line.
if ($old_impact <= 0 || $new_impact <= 0) {
break;
}
} while (true);

Expand Down
Expand Up @@ -15,6 +15,23 @@ public function testRendering() {
import apple;
import banana;
import cat;
import dog;
EOTEXT;

$remline_original = <<<EOTEXT
import apple;
import banana;
import cat;
import dog;
EOTEXT;

$remline_replacement = <<<EOTEXT
import apple;
import banana;
import cat;
import dog;
EOTEXT;
Expand Down Expand Up @@ -89,6 +106,13 @@ public function testRendering() {
'original' => $midline_original,
'replacement' => $midline_replacement,
),

'remline' => array(
'line' => 1,
'char' => 1,
'original' => $remline_original,
'replacement' => $remline_replacement,
),
);

$defaults = array(
Expand Down
12 changes: 12 additions & 0 deletions src/lint/renderer/__tests__/data/remline.expect
@@ -0,0 +1,12 @@
>>> Lint for path/to/example.c:


Warning (WARN123) Lint Warning
Consider this.

1 import apple;
2 import banana;
3 ~
>>> - 4 ~
5 import cat;
6 import dog;
6 changes: 6 additions & 0 deletions src/lint/renderer/__tests__/data/remline.txt
@@ -0,0 +1,6 @@
import apple;
import banana;


import cat;
import dog;

0 comments on commit cbc785d

Please sign in to comment.