From 6e5be59ad618d32f116e9348e97d9be9ecf6d79f Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 23 Aug 2013 11:52:54 -0700 Subject: [PATCH] Port flake8 to ArcanistExternalLinter Summary: Ref T3186. Brings another linter onboard. This one uses the stdin stuff. The unit test was ostensibly broken so I fixed it, but that might just be some kind of version issue. Test Plan: Unit tests. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3186 Differential Revision: https://secure.phabricator.com/D6802 --- src/__phutil_library_map__.php | 2 +- src/lint/linter/ArcanistExternalLinter.php | 2 +- src/lint/linter/ArcanistFlake8Linter.php | 79 ++++++++----------- .../ArcanistFlake8LinterTestCase.php | 2 +- .../{python => flake8}/undefined.lint-test | 4 +- 5 files changed, 40 insertions(+), 49 deletions(-) rename src/lint/linter/__tests__/{python => flake8}/undefined.lint-test (67%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 64785c92..627d50f7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -221,7 +221,7 @@ phutil_register_library_map(array( 'ArcanistFeatureWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistFilenameLinter' => 'ArcanistLinter', 'ArcanistFlagWorkflow' => 'ArcanistBaseWorkflow', - 'ArcanistFlake8Linter' => 'ArcanistLinter', + 'ArcanistFlake8Linter' => 'ArcanistExternalLinter', 'ArcanistFlake8LinterTestCase' => 'ArcanistArcanistLinterTestCase', 'ArcanistFutureLinter' => 'ArcanistLinter', 'ArcanistGeneratedLinter' => 'ArcanistLinter', diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index d8423754..4cb64f1c 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -371,7 +371,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter { '%C %C', $bin, $this->getReadDataFromStdinFilename()); - $future->write($this->getFileData($path)); + $future->write($this->getEngine()->loadData($path)); } else { // TODO: In commit hook mode, we need to do more handling here. $disk_path = $this->getEngine()->getFilePathOnDisk($path); diff --git a/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php index 2a7690a4..61aa644c 100644 --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -6,17 +6,15 @@ * * @group linter */ -final class ArcanistFlake8Linter extends ArcanistLinter { - - public function willLintPaths(array $paths) { - return; - } +final class ArcanistFlake8Linter extends ArcanistExternalLinter { public function getLinterName() { return 'flake8'; } - public function getFlake8Options() { + public function getDefaultFlags() { + // TODO: Deprecated. + $working_copy = $this->getEngine()->getWorkingCopy(); $options = $working_copy->getConfig('lint.flake8.options', ''); @@ -24,59 +22,44 @@ final class ArcanistFlake8Linter extends ArcanistLinter { return $options; } - public function getFlake8Path() { + public function getDefaultBinary() { $working_copy = $this->getEngine()->getWorkingCopy(); $prefix = $working_copy->getConfig('lint.flake8.prefix'); $bin = $working_copy->getConfig('lint.flake8.bin', 'flake8'); - if ($prefix !== null) { - if (!Filesystem::pathExists($prefix.'/'.$bin)) { - throw new ArcanistUsageException( - "Unable to find flake8 binary in a specified directory. Make sure ". - "that 'lint.flake8.prefix' and 'lint.flake8.bin' keys are set ". - "correctly. If you'd rather use a copy of flake8 installed ". - "globally, you can just remove these keys from your .arcconfig."); - } - - $bin = csprintf("%s/%s", $prefix, $bin); - - return $bin; + if ($prefix || ($bin != 'flake8')) { + return $prefix.'/'.$bin; } - // Look for globally installed flake8 - list($err) = exec_manual('which %s', $bin); - if ($err) { - throw new ArcanistUsageException( - "flake8 does not appear to be installed on this system. Install it ". - "(e.g., with 'easy_install flake8') or configure ". - "'lint.flake8.prefix' in your .arcconfig to point to the directory ". - "where it resides."); - } - - return $bin; + return 'flake8'; } - public function lintPath($path) { - $flake8_bin = $this->getFlake8Path(); - $options = $this->getFlake8Options(); + public function getInstallInstructions() { + return pht('Install flake8 using `easy_install flake8`.'); + } - $f = new ExecFuture("%C %C -", $flake8_bin, $options); - $f->write($this->getData($path)); + public function supportsReadDataFromStdin() { + return true; + } - list($err, $stdout, $stderr) = $f->resolve(); + public function getReadDataFromStdinFilename() { + return '-'; + } - if ($err === 2) { - throw new Exception("flake8 failed to run correctly:\n".$stderr); - } + public function shouldExpectCommandErrors() { + return true; + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + $lines = phutil_split_lines($stdout, $retain_endings = false); - $lines = explode("\n", $stdout); $messages = array(); foreach ($lines as $line) { $matches = null; // stdin:2: W802 undefined name 'foo' # pyflakes // stdin:3:1: E302 expected 2 blank lines, found 1 # pep8 - if (!preg_match('/^(.*?):(\d+):(?:(\d+):)? (\S+) (.*)$/', - $line, $matches)) { + $regexp = '/^(.*?):(\d+):(?:(\d+):)? (\S+) (.*)$/'; + if (!preg_match($regexp, $line, $matches)) { continue; } foreach ($matches as $key => $match) { @@ -92,14 +75,22 @@ final class ArcanistFlake8Linter extends ArcanistLinter { $message = new ArcanistLintMessage(); $message->setPath($path); $message->setLine($matches[2]); - if (!empty($matches[3])) + if (!empty($matches[3])) { $message->setChar($matches[3]); + } $message->setCode($matches[4]); $message->setName($this->getLinterName().' '.$matches[3]); $message->setDescription($matches[5]); $message->setSeverity($severity); - $this->addLintMessage($message); + + $messages[] = $message; } + + if ($err && !$messages) { + return false; + } + + return $messages; } } diff --git a/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php b/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php index 48a0f5c3..ec89c043 100644 --- a/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistFlake8LinterTestCase.php @@ -9,7 +9,7 @@ final class ArcanistFlake8LinterTestCase public function testFlake8Lint() { return $this->executeTestsInDirectory( - dirname(__FILE__).'/python/', + dirname(__FILE__).'/flake8/', new ArcanistFlake8Linter()); } diff --git a/src/lint/linter/__tests__/python/undefined.lint-test b/src/lint/linter/__tests__/flake8/undefined.lint-test similarity index 67% rename from src/lint/linter/__tests__/python/undefined.lint-test rename to src/lint/linter/__tests__/flake8/undefined.lint-test index 72f6dfc2..c1fe50c9 100644 --- a/src/lint/linter/__tests__/python/undefined.lint-test +++ b/src/lint/linter/__tests__/flake8/undefined.lint-test @@ -3,5 +3,5 @@ x = 'y' def hello(): return foo ~~~~~~~~~~ -warning:1:8 -warning:4: +error:3:1 +warning:4:1