diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6b04b743..40f0f1fb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -333,7 +333,7 @@ phutil_register_library_map(array( 'ArcanistPuppetLintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistPyFlakesLinter' => 'ArcanistExternalLinter', 'ArcanistPyFlakesLinterTestCase' => 'ArcanistExternalLinterTestCase', - 'ArcanistPyLintLinter' => 'ArcanistLinter', + 'ArcanistPyLintLinter' => 'ArcanistExternalLinter', 'ArcanistPyLintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase', 'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase', diff --git a/src/lint/linter/ArcanistPyLintLinter.php b/src/lint/linter/ArcanistPyLintLinter.php index ce1c60de..943a54f8 100644 --- a/src/lint/linter/ArcanistPyLintLinter.php +++ b/src/lint/linter/ArcanistPyLintLinter.php @@ -1,272 +1,199 @@ getEngine()->getConfigurationManager(); + private $config; - $error_regexp = $config->getConfigFromAnySource( - 'lint.pylint.codes.error'); - $warning_regexp = $config->getConfigFromAnySource( - 'lint.pylint.codes.warning'); - $advice_regexp = $config->getConfigFromAnySource( - 'lint.pylint.codes.advice'); - - if (!$error_regexp && !$warning_regexp && !$advice_regexp) { - throw new ArcanistUsageException( - pht( - "You are invoking the PyLint linter but have not configured any of ". - "'%s', '%s', or '%s'. Consult the documentation for %s.", - 'lint.pylint.codes.error', - 'lint.pylint.codes.warning', - 'lint.pylint.codes.advice', - __CLASS__)); - } - - $code_map = array( - ArcanistLintSeverity::SEVERITY_ERROR => $error_regexp, - ArcanistLintSeverity::SEVERITY_WARNING => $warning_regexp, - ArcanistLintSeverity::SEVERITY_ADVICE => $advice_regexp, - ); - - foreach ($code_map as $sev => $codes) { - if ($codes === null) { - continue; - } - if (!is_array($codes)) { - $codes = array($codes); - } - foreach ($codes as $code_re) { - if (preg_match("/{$code_re}/", $code)) { - return $sev; - } - } - } - - // If the message code doesn't match any of the provided regex's, - // then just disable it. - return ArcanistLintSeverity::SEVERITY_DISABLED; + public function getInfoName() { + return 'PyLint'; } - private function getPyLintPath() { - $pylint_bin = 'pylint'; - - // Use the PyLint prefix specified in the config file - $config = $this->getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.pylint.prefix'); - if ($prefix !== null) { - $pylint_bin = $prefix.'/bin/'.$pylint_bin; - } - - if (!Filesystem::pathExists($pylint_bin)) { - - list($err) = exec_manual('which %s', $pylint_bin); - if ($err) { - throw new ArcanistMissingLinterException( - pht( - "PyLint does not appear to be installed on this system. Install ". - "it (e.g., with '%s') or configure '%s' in your %s to point to ". - "the directory where it resides.", - 'sudo easy_install pylint', - 'lint.pylint.prefix', - '.arcconfig')); - } - } - - return $pylint_bin; + public function getInfoURI() { + return 'http://www.pylint.org/'; } - private function getPyLintPythonPath() { - // Get non-default install locations for pylint and its dependencies - // libraries. - $config = $this->getEngine()->getConfigurationManager(); - $prefixes = array( - $config->getConfigFromAnySource('lint.pylint.prefix'), - $config->getConfigFromAnySource('lint.pylint.logilab_astng.prefix'), - $config->getConfigFromAnySource('lint.pylint.logilab_common.prefix'), - ); - - // Add the libraries to the python search path - $python_path = array(); - foreach ($prefixes as $prefix) { - if ($prefix !== null) { - $python_path[] = $prefix.'/lib/python2.7/site-packages'; - $python_path[] = $prefix.'/lib/python2.7/dist-packages'; - $python_path[] = $prefix.'/lib/python2.6/site-packages'; - $python_path[] = $prefix.'/lib/python2.6/dist-packages'; - } - } - - $working_copy = $this->getEngine()->getWorkingCopy(); - $config_paths = $config->getConfigFromAnySource('lint.pylint.pythonpath'); - if ($config_paths !== null) { - foreach ($config_paths as $config_path) { - if ($config_path !== null) { - $python_path[] = Filesystem::resolvePath( - $config_path, - $working_copy->getProjectRoot()); - } - } - } - - $python_path[] = ''; - return implode(':', $python_path); - } - - private function getPyLintOptions() { - // '-rn': don't print lint report/summary at end - $options = array('-rn'); - - // Version 0.x.x include the pylint message ids in the output - if (version_compare($this->getLinterVersion(), '1', 'lt')) { - array_push($options, '-iy', '--output-format=text'); - } - // Version 1.x.x set the output specifically to the 0.x.x format - else { - array_push($options, "--msg-template='{msg_id}:{line:3d}: {obj}: {msg}'"); - } - - $working_copy = $this->getEngine()->getWorkingCopy(); - $config = $this->getEngine()->getConfigurationManager(); - - // Specify an --rcfile, either absolute or relative to the project root. - // Stupidly, the command line args above are overridden by rcfile, so be - // careful. - $rcfile = $config->getConfigFromAnySource('lint.pylint.rcfile'); - if ($rcfile !== null) { - $rcfile = Filesystem::resolvePath( - $rcfile, - $working_copy->getProjectRoot()); - $options[] = csprintf('--rcfile=%s', $rcfile); - } - - // Add any options defined in the config file for PyLint - $config_options = $config->getConfigFromAnySource('lint.pylint.options'); - if ($config_options !== null) { - $options = array_merge($options, $config_options); - } - - return implode(' ', $options); + public function getInfoDescription() { + return pht( + 'PyLint is a Python source code analyzer which looks for '. + 'programming errors, helps enforcing a coding standard and '. + 'sniffs for some code smells.'); } public function getLinterName() { return 'PyLint'; } - private function getLinterVersion() { - $pylint_bin = $this->getPyLintPath(); - $options = '--version'; - - list($stdout) = execx('%s %s', $pylint_bin, $options); - - $lines = phutil_split_lines($stdout, false); - $matches = null; - - // If the version command didn't return anything or the regex didn't match - // Assume a future version that at least is compatible with 1.x.x - if (count($lines) == 0 || - !preg_match('/pylint\s((?:\d+\.?)+)/', $lines[0], $matches)) { - return '999'; - } - - return $matches[1]; + public function getLinterConfigurationName() { + return 'pylint'; } - public function lintPath($path) { - $pylint_bin = $this->getPyLintPath(); - $python_path = $this->getPyLintPythonPath(); - $options = $this->getPyLintOptions(); - $path_on_disk = $this->getEngine()->getFilePathOnDisk($path); + public function getDefaultBinary() { + $prefix = $this->getDeprecatedConfiguration('lint.pylint.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.pylint.bin', 'pylint'); - try { - list($stdout, $_) = execx( - '/usr/bin/env PYTHONPATH=%s$PYTHONPATH %s %C %s', - $python_path, - $pylint_bin, - $options, - $path_on_disk); - } catch (CommandException $e) { - if ($e->getError() == 32) { - // According to ##man pylint## the exit status of 32 means there was a - // usage error. That's bad, so actually exit abnormally. - throw $e; - } else { - // The other non-zero exit codes mean there were messages issued, - // which is expected, so don't exit. - $stdout = $e->getStdout(); - } + if ($prefix) { + return $prefix.'/bin/'.$bin; + } else { + return $bin; + } + } + + public function getVersion() { + list($stdout) = execx('%C --version', $this->getExecutableCommand()); + + $matches = array(); + $regex = '/^pylint (?P\d+\.\d+\.\d+),/'; + if (preg_match($regex, $stdout, $matches)) { + return $matches['version']; + } else { + return false; + } + } + + public function getInstallInstructions() { + return pht( + 'Install PyLint using `%s`.', + 'pip install pylint'); + } + + public function shouldExpectCommandErrors() { + return true; + } + + public function getLinterConfigurationOptions() { + $options = array( + 'pylint.config' => array( + 'type' => 'optional string', + 'help' => pht('Pass in a custom configuration file path.'), + ), + ); + + return $options + parent::getLinterConfigurationOptions(); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'pylint.config': + $this->config = $value; + return; + + default: + return parent::setLinterConfigurationValue($key, $value); + } + } + + protected function getMandatoryFlags() { + $options = array(); + + $options[] = '--reports=no'; + $options[] = '--msg-template="{line}|{column}|{msg_id}|{symbol}|{msg}"'; + + // Specify an `--rcfile`, either absolute or relative to the project root. + // Stupidly, the command line args above are overridden by rcfile, so be + // careful. + $config = $this->config; + if (!$config) { + $config = $this->getDeprecatedConfiguration('lint.pylint.rcfile'); + } + + if ($config !== null) { + $options[] = '--rcfile='.$config; + } + + return $options; + } + + protected function getDefaultFlags() { + $options = array(); + + // Add any options defined in the config file for PyLint. + $config_options = $this->getDeprecatedConfiguration( + 'lint.pylint.options', + array()); + $options = array_merge($options, $config_options); + + $installed_version = $this->getVersion(); + $minimum_version = '1.0.0'; + if (version_compare($installed_version, $minimum_version, '<')) { + throw new ArcanistMissingLinterException( + pht( + '%s is not compatible with the installed version of pylint. '. + 'Minimum version: %s; installed version: %s.', + __CLASS__, + $minimum_version, + $installed_version)); + } + + return $options; + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + if ($err === 32) { + // According to `man pylint` the exit status of 32 means there was a + // usage error. That's bad, so actually exit abnormally. + return false; } $lines = phutil_split_lines($stdout, false); $messages = array(); + foreach ($lines as $line) { - $matches = null; - $regex = '/([A-Z]\d+): *(\d+)(?:|,\d*): *(.*)$/'; - if (!preg_match($regex, $line, $matches)) { + $matches = explode('|', $line, 5); + + if (count($matches) < 5) { continue; } - foreach ($matches as $key => $match) { - $matches[$key] = trim($match); - } - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($matches[2]); - $message->setCode($matches[1]); - $message->setName($this->getLinterName().' '.$matches[1]); - $message->setDescription($matches[3]); - $message->setSeverity($this->getMessageCodeSeverity($matches[1])); - $this->addLintMessage($message); + $message = id(new ArcanistLintMessage()) + ->setPath($path) + ->setLine($matches[0]) + ->setChar($matches[1]) + ->setCode($matches[2]) + ->setSeverity($this->getLintMessageSeverity($matches[2])) + ->setName(ucwords(str_replace('-', ' ', $matches[3]))) + ->setDescription($matches[4]); + + $messages[] = $message; + } + + if ($err && !$messages) { + return false; + } + + return $messages; + } + + protected function getDefaultMessageSeverity($code) { + switch (substr($code, 0, 1)) { + case 'R': + case 'C': + return ArcanistLintSeverity::SEVERITY_ADVICE; + case 'W': + return ArcanistLintSeverity::SEVERITY_WARNING; + case 'E': + case 'F': + return ArcanistLintSeverity::SEVERITY_ERROR; + default: + return ArcanistLintSeverity::SEVERITY_DISABLED; } } + protected function getLintCodeFromLinterConfigurationKey($code) { + if (!preg_match('/^(R|C|W|E|F)\d{4}$/', $code)) { + throw new Exception( + pht( + 'Unrecognized lint message code "%s". Expected a valid Pylint '. + 'lint code like "%s", or "%s", or "%s".', + $code, + 'C0111', + 'E0602', + 'W0611')); + } + + return $code; + } + }