diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php --- a/src/lint/ArcanistLintMessage.php +++ b/src/lint/ArcanistLintMessage.php @@ -8,41 +8,33 @@ protected $path; protected $line; protected $char; + protected $linterName; protected $code; protected $severity; protected $name; protected $description; protected $originalText; protected $replacementText; - protected $appliedToDisk; - protected $dependentMessages = array(); - protected $otherLocations = array(); - protected $obsolete; protected $granularity; - protected $bypassChangedLineFiltering; + protected $otherLocations = array(); + + private $appliedToDisk; + private $obsolete; public static function newFromDictionary(array $dict) { - $message = new ArcanistLintMessage(); - - $message->setPath($dict['path']); - $message->setLine($dict['line']); - $message->setChar($dict['char']); - $message->setCode($dict['code']); - $message->setSeverity($dict['severity']); - $message->setName($dict['name']); - $message->setDescription($dict['description']); - if (isset($dict['original'])) { - $message->setOriginalText($dict['original']); - } - if (isset($dict['replacement'])) { - $message->setReplacementText($dict['replacement']); - } - $message->setGranularity(idx($dict, 'granularity')); - $message->setOtherLocations(idx($dict, 'locations', array())); - if (isset($dict['bypassChangedLineFiltering'])) { - $message->bypassChangedLineFiltering($dict['bypassChangedLineFiltering']); - } - return $message; + return id(new ArcanistLintMessage()) + ->setPath($dict['path']) + ->setLine($dict['line']) + ->setChar($dict['char']) + ->setLinterName($dict['linterName']) + ->setCode($dict['code']) + ->setSeverity($dict['severity']) + ->setName($dict['name']) + ->setDescription($dict['description']) + ->setOriginalText($dict['original']) + ->setReplacementText($dict['replacement']) + ->setGranularity(idx($dict, 'granularity')) + ->setOtherLocations(idx($dict, 'locations', array())); } public function toDictionary() { @@ -50,6 +42,7 @@ 'path' => $this->getPath(), 'line' => $this->getLine(), 'char' => $this->getChar(), + 'linterName' => $this->getLinterName(), 'code' => $this->getCode(), 'severity' => $this->getSeverity(), 'name' => $this->getName(), @@ -58,17 +51,20 @@ 'replacement' => $this->getReplacementText(), 'granularity' => $this->getGranularity(), 'locations' => $this->getOtherLocations(), - 'bypassChangedLineFiltering' => $this->shouldBypassChangedLineFiltering(), ); } + public function getPath() { + return $this->path; + } + public function setPath($path) { $this->path = $path; return $this; } - public function getPath() { - return $this->path; + public function getLine() { + return $this->line; } public function setLine($line) { @@ -76,8 +72,8 @@ return $this; } - public function getLine() { - return $this->line; + public function getChar() { + return $this->char; } public function setChar($char) { @@ -85,12 +81,12 @@ return $this; } - public function getChar() { - return $this->char; + public function getLinterName() { + return $this->linterName; } - public function setCode($code) { - $this->code = $code; + public function setLinterName($linter_name) { + $this->linterName = $linter_name; return $this; } @@ -98,8 +94,8 @@ return $this->code; } - public function setSeverity($severity) { - $this->severity = $severity; + public function setCode($code) { + $this->code = $code; return $this; } @@ -107,8 +103,8 @@ return $this->severity; } - public function setName($name) { - $this->name = $name; + public function setSeverity($severity) { + $this->severity = $severity; return $this; } @@ -116,8 +112,8 @@ return $this->name; } - public function setDescription($description) { - $this->description = $description; + public function setName($name) { + $this->name = $name; return $this; } @@ -125,8 +121,8 @@ return $this->description; } - public function setOriginalText($original) { - $this->originalText = $original; + public function setDescription($description) { + $this->description = $description; return $this; } @@ -134,8 +130,8 @@ return $this->originalText; } - public function setReplacementText($replacement) { - $this->replacementText = $replacement; + public function setOriginalText($original) { + $this->originalText = $original; return $this; } @@ -143,33 +139,33 @@ return $this->replacementText; } - /** - * @param dict Keys 'path', 'line', 'char', 'original'. - */ - public function setOtherLocations(array $locations) { - assert_instances_of($locations, 'array'); - $this->otherLocations = $locations; + public function setReplacementText($replacement) { + $this->replacementText = $replacement; return $this; } - public function getOtherLocations() { - return $this->otherLocations; + public function getGranularity() { + return $this->granularity; } - public function isError() { - return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_ERROR; + public function setGranularity($granularity) { + $this->granularity = $granularity; + return $this; } - public function isWarning() { - return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_WARNING; + public function getOtherLocations() { + // TODO: This should probably be `list<ArcanistLintMessage>`. + return $this->otherLocations; } - public function isAutofix() { - return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_AUTOFIX; + public function setOtherLocations(array $locations) { + assert_instances_of($locations, 'array'); + $this->otherLocations = $locations; + return $this; } - public function hasFileContext() { - return ($this->getLine() !== null); + public function getObsolete() { + return $this->obsolete; } public function setObsolete($obsolete) { @@ -177,52 +173,40 @@ return $this; } - public function getObsolete() { - return $this->obsolete; - } - public function isPatchable() { - return ($this->getReplacementText() !== null) && - ($this->getReplacementText() !== $this->getOriginalText()); + public function isAutofix() { + return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_AUTOFIX; } - public function didApplyPatch() { - if ($this->appliedToDisk) { - return $this; - } - $this->appliedToDisk = true; - foreach ($this->dependentMessages as $message) { - $message->didApplyPatch(); - } - return $this; + public function isError() { + return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_ERROR; } - public function isPatchApplied() { - return $this->appliedToDisk; + public function isWarning() { + return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_WARNING; } - public function setGranularity($granularity) { - $this->granularity = $granularity; - return $this; - } - public function getGranularity() { - return $this->granularity; + public function hasFileContext() { + return $this->getLine() !== null; } - public function setDependentMessages(array $messages) { - assert_instances_of($messages, __CLASS__); - $this->dependentMessages = $messages; + + public function didApplyPatch() { + if (!$this->appliedToDisk) { + $this->appliedToDisk = true; + } + return $this; } - public function setBypassChangedLineFiltering($bypass_changed_lines) { - $this->bypassChangedLineFiltering = $bypass_changed_lines; - return $this; + public function isPatchable() { + return $this->getReplacementText() !== null && + $this->getReplacementText() !== $this->getOriginalText(); } - public function shouldBypassChangedLineFiltering() { - return $this->bypassChangedLineFiltering; + public function isPatchApplied() { + return $this->appliedToDisk; } } diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -331,9 +331,7 @@ // path is a directory or a binary file so we should not exclude // warnings. - if (!$this->changedLines || - $message->isError() || - $message->shouldBypassChangedLineFiltering()) { + if (!$this->changedLines || $message->isError()) { return true; } diff --git a/src/lint/linter/ArcanistClosureLinter.php b/src/lint/linter/ArcanistClosureLinter.php --- a/src/lint/linter/ArcanistClosureLinter.php +++ b/src/lint/linter/ArcanistClosureLinter.php @@ -47,13 +47,11 @@ continue; } - $message = id(new ArcanistLintMessage()) + $messages[] = id(new ArcanistLintMessage()) ->setPath($path) ->setLine($matches[1]) - ->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR) - ->setCode($this->getLinterName().$matches[2]) + ->setCode($matches[2]) ->setDescription($matches[3]); - $messages[] = $message; } return $messages; diff --git a/src/lint/linter/ArcanistCoffeeLintLinter.php b/src/lint/linter/ArcanistCoffeeLintLinter.php --- a/src/lint/linter/ArcanistCoffeeLintLinter.php +++ b/src/lint/linter/ArcanistCoffeeLintLinter.php @@ -97,7 +97,6 @@ $message = id(new ArcanistLintMessage()) ->setPath($path) ->setLine($report['lineNumber']) - ->setCode($this->getLinterName()) ->setName(ucwords(str_replace('_', ' ', $report['name']))) ->setDescription($report['message']) ->setOriginalText(idx($report, 'line')); @@ -110,10 +109,6 @@ case 'error': $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); break; - - default: - $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); - break; } $messages[] = $message; diff --git a/src/lint/linter/ArcanistCppcheckLinter.php b/src/lint/linter/ArcanistCppcheckLinter.php --- a/src/lint/linter/ArcanistCppcheckLinter.php +++ b/src/lint/linter/ArcanistCppcheckLinter.php @@ -81,12 +81,11 @@ $messages = array(); foreach ($errors as $error) { foreach ($error->getElementsByTagName('location') as $location) { - $message = new ArcanistLintMessage(); - $message->setPath($location->getAttribute('file')); - $message->setLine($location->getAttribute('line')); - $message->setCode('Cppcheck'); - $message->setName($error->getAttribute('id')); - $message->setDescription($error->getAttribute('msg')); + $message = id(new ArcanistLintMessage()) + ->setPath($location->getAttribute('file')) + ->setLine($location->getAttribute('line')) + ->setName($error->getAttribute('id')) + ->setDescription($error->getAttribute('msg')); switch ($error->getAttribute('severity')) { case 'error': diff --git a/src/lint/linter/ArcanistJSONLintLinter.php b/src/lint/linter/ArcanistJSONLintLinter.php --- a/src/lint/linter/ArcanistJSONLintLinter.php +++ b/src/lint/linter/ArcanistJSONLintLinter.php @@ -68,12 +68,10 @@ if ($match) { $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches['line']); - $message->setChar($matches['column']); - $message->setCode($this->getLinterName()); - $message->setDescription(ucfirst($matches['description'])); - $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + ->setPath($path) + ->setLine($matches['line']) + ->setChar($matches['column']) + ->setDescription(ucfirst($matches['description'])); $messages[] = $message; } diff --git a/src/lint/linter/ArcanistLesscLinter.php b/src/lint/linter/ArcanistLesscLinter.php --- a/src/lint/linter/ArcanistLesscLinter.php +++ b/src/lint/linter/ArcanistLesscLinter.php @@ -165,13 +165,12 @@ $code = $this->getLintCodeFromLinterConfigurationKey($matches['name']); $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches['line']); - $message->setChar($matches['column']); - $message->setCode($this->getLintMessageFullCode($code)); - $message->setSeverity($this->getLintMessageSeverity($code)); - $message->setName($this->getLintMessageName($code)); - $message->setDescription(ucfirst($matches['description'])); + ->setPath($path) + ->setLine($matches['line']) + ->setChar($matches['column']) + ->setCode($this->getLintMessageFullCode($code)) + ->setName($this->getLintMessageName($code)) + ->setDescription(ucfirst($matches['description'])); $messages[] = $message; } diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -361,11 +361,6 @@ return 0; } - - final public function getLintMessageFullCode($short_code) { - return $this->getLinterName().$short_code; - } - final public function getLintMessageSeverity($code) { $map = $this->customSeverityMap; if (isset($map[$code])) { @@ -403,11 +398,24 @@ return pht('Unknown lint message!'); } - final protected function addLintMessage(ArcanistLintMessage $message) { + final private function addLintMessage(ArcanistLintMessage $message) { $root = $this->getProjectRoot(); $path = Filesystem::resolvePath($message->getPath(), $root); $message->setPath(Filesystem::readablePath($path, $root)); + + $message->setLinterName($this->getLinterName()); + + // If the linter message doesn't have a name, lookup the name + // from the linter code. + if (!$message->getName()) { + $message->setName($this->getLintMessageName($message->getCode())); + } + + if ($message->getSeverity() === null && $this->canCustomizeLintSeverities()) { + $this->getLintMessageSeverity($message->getCode()); + } + $this->messages[] = $message; return $message; } @@ -428,7 +436,7 @@ ->setPath($this->getActivePath()) ->setLine($line) ->setChar($char) - ->setCode($this->getLintMessageFullCode($code)) + ->setCode($code) ->setSeverity($this->getLintMessageSeverity($code)) ->setName($this->getLintMessageName($code)) ->setDescription($desc) diff --git a/src/lint/linter/ArcanistMergeConflictLinter.php b/src/lint/linter/ArcanistMergeConflictLinter.php --- a/src/lint/linter/ArcanistMergeConflictLinter.php +++ b/src/lint/linter/ArcanistMergeConflictLinter.php @@ -27,7 +27,7 @@ public function getLintNameMap() { return array( - self::LINT_MERGECONFLICT => pht('Unresolved merge conflict'), + self::LINT_MERGECONFLICT => pht('Unresolved Merge Conflict'), ); } diff --git a/src/lint/linter/ArcanistNoLintLinter.php b/src/lint/linter/ArcanistNoLintLinter.php --- a/src/lint/linter/ArcanistNoLintLinter.php +++ b/src/lint/linter/ArcanistNoLintLinter.php @@ -11,8 +11,8 @@ public function getInfoDescription() { return pht( - 'Allows you to disable all lint messages for a file by putting "%s" in '. - 'the file body.', + 'Allows you to disable all lint messages for a file '. + 'by putting "%s" in the file body.', '@'.'nolint'); } diff --git a/src/lint/linter/ArcanistPEP8Linter.php b/src/lint/linter/ArcanistPEP8Linter.php --- a/src/lint/linter/ArcanistPEP8Linter.php +++ b/src/lint/linter/ArcanistPEP8Linter.php @@ -58,16 +58,13 @@ foreach ($matches as $key => $match) { $matches[$key] = trim($match); } - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches[2]); - $message->setChar($matches[3]); - $message->setCode($matches[4]); - $message->setName('PEP8 '.$matches[4]); - $message->setDescription($matches[5]); - $message->setSeverity($this->getLintMessageSeverity($matches[4])); - - $messages[] = $message; + + $messages[] = id(new ArcanistLintMessage()) + ->setPath($path) + ->setLine($matches[2]) + ->setChar($matches[3]) + ->setCode($matches[4]) + ->setDescription($matches[5]); } return $messages; diff --git a/src/lint/linter/ArcanistPhpLinter.php b/src/lint/linter/ArcanistPhpLinter.php --- a/src/lint/linter/ArcanistPhpLinter.php +++ b/src/lint/linter/ArcanistPhpLinter.php @@ -68,14 +68,11 @@ $regex = '/^(PHP )?(?<type>.+) error: +(?<error>.+) in (?<file>.+) '. 'on line (?<line>\d+)$/m'; if (preg_match($regex, $stdout, $matches)) { - $code = $this->getLintCodeFromLinterConfigurationKey($matches['type']); - $message = id(new ArcanistLintMessage()) ->setPath($path) ->setLine($matches['line']) - ->setCode($this->getLinterName().$code) - ->setName($this->getLintMessageName($code)) - ->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR) + ->setCode($this->getLintCodeFromLinterConfigurationKey( + $matches['type'])) ->setDescription($matches['error']); // `php -l` only returns the first error. diff --git a/src/lint/linter/ArcanistPhpcsLinter.php b/src/lint/linter/ArcanistPhpcsLinter.php --- a/src/lint/linter/ArcanistPhpcsLinter.php +++ b/src/lint/linter/ArcanistPhpcsLinter.php @@ -103,22 +103,19 @@ continue; } - if ($child->tagName == 'error') { - $prefix = 'E'; - } else { - $prefix = 'W'; + $message = id(new ArcanistLintMessage()) + ->setPath($path) + ->setLine($child->getAttribute('line')) + ->setChar($child->getAttribute('column')) + ->setCode($child->getAttribute('source')) + ->setDescription($child->nodeValue); + + switch ($child->tagName) { + case 'error': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + break; } - $code = 'PHPCS.'.$prefix.'.'.$child->getAttribute('source'); - - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($child->getAttribute('line')); - $message->setChar($child->getAttribute('column')); - $message->setCode($code); - $message->setDescription($child->nodeValue); - $message->setSeverity($this->getLintMessageSeverity($code)); - $messages[] = $message; } } @@ -127,11 +124,7 @@ } protected function getDefaultMessageSeverity($code) { - if (preg_match('/^PHPCS\\.W\\./', $code)) { - return ArcanistLintSeverity::SEVERITY_WARNING; - } else { - return ArcanistLintSeverity::SEVERITY_ERROR; - } + return ArcanistLintSeverity::SEVERITY_WARNING; } protected function getLintCodeFromLinterConfigurationKey($code) { diff --git a/src/lint/linter/ArcanistPuppetLintLinter.php b/src/lint/linter/ArcanistPuppetLintLinter.php --- a/src/lint/linter/ArcanistPuppetLintLinter.php +++ b/src/lint/linter/ArcanistPuppetLintLinter.php @@ -112,7 +112,7 @@ ->setPath($path) ->setLine($matches[0]) ->setChar($matches[1]) - ->setCode($this->getLinterName()) + ->setCode($matches[3]) ->setName(ucwords(str_replace('_', ' ', $matches[3]))) ->setDescription(ucfirst($matches[4])); diff --git a/src/lint/linter/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -67,12 +67,11 @@ $severity = ArcanistLintSeverity::SEVERITY_ERROR; } - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches[2]); - $message->setCode($this->getLinterName()); - $message->setDescription($description); - $message->setSeverity($severity); + $message = id(new ArcanistLintMessage()) + ->setPath($path) + ->setLine($matches[2]) + ->setDescription($description) + ->setSeverity($severity); $messages[] = $message; } @@ -80,8 +79,4 @@ return $messages; } - protected function canCustomizeLintSeverities() { - return false; - } - } diff --git a/src/lint/linter/ArcanistPyLintLinter.php b/src/lint/linter/ArcanistPyLintLinter.php --- a/src/lint/linter/ArcanistPyLintLinter.php +++ b/src/lint/linter/ArcanistPyLintLinter.php @@ -135,7 +135,6 @@ ->setLine($matches[0]) ->setChar($matches[1]) ->setCode($matches[2]) - ->setSeverity($this->getLintMessageSeverity($matches[2])) ->setName(ucwords(str_replace('-', ' ', $matches[3]))) ->setDescription($matches[4]); diff --git a/src/lint/linter/ArcanistRuboCopLinter.php b/src/lint/linter/ArcanistRuboCopLinter.php --- a/src/lint/linter/ArcanistRuboCopLinter.php +++ b/src/lint/linter/ArcanistRuboCopLinter.php @@ -86,12 +86,26 @@ foreach ($file['offenses'] as $offense) { $message = id(new ArcanistLintMessage()) ->setPath($file['path']) - ->setDescription($offense['message']) ->setLine($offense['location']['line']) ->setChar($offense['location']['column']) - ->setSeverity($this->getLintMessageSeverity($offense['severity'])) - ->setName($this->getLinterName()) - ->setCode($offense['cop_name']); + ->setCode($offense['cop_name']) + ->setDescription($offense['message']); + + switch ($code) { + case 'convention': + case 'refactor': + case 'warning': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + break; + case 'error': + case 'fatal': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + break; + default: + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); + break; + } + $messages[] = $message; } } @@ -99,22 +113,4 @@ return $messages; } - /** - * Take the string from RuboCop's severity terminology and return an - * @{class:ArcanistLintSeverity}. - */ - protected function getDefaultMessageSeverity($code) { - switch ($code) { - case 'convention': - case 'refactor': - case 'warning': - return ArcanistLintSeverity::SEVERITY_WARNING; - case 'error': - case 'fatal': - return ArcanistLintSeverity::SEVERITY_ERROR; - default: - return ArcanistLintSeverity::SEVERITY_ADVICE; - } - } - } diff --git a/src/lint/linter/ArcanistRubyLinter.php b/src/lint/linter/ArcanistRubyLinter.php --- a/src/lint/linter/ArcanistRubyLinter.php +++ b/src/lint/linter/ArcanistRubyLinter.php @@ -70,13 +70,10 @@ $code = head(explode(',', $matches[3])); - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches[2]); - $message->setCode($this->getLinterName()); - $message->setName(pht('Syntax Error')); - $message->setDescription($matches[3]); - $message->setSeverity($this->getLintMessageSeverity($code)); + $message = id(new ArcanistLintMessage()) + ->setPath($path) + ->setLine($matches[2]) + ->setDescription($matches[3]); $messages[] = $message; } diff --git a/src/lint/linter/ArcanistXMLLinter.php b/src/lint/linter/ArcanistXMLLinter.php --- a/src/lint/linter/ArcanistXMLLinter.php +++ b/src/lint/linter/ArcanistXMLLinter.php @@ -44,8 +44,7 @@ ->setPath($path) ->setLine($error->line) ->setChar($error->column ? $error->column : null) - ->setCode($this->getLintMessageFullCode($error->code)) - ->setName(pht('LibXML Error')) + ->setCode($error->code) ->setDescription(trim($error->message)); switch ($error->level) { @@ -61,13 +60,9 @@ case LIBXML_ERR_FATAL: $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); break; - - default: - $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); - break; } - $this->addLintMessage($message); + $messages[] = $message; } }