diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index 08d818c7..424c4c6a 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -1,339 +1,348 @@ 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() { $stopped = array(); $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."); } $exceptions = array(); foreach ($linters as $linter_name => $linter) { try { $linter->setEngine($this); if (!$linter->canRun()) { continue; } $paths = $linter->getPaths(); 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($stopped[$path])) { unset($paths[$key]); } } $paths = array_values($paths); if ($paths) { $linter->willLintPaths($paths); foreach ($paths as $path) { $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; } $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; } } 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; } public function getResults() { return $this->results; } 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(); $result->setPath($path); $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 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/engine/ComprehensiveLintEngine.php b/src/lint/engine/ComprehensiveLintEngine.php index 43d3efb1..432d4c00 100644 --- a/src/lint/engine/ComprehensiveLintEngine.php +++ b/src/lint/engine/ComprehensiveLintEngine.php @@ -1,52 +1,54 @@ getPaths(); foreach ($paths as $key => $path) { if (!$this->pathExists($path)) { unset($paths[$key]); } if (preg_match('@^externals/@', $path)) { // Third-party stuff lives in /externals/; don't run lint engines // against it. unset($paths[$key]); } } $text_paths = preg_grep('/\.(php|css|hpp|cpp|l|y)$/', $paths); $linters[] = id(new ArcanistGeneratedLinter())->setPaths($text_paths); $linters[] = id(new ArcanistNoLintLinter())->setPaths($text_paths); $linters[] = id(new ArcanistTextLinter())->setPaths($text_paths); $linters[] = id(new ArcanistFilenameLinter())->setPaths($paths); $linters[] = id(new ArcanistXHPASTLinter()) ->setPaths(preg_grep('/\.php$/', $paths)); $linters[] = id(new ArcanistApacheLicenseLinter()) ->setPaths(preg_grep('/\.(php|cpp|hpp|l|y)$/', $paths)); $py_paths = preg_grep('/\.py$/', $paths); $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths); - $linters[] = id(new ArcanistPEP8Linter())->setPaths($py_paths); + $linters[] = id(new ArcanistPEP8Linter()) + ->setConfig(array('options' => $this->getPEP8WithTextOptions())) + ->setPaths($py_paths); $linters[] = id(new ArcanistRubyLinter()) ->setPaths(preg_grep('/\.rb$/', $paths)); $linters[] = id(new ArcanistJSHintLinter()) ->setPaths(preg_grep('/\.js$/', $paths)); return $linters; } } diff --git a/src/lint/linter/ArcanistPEP8Linter.php b/src/lint/linter/ArcanistPEP8Linter.php index 3e3f4972..2564fe8a 100644 --- a/src/lint/linter/ArcanistPEP8Linter.php +++ b/src/lint/linter/ArcanistPEP8Linter.php @@ -1,117 +1,115 @@ getEngine()->getWorkingCopy(); $options = $working_copy->getConfig('lint.pep8.options'); if ($options === null) { - // W293 (blank line contains whitespace) is redundant when used - // alongside TXT6, causing pain to python programmers. - return '--ignore=W293'; + $options = $this->getConfig('options'); } return $options; } public function getPEP8Path() { $working_copy = $this->getEngine()->getWorkingCopy(); $prefix = $working_copy->getConfig('lint.pep8.prefix'); $bin = $working_copy->getConfig('lint.pep8.bin'); if ($bin === null && $prefix === null) { $bin = csprintf('/usr/bin/env python2.6 %s', phutil_get_library_root('arcanist'). '/../externals/pep8/pep8.py'); } else { if ($bin === null) { $bin = 'pep8'; } if ($prefix !== null) { if (!Filesystem::pathExists($prefix.'/'.$bin)) { throw new ArcanistUsageException( "Unable to find PEP8 binary in a specified directory. Make sure ". "that 'lint.pep8.prefix' and 'lint.pep8.bin' keys are set ". "correctly. If you'd rather use a copy of PEP8 installed ". "globally, you can just remove these keys from your .arcconfig"); } $bin = csprintf("%s/%s", $prefix, $bin); return $bin; } // Look for globally installed PEP8 list($err) = exec_manual('which %s', $bin); if ($err) { throw new ArcanistUsageException( "PEP8 does not appear to be installed on this system. Install it ". "(e.g., with 'easy_install pep8') or configure ". "'lint.pep8.prefix' in your .arcconfig to point to the directory ". "where it resides."); } } return $bin; } public function lintPath($path) { $pep8_bin = $this->getPEP8Path(); $options = $this->getPEP8Options(); list($rc, $stdout) = exec_manual( "%C %C %s", $pep8_bin, $options, $this->getEngine()->getFilePathOnDisk($path)); $lines = explode("\n", $stdout); $messages = array(); foreach ($lines as $line) { $matches = null; if (!preg_match('/^(.*?):(\d+):(\d+): (\S+) (.*)$/', $line, $matches)) { continue; } foreach ($matches as $key => $match) { $matches[$key] = trim($match); } if (!$this->isMessageEnabled($matches[4])) { continue; } $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(ArcanistLintSeverity::SEVERITY_WARNING); $this->addLintMessage($message); } } }