Differential D10738 Diff 29533 src/lint/linter/ArcanistRuboCopLinter.php

Changeset View

Standalone View

src/lint/linter/ArcanistRuboCopLinter.php

This file was added.

				<?php

				final class ArcanistRuboCopLinter extends ArcanistExternalLinter {

				private $config;

				public function getInfoName() {
				return 'RuboCop';
				}

				public function getInfoURI() {
				return 'http://batsov.com/rubocop';
				}

				public function getInfoDescription() {
				return pht(
				'RuboCop is a Ruby static code analyzer, based on the community Ruby '.
				'style guide.');
				}

				public function getLinterName() {
				return 'RuboCop';
				}

				public function getLinterConfigurationName() {
				return 'rubocop';
				}

				public function getDefaultBinary() {
				return 'rubocop';
				}

				public function getVersion() {
				list($stdout) = execx('%C --version', $this->getExecutableCommand());

				$matches = array();
				if (preg_match('/^(?P<version>\d+\.\d+\.\d+)$/', $stdout, $matches)) {
				return $matches['version'];
				} else {
				return false;
				}
				}

				public function getInstallInstructions() {
				return pht('Install RuboCop using `%s`.', 'gem install rubocop');
				}
				joshuaspenceUnsubmitted Not Done Inline Actions Prefer to write this as `pht('Install RuboCop using` %s`, 'gem install rubocop') `because` 'gem install rubocop'` isn't really translatable. joshuaspence: Prefer to write this as `pht('Install RuboCop using `%s`, 'gem install rubocop')` because `'gem…
				joshuaspenceUnsubmitted Not Done Inline Actions This looks pretty awkward, try `pht('Install RuboCop using` `%s`.', 'gem install rubocop') instead. joshuaspence: This looks pretty awkward, try `pht('Install RuboCop using ```%s```.', 'gem install rubocop')`…
				cburroughsAuthorUnsubmitted Not Done Inline Actions Done. cburroughs: Done.

				protected function getMandatoryFlags() {
				$options = array(
				'--format=json',
				);
				joshuaspenceUnsubmitted Not Done Inline Actions After D11322, this is the default behavior. joshuaspence: After D11322, this is the default behavior.

				joshuaspenceUnsubmitted Not Done Inline Actions I think that maybe it would be better (or, at least, more consistent) to remove this flag and to, instead, use the `.arclint` file to manage exclusions. joshuaspence: I think that //maybe// it would be better (or, at least, more consistent) to remove this flag…
				cburroughsAuthorUnsubmitted Not Done Inline Actions This flag is confusing https://github.com/bbatsov/rubocop/issues/893 With this flag: rubocop exclusions win over arclint inclusions. Without this flag: rubocop exclusions won't matter for the purposes of arc lint. So there is probably some complicated case where it is useful, but I agree it's probably more consistent with the rest of arcanist to remove. cburroughs: This flag is confusing https://github.com/bbatsov/rubocop/issues/893 With this flag: rubocop…
				if ($this->config) {
				joshuaspenceUnsubmitted Not Done Inline Actions Probably not required with `--format=json` joshuaspence: Probably not required with `--format=json`
				cburroughsAuthorUnsubmitted Not Done Inline Actions Done. cburroughs: Done.
				$options[] = '--config='.$this->config;
				}
				joshuaspenceUnsubmitted Not Done Inline Actions This is the default behavior, so you can just omit this method entirely. joshuaspence: This is the default behavior, so you can just omit this method entirely.

				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())
				joshuaspenceUnsubmitted Not Done Inline Actions Prefer `phutil_json_decode` or, at the very least, `json_decode($stdout, true)`. joshuaspence: Prefer `phutil_json_decode` or, at the very least, `json_decode($stdout, true)`.
				->setCode($offense['cop_name']);
				$messages[] = $message;
				}
				joshuaspenceUnsubmitted Not Done Inline Actions Prefer to write this by chaining the `set` methods together. joshuaspence:* Prefer to write this by chaining the `set*` methods together.
				cburroughsAuthorUnsubmitted Not Done Inline Actions Done. cburroughs: Done.
				}

				return $messages;
				}

				/*
				Take the string from RuboCop's severity terminology and return an
				ArcanistLintSeverity
				*/
				epriestleyUnsubmitted Not Done Inline Actions Prefer docblock style. epriestley: Prefer docblock style.
				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;
				}
				}

				}
				joshuaspenceUnsubmitted Not Done Inline Actions This method seems a little messy. I'd probably just move it inline. joshuaspence: This method seems a little messy. I'd probably just move it inline.