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>

Eric Sunshine committed Dec 13, 2021 at 01:30 UTC d73f5cfa89ae9fcc76b1b1528f4daf944ce2a931
9 files changed +37 -35
t/Makefile
+2 -2
@@ -72,8 +72,8 @@ clean-chainlint:
72 check-chainlint:
73 @mkdir -p '$(CHAINLINTTMP_SQ)' && \
74 sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \
75 - cat $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
76 - $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual && \
75 + sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
76 + $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
77 diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
78
79 test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
t/chainlint.sed
+25 -13
@@ -131,11 +131,15 @@ b
131 h
132 bnextln
133 }
134 -# "(..." line -- split off and stash "(", then process "..." as its own line
134 +# "(..." line -- "(" opening subshell cuddled with command; temporarily replace
135 +# "(" with sentinel "^" and process the line as if "(" had been seen solo on
136 +# the preceding line; this temporary replacement prevents several rules from
137 +# accidentally thinking "(" introduces a nested subshell; "^" is changed back
138 +# to "(" at output time
139 x
136 -s/.*/(/
140 +s/.*//
141 x
138 -s/(//
142 +s/(/^/
143 bslurp
144
145 :nextln
@@ -168,12 +172,12 @@ s/.*\n//
172 /"[^"]*#[^"]*"/!s/[ ]#.*$//
173 }
174 # one-liner "case ... esac"
171 -/^[ ]*case[ ]*..*esac/bchkchn
175 +/^[ ^]*case[ ]*..*esac/bchkchn
176 # multi-line "case ... esac"
173 -/^[ ]*case[ ]..*[ ]in/bcase
177 +/^[ ^]*case[ ]..*[ ]in/bcase
178 # multi-line "for ... done" or "while ... done"
175 -/^[ ]*for[ ]..*[ ]in/bcont
176 -/^[ ]*while[ ]/bcont
179 +/^[ ^]*for[ ]..*[ ]in/bcont
180 +/^[ ^]*while[ ]/bcont
181 /^[ ]*do[ ]/bcont
182 /^[ ]*do[ ]*$/bcont
183 /;[ ]*do/bcont
@@ -184,7 +188,7 @@ s/.*\n//
188 /||[ ]*exit[ ]/bcont
189 /||[ ]*exit[ ]*$/bcont
190 # multi-line "if...elsif...else...fi"
187 -/^[ ]*if[ ]/bcont
191 +/^[ ^]*if[ ]/bcont
192 /^[ ]*then[ ]/bcont
193 /^[ ]*then[ ]*$/bcont
194 /;[ ]*then/bcont
@@ -197,15 +201,15 @@ s/.*\n//
201 /^[ ]*fi[ ]*[<>|]/bdone
202 /^[ ]*fi[ ]*)/bdone
203 # nested one-liner "(...) &&"
200 -/^[ ]*(.*)[ ]*&&[ ]*$/bchkchn
204 +/^[ ^]*(.*)[ ]*&&[ ]*$/bchkchn
205 # nested one-liner "(...)"
202 -/^[ ]*(.*)[ ]*$/bchkchn
206 +/^[ ^]*(.*)[ ]*$/bchkchn
207 # nested one-liner "(...) >x" (or "2>x" or "<x" or "|x")
204 -/^[ ]*(.*)[ ]*[0-9]*[<>|]/bchkchn
208 +/^[ ^]*(.*)[ ]*[0-9]*[<>|]/bchkchn
209 # nested multi-line "(...\n...)"
206 -/^[ ]*(/bnest
210 +/^[ ^]*(/bnest
211 # multi-line "{...\n...}"
208 -/^[ ]*{/bblock
212 +/^[ ^]*{/bblock
213 # closing ")" on own line -- exit subshell
214 /^[ ]*)/bclssolo
215 # "$((...))" -- arithmetic expansion; not closing ")"
@@ -237,6 +241,7 @@ s/.*\n//
241 :cont
242 # retrieve and print previous line
243 x
244 +s/^\([ ]*\)^/\1(/
245 s/?!HERE?!/<</g
246 n
247 bslurp
@@ -292,6 +297,7 @@ bfolded
297 # found "case ... in" -- pass through untouched
298 :case
299 x
300 +s/^\([ ]*\)^/\1(/
301 s/?!HERE?!/<</g
302 n
303 :cascom
@@ -326,6 +332,7 @@ bchkchn
332 :nest
333 x
334 :nstslrp
335 +s/^\([ ]*\)^/\1(/
336 s/?!HERE?!/<</g
337 n
338 :nstcom
@@ -354,6 +361,7 @@ bchkchn
361 # found multi-line "{...\n...}" block -- pass through untouched
362 :block
363 x
364 +s/^\([ ]*\)^/\1(/
365 s/?!HERE?!/<</g
366 n
367 :blkcom
@@ -371,17 +379,21 @@ bblock
379 :clssolo
380 x
381 s/\( ?!AMP?!\)* ?!AMP?!$//
382 +s/^\([ ]*\)^/\1(/
383 s/?!HERE?!/<</g
384 p
385 x
386 +s/^\([ ]*\)^/\1(/
387 s/?!HERE?!/<</g
388 b
389
390 # found closing "...)" -- exit subshell loop
391 :close
392 x
393 +s/^\([ ]*\)^/\1(/
394 s/?!HERE?!/<</g
395 p
396 x
397 +s/^\([ ]*\)^/\1(/
398 s/?!HERE?!/<</g
399 b
t/chainlint/close-nested-and-parent-together.expect
+1 -2
@@ -1,4 +1,3 @@
1 -(
2 -cd foo &&
1 +(cd foo &&
2 (bar &&
3 baz))
t/chainlint/complex-if-in-cuddled-loop.expect
+1 -2
@@ -1,5 +1,4 @@
1 -(
2 -for i in a b c; do
1 +(for i in a b c; do
2 if test "$(echo $(waffle bat))" = "eleventeen" &&
3 test "$x" = "$y"; then
4 :
t/chainlint/cuddled-if-then-else.expect
+1 -2
@@ -1,5 +1,4 @@
1 -(
2 -if test -z ""; then
1 +(if test -z ""; then
2 echo empty
3 else
4 echo bizzy
t/chainlint/cuddled-loop.expect
+1 -2
@@ -1,5 +1,4 @@
1 -(
2 - while read x
1 +( while read x
2 do foobar bop || exit 1
3 done <file ) &&
4 outside subshell
t/chainlint/cuddled.expect
+4 -8
@@ -1,10 +1,8 @@
1 -(
2 -cd foo &&
1 +(cd foo &&
2 bar
3 ) &&
4
6 -(
7 -cd foo ?!AMP?!
5 +(cd foo ?!AMP?!
6 bar
7 ) &&
8
@@ -12,10 +10,8 @@ cd foo ?!AMP?!
10 cd foo &&
11 bar) &&
12
15 -(
16 -cd foo &&
13 +(cd foo &&
14 bar) &&
15
19 -(
20 -cd foo ?!AMP?!
16 +(cd foo ?!AMP?!
17 bar)
t/chainlint/inline-comment.expect
+1 -2
@@ -4,6 +4,5 @@
4 flibble "not a # comment"
5 ) &&
6
7 -(
8 -cd foo &&
7 +(cd foo &&
8 flibble "not a # comment")
t/chainlint/semicolon.expect
+1 -2
@@ -13,8 +13,7 @@
13 (
14 foo;
15 ) &&
16 -(
17 -cd foo &&
16 +(cd foo &&
17 for i in a b c; do
18 echo;
19 done)