Differential D10738 Diff 26566 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 `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.

				public function shouldExpectCommandErrors() {
				return true;
				}
				joshuaspenceUnsubmitted Not Done Inline Actions After D11322, this is the default behavior. joshuaspence: After D11322, this is the default behavior.

				public function supportsReadDataFromStdin() {
				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…
				return false;
				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.
				}
				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.

				protected function getMandatoryFlags() {
				$options = array(
				'--format=json',
				'--force-exclusion',
				'--no-color',
				);

				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();
				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)`.

				foreach ($results['files'] as $file) {
				foreach ($file['offenses'] as $offense) {
				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.
				$message = new ArcanistLintMessage();

				$message->setPath($file['path']);
				$message->setDescription($offense['message']);

				$message->setLine($offense['location']['line']);
				$message->setChar($offense['location']['column']);

				switch ($offense['severity']) {
				case 'warning':
				epriestleyUnsubmitted Not Done Inline Actions Prefer docblock style. epriestley: Prefer docblock style.
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING);
				break;
				case 'error':
				case 'fatal':
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
				break;
				default:
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE);
				break;
				}

				$message->setName($this->getLinterName());
				$message->setCode($offense['cop_name']);

				$messages[] = $message;
				}
				}

				return $messages;
				}
				}
				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.