From c013124690e44296c3de0b49a0f0a6bfef03a106 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Mon, 5 May 2014 18:58:13 -0700 Subject: [PATCH] Convert more linters to modern formats with `.arclint` support Summary: Ref T3186. Ref T2039. - Convert JSHint to modern format; improve granularity of errors. - Convert PyFlakes to modern format; - Remove ApacheLicenseLinter and LicenseLinter (these have been deprecated for a very long time). This is somewhat disruptive and will break some users by no longer respecting various path/config options. I'll sequence documentation and deprecation warnings in front of these. Test Plan: Ran unit tests. Reviewers: btrahan, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin, joshuaspence, aran Maniphest Tasks: T3186, T2039 Differential Revision: https://secure.phabricator.com/D6810 --- src/__phutil_library_map__.php | 4 +- src/lint/linter/ArcanistPyFlakesLinter.php | 94 +++++++++---------- .../ArcanistPyFlakesLinterTestCase.php | 12 +++ .../linter/__tests__/jshint/jshint.lint-test | 11 +++ .../__tests__/pyflakes/pyflakes.lint-test | 7 ++ 5 files changed, 76 insertions(+), 52 deletions(-) create mode 100644 src/lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php create mode 100644 src/lint/linter/__tests__/jshint/jshint.lint-test create mode 100644 src/lint/linter/__tests__/pyflakes/pyflakes.lint-test diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9994911e..2c36f3b9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -124,6 +124,7 @@ phutil_register_library_map(array( 'ArcanistPhutilXHPASTLinter' => 'lint/linter/ArcanistPhutilXHPASTLinter.php', 'ArcanistPhutilXHPASTLinterTestCase' => 'lint/linter/__tests__/ArcanistPhutilXHPASTLinterTestCase.php', 'ArcanistPyFlakesLinter' => 'lint/linter/ArcanistPyFlakesLinter.php', + 'ArcanistPyFlakesLinterTestCase' => 'lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php', 'ArcanistPyLintLinter' => 'lint/linter/ArcanistPyLintLinter.php', 'ArcanistRepositoryAPI' => 'repository/api/ArcanistRepositoryAPI.php', 'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php', @@ -274,7 +275,8 @@ phutil_register_library_map(array( 'ArcanistPhutilTestTerminatedException' => 'Exception', 'ArcanistPhutilXHPASTLinter' => 'ArcanistBaseXHPASTLinter', 'ArcanistPhutilXHPASTLinterTestCase' => 'ArcanistArcanistLinterTestCase', - 'ArcanistPyFlakesLinter' => 'ArcanistLinter', + 'ArcanistPyFlakesLinter' => 'ArcanistExternalLinter', + 'ArcanistPyFlakesLinterTestCase' => 'ArcanistArcanistLinterTestCase', 'ArcanistPyLintLinter' => 'ArcanistLinter', 'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase', 'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase', diff --git a/src/lint/linter/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php index 25674364..6ca517cb 100644 --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -5,69 +5,54 @@ * * @group linter */ -final class ArcanistPyFlakesLinter extends ArcanistLinter { +final class ArcanistPyFlakesLinter extends ArcanistExternalLinter { public function getLinterName() { - return 'PyFlakes'; + return 'PYFLAKES'; } - public function getPyFlakesOptions() { - return null; + public function getLinterConfigurationName() { + return 'pyflakes'; } - public function lintPath($path) { + public function getDefaultBinary() { $config = $this->getEngine()->getConfigurationManager(); - $pyflakes_path = $config->getConfigFromAnySource('lint.pyflakes.path'); - $pyflakes_prefix = $config->getConfigFromAnySource('lint.pyflakes.prefix'); + $prefix = $config->getConfigFromAnySource('lint.pyflakes.prefix'); + $bin = $config->getConfigFromAnySource('lint.pyflakes.bin', 'pyflakes'); - // Default to just finding pyflakes in the users path - $pyflakes_bin = 'pyflakes'; - $python_path = array(); - - // If a pyflakes path was specified, then just use that as the - // pyflakes binary and assume that the libraries will be imported - // correctly. - // - // If no pyflakes path was specified and a pyflakes prefix was - // specified, then use the binary from this prefix and add it to - // the PYTHONPATH environment variable so that the libs are imported - // correctly. This is useful when pyflakes is installed into a - // non-default location. - if ($pyflakes_path !== null) { - $pyflakes_bin = $pyflakes_path; - } else if ($pyflakes_prefix !== null) { - $pyflakes_bin = $pyflakes_prefix.'/bin/pyflakes'; - $python_path[] = $pyflakes_prefix.'/lib/python2.7/site-packages'; - $python_path[] = $pyflakes_prefix.'/lib/python2.7/dist-packages'; - $python_path[] = $pyflakes_prefix.'/lib/python2.6/site-packages'; - $python_path[] = $pyflakes_prefix.'/lib/python2.6/dist-packages'; + if ($prefix) { + return $prefix.'/'.$bin; + } else { + return $bin; } - $python_path[] = ''; - $python_path = implode(':', $python_path); - $options = $this->getPyFlakesOptions(); + } - $f = new ExecFuture( - '/usr/bin/env PYTHONPATH=%s$PYTHONPATH %s %C', - $python_path, - $pyflakes_bin, - $options); - $f->write($this->getData($path)); + public function getVersion() { + list($stdout) = execx('%C --version', $this->getExecutableCommand()); - try { - list($stdout, $_) = $f->resolvex(); - } catch (CommandException $e) { - // PyFlakes will return an exit code of 1 if warnings/errors - // are found but print nothing to stderr in this case. Therefore, - // if we see any output on stderr or a return code other than 1 or 0, - // pyflakes failed. - if ($e->getError() !== 1 || $e->getStderr() !== '') { - throw $e; - } else { - $stdout = $e->getStdout(); - } + $matches = array(); + if (preg_match('/^(?P\d+\.\d+\.\d+)$/', $stdout, $matches)) { + return $matches['version']; + } else { + return false; } + } + + public function getInstallInstructions() { + return pht('Install pyflakes with `pip install pyflakes`.'); + } + + public function shouldExpectCommandErrors() { + return true; + } + + public function supportsReadDataFromStdin() { + return true; + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + $lines = phutil_split_lines($stdout, false); - $lines = explode("\n", $stdout); $messages = array(); foreach ($lines as $line) { $matches = null; @@ -92,8 +77,15 @@ final class ArcanistPyFlakesLinter extends ArcanistLinter { $message->setCode($this->getLinterName()); $message->setDescription($description); $message->setSeverity($severity); - $this->addLintMessage($message); + + $messages[] = $message; } + + if ($err && !$messages) { + return false; + } + + return $messages; } } diff --git a/src/lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php b/src/lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php new file mode 100644 index 00000000..aee34131 --- /dev/null +++ b/src/lint/linter/__tests__/ArcanistPyFlakesLinterTestCase.php @@ -0,0 +1,12 @@ +executeTestsInDirectory( + dirname(__FILE__).'/pyflakes/', + new ArcanistPyFlakesLinter()); + } + +} diff --git a/src/lint/linter/__tests__/jshint/jshint.lint-test b/src/lint/linter/__tests__/jshint/jshint.lint-test new file mode 100644 index 00000000..287f7e95 --- /dev/null +++ b/src/lint/linter/__tests__/jshint/jshint.lint-test @@ -0,0 +1,11 @@ +function f() { + for (ii = 0; ii < 3; ii++) { + g() + } +} + +{ + +~~~~~~~~~~ +warning:3:8 +error:7:1 \ No newline at end of file diff --git a/src/lint/linter/__tests__/pyflakes/pyflakes.lint-test b/src/lint/linter/__tests__/pyflakes/pyflakes.lint-test new file mode 100644 index 00000000..35d0afea --- /dev/null +++ b/src/lint/linter/__tests__/pyflakes/pyflakes.lint-test @@ -0,0 +1,7 @@ +import sys, os + +x += 1 +~~~~~~~~~~ +warning:1: +warning:1: +error:3: