diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 348c4ed..0d5c32f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,8 @@ jobs: env: # NOTE: use env to pass the output in order to avoid possible injection attacks FILES: "${{ steps.files.outputs.added_modified }}" + - name: checkstyle + run: ./scripts/checkstyle.py - name: Shellcheck run: shellcheck --severity=error bin/* ./*.sh - name: Lint and format Python with Ruff diff --git a/bin/git-abort b/bin/git-abort index 201fa48..af1712b 100755 --- a/bin/git-abort +++ b/bin/git-abort @@ -2,7 +2,7 @@ set -euo pipefail -function discover_op() { +discover_op() { local gitdir # git rev-parse emits an error if not in a git repo so only need to bail out gitdir="$(git rev-parse --git-dir)" || exit @@ -14,7 +14,7 @@ function discover_op() { done } -function validate_op() { +validate_op() { local op="$1" if [ -z "$op" ]; then echo "No active operation found" >&2 @@ -26,7 +26,7 @@ function validate_op() { fi } -function discover_action() { +discover_action() { local action=${1/git-/} if [ "$action" != "abort" ] && [ "$action" != "continue" ]; then echo "Invalid action: $1" >&2 diff --git a/bin/git-bulk b/bin/git-bulk index 1f414ea..06eecd2 100755 --- a/bin/git-bulk +++ b/bin/git-bulk @@ -30,7 +30,7 @@ cdfail() { } # add another workspace to global git config -function addworkspace { +addworkspace() { git config --global bulkworkspaces."$wsname" "$wsdir"; if [ -n "$source" ]; then if [ ! -d "$wsdir" ]; then echo 1>&2 "Path of workspace doesn't exist, make it first."; exit 1; fi @@ -59,19 +59,19 @@ function addworkspace { } # add current directory -function addcurrent { git config --global bulkworkspaces."$wsname" "$PWD"; } +addcurrent() { git config --global bulkworkspaces."$wsname" "$PWD"; } # remove workspace from global git config -function removeworkspace { checkWSName && git config --global --unset bulkworkspaces."$wsname"; } +removeworkspace() { checkWSName && git config --global --unset bulkworkspaces."$wsname"; } # remove workspace from global git config -function purge { git config --global --remove-section bulkworkspaces; } +purge() { git config --global --remove-section bulkworkspaces; } # list all current workspace locations defined -function listall { git config --global --get-regexp bulkworkspaces; } +listall() { git config --global --get-regexp bulkworkspaces; } # guarded execution of a git command in one specific repository -function guardedExecution () { +guardedExecution () { if [ "${quiet?}" != "true" ] || $guardedmode; then echo 1>&2 "${bldred}->${reset} executing ${inverse}git $gitcommand${reset} in repository ${leadingpath%/*}/${bldred}${curdir##*/}${reset}" fi @@ -88,7 +88,7 @@ function guardedExecution () { } # check if the passed command is known as a core git command -function checkGitCommand () { +checkGitCommand () { if git help -a | grep -o -q "\b${corecommand}\b"; then echo 1>&2 "Core command \"$corecommand\" accepted." else @@ -101,7 +101,7 @@ function checkGitCommand () { } # check if workspace name is registered -function checkWSName () { +checkWSName () { while read -r workspace; do parseWsName "$workspace" if [[ $rwsname == "$wsname" ]]; then return; fi @@ -111,7 +111,7 @@ function checkWSName () { } # parse out wsname from workspacespec -function parseWsName () { +parseWsName () { local wsspec="$1" # Get the workspace value from its specification in the `.gitconfig`. # May be an absolute path or a variable name of the form: `$VARNAME` @@ -128,7 +128,7 @@ function parseWsName () { } # detects the wsname of the current directory -function wsnameToCurrent () { +wsnameToCurrent () { while read -r workspace; do if [ -z "$workspace" ]; then continue; fi parseWsName "$workspace" @@ -140,7 +140,7 @@ function wsnameToCurrent () { } # helper to check number of arguments. -function allowedargcount () { +allowedargcount () { if [ "$paramcount" -ne "${1:-0}" ] && [ "$paramcount" -ne "${2:-0}" ]; then echo 1>&2 "error: wrong number of arguments" && usage; exit 1; @@ -148,7 +148,7 @@ function allowedargcount () { } # execute the bulk operation -function executBulkOp () { +executBulkOp () { checkGitCommand if ! $allwsmode && ! $singlemode; then wsnameToCurrent; fi # by default git bulk works within the 'current' workspace listall | while read -r workspacespec; do @@ -194,7 +194,7 @@ while [ "${#}" -ge 1 ] ; do --listall|--purge) butilcommand="${1:2}" && break ;; --removeworkspace|--addcurrent|--addworkspace) - butilcommand="${1:2}" && wsname="$2" && wsdir="$3" && if [ "$4" == "--from" ]; then source="$5"; fi && break ;; + butilcommand="${1:2}" && wsname="$2" && wsdir="$3" && if [ "$4" = "--from" ]; then source="$5"; fi && break ;; --no-follow-symlinks) no_follow_symlinks=true ;; --no-follow-hidden) diff --git a/bin/git-clear b/bin/git-clear index 0c8cbd1..6fd3ae8 100755 --- a/bin/git-clear +++ b/bin/git-clear @@ -2,7 +2,7 @@ PROGNAME="git-clear" FORCE=0 -function _usage() { +_usage() { cat << EOF usage: $PROGNAME options usage: $PROGNAME -h|help|? @@ -38,6 +38,6 @@ else clean=y fi -if [ "$clean" == "y" ]; then +if [ "$clean" = "y" ]; then git clean -d -f -x && git reset --hard fi diff --git a/bin/git-clear-soft b/bin/git-clear-soft index 0473b57..1274d21 100755 --- a/bin/git-clear-soft +++ b/bin/git-clear-soft @@ -2,6 +2,6 @@ echo -n "Sure? - This command may delete files that cannot be recovered. Files and directories in .gitignore will be preserved [y/N]: " read -r answer -if [ "$answer" == "y" ] +if [ "$answer" = "y" ] then git clean -d -f && git reset --hard fi diff --git a/bin/git-ignore b/bin/git-ignore index e380a2e..34a1791 100755 --- a/bin/git-ignore +++ b/bin/git-ignore @@ -2,7 +2,7 @@ GIT_DIR=$(git rev-parse --git-dir 2>/dev/null) -function show_contents { +show_contents() { local file="${2/#~/$HOME}" if [ -f "$file" ]; then echo "$1 gitignore: $2" && cat "$file" @@ -11,7 +11,7 @@ function show_contents { fi } -function cd_to_git_root { +cd_to_git_root() { local error_level="$1" if ! git rev-parse --git-dir &>/dev/null; then @@ -29,7 +29,7 @@ function cd_to_git_root { fi } -function global_ignore() { +global_ignore() { if ! git config --global core.excludesFile 2>/dev/null; then if [ -f "$HOME/.gitignore" ]; then echo "$HOME/.gitignore" @@ -39,11 +39,11 @@ function global_ignore() { fi } -function show_global { +show_global() { show_contents Global "$(global_ignore)" } -function add_global { +add_global() { local global_gitignore global_gitignore="$(global_ignore)" if [ -z "$global_gitignore" ]; then @@ -56,28 +56,28 @@ function add_global { fi } -function show_local { +show_local() { cd_to_git_root --warn show_contents Local .gitignore } -function add_local { +add_local() { cd_to_git_root --warn add_patterns .gitignore "$@" } -function show_private { +show_private() { cd_to_git_root --error show_contents Private "${GIT_DIR}/info/exclude" } -function add_private { +add_private() { cd_to_git_root --error test -d "${GIT_DIR}/info" || mkdir -p "${GIT_DIR}/info" add_patterns "${GIT_DIR}/info/exclude" "$@" } -function add_patterns { +add_patterns() { echo "Adding pattern(s) to: $1" local file="${1/#~/$HOME}" dir_name=$(dirname "$file") diff --git a/bin/git-merge-into b/bin/git-merge-into index 0561a8d..e547555 100755 --- a/bin/git-merge-into +++ b/bin/git-merge-into @@ -18,7 +18,7 @@ then git stash fi -if [ "${!#}" == '--ff-only' ]; then +if [ "${!#}" = '--ff-only' ]; then case $# in 2 ) # dest --ff git push "$(git rev-parse --show-toplevel)" "$cur_branch":"$1";; diff --git a/bin/git-paste b/bin/git-paste index 33643dd..173367c 100755 --- a/bin/git-paste +++ b/bin/git-paste @@ -2,7 +2,7 @@ set -e set -o pipefail -if ! command -v pastebinit >/dev/null 2>&1; then +if ! command -v pastebinit &>/dev/null; then echo >&2 "To run 'git paste', you need to install pastebinit in your system" exit 1 fi diff --git a/bin/git-psykorebase b/bin/git-psykorebase index b3e1a7c..a5e7577 100755 --- a/bin/git-psykorebase +++ b/bin/git-psykorebase @@ -5,7 +5,7 @@ SECONDARY_BRANCH="" FF="--ff" CONTINUE="no" -function current_branch() { +current_branch() { git rev-parse --abbrev-ref HEAD } diff --git a/bin/git-pull-request b/bin/git-pull-request index 3bd2060..e31de4f 100755 --- a/bin/git-pull-request +++ b/bin/git-pull-request @@ -43,7 +43,7 @@ if [ -z "$remote" ]; then echo 'no upstream found, push to origin as default' remote="origin" fi -[ "$remote" == "." ] && abort "the upstream should be a remote branch." +[ "$remote" = "." ] && abort "the upstream should be a remote branch." # make sure it's pushed diff --git a/bin/git-rebase-patch b/bin/git-rebase-patch index 395304e..521d5fd 100755 --- a/bin/git-rebase-patch +++ b/bin/git-rebase-patch @@ -30,7 +30,7 @@ do GIT_INDEX_FILE=$index git read-tree "$rev" # Try to apply the patch. - GIT_INDEX_FILE=$index git apply --cached "$1" >/dev/null 2>&1 + GIT_INDEX_FILE=$index git apply --cached "$1" &>/dev/null patch_failed=$? # Do it again, but show the error, if the problem is the patch itself. diff --git a/bin/git-release b/bin/git-release index 676495d..8df102b 100755 --- a/bin/git-release +++ b/bin/git-release @@ -119,7 +119,7 @@ if test $# -gt 0; then fi declare -a sign_args - if [ "$sign" == true ]; then + if [ "$sign" = true ]; then sign_args=("-s") fi diff --git a/bin/git-setup b/bin/git-setup index d1319ec..2a8c82d 100755 --- a/bin/git-setup +++ b/bin/git-setup @@ -2,7 +2,7 @@ COMMIT_MESSAGE='Initial commit' -if [ "$1" == "-m" ]; then +if [ "$1" = "-m" ]; then COMMIT_MESSAGE=$2 shift; shift fi diff --git a/bin/git-standup b/bin/git-standup index c0787f5..89f8afe 100755 --- a/bin/git-standup +++ b/bin/git-standup @@ -35,7 +35,7 @@ in_git_repo=$? # Use colors, but only if connected to a terminal, and that terminal # supports them. -if command -v tput >/dev/null 2>&1; then +if command -v tput &>/dev/null; then ncolors=$(tput colors) fi if [[ -t 1 ]] && [[ -n "$ncolors" ]] && [[ "$ncolors" -ge 8 ]] ; then diff --git a/bin/git-summary b/bin/git-summary index b69a9ae..3dbe1b6 100755 --- a/bin/git-summary +++ b/bin/git-summary @@ -206,10 +206,10 @@ COLUMN_CMD_DELIMTER="¬" # Hopefully, this symbol is not used in branch names... SP="$COLUMN_CMD_DELIMTER|" print_summary_by_line() { - if [ "$OUTPUT_STYLE" == "tabular" ]; then + if [ "$OUTPUT_STYLE" = "tabular" ]; then tabular_headers="# Repo $SP Lines" echo -e "$tabular_headers\n$project $SP $(line_count "${paths[@]}")" | column -t -s "$COLUMN_CMD_DELIMTER" - elif [ "$OUTPUT_STYLE" == "oneline" ]; then + elif [ "$OUTPUT_STYLE" = "oneline" ]; then echo "$project / lines: $(line_count "${paths[@]}")" elif [ -n "$SUMMARY_BY_LINE" ]; then echo @@ -221,10 +221,10 @@ print_summary_by_line() { } print_summary() { - if [ "$OUTPUT_STYLE" == "tabular" ]; then + if [ "$OUTPUT_STYLE" = "tabular" ]; then tabular_headers="# Repo $SP Age $SP Last active $SP Active on $SP Commits $SP Uncommitted $SP Branch" echo -e "$tabular_headers\n$project $SP $(repository_age) $SP $(last_active) $SP $(active_days "$commit") days $SP $(commit_count "$commit") $SP $(uncommitted_changes_count) $SP $(current_branch_name)" | column -t -s "$COLUMN_CMD_DELIMTER" - elif [ "$OUTPUT_STYLE" == "oneline" ]; then + elif [ "$OUTPUT_STYLE" = "oneline" ]; then echo "$project / age: $(repository_age) / last active: $(last_active) / active on $(active_days "$commit") days / commits: $(commit_count "$commit") / uncommitted: $(uncommitted_changes_count) / branch: $(current_branch_name)" else echo @@ -236,7 +236,7 @@ print_summary() { echo " commits : $(commit_count "$commit")" # The file count doesn't support passing a git ref so ignore it if a ref is given - if [ "$commit" == "HEAD" ]; then + if [ "$commit" = "HEAD" ]; then echo " files : $(file_count)" fi echo " uncommitted : $(uncommitted_changes_count)" diff --git a/scripts/checkstyle.py b/scripts/checkstyle.py new file mode 100755 index 0000000..6e2d3dd --- /dev/null +++ b/scripts/checkstyle.py @@ -0,0 +1,262 @@ +#!/usr/bin/env python3 +import re +import os +import argparse +from pathlib import Path +from typing import Callable, List, Dict, Any # compat + +# This file checks Bash and Shell scripts for violations not found with +# shellcheck or existing methods. You can use it in several ways: +# +# Lint all .bash, .sh, .bats files along with 'bin/asdf' and print out violations: +# $ ./scripts/checkstyle.py +# +# The former, but also fix all violations. This must be ran until there +# are zero violations since any line can have more than one violation: +# $ ./scripts/checkstyle.py --fix +# +# Lint a particular file: +# $ ./scripts/checkstyle.py ./lib/functions/installs.bash +# +# Check to ensure all regular expressions are working as intended: +# $ ./scripts/checkstyle.py --internal-test-regex + +Rule = Dict[str, Any] + +class c: + RED = '\033[91m' + GREEN = '\033[92m' + YELLOW = '\033[93m' + BLUE = '\033[94m' + MAGENTA = '\033[95m' + CYAN = '\033[96m' + RESET = '\033[0m' + BOLD = '\033[1m' + UNDERLINE = '\033[4m' + LINK: Callable[[str, str], str] = lambda href, text: f'\033]8;;{href}\a{text}\033]8;;\a' + +def utilGetStrs(line: Any, m: Any): + return ( + line[0:m.start('match')], + line[m.start('match'):m.end('match')], + line[m.end('match'):] + ) + +# Before: printf '%s\\n' '^w^' +# After: printf '%s\n' '^w^' +def noDoubleBackslashFixer(line: str, m: Any) -> str: + prestr, midstr, poststr = utilGetStrs(line, m) + + return f'{prestr}{midstr[1:]}{poststr}' + +# Before: $(pwd) +# After: $PWD +def noPwdCaptureFixer(line: str, m: Any) -> str: + prestr, _, poststr = utilGetStrs(line, m) + + return f'{prestr}$PWD{poststr}' + +# Before: [ a == b ] +# After: [ a = b ] +def noTestDoubleEqualsFixer(line: str, m: Any) -> str: + prestr, _, poststr = utilGetStrs(line, m) + + return f'{prestr}={poststr}' + +# Before: function fn() { ... +# After: fn() { ... +# --- +# Before: function fn { ... +# After fn() { ... +def noFunctionKeywordFixer(line: str, m: Any) -> str: + prestr, midstr, poststr = utilGetStrs(line, m) + + midstr = midstr.strip() + midstr = midstr[len('function'):] + midstr = midstr.strip() + + parenIdx = midstr.find('(') + if parenIdx != -1: + midstr = midstr[:parenIdx] + + return f'{prestr}{midstr}() {poststr}' + +# Before: >/dev/null 2>&1 +# After: &>/dev/null +# --- +# Before: 2>/dev/null 1>&2 +# After: &>/dev/null +def noVerboseRedirectionFixer(line: str, m: Any) -> str: + prestr, _, poststr = utilGetStrs(line, m) + + return f'{prestr}&>/dev/null{poststr}' + +def lintfile(file: Path, rules: List[Rule], options: Dict[str, Any]): + content_arr = file.read_text().split('\n') + + for line_i, line in enumerate(content_arr): + if 'checkstyle-ignore' in line: + continue + + for rule in rules: + should_run = False + if 'sh' in rule['fileTypes']: + if file.name.endswith('.sh'): + should_run = True + if 'bash' in rule['fileTypes']: + if file.name.endswith('.bash') or file.name.endswith('.bats') or file.name.startswith('git-'): + should_run = True + + if options['verbose']: + print(f'{str(file)}: {should_run}') + + if not should_run: + continue + + m = re.search(rule['regex'], line) + if m is not None and m.group('match') is not None: + dir = os.path.relpath(file.resolve(), Path.cwd()) + prestr = line[0:m.start('match')] + midstr = line[m.start('match'):m.end('match')] + poststr = line[m.end('match'):] + + print(f'{c.CYAN}{dir}{c.RESET}:{line_i + 1}') + print(f'{c.MAGENTA}{rule["name"]}{c.RESET}: {rule["reason"]}') + print(f'{prestr}{c.RED}{midstr}{c.RESET}{poststr}') + print() + + if options['fix']: + content_arr[line_i] = rule['fixerFn'](line, m) + + rule['found'] += 1 + + if options['fix']: + file.write_text('\n'.join(content_arr)) + +def main(): + rules: List[Rule] = [ + { + 'name': 'no-pwd-capture', + 'regex': '(?P\\$\\(pwd\\))', + 'reason': '$PWD is essentially equivalent to $(pwd) without the overhead of a subshell', + 'fileTypes': ['bash', 'sh'], + 'fixerFn': noPwdCaptureFixer, + 'testPositiveMatches': [ + '$(pwd)' + ], + 'testNegativeMatches': [ + '$PWD' + ], + }, + { + 'name': 'no-test-double-equals', + 'regex': '(?==).*?]', + 'reason': 'Disallow double equals in places where they are not necessary for consistency', + 'fileTypes': ['bash', 'sh'], + 'fixerFn': noTestDoubleEqualsFixer, + 'testPositiveMatches': [ + '[ a == b ]', + '[ "${lines[0]}" == blah ]', + ], + 'testNegativeMatches': [ + '[ a = b ]', + '[[ a = b ]]', + '[[ a == b ]]', + '[ a = b ] || [[ a == b ]]', + '[[ a = b ]] || [[ a == b ]]', + '[[ "${lines[0]}" == \'usage: \'* ]]', + '[ "${lines[0]}" = blah ]', + ], + }, + { + 'name': 'no-function-keyword', + 'regex': '^[ \\t]*(?Pfunction .*?(?:\\([ \\t]*\\))?[ \\t]*){', + 'reason': 'Only allow functions declared like `fn_name() {{ :; }}` for consistency (see ' + c.LINK('https://www.shellcheck.net/wiki/SC2113', 'ShellCheck SC2113') + ')', + 'fileTypes': ['bash', 'sh'], + 'fixerFn': noFunctionKeywordFixer, + 'testPositiveMatches': [ + 'function fn() { :; }', + 'function fn { :; }', + ], + 'testNegativeMatches': [ + 'fn() { :; }', + ], + }, + { + 'name': 'no-verbose-redirection', + 'regex': '(?P(>/dev/null 2>&1|2>/dev/null 1>&2))', + 'reason': 'Use `&>/dev/null` instead of `>/dev/null 2>&1` or `2>/dev/null 1>&2` for consistency', + 'fileTypes': ['bash'], + 'fixerFn': noVerboseRedirectionFixer, + 'testPositiveMatches': [ + 'echo woof >/dev/null 2>&1', + 'echo woof 2>/dev/null 1>&2', + ], + 'testNegativeMatches': [ + 'echo woof &>/dev/null', + 'echo woof >&/dev/null', + ], + }, + ] + [rule.update({ 'found': 0 }) for rule in rules] + + parser = argparse.ArgumentParser() + parser.add_argument('files', metavar='FILES', nargs='*') + parser.add_argument('--fix', action='store_true') + parser.add_argument('--verbose', action='store_true') + parser.add_argument('--internal-test-regex', action='store_true') + args = parser.parse_args() + + if args.internal_test_regex: + for rule in rules: + for positiveMatch in rule['testPositiveMatches']: + m: Any = re.search(rule['regex'], positiveMatch) + if m is None or m.group('match') is None: + print(f'{c.MAGENTA}{rule["name"]}{c.RESET}: Failed {c.CYAN}positive{c.RESET} test:') + print(f'=> {positiveMatch}') + print() + + for negativeMatch in rule['testNegativeMatches']: + m: Any = re.search(rule['regex'], negativeMatch) + if m is not None and m.group('match') is not None: + print(f'{c.MAGENTA}{rule["name"]}{c.RESET}: Failed {c.YELLOW}negative{c.RESET} test:') + print(f'=> {negativeMatch}') + print() + print('Done.') + return + + options = { + 'fix': args.fix, + 'verbose': args.verbose, + } + + # parse files and print matched lints + if len(args.files) > 0: + for file in args.files: + p = Path(file) + if p.is_file(): + lintfile(p, rules, options) + else: + for file in Path.cwd().glob('**/*'): + if '.git' in str(file.absolute()): + continue + + if file.is_file(): + lintfile(file, rules, options) + + # print final results + print(f'{c.UNDERLINE}TOTAL ISSUES{c.RESET}') + for rule in rules: + print(f'{c.MAGENTA}{rule["name"]}{c.RESET}: {rule["found"]}') + + grand_total = sum([rule['found'] for rule in rules]) + print(f'GRAND TOTAL: {grand_total}') + print(f'{c.BOLD}{c.YELLOW}NOTE:{c.RESET} Run "./scripts/checkstyle.py --fix" to automatically fix all issues (may need to run multiple times)') + + # exit + if grand_total == 0: + exit(0) + else: + exit(2) + +main()