Lint shell files with shfmt, fix bash-exec version

[ferrarimarco]

Fixes #492

Proposed Changes

1. Run shfmt in dry-run/check mode
2. Fix bash-exec version
3. Fix terrascan version
4. Check if the report directory exists before listing its contents
5. Remove duplicate "Found [zsh] script" messages
6. Lowered "Found [zsh] script" messages to debug level
7. Raised "Ignoring zsh script" messages to warning level

Readiness Checklist

Author/Contributor

• If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

• Label as breaking if this is a large fundamental change
• Label as either automation, bug, documentation, enhancement, infrastructure, or performance

[ferrarimarco]

The PR is failing because we don't have an .editorconfig file that shfmt can use, so it runs with its defaults. Shall we exclude this linter if an .editorconfig file is not available, and issue a warning?

[admiralAwkbar]

Can we change this to:

#################
# Install shfmt #
#################
ENV GO111MODULE=on
    GOROOT=/usr/lib/go
    GOPATH=/go
    PATH="$PATH":"$GOROOT"/bin
    PATH="$PATH":"$GOPATH"/bin

RUN mkdir -p ${GOPATH}/src ${GOPATH}/bin \
    && go get mvdan.cc/sh/v3/cmd/shfmt

[admiralAwkbar]

this should create fewer layers and help keep the image in good shape

[ferrarimarco]

Done

[ferrarimarco]

PS: Had to use a separate instructions for PATH if we want to reuse GOROOT and GOPATH

[admiralAwkbar]

@ferrarimarco Are you proposing to use a top level .editorconfig for the repository to help solve this? or should we exclude this from our check? Trying to find the best solution for this as well...

Also, do we need to check and load the .editorconfig like the rest of the linter rules files?

[ferrarimarco]

1. we don't need to load .editorconfig manually. shfmt picks it up automatically, if one is available, and recursively scans up the hierarchy until one with root = true is found.

2. I suppose we might have a .editorconfig file in the root of the repo, so that you can enforce consistent standards. In this case, the .editorconfig file in the editorconfig-checker test case directory has to be updated to include the root = true directive to avoid breaking that test case if the root .editorconfig doesn't match the test case one. The alternative would be excluding this linter if an .editorconfig file is not available, and perhaps issue a warning (much like the ansible-lint linter is doing?)

3. There's a slightly weird log line that you might help diagnose. Both test cases for shell_shfmt I provided are logged as (expected) failures, but the good one should have the successfully linted lines, right? Can you have a look?

[ferrarimarco]

Now the build failed for a temporary network failure (I suppose). You might want to restart it and try again.

[admiralAwkbar]

@ferrarimarco
https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork
Can you enable so i can try to help as well :)

[ferrarimarco]

It's already enabled :)

[ferrarimarco]

@admiralAwkbar This is what I meant in my point 3 above:

2020-08-31 16:19:48 [INFO  ]   ----------------------------------------------
2020-08-31 16:19:48 [INFO  ]   Testing Codebase [SHELL_SHFMT] files...
2020-08-31 16:19:48 [INFO  ]   ----------------------------------------------
2020-08-31 16:19:48 [INFO  ]   ----------------------------------------------
2020-08-31 16:19:48 [INFO  ]   Successfully found binary for [shfmt] in system location: [/go/bin/shfmt]
2020-08-31 16:19:48 [INFO  ]   ---------------------------
2020-08-31 16:19:48 [INFO  ]   File:[/tmp/lint/.automation/test/shell_shfmt/shell_shfmt_bad_1.sh]
2020-08-31 16:19:48 [INFO  ]    - File:[shell_shfmt_bad_1.sh] failed test case with [shfmt] successfully
2020-08-31 16:19:48 [INFO  ]   ---------------------------
2020-08-31 16:19:48 [INFO  ]   File:[/tmp/lint/.automation/test/shell_shfmt/shell_shfmt_good_1.sh]
2020-08-31 16:19:48 [INFO  ]    - File:[shell_shfmt_good_1.sh] failed test case with [shfmt] successfully
2020-08-31 16:19:49 [WARN  ]   No TAP expected file found at:[/tmp/lint/.automation/test/shell_shfmt/reports/expected-SHELL_SHFMT.tap]
2020-08-31 16:19:49 [INFO  ]   skipping report assertions
2020-08-31 16:19:49 [INFO  ]   ----------------------------------------------

[ferrarimarco]

Addition to my point 2 above: shfmt can be configured either via .editorconfig or via command arguments. I suppose that since super-linter doesn't want to enforce a formatting standard, and don't (currently?) have a way to personalize linter arguments, I guess we should exclude this linter if an .editorconfig file is not available.

[admiralAwkbar]

@ferrarimarco I agree, if the config is not found, then we need to disable and warn the user that it's being skipped.

Can you add and merge the master back to it? added some fixes and should be a real blast...

[ferrarimarco]

done!

[ferrarimarco]

Additionally, I've tuned ZSH detection output messages a bit. See my description above

[admiralAwkbar]

@ferrarimarco Very nice work!