diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php index ea233e3d..a4dfffa9 100644 --- a/src/lint/renderer/ArcanistConsoleLintRenderer.php +++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php @@ -216,8 +216,11 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { } for ($ii = $start; $ii < $start + $new_impact; $ii++) { + // If the patch was at the end of the file and ends with a newline, we + // won't have an actual entry in the array for the last line, even though + // we want to show it in the diff. $out[] = array( - 'text' => $new_lines[$ii - 1], + 'text' => idx($new_lines, $ii - 1, ''), 'type' => '+', 'chevron' => ($ii == $start), ); @@ -246,9 +249,17 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { } } + // If the line doesn't actually end in a newline, add one so the layout + // doesn't mess up. This can happen when the last line of the old file + // didn't have a newline at the end. + $text = $spec['text']; + if (!preg_match('/\n\z/', $spec['text'])) { + $text .= "\n"; + } + $result[] = $this->renderLine( idx($spec, 'number'), - $spec['text'], + $text, $chevron, idx($spec, 'type')); @@ -298,6 +309,15 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { $offset += strlen($line); } + // If the last line ends in a newline, add a virtual offset for the final + // line with no characters on it. This allows lint messages to target the + // last line of the file at character 1. + if ($lines) { + if (preg_match('/\n\z/', $line)) { + $line_map[$number] = $offset; + } + } + return $line_map; } diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php index 9014288b..f8928229 100644 --- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php +++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php @@ -120,6 +120,28 @@ EOTEXT; 'original' => "\n", 'replacement' => '', ), + + 'eofnewline' => array( + 'line' => 1, + 'char' => 7, + 'original' => '', + 'replacement' => "\n", + ), + + 'eofmultilinechar' => array( + 'line' => 5, + 'char' => 3, + 'original' => '', + 'replacement' => "\nX\nY\n", + ), + + 'eofmultilineline' => array( + 'line' => 6, + 'char' => 1, + 'original' => '', + 'replacement' => "\nX\nY\n", + ), + ); $defaults = array( @@ -132,8 +154,6 @@ EOTEXT; foreach ($map as $key => $test_case) { $data = $this->readTestData("{$key}.txt"); - $data = preg_replace('/~+\s*$/m', '', $data); - $expect = $this->readTestData("{$key}.expect"); $test_case = $test_case + $defaults; @@ -178,11 +198,6 @@ EOTEXT; throw $ex; } - // Trim "~" off the ends of lines. This allows the "expect" file to test - // for trailing whitespace without actually containing trailing - // whitespace. - $expect = preg_replace('/~+$/m', '', $expect); - $this->assertEqual( $expect, $actual, @@ -194,7 +209,19 @@ EOTEXT; private function readTestData($filename) { $path = dirname(__FILE__).'/data/'.$filename; - return Filesystem::readFile($path); + $data = Filesystem::readFile($path); + + // If we find "~~~" at the end of the file, get rid of it and any whitespace + // afterwards. This allows specifying data files with trailing empty + // lines. + $data = preg_replace('/~~~\s*\z/', '', $data); + + // Trim "~" off the ends of lines. This allows the "expect" file to test + // for trailing whitespace without actually containing trailing + // whitespace. + $data = preg_replace('/~$/m', '', $data); + + return $data; } } diff --git a/src/lint/renderer/__tests__/data/eofmultilinechar.expect b/src/lint/renderer/__tests__/data/eofmultilinechar.expect new file mode 100644 index 00000000..5ec8fdbe --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofmultilinechar.expect @@ -0,0 +1,12 @@ +>>> Lint for path/to/example.c: + + + Warning (WARN123) Lint Warning + Consider this. + + 3 3 + 4 4 + 5 5 + >>> + ~ + + X + + Y diff --git a/src/lint/renderer/__tests__/data/eofmultilinechar.txt b/src/lint/renderer/__tests__/data/eofmultilinechar.txt new file mode 100644 index 00000000..8a1218a1 --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofmultilinechar.txt @@ -0,0 +1,5 @@ +1 +2 +3 +4 +5 diff --git a/src/lint/renderer/__tests__/data/eofmultilineline.expect b/src/lint/renderer/__tests__/data/eofmultilineline.expect new file mode 100644 index 00000000..5ec8fdbe --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofmultilineline.expect @@ -0,0 +1,12 @@ +>>> Lint for path/to/example.c: + + + Warning (WARN123) Lint Warning + Consider this. + + 3 3 + 4 4 + 5 5 + >>> + ~ + + X + + Y diff --git a/src/lint/renderer/__tests__/data/eofmultilineline.txt b/src/lint/renderer/__tests__/data/eofmultilineline.txt new file mode 100644 index 00000000..8a1218a1 --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofmultilineline.txt @@ -0,0 +1,5 @@ +1 +2 +3 +4 +5 diff --git a/src/lint/renderer/__tests__/data/eofnewline.expect b/src/lint/renderer/__tests__/data/eofnewline.expect new file mode 100644 index 00000000..5310bdb9 --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofnewline.expect @@ -0,0 +1,9 @@ +>>> Lint for path/to/example.c: + + + Warning (WARN123) Lint Warning + Consider this. + + >>> - 1 abcdef + + abcdef + + ~ diff --git a/src/lint/renderer/__tests__/data/eofnewline.txt b/src/lint/renderer/__tests__/data/eofnewline.txt new file mode 100644 index 00000000..9836135e --- /dev/null +++ b/src/lint/renderer/__tests__/data/eofnewline.txt @@ -0,0 +1 @@ +abcdef~~~ diff --git a/src/lint/renderer/__tests__/data/extrawhitespace.txt b/src/lint/renderer/__tests__/data/extrawhitespace.txt index ba587d4c..00ba812f 100644 --- a/src/lint/renderer/__tests__/data/extrawhitespace.txt +++ b/src/lint/renderer/__tests__/data/extrawhitespace.txt @@ -1,3 +1,3 @@ Adrift upon the sea. -~ +~~~