I'd prefer if we minimized the number of shell scripts instead, since the number of people that can (and are willing to) review them is almost zero. Though, at least enforcing some sensible syntax can't hurt.
Concept ACK.

@MarcoFalke I agree that the long-term plan should be to minimize the number of shell scripts. In the short-term I think it is a good idea to automate the mechanical part of the review process. That will allow for us scarce shell script reviewers to focus on the important stuff rather on trivial things that can be automated away using shellcheck.

Personally I run shellcheck manually on all shell scripts I review and ask for adjustments to please shellcheck. The recommendations given are very good and in line what a skilled human reviewer would suggest. My experience is that shellcheck is a really solid piece of linting software with a surprisingly low rate of false positives.

utACK 6a4a156

utACK 6a4a156d269da0b286957a5b4c480e7f7429879b

@theuni @sipa Would you mind reviewing this PR? As shell script connoisseurs, your input would be appreciated :-)

I'm not up to date on shell linting tools. Does this detect dangerous constructions, or is it just about style?

I'd prefer if we minimized the number of shell scripts instead, since the number of people that can (and are willing to) review them is almost zero.

Indeed. In general I'd prefer Python scripts, although shell scripts are fast to hack together they're easier to review, and maintain on the long run. Though for linting other shell scripts a shell script makes sense I guess.

utACK 6a4a156d269da0b286957a5b4c480e7f7429879b

@laanwj The focus on shellcheck is to detect dangerous constructs, not styling issues.

From the shellcheck README.md:

The goals of ShellCheck are

• To point out and clarify typical beginner's syntax issues that cause a shell to give cryptic error messages.
• To point out and clarify typical intermediate level semantic problems that cause a shell to behave strangely and counter-intuitively.
• To point out subtle caveats, corner cases and pitfalls that may cause an advanced user's otherwise working script to fail under future circumstances.

The gallery of bad shell scripts gives a couple of examples of issues that shellcheck can detect.

I've used shellcheck for years and my experience is that the rate of false positives is surprisingly low. Lower than most other language linters I've used. A very impressive piece of software.

To point out subtle caveats, corner cases and pitfalls that may cause an advanced user's otherwise working script to fail under future circumstances.

SGTM, thanks for elaborating.
utACK 6a4a156d269da0b286957a5b4c480e7f7429879b

Looks like this fails at the moment, at least when I run it locally;

$ contrib/devtools/lint-shell.sh; echo $?                                                                                     

In contrib/devtools/lint-logs.sh line 21:
    echo "All calls to LogPrintf() should be terminated with \\n"
          ^-- SC2028: echo won't expand escape sequences. Consider printf.

1

Pushed an updated version which excludes src/(leveldb|secp256k1|univalue)/ from linting (previously missed leveldb) and added a missing exclude for SC2028 (we intentionally use \\n in a call to echo).

@laanwj Please retry with the version I pushed the second before your comment :-)

Right, passes now re-utACK 1499fdc

Going to merge this under the provision that we can refine or disable it if it causes problems.

@MarcoFalke Thanks for merging! Ping me if any troubles or false positives are encountered with this linter and I'll fix!