diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php index 25f193d7..a0c71c09 100644 --- a/src/lint/ArcanistLintMessage.php +++ b/src/lint/ArcanistLintMessage.php @@ -1,198 +1,213 @@ 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->setOtherLocations(idx($dict, 'locations', array())); return $message; } public function toDictionary() { return array( 'path' => $this->getPath(), 'line' => $this->getLine(), 'char' => $this->getChar(), 'code' => $this->getCode(), 'severity' => $this->getSeverity(), 'name' => $this->getName(), 'description' => $this->getDescription(), 'original' => $this->getOriginalText(), 'replacement' => $this->getReplacementText(), + 'locations' => $this->getOtherLocations(), ); } public function setPath($path) { $this->path = $path; return $this; } public function getPath() { return $this->path; } public function setLine($line) { $this->line = $line; return $this; } public function getLine() { return $this->line; } public function setChar($char) { $this->char = $char; return $this; } public function getChar() { return $this->char; } public function setCode($code) { $this->code = $code; return $this; } public function getCode() { return $this->code; } public function setSeverity($severity) { $this->severity = $severity; return $this; } public function getSeverity() { return $this->severity; } public function setName($name) { $this->name = $name; return $this; } public function getName() { return $this->name; } public function setDescription($description) { $this->description = $description; return $this; } public function getDescription() { return $this->description; } public function setOriginalText($original) { $this->originalText = $original; return $this; } public function getOriginalText() { return $this->originalText; } public function setReplacementText($replacement) { $this->replacementText = $replacement; return $this; } public function getReplacementText() { 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; } public function isWarning() { return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_WARNING; } public function isAutofix() { return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_AUTOFIX; } public function hasFileContext() { return ($this->getLine() !== null); } public function setObsolete($obsolete) { $this->obsolete = $obsolete; return $this; } public function getObsolete() { return $this->obsolete; } public function isPatchable() { return ($this->getReplacementText() !== null) && ($this->getReplacementText() !== $this->getOriginalText()); } public function didApplyPatch() { if ($this->appliedToDisk) { return $this; } $this->appliedToDisk = true; foreach ($this->dependentMessages as $message) { $message->didApplyPatch(); } return $this; } public function isPatchApplied() { return $this->appliedToDisk; } public function setUncacheable($bool) { $this->uncacheable = $bool; return $this; } public function isUncacheable() { return $this->uncacheable; } public function setDependentMessages(array $messages) { assert_instances_of($messages, 'ArcanistLintMessage'); $this->dependentMessages = $messages; return $this; } } diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index c46a8922..9025b839 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -1,394 +1,409 @@ workingCopy = $working_copy; return $this; } public function getWorkingCopy() { return $this->workingCopy; } public function setPaths($paths) { $this->paths = $paths; return $this; } public function getPaths() { return $this->paths; } public function setPathChangedLines($path, $changed) { if ($changed === null) { $this->changedLines[$path] = null; } else { $this->changedLines[$path] = array_fill_keys($changed, true); } return $this; } public function getPathChangedLines($path) { return idx($this->changedLines, $path); } public function setFileData($data) { $this->fileData = $data + $this->fileData; return $this; } public function setCommitHookMode($mode) { $this->commitHookMode = $mode; return $this; } public function setHookAPI(ArcanistHookAPI $hook_api) { $this->hookAPI = $hook_api; return $this; } public function getHookAPI() { return $this->hookAPI; } public function setEnableAsyncLint($enable_async_lint) { $this->enableAsyncLint = $enable_async_lint; return $this; } public function getEnableAsyncLint() { return $this->enableAsyncLint; } public function loadData($path) { if (!isset($this->fileData[$path])) { if ($this->getCommitHookMode()) { $this->fileData[$path] = $this->getHookAPI() ->getCurrentFileData($path); } else { $disk_path = $this->getFilePathOnDisk($path); $this->fileData[$path] = Filesystem::readFile($disk_path); } } return $this->fileData[$path]; } public function pathExists($path) { if ($this->getCommitHookMode()) { $file_data = $this->loadData($path); return ($file_data !== null); } else { $disk_path = $this->getFilePathOnDisk($path); return Filesystem::pathExists($disk_path); } } public function getFilePathOnDisk($path) { return Filesystem::resolvePath( $path, $this->getWorkingCopy()->getProjectRoot()); } public function setMinimumSeverity($severity) { $this->minimumSeverity = $severity; return $this; } public function getCommitHookMode() { return $this->commitHookMode; } public function run() { $linters = $this->buildLinters(); if (!$linters) { throw new ArcanistNoEffectException("No linters to run."); } $have_paths = false; foreach ($linters as $linter) { if ($linter->getPaths()) { $have_paths = true; break; } } if (!$have_paths) { throw new ArcanistNoEffectException("No paths are lintable."); } $versions = array($this->getCacheVersion()); foreach ($linters as $linter) { $versions[] = get_class($linter).':'.$linter->getCacheVersion(); } $this->cacheVersion = crc32(implode("\n", $versions)); $linters_paths = array(); foreach ($linters as $linter_name => $linter) { $linter->setEngine($this); if (!$linter->canRun()) { continue; } $paths = $linter->getPaths(); $cache_granularity = $linter->getCacheGranularity(); foreach ($paths as $key => $path) { // Make sure each path has a result generated, even if it is empty // (i.e., the file has no lint messages). $result = $this->getResultForPath($path); if (isset($this->cachedResults[$path][$this->cacheVersion])) { if ($cache_granularity == ArcanistLinter::GRANULARITY_FILE) { unset($paths[$key]); } } } $paths = array_values($paths); $linters_paths[$linter_name] = $paths; if ($paths) { $linter->willLintPaths($paths); } } $stopped = array(); $exceptions = array(); foreach ($linters as $linter_name => $linter) { try { foreach ($linters_paths[$linter_name] as $path) { if (isset($stopped[$path])) { continue; } $linter->willLintPath($path); $linter->lintPath($path); if ($linter->didStopAllLinters()) { $stopped[$path] = true; } } $minimum = $this->minimumSeverity; foreach ($linter->getLintMessages() as $message) { if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) { continue; } if (!$this->isRelevantMessage($message)) { continue; } if ($cache_granularity != ArcanistLinter::GRANULARITY_FILE) { $message->setUncacheable(true); } $result = $this->getResultForPath($message->getPath()); $result->addMessage($message); } } catch (Exception $ex) { if (!is_string($linter_name)) { $linter_name = get_class($linter); } $exceptions[$linter_name] = $ex; } } if ($this->cachedResults) { foreach ($this->cachedResults as $path => $messages) { foreach (idx($messages, $this->cacheVersion, array()) as $message) { $this->getResultForPath($path)->addMessage( ArcanistLintMessage::newFromDictionary($message)); } } } foreach ($this->results as $path => $result) { $disk_path = $this->getFilePathOnDisk($path); $result->setFilePathOnDisk($disk_path); if (isset($this->fileData[$path])) { $result->setData($this->fileData[$path]); } else if ($disk_path && Filesystem::pathExists($disk_path)) { // TODO: this may cause us to, e.g., load a large binary when we only // raised an error about its filename. We could refine this by looking // through the lint messages and doing this load only if any of them // have original/replacement text or something like that. try { $this->fileData[$path] = Filesystem::readFile($disk_path); $result->setData($this->fileData[$path]); } catch (FilesystemException $ex) { // Ignore this, it's noncritical that we access this data and it // might be unreadable or a directory or whatever else for plenty // of legitimate reasons. } } } if ($exceptions) { throw new PhutilAggregateException('Some linters failed:', $exceptions); } return $this->results; } /** * @param dict>> * @return this */ public function setCachedResults(array $results) { $this->cachedResults = $results; return $this; } public function getResults() { return $this->results; } 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(); + + foreach ($locations as $location) { + $path = idx($location, 'path', $message->getPath()); + + if (!array_key_exists($path, $this->changedLines)) { + continue; + } - for ($l = $message->getLine(); $l <= $last_line; $l++) { - if (!empty($changed[$l])) { + $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; } protected function getResultForPath($path) { if (empty($this->results[$path])) { $result = new ArcanistLintResult(); $result->setPath($path); $result->setCacheVersion($this->cacheVersion); $this->results[$path] = $result; } return $this->results[$path]; } public function getLineAndCharFromOffset($path, $offset) { if (!isset($this->charToLine[$path])) { $char_to_line = array(); $line_to_first_char = array(); $lines = explode("\n", $this->loadData($path)); $line_number = 0; $line_start = 0; foreach ($lines as $line) { $len = strlen($line) + 1; // Account for "\n". $line_to_first_char[] = $line_start; $line_start += $len; for ($ii = 0; $ii < $len; $ii++) { $char_to_line[] = $line_number; } $line_number++; } $this->charToLine[$path] = $char_to_line; $this->lineToFirstChar[$path] = $line_to_first_char; } $line = $this->charToLine[$path][$offset]; $char = $offset - $this->lineToFirstChar[$path][$line]; return array($line, $char); } public function getPostponedLinters() { return $this->postponedLinters; } public function setPostponedLinters(array $linters) { $this->postponedLinters = $linters; return $this; } protected function getCacheVersion() { return 0; } protected function getPEP8WithTextOptions() { // E101 is subset of TXT2 (Tab Literal). // E501 is same as TXT3 (Line Too Long). // W291 is same as TXT6 (Trailing Whitespace). // W292 is same as TXT4 (File Does Not End in Newline). // W293 is same as TXT6 (Trailing Whitespace). return '--ignore=E101,E501,W291,W292,W293'; } } diff --git a/src/lint/linter/ArcanistConduitLinter.php b/src/lint/linter/ArcanistConduitLinter.php index c9e50cf9..741d04fb 100644 --- a/src/lint/linter/ArcanistConduitLinter.php +++ b/src/lint/linter/ArcanistConduitLinter.php @@ -1,94 +1,95 @@ must match passed in path. * 'line' * 'char' * 'code' * 'severity' => Must match a constant in ArcanistLintSeverity. * '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. */ final class ArcanistConduitLinter extends ArcanistLinter { const CONDUIT_METHOD = 'lint.getalllint'; private $conduitURI; private $linterName; private $lintByPath; // array(/pa/th/ => ), valid after willLintPaths(). public function __construct($conduit_uri, $linter_name) { $this->conduitURI = $conduit_uri; $this->linterName = $linter_name; } public function willLintPaths(array $paths) { // Load all file path data into $this->data. array_map(array($this, 'getData'), $paths); $conduit = new ConduitClient($this->conduitURI); $this->lintByPath = $conduit->callMethodSynchronous( self::CONDUIT_METHOD, array( 'file_contents' => $this->data, ) ); } public function lintPath($path) { $lint_for_path = idx($this->lintByPath, $path); if (!$lint_for_path) { return; } foreach ($lint_for_path as $lint) { $this->addLintMessage(ArcanistLintMessage::newFromDictionary($lint)); } } public function getLinterName() { return $this->linterName; } public function getLintSeverityMap() { // The rationale here is that this class will only be used for custom // linting in installations. No two server endpoints will be the same across // different instantiations. Therefore, the server can handle all severity // customization directly. throw new ArcanistUsageException( 'ArcanistConduitLinter does not support client-side severity '. 'customization.' ); } public function getLintNameMap() { // See getLintSeverityMap for rationale. throw new ArcanistUsageException( 'ArcanistConduitLinter does not support a name map.' ); } } diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index 909aa71b..eb2b9d9d 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -1,223 +1,221 @@ customSeverityMap = $map; return $this; } public function setConfig(array $config) { $this->config = $config; return $this; } protected function getConfig($key, $default = null) { return idx($this->config, $key, $default); } public function getActivePath() { return $this->activePath; } public function stopAllLinters() { $this->stopAllLinters = true; return $this; } public function didStopAllLinters() { return $this->stopAllLinters; } public function addPath($path) { $this->paths[$path] = $path; return $this; } public function setPaths(array $paths) { $this->paths = $paths; return $this; } public function getPaths() { return array_values($this->paths); } public function addData($path, $data) { $this->data[$path] = $data; return $this; } protected function getData($path) { if (!array_key_exists($path, $this->data)) { $this->data[$path] = $this->getEngine()->loadData($path); } return $this->data[$path]; } public function setEngine(ArcanistLintEngine $engine) { $this->engine = $engine; return $this; } protected function getEngine() { return $this->engine; } public function getCacheVersion() { return 0; } public function getLintMessageFullCode($short_code) { return $this->getLinterName().$short_code; } public function getLintMessageSeverity($code) { $map = $this->customSeverityMap; if (isset($map[$code])) { return $map[$code]; } $map = $this->getLintSeverityMap(); if (isset($map[$code])) { return $map[$code]; } return ArcanistLintSeverity::SEVERITY_ERROR; } public function isMessageEnabled($code) { return ($this->getLintMessageSeverity($code) !== ArcanistLintSeverity::SEVERITY_DISABLED); } public function getLintMessageName($code) { $map = $this->getLintNameMap(); if (isset($map[$code])) { return $map[$code]; } return "Unknown lint message!"; } protected function addLintMessage(ArcanistLintMessage $message) { if (!$this->getEngine()->getCommitHookMode()) { $root = $this->getEngine()->getWorkingCopy()->getProjectRoot(); $path = Filesystem::resolvePath($message->getPath(), $root); $message->setPath(Filesystem::readablePath($path, $root)); } $this->messages[] = $message; return $message; } public function getLintMessages() { 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, $code, $desc, $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, - ); - - if ($original !== null) { - $dict['original'] = $original; - } - if ($replacement !== null) { - $dict['replacement'] = $replacement; - } + $message = $this->newLintAtLine($line, $char, $code, $desc) + ->setOriginalText($original) + ->setReplacementText($replacement); - return $this->addLintMessage(ArcanistLintMessage::newFromDictionary($dict)); + return $this->addLintMessage($message); } protected function raiseLintAtPath( $code, $desc) { return $this->raiseLintAtLine(null, null, $code, $desc, null, null); } protected function raiseLintAtOffset( $offset, $code, $desc, $original = null, $replacement = null) { $path = $this->getActivePath(); $engine = $this->getEngine(); if ($offset === null) { $line = null; $char = null; } else { list($line, $char) = $engine->getLineAndCharFromOffset($path, $offset); } return $this->raiseLintAtLine( $line + 1, $char + 1, $code, $desc, $original, $replacement); } public function willLintPath($path) { $this->stopAllLinters = false; $this->activePath = $path; } public function canRun() { return true; } abstract public function willLintPaths(array $paths); abstract public function lintPath($path); abstract public function getLinterName(); public function getLintSeverityMap() { return array(); } public function getLintNameMap() { return array(); } public function getCacheGranularity() { return self::GRANULARITY_FILE; } }