Skip to content

Commit c2c29cc

Browse files
sunshinecogitster
authored andcommitted
chainlint: match arbitrary here-docs tags rather than hard-coded names
chainlint.sed swallows top-level here-docs to avoid being fooled by content which might look like start-of-subshell. It likewise swallows here-docs in subshells to avoid marking content lines as breaking the &&-chain, and to avoid being fooled by content which might look like end-of-subshell, start-of-nested-subshell, or other specially-recognized constructs. At the time of implementation, it was believed that it was not possible to support arbitrary here-doc tag names since 'sed' provides no way to stash the opening tag name in a variable for later comparison against a line signaling end-of-here-doc. Consequently, tag names are hard-coded, with "EOF" being the only tag recognized at the top-level, and only "EOF", "EOT", and "INPUT_END" being recognized within subshells. Also, special care was taken to avoid being confused by here-docs nested within other here-docs. In practice, this limited number of hard-coded tag names has been "good enough" for the 13000+ existing Git test, despite many of those tests using tags other than the recognized ones, since the bodies of those here-docs do not contain content which would fool the linter. Nevertheless, the situation is not ideal since someone writing new tests, and choosing a name not in the "blessed" set could potentially trigger a false-positive. To address this shortcoming, upgrade chainlint.sed to handle arbitrary here-doc tag names, both at the top-level and within subshells. Signed-off-by: Eric Sunshine <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ace64e5 commit c2c29cc

File tree

7 files changed

+67
-23
lines changed

7 files changed

+67
-23
lines changed

t/chainlint.sed

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,22 @@
6161
# "else", and "fi" in if-then-else likewise must not end with "&&", thus
6262
# receives similar treatment.
6363
#
64+
# Swallowing here-docs with arbitrary tags requires a bit of finesse. When a
65+
# line such as "cat <<EOF >out" is seen, the here-doc tag is moved to the front
66+
# of the line enclosed in angle brackets as a sentinel, giving "<EOF>cat >out".
67+
# As each subsequent line is read, it is appended to the target line and a
68+
# (whitespace-loose) back-reference match /^<(.*)>\n\1$/ is attempted to see if
69+
# the content inside "<...>" matches the entirety of the newly-read line. For
70+
# instance, if the next line read is "some data", when concatenated with the
71+
# target line, it becomes "<EOF>cat >out\nsome data", and a match is attempted
72+
# to see if "EOF" matches "some data". Since it doesn't, the next line is
73+
# attempted. When a line consisting of only "EOF" (and possible whitespace) is
74+
# encountered, it is appended to the target line giving "<EOF>cat >out\nEOF",
75+
# in which case the "EOF" inside "<...>" does match the text following the
76+
# newline, thus the closing here-doc tag has been found. The closing tag line
77+
# and the "<...>" prefix on the target line are then discarded, leaving just
78+
# the target line "cat >out".
79+
#
6480
# To facilitate regression testing (and manual debugging), a ">" annotation is
6581
# applied to the line containing ")" which closes a subshell, ">>" to a line
6682
# closing a nested subshell, and ">>>" to a line closing both at once. This
@@ -78,14 +94,17 @@
7894

7995
# here-doc -- swallow it to avoid false hits within its body (but keep the
8096
# command to which it was attached)
81-
/<<[ ]*[-\\]*EOF[ ]*/ {
82-
s/[ ]*<<[ ]*[-\\]*EOF//
83-
h
97+
/<<[ ]*[-\\]*[A-Za-z0-9_]/ {
98+
s/^\(.*\)<<[ ]*[-\\]*\([A-Za-z0-9_][A-Za-z0-9_]*\)/<\2>\1<</
99+
s/[ ]*<<//
84100
:hereslurp
85101
N
86-
s/.*\n//
87-
/^[ ]*EOF[ ]*$/!bhereslurp
88-
x
102+
/^<\([^>]*\)>.*\n[ ]*\1[ ]*$/!{
103+
s/\n.*$//
104+
bhereslurp
105+
}
106+
s/^<[^>]*>//
107+
s/\n.*$//
89108
}
90109

91110
# one-liner "(...) &&"
@@ -139,9 +158,7 @@ s/.*\n//
139158
/"[^'"]*'[^'"]*"/!bsqstring
140159
}
141160
# here-doc -- swallow it
142-
/<<[ ]*[-\\]*EOF/bheredoc
143-
/<<[ ]*[-\\]*EOT/bheredoc
144-
/<<[ ]*[-\\]*INPUT_END/bheredoc
161+
/<<[ ]*[-\\]*[A-Za-z0-9_]/bheredoc
145162
# comment or empty line -- discard since final non-comment, non-empty line
146163
# before closing ")", "done", "elsif", "else", or "fi" will need to be
147164
# re-visited to drop "suspect" marking since final line of those constructs
@@ -249,23 +266,17 @@ s/\n//
249266
bcheckchain
250267

251268
# 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
269+
# the command to which it was attached)
255270
:heredoc
256-
/EOF/{ s/[ ]*<<[ ]*[-\\]*EOF//; s/^/EOF/; }
257-
/EOT/{ s/[ ]*<<[ ]*[-\\]*EOT//; s/^/EOT/; }
258-
/INPUT_END/{ s/[ ]*<<[ ]*[-\\]*INPUT_END//; s/^/INPUT_END/; }
271+
s/^\(.*\)<<[ ]*[-\\]*\([A-Za-z0-9_][A-Za-z0-9_]*\)/<\2>\1<</
272+
s/[ ]*<<//
259273
:hereslurpsub
260274
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//
275+
/^<\([^>]*\)>.*\n[ ]*\1[ ]*$/!{
276+
s/\n.*$//
277+
bhereslurpsub
278+
}
279+
s/^<[^>]*>//
269280
s/\n.*$//
270281
bcheckchain
271282

t/chainlint/here-doc.expect

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
boodle wobba gorgo snoot wafta snurb &&
22

3+
cat >foo &&
4+
35
horticulture

t/chainlint/here-doc.test

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ quoth the raven,
77
nevermore...
88
EOF
99

10+
# LINT: swallow here-doc with arbitrary tag
11+
cat <<-Arbitrary_Tag_42 >foo &&
12+
snoz
13+
boz
14+
woz
15+
Arbitrary_Tag_42
16+
1017
# LINT: swallow here-doc (EOF is last line of test)
1118
horticulture <<\EOF
1219
gomez

t/chainlint/nested-here-doc.expect

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
cat >foop &&
2+
13
(
24
cat &&
35
?!AMP?! cat

t/chainlint/nested-here-doc.test

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
# LINT: inner "EOF" not misintrepreted as closing ARBITRARY here-doc
2+
cat <<ARBITRARY >foop &&
3+
naddle
4+
fub <<EOF
5+
nozzle
6+
noodle
7+
EOF
8+
formp
9+
ARBITRARY
10+
111
(
212
# LINT: inner "EOF" not misintrepreted as closing INPUT_END here-doc
313
cat <<-\INPUT_END &&

t/chainlint/subshell-here-doc.expect

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,8 @@
22
echo wobba gorgo snoot wafta snurb &&
33
?!AMP?! cat >bip
44
echo >bop
5+
>) &&
6+
(
7+
cat >bup &&
8+
meep
59
>)

t/chainlint/subshell-here-doc.test

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,12 @@
2020
wednesday
2121
pugsly
2222
EOF
23+
) &&
24+
(
25+
# LINT: swallow here-doc with arbitrary tag
26+
cat <<-\ARBITRARY >bup &&
27+
glink
28+
FIZZ
29+
ARBITRARY
30+
meep
2331
)

0 commit comments

Comments
 (0)