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 sh/bash scripts.'); | |||||
} | |||||
public function getLinterName() { | |||||
return 'SHELLCHECK'; | |||||
} | |||||
public function getLinterConfigurationName() { | |||||
return 'shellcheck'; | |||||
} | |||||
public function getLinterConfigurationOptions() { | |||||
$options = array( | |||||
'shellcheck.shell' => array( | |||||
'type' => 'optional string', | |||||
'help' => pht('Specify dialect (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 `cabal install shellcheck`.'); | |||||
} | |||||
public function supportsReadDataFromStdin() { | |||||
return true; | |||||
} | |||||
public function getReadDataFromStdinFilename() { | |||||
return '-'; | |||||
} | |||||
protected function getMandatoryFlags() { | |||||
$options = array(); | |||||
$options[] = '--format=checkstyle'; | |||||
if ($this->shell) { | |||||
$options[] = '--shell='.$this->shell; | |||||
} | |||||
epriestley: If possible, let's get rid of this per T7674 (basically, favor `lint <filename>` form because a… | |||||
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->childNodes 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; | |||||
} | |||||
$messages[] = $message; | |||||
} | |||||
} | |||||
return $messages; | |||||
} | |||||
} | |||||
Not Done Inline ActionsI 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… | |||||
Not Done Inline ActionsAs 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… | |||||
Not Done Inline ActionsRight. 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… |
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).