Re: [PATCH v2 2/6] chainlint: match 'quoted' here-doc tags

2 messages, 2 authors, 2018-08-13 · open the first message on its own page

Re: [PATCH v2 2/6] chainlint: match 'quoted' here-doc tags

From: Junio C Hamano <hidden>
Date: 2018-08-13 19:27:28

Eric Sunshine [off-list ref] writes:
A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress
interpolation within the body. Although, chainlint recognizes escaped
tags, it does not know about quoted tags. For completeness, teach it to
recognize quoted tags, as well.
Is this step merely completeness and future-proofing, or do we have
existing instances that the linter would be confused without this
patch?  I am primarily wondering what the reason is that <<"EOF" is
not looked for while we are at it (the answer could be that we
have tests that use <<'EOF' but not <<"EOF").
quoted hunk
Signed-off-by: Eric Sunshine <redacted>
---
 t/chainlint.sed                      | 8 ++++----
 t/chainlint/here-doc.expect          | 2 ++
 t/chainlint/here-doc.test            | 7 +++++++
 t/chainlint/subshell-here-doc.expect | 1 +
 t/chainlint/subshell-here-doc.test   | 4 ++++
 5 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/t/chainlint.sed b/t/chainlint.sed
index 2af1a687f8..2901494e8a 100644
--- a/t/chainlint.sed
+++ b/t/chainlint.sed
@@ -94,8 +94,8 @@
 
 # here-doc -- swallow it to avoid false hits within its body (but keep the
 # command to which it was attached)
-/<<[ 	]*[-\\]*[A-Za-z0-9_]/ {
-	s/^\(.*\)<<[ 	]*[-\\]*\([A-Za-z0-9_][A-Za-z0-9_]*\)/<\2>\1<</
+/<<[ 	]*[-\\']*[A-Za-z0-9_]/ {
+	s/^\(.*\)<<[ 	]*[-\\']*\([A-Za-z0-9_][A-Za-z0-9_]*\)'*/<\2>\1<</
 	s/[ 	]*<<//
 	:hereslurp
 	N
@@ -158,7 +158,7 @@ s/.*\n//
 	/"[^'"]*'[^'"]*"/!bsqstring
 }
 # here-doc -- swallow it
-/<<[ 	]*[-\\]*[A-Za-z0-9_]/bheredoc
+/<<[ 	]*[-\\']*[A-Za-z0-9_]/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
@@ -268,7 +268,7 @@ bcheckchain
 # found here-doc -- swallow it to avoid false hits within its body (but keep
 # the command to which it was attached)
 :heredoc
-s/^\(.*\)<<[ 	]*[-\\]*\([A-Za-z0-9_][A-Za-z0-9_]*\)/<\2>\1<</
+s/^\(.*\)<<[ 	]*[-\\']*\([A-Za-z0-9_][A-Za-z0-9_]*\)'*/<\2>\1<</
 s/[ 	]*<<//
 :hereslurpsub
 N
diff --git a/t/chainlint/here-doc.expect b/t/chainlint/here-doc.expect
index 33bc3cc0b4..aff6568716 100644
--- a/t/chainlint/here-doc.expect
+++ b/t/chainlint/here-doc.expect
@@ -2,4 +2,6 @@ boodle wobba        gorgo snoot        wafta snurb &&
 
 cat >foo &&
 
+cat >bar &&
+
 horticulture
diff --git a/t/chainlint/here-doc.test b/t/chainlint/here-doc.test
index 8986eefe74..f2bb14b693 100644
--- a/t/chainlint/here-doc.test
+++ b/t/chainlint/here-doc.test
@@ -14,6 +14,13 @@ boz
 woz
 Arbitrary_Tag_42
 
+# LINT: swallow 'quoted' here-doc
+cat <<'FUMP' >bar &&
+snoz
+boz
+woz
+FUMP
+
 # LINT: swallow here-doc (EOF is last line of test)
 horticulture <<\EOF
 gomez
diff --git a/t/chainlint/subshell-here-doc.expect b/t/chainlint/subshell-here-doc.expect
index 7c2da63bc7..7663ea7fc4 100644
--- a/t/chainlint/subshell-here-doc.expect
+++ b/t/chainlint/subshell-here-doc.expect
@@ -5,5 +5,6 @@
 >) &&
 (
 	cat >bup &&
+	cat >bup2 &&
 	meep
 >)
diff --git a/t/chainlint/subshell-here-doc.test b/t/chainlint/subshell-here-doc.test
index 05139af0b5..b6b5a9b33a 100644
--- a/t/chainlint/subshell-here-doc.test
+++ b/t/chainlint/subshell-here-doc.test
@@ -27,5 +27,9 @@
 	glink
 	FIZZ
 	ARBITRARY
+	cat <<-'ARBITRARY2' >bup2 &&
+	glink
+	FIZZ
+	ARBITRARY2
 	meep
 )

Re: [PATCH v2 2/6] chainlint: match 'quoted' here-doc tags

From: Eric Sunshine <hidden>
Date: 2018-08-13 20:13:09

On Mon, Aug 13, 2018 at 3:27 PM Junio C Hamano [off-list ref] wrote:
Eric Sunshine [off-list ref] writes:
quoted
A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress
interpolation within the body. Although, chainlint recognizes escaped
tags, it does not know about quoted tags. For completeness, teach it to
recognize quoted tags, as well.
Is this step merely completeness and future-proofing, or do we have
existing instances that the linter would be confused without this
patch?
We do have quite a few instances of single-quoted 'EOF' in the tests
already, though none of them confuse the linter, so this change is for
completeness and future-proofing.
I am primarily wondering what the reason is that <<"EOF" is
not looked for while we are at it (the answer could be that we
have tests that use <<'EOF' but not <<"EOF").
It's an oversight on my part. I've gotten so used to \EOF (and before
working on Git, 'EOF'), that "EOF" just slipped my mind[1].

I do think it's worth a re-roll. Thanks for noticing.

[1]: What did not slip my mind is that quoting _any_ part of the tag
suppresses interpolation, so 'E'OF, E'O'F, and even E''OF are all
valid, however, that's so ugly that I really didn't want to support
it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help