Shellcheck checker

[swsnr]

See http://www.shellcheck.net/about.html

[yasuyk]

I will try it. I'm going to name this syntax checker sh-shellcheck. what do you think about it?

[swsnr]

Don't know. Seems fitting, but I'm not sure what Shellcheck actually checks. Is it Bash or POSIX sh?

[yasuyk]

Quoting http://www.shellcheck.net/about.html:

ShellCheck is a static analysis and linting tool for sh/bash scripts

It seems Bash and POSIX sh.

[swsnr]

Cool :-) I wonder how it tells them apart, though. Is there some kind of an option?

[yasuyk]

See source code, https://github.com/koalaman/shellcheck/blob/0ec62390d57480a449afb7d450054a28de63fc8a/ShellCheck/Analytics.hs#L80-L87 .

It decide shell by shebang. Oh, shellcheck support zsh and ksh, also!

[swsnr]

Wow. But what if the file being checked has no shebang? Can we force shellcheck to assume a specific shell?

Emacs' shell mode already guesses the shell, so shellcheck should not guess again, lest it guesses something different and reports false errors.

[swsnr]

Sorry for asking all these questions, SVD letting you do all the research. I'm not at my computer, and reading source code on the phone just doesn't work well.

[yasuyk]

@lunaryorn No problem.

Wow. But what if the file being checked has no shebang?

If there is no sheebang, shellcheck regard shell as Bash.

Can we force shellcheck to assume a specific shell?

shellcheck currently has no option to assume a specific shell.

[swsnr]

If there is no sheebang, shellcheck regard shell as Bash.

That's bad. My .zshrc doesn't have any Shebang, and it's surely no Bash code :)

shellcheck currently has no option to assume a specific shell.

Imho, we need such an option before we can think about adding this syntax checker. Should we open an issue at Shellcheck?

[yasuyk]

OK, I opened an issue.

[swsnr]

@yasuyk Thank you. We'll see. I hope that the issue is closed quickly. Shellcheck looks really cool :)

[yasuyk]

Right! I had not known about Shellcheck until I see this issue, and Shellcheck is very useful after I used it. :-)

[koalaman]

Shellcheck's commit 4968e7d9f adds a -s for this, e.g. shellcheck -s ksh filename!

It supports bash, sh, ksh and zsh, along with some aliases like dash, ash and ksh93.

[yasuyk]

@koalaman Great! Thank you for the awesome work! When do you update version which includes this feature on hackage?

[swsnr]

@yasuyk With this new option in Shellcheck, we should add a single syntax checker sh-shellcheck, along the following lines:

(defconst flycheck-shellcheck-supported-shells '((bash . "bash") (sh . "sh") (ksh88 . "ksh") (zsh . "zsh")))

(flycheck-define-checker sh-shellcheck
  :command ("shellcheck" "-s" (eval (cdr (assq sh-shell flycheck-shellcheck-supported-shells))) source)
  :modes sh-mode
  :predicate (lambda () (assq sh-shell flycheck-shellcheck-supported-shells))
  :error-patterns …)

As part of this PR, we should rename all existing shell script checkers to follow the pattern sh-<SHELLTYPE>, e.g. sh-zsh, sh-bash, etc. The current sh-* checkers would go as sh-posix-dash and sh-posix-bash then. That'd easy chaining to shell check.

What do you think?

[yasuyk]

Excellent! I agree with you. 👍

[yasuyk]

@lunaryorn Shall I rename all existing shell script checkers?

[swsnr]

I'm already at it.

[yasuyk]

OK, then I will tackle Spellcheck checker.

[swsnr]

@yasuyk See 049babb

[swsnr]

Since Shellcheck now supports ksh88, we can simplify it a bit imho. Also I noticed that Shellcheck can output Checkstyle XML, so we do not need custom error patterns. We can just use the builtin flycheck-parse-checkstyle parser:

(defconst flycheck-shellcheck-supported-shells '(bash sh ksh88 zsh)))

(flycheck-define-checker sh-shellcheck
  :command ("shellcheck" "-f" "checkstyle" "-s" (eval (symbol-name sh-shell)) source)
  :modes sh-mode
  :predicate (lambda () (memq sh-shell flycheck-shellcheck-supported-shells))
  :error-parser flycheck-parse-checkstyle)

[yasuyk]

Wow, quick work!

[swsnr]

@yasuyk Please take a look at the shell-check branch. I added the necessary code to install shellcheck on the VM, and I also added the syntax checker itself, including the chaining from the shell checkers. Consider it as a draft of this checker.

I didn't test it, nor add any tests, though. Let me know what you think, and whether you can work with these changes.

[yasuyk]

@lunaryorn OK. I have written test. I will merge tests to shell-check brantch.

[yasuyk]

@lunaryorn I added test for shel-check. make test on OS X is passed. make vagrant-test is in process now. My Vagrant environment is very slow..

[yasuyk]

make vagrant-test has been done, and test cases for sh-* checkers are passed.

[swsnr]

@yasuyk By the way, if you add a new syntax checker, you usually don't need to run the entire test suite.

It's ok to just run the tests for the specific language and the documentation tests, i.e. cask exec ert-runner -t documentation,language-sh for shell scripts, or cask exec ert-runner -t documentation,language-python for Python.

Syntax checkers for other languages aren't affected by adding a new checker, and the rest of the tests runs on Travis CI.

[yasuyk]

I see! I will do so from next time.

[swsnr]

@yasuyk Merged. Thank you for your help.

[koalaman]

If one of you will take a screenshot similar to the ones on http://www.shellcheck.net/about.html , I can upload it and link to flycheck

[yasuyk]

@lunaryorn Great! Thanks.

[yasuyk]

I taked a screenshot with Spolsky theme and Source Code Pro font.

@lunaryorn @koalaman What do you say?

[swsnr]

@yasuyk @koalaman I'd like to have Flycheck's error list in the screenshot as well, so I did another one with Zenburn and Source Code Pro:

It's slightly larger, but imho better conveys how Flycheck would look like when being used in a real editing session, e.g. with window borders, etc.

[yasuyk]

@lunaryorn Looks good. 👍

[koalaman]

@yasuyk @lunaryorn Great, thanks! Small is good, It's basically just a thumbnail to link to the project.

[swsnr]

@yasuyk @koalaman I used Shellcheck for the last days, and from my personal experience, I'd like to disable it for Zsh again.

It's reporting too many false positives, and often entirely fails to parse Zsh code. For instance, it chokes on glob modifiers (e.g. /foo/*/bar(N)), and doesn't handle many special expansion parameters (e.g. $+commands[foo], for which it reports “$ is not used specially and should therefore be escaped.”). Apparently it lacks support for many advanced (seemingly non-bash, non-posix) features of Zsh.

Comments?

[yasuyk]

@lunaryorn You can say that again. ShellCheck report errors in my correct .zshrc.

[koalaman]

Sounds reasonable. I haven't put a lot of work into advanced zsh syntax, and there haven't been any bug reports.

[swsnr]

@koalaman Try Shellcheck on prezto sources. Some of its modules use fairly advanced expressions, which Shellcheck currently fails to parse. I think it'd serve quite well as a general test case for Zsh support.