chainlint.sed: stop splitting "(..." into separate lines "(" and "..."

Because `sed` is line-oriented, for ease of implementation, when
chainlint.sed encounters an opening subshell in which the first command
is cuddled with the "(", it splits the line into two lines: one
containing only "(", and the other containing whatever follows "(".
This allows chainlint.sed to get by with a single set of regular
expressions for matching shell statements rather than having to
duplicate each expression (one set for matching non-cuddled statements,
and one set for matching cuddled statements).

However, although syntactically and semantically immaterial, this
transformation has no value to test authors and might even confuse them
into thinking that the linter is misbehaving by inserting (whitespace)
line-noise into the shell code it is validating. Moreover, it also
allows an implementation detail of chainlint.sed to seep into the
chainlint self-test "expect" files, which potentially makes it difficult
to reuse the self-tests should a more capable chainlint ever be
developed.

To address these concerns, stop splitting cuddled "(..." into two lines.

Note that, as an implementation artifact, due to sed's line-oriented
nature, this change inserts a blank line at output time just before the
"(..." line is emitted. It would be possible to suppress this blank line
but doing so would add a fair bit of complexity to chainlint.sed.
Therefore, rather than suppressing the extra blank line, the Makefile's
`check-chainlint` target which runs the chainlint self-tests is instead
modified to ignore blank lines when comparing chainlint output against
the self-test "expect" output. This is a reasonable compromise for two
reasons. First, the purpose of the chainlint self-tests is to verify
that the ?!AMP?! annotations are being correctly added; precise
whitespace is immaterial. Second, by necessity, chainlint.sed itself
already throws away all blank lines within subshells since, when
checking for a broken &&-chain, it needs to check the final _statement_
in a subshell, not the final _line_ (which might be blank), thus it has
never made any attempt to precisely reproduce blank lines in its output.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/Makefile b/t/Makefile
index f4ae40be46a4788228e758304e7fe57007a66a50..46cd5fc5273dc0bcb907ab49ce8c97c4783053be 100644 (file)
--- a/t/Makefile
+++ b/t/Makefile
@@ -72,8 +72,8 @@ clean-chainlint:
 check-chainlint:
        @mkdir -p '$(CHAINLINTTMP_SQ)' && \
        sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \
-       cat $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
-       $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual && \
+       sed -e '/^[     ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
+       $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[   ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
        diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
 
 test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \

diff --git a/t/chainlint.sed b/t/chainlint.sed
index b1505ef2cd94d314a3f068ea93d74ce8e674d1eb..dc4ce37cb5188a63cf3a831a16d21363cfffd3f2 100644 (file)
--- a/t/chainlint.sed
+++ b/t/chainlint.sed
@@ -131,11 +131,15 @@ b
        h
        bnextln
 }
-# "(..." line -- split off and stash "(", then process "..." as its own line
+# "(..." line -- "(" opening subshell cuddled with command; temporarily replace
+# "(" with sentinel "^" and process the line as if "(" had been seen solo on
+# the preceding line; this temporary replacement prevents several rules from
+# accidentally thinking "(" introduces a nested subshell; "^" is changed back
+# to "(" at output time
 x
-s/.*/(/
+s/.*//
 x
-s/(//
+s/(/^/
 bslurp
 
 :nextln
@@ -168,12 +172,12 @@ s/.*\n//
        /"[^"]*#[^"]*"/!s/[     ]#.*$//
 }
 # one-liner "case ... esac"
-/^[    ]*case[         ]*..*esac/bchkchn
+/^[    ^]*case[        ]*..*esac/bchkchn
 # multi-line "case ... esac"
-/^[    ]*case[         ]..*[   ]in/bcase
+/^[    ^]*case[        ]..*[   ]in/bcase
 # multi-line "for ... done" or "while ... done"
-/^[    ]*for[  ]..*[   ]in/bcont
-/^[    ]*while[        ]/bcont
+/^[    ^]*for[         ]..*[   ]in/bcont
+/^[    ^]*while[       ]/bcont
 /^[    ]*do[   ]/bcont
 /^[    ]*do[   ]*$/bcont
 /;[    ]*do/bcont
@@ -184,7 +188,7 @@ s/.*\n//
 /||[   ]*exit[         ]/bcont
 /||[   ]*exit[         ]*$/bcont
 # multi-line "if...elsif...else...fi"
-/^[    ]*if[   ]/bcont
+/^[    ^]*if[  ]/bcont
 /^[    ]*then[         ]/bcont
 /^[    ]*then[         ]*$/bcont
 /;[    ]*then/bcont
@@ -197,15 +201,15 @@ s/.*\n//
 /^[    ]*fi[   ]*[<>|]/bdone
 /^[    ]*fi[   ]*)/bdone
 # nested one-liner "(...) &&"
-/^[    ]*(.*)[         ]*&&[   ]*$/bchkchn
+/^[    ^]*(.*)[        ]*&&[   ]*$/bchkchn
 # nested one-liner "(...)"
-/^[    ]*(.*)[         ]*$/bchkchn
+/^[    ^]*(.*)[        ]*$/bchkchn
 # nested one-liner "(...) >x" (or "2>x" or "<x" or "|x")
-/^[    ]*(.*)[         ]*[0-9]*[<>|]/bchkchn
+/^[    ^]*(.*)[        ]*[0-9]*[<>|]/bchkchn
 # nested multi-line "(...\n...)"
-/^[    ]*(/bnest
+/^[    ^]*(/bnest
 # multi-line "{...\n...}"
-/^[    ]*{/bblock
+/^[    ^]*{/bblock
 # closing ")" on own line -- exit subshell
 /^[    ]*)/bclssolo
 # "$((...))" -- arithmetic expansion; not closing ")"
@@ -237,6 +241,7 @@ s/.*\n//
 :cont
 # retrieve and print previous line
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 n
 bslurp
@@ -292,6 +297,7 @@ bfolded
 # found "case ... in" -- pass through untouched
 :case
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 n
 :cascom
@@ -326,6 +332,7 @@ bchkchn
 :nest
 x
 :nstslrp
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 n
 :nstcom
@@ -354,6 +361,7 @@ bchkchn
 # found multi-line "{...\n...}" block -- pass through untouched
 :block
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 n
 :blkcom
@@ -371,17 +379,21 @@ bblock
 :clssolo
 x
 s/\( ?!AMP?!\)* ?!AMP?!$//
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 p
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 b
 
 # found closing "...)" -- exit subshell loop
 :close
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 p
 x
+s/^\([         ]*\)^/\1(/
 s/?!HERE?!/<</g
 b

diff --git a/t/chainlint/close-nested-and-parent-together.expect b/t/chainlint/close-nested-and-parent-together.expect
index 5ef509eb4987593e56027e8f8bb0171d75ab23d8..72d482f76dd20f0d2ca7bbffd177417a257992e2 100644 (file)
--- a/t/chainlint/close-nested-and-parent-together.expect
+++ b/t/chainlint/close-nested-and-parent-together.expect
@@ -1,4 +1,3 @@
-(
-cd foo &&
+(cd foo &&
        (bar &&
                baz))

diff --git a/t/chainlint/complex-if-in-cuddled-loop.expect b/t/chainlint/complex-if-in-cuddled-loop.expect
index b8aa626ed0e7f2dc63cfa5d9450ac986b7d74a67..2fca1834095817a0aba1bce38a5a837dea0debae 100644 (file)
--- a/t/chainlint/complex-if-in-cuddled-loop.expect
+++ b/t/chainlint/complex-if-in-cuddled-loop.expect
@@ -1,5 +1,4 @@
-(
-for i in a b c; do
+(for i in a b c; do
    if test "$(echo $(waffle bat))" = "eleventeen" &&
      test "$x" = "$y"; then
      :

diff --git a/t/chainlint/cuddled-if-then-else.expect b/t/chainlint/cuddled-if-then-else.expect
index 4e089b087a867566054120b67e663c73cc58f6aa..1d8ed58c4948ea2b975de2742ccad603f48042dc 100644 (file)
--- a/t/chainlint/cuddled-if-then-else.expect
+++ b/t/chainlint/cuddled-if-then-else.expect
@@ -1,5 +1,4 @@
-(
-if test -z ""; then
+(if test -z ""; then
     echo empty
  else
     echo bizzy

diff --git a/t/chainlint/cuddled-loop.expect b/t/chainlint/cuddled-loop.expect
index 7932303763285828a78fb11465946bee8a477508..9cf260708e6a5c07282f37a03560781db8504ec0 100644 (file)
--- a/t/chainlint/cuddled-loop.expect
+++ b/t/chainlint/cuddled-loop.expect
@@ -1,5 +1,4 @@
-(
- while read x
+( while read x
   do foobar bop || exit 1
   done <file ) &&
 outside subshell

diff --git a/t/chainlint/cuddled.expect b/t/chainlint/cuddled.expect
index 773476adc85138d9bdbf584bcb0d0a12573b033e..c3e0be404742cb62808ffa43026573dbe5cb9cd1 100644 (file)
--- a/t/chainlint/cuddled.expect
+++ b/t/chainlint/cuddled.expect
@@ -1,10 +1,8 @@
-(
-cd foo &&
+(cd foo &&
        bar
 ) &&
 
-(
-cd foo ?!AMP?!
+(cd foo ?!AMP?!
        bar
 ) &&
 
@@ -12,10 +10,8 @@ cd foo ?!AMP?!
        cd foo &&
        bar) &&
 
-(
-cd foo &&
+(cd foo &&
        bar) &&
 
-(
-cd foo ?!AMP?!
+(cd foo ?!AMP?!
        bar)

diff --git a/t/chainlint/inline-comment.expect b/t/chainlint/inline-comment.expect
index f6b42757d235d32420b57e8380d590b029dde89b..dd0dace077f0e093ccda9dc33b3296830e13c8d2 100644 (file)
--- a/t/chainlint/inline-comment.expect
+++ b/t/chainlint/inline-comment.expect
@@ -4,6 +4,5 @@
        flibble "not a # comment"
 ) &&
 
-(
-cd foo &&
+(cd foo &&
        flibble "not a # comment")

diff --git a/t/chainlint/semicolon.expect b/t/chainlint/semicolon.expect
index 05141a96cf3876cf5c6c7689c30cf9387e3d3d3b..ed0b3707ae90139de0aec8e4ccd8d08301cff1dc 100644 (file)
--- a/t/chainlint/semicolon.expect
+++ b/t/chainlint/semicolon.expect
@@ -13,8 +13,7 @@
 (
        foo;
 ) &&
-(
-cd foo &&
+(cd foo &&
        for i in a b c; do
                echo;
        done)