rhysd / actionlint

:octocat: Static checker for GitHub Actions workflow files
https://rhysd.github.io/actionlint/
MIT License
2.53k stars 151 forks source link

Feature request: Allow usage of .shellcheckrc #395

Closed piotrekkr closed 3 months ago

piotrekkr commented 4 months ago

Thanks for this great tool.

There seems to be one issue that kind of blocks adapting this tool fully into existing codebase. It seems actionlint does not use .shellcheckrc file at all because of this --norc in here https://github.com/rhysd/actionlint/blob/3a2f2c755b6442ec7999b28dfd107e1bb9853389/rule_shellcheck.go#L172 Would you consider removing this flag? Would this make shellcheck load file we have in project? Thanks

rhysd commented 3 months ago

It's intentional. When you have ~/.shellcheckrc in your local, it affects actionlint behavior in your local (and it will behave differently on CI). Such side effect should be avoided as much as possible.

What do you need to configure by .shellcheckrc file?

piotrekkr commented 3 months ago

@rhysd I wanted to disable this one error SC2129. As you can see on screenshot it is a style suggestion and would be nice if I could not be forced to fix this if this is not real issue. In general would be nice to be able to configure instead of only disabling whole shellcheck. We plan to use .shellcheckrc in project directory so no real issue there if it is in GIT.

image

rhysd commented 3 months ago

Thank you for sharing your motivation. I'll consider how to meet the requirement.

rhysd commented 3 months ago

@piotrekkr It seems that passing arguments via SHELLCHECK_OPTS variable can be a solution for this.

SHELLCHECK_OPTS='-e SC2129' actionlint

(Related to #258)

ericcornelissen commented 3 months ago

It's intentional. When you have ~/.shellcheckrc in your local, it affects actionlint behavior in your local (and it will behave differently on CI). Such side effect should be avoided as much as possible.

This clarifies a lot for me, coming from #258. At least for me, I would like actionlint to utilize the .shellcheckrc configuration I have for the project I'm running it against - I agree that it's undesirable to use ~/.shellcheckrc for the reason you point out.

While using the SHELLCHECK_OPTS environment variable is a viable workaround, perhaps a long term solution would be for actionlint to (perhaps optionally) use, if present, the .shellcheckrc file in the working directory where actionlint is invoked (or actionlint could even accept the location of the .shellcheckrc file as an argument). I don't think shellcheck currently supports this use case, but there is an open issue for it: https://github.com/koalaman/shellcheck/issues/1879

The primary reason I think this would be a better solution is because it can help avoid duplication of the ShellCheck configuration, what do you think @rhysd?

piotrekkr commented 3 months ago

@rhysd Yeah this could work if we did not use devops-actions/actionlint for linting.

  run-actionlint:
    name: Run actionlint
    runs-on: ubuntu-22.04
    timeout-minutes: 3
    steps:
      - name: Checkout app code
        uses: actions/checkout@v4
        with:
          persist-credentials: false
          show-progress: false

      - name: Run actionlint
        uses: devops-actions/actionlint@v0.1.2

It is wrapper around this tool. There seems to be no way to pass this env there. I'll create ticket in their repo about adding this somehow (maybe as input or to just pass env to docker). Anyway, thanks for your help. Will close this one.