[PATCH 09/15] chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?!
From: Eric Sunshine <hidden>
Date: 2021-12-13 06:32:15
Subsystem:
the rest · Maintainer:
Linus Torvalds
From inception, when chainlint.sed encountered a line using semicolon to
separate commands rather than `&&`, it would insert a ?!SEMI?!
annotation at the beginning of the line rather ?!AMP?! even though the
&&-chain is also broken by the semicolon. Given a line such as:
?!SEMI?! cmd1; cmd2 &&
the ?!SEMI?! annotation makes it easier to see what the problem is than
if the output had been:
?!AMP?! cmd1; cmd2 &&
which might confuse the test author into thinking that the linter is
broken (since the line clearly ends with `&&`).
However, now that the ?!AMP?! an ?!SEMI?! annotations are inserted at
the point of breakage rather than at the beginning of the line, and
taking into account that both represent a broken &&-chain, there is
little reason to distinguish between the two. Using ?!AMP?! alone is
sufficient to point the test author at the problem. For instance, in:
cmd1; ?!AMP?! cmd2 &&
cmd3
it is clear that the &&-chain is broken between `cmd1` and `cmd2`.
Likewise, in:
cmd1 && cmd2 ?!AMP?!
cmd3
it is clear that the &&-chain is broken between `cmd2` and `cmd3`.
Finally, in:
cmd1; ?!AMP?! cmd2 ?!AMP?!
cmd3
it is clear that the &&-chain is broken between each command.
Hence, there is no longer a good reason to make a distinction between a
broken &&-chain due to a semicolon and a broken chain due to a missing
`&&` at end-of-line. Therefore, drop the ?!SEMI?! annotation and use
?!AMP?! exclusively.
Signed-off-by: Eric Sunshine <redacted>
---
t/chainlint.sed | 21 ++++++++++-----------
t/chainlint/negated-one-liner.expect | 4 ++--
t/chainlint/one-liner.expect | 6 +++---
t/chainlint/semicolon.expect | 10 +++++-----
t/chainlint/subshell-one-liner.expect | 8 ++++----
5 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/t/chainlint.sed b/t/chainlint.sed
index 91077b6e26..f5fcca09ca 100644
--- a/t/chainlint.sed
+++ b/t/chainlint.sed@@ -24,9 +24,9 @@ # in order to avoid misinterpreting the ")" in constructs such as "x=$(...)" # and "case $x in *)" as ending the subshell. # -# Lines missing a final "&&" are flagged with "?!AMP?!", and lines which chain -# commands with ";" internally rather than "&&" are flagged "?!SEMI?!". A line -# may be flagged for both violations. +# Lines missing a final "&&" are flagged with "?!AMP?!", as are lines which +# chain commands with ";" internally rather than "&&". A line may be flagged +# for both violations. # # Detection of a missing &&-link in a multi-line subshell is complicated by the # fact that the last statement before the closing ")" must not end with "&&".
@@ -47,9 +47,8 @@ # "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold" # area) since the final statement of a subshell must not end with "&&". The # final line of a subshell may still break the &&-chain by using ";" internally -# to chain commands together rather than "&&", so "?!SEMI?!" is not removed -# from such a line; however, if the line ends with "?!SEMI?!", then the ";" is -# harmless and the annotation is removed. +# to chain commands together rather than "&&", but an internal "?!AMP?!" is +# never removed from a line even though a line-ending "?!AMP?!" might be. # # Care is taken to recognize the last _statement_ of a multi-line subshell, not # necessarily the last textual _line_ within the subshell, since &&-chaining
@@ -127,7 +126,7 @@ b # "&&" (but not ";" in a string) :oneline /;/{ - /"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/ + /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/ } b
@@ -231,7 +230,7 @@ s/.*\n// # string and not ";;" in one-liner "case...esac") /;/{ /;;/!{ - /"[^"]*;[^"]*"/!s/;/; ?!SEMI?!/ + /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/ } } # line ends with pipe "...|" -- valid; not missing "&&"
@@ -304,7 +303,7 @@ bcase # that line legitimately lacks "&&" :else x -s/\( ?!SEMI?!\)* ?!AMP?!$// +s/\( ?!AMP?!\)* ?!AMP?!$// x bcont
@@ -312,7 +311,7 @@ bcont # "suspect" from final contained line since that line legitimately lacks "&&" :done x -s/\( ?!SEMI?!\)* ?!AMP?!$// +s/\( ?!AMP?!\)* ?!AMP?!$// x # is 'done' or 'fi' cuddled with ")" to close subshell? /done.*)/bclose
@@ -355,7 +354,7 @@ bblock # since that line legitimately lacks "&&" and exit subshell loop :clssolo x -s/\( ?!SEMI?!\)* ?!AMP?!$// +s/\( ?!AMP?!\)* ?!AMP?!$// p x s/^/>/
diff --git a/t/chainlint/negated-one-liner.expect b/t/chainlint/negated-one-liner.expect
index 60baf84b7a..ad4c2d949e 100644
--- a/t/chainlint/negated-one-liner.expect
+++ b/t/chainlint/negated-one-liner.expect@@ -1,5 +1,5 @@ ! (foo && bar) && ! (foo && bar) >baz && -! (foo; ?!SEMI?! bar) && -! (foo; ?!SEMI?! bar) >baz +! (foo; ?!AMP?! bar) && +! (foo; ?!AMP?! bar) >baz
diff --git a/t/chainlint/one-liner.expect b/t/chainlint/one-liner.expect
index 3b46554728..57a7a444c1 100644
--- a/t/chainlint/one-liner.expect
+++ b/t/chainlint/one-liner.expect@@ -2,8 +2,8 @@ (foo && bar) | (foo && bar) >baz && -(foo; ?!SEMI?! bar) && -(foo; ?!SEMI?! bar) | -(foo; ?!SEMI?! bar) >baz && +(foo; ?!AMP?! bar) && +(foo; ?!AMP?! bar) | +(foo; ?!AMP?! bar) >baz && (foo "bar; baz")
diff --git a/t/chainlint/semicolon.expect b/t/chainlint/semicolon.expect
index 0e6389f532..54a08ce582 100644
--- a/t/chainlint/semicolon.expect
+++ b/t/chainlint/semicolon.expect@@ -1,14 +1,14 @@ ( - cat foo ; ?!SEMI?! echo bar ?!AMP?! - cat foo ; ?!SEMI?! echo bar + cat foo ; ?!AMP?! echo bar ?!AMP?! + cat foo ; ?!AMP?! echo bar >) && ( - cat foo ; ?!SEMI?! echo bar && - cat foo ; ?!SEMI?! echo bar + cat foo ; ?!AMP?! echo bar && + cat foo ; ?!AMP?! echo bar >) && ( echo "foo; bar" && - cat foo; ?!SEMI?! echo bar + cat foo; ?!AMP?! echo bar >) && ( foo;
diff --git a/t/chainlint/subshell-one-liner.expect b/t/chainlint/subshell-one-liner.expect
index 432217801b..4b44632b09 100644
--- a/t/chainlint/subshell-one-liner.expect
+++ b/t/chainlint/subshell-one-liner.expect@@ -2,13 +2,13 @@ (foo && bar) && (foo && bar) | (foo && bar) >baz && - (foo; ?!SEMI?! bar) && - (foo; ?!SEMI?! bar) | - (foo; ?!SEMI?! bar) >baz && + (foo; ?!AMP?! bar) && + (foo; ?!AMP?! bar) | + (foo; ?!AMP?! bar) >baz && (foo || exit 1) && (foo || exit 1) | (foo || exit 1) >baz && (foo && bar) ?!AMP?! - (foo && bar; ?!SEMI?! baz) ?!AMP?! + (foo && bar; ?!AMP?! baz) ?!AMP?! foobar >)
--
2.34.1.397.gfae76fe5da