mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 06:42:41 +01:00
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
This commit is contained in:
parent
d2f0ffac7d
commit
1914bce11e
1 changed files with 29 additions and 10 deletions
|
@ -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();
|
||||
|
|
Loading…
Reference in a new issue