Skip to content

Commit

Permalink
[sh-options] Turn strict-arith on by default.
Browse files Browse the repository at this point in the history
Adjusted spec tests.

This behavior is unsafe/annoying and slightly incompatible with bash.
Consider:

x=foo

- echo $(( x )) turned to 0 in OSH because 'foo' doesn't look like an
  integer.
- echo $(( x )) in bash would be 42 if the variable 'foo' is 42.  It
  happened to be 0 because in many cases 'foo' is undefined.

The user can still 'shopt -u strict-arith', and I've done that in many
of the tests.
  • Loading branch information
Andy Chu committed Jun 29, 2019
1 parent 0d1abff commit 04a8ae9
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 39 deletions.
25 changes: 11 additions & 14 deletions osh/expr_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,28 @@
def _StringToInteger(s, span_id=const.NO_INTEGER):
"""Use bash-like rules to coerce a string to an integer.
Runtime parsing enables silly stuff like $(( $(echo 1)$(echo 2) + 1 )) => 13
0xAB -- hex constant
010 -- octable constant
042 -- octal constant
42 -- decimal constant
64#z -- arbitary base constant
bare word: variable
quoted word: string
Dumb stuff like $(( $(echo 1)$(echo 2) + 1 )) => 13 is possible.
bare word: variable
quoted word: string (not done?)
"""
# TODO: In non-strict mode, empty string becomes zero. In strict mode, it's
# a runtime error.

if s.startswith('0x'):
try:
integer = int(s, 16)
except ValueError:
# TODO: Show line number
e_die('Invalid hex constant %r', s, span_id=span_id)
return integer

if s.startswith('0'):
try:
integer = int(s, 8)
except ValueError:
e_die('Invalid octal constant %r', s, span_id=span_id) # TODO: Show line number
e_die('Invalid octal constant %r', s, span_id=span_id)
return integer

if '#' in s:
Expand Down Expand Up @@ -92,7 +90,7 @@ def _StringToInteger(s, span_id=const.NO_INTEGER):
n *= base
return integer

# Plain integer
# Normal base 10 integer
try:
integer = int(s)
except ValueError:
Expand Down Expand Up @@ -220,13 +218,12 @@ def _ValToArith(self, val, span_id, int_coerce=True):
if val.tag == value_e.Undef: # 'nounset' already handled before got here
# Happens upon a[undefined]=42, which unfortunately turns into a[0]=42.
#log('blame_word %s arena %s', blame_word, self.arena)
e_die('Coercing undefined value to 0 in arithmetic context',
span_id=span_id)
e_die('Undefined value in arithmetic context '
'(0 if shopt -u strict-arith)', span_id=span_id)
return 0

if val.tag == value_e.Str:
# may raise FatalRuntimeError
return _StringToInteger(val.s, span_id=span_id)
return _StringToInteger(val.s, span_id=span_id) # calls e_die

if val.tag == value_e.StrArray: # array is valid on RHS, but not on left
return val.strs
Expand Down
27 changes: 11 additions & 16 deletions osh/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,28 +113,22 @@ def Disable(self):

# NOTE: strict-errexit CANNOT be on by default.
# - some are PARSING:
# - strict-glob-parse
# - strict-backslash-parse
# - strict-glob
# - strict-backslash
# - some are runtime:
# - strict-arith-eval
# - strict-arith
# - strict-word-eval
#
# Syntax idea:
# shopt -s 'strict-*' 2>/dev/null || true
# You could apply LooksLikeGlob to the argument and then match it against
# SHOPT_OPTION_NAMES.

'strict-argv', # empty argv not allowed
'strict-arith', # string to integer conversions
'strict-array', # no implicit conversion between string and array
'strict-backslash', # BadBackslash
'strict-control-flow', # break/continue at top level is fatal
'strict-errexit', # inherited to command subs, etc.
'strict-glob', # GlobParser

# strict-arith-eval?
# TODO: strict-array could be in here? And turn it on by default?
'strict-word-eval', # negative slices, unicode

# Not implemented
'strict-backslash', # BadBackslash
'strict-glob', # GlobParser
)


Expand Down Expand Up @@ -167,10 +161,11 @@ def __init__(self, mem, readline):

# OSH-specific options.

self.strict_argv = False

# The only one that's ON by default.
# e.g. $(( x )) where x doesn't look like integer is fatal
self.strict_arith = False
self.strict_arith = True

self.strict_argv = False

# No implicit conversions between string and array.
# Several problems (not all implemented):
Expand Down
16 changes: 11 additions & 5 deletions spec/arith.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ echo $((i+1))
## stdout: 2

#### SimpleVarSub within arith
echo $(($j + 1))
## stdout: 1
j=0
echo $(($j + 42))
## stdout: 42

#### BracedVarSub within ArithSub
echo $((${j:-5} + 1))
Expand Down Expand Up @@ -68,6 +69,7 @@ echo $((`echo 1` + 2))

#### Invalid string to int
# bash, mksh, and zsh all treat strings that don't look like numbers as zero.
shopt -u strict-arith || true
s=foo
echo $((s+5))
## OK dash stdout-json: ""
Expand Down Expand Up @@ -121,13 +123,15 @@ echo $a
## N-I dash stdout-json: ""

#### Increment undefined variables
shopt -u strict-arith || true
(( undef1++ ))
(( ++undef2 ))
echo "[$undef1][$undef2]"
## stdout: [1][1]
## N-I dash stdout-json: "[][]\n"
## N-I dash stdout: [][]

#### Increment and decrement array
shopt -u strict-arith || true
a=(5 6 7 8)
(( a[0]++, ++a[1], a[2]--, --a[3] ))
(( undef[0]++, ++undef[1], undef[2]--, --undef[3] ))
Expand Down Expand Up @@ -261,7 +265,8 @@ foo=5
x=oo
echo $(( foo + f$x + 1 ))
## stdout: 11
## OK osh stdout: 6
## OK osh stdout-json: ""
## OK osh status: 1

#### Bizarre recursive name evaluation - result of runtime parse/eval
foo=5
Expand All @@ -270,7 +275,8 @@ spam=bar
eggs=spam
echo $((foo+1)) $((bar+1)) $((spam+1)) $((eggs+1))
## stdout: 6 6 6 6
## OK osh stdout: 6 1 1 1
## OK osh stdout-json: ""
## OK osh status: 1
## N-I dash stdout-json: ""
## N-I dash status: 2

Expand Down
15 changes: 12 additions & 3 deletions spec/dbracket.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,20 @@ fi
# http://tldp.org/LDP/abs/html/testconstructs.html#DBLBRACKETS

#### Octal literals with -eq
shopt -u strict-arith || true
decimal=15
octal=017 # = 15 (decimal)
[[ $decimal -eq $octal ]] && echo true
[[ $decimal -eq ZZZ$octal ]] || echo false
## stdout-json: "true\nfalse\n"
## STDOUT:
true
false
## END
## N-I mksh stdout: false
# mksh doesn't implement this syntax for literals.

#### Hex literals with -eq
shopt -u strict-arith || true
decimal=15
hex=0x0f # = 15 (decimal)
[[ $decimal -eq $hex ]] && echo true
Expand All @@ -122,10 +127,13 @@ hex=0x0f # = 15 (decimal)

#### -eq on strings
# This is lame behavior: it does a conversion to 0 first for any string
shopt -u strict-arith || true
[[ a -eq a ]] && echo true
[[ a -eq b ]] && echo true
## stdout-json: "true\ntrue\n"
## OK bash/mksh stdout-json: "true\ntrue\n"
## STDOUT:
true
true
## END

#### [[ compare with literal -f (compare with test-builtin.test.sh)
var=-f
Expand Down Expand Up @@ -214,6 +222,7 @@ true
## N-I osh status: 1

#### -eq coercion produces weird results
shopt -u strict-arith || true
[[ '' -eq 0 ]] && echo true
## stdout: true

Expand Down
2 changes: 1 addition & 1 deletion spec/sh-options.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ shopt -u all:strict
show-strict
## STDOUT:
shopt -u strict-argv
shopt -u strict-arith
shopt -s strict-arith
-
shopt -s strict-argv
shopt -s strict-arith
Expand Down
1 change: 1 addition & 0 deletions spec/strict-options.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ argv "${undef[@]}"
## N-I dash stdout-json: ""

#### automatically creating arrays are INDEXED, not associative
shopt -u strict-arith || true

undef[2]=x
undef[3]=y
Expand Down

0 comments on commit 04a8ae9

Please sign in to comment.