From 1914bce11ecd594e236d3fef7688fb7d71ddf1c1 Mon Sep 17 00:00:00 2001 From: vrana Date: Sun, 22 Jul 2012 12:52:10 -0700 Subject: [PATCH] Enable raising lint warnings for mulitple lines at once Summary: We have a lint rule checking if some string is too long. This string can span multiple lines. If we report a warning at the first line then it is muted if the first line wasn't modified. We need to say that this whole block is wrong and report it when at least one line from the block was modified. Test Plan: Changed a lint rule to call `raiseLintAtLines()` and verified that the warning is reported even if the changed line isn't first. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3029 --- src/lint/engine/ArcanistLintEngine.php | 39 +++++++++++++++++++------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index 858285c0..59a05d2c 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -229,16 +229,8 @@ abstract class ArcanistLintEngine { if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) { continue; } - // When a user runs "arc lint", we default to raising only warnings on - // lines they have changed (errors are still raised anywhere in the - // file). The list of $changed lines may be null, to indicate that the - // path is a directory or a binary file so we should not exclude - // warnings. - $changed = $this->getPathChangedLines($message->getPath()); - if ($changed !== null && !$message->isError() && $message->getLine()) { - if (empty($changed[$message->getLine()])) { - continue; - } + if (!$this->isRelevantMessage($message)) { + continue; } $result = $this->getResultForPath($message->getPath()); $result->addMessage($message); @@ -271,6 +263,33 @@ abstract class ArcanistLintEngine { abstract protected function buildLinters(); + private function isRelevantMessage($message) { + // When a user runs "arc lint", we default to raising only warnings on + // lines they have changed (errors are still raised anywhere in the + // file). The list of $changed lines may be null, to indicate that the + // path is a directory or a binary file so we should not exclude + // warnings. + + $changed = $this->getPathChangedLines($message->getPath()); + + if ($changed === null || $message->isError() || !$message->getLine()) { + return true; + } + + $last_line = $message->getLine(); + if ($message->getOriginalText()) { + $last_line += substr_count($message->getOriginalText(), "\n"); + } + + for ($l = $message->getLine(); $l <= $last_line; $l++) { + if (!empty($changed[$l])) { + return true; + } + } + + return false; + } + private function getResultForPath($path) { if (empty($this->results[$path])) { $result = new ArcanistLintResult();