fix: lint errors from scripts/checkstyle.py

[hyperupcall]

Summary

Sorry for the onslaught of PR's, this is going to be the last one until all the other ones are either closed or merged. This adds preliminary support for an automated way of enforcing particular styles that shellcheck doesn't catch.

Right now, it's a simple Python file (instead the originally proposed Perl for obvious readability reasons), which just checks a single "rule" - "unecessary printf backslashes".

The second commit uses this rule to format a single file like so:

$ ./scripts/checkstyle.py ./lib/functions/plugins.bash --fix

• If --fix is not supplied, then it only prints out the errors
• If no file paths are supplied, then it reads all of the files matching *.{bash,sh,bats}.
• The comand sometimes needs to be run multiple times if there is more than one style violation on a single line.

I have the regular expressions and will write auto-fixers for each of the fixed things from #1349, but I'm only adding them one-at-a-time as stated.

Soon I guess it would be good to add GitHub actions and stuff, but I think should that be in a separate PR? I'm also looking for feedback on the checkstyle script, hopefully it's readable enough?

[jthegedus]

My personal opinions of Python aside, this looks fine to me as a starting point. I prefer to use existing tooling if it exists with options to configure regex checks and fixes, but I couldn't find anything with a quick look. So, something custom now that is usable is great. We can always come back to it.

Would be good to add Python to this repository's .tool-versions file.

And adding the GitHub Action step now would be fine, no need to wait for another PR, just another step in the .github/workflows/lint.yml file.

[hyperupcall]

Sounds good! - In that case I'll run the script on every file so there aren't a million different lint commits

[hyperupcall]

I made those changed 👍

The regex was also updated to match within any case of double-quotes (not just after printf statements) as I mentioned here. This caught one extra violation at ./lib/commands/command-current.bash:47

[jthegedus]

@Stratus3D

Building on your comment - #1349 (comment)

@hyperupcall while the PR is too much to merge all at once, it would be great if we had some automated way of enforcing these things via github actions. We currently require shfmt and shellcheck to pass for PRs, is there another tool we can use to enforce better Bash conventions? I know I would typically NOT catch all these things when reviewing a PR manually.

Is there a reason we can't capture this like we do test/banned_commands.bats?

While the automated fix is nice, we only need the automated fix once, then we can just ban with regex checks right?

[jthegedus]

@Stratus3D I am good with the changes here, but will leave final approval & merge to you.

I am going to merge, we can always back out these scripts which are part of CI.

[jthegedus]

Cheers @hyperupcall