Skip to content

Commit 832c68b

Browse files
sunshinecogitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a8f30ee commit 832c68b

File tree

7 files changed

+62
-2
lines changed

7 files changed

+62
-2
lines changed

t/chainlint.pl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,14 @@ sub accumulate {
503503
goto DONE if $token =~ /\$\?/;
504504
}
505505

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+
506514
# flag missing "&&" at end of previous command
507515
my $n = find_non_nl($tokens);
508516
splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0;

t/chainlint/chain-break-false.expect

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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+
116
for i in 1 2 3 4 ; do
217
git checkout main -b $i || return $?
318
test_commit $i $i $i tag$i || return $?

t/chainlint/chain-break-return-exit.test

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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+
119
for i in 1 2 3 4 ; do
220
# LINT: broken &&-chain okay if explicit "return $?" signals failure
321
git checkout main -b $i || return $?

t/chainlint/if-in-loop.expect

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
do
44
if false
55
then
6-
echo "err" ?!AMP?!
6+
echo "err"
77
exit 1
88
fi ?!AMP?!
99
foo

t/chainlint/if-in-loop.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
do
44
if false
55
then
6-
# LINT: missing "&&" on "echo"
6+
# LINT: missing "&&" on "echo" okay since "exit 1" signals error explicitly
77
echo "err"
88
exit 1
99
# LINT: missing "&&" on "fi"

0 commit comments

Comments
 (0)