From 878f9883507492fe1734561ea8ddc3c4e9de3c1d Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:33 -0400 Subject: [PATCH 01/11] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells The --chain-lint option detects broken &&-chains by forcing the test to exit early (as the very first step) with a sentinel value. If that sentinel is the test's overall exit code, then the &&-chain is intact; if not, then the chain is broken. Unfortunately, this detection does not extend to &&-chains within subshells even when the subshell itself is properly linked into the outer &&-chain. Address this shortcoming by feeding the body of the test to a lightweight "linter" which can peer inside subshells and identify broken &&-chains by pure textual inspection. Although the linter does not actually parse shell scripts, it has enough knowledge of shell syntax to reliably deal with formatting style variations (as evolved over the years) and to avoid being fooled by non-shell content (such as inside here-docs and multi-line strings). It recognizes modern subshell formatting: statement1 && ( statement2 && statement3 ) && statement4 as well as old-style: statement1 && (statement2 && statement3) && statement4 Heuristics are employed to properly identify the extent of a subshell formatted in the old-style since a number of legitimate constructs may superficially appear to close the subshell even though they don't. For example, it understands that neither "x=$(command)" nor "case $x in *)" end a subshell, despite the ")" at the end of line. Due to limitations of the tool used ('sed') and its inherent line-by-line processing, only subshells one level deep are handled, as well as one-liner subshells one level below that. Subshells deeper than that or multi-line subshells at level two are passed through as-is, thus &&-chains in their bodies are not checked. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint.sed | 346 ++++++++++++++++++++++++++++++++++++++++++++++++ t/test-lib.sh | 3 +- 2 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 t/chainlint.sed diff --git a/t/chainlint.sed b/t/chainlint.sed new file mode 100644 index 0000000000..a0de8a3882 --- /dev/null +++ b/t/chainlint.sed @@ -0,0 +1,346 @@ +#------------------------------------------------------------------------------ +# Detect broken &&-chains in tests. +# +# At present, only &&-chains in subshells are examined by this linter; +# top-level &&-chains are instead checked directly by the test framework. Like +# the top-level &&-chain linter, the subshell linter (intentionally) does not +# check &&-chains within {...} blocks. +# +# Checking for &&-chain breakage is done line-by-line by pure textual +# inspection. +# +# Incomplete lines (those ending with "\") are stitched together with following +# lines to simplify processing, particularly of "one-liner" statements. +# Top-level here-docs are swallowed to avoid false positives within the +# here-doc body, although the statement to which the here-doc is attached is +# retained. +# +# Heuristics are used to detect end-of-subshell when the closing ")" is cuddled +# with the final subshell statement on the same line: +# +# (cd foo && +# bar) +# +# in order to avoid misinterpreting the ")" in constructs such as "x=$(...)" +# and "case $x in *)" as ending the subshell. +# +# Lines missing a final "&&" are flagged with "?!AMP?!", and lines which chain +# commands with ";" internally rather than "&&" are flagged "?!SEMI?!". A line +# may be flagged for both violations. +# +# Detection of a missing &&-link in a multi-line subshell is complicated by the +# fact that the last statement before the closing ")" must not end with "&&". +# Since processing is line-by-line, it is not known whether a missing "&&" is +# legitimate or not until the _next_ line is seen. To accommodate this, within +# multi-line subshells, each line is stored in sed's "hold" area until after +# the next line is seen and processed. If the next line is a stand-alone ")", +# then a missing "&&" on the previous line is legitimate; otherwise a missing +# "&&" is a break in the &&-chain. +# +# ( +# cd foo && +# bar +# ) +# +# In practical terms, when "bar" is encountered, it is flagged with "?!AMP?!", +# but when the stand-alone ")" line is seen which closes the subshell, the +# "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold" +# area) since the final statement of a subshell must not end with "&&". The +# final line of a subshell may still break the &&-chain by using ";" internally +# to chain commands together rather than "&&", so "?!SEMI?!" is never removed +# from a line (even though "?!AMP?!" might be). +# +# Care is taken to recognize the last _statement_ of a multi-line subshell, not +# necessarily the last textual _line_ within the subshell, since &&-chaining +# applies to statements, not to lines. Consequently, blank lines, comment +# lines, and here-docs are swallowed (but not the command to which the here-doc +# is attached), leaving the last statement in the "hold" area, not the last +# line, thus simplifying &&-link checking. +# +# The final statement before "done" in for- and while-loops, and before "elif", +# "else", and "fi" in if-then-else likewise must not end with "&&", thus +# receives similar treatment. +# +# To facilitate regression testing (and manual debugging), a ">" annotation is +# applied to the line containing ")" which closes a subshell, ">>" to a line +# closing a nested subshell, and ">>>" to a line closing both at once. This +# makes it easy to detect whether the heuristics correctly identify +# end-of-subshell. +#------------------------------------------------------------------------------ + +# incomplete line -- slurp up next line +:squash +/\\$/ { + N + s/\\\n// + bsquash +} + +# here-doc -- swallow it to avoid false hits within its body (but keep the +# command to which it was attached) +/<<[ ]*[-\\]*EOF[ ]*/ { + s/[ ]*<<[ ]*[-\\]*EOF// + h + :hereslurp + N + s/.*\n// + /^[ ]*EOF[ ]*$/!bhereslurp + x +} + +# one-liner "(...) &&" +/^[ ]*!*[ ]*(..*)[ ]*&&[ ]*$/boneline + +# same as above but without trailing "&&" +/^[ ]*!*[ ]*(..*)[ ]*$/boneline + +# one-liner "(...) >x" (or "2>x" or "|&]/boneline + +# multi-line "(...\n...)" +/^[ ]*(/bsubshell + +# innocuous line -- print it and advance to next line +b + +# found one-liner "(...)" -- mark suspect if it uses ";" internally rather than +# "&&" (but not ";" in a string) +:oneline +/;/{ + /"[^"]*;[^"]*"/!s/^/?!SEMI?!/ +} +b + +:subshell +# bare "(" line? +/^[ ]*([ ]*$/ { + # stash for later printing + h + bnextline +} +# "(..." line -- split off and stash "(", then process "..." as its own line +x +s/.*/(/ +x +s/(// +bslurp + +:nextline +N +s/.*\n// + +:slurp +# incomplete line "...\" +/\\$/bincomplete +# multi-line quoted string "...\n..." +/^[^"]*"[^"]*$/bdqstring +# multi-line quoted string '...\n...' (but not contraction in string "it's so") +/^[^']*'[^']*$/{ + /"[^'"]*'[^'"]*"/!bsqstring +} +# here-doc -- swallow it +/<<[ ]*[-\\]*EOF/bheredoc +/<<[ ]*[-\\]*EOT/bheredoc +/<<[ ]*[-\\]*INPUT_END/bheredoc +# comment or empty line -- discard since final non-comment, non-empty line +# before closing ")", "done", "elsif", "else", or "fi" will need to be +# re-visited to drop "suspect" marking since final line of those constructs +# legitimately lacks "&&", so "suspect" mark must be removed +/^[ ]*#/bnextline +/^[ ]*$/bnextline +# in-line comment -- strip it (but not "#" in a string, Bash ${#...} array +# length, or Perforce "//depot/path#42" revision in filespec) +/[ ]#/{ + /"[^"]*#[^"]*"/!s/[ ]#.*$// +} +# one-liner "case ... esac" +/^[ ]*case[ ]*..*esac/bcheckchain +# multi-line "case ... esac" +/^[ ]*case[ ]..*[ ]in/bcase +# multi-line "for ... done" or "while ... done" +/^[ ]*for[ ]..*[ ]in/bcontinue +/^[ ]*while[ ]/bcontinue +/^[ ]*do[ ]/bcontinue +/^[ ]*do[ ]*$/bcontinue +/;[ ]*do/bcontinue +/^[ ]*done[ ]*&&[ ]*$/bdone +/^[ ]*done[ ]*$/bdone +/^[ ]*done[ ]*[<>|]/bdone +/^[ ]*done[ ]*)/bdone +/||[ ]*exit[ ]/bcontinue +/||[ ]*exit[ ]*$/bcontinue +# multi-line "if...elsif...else...fi" +/^[ ]*if[ ]/bcontinue +/^[ ]*then[ ]/bcontinue +/^[ ]*then[ ]*$/bcontinue +/;[ ]*then/bcontinue +/^[ ]*elif[ ]/belse +/^[ ]*elif[ ]*$/belse +/^[ ]*else[ ]/belse +/^[ ]*else[ ]*$/belse +/^[ ]*fi[ ]*&&[ ]*$/bdone +/^[ ]*fi[ ]*$/bdone +/^[ ]*fi[ ]*[<>|]/bdone +/^[ ]*fi[ ]*)/bdone +# nested one-liner "(...) &&" +/^[ ]*(.*)[ ]*&&[ ]*$/bcheckchain +# nested one-liner "(...)" +/^[ ]*(.*)[ ]*$/bcheckchain +# nested one-liner "(...) >x" (or "2>x" or "|]/bcheckchain +# nested multi-line "(...\n...)" +/^[ ]*(/bnest +# multi-line "{...\n...}" +/^[ ]*{/bblock +# closing ")" on own line -- exit subshell +/^[ ]*)/bclosesolo +# "$((...))" -- arithmetic expansion; not closing ")" +/\$(([^)][^)]*))[^)]*$/bcheckchain +# "$(...)" -- command substitution; not closing ")" +/\$([^)][^)]*)[^)]*$/bcheckchain +# multi-line "$(...\n...)" -- command substitution; treat as nested subshell +/\$([ ]*$/bnest +# "=(...)" -- Bash array assignment; not closing ")" +/=(/bcheckchain +# closing "...) &&" +/)[ ]*&&[ ]*$/bclose +# closing "...)" +/)[ ]*$/bclose +# closing "...) >x" (or "2>x" or "|]/bclose +:checkchain +# mark suspect if line uses ";" internally rather than "&&" (but not ";" in a +# string and not ";;" in one-liner "case...esac") +/;/{ + /;;/!{ + /"[^"]*;[^"]*"/!s/^/?!SEMI?!/ + } +} +# line ends with pipe "...|" -- valid; not missing "&&" +/|[ ]*$/bcontinue +# missing end-of-line "&&" -- mark suspect +/&&[ ]*$/!s/^/?!AMP?!/ +:continue +# retrieve and print previous line +x +n +bslurp + +# found incomplete line "...\" -- slurp up next line +:incomplete +N +s/\\\n// +bslurp + +# found multi-line double-quoted string "...\n..." -- slurp until end of string +:dqstring +s/"//g +N +s/\n// +/"/!bdqstring +bcheckchain + +# found multi-line single-quoted string '...\n...' -- slurp until end of string +:sqstring +s/'//g +N +s/\n// +/'/!bsqstring +bcheckchain + +# found here-doc -- swallow it to avoid false hits within its body (but keep +# the command to which it was attached); take care to handle here-docs nested +# within here-docs by only recognizing closing tag matching outer here-doc +# opening tag +:heredoc +/EOF/{ s/[ ]*<<[ ]*[-\\]*EOF//; s/^/EOF/; } +/EOT/{ s/[ ]*<<[ ]*[-\\]*EOT//; s/^/EOT/; } +/INPUT_END/{ s/[ ]*<<[ ]*[-\\]*INPUT_END//; s/^/INPUT_END/; } +:hereslurpsub +N +/^EOF.*\n[ ]*EOF[ ]*$/bhereclose +/^EOT.*\n[ ]*EOT[ ]*$/bhereclose +/^INPUT_END.*\n[ ]*INPUT_END[ ]*$/bhereclose +bhereslurpsub +:hereclose +s/^EOF// +s/^EOT// +s/^INPUT_END// +s/\n.*$// +bcheckchain + +# found "case ... in" -- pass through untouched +:case +x +n +/^[ ]*esac/bslurp +bcase + +# found "else" or "elif" -- drop "suspect" from final line before "else" since +# that line legitimately lacks "&&" +:else +x +s/?!AMP?!// +x +bcontinue + +# found "done" closing for-loop or while-loop, or "fi" closing if-then -- drop +# "suspect" from final contained line since that line legitimately lacks "&&" +:done +x +s/?!AMP?!// +x +# is 'done' or 'fi' cuddled with ")" to close subshell? +/done.*)/bclose +/fi.*)/bclose +bcheckchain + +# found nested multi-line "(...\n...)" -- pass through untouched +:nest +x +:nestslurp +n +# closing ")" on own line -- stop nested slurp +/^[ ]*)/bnestclose +# comment -- not closing ")" if in comment +/^[ ]*#/bnestcontinue +# "$((...))" -- arithmetic expansion; not closing ")" +/\$(([^)][^)]*))[^)]*$/bnestcontinue +# "$(...)" -- command substitution; not closing ")" +/\$([^)][^)]*)[^)]*$/bnestcontinue +# closing "...)" -- stop nested slurp +/)/bnestclose +:nestcontinue +x +bnestslurp +:nestclose +s/^/>>/ +# is it "))" which closes nested and parent subshells? +/)[ ]*)/bslurp +bcheckchain + +# found multi-line "{...\n...}" block -- pass through untouched +:block +x +n +# closing "}" -- stop block slurp +/}/bcheckchain +bblock + +# found closing ")" on own line -- drop "suspect" from final line of subshell +# since that line legitimately lacks "&&" and exit subshell loop +:closesolo +x +s/?!AMP?!// +p +x +s/^/>/ +b + +# found closing "...)" -- exit subshell loop +:close +x +p +x +s/^/>/ +b diff --git a/t/test-lib.sh b/t/test-lib.sh index 28315706be..78f7097746 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -675,7 +675,8 @@ test_run_ () { trace= # 117 is magic because it is unlikely to match the exit # code of other programs - if test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" + if $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') || + test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" then error "bug in the test script: broken &&-chain or run-away HERE-DOC: $1" fi From 803394459d40f8d07cef11d49900ba6f72887869 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:34 -0400 Subject: [PATCH 02/11] t/Makefile: add machinery to check correctness of chainlint.sed The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. Although the heuristics work well, they are still best-guesses and future changes could accidentally break assumptions upon which they are based. To protect against this possibility, tests checking correctness of the linter itself will be added. As preparation, add a new makefile "check-chainlint" target and associated machinery. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/.gitignore | 1 + t/Makefile | 25 +++++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/t/.gitignore b/t/.gitignore index 4e731dc1e3..348715f0e4 100644 --- a/t/.gitignore +++ b/t/.gitignore @@ -1,3 +1,4 @@ /trash directory* /test-results /.prove +/chainlinttmp diff --git a/t/Makefile b/t/Makefile index 96317a35f4..c83fd18861 100644 --- a/t/Makefile +++ b/t/Makefile @@ -18,8 +18,10 @@ TEST_LINT ?= test-lint ifdef TEST_OUTPUT_DIRECTORY TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results +CHAINLINTTMP = $(TEST_OUTPUT_DIRECTORY)/chainlinttmp else TEST_RESULTS_DIRECTORY = test-results +CHAINLINTTMP = chainlinttmp endif # Shell quote; @@ -27,14 +29,17 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH)) PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY)) +CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP)) T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)) TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh)) THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh))) +CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test))) +CHAINLINT = sed -f chainlint.sed all: $(DEFAULT_TEST_TARGET) -test: pre-clean $(TEST_LINT) +test: pre-clean check-chainlint $(TEST_LINT) $(MAKE) aggregate-results-and-cleanup failed: @@ -43,7 +48,7 @@ failed: sed -n 's/\.counts$$/.sh/p') && \ test -z "$$failed" || $(MAKE) $$failed -prove: pre-clean $(TEST_LINT) +prove: pre-clean check-chainlint $(TEST_LINT) @echo "*** prove ***"; $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) $(MAKE) clean-except-prove-cache @@ -53,13 +58,25 @@ $(T): pre-clean: $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)' -clean-except-prove-cache: +clean-except-prove-cache: clean-chainlint $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)' $(RM) -r valgrind/bin clean: clean-except-prove-cache $(RM) .prove +clean-chainlint: + $(RM) -r '$(CHAINLINTTMP_SQ)' + +check-chainlint: + @mkdir -p '$(CHAINLINTTMP_SQ)' && \ + err=0 && \ + for i in $(CHAINLINTTESTS); do \ + $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/$$i.actual && \ + diff -u chainlint/$$i.expect '$(CHAINLINTTMP_SQ)'/$$i.actual || err=1; \ + done && exit $$err + test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \ test-lint-filenames @@ -102,4 +119,4 @@ valgrind: perf: $(MAKE) -C perf/ all -.PHONY: pre-clean $(T) aggregate-results clean valgrind perf +.PHONY: pre-clean $(T) aggregate-results clean valgrind perf check-chainlint clean-chainlint From 5238710eb404cc81de511743811f92759cb7ca7b Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:35 -0400 Subject: [PATCH 03/11] t/chainlint: add chainlint "basic" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/arithmetic-expansion.expect | 9 +++++++++ t/chainlint/arithmetic-expansion.test | 11 ++++++++++ t/chainlint/broken-chain.expect | 6 ++++++ t/chainlint/broken-chain.test | 8 ++++++++ t/chainlint/close-subshell.expect | 25 +++++++++++++++++++++++ t/chainlint/close-subshell.test | 27 +++++++++++++++++++++++++ t/chainlint/command-substitution.expect | 9 +++++++++ t/chainlint/command-substitution.test | 11 ++++++++++ t/chainlint/exit-subshell.expect | 5 +++++ t/chainlint/exit-subshell.test | 6 ++++++ t/chainlint/multi-line-string.expect | 9 +++++++++ t/chainlint/multi-line-string.test | 15 ++++++++++++++ t/chainlint/pipe.expect | 8 ++++++++ t/chainlint/pipe.test | 12 +++++++++++ t/chainlint/semicolon.expect | 20 ++++++++++++++++++ t/chainlint/semicolon.test | 25 +++++++++++++++++++++++ 16 files changed, 206 insertions(+) create mode 100644 t/chainlint/arithmetic-expansion.expect create mode 100644 t/chainlint/arithmetic-expansion.test create mode 100644 t/chainlint/broken-chain.expect create mode 100644 t/chainlint/broken-chain.test create mode 100644 t/chainlint/close-subshell.expect create mode 100644 t/chainlint/close-subshell.test create mode 100644 t/chainlint/command-substitution.expect create mode 100644 t/chainlint/command-substitution.test create mode 100644 t/chainlint/exit-subshell.expect create mode 100644 t/chainlint/exit-subshell.test create mode 100644 t/chainlint/multi-line-string.expect create mode 100644 t/chainlint/multi-line-string.test create mode 100644 t/chainlint/pipe.expect create mode 100644 t/chainlint/pipe.test create mode 100644 t/chainlint/semicolon.expect create mode 100644 t/chainlint/semicolon.test diff --git a/t/chainlint/arithmetic-expansion.expect b/t/chainlint/arithmetic-expansion.expect new file mode 100644 index 0000000000..09457d3196 --- /dev/null +++ b/t/chainlint/arithmetic-expansion.expect @@ -0,0 +1,9 @@ +( + foo && + bar=$((42 + 1)) && + baz +>) && +( +?!AMP?! bar=$((42 + 1)) + baz +>) diff --git a/t/chainlint/arithmetic-expansion.test b/t/chainlint/arithmetic-expansion.test new file mode 100644 index 0000000000..16206960d8 --- /dev/null +++ b/t/chainlint/arithmetic-expansion.test @@ -0,0 +1,11 @@ +( + foo && +# LINT: closing ")" of $((...)) not misinterpreted as subshell-closing ")" + bar=$((42 + 1)) && + baz +) && +( +# LINT: missing "&&" on $((...)) + bar=$((42 + 1)) + baz +) diff --git a/t/chainlint/broken-chain.expect b/t/chainlint/broken-chain.expect new file mode 100644 index 0000000000..55b0f42a53 --- /dev/null +++ b/t/chainlint/broken-chain.expect @@ -0,0 +1,6 @@ +( + foo && +?!AMP?! bar + baz && + wop +>) diff --git a/t/chainlint/broken-chain.test b/t/chainlint/broken-chain.test new file mode 100644 index 0000000000..3cc67b65d0 --- /dev/null +++ b/t/chainlint/broken-chain.test @@ -0,0 +1,8 @@ +( + foo && +# LINT: missing "&&" from 'bar' + bar + baz && +# LINT: final statement before closing ")" legitimately lacks "&&" + wop +) diff --git a/t/chainlint/close-subshell.expect b/t/chainlint/close-subshell.expect new file mode 100644 index 0000000000..184688718a --- /dev/null +++ b/t/chainlint/close-subshell.expect @@ -0,0 +1,25 @@ +( + foo +>) && +( + bar +>) >out && +( + baz +>) 2>err && +( + boo +>) ) | wuzzle && +( + bop +>) | fazz fozz && +( + bup +>) | +fuzzle && +( + yop +>) diff --git a/t/chainlint/close-subshell.test b/t/chainlint/close-subshell.test new file mode 100644 index 0000000000..508ca447fd --- /dev/null +++ b/t/chainlint/close-subshell.test @@ -0,0 +1,27 @@ +# LINT: closing ")" with various decorations ("&&", ">", "|", etc.) +( + foo +) && +( + bar +) >out && +( + baz +) 2>err && +( + boo +) ) && +( +?!AMP?! bar=$(gobble blocks) + baz +>) diff --git a/t/chainlint/command-substitution.test b/t/chainlint/command-substitution.test new file mode 100644 index 0000000000..3bbb002a4c --- /dev/null +++ b/t/chainlint/command-substitution.test @@ -0,0 +1,11 @@ +( + foo && +# LINT: closing ")" of $(...) not misinterpreted as subshell-closing ")" + bar=$(gobble) && + baz +) && +( +# LINT: missing "&&" on $(...) + bar=$(gobble blocks) + baz +) diff --git a/t/chainlint/exit-subshell.expect b/t/chainlint/exit-subshell.expect new file mode 100644 index 0000000000..bf78454f74 --- /dev/null +++ b/t/chainlint/exit-subshell.expect @@ -0,0 +1,5 @@ +( + foo || exit 1 + bar && + baz +>) diff --git a/t/chainlint/exit-subshell.test b/t/chainlint/exit-subshell.test new file mode 100644 index 0000000000..4e6ab69b88 --- /dev/null +++ b/t/chainlint/exit-subshell.test @@ -0,0 +1,6 @@ +( +# LINT: "|| exit {n}" valid subshell escape without hurting &&-chain + foo || exit 1 + bar && + baz +) diff --git a/t/chainlint/multi-line-string.expect b/t/chainlint/multi-line-string.expect new file mode 100644 index 0000000000..8334c4cc8e --- /dev/null +++ b/t/chainlint/multi-line-string.expect @@ -0,0 +1,9 @@ +( + x=line 1 line 2 line 3" && +?!AMP?! y=line 1 line2' + foobar +>) && +( + echo "there's nothing to see here" && + exit +>) diff --git a/t/chainlint/multi-line-string.test b/t/chainlint/multi-line-string.test new file mode 100644 index 0000000000..14cb44d51c --- /dev/null +++ b/t/chainlint/multi-line-string.test @@ -0,0 +1,15 @@ +( + x="line 1 + line 2 + line 3" && +# LINT: missing "&&" on assignment + y='line 1 + line2' + foobar +) && +( +# LINT: apostrophe (in a contraction) within string not misinterpreted as +# LINT: starting multi-line single-quoted string + echo "there's nothing to see here" && + exit +) diff --git a/t/chainlint/pipe.expect b/t/chainlint/pipe.expect new file mode 100644 index 0000000000..211b901dbc --- /dev/null +++ b/t/chainlint/pipe.expect @@ -0,0 +1,8 @@ +( + foo | + bar | + baz && + fish | +?!AMP?! cow + sunder +>) diff --git a/t/chainlint/pipe.test b/t/chainlint/pipe.test new file mode 100644 index 0000000000..e6af4de916 --- /dev/null +++ b/t/chainlint/pipe.test @@ -0,0 +1,12 @@ +( +# LINT: no "&&" needed on line ending with "|" + foo | + bar | + baz && + +# LINT: final line of pipe sequence ('cow') lacking "&&" + fish | + cow + + sunder +) diff --git a/t/chainlint/semicolon.expect b/t/chainlint/semicolon.expect new file mode 100644 index 0000000000..1d79384606 --- /dev/null +++ b/t/chainlint/semicolon.expect @@ -0,0 +1,20 @@ +( +?!AMP?!?!SEMI?! cat foo ; echo bar +?!SEMI?! cat foo ; echo bar +>) && +( +?!SEMI?! cat foo ; echo bar && +?!SEMI?! cat foo ; echo bar +>) && +( + echo "foo; bar" && +?!SEMI?! cat foo; echo bar +>) && +( +?!SEMI?! foo; +>) && +( +cd foo && + for i in a b c; do +?!SEMI?! echo; +> done) diff --git a/t/chainlint/semicolon.test b/t/chainlint/semicolon.test new file mode 100644 index 0000000000..d82c8ebbc0 --- /dev/null +++ b/t/chainlint/semicolon.test @@ -0,0 +1,25 @@ +( +# LINT: missing internal "&&" and ending "&&" + cat foo ; echo bar +# LINT: final statement before ")" only missing internal "&&" + cat foo ; echo bar +) && +( +# LINT: missing internal "&&" + cat foo ; echo bar && + cat foo ; echo bar +) && +( +# LINT: not fooled by semicolon in string + echo "foo; bar" && + cat foo; echo bar +) && +( +# LINT: unnecessary terminating semicolon + foo; +) && +(cd foo && + for i in a b c; do +# LINT: unnecessary terminating semicolon + echo; + done) From 7b9067901260ece0274976a7fc39e84796ceb852 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:36 -0400 Subject: [PATCH 04/11] t/chainlint: add chainlint "whitespace" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/blank-line.expect | 4 ++++ t/chainlint/blank-line.test | 10 ++++++++++ t/chainlint/comment.expect | 4 ++++ t/chainlint/comment.test | 11 +++++++++++ t/chainlint/here-doc.expect | 3 +++ t/chainlint/here-doc.test | 16 ++++++++++++++++ t/chainlint/incomplete-line.expect | 4 ++++ t/chainlint/incomplete-line.test | 12 ++++++++++++ t/chainlint/inline-comment.expect | 9 +++++++++ t/chainlint/inline-comment.test | 12 ++++++++++++ t/chainlint/subshell-here-doc.expect | 5 +++++ t/chainlint/subshell-here-doc.test | 23 +++++++++++++++++++++++ 12 files changed, 113 insertions(+) create mode 100644 t/chainlint/blank-line.expect create mode 100644 t/chainlint/blank-line.test create mode 100644 t/chainlint/comment.expect create mode 100644 t/chainlint/comment.test create mode 100644 t/chainlint/here-doc.expect create mode 100644 t/chainlint/here-doc.test create mode 100644 t/chainlint/incomplete-line.expect create mode 100644 t/chainlint/incomplete-line.test create mode 100644 t/chainlint/inline-comment.expect create mode 100644 t/chainlint/inline-comment.test create mode 100644 t/chainlint/subshell-here-doc.expect create mode 100644 t/chainlint/subshell-here-doc.test diff --git a/t/chainlint/blank-line.expect b/t/chainlint/blank-line.expect new file mode 100644 index 0000000000..3be939ed38 --- /dev/null +++ b/t/chainlint/blank-line.expect @@ -0,0 +1,4 @@ +( + nothing && + something +>) diff --git a/t/chainlint/blank-line.test b/t/chainlint/blank-line.test new file mode 100644 index 0000000000..f6dd14302b --- /dev/null +++ b/t/chainlint/blank-line.test @@ -0,0 +1,10 @@ +( + + nothing && + + something +# LINT: swallow blank lines since final _statement_ before subshell end is +# LINT: significant to "&&"-check, not final _line_ (which might be blank) + + +) diff --git a/t/chainlint/comment.expect b/t/chainlint/comment.expect new file mode 100644 index 0000000000..3be939ed38 --- /dev/null +++ b/t/chainlint/comment.expect @@ -0,0 +1,4 @@ +( + nothing && + something +>) diff --git a/t/chainlint/comment.test b/t/chainlint/comment.test new file mode 100644 index 0000000000..113c0c466f --- /dev/null +++ b/t/chainlint/comment.test @@ -0,0 +1,11 @@ +( +# LINT: swallow comment lines + # comment 1 + nothing && + # comment 2 + something +# LINT: swallow comment lines since final _statement_ before subshell end is +# LINT: significant to "&&"-check, not final _line_ (which might be comment) + # comment 3 + # comment 4 +) diff --git a/t/chainlint/here-doc.expect b/t/chainlint/here-doc.expect new file mode 100644 index 0000000000..2328fe7753 --- /dev/null +++ b/t/chainlint/here-doc.expect @@ -0,0 +1,3 @@ +boodle wobba gorgo snoot wafta snurb && + +horticulture diff --git a/t/chainlint/here-doc.test b/t/chainlint/here-doc.test new file mode 100644 index 0000000000..bd36f6e1d3 --- /dev/null +++ b/t/chainlint/here-doc.test @@ -0,0 +1,16 @@ +# LINT: stitch together incomplete \-ending lines +# LINT: swallow here-doc to avoid false positives in content +boodle wobba \ + gorgo snoot \ + wafta snurb <) diff --git a/t/chainlint/incomplete-line.test b/t/chainlint/incomplete-line.test new file mode 100644 index 0000000000..d856658083 --- /dev/null +++ b/t/chainlint/incomplete-line.test @@ -0,0 +1,12 @@ +# LINT: stitch together all incomplete \-ending lines +line 1 \ +line 2 \ +line 3 \ +line 4 && +( +# LINT: stitch together all incomplete \-ending lines (subshell) + line 5 \ + line 6 \ + line 7 \ + line 8 +) diff --git a/t/chainlint/inline-comment.expect b/t/chainlint/inline-comment.expect new file mode 100644 index 0000000000..fc9f250ac4 --- /dev/null +++ b/t/chainlint/inline-comment.expect @@ -0,0 +1,9 @@ +( + foobar && +?!AMP?! barfoo + flibble "not a # comment" +>) && + +( +cd foo && +> flibble "not a # comment") diff --git a/t/chainlint/inline-comment.test b/t/chainlint/inline-comment.test new file mode 100644 index 0000000000..8f26856e77 --- /dev/null +++ b/t/chainlint/inline-comment.test @@ -0,0 +1,12 @@ +( +# LINT: swallow inline comment (leaving command intact) + foobar && # comment 1 +# LINT: mispositioned "&&" (correctly) swallowed with comment + barfoo # wrong position for && +# LINT: "#" in string not misinterpreted as comment + flibble "not a # comment" +) && + +# LINT: "#" in string in cuddled subshell not misinterpreted as comment +(cd foo && + flibble "not a # comment") diff --git a/t/chainlint/subshell-here-doc.expect b/t/chainlint/subshell-here-doc.expect new file mode 100644 index 0000000000..19d5aff233 --- /dev/null +++ b/t/chainlint/subshell-here-doc.expect @@ -0,0 +1,5 @@ +( + echo wobba gorgo snoot wafta snurb && +?!AMP?! cat >bip + echo >bop +>) diff --git a/t/chainlint/subshell-here-doc.test b/t/chainlint/subshell-here-doc.test new file mode 100644 index 0000000000..9c3564c247 --- /dev/null +++ b/t/chainlint/subshell-here-doc.test @@ -0,0 +1,23 @@ +( +# LINT: stitch together incomplete \-ending lines +# LINT: swallow here-doc to avoid false positives in content + echo wobba \ + gorgo snoot \ + wafta snurb <<-EOF && + quoth the raven, + nevermore... + EOF + +# LINT: missing "&&" on 'cat' + cat <bip + fish fly high + EOF + +# LINT: swallow here-doc (EOF is last line of subshell) + echo <<-\EOF >bop + gomez + morticia + wednesday + pugsly + EOF +) From 90a880393a5fd7cff9842d553786949c7cafb48b Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:37 -0400 Subject: [PATCH 05/11] t/chainlint: add chainlint "one-liner" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/negated-one-liner.expect | 5 +++++ t/chainlint/negated-one-liner.test | 7 +++++++ t/chainlint/one-liner.expect | 9 +++++++++ t/chainlint/one-liner.test | 12 ++++++++++++ t/chainlint/subshell-one-liner.expect | 14 ++++++++++++++ t/chainlint/subshell-one-liner.test | 24 ++++++++++++++++++++++++ 6 files changed, 71 insertions(+) create mode 100644 t/chainlint/negated-one-liner.expect create mode 100644 t/chainlint/negated-one-liner.test create mode 100644 t/chainlint/one-liner.expect create mode 100644 t/chainlint/one-liner.test create mode 100644 t/chainlint/subshell-one-liner.expect create mode 100644 t/chainlint/subshell-one-liner.test diff --git a/t/chainlint/negated-one-liner.expect b/t/chainlint/negated-one-liner.expect new file mode 100644 index 0000000000..cf18429d03 --- /dev/null +++ b/t/chainlint/negated-one-liner.expect @@ -0,0 +1,5 @@ +! (foo && bar) && +! (foo && bar) >baz && + +?!SEMI?!! (foo; bar) && +?!SEMI?!! (foo; bar) >baz diff --git a/t/chainlint/negated-one-liner.test b/t/chainlint/negated-one-liner.test new file mode 100644 index 0000000000..c9598e9153 --- /dev/null +++ b/t/chainlint/negated-one-liner.test @@ -0,0 +1,7 @@ +# LINT: top-level one-liner subshell +! (foo && bar) && +! (foo && bar) >baz && + +# LINT: top-level one-liner subshell missing internal "&&" +! (foo; bar) && +! (foo; bar) >baz diff --git a/t/chainlint/one-liner.expect b/t/chainlint/one-liner.expect new file mode 100644 index 0000000000..237f227349 --- /dev/null +++ b/t/chainlint/one-liner.expect @@ -0,0 +1,9 @@ +(foo && bar) && +(foo && bar) | +(foo && bar) >baz && + +?!SEMI?!(foo; bar) && +?!SEMI?!(foo; bar) | +?!SEMI?!(foo; bar) >baz + +(foo "bar; baz") diff --git a/t/chainlint/one-liner.test b/t/chainlint/one-liner.test new file mode 100644 index 0000000000..ec9acb9825 --- /dev/null +++ b/t/chainlint/one-liner.test @@ -0,0 +1,12 @@ +# LINT: top-level one-liner subshell +(foo && bar) && +(foo && bar) | +(foo && bar) >baz && + +# LINT: top-level one-liner subshell missing internal "&&" +(foo; bar) && +(foo; bar) | +(foo; bar) >baz + +# LINT: ";" in string not misinterpreted as broken &&-chain +(foo "bar; baz") diff --git a/t/chainlint/subshell-one-liner.expect b/t/chainlint/subshell-one-liner.expect new file mode 100644 index 0000000000..51162821d7 --- /dev/null +++ b/t/chainlint/subshell-one-liner.expect @@ -0,0 +1,14 @@ +( + (foo && bar) && + (foo && bar) | + (foo && bar) >baz && +?!SEMI?! (foo; bar) && +?!SEMI?! (foo; bar) | +?!SEMI?! (foo; bar) >baz && + (foo || exit 1) && + (foo || exit 1) | + (foo || exit 1) >baz && +?!AMP?! (foo && bar) +?!AMP?!?!SEMI?! (foo && bar; baz) + foobar +>) diff --git a/t/chainlint/subshell-one-liner.test b/t/chainlint/subshell-one-liner.test new file mode 100644 index 0000000000..37fa643c20 --- /dev/null +++ b/t/chainlint/subshell-one-liner.test @@ -0,0 +1,24 @@ +( +# LINT: nested one-liner subshell + (foo && bar) && + (foo && bar) | + (foo && bar) >baz && + +# LINT: nested one-liner subshell missing internal "&&" + (foo; bar) && + (foo; bar) | + (foo; bar) >baz && + +# LINT: nested one-liner subshell with "|| exit" + (foo || exit 1) && + (foo || exit 1) | + (foo || exit 1) >baz && + +# LINT: nested one-liner subshell lacking ending "&&" + (foo && bar) + +# LINT: nested one-liner subshell missing internal "&&" and lacking ending "&&" + (foo && bar; baz) + + foobar +) From bb4efbc5df9d0c96392dd14fc3ab6164949a2f52 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:38 -0400 Subject: [PATCH 06/11] t/chainlint: add chainlint "nested subshell" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/block.expect | 12 +++++++ t/chainlint/block.test | 15 +++++++++ ...ti-line-nested-command-substitution.expect | 9 ++++++ ...ulti-line-nested-command-substitution.test | 9 ++++++ t/chainlint/nested-cuddled-subshell.expect | 19 ++++++++++++ t/chainlint/nested-cuddled-subshell.test | 31 +++++++++++++++++++ t/chainlint/nested-here-doc.expect | 5 +++ t/chainlint/nested-here-doc.test | 23 ++++++++++++++ t/chainlint/nested-subshell-comment.expect | 11 +++++++ t/chainlint/nested-subshell-comment.test | 13 ++++++++ t/chainlint/nested-subshell.expect | 12 +++++++ t/chainlint/nested-subshell.test | 14 +++++++++ 12 files changed, 173 insertions(+) create mode 100644 t/chainlint/block.expect create mode 100644 t/chainlint/block.test create mode 100644 t/chainlint/multi-line-nested-command-substitution.expect create mode 100644 t/chainlint/multi-line-nested-command-substitution.test create mode 100644 t/chainlint/nested-cuddled-subshell.expect create mode 100644 t/chainlint/nested-cuddled-subshell.test create mode 100644 t/chainlint/nested-here-doc.expect create mode 100644 t/chainlint/nested-here-doc.test create mode 100644 t/chainlint/nested-subshell-comment.expect create mode 100644 t/chainlint/nested-subshell-comment.test create mode 100644 t/chainlint/nested-subshell.expect create mode 100644 t/chainlint/nested-subshell.test diff --git a/t/chainlint/block.expect b/t/chainlint/block.expect new file mode 100644 index 0000000000..fed7e89ae8 --- /dev/null +++ b/t/chainlint/block.expect @@ -0,0 +1,12 @@ +( + foo && + { + echo a + echo b + } && + bar && + { + echo c +?!AMP?! } + baz +>) diff --git a/t/chainlint/block.test b/t/chainlint/block.test new file mode 100644 index 0000000000..d859151af1 --- /dev/null +++ b/t/chainlint/block.test @@ -0,0 +1,15 @@ +( +# LINT: missing "&&" in block not currently detected (for consistency with +# LINT: --chain-lint at top level and to provide escape hatch if needed) + foo && + { + echo a + echo b + } && + bar && +# LINT: missing "&&" at closing "}" + { + echo c + } + baz +) diff --git a/t/chainlint/multi-line-nested-command-substitution.expect b/t/chainlint/multi-line-nested-command-substitution.expect new file mode 100644 index 0000000000..19c023b1c8 --- /dev/null +++ b/t/chainlint/multi-line-nested-command-substitution.expect @@ -0,0 +1,9 @@ +( + foo && + x=$( + echo bar | + cat +>> ) && + echo ok +>) | +sort diff --git a/t/chainlint/multi-line-nested-command-substitution.test b/t/chainlint/multi-line-nested-command-substitution.test new file mode 100644 index 0000000000..ca0620ab6b --- /dev/null +++ b/t/chainlint/multi-line-nested-command-substitution.test @@ -0,0 +1,9 @@ +( + foo && + x=$( + echo bar | + cat + ) && + echo ok +) | +sort diff --git a/t/chainlint/nested-cuddled-subshell.expect b/t/chainlint/nested-cuddled-subshell.expect new file mode 100644 index 0000000000..c2a59ffc33 --- /dev/null +++ b/t/chainlint/nested-cuddled-subshell.expect @@ -0,0 +1,19 @@ +( + (cd foo && + bar +>> ) && + (cd foo && + bar +?!AMP?!>> ) + ( + cd foo && +>> bar) && + ( + cd foo && +?!AMP?!>> bar) + (cd foo && +>> bar) && + (cd foo && +?!AMP?!>> bar) + foobar +>) diff --git a/t/chainlint/nested-cuddled-subshell.test b/t/chainlint/nested-cuddled-subshell.test new file mode 100644 index 0000000000..8fd656c7b5 --- /dev/null +++ b/t/chainlint/nested-cuddled-subshell.test @@ -0,0 +1,31 @@ +( +# LINT: opening "(" cuddled with first nested subshell statement + (cd foo && + bar + ) && + +# LINT: same but "&&" missing + (cd foo && + bar + ) + +# LINT: closing ")" cuddled with final nested subshell statement + ( + cd foo && + bar) && + +# LINT: same but "&&" missing + ( + cd foo && + bar) + +# LINT: "(" and ")" cuddled with first and final subshell statements + (cd foo && + bar) && + +# LINT: same but "&&" missing + (cd foo && + bar) + + foobar +) diff --git a/t/chainlint/nested-here-doc.expect b/t/chainlint/nested-here-doc.expect new file mode 100644 index 0000000000..559301e005 --- /dev/null +++ b/t/chainlint/nested-here-doc.expect @@ -0,0 +1,5 @@ +( + cat && +?!AMP?! cat + foobar +>) diff --git a/t/chainlint/nested-here-doc.test b/t/chainlint/nested-here-doc.test new file mode 100644 index 0000000000..027e0bb3ff --- /dev/null +++ b/t/chainlint/nested-here-doc.test @@ -0,0 +1,23 @@ +( +# LINT: inner "EOF" not misintrepreted as closing INPUT_END here-doc + cat <<-\INPUT_END && + fish are mice + but geese go slow + data <> ) + fuzzy +>) diff --git a/t/chainlint/nested-subshell-comment.test b/t/chainlint/nested-subshell-comment.test new file mode 100644 index 0000000000..0ff136ab3c --- /dev/null +++ b/t/chainlint/nested-subshell-comment.test @@ -0,0 +1,13 @@ +( + foo && + ( + bar && +# LINT: ")" in comment in nested subshell not misinterpreted as closing ")" + # bottles wobble while fiddles gobble + # minor numbers of cows (or do they?) + baz && + snaff +# LINT: missing "&&" on ')' + ) + fuzzy +) diff --git a/t/chainlint/nested-subshell.expect b/t/chainlint/nested-subshell.expect new file mode 100644 index 0000000000..c8165ad19e --- /dev/null +++ b/t/chainlint/nested-subshell.expect @@ -0,0 +1,12 @@ +( + cd foo && + ( + echo a && + echo b +>> ) >file && + cd foo && + ( + echo a + echo b +>> ) >file +>) diff --git a/t/chainlint/nested-subshell.test b/t/chainlint/nested-subshell.test new file mode 100644 index 0000000000..998b05a47d --- /dev/null +++ b/t/chainlint/nested-subshell.test @@ -0,0 +1,14 @@ +( + cd foo && + ( + echo a && + echo b + ) >file && + + cd foo && + ( +# LINT: nested multi-line subshell not presently checked for missing "&&" + echo a + echo b + ) >file +) From ebcbbe060f5805bacafaf971fe9c0c0519b18f61 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:39 -0400 Subject: [PATCH 07/11] t/chainlint: add chainlint "loop" and "conditional" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/case.expect | 19 +++++++++++++++++++ t/chainlint/case.test | 23 +++++++++++++++++++++++ t/chainlint/exit-loop.expect | 24 ++++++++++++++++++++++++ t/chainlint/exit-loop.test | 27 +++++++++++++++++++++++++++ t/chainlint/for-loop.expect | 11 +++++++++++ t/chainlint/for-loop.test | 19 +++++++++++++++++++ t/chainlint/if-then-else.expect | 19 +++++++++++++++++++ t/chainlint/if-then-else.test | 28 ++++++++++++++++++++++++++++ t/chainlint/while-loop.expect | 11 +++++++++++ t/chainlint/while-loop.test | 19 +++++++++++++++++++ 10 files changed, 200 insertions(+) create mode 100644 t/chainlint/case.expect create mode 100644 t/chainlint/case.test create mode 100644 t/chainlint/exit-loop.expect create mode 100644 t/chainlint/exit-loop.test create mode 100644 t/chainlint/for-loop.expect create mode 100644 t/chainlint/for-loop.test create mode 100644 t/chainlint/if-then-else.expect create mode 100644 t/chainlint/if-then-else.test create mode 100644 t/chainlint/while-loop.expect create mode 100644 t/chainlint/while-loop.test diff --git a/t/chainlint/case.expect b/t/chainlint/case.expect new file mode 100644 index 0000000000..41f121fbbf --- /dev/null +++ b/t/chainlint/case.expect @@ -0,0 +1,19 @@ +( + case "$x" in + x) foo ;; + *) bar ;; + esac && + foobar +>) && +( + case "$x" in + x) foo ;; + *) bar ;; +?!AMP?! esac + foobar +>) && +( + case "$x" in 1) true;; esac && +?!AMP?! case "$y" in 2) false;; esac + foobar +>) diff --git a/t/chainlint/case.test b/t/chainlint/case.test new file mode 100644 index 0000000000..5ef6ff7db5 --- /dev/null +++ b/t/chainlint/case.test @@ -0,0 +1,23 @@ +( +# LINT: "...)" arms in 'case' not misinterpreted as subshell-closing ")" + case "$x" in + x) foo ;; + *) bar ;; + esac && + foobar +) && +( +# LINT: missing "&&" on 'esac' + case "$x" in + x) foo ;; + *) bar ;; + esac + foobar +) && +( +# LINT: "...)" arm in one-liner 'case' not misinterpreted as closing ")" + case "$x" in 1) true;; esac && +# LINT: same but missing "&&" + case "$y" in 2) false;; esac + foobar +) diff --git a/t/chainlint/exit-loop.expect b/t/chainlint/exit-loop.expect new file mode 100644 index 0000000000..84d8bdebc0 --- /dev/null +++ b/t/chainlint/exit-loop.expect @@ -0,0 +1,24 @@ +( + for i in a b c + do + foo || exit 1 + bar && + baz + done +>) && +( + while true + do + foo || exit 1 + bar && + baz + done +>) && +( + i=0 && + while test $i -lt 10 + do + echo $i || exit + i=$(($i + 1)) + done +>) diff --git a/t/chainlint/exit-loop.test b/t/chainlint/exit-loop.test new file mode 100644 index 0000000000..2f038207e1 --- /dev/null +++ b/t/chainlint/exit-loop.test @@ -0,0 +1,27 @@ +( + for i in a b c + do +# LINT: "|| exit {n}" valid for-loop escape in subshell; no "&&" needed + foo || exit 1 + bar && + baz + done +) && +( + while true + do +# LINT: "|| exit {n}" valid while-loop escape in subshell; no "&&" needed + foo || exit 1 + bar && + baz + done +) && +( + i=0 && + while test $i -lt 10 + do +# LINT: "|| exit" (sans exit code) valid escape in subshell; no "&&" needed + echo $i || exit + i=$(($i + 1)) + done +) diff --git a/t/chainlint/for-loop.expect b/t/chainlint/for-loop.expect new file mode 100644 index 0000000000..c33cf56ee7 --- /dev/null +++ b/t/chainlint/for-loop.expect @@ -0,0 +1,11 @@ +( + for i in a b c + do +?!AMP?! echo $i + cat +?!AMP?! done + for i in a b c; do + echo $i && + cat $i + done +>) diff --git a/t/chainlint/for-loop.test b/t/chainlint/for-loop.test new file mode 100644 index 0000000000..7db76262bc --- /dev/null +++ b/t/chainlint/for-loop.test @@ -0,0 +1,19 @@ +( +# LINT: 'for', 'do', 'done' do not need "&&" + for i in a b c + do +# LINT: missing "&&" on 'echo' + echo $i +# LINT: last statement of while does not need "&&" + cat <<-\EOF + bar + EOF +# LINT: missing "&&" on 'done' + done + +# LINT: 'do' on same line as 'for' + for i in a b c; do + echo $i && + cat $i + done +) diff --git a/t/chainlint/if-then-else.expect b/t/chainlint/if-then-else.expect new file mode 100644 index 0000000000..5953c7bfbc --- /dev/null +++ b/t/chainlint/if-then-else.expect @@ -0,0 +1,19 @@ +( + if test -n "" + then +?!AMP?! echo very + echo empty + elif test -z "" + echo foo + else + echo foo && + cat +?!AMP?! fi + echo poodle +>) && +( + if test -n ""; then + echo very && +?!AMP?! echo empty + if +>) diff --git a/t/chainlint/if-then-else.test b/t/chainlint/if-then-else.test new file mode 100644 index 0000000000..9bd8e9a4c6 --- /dev/null +++ b/t/chainlint/if-then-else.test @@ -0,0 +1,28 @@ +( +# LINT: 'if', 'then', 'elif', 'else', 'fi' do not need "&&" + if test -n "" + then +# LINT: missing "&&" on 'echo' + echo very +# LINT: last statement before 'elif' does not need "&&" + echo empty + elif test -z "" +# LINT: last statement before 'else' does not need "&&" + echo foo + else + echo foo && +# LINT: last statement before 'fi' does not need "&&" + cat <<-\EOF + bar + EOF +# LINT: missing "&&" on 'fi' + fi + echo poodle +) && +( +# LINT: 'then' on same line as 'if' + if test -n ""; then + echo very && + echo empty + if +) diff --git a/t/chainlint/while-loop.expect b/t/chainlint/while-loop.expect new file mode 100644 index 0000000000..13cff2c0a5 --- /dev/null +++ b/t/chainlint/while-loop.expect @@ -0,0 +1,11 @@ +( + while true + do +?!AMP?! echo foo + cat +?!AMP?! done + while true; do + echo foo && + cat bar + done +>) diff --git a/t/chainlint/while-loop.test b/t/chainlint/while-loop.test new file mode 100644 index 0000000000..f1df085bf0 --- /dev/null +++ b/t/chainlint/while-loop.test @@ -0,0 +1,19 @@ +( +# LINT: 'while, 'do', 'done' do not need "&&" + while true + do +# LINT: missing "&&" on 'echo' + echo foo +# LINT: last statement of while does not need "&&" + cat <<-\EOF + bar + EOF +# LINT: missing "&&" on 'done' + done + +# LINT: 'do' on same line as 'while' + while true; do + echo foo && + cat bar + done +) From 24c86180649baec2dde4d5e5b6350d048f5d6bb3 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:40 -0400 Subject: [PATCH 08/11] t/chainlint: add chainlint "cuddled" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/cuddled-if-then-else.expect | 7 +++++++ t/chainlint/cuddled-if-then-else.test | 7 +++++++ t/chainlint/cuddled-loop.expect | 5 +++++ t/chainlint/cuddled-loop.test | 7 +++++++ t/chainlint/cuddled.expect | 21 +++++++++++++++++++++ t/chainlint/cuddled.test | 23 +++++++++++++++++++++++ 6 files changed, 70 insertions(+) create mode 100644 t/chainlint/cuddled-if-then-else.expect create mode 100644 t/chainlint/cuddled-if-then-else.test create mode 100644 t/chainlint/cuddled-loop.expect create mode 100644 t/chainlint/cuddled-loop.test create mode 100644 t/chainlint/cuddled.expect create mode 100644 t/chainlint/cuddled.test diff --git a/t/chainlint/cuddled-if-then-else.expect b/t/chainlint/cuddled-if-then-else.expect new file mode 100644 index 0000000000..ab2a026fbc --- /dev/null +++ b/t/chainlint/cuddled-if-then-else.expect @@ -0,0 +1,7 @@ +( +if test -z ""; then + echo empty + else + echo bizzy +> fi) && +echo foobar diff --git a/t/chainlint/cuddled-if-then-else.test b/t/chainlint/cuddled-if-then-else.test new file mode 100644 index 0000000000..eed774a9d6 --- /dev/null +++ b/t/chainlint/cuddled-if-then-else.test @@ -0,0 +1,7 @@ +# LINT: 'if' cuddled with "(" and ")"; indented with spaces, not tabs +(if test -z ""; then + echo empty + else + echo bizzy + fi) && +echo foobar diff --git a/t/chainlint/cuddled-loop.expect b/t/chainlint/cuddled-loop.expect new file mode 100644 index 0000000000..8c0260d7f1 --- /dev/null +++ b/t/chainlint/cuddled-loop.expect @@ -0,0 +1,5 @@ +( + while read x + do foobar bop || exit 1 +> done ) && + +( +?!AMP?!cd foo + bar +>) && + +( + cd foo && +> bar) && + +( +cd foo && +> bar) && + +( +?!AMP?!cd foo +> bar) diff --git a/t/chainlint/cuddled.test b/t/chainlint/cuddled.test new file mode 100644 index 0000000000..0499fa4180 --- /dev/null +++ b/t/chainlint/cuddled.test @@ -0,0 +1,23 @@ +# LINT: first subshell statement cuddled with opening "("; for implementation +# LINT: simplicity, "(..." is split into two lines, "(" and "..." +(cd foo && + bar +) && + +# LINT: same with missing "&&" +(cd foo + bar +) && + +# LINT: closing ")" cuddled with final subshell statement +( + cd foo && + bar) && + +# LINT: "(" and ")" cuddled with first and final subshell statements +(cd foo && + bar) && + +# LINT: same with missing "&&" +(cd foo + bar) From 1f718b0b78f4fe1b6686c6bbae4ec4859d240070 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:41 -0400 Subject: [PATCH 09/11] t/chainlint: add chainlint "complex" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- .../close-nested-and-parent-together.expect | 4 ++++ t/chainlint/close-nested-and-parent-together.test | 3 +++ t/chainlint/complex-if-in-cuddled-loop.expect | 10 ++++++++++ t/chainlint/complex-if-in-cuddled-loop.test | 11 +++++++++++ t/chainlint/if-in-loop.expect | 12 ++++++++++++ t/chainlint/if-in-loop.test | 15 +++++++++++++++ t/chainlint/loop-in-if.expect | 12 ++++++++++++ t/chainlint/loop-in-if.test | 15 +++++++++++++++ 8 files changed, 82 insertions(+) create mode 100644 t/chainlint/close-nested-and-parent-together.expect create mode 100644 t/chainlint/close-nested-and-parent-together.test create mode 100644 t/chainlint/complex-if-in-cuddled-loop.expect create mode 100644 t/chainlint/complex-if-in-cuddled-loop.test create mode 100644 t/chainlint/if-in-loop.expect create mode 100644 t/chainlint/if-in-loop.test create mode 100644 t/chainlint/loop-in-if.expect create mode 100644 t/chainlint/loop-in-if.test diff --git a/t/chainlint/close-nested-and-parent-together.expect b/t/chainlint/close-nested-and-parent-together.expect new file mode 100644 index 0000000000..2a910f9d66 --- /dev/null +++ b/t/chainlint/close-nested-and-parent-together.expect @@ -0,0 +1,4 @@ +( +cd foo && + (bar && +>>> baz)) diff --git a/t/chainlint/close-nested-and-parent-together.test b/t/chainlint/close-nested-and-parent-together.test new file mode 100644 index 0000000000..72d482f76d --- /dev/null +++ b/t/chainlint/close-nested-and-parent-together.test @@ -0,0 +1,3 @@ +(cd foo && + (bar && + baz)) diff --git a/t/chainlint/complex-if-in-cuddled-loop.expect b/t/chainlint/complex-if-in-cuddled-loop.expect new file mode 100644 index 0000000000..9674b88cf2 --- /dev/null +++ b/t/chainlint/complex-if-in-cuddled-loop.expect @@ -0,0 +1,10 @@ +( +for i in a b c; do + if test "$(echo $(waffle bat))" = "eleventeen" && + test "$x" = "$y"; then + : + else + echo >file + fi +> done) && +test ! -f file diff --git a/t/chainlint/complex-if-in-cuddled-loop.test b/t/chainlint/complex-if-in-cuddled-loop.test new file mode 100644 index 0000000000..571bbd85cd --- /dev/null +++ b/t/chainlint/complex-if-in-cuddled-loop.test @@ -0,0 +1,11 @@ +# LINT: 'for' loop cuddled with "(" and ")" and nested 'if' with complex +# LINT: multi-line condition; indented with spaces, not tabs +(for i in a b c; do + if test "$(echo $(waffle bat))" = "eleventeen" && + test "$x" = "$y"; then + : + else + echo >file + fi + done) && +test ! -f file diff --git a/t/chainlint/if-in-loop.expect b/t/chainlint/if-in-loop.expect new file mode 100644 index 0000000000..03d3ceb22d --- /dev/null +++ b/t/chainlint/if-in-loop.expect @@ -0,0 +1,12 @@ +( + for i in a b c + do + if false + then +?!AMP?! echo "err" + exit 1 +?!AMP?! fi + foo +?!AMP?! done + bar +>) diff --git a/t/chainlint/if-in-loop.test b/t/chainlint/if-in-loop.test new file mode 100644 index 0000000000..daf22da164 --- /dev/null +++ b/t/chainlint/if-in-loop.test @@ -0,0 +1,15 @@ +( + for i in a b c + do + if false + then +# LINT: missing "&&" on 'echo' + echo "err" + exit 1 +# LINT: missing "&&" on 'fi' + fi + foo +# LINT: missing "&&" on 'done' + done + bar +) diff --git a/t/chainlint/loop-in-if.expect b/t/chainlint/loop-in-if.expect new file mode 100644 index 0000000000..088e622c31 --- /dev/null +++ b/t/chainlint/loop-in-if.expect @@ -0,0 +1,12 @@ +( + if true + then + while true + do +?!AMP?! echo "pop" + echo "glup" +?!AMP?! done + foo +?!AMP?! fi + bar +>) diff --git a/t/chainlint/loop-in-if.test b/t/chainlint/loop-in-if.test new file mode 100644 index 0000000000..93e8ba8e4d --- /dev/null +++ b/t/chainlint/loop-in-if.test @@ -0,0 +1,15 @@ +( + if true + then + while true + do +# LINT: missing "&&" on 'echo' + echo "pop" + echo "glup" +# LINT: missing "&&" on 'done' + done + foo +# LINT: missing "&&" on 'fi' + fi + bar +) From 950079b7b6342309059c8c0f9e7340f43e82bf93 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Wed, 11 Jul 2018 02:46:42 -0400 Subject: [PATCH 10/11] t/chainlint: add chainlint "specialized" test cases The --chain-lint option uses heuristics and knowledge of shell syntax to detect broken &&-chains in subshells by pure textual inspection. The heuristics handle a range of stylistic variations in existing tests (evolved over the years), however, they are still best-guesses. As such, it is possible for future changes to accidentally break assumptions upon which the heuristics are based. Protect against this possibility by adding tests which check the linter itself for correctness. In addition to protecting against regressions, these tests help document (for humans) expected behavior, which is important since the linter's implementation language ('sed') does not necessarily lend itself to easy comprehension. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint/bash-array.expect | 10 ++++++++++ t/chainlint/bash-array.test | 12 ++++++++++++ t/chainlint/p4-filespec.expect | 4 ++++ t/chainlint/p4-filespec.test | 5 +++++ 4 files changed, 31 insertions(+) create mode 100644 t/chainlint/bash-array.expect create mode 100644 t/chainlint/bash-array.test create mode 100644 t/chainlint/p4-filespec.expect create mode 100644 t/chainlint/p4-filespec.test diff --git a/t/chainlint/bash-array.expect b/t/chainlint/bash-array.expect new file mode 100644 index 0000000000..c4a830d1c1 --- /dev/null +++ b/t/chainlint/bash-array.expect @@ -0,0 +1,10 @@ +( + foo && + bar=(gumbo stumbo wumbo) && + baz +>) && +( + foo && + bar=${#bar[@]} && + baz +>) diff --git a/t/chainlint/bash-array.test b/t/chainlint/bash-array.test new file mode 100644 index 0000000000..92bbb777b8 --- /dev/null +++ b/t/chainlint/bash-array.test @@ -0,0 +1,12 @@ +( + foo && +# LINT: ")" in Bash array assignment not misinterpreted as subshell-closing ")" + bar=(gumbo stumbo wumbo) && + baz +) && +( + foo && +# LINT: Bash array length operator not misinterpreted as comment + bar=${#bar[@]} && + baz +) diff --git a/t/chainlint/p4-filespec.expect b/t/chainlint/p4-filespec.expect new file mode 100644 index 0000000000..98b3d881fd --- /dev/null +++ b/t/chainlint/p4-filespec.expect @@ -0,0 +1,4 @@ +( + p4 print -1 //depot/fiddle#42 >file && + foobar +>) diff --git a/t/chainlint/p4-filespec.test b/t/chainlint/p4-filespec.test new file mode 100644 index 0000000000..4fd2d6e2b8 --- /dev/null +++ b/t/chainlint/p4-filespec.test @@ -0,0 +1,5 @@ +( +# LINT: Perforce revspec in filespec not misinterpreted as in-line comment + p4 print -1 //depot/fiddle#42 >file && + foobar +) From ace64e56c10a61b184e1fcfc0dae34b4e78036de Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 31 Jul 2018 01:03:20 -0400 Subject: [PATCH 11/11] t/chainlint.sed: drop extra spaces from regex character class This character class, like many others in this script, matches horizontal whitespace consisting of spaces and tabs, however, a few extra, entirely harmless, spaces somehow slipped into the expression. Removing them is purely a cosmetic fix. While at it, re-indent three lines with a single TAB each which were incorrectly indented with six spaces. Also, a purely cosmetic fix. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/chainlint.sed | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/chainlint.sed b/t/chainlint.sed index a0de8a3882..5f0882cb38 100644 --- a/t/chainlint.sed +++ b/t/chainlint.sed @@ -71,9 +71,9 @@ # incomplete line -- slurp up next line :squash /\\$/ { - N - s/\\\n// - bsquash + N + s/\\\n// + bsquash } # here-doc -- swallow it to avoid false hits within its body (but keep the @@ -199,7 +199,7 @@ s/.*\n// # "$(...)" -- command substitution; not closing ")" /\$([^)][^)]*)[^)]*$/bcheckchain # multi-line "$(...\n...)" -- command substitution; treat as nested subshell -/\$([ ]*$/bnest +/\$([ ]*$/bnest # "=(...)" -- Bash array assignment; not closing ")" /=(/bcheckchain # closing "...) &&"