mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-27 16:00:58 +01:00
Allow specifying multiple locations of the same lint error
Summary: Some errors (duplicate declaration, invalid number of arguments) have more related places. We need to notify user if he changes any related place. This could be currently achieved by triggering errors instead of warnings or by including both files in the range (impossible if the locations are in different files) or by issuing multiple errors. All options are too aggressive. Test Plan: Issued error on unmodified line with other location on modified line. Reviewers: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4392
This commit is contained in:
parent
80cd881fb1
commit
a82493a84d
4 changed files with 56 additions and 27 deletions
|
@ -18,6 +18,7 @@ final class ArcanistLintMessage {
|
|||
protected $replacementText;
|
||||
protected $appliedToDisk;
|
||||
protected $dependentMessages = array();
|
||||
protected $otherLocations = array();
|
||||
protected $obsolete;
|
||||
protected $uncacheable;
|
||||
|
||||
|
@ -37,6 +38,7 @@ final class ArcanistLintMessage {
|
|||
if (isset($dict['replacement'])) {
|
||||
$message->setReplacementText($dict['replacement']);
|
||||
}
|
||||
$message->setOtherLocations(idx($dict, 'locations', array()));
|
||||
return $message;
|
||||
}
|
||||
|
||||
|
@ -51,6 +53,7 @@ final class ArcanistLintMessage {
|
|||
'description' => $this->getDescription(),
|
||||
'original' => $this->getOriginalText(),
|
||||
'replacement' => $this->getReplacementText(),
|
||||
'locations' => $this->getOtherLocations(),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -135,6 +138,18 @@ final class ArcanistLintMessage {
|
|||
return $this->replacementText;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param dict Keys 'path', 'line', 'char', 'original'.
|
||||
*/
|
||||
public function setOtherLocations(array $locations) {
|
||||
$this->otherLocations = $locations;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getOtherLocations() {
|
||||
return $this->otherLocations;
|
||||
}
|
||||
|
||||
public function isError() {
|
||||
return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_ERROR;
|
||||
}
|
||||
|
|
|
@ -305,28 +305,43 @@ abstract class ArcanistLintEngine {
|
|||
|
||||
abstract protected function buildLinters();
|
||||
|
||||
private function isRelevantMessage($message) {
|
||||
private function isRelevantMessage(ArcanistLintMessage $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()) {
|
||||
if (!$this->changedLines || $message->isError()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$last_line = $message->getLine();
|
||||
if ($message->getOriginalText()) {
|
||||
$last_line += substr_count($message->getOriginalText(), "\n");
|
||||
}
|
||||
$locations = $message->getOtherLocations();
|
||||
$locations[] = $message->toDictionary();
|
||||
|
||||
for ($l = $message->getLine(); $l <= $last_line; $l++) {
|
||||
if (!empty($changed[$l])) {
|
||||
foreach ($locations as $location) {
|
||||
$path = idx($location, 'path', $message->getPath());
|
||||
|
||||
if (!array_key_exists($path, $this->changedLines)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$changed = $this->getPathChangedLines($path);
|
||||
|
||||
if ($changed === null || !$location['line']) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$last_line = $location['line'];
|
||||
if (isset($location['original'])) {
|
||||
$last_line += substr_count($location['original'], "\n");
|
||||
}
|
||||
|
||||
for ($l = $location['line']; $l <= $last_line; $l++) {
|
||||
if (!empty($changed[$l])) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
|
|
|
@ -29,6 +29,7 @@
|
|||
* 'name'
|
||||
* 'description'
|
||||
* 'original' & 'replacement' => optional patch information
|
||||
* 'locations' => other locations of the same error (in the same format)
|
||||
*
|
||||
* This class is intended for customization via instantiation, not via
|
||||
* subclassing.
|
||||
|
|
|
@ -135,6 +135,17 @@ abstract class ArcanistLinter {
|
|||
return $this->messages;
|
||||
}
|
||||
|
||||
protected function newLintAtLine($line, $char, $code, $desc) {
|
||||
return id(new ArcanistLintMessage())
|
||||
->setPath($this->getActivePath())
|
||||
->setLine($line)
|
||||
->setChar($char)
|
||||
->setCode($this->getLintMessageFullCode($code))
|
||||
->setSeverity($this->getLintMessageSeverity($code))
|
||||
->setName($this->getLintMessageName($code))
|
||||
->setDescription($desc);
|
||||
}
|
||||
|
||||
protected function raiseLintAtLine(
|
||||
$line,
|
||||
$char,
|
||||
|
@ -143,24 +154,11 @@ abstract class ArcanistLinter {
|
|||
$original = null,
|
||||
$replacement = null) {
|
||||
|
||||
$dict = array(
|
||||
'path' => $this->getActivePath(),
|
||||
'line' => $line,
|
||||
'char' => $char,
|
||||
'code' => $this->getLintMessageFullCode($code),
|
||||
'severity' => $this->getLintMessageSeverity($code),
|
||||
'name' => $this->getLintMessageName($code),
|
||||
'description' => $desc,
|
||||
);
|
||||
$message = $this->newLintAtLine($line, $char, $code, $desc)
|
||||
->setOriginalText($original)
|
||||
->setReplacementText($replacement);
|
||||
|
||||
if ($original !== null) {
|
||||
$dict['original'] = $original;
|
||||
}
|
||||
if ($replacement !== null) {
|
||||
$dict['replacement'] = $replacement;
|
||||
}
|
||||
|
||||
return $this->addLintMessage(ArcanistLintMessage::newFromDictionary($dict));
|
||||
return $this->addLintMessage($message);
|
||||
}
|
||||
|
||||
protected function raiseLintAtPath(
|
||||
|
|
Loading…
Reference in a new issue