Optimistically match shellcheck errors to source

Open dpsutton opened this issue 5 months ago • 0 comments

Addresses https://github.com/rhysd/actionlint/issues/360

The yaml ast node exposes if the string nodes are literals or not. If so, we can preserve shellcheck errors to their source in the yaml block rather than just the runs: key.

Example from sqlite-jdbc:

      - name: Build matrix from Makefile
        id: set-matrix
        # parse the Makefile to retrieve the list of targets in 'native-all', without 'native'
        run: |
          matrix=$((
            echo '{ "target" : ['
            sed -n "/^native-all *: */ { s///; p }" Makefile | sed "s/^native\s//g" | sed 's/ /, /g' | xargs -n 1 echo | sed -r 's/^([^,]*)(,?)$/"\1"\2/'
            echo " ]}"
          ) | jq -c .)
          echo $matrix | jq .
          echo "matrix=$matrix" >> $GITHUB_OUTPUT

output of

❯ actionlint -version
v1.7.7
installed by building from source
built with go1.21.6 compiler for darwin/arm64

.github/workflows/build-native.yml:68:9: shellcheck reported issue in this script: SC1102:error:1:8: Shells disambiguate $(( differently or not at all. For $(command substitution), add space after $( . For $((arithmetics)), fix parsing errors [shellcheck]
   |
68 |         run: |
   |         ^~~~
.github/workflows/build-native.yml:68:9: shellcheck reported issue in this script: SC2086:info:6:6: Double quote to prevent globbing and word splitting [shellcheck]
   |
68 |         run: |
   |         ^~~~
.github/workflows/build-native.yml:68:9: shellcheck reported issue in this script: SC2086:info:7:26: Double quote to prevent globbing and word splitting [shellcheck]
   |
68 |         run: |
   |         ^~~~

Output from this branch:

.github/workflows/build-native.yml:69:18: shellcheck reported issue in this script: SC1102:error:1:8: Shells disambiguate $(( differently or not at all. For $(command substitution), add space after $( . For $((arithmetics)), fix parsing errors [shellcheck]
   |
69 |           matrix=$((
   |                  ^~~
.github/workflows/build-native.yml:74:16: shellcheck reported issue in this script: SC2086:info:6:6: Double quote to prevent globbing and word splitting [shellcheck]
   |
74 |           echo $matrix | jq .
   |                ^~~~~~~
.github/workflows/build-native.yml:75:36: shellcheck reported issue in this script: SC2086:info:7:26: Double quote to prevent globbing and word splitting [shellcheck]
   |
75 |           echo "matrix=$matrix" >> $GITHUB_OUTPUT
   |                                    ^~~~~~~~~~~~~~

The difference becomes incredibly stark with greater than 10 errors. The existing output feels empty.

i don't beleive this is ready to merge as is. I'm a newcomer to the go language so I expect the code is not idiomatic. This project also has excellent testing coverage and documentation that this changeset does not come close to as is.

My first question is if you are open to this change. If so, I am very interseted in your thoughts to get this to the quality you like. Things that might be of interest:

put this feature behind a flag
gather shell scripts from top 1000 github repos and see the results similar to the popular actions
extensive tests in testdata with expected output
update the documentation examples to generate output
extend this to pyflakes mechanism

Thank you for the project

Screenshots to give visceral feel