Add shellcheck to CI (and fixup shell scripts to get the check to pass) #4015
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This does two things:
- Makes it clearer what's being done to people unfamiliar
with the word "linting"
- Expands the scope of the workflow to allow shellcheck
|
Neat, we have some hits right off the bat: Notice: ./.ci.sh:25:10: note: Not following: python/etc/profile.d/conda.sh was not specified as input (see shellcheck -x). [SC1091]
Notice: ./install_sct:36:15: note: Double quote to prevent globbing and word splitting. [SC2086]
Notice: ./install_sct:115:3: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:116:3: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:116:6: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:117:5: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:118:8: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:120:5: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:122:5: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:128:3: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:129:3: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:135:7: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:136:7: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
Notice: ./install_sct:137:7: note: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). [SC2317]
- Error: ./install_sct:156:57: error: < is for string comparisons. Use -lt instead. [SC2071]
Notice: ./batch_processing.sh:45:19: note: Use $(...) notation instead of legacy backticks `...`. [SC2006]
- Warning: ./batch_processing.sh:49:32: warning: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. [SC2166]
Notice: ./batch_processing.sh:55:7: note: Use $(...) notation instead of legacy backticks `...`. [SC2006]
Notice: ./batch_processing.sh:259:5: note: Use $(...) notation instead of legacy backticks `...`. [SC2006]
Notice: ./batch_processing.sh:262:24: note: Use $(...) notation instead of legacy backticks `...`. [SC2006]
Notice: ./batch_processing.sh:263:24: note: Use $(...) notation instead of legacy backticks `...`. [SC2006]
Notice: ./batch_processing.sh:264:27: note: $/${} is unnecessary on arithmetic variables. [SC2004]
Notice: ./batch_processing.sh:264:52: note: $/${} is unnecessary on arithmetic variables. [SC2004]
Notice: ./batch_processing.sh:264:80: note: $/${} is unnecessary on arithmetic variables. [SC2004]I'll see if I can get the check to pass. |
I thought since `install_sct` doesn't have a `.sh` extension, then the action would miss it. But, looking at the logs, it caught it! So, `additional_files` causes a redundant check of `install_sct`
joshuacwnewton
changed the title
From https://www.shellcheck.net/wiki/SC2166: > Prefer `[ p ] && [ q ]` as `[ p -a q ]` is not well defined.
From https://www.shellcheck.net/wiki/SC2006: > Use `$(...)` notation instead of legacy backticked `...`.
From https://www.shellcheck.net/wiki/SC2004: > `$` / `${}` is unnecessary on arithmetic variables.
From https://www.shellcheck.net/wiki/SC2071: < and >, in both [[ and [ (when escaped) will do a lexicographical comparison, not a numerical comparison. This means that [[ 5 > 10 ]] is true because 5 comes after 10 alphabetically. Meanwhile [[ 5 -gt 10 ]] is false because 5 does not come after 10 numerically. If you want to compare numbers by value, use the numerical comparison operators -gt, -ge, -lt and -le.
From https://www.shellcheck.net/wiki/SC2086: > Double quote to prevent globbing and word splitting.
From https://www.shellcheck.net/wiki/SC2317: > Command appears to be unreachable. Check usage (or ignore if invoked indirectly). Specifically: > ShellCheck may incorrectly believe that code is unreachable > if it's invoked by variable name or in a trap. In such a case, > please Ignore the message. These functions are invoked via traps, so we ignore as instructed.
From https://www.shellcheck.net/wiki/SC1091: > Not following: python/etc/profile.d/conda.sh was not specified > as input (see shellcheck -x) This error occurs because this file is not present when shellcheck is run. This is intentional, we don't want to run `install_sct` just to lint the code. So, we intentionally ignore the issue.
|
All fixed! No notices, warnings, or errors here. So, we should be able to turn on stricter checks. |
mguaypaq
approved these changes
Jan 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR expands the
lint_code.ymlworkflow file so that it runsshellcheckin addition toflake8, which would help us ensure the quality of scripts likeinstall_sctandbatch_processing.sh.Linked issues
Fixes #3143.