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 <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine committed
Jul 11, 2018 at 02:46 UTC
878f9883507492fe1734561ea8ddc3c4e9de3c1d
2 files changed
+348
-1
t/chainlint.sed
new
+346
@@ -0,0 +1,346 @@
1
+#------------------------------------------------------------------------------
2
+# Detect broken &&-chains in tests.
3
+#
4
+# At present, only &&-chains in subshells are examined by this linter;
5
+# top-level &&-chains are instead checked directly by the test framework. Like
6
+# the top-level &&-chain linter, the subshell linter (intentionally) does not
7
+# check &&-chains within {...} blocks.
8
+#
9
+# Checking for &&-chain breakage is done line-by-line by pure textual
10
+# inspection.
11
+#
12
+# Incomplete lines (those ending with "\") are stitched together with following
13
+# lines to simplify processing, particularly of "one-liner" statements.
14
+# Top-level here-docs are swallowed to avoid false positives within the
15
+# here-doc body, although the statement to which the here-doc is attached is
16
+# retained.
17
+#
18
+# Heuristics are used to detect end-of-subshell when the closing ")" is cuddled
19
+# with the final subshell statement on the same line:
20
+#
21
+# (cd foo &&
22
+# bar)
23
+#
24
+# in order to avoid misinterpreting the ")" in constructs such as "x=$(...)"
25
+# and "case $x in *)" as ending the subshell.
26
+#
27
+# Lines missing a final "&&" are flagged with "?!AMP?!", and lines which chain
28
+# commands with ";" internally rather than "&&" are flagged "?!SEMI?!". A line
29
+# may be flagged for both violations.
30
+#
31
+# Detection of a missing &&-link in a multi-line subshell is complicated by the
32
+# fact that the last statement before the closing ")" must not end with "&&".
33
+# Since processing is line-by-line, it is not known whether a missing "&&" is
34
+# legitimate or not until the _next_ line is seen. To accommodate this, within
35
+# multi-line subshells, each line is stored in sed's "hold" area until after
36
+# the next line is seen and processed. If the next line is a stand-alone ")",
37
+# then a missing "&&" on the previous line is legitimate; otherwise a missing
38
+# "&&" is a break in the &&-chain.
39
+#
40
+# (
41
+# cd foo &&
42
+# bar
43
+# )
44
+#
45
+# In practical terms, when "bar" is encountered, it is flagged with "?!AMP?!",
46
+# but when the stand-alone ")" line is seen which closes the subshell, the
47
+# "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold"
48
+# area) since the final statement of a subshell must not end with "&&". The
49
+# final line of a subshell may still break the &&-chain by using ";" internally
50
+# to chain commands together rather than "&&", so "?!SEMI?!" is never removed
51
+# from a line (even though "?!AMP?!" might be).
52
+#
53
+# Care is taken to recognize the last _statement_ of a multi-line subshell, not
54
+# necessarily the last textual _line_ within the subshell, since &&-chaining
55
+# applies to statements, not to lines. Consequently, blank lines, comment
56
+# lines, and here-docs are swallowed (but not the command to which the here-doc
57
+# is attached), leaving the last statement in the "hold" area, not the last
58
+# line, thus simplifying &&-link checking.
59
+#
60
+# The final statement before "done" in for- and while-loops, and before "elif",
61
+# "else", and "fi" in if-then-else likewise must not end with "&&", thus
62
+# receives similar treatment.
63
+#
64
+# To facilitate regression testing (and manual debugging), a ">" annotation is
65
+# applied to the line containing ")" which closes a subshell, ">>" to a line
66
+# closing a nested subshell, and ">>>" to a line closing both at once. This
67
+# makes it easy to detect whether the heuristics correctly identify
68
+# end-of-subshell.
69
+#------------------------------------------------------------------------------
70
+
71
+# incomplete line -- slurp up next line
72
+:squash
73
+/\\$/ {
74
+ N
75
+ s/\\\n//
76
+ bsquash
77
+}
78
+
79
+# here-doc -- swallow it to avoid false hits within its body (but keep the
80
+# command to which it was attached)
81
+/<<[ ]*[-\\]*EOF[ ]*/ {
82
+ s/[ ]*<<[ ]*[-\\]*EOF//
83
+ h
84
+ :hereslurp
85
+ N
86
+ s/.*\n//
87
+ /^[ ]*EOF[ ]*$/!bhereslurp
88
+ x
89
+}
90
+
91
+# one-liner "(...) &&"
92
+/^[ ]*!*[ ]*(..*)[ ]*&&[ ]*$/boneline
93
+
94
+# same as above but without trailing "&&"
95
+/^[ ]*!*[ ]*(..*)[ ]*$/boneline
96
+
97
+# one-liner "(...) >x" (or "2>x" or "<x" or "|x" or "&"
98
+/^[ ]*!*[ ]*(..*)[ ]*[0-9]*[<>|&]/boneline
99
+
100
+# multi-line "(...\n...)"
101
+/^[ ]*(/bsubshell
102
+
103
+# innocuous line -- print it and advance to next line
104
+b
105
+
106
+# found one-liner "(...)" -- mark suspect if it uses ";" internally rather than
107
+# "&&" (but not ";" in a string)
108
+:oneline
109
+/;/{
110
+ /"[^"]*;[^"]*"/!s/^/?!SEMI?!/
111
+}
112
+b
113
+
114
+:subshell
115
+# bare "(" line?
116
+/^[ ]*([ ]*$/ {
117
+ # stash for later printing
118
+ h
119
+ bnextline
120
+}
121
+# "(..." line -- split off and stash "(", then process "..." as its own line
122
+x
123
+s/.*/(/
124
+x
125
+s/(//
126
+bslurp
127
+
128
+:nextline
129
+N
130
+s/.*\n//
131
+
132
+:slurp
133
+# incomplete line "...\"
134
+/\\$/bincomplete
135
+# multi-line quoted string "...\n..."
136
+/^[^"]*"[^"]*$/bdqstring
137
+# multi-line quoted string '...\n...' (but not contraction in string "it's so")
138
+/^[^']*'[^']*$/{
139
+ /"[^'"]*'[^'"]*"/!bsqstring
140
+}
141
+# here-doc -- swallow it
142
+/<<[ ]*[-\\]*EOF/bheredoc
143
+/<<[ ]*[-\\]*EOT/bheredoc
144
+/<<[ ]*[-\\]*INPUT_END/bheredoc
145
+# comment or empty line -- discard since final non-comment, non-empty line
146
+# before closing ")", "done", "elsif", "else", or "fi" will need to be
147
+# re-visited to drop "suspect" marking since final line of those constructs
148
+# legitimately lacks "&&", so "suspect" mark must be removed
149
+/^[ ]*#/bnextline
150
+/^[ ]*$/bnextline
151
+# in-line comment -- strip it (but not "#" in a string, Bash ${#...} array
152
+# length, or Perforce "//depot/path#42" revision in filespec)
153
+/[ ]#/{
154
+ /"[^"]*#[^"]*"/!s/[ ]#.*$//
155
+}
156
+# one-liner "case ... esac"
157
+/^[ ]*case[ ]*..*esac/bcheckchain
158
+# multi-line "case ... esac"
159
+/^[ ]*case[ ]..*[ ]in/bcase
160
+# multi-line "for ... done" or "while ... done"
161
+/^[ ]*for[ ]..*[ ]in/bcontinue
162
+/^[ ]*while[ ]/bcontinue
163
+/^[ ]*do[ ]/bcontinue
164
+/^[ ]*do[ ]*$/bcontinue
165
+/;[ ]*do/bcontinue
166
+/^[ ]*done[ ]*&&[ ]*$/bdone
167
+/^[ ]*done[ ]*$/bdone
168
+/^[ ]*done[ ]*[<>|]/bdone
169
+/^[ ]*done[ ]*)/bdone
170
+/||[ ]*exit[ ]/bcontinue
171
+/||[ ]*exit[ ]*$/bcontinue
172
+# multi-line "if...elsif...else...fi"
173
+/^[ ]*if[ ]/bcontinue
174
+/^[ ]*then[ ]/bcontinue
175
+/^[ ]*then[ ]*$/bcontinue
176
+/;[ ]*then/bcontinue
177
+/^[ ]*elif[ ]/belse
178
+/^[ ]*elif[ ]*$/belse
179
+/^[ ]*else[ ]/belse
180
+/^[ ]*else[ ]*$/belse
181
+/^[ ]*fi[ ]*&&[ ]*$/bdone
182
+/^[ ]*fi[ ]*$/bdone
183
+/^[ ]*fi[ ]*[<>|]/bdone
184
+/^[ ]*fi[ ]*)/bdone
185
+# nested one-liner "(...) &&"
186
+/^[ ]*(.*)[ ]*&&[ ]*$/bcheckchain
187
+# nested one-liner "(...)"
188
+/^[ ]*(.*)[ ]*$/bcheckchain
189
+# nested one-liner "(...) >x" (or "2>x" or "<x" or "|x")
190
+/^[ ]*(.*)[ ]*[0-9]*[<>|]/bcheckchain
191
+# nested multi-line "(...\n...)"
192
+/^[ ]*(/bnest
193
+# multi-line "{...\n...}"
194
+/^[ ]*{/bblock
195
+# closing ")" on own line -- exit subshell
196
+/^[ ]*)/bclosesolo
197
+# "$((...))" -- arithmetic expansion; not closing ")"
198
+/\$(([^)][^)]*))[^)]*$/bcheckchain
199
+# "$(...)" -- command substitution; not closing ")"
200
+/\$([^)][^)]*)[^)]*$/bcheckchain
201
+# multi-line "$(...\n...)" -- command substitution; treat as nested subshell
202
+/\$([ ]*$/bnest
203
+# "=(...)" -- Bash array assignment; not closing ")"
204
+/=(/bcheckchain
205
+# closing "...) &&"
206
+/)[ ]*&&[ ]*$/bclose
207
+# closing "...)"
208
+/)[ ]*$/bclose
209
+# closing "...) >x" (or "2>x" or "<x" or "|x")
210
+/)[ ]*[<>|]/bclose
211
+:checkchain
212
+# mark suspect if line uses ";" internally rather than "&&" (but not ";" in a
213
+# string and not ";;" in one-liner "case...esac")
214
+/;/{
215
+ /;;/!{
216
+ /"[^"]*;[^"]*"/!s/^/?!SEMI?!/
217
+ }
218
+}
219
+# line ends with pipe "...|" -- valid; not missing "&&"
220
+/|[ ]*$/bcontinue
221
+# missing end-of-line "&&" -- mark suspect
222
+/&&[ ]*$/!s/^/?!AMP?!/
223
+:continue
224
+# retrieve and print previous line
225
+x
226
+n
227
+bslurp
228
+
229
+# found incomplete line "...\" -- slurp up next line
230
+:incomplete
231
+N
232
+s/\\\n//
233
+bslurp
234
+
235
+# found multi-line double-quoted string "...\n..." -- slurp until end of string
236
+:dqstring
237
+s/"//g
238
+N
239
+s/\n//
240
+/"/!bdqstring
241
+bcheckchain
242
+
243
+# found multi-line single-quoted string '...\n...' -- slurp until end of string
244
+:sqstring
245
+s/'//g
246
+N
247
+s/\n//
248
+/'/!bsqstring
249
+bcheckchain
250
+
251
+# found here-doc -- swallow it to avoid false hits within its body (but keep
252
+# the command to which it was attached); take care to handle here-docs nested
253
+# within here-docs by only recognizing closing tag matching outer here-doc
254
+# opening tag
255
+:heredoc
256
+/EOF/{ s/[ ]*<<[ ]*[-\\]*EOF//; s/^/EOF/; }
257
+/EOT/{ s/[ ]*<<[ ]*[-\\]*EOT//; s/^/EOT/; }
258
+/INPUT_END/{ s/[ ]*<<[ ]*[-\\]*INPUT_END//; s/^/INPUT_END/; }
259
+:hereslurpsub
260
+N
261
+/^EOF.*\n[ ]*EOF[ ]*$/bhereclose
262
+/^EOT.*\n[ ]*EOT[ ]*$/bhereclose
263
+/^INPUT_END.*\n[ ]*INPUT_END[ ]*$/bhereclose
264
+bhereslurpsub
265
+:hereclose
266
+s/^EOF//
267
+s/^EOT//
268
+s/^INPUT_END//
269
+s/\n.*$//
270
+bcheckchain
271
+
272
+# found "case ... in" -- pass through untouched
273
+:case
274
+x
275
+n
276
+/^[ ]*esac/bslurp
277
+bcase
278
+
279
+# found "else" or "elif" -- drop "suspect" from final line before "else" since
280
+# that line legitimately lacks "&&"
281
+:else
282
+x
283
+s/?!AMP?!//
284
+x
285
+bcontinue
286
+
287
+# found "done" closing for-loop or while-loop, or "fi" closing if-then -- drop
288
+# "suspect" from final contained line since that line legitimately lacks "&&"
289
+:done
290
+x
291
+s/?!AMP?!//
292
+x
293
+# is 'done' or 'fi' cuddled with ")" to close subshell?
294
+/done.*)/bclose
295
+/fi.*)/bclose
296
+bcheckchain
297
+
298
+# found nested multi-line "(...\n...)" -- pass through untouched
299
+:nest
300
+x
301
+:nestslurp
302
+n
303
+# closing ")" on own line -- stop nested slurp
304
+/^[ ]*)/bnestclose
305
+# comment -- not closing ")" if in comment
306
+/^[ ]*#/bnestcontinue
307
+# "$((...))" -- arithmetic expansion; not closing ")"
308
+/\$(([^)][^)]*))[^)]*$/bnestcontinue
309
+# "$(...)" -- command substitution; not closing ")"
310
+/\$([^)][^)]*)[^)]*$/bnestcontinue
311
+# closing "...)" -- stop nested slurp
312
+/)/bnestclose
313
+:nestcontinue
314
+x
315
+bnestslurp
316
+:nestclose
317
+s/^/>>/
318
+# is it "))" which closes nested and parent subshells?
319
+/)[ ]*)/bslurp
320
+bcheckchain
321
+
322
+# found multi-line "{...\n...}" block -- pass through untouched
323
+:block
324
+x
325
+n
326
+# closing "}" -- stop block slurp
327
+/}/bcheckchain
328
+bblock
329
+
330
+# found closing ")" on own line -- drop "suspect" from final line of subshell
331
+# since that line legitimately lacks "&&" and exit subshell loop
332
+:closesolo
333
+x
334
+s/?!AMP?!//
335
+p
336
+x
337
+s/^/>/
338
+b
339
+
340
+# found closing "...)" -- exit subshell loop
341
+:close
342
+x
343
+p
344
+x
345
+s/^/>/
346
+b
t/test-lib.sh
+2
-1
@@ -675,7 +675,8 @@ test_run_ () {
675
trace=
676
# 117 is magic because it is unlikely to match the exit
677
# code of other programs
678
- if test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)"
678
+ if $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') ||
679
+ test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)"
680
then
681
error "bug in the test script: broken &&-chain or run-away HERE-DOC: $1"
682
fi