koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.39k stars 1.78k forks source link

Support parsing `ssinclude` "syntax" so that shellcheck can be used on linode stackscripts #2247

Open G-Rath opened 3 years ago

G-Rath commented 3 years ago

For new checks and feature suggestions

Here's a snippet or screenshot that shows the problem:


#!/bin/bash

source <ssinclude StackScriptID="[NUMBER]">

Here's what shellcheck currently says:

$ shellcheck myscript

Line 1:
source <ssinclude StackScriptID="[NUMBER]">
^-- SC1009: The mentioned syntax error was in this simple command.
                                          ^-- SC1073: Couldn't parse this redirection. Fix to allow more checks.
                                           ^-- SC1072: Fix any mentioned problems and try again.

Here's what I wanted or expected to see:

I'd like ShellCheck to consider this "valid" syntax for the sake of being able to check the rest of the script. Granted, I know that this isn't valid and really wish Linode supported an alternative valid-in-bash syntax but this is how Linode has chosen to do it.

I like to write code as natively as possible so that native tooling can work, but unfortunately in this case there's not really anyway around it if I want to include other stackscripts.

I'm not wanting shellcheck to actually do any checks - just have a very strict parsing rule that looks for <ssinclude StackScriptID=.*> before declaring invalid syntax so that we can get all the shellcheck goodness when writing StackScripts for Linode.

I'd be happy to help implement this, but I've never worked with Haskell so don't know how far I'll be able to get by myself 😅

TinCanTech commented 3 years ago

Does shellcheck still choke if you disable the check ?

G-Rath commented 3 years ago

@TinCanTech I didn't think you could disable parser errors - I'll give it a shot right now

G-Rath commented 3 years ago

So you can "ignore" them, but it seems that shellcheck still can't parse the rest of the file:

# shellcheck disable=SC1009,SC1073,SC1072
source <ssinclude StackScriptID="[NUMBER]">

php -d memory_limit=-1 $(which composer) update

That results in no output, but if I comment out the ssinclude line we get:

Line 1:
# shellcheck disable=SC1009,SC1073,SC1072
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

Line 4:
php -d memory_limit=-1 $(which composer) update
                       ^-- SC2046: Quote this to prevent word splitting.

I wouldn't be surprised if there are some checks that can still happen with the invalid syntax in the file, but would be nice it we could have all of them.

TinCanTech commented 3 years ago

Add a shebang and try it. However, I think you may be beyond the limit with current shellcheck.

I guess it would need a shebang and a completely new linter for your syntax.

G-Rath commented 3 years ago

Still the same issue with a hashbang.

I'm not sure why it would need a new linter for the syntax? (unless that's a small thing to do?) I would have thought the issue lies in the parser marking the syntax as invalid, but that could be not understanding how shellcheck works :)

TinCanTech commented 3 years ago

Characters < and > have quite well defined meanings in current shells, so how to ignore them for your use case ? I don't know either. I was just curious how SC would react. It feels like a lot of work though ..