Differential D11657 Diff 28290 src/lint/linter/ArcanistShellCheckLinter.php

Changeset View

Standalone View

src/lint/linter/ArcanistShellCheckLinter.php

This file was added.

				<?php

				final class ArcanistShellCheckLinter extends ArcanistExternalLinter {

				private $shell;

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

				public function getInfoURI() {
				return 'http://www.shellcheck.net/';
				}

				public function getInfoDescription() {
				return pht(
				'ShellCheck is a static analysis and linting tool for %s scripts.',
				'sh/bash');
				}

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

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

				public function getLinterConfigurationOptions() {
				$options = array(
				'shellcheck.shell' => array(
				'type' => 'optional string',
				'help' => pht(
				'Specify shell dialect (%s, %s, %s, %s).',
				'bash',
				'sh',
				'ksh',
				'zsh'),
				),
				);

				return $options + parent::getLinterConfigurationOptions();
				}

				public function setLinterConfigurationValue($key, $value) {
				switch ($key) {
				case 'shellcheck.shell':
				$this->setShell($value);
				return;

				default:
				return parent::setLinterConfigurationValue($key, $value);
				}
				}

				public function setShell($shell) {
				$this->shell = $shell;
				return $this;
				}

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

				public function getInstallInstructions() {
				return pht(
				'Install ShellCheck with `%s`.',
				'cabal install shellcheck');
				}

				public function supportsReadDataFromStdin() {
				return true;
				}

				public function getReadDataFromStdinFilename() {
				return '-';
				}
				epriestleyUnsubmitted Done Inline Actions If possible, let's get rid of this per T7674 (basically, favor `lint <filename>` form because a bunch of linters freak out and behave differently in subtle ways when given `lint <filename>` vs `echo file \| lint`, and everyone expects the `lint <filename>` behaviors). epriestley: If possible, let's get rid of this per T7674 (basically, favor `lint <filename>` form because a…

				protected function getMandatoryFlags() {
				$options = array();

				$options[] = '--format=checkstyle';

				if ($this->shell) {
				$options[] = '--shell='.$this->shell;
				}

				return $options;
				}

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

				$matches = null;
				if (preg_match('/^version: (\d(?:\.\d){2})$/', $stdout, $matches)) {
				return $matches[1];
				}

				return null;
				}

				protected function parseLinterOutput($path, $err, $stdout, $stderr) {
				$report_dom = new DOMDocument();
				$ok = @$report_dom->loadXML($stdout);

				if (!$ok) {
				return false;
				}

				$files = $report_dom->getElementsByTagName('file');
				$messages = array();

				foreach ($files as $file) {
				foreach ($file->getElementsByTagName('error') as $child) {
				$code = str_replace('ShellCheck.', '', $child->getAttribute('source'));

				$message = id(new ArcanistLintMessage())
				->setPath($path)
				->setLine($child->getAttribute('line'))
				->setChar($child->getAttribute('column'))
				->setCode($code)
				->setDescription($child->getAttribute('message'));

				switch ($child->getAttribute('severity')) {
				case 'error':
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
				break;

				case 'warning':
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING);
				break;

				case 'info':
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE);
				break;

				default:
				$message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
				break;
				}
				epriestleyUnsubmitted Not Done Inline Actions I think this is not correct, and leads to the same issue as D12034 (linter does not respect severity map). Basically: linters should not set severity; instead, they should set a code, and the code should imply a severity via getLintSeverityMap(). This will let users override the severity. (Possibly they may have to call `setSeverity($this->getLintMessageSeverity($code))` manually right now, but we should make that implicit if it isn't already.) epriestley: I think this is not correct, and leads to the same issue as D12034 (linter does not respect…
				cburroughsUnsubmitted Not Done Inline Actions As far as I can tell from existing examples, there isn't an implicit `setSeverity`. FWIW I was struggling with this for D10738 and find it confusing that `getDefaultMessageSeverity` needs to be overridden less as 'return a constant' and more as `parseALinterDependentString`. cburroughs: As far as I can tell from existing examples, there isn't an implicit `setSeverity`. FWIW I was…
				joshuaspenceAuthorUnsubmitted Not Done Inline Actions Right. But the intention of this block of code is not to allow custom severities but rather to respect the external linter severities, i.e. if the external linter flags a warning then we respect this severity. joshuaspence: Right. But the intention of this block of code is not to allow custom severities but rather to…

				$messages[] = $message;
				}
				}

				return $messages;
				}
				}