Skip to content

Commit

Permalink
[strict_errexit] Allow assignments as conditionals.
Browse files Browse the repository at this point in the history
For example

    if x=$(false); then ...

and

    while x=$(true); do ...

The error message had the wrong location, which is #1107.  But we don't
need this error message at all.

I don't personally like this idiom.  But maybe someone uses it.

https://utcc.utoronto.ca/~cks/space/blog/programming/BourneIfCanSetVars?showcomments

Also fix location.SpanForCommand() function, although it's not tickled
because of the change above.
  • Loading branch information
Andy C committed Apr 24, 2022
1 parent b4f2bdf commit 55db3f0
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 4 deletions.
11 changes: 8 additions & 3 deletions frontend/location.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
from __future__ import print_function

from _devbuild.gen.syntax_asdl import (
command_e, command_t, command__Simple, command__Pipeline, command__AndOr,
command__DoGroup, command__Sentence, command__Subshell,
command__WhileUntil, command__If, command__Case, command__TimeBlock,
command_e, command_t, command__Simple, command__ShAssignment,
command__Pipeline, command__AndOr, command__DoGroup, command__Sentence,
command__Subshell, command__WhileUntil, command__If, command__Case,
command__TimeBlock,
BraceGroup,

arith_expr_e, arith_expr_t, compound_word, Token,
Expand Down Expand Up @@ -45,6 +46,10 @@ def SpanForCommand(node):
elif len(node.redirects):
return node.redirects[0].op.span_id

if tag == command_e.ShAssignment:
node = cast(command__ShAssignment, UP_node)
return node.spids[0]

if tag == command_e.Pipeline:
node = cast(command__Pipeline, UP_node)
return node.spids[0] # first |
Expand Down
3 changes: 2 additions & 1 deletion osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ def _HasManyStatuses(node):
UP_node = node
with tagswitch(node) as case:
# command subs in words are detected by allow_command_sub in ctx_ErrExit
if case(command_e.Simple, command_e.DBracket, command_e.DParen):
if case(command_e.Simple, command_e.ShAssignment, command_e.DBracket,
command_e.DParen):
return False

elif case(command_e.Pipeline):
Expand Down
1 change: 1 addition & 0 deletions spec/assign.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -669,3 +669,4 @@ dict:3
## END
## N-I dash/mksh status: 99
## N-I dash/mksh stdout-json: ""

59 changes: 59 additions & 0 deletions spec/errexit.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -390,3 +390,62 @@ status=0
ft
status=0
## END

#### ShAssignment used as conditional

while x=$(false)
do
echo while
done

if x=$(false)
then
echo if
fi

if x=$(true)
then
echo yes
fi

# Same thing with errexit -- NOT affected
set -o errexit

while x=$(false)
do
echo while
done

if x=$(false)
then
echo if
fi

if x=$(true)
then
echo yes
fi

# Same thing with strict_errexit -- NOT affected
shopt -s strict_errexit || true

while x=$(false)
do
echo while
done

if x=$(false)
then
echo if
fi

if x=$(true)
then
echo yes
fi

## STDOUT:
yes
yes
yes
## END
23 changes: 23 additions & 0 deletions test/runtime-errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,32 @@ strict_errexit_1() {
_strict-errexit-case 'shopt -s oil:basic
proc p { echo p }
if p { echo hi }'

# issue #1107
_strict-errexit-case '
myfunc() {
return 1
}
foo=$(true)
# test assignment without proc
while bar=$(false)
do
echo yes
done
# issue 1007 was caused using command.ShAssignment, rather than the more common
# command.Sentence with ;
while spam=$(myfunc)
do
echo yes
done
'
}

# OLD WAY OF BLAMING
# Note: most of these don't fail
strict_errexit_2() {
# Test out all the location info

Expand Down

0 comments on commit 55db3f0

Please sign in to comment.