Page MenuHomePhabricator
Files F15437019

D9109.id21793.diff
No OneTemporary
Actions

D9109.id21793.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
@@ -139,6 +139,7 @@
'ArcanistPyFlakesLinter' => 'lint/linter/ArcanistPyFlakesLinter.php',
'ArcanistPyFlakesLinterTestCase' => 'lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php',
'ArcanistPyLintLinter' => 'lint/linter/ArcanistPyLintLinter.php',
+ 'ArcanistPyLintLinterTestCase' => 'lint/linter/__tests__/ArcanistPyLintLinterTestCase.php',
'ArcanistRepositoryAPI' => 'repository/api/ArcanistRepositoryAPI.php',
'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php',
'ArcanistRepositoryAPIStateTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php',
@@ -305,7 +306,8 @@
'ArcanistPuppetLintLinterTestCase' => 'ArcanistArcanistLinterTestCase',
'ArcanistPyFlakesLinter' => 'ArcanistExternalLinter',
'ArcanistPyFlakesLinterTestCase' => 'ArcanistArcanistLinterTestCase',
- 'ArcanistPyLintLinter' => 'ArcanistLinter',
+ 'ArcanistPyLintLinter' => 'ArcanistExternalLinter',
+ 'ArcanistPyLintLinterTestCase' => 'ArcanistArcanistLinterTestCase',
'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase',
'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase',
'ArcanistRevertWorkflow' => 'ArcanistBaseWorkflow',
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,274 +1,151 @@
<?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.
- *
- * @group linter
+ * 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();
-
- $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(
- "You are invoking the PyLint linter but have not configured any of ".
- "'lint.pylint.codes.error', 'lint.pylint.codes.warning', or ".
- "'lint.pylint.codes.advice'. Consult the documentation for ".
- "ArcanistPyLintLinter.");
- }
-
- $code_map = array(
- ArcanistLintSeverity::SEVERITY_ERROR => $error_regexp,
- ArcanistLintSeverity::SEVERITY_WARNING => $warning_regexp,
- ArcanistLintSeverity::SEVERITY_ADVICE => $advice_regexp,
- );
+ public function getInfoName() {
+ return 'PyLint';
+ }
- 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 getInfoURI() {
+ return 'http://www.pylint.org/';
+ }
- // If the message code doesn't match any of the provided regex's,
- // then just disable it.
- return ArcanistLintSeverity::SEVERITY_DISABLED;
+ 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.');
}
- private function getPyLintPath() {
- $pylint_bin = "pylint";
+ public function getLinterName() {
+ return '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;
- }
+ public function getLinterConfigurationName() {
+ return 'pylint';
+ }
- if (!Filesystem::pathExists($pylint_bin)) {
+ public function getDefaultBinary() {
+ $prefix = $this->getDeprecatedConfiguration('lint.pylint.prefix');
+ $bin = $this->getDeprecatedConfiguration('lint.pylint.bin', 'pylint');
- list($err) = exec_manual('which %s', $pylint_bin);
- if ($err) {
- throw new ArcanistUsageException(
- "PyLint does not appear to be installed on this system. Install it ".
- "(e.g., with 'sudo easy_install pylint') or configure ".
- "'lint.pylint.prefix' in your .arcconfig to point to the directory ".
- "where it resides.");
- }
+ if ($prefix) {
+ return $prefix.'/bin/'.$bin;
+ } else {
+ return $bin;
}
-
- return $pylint_bin;
}
- 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';
- }
- }
+ public function getVersion() {
+ list($stdout) = execx('%C --version', $this->getExecutableCommand());
- $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());
- }
- }
+ $matches = array();
+ $regex = '/^pylint (?P<version>\d+\.\d+\.\d+),$/m';
+ if (preg_match($regex, $stdout, $matches)) {
+ return $matches['version'];
+ } else {
+ return false;
}
+ }
- $python_path[] = '';
- return implode(":", $python_path);
+ public function getInstallInstructions() {
+ return pht('Install PyLint using `pip install pylint`.');
}
- private function getPyLintOptions() {
- // '-rn': don't print lint report/summary at end
- $options = array('-rn');
+ public function shouldExpectCommandErrors() {
+ return true;
+ }
- // 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}'");
- }
+ public function getMandatoryFlags() {
+ return array(
+ '--reports=no',
+ '--msg-template="{line}|{column}|{msg_id}|{symbol}|{msg}"',
+ );
+ }
- $working_copy = $this->getEngine()->getWorkingCopy();
- $config = $this->getEngine()->getConfigurationManager();
+ public function getDefaultFlags() {
+ $options = array();
// 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');
+ $rcfile = $this->getDeprecatedConfiguration('lint.pylint.rcfile');
if ($rcfile !== null) {
- $rcfile = Filesystem::resolvePath(
- $rcfile,
- $working_copy->getProjectRoot());
- $options[] = csprintf('--rcfile=%s', $rcfile);
+ $options[] = '--rcfile='.$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);
- }
+ // Add any options defined in the config file for PyLint.
+ $config_options = $this->getDeprecatedConfiguration(
+ 'lint.pylint.options',
+ array());
+ $options = array_merge($options, $config_options);
- return implode(" ", $options);
+ return $options;
}
- public function getLinterName() {
- return 'PyLint';
- }
-
- private function getLinterVersion() {
-
- $pylint_bin = $this->getPyLintPath();
- $options = '--version';
+ 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;
+ }
- list($stdout) = execx(
- '%s %s',
- $pylint_bin,
- $options);
+ $lines = phutil_split_lines($stdout, false);
+ $messages = array();
- $lines = explode("\n", $stdout);
- $matches = null;
+ foreach ($lines as $line) {
+ $matches = explode('|', $line, 5);
+
+ if (count($matches) === 5) {
+ $message = new ArcanistLintMessage();
+ $message->setPath($path);
+ $message->setLine($matches[0]);
+ $message->setChar($matches[1]);
+ $message->setCode($matches[2]);
+ $message->setName(ucwords(str_replace('-', ' ', $matches[3])));
+ $message->setDescription($matches[4]);
+ $message->setSeverity($this->getMessageCodeSeverity($matches[2]));
+
+ $messages[] = $message;
+ }
+ }
- // 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";
+ if ($err && !$messages) {
+ return false;
}
- return $matches[1];
+ return $messages;
}
- 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 getMessageCodeSeverity($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;
}
+ }
- $lines = explode("\n", $stdout);
- $messages = array();
- foreach ($lines as $line) {
- $matches = null;
- if (!preg_match(
- '/([A-Z]\d+): *(\d+)(?:|,\d*): *(.*)$/',
- $line, $matches)) {
- 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);
+ 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;
}
}
diff --git a/src/lint/linter/__tests__/ArcanistPyLintLinterTestCase.php b/src/lint/linter/__tests__/ArcanistPyLintLinterTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/ArcanistPyLintLinterTestCase.php
@@ -0,0 +1,12 @@
+<?php
+
+final class ArcanistPyLintLinterTestCase
+ extends ArcanistArcanistLinterTestCase {
+
+ public function testPyLintLinter() {
+ return $this->executeTestsInDirectory(
+ dirname(__FILE__).'/pylint/',
+ new ArcanistPyLintLinter());
+ }
+
+}
diff --git a/src/lint/linter/__tests__/pylint/pylint.lint-test b/src/lint/linter/__tests__/pylint/pylint.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/pylint/pylint.lint-test
@@ -0,0 +1,7 @@
+import sys, os
+
+x += 1
+~~~~~~~~~~
+warning:1:0
+advice:1:0
+error:3:0

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 5:03 PM (1 w, 5 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7717296
Default Alt Text
D9109.id21793.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility