chainlint.pl: don't flag broken &&-chain if failure indicated explicitly
There are quite a few tests which print an error messages and then explicitly signal failure with `false`, `return 1`, or `exit 1` as the final command in an `if` branch. In these cases, the tests don't bother maintaining the &&-chain between `echo` and the explicit "test failed" indicator. Since such constructs are manually signaling failure, their &&-chain breakage is legitimate and safe -- both for the command immediately preceding `false`, `return`, or `exit`, as well as for all preceding commands in the `if` branch. Therefore, stop flagging &&-chain breakage in these sorts of cases. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine committed
Sep 1, 2022 at 00:29 UTC
832c68b3c210267c93e1dcb2f2763372339ca36c
7 files changed
+62
-2
t/chainlint.pl
+8
@@ -503,6 +503,14 @@ sub accumulate {
503
goto DONE if $token =~ /\$\?/;
504
}
505
506
+ # if this command is "false", "return 1", or "exit 1" (which signal
507
+ # failure explicitly), then okay for all preceding commands to be
508
+ # missing "&&"
509
+ if ($$cmd[0] =~ /^(?:false|return|exit)$/) {
510
+ @$tokens = grep(!/^\?!AMP\?!$/, @$tokens);
511
+ goto DONE;
512
+ }
513
+
514
# flag missing "&&" at end of previous command
515
my $n = find_non_nl($tokens);
516
splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0;
t/chainlint/chain-break-false.expect
new
+9
@@ -0,0 +1,9 @@
1
+if condition not satisified
2
+then
3
+ echo it did not work...
4
+ echo failed!
5
+ false
6
+else
7
+ echo it went okay ?!AMP?!
8
+ congratulate user
9
+fi
t/chainlint/chain-break-false.test
new
+10
@@ -0,0 +1,10 @@
1
+# LINT: broken &&-chain okay if explicit "false" signals failure
2
+if condition not satisified
3
+then
4
+ echo it did not work...
5
+ echo failed!
6
+ false
7
+else
8
+ echo it went okay
9
+ congratulate user
10
+fi
t/chainlint/chain-break-return-exit.expect
+15
@@ -1,3 +1,18 @@
1
+case "$(git ls-files)" in
2
+one ) echo pass one ;;
3
+* ) echo bad one ; return 1 ;;
4
+esac &&
5
+(
6
+ case "$(git ls-files)" in
7
+ two ) echo pass two ;;
8
+ * ) echo bad two ; exit 1 ;;
9
+esac
10
+) &&
11
+case "$(git ls-files)" in
12
+dir/two"$LF"one ) echo pass both ;;
13
+* ) echo bad ; return 1 ;;
14
+esac &&
15
+
16
for i in 1 2 3 4 ; do
17
git checkout main -b $i || return $?
18
test_commit $i $i $i tag$i || return $?
t/chainlint/chain-break-return-exit.test
+18
@@ -1,3 +1,21 @@
1
+case "$(git ls-files)" in
2
+one) echo pass one ;;
3
+# LINT: broken &&-chain okay if explicit "return 1" signals failuire
4
+*) echo bad one; return 1 ;;
5
+esac &&
6
+(
7
+ case "$(git ls-files)" in
8
+ two) echo pass two ;;
9
+# LINT: broken &&-chain okay if explicit "exit 1" signals failuire
10
+ *) echo bad two; exit 1 ;;
11
+ esac
12
+) &&
13
+case "$(git ls-files)" in
14
+dir/two"$LF"one) echo pass both ;;
15
+# LINT: broken &&-chain okay if explicit "return 1" signals failuire
16
+*) echo bad; return 1 ;;
17
+esac &&
18
+
19
for i in 1 2 3 4 ; do
20
# LINT: broken &&-chain okay if explicit "return $?" signals failure
21
git checkout main -b $i || return $?
t/chainlint/if-in-loop.expect
+1
-1
@@ -3,7 +3,7 @@
3
do
4
if false
5
then
6
- echo "err" ?!AMP?!
6
+ echo "err"
7
exit 1
8
fi ?!AMP?!
9
foo
t/chainlint/if-in-loop.test
+1
-1
@@ -3,7 +3,7 @@
3
do
4
if false
5
then
6
-# LINT: missing "&&" on "echo"
6
+# LINT: missing "&&" on "echo" okay since "exit 1" signals error explicitly
7
echo "err"
8
exit 1
9
# LINT: missing "&&" on "fi"