[feature request] Run shellcheck (or equivalent linter) on individual steps

[falonso-alo]

Is your feature request related to a problem? Please describe.

There are lots of subtle issues related to executing bash. However, because Github Actions uses YAML you lose useful shell linting options.

Today, I ran into a set of issues with bash environment variables. Here is a script with some nonintuitive (for me at least) behavior:

export env=prod
echo "${env}"              # => prod
env="uat" echo "${env}"    # => prod
env="uat" && echo "${env}" # => uat

The Shellcheck VS Code extension actually catches this issue, but you can't use it from Github Actions because the shell is embedded in the YAML.

Here are the linting warnings provided by Shellcheck's VS Code extension for shell files:

Describe the solution you'd like

I would like to see the linting warnings from Shellcheck appear in the Github Actions steps.

Please let me know if additional clarification is required.

Additional context

• Shellcheck - Homepage
• Shellcheck - VS Code Extension

[felipesu19]

Hey, this seems like a very interesting feature idea, but upon first reading, I think there are complexities that make it more challenging to implement than it seems at first sight

• Do we need to require that anyone who installs the actions linter also installs shellcheck?
• Do we need to provide configuration to allow users to add a shell linter instead?
• How do we know if the command is running in a bash vs zsh vs some other form of shell in the yaml, e.g. if someone is using a windows runner?
• What are limitations present in workflow runs that aren't in traditional shells? Do we need to lint those too to avoid unexpected behaviors?

I don't think its unachievable but I do think its the type of thing that requires careful thought and design, and is probably out of scope for where this project is right now. That said if someone submitted a PR that did this, I'd certainly review it.

[falonso-alo]

Those are great points! I hadn't considered Windows runners and I wasn't aware that you could change the shell type in other OSes. It does seem the linter would need to be context sensitive and rely on environment information, but I don't know how the extension is architected so that may be infeasible.

I'm pretty swamped right now so I don't think this is the sort of thing I could reasonably contribute to. Maybe in the future.

Thanks for the response!

[felipesu19]

Since we're not ready to support this either, I'm going to close this for now, and its something we can revisit in the future. We appreciate the suggestion.

[brianjmurrell]

So if it's something you might visit in the future, or would welcome a PR for, why would you close the issue suggesting it? Leaving the issue open indicates that it's something that could be considered for development.

What if somebody who wants to contribute to this project comes along looking for issues that need some time put into them? They certainly won't find this one as it's closed.

So again, why close this issue?

[brianjmurrell]

For anyone else coming across this issue, there is a more generic request for any appropriate linter to be used on a run: script, based on the default shell for the platform and/or defaults.run.shell is in #114. It was also closed as "complicated" and related to this issue. While #114 is a more comprehensive implementation and doesn't just work when the shell is the borne [again] shell, this one has a nice description of the general idea.

[felipesu19]

So if it's something you might visit in the future, or would welcome a PR for, why would you close the issue suggesting it? Leaving the issue open indicates that it's something that could be considered for development.

What if somebody who wants to contribute to this project comes along looking for issues that need some time put into them? They certainly won't find this one as it's closed.

So again, why close this issue?

In general, we try to keep open issues for work we're actively tracking or that we intend to do. Even if someone opened a PR for this there's no guarantee we'd merge it in because there's a lot of complexity involved in both maintenance burden and generally figuring out usability and dependencies. I don't want to leave it open and have someone pour their effort into a PR without a guarantee we'd actually merge it.

So yeah, sorry, I know that's disappointing to hear, it's really not a ding on the idea. If we had unlimited time and engineers I'm sure its something we'd want to explore.

[tarurar]

Looks like the only workaround here is to put shell script into separate .sh file and reference it.

[brianjmurrell]

Looks like the only workaround here is to put shell script into separate .sh file and reference it.

Indeed. Buy that gets very messy very quickly as it means that in the case of duplicating workflows across projects (or even better, using reusable workflows) you now have to duplicate (and maintain!!!!!) all of the separate script files across all of the repos that you are duplicating (or using reusable) workflows across.

It would sure be helpful if such external scripts could be maintained alongside resuable workflows so that those workflows and their scripts were all maintained in a single place, but the only way to really do that is to break out every step you refer to a script in into an action that you can run with uses: in the reusable workflow. Something else that gets very messy very quickly.