diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -168,6 +168,8 @@ 'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php', 'ArcanistSetConfigWorkflow' => 'workflow/ArcanistSetConfigWorkflow.php', 'ArcanistSettings' => 'configuration/ArcanistSettings.php', + 'ArcanistShellCheckLinter' => 'lint/linter/ArcanistShellCheckLinter.php', + 'ArcanistShellCheckLinterTestCase' => 'lint/linter/__tests__/ArcanistShellCheckLinterTestCase.php', 'ArcanistShellCompleteWorkflow' => 'workflow/ArcanistShellCompleteWorkflow.php', 'ArcanistSingleLintEngine' => 'lint/engine/ArcanistSingleLintEngine.php', 'ArcanistSpellingLinter' => 'lint/linter/ArcanistSpellingLinter.php', @@ -354,6 +356,8 @@ 'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistScriptAndRegexLinter' => 'ArcanistLinter', 'ArcanistSetConfigWorkflow' => 'ArcanistWorkflow', + 'ArcanistShellCheckLinter' => 'ArcanistExternalLinter', + 'ArcanistShellCheckLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistShellCompleteWorkflow' => 'ArcanistWorkflow', 'ArcanistSingleLintEngine' => 'ArcanistLintEngine', 'ArcanistSpellingLinter' => 'ArcanistLinter', diff --git a/src/lint/linter/ArcanistShellCheckLinter.php b/src/lint/linter/ArcanistShellCheckLinter.php new file mode 100644 --- /dev/null +++ b/src/lint/linter/ArcanistShellCheckLinter.php @@ -0,0 +1,141 @@ + 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'); + } + + 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; + } + + $messages[] = $message; + } + } + + return $messages; + } +} diff --git a/src/lint/linter/__tests__/ArcanistShellCheckLinterTestCase.php b/src/lint/linter/__tests__/ArcanistShellCheckLinterTestCase.php new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/ArcanistShellCheckLinterTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/shellcheck/'); + } + +} diff --git a/src/lint/linter/__tests__/shellcheck/backticks.lint-test b/src/lint/linter/__tests__/shellcheck/backticks.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/shellcheck/backticks.lint-test @@ -0,0 +1,5 @@ +#!/bin/sh +`backticks are cool` +~~~~~~~~~~ +warning:2:1 +advice:2:1 diff --git a/src/lint/linter/__tests__/shellcheck/bash-config.lint-test b/src/lint/linter/__tests__/shellcheck/bash-config.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/shellcheck/bash-config.lint-test @@ -0,0 +1,12 @@ +#!/bin/sh +if [[ $X ]]; then + echo "derp" +fi +~~~~~~~~~~ +~~~~~~~~~~ +~~~~~~~~~~ +{ + "config": { + "shellcheck.shell": "bash" + } +} diff --git a/src/lint/linter/__tests__/shellcheck/bash-shebang.lint-test b/src/lint/linter/__tests__/shellcheck/bash-shebang.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/shellcheck/bash-shebang.lint-test @@ -0,0 +1,5 @@ +#!/bin/bash +if [[ $X ]]; then + echo "derp" +fi +~~~~~~~~~~ diff --git a/src/lint/linter/__tests__/shellcheck/bashism.lint-test b/src/lint/linter/__tests__/shellcheck/bashism.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/shellcheck/bashism.lint-test @@ -0,0 +1,6 @@ +#!/bin/sh +if [[ $X ]]; then + echo "derp" +fi +~~~~~~~~~~ +warning:2:4 diff --git a/src/lint/linter/__tests__/shellcheck/syntax-error.lint-test b/src/lint/linter/__tests__/shellcheck/syntax-error.lint-test new file mode 100644 --- /dev/null +++ b/src/lint/linter/__tests__/shellcheck/syntax-error.lint-test @@ -0,0 +1,6 @@ +#!/bin/sh +$( +~~~~~~~~~~ +error:2:1 +advice:2:1 +error:3:1