Page MenuHomePhabricator
Files F15386976

D9109.id30870.diff
No OneTemporary
Actions

D9109.id30870.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -333,7 +333,7 @@
'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
--- a/src/lint/linter/ArcanistPyLintLinter.php
+++ b/src/lint/linter/ArcanistPyLintLinter.php
@@ -1,272 +1,199 @@
<?php
/**
- * Uses "PyLint" to detect various errors in Python code. To use this linter,
- * you must install pylint and configure which codes you want to be reported as
- * errors, warnings and advice.
- *
- * You should be able to install pylint with ##sudo easy_install pylint##. If
- * your system is unusual, you can manually specify the location of pylint and
- * its dependencies by configuring these keys in your .arcconfig:
- *
- * lint.pylint.prefix
- * lint.pylint.logilab_astng.prefix
- * lint.pylint.logilab_common.prefix
- *
- * You can specify additional command-line options to pass to PyLint by
- * setting ##lint.pylint.options##. You may also specify a list of additional
- * entries for PYTHONPATH with ##lint.pylint.pythonpath##. Those can be
- * absolute or relative to the project root.
- *
- * If you have a PyLint rcfile, specify its path with
- * ##lint.pylint.rcfile##. It can be absolute or relative to the project
- * root. Be sure not to define ##output-format##, or if you do, set it to
- * ##text##.
- *
- * Specify which PyLint messages map to which Arcanist messages by defining
- * the following regular expressions:
- *
- * lint.pylint.codes.error
- * lint.pylint.codes.warning
- * lint.pylint.codes.advice
- *
- * The regexps are run in that order; the first to match determines which
- * Arcanist severity applies, if any. For example, to capture all PyLint
- * "E...." errors as Arcanist errors, set ##lint.pylint.codes.error## to:
- *
- * ^E.*
- *
- * You can also match more granularly:
- *
- * ^E(0001|0002)$
- *
- * According to ##man pylint##, there are 5 kind of messages:
- *
- * (C) convention, for programming standard violation
- * (R) refactor, for bad code smell
- * (W) warning, for python specific problems
- * (E) error, for probable bugs in the code
- * (F) fatal, if an error occurred which prevented pylint from
- * doing further processing.
+ * Uses "PyLint" to detect various errors in Python code.
*/
-final class ArcanistPyLintLinter extends ArcanistLinter {
+final class ArcanistPyLintLinter extends ArcanistExternalLinter {
- private function getMessageCodeSeverity($code) {
- $config = $this->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');
+ public function getInfoName() {
+ return 'PyLint';
+ }
- 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__));
- }
+ public function getInfoURI() {
+ return 'http://www.pylint.org/';
+ }
- $code_map = array(
- ArcanistLintSeverity::SEVERITY_ERROR => $error_regexp,
- ArcanistLintSeverity::SEVERITY_WARNING => $warning_regexp,
- ArcanistLintSeverity::SEVERITY_ADVICE => $advice_regexp,
- );
+ 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.');
+ }
- 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;
- }
- }
- }
+ public function getLinterName() {
+ return 'PyLint';
+ }
- // If the message code doesn't match any of the provided regex's,
- // then just disable it.
- return ArcanistLintSeverity::SEVERITY_DISABLED;
+ public function getLinterConfigurationName() {
+ return 'pylint';
}
- private function getPyLintPath() {
- $pylint_bin = 'pylint';
+ public function getDefaultBinary() {
+ $prefix = $this->getDeprecatedConfiguration('lint.pylint.prefix');
+ $bin = $this->getDeprecatedConfiguration('lint.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 ($prefix) {
+ return $prefix.'/bin/'.$bin;
+ } else {
+ return $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'));
- }
- }
+ public function getVersion() {
+ list($stdout) = execx('%C --version', $this->getExecutableCommand());
- return $pylint_bin;
+ $matches = array();
+ $regex = '/^pylint (?P<version>\d+\.\d+\.\d+),/';
+ if (preg_match($regex, $stdout, $matches)) {
+ return $matches['version'];
+ } else {
+ return false;
+ }
}
- 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'),
- );
+ public function getInstallInstructions() {
+ return pht(
+ 'Install PyLint using `%s`.',
+ 'pip install pylint');
+ }
- // 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';
- }
- }
+ public function shouldExpectCommandErrors() {
+ return true;
+ }
- $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());
- }
- }
- }
+ public function getLinterConfigurationOptions() {
+ $options = array(
+ 'pylint.config' => array(
+ 'type' => 'optional string',
+ 'help' => pht('Pass in a custom configuration file path.'),
+ ),
+ );
- $python_path[] = '';
- return implode(':', $python_path);
+ return $options + parent::getLinterConfigurationOptions();
}
- private function getPyLintOptions() {
- // '-rn': don't print lint report/summary at end
- $options = array('-rn');
+ public function setLinterConfigurationValue($key, $value) {
+ switch ($key) {
+ case 'pylint.config':
+ $this->config = $value;
+ return;
- // 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}'");
+ default:
+ return parent::setLinterConfigurationValue($key, $value);
}
+ }
- $working_copy = $this->getEngine()->getWorkingCopy();
- $config = $this->getEngine()->getConfigurationManager();
+ protected function getMandatoryFlags() {
+ $options = array();
- // Specify an --rcfile, either absolute or relative to the project root.
+ $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.
- $rcfile = $config->getConfigFromAnySource('lint.pylint.rcfile');
- if ($rcfile !== null) {
- $rcfile = Filesystem::resolvePath(
- $rcfile,
- $working_copy->getProjectRoot());
- $options[] = csprintf('--rcfile=%s', $rcfile);
+ $config = $this->config;
+ if (!$config) {
+ $config = $this->getDeprecatedConfiguration('lint.pylint.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);
+ if ($config !== null) {
+ $options[] = '--rcfile='.$config;
}
- return implode(' ', $options);
- }
-
- public function getLinterName() {
- return 'PyLint';
+ return $options;
}
- private function getLinterVersion() {
- $pylint_bin = $this->getPyLintPath();
- $options = '--version';
+ protected function getDefaultFlags() {
+ $options = array();
- list($stdout) = execx('%s %s', $pylint_bin, $options);
+ // Add any options defined in the config file for PyLint.
+ $config_options = $this->getDeprecatedConfiguration(
+ 'lint.pylint.options',
+ array());
+ $options = array_merge($options, $config_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';
+ $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 $matches[1];
+ return $options;
}
- public function lintPath($path) {
- $pylint_bin = $this->getPyLintPath();
- $python_path = $this->getPyLintPythonPath();
- $options = $this->getPyLintOptions();
- $path_on_disk = $this->getEngine()->getFilePathOnDisk($path);
-
- 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();
- }
+ 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;
}
}

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 1:36 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7370119
Default Alt Text
D9109.id30870.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility