From 41ddd34aeb26be5760107df4d87be7a3da7e2ad0 Mon Sep 17 00:00:00 2001 From: cburroughs Date: Mon, 6 Apr 2015 09:11:06 -0700 Subject: [PATCH] Added RuboCop linter Test Plan: Add the following JSON to your `.arclint`: ```lang=json "rubocop": { "type": "rubocop", "include": "(\\.rb$)" } ``` Then, add a ruby file with errors, for instance: ```lang=ruby def hello() puts 'world' end ``` Run `arc lint`. It should come up with something along the line of: "Omit the parentheses in defs when the method doesn't accept any arguments." Reviewers: joshuaspence, remon, #blessed_reviewers, epriestley Reviewed By: joshuaspence, #blessed_reviewers, epriestley Subscribers: reu, calfzhou, jjooss, cburroughs, chad, Korvin, epriestley Differential Revision: https://secure.phabricator.com/D10738 --- src/__phutil_library_map__.php | 4 + src/lint/linter/ArcanistRuboCopLinter.php | 120 ++++++++++++++++++ .../ArcanistRuboCopLinterTestCase.php | 10 ++ .../__tests__/rubocop/convention.lint-test | 4 + .../linter/__tests__/rubocop/error.lint-test | 4 + .../__tests__/rubocop/no_errors.lint-test | 4 + .../__tests__/rubocop/warning.lint-test | 5 + 7 files changed, 151 insertions(+) create mode 100644 src/lint/linter/ArcanistRuboCopLinter.php create mode 100644 src/lint/linter/__tests__/ArcanistRuboCopLinterTestCase.php create mode 100644 src/lint/linter/__tests__/rubocop/convention.lint-test create mode 100644 src/lint/linter/__tests__/rubocop/error.lint-test create mode 100644 src/lint/linter/__tests__/rubocop/no_errors.lint-test create mode 100644 src/lint/linter/__tests__/rubocop/warning.lint-test diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0a56871d..85e59010 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -161,6 +161,8 @@ phutil_register_library_map(array( 'ArcanistRepositoryAPIMiscTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIMiscTestCase.php', 'ArcanistRepositoryAPIStateTestCase' => 'repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php', 'ArcanistRevertWorkflow' => 'workflow/ArcanistRevertWorkflow.php', + 'ArcanistRuboCopLinter' => 'lint/linter/ArcanistRuboCopLinter.php', + 'ArcanistRuboCopLinterTestCase' => 'lint/linter/__tests__/ArcanistRuboCopLinterTestCase.php', 'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php', 'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php', 'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php', @@ -347,6 +349,8 @@ phutil_register_library_map(array( 'ArcanistRepositoryAPIMiscTestCase' => 'ArcanistTestCase', 'ArcanistRepositoryAPIStateTestCase' => 'ArcanistTestCase', 'ArcanistRevertWorkflow' => 'ArcanistWorkflow', + 'ArcanistRuboCopLinter' => 'ArcanistExternalLinter', + 'ArcanistRuboCopLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistRubyLinter' => 'ArcanistExternalLinter', 'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistScriptAndRegexLinter' => 'ArcanistLinter', diff --git a/src/lint/linter/ArcanistRuboCopLinter.php b/src/lint/linter/ArcanistRuboCopLinter.php new file mode 100644 index 00000000..0647a7bc --- /dev/null +++ b/src/lint/linter/ArcanistRuboCopLinter.php @@ -0,0 +1,120 @@ +getExecutableCommand()); + + $matches = array(); + if (preg_match('/^(?P\d+\.\d+\.\d+)$/', $stdout, $matches)) { + return $matches['version']; + } else { + return false; + } + } + + public function getInstallInstructions() { + return pht('Install RuboCop using `%s`.', 'gem install rubocop'); + } + + protected function getMandatoryFlags() { + $options = array( + '--format=json', + ); + + if ($this->config) { + $options[] = '--config='.$this->config; + } + + return $options; + } + + public function getLinterConfigurationOptions() { + $options = array( + 'rubocop.config' => array( + 'type' => 'optional string', + 'help' => pht('A custom configuration file.'), + ), + ); + + return $options + parent::getLinterConfigurationOptions(); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'rubocop.config': + $this->config = $value; + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + $results = phutil_json_decode($stdout); + $messages = array(); + + foreach ($results['files'] as $file) { + foreach ($file['offenses'] as $offense) { + $message = id(new ArcanistLintMessage()) + ->setPath($file['path']) + ->setDescription($offense['message']) + ->setLine($offense['location']['line']) + ->setChar($offense['location']['column']) + ->setSeverity($this->getLintMessageSeverity($offense['severity'])) + ->setName($this->getLinterName()) + ->setCode($offense['cop_name']); + $messages[] = $message; + } + } + + return $messages; + } + + /* + Take the string from RuboCop's severity terminology and return an + ArcanistLintSeverity + */ + protected function getDefaultMessageSeverity($code) { + switch ($code) { + case 'convention': + case 'refactor': + case 'warning': + return ArcanistLintSeverity::SEVERITY_WARNING; + case 'error': + case 'fatal': + return ArcanistLintSeverity::SEVERITY_ERROR; + default: + return ArcanistLintSeverity::SEVERITY_ADVICE; + } + } + +} diff --git a/src/lint/linter/__tests__/ArcanistRuboCopLinterTestCase.php b/src/lint/linter/__tests__/ArcanistRuboCopLinterTestCase.php new file mode 100644 index 00000000..1838cdf6 --- /dev/null +++ b/src/lint/linter/__tests__/ArcanistRuboCopLinterTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/rubocop/'); + } + +} diff --git a/src/lint/linter/__tests__/rubocop/convention.lint-test b/src/lint/linter/__tests__/rubocop/convention.lint-test new file mode 100644 index 00000000..6d91a080 --- /dev/null +++ b/src/lint/linter/__tests__/rubocop/convention.lint-test @@ -0,0 +1,4 @@ +def hello() +end +~~~~~~~~~~ +warning:1:10 diff --git a/src/lint/linter/__tests__/rubocop/error.lint-test b/src/lint/linter/__tests__/rubocop/error.lint-test new file mode 100644 index 00000000..32296dc4 --- /dev/null +++ b/src/lint/linter/__tests__/rubocop/error.lint-test @@ -0,0 +1,4 @@ +def hello + puts 'world' +~~~~~~~~~~ +error:3:1 diff --git a/src/lint/linter/__tests__/rubocop/no_errors.lint-test b/src/lint/linter/__tests__/rubocop/no_errors.lint-test new file mode 100644 index 00000000..46226a85 --- /dev/null +++ b/src/lint/linter/__tests__/rubocop/no_errors.lint-test @@ -0,0 +1,4 @@ +def hello + puts 'hello world' +end +~~~~~~~~~~ diff --git a/src/lint/linter/__tests__/rubocop/warning.lint-test b/src/lint/linter/__tests__/rubocop/warning.lint-test new file mode 100644 index 00000000..8cec4c72 --- /dev/null +++ b/src/lint/linter/__tests__/rubocop/warning.lint-test @@ -0,0 +1,5 @@ +def hello(unused) + puts 'hi' +end +~~~~~~~~~ +warning:1:11