[osh/word_eval] Respect compat_array when evaluating var refs

Fixes #964.

Only 1 failing test in spec/var-ref!

osh/sh_expr_eval.py

@@ -260,6 +260,11 @@ def ParseVarRef(self, ref_str, span_id):
         e_die('Invalid var ref', span_id=span_id)
 
     #log('ParseVarRef %s', bvs_part)
+
+    # Hack: There is no ${ on the "virtual" braced_var_sub, but we can add one
+    # for error messages
+    bvs_part.spids.append(span_id)
+
     return bvs_part
 
 

osh/word_eval.py

@@ -1043,10 +1043,50 @@ def _EmptyMaybeStrArrayOrError(self, token):
     else:
       return value.MaybeStrArray([])
 
+  def _EvalBracketOp(self, val, part, quoted, var_name, maybe_decay_array):
+    # type: (value_t, braced_var_sub, bool, Optional[str], List[bool]) -> Tuple[value_t, a_index_t]
+
+    var_index = None  # type: a_index_t
+    if part.bracket_op:
+      bracket_op = part.bracket_op
+      UP_bracket_op = bracket_op
+      with tagswitch(bracket_op) as case:
+        if case(bracket_op_e.WholeArray):
+          val = self._WholeArray(val, part, quoted, maybe_decay_array)
+
+        elif case(bracket_op_e.ArrayIndex):
+          bracket_op = cast(bracket_op__ArrayIndex, UP_bracket_op)
+          val, var_index = self._ArrayIndex(val, part)
+
+        else:
+          raise AssertionError(bracket_op.tag_())
+
+    else:  # no bracket op
+      if var_name and val.tag_() in (value_e.MaybeStrArray, value_e.AssocArray):
+        if CheckCompatArray(var_name, self.exec_opts,
+                            not (part.prefix_op or part.suffix_op)):
+          # for ${BASH_SOURCE}, etc.
+          val = ResolveCompatArray(val)
+        else:
+          tmp = part.suffix_op
+          # TODO: An IR for ${} might simplify these lengthy conditions
+          if (tmp and tmp.tag_() == suffix_op_e.Nullary and 
+              #cast(Token, tmp).id in (Id.VOp0_a, Id.VOp0_Q)):
+              cast(Token, tmp).id == Id.VOp0_a):
+            # ${array@a} is a string
+            pass
+          else:
+            e_die("Array %r can't be referred to as a scalar (without @ or *)",
+                  var_name, part=part)
+
+    return val, var_index
+
   def _VarRefValue(self, part, quoted, maybe_decay_array):
     # type: (braced_var_sub, bool, List[bool]) -> value_t
     """Duplicates some logic from _EvalBracedVarSub, but returns a value_t."""
 
+    var_name = None  # type: str
+
     # 1. Evaluate from (var_name, var_num, token Id) -> value
     if part.token.id == Id.VSub_Name:
       var_name = part.token.val
@@ -1059,21 +1099,8 @@ def _VarRefValue(self, part, quoted, maybe_decay_array):
       # $* decays
       val = self._EvalSpecialVar(part.token.id, quoted, maybe_decay_array)
 
-    if part.bracket_op:
-      bracket_op = part.bracket_op
-      UP_bracket_op = bracket_op
-      with tagswitch(bracket_op) as case:
-        if case(bracket_op_e.WholeArray):
-          val = self._WholeArray(val, part, quoted, maybe_decay_array)
-
-        elif case(bracket_op_e.ArrayIndex):
-          bracket_op = cast(bracket_op__ArrayIndex, UP_bracket_op)
-          val, var_index = self._ArrayIndex(val, part)
-          # NOTE: In the normal case, var_index is used by _ApplyTestOp?
-
-        else:
-          raise AssertionError(bracket_op.tag_())
-
+    # We don't need var_index because it's only for L-Values of test ops?
+    val, unused = self._EvalBracketOp(val, part, quoted, var_name, maybe_decay_array)
     return val
 
   def _EvalBracedVarSub(self, part, part_vals, quoted):
@@ -1113,6 +1140,9 @@ def _EvalBracedVarSub(self, part, part_vals, quoted):
     # 6. test op
     # 7. maybe_decay_array
 
+    # maybe_decay_array is for joining "${a[*]}" and unquoted ${a[@]} AFTER
+    # suffix ops are applied.  If we take the length with a prefix op, the
+    # distinction is ignored.
     maybe_decay_array = [False]  # for $*, ${a[*]}, etc.
     var_name = None  # type: str  # For ${foo=default}
 
@@ -1149,44 +1179,8 @@ def _EvalBracedVarSub(self, part, part_vals, quoted):
       # $* decays
       val = self._EvalSpecialVar(part.token.id, quoted, maybe_decay_array)
 
-    var_index = None  # type: a_index_t
-
-    # 2. Bracket: value -> (value v, bool maybe_decay_array)
-    # maybe_decay_array is for joining ${a[*]} and unquoted ${a[@]} AFTER
-    # suffix ops are applied.  If we take the length with a prefix op, the
-    # distinction is ignored.
-    if part.bracket_op:
-      bracket_op = part.bracket_op
-      UP_bracket_op = bracket_op
-      with tagswitch(bracket_op) as case:
-        if case(bracket_op_e.WholeArray):
-          val = self._WholeArray(val, part, quoted, maybe_decay_array)
-
-        elif case(bracket_op_e.ArrayIndex):
-          bracket_op = cast(bracket_op__ArrayIndex, UP_bracket_op)
-          val, var_index = self._ArrayIndex(val, part)
-          # NOTE: var_index used below by _ApplyTestOp
-
-        else:
-          raise AssertionError(bracket_op.tag_())
-
-    else:  # no bracket op
-      if var_name and val.tag_() in (value_e.MaybeStrArray, value_e.AssocArray):
-        if CheckCompatArray(var_name, self.exec_opts,
-                            not (part.prefix_op or part.suffix_op)):
-          # for ${BASH_SOURCE}, etc.
-          val = ResolveCompatArray(val)
-        else:
-          tmp = part.suffix_op
-          # TODO: An IR for ${} might simplify these lengthy conditions
-          if (tmp and tmp.tag_() == suffix_op_e.Nullary and 
-              #cast(Token, tmp).id in (Id.VOp0_a, Id.VOp0_Q)):
-              cast(Token, tmp).id == Id.VOp0_a):
-            # ${array@a} is a string
-            pass
-          else:
-            e_die("Array %r can't be referred to as a scalar (without @ or *)",
-                  var_name, part=part)
+    # 2. Bracket Op
+    val, var_index = self._EvalBracketOp(val, part, quoted, var_name, maybe_decay_array)
 
     # Do the _EmptyStrOrError up front here, EXCEPT in the case of Kind.VTest
     suffix_is_test = False

osh/word_parse.py

@@ -1710,7 +1710,6 @@ def ParseVarRef(self):
     self._Peek()
     if self.token_type != Id.Eof_Real:
       p_die('Expected end of var ref', token=self.cur_token)
-
     return part
 
   def LookAhead(self):

spec/var-ref.test.sh

@@ -212,23 +212,39 @@ echo bad=${!badref}
 ## OK bash status: 0
 ## OK bash stdout: bad=
 
-#### var ref TO array var
+#### array ref without eval_unsafe_arith
+shopt -s compat_array
+
+declare -a array=(ale bean)
+ref='array[0]'
+echo ${!ref}
+## status: 1
+## stdout-json: ""
+## N-I bash status: 0
+## N-I bash STDOUT:
+ale
+## END
+
+#### array ref without compat_array
 shopt -s eval_unsafe_arith
 
 declare -a array=(ale bean)
+ref='array'
+echo ${!ref}
+## status: 1
+## stdout-json: ""
+## N-I bash status: 0
+## N-I bash STDOUT:
+ale
+## END
 
-ref=array
-ref_AT='array[@]'  # this gives you the whole thing, gah
+#### var ref TO array var
+shopt -s eval_unsafe_arith compat_array
 
-if [[ $SH == *osh ]]; then
-  # there should be errors for some of these?
-  echo ${!ref}
-  echo ${!ref_AT}
+declare -a array=(ale bean)
 
-  # This allows the array == array[0] behavior.  And can work for indirect
-  # references too?
-  shopt -s compat_array
-fi
+ref='array'  # when compat_array is on, this is like array[0]
+ref_AT='array[@]'
 
 echo ${!ref}
 echo ${!ref_AT}
@@ -260,7 +276,7 @@ f 'array[*]'
 ## END
 
 #### var ref TO assoc array a[key]
-shopt -s eval_unsafe_arith
+shopt -s eval_unsafe_arith compat_array
 
 declare -A assoc=([ale]=bean [corn]=dip)
 ref=assoc
@@ -287,14 +303,14 @@ ref_SUB_QUOTED=bean
 ref_SUB_BAD=
 ## END
 
-#### var ref to nasty complex array references
-shopt -s eval_unsafe_arith
+#### var ref TO array with arbitrary subscripts
+shopt -s eval_unsafe_arith compat_array
 
-i=0
 f() {
-    ((i++))
-    val=$(echo "${!1}")
-    [ "$val" = y ] && echo -n "$i "
+  local val=$(echo "${!1}")
+  if test "$val" = y; then 
+    echo "works: $1"
+  fi
 }
 # Warmup: nice plain array reference
 a=(x y)
@@ -313,8 +329,6 @@ f 'a[b*]'  # operand expected
 f 'a[1"]'  # bad substitution
 #
 # Allowed: most everything else in section 3.5 "Shell Expansions".
-# tilde expansion
-( PWD=1; f 'a[~+]' ); ((i++))
 # shell parameter expansion
 b=1
 f 'a[$b]'
@@ -324,12 +338,29 @@ f 'a[${c:-1}]'
 f 'a[$(echo 1)]'
 # arithmetic expansion
 f 'a[$(( 3 - 2 ))]'
-echo end
+
 # All of these are undocumented and probably shouldn't exist,
 # though it's always possible some will turn up in the wild and
 # we'll end up implementing them.
-## stdout: 1 end
-## OK bash stdout: 1 7 8 9 10 11 end
+
+## STDOUT:
+works: a[1]
+works: a[$b]
+works: a[${c:-1}]
+works: a[$(echo 1)]
+works: a[$(( 3 - 2 ))]
+## END
+
+#### Bizarre tilde expansion in array index
+a=(x y)
+PWD=1
+ref='a[~+]'
+echo ${!ref}
+## status: 1
+## BUG bash status: 0
+## BUG bash STDOUT:
+y
+## END
 
 #### Indirect expansion TO fancy expansion features bash disallows
 shopt -s eval_unsafe_arith

test/spec.sh

@@ -729,7 +729,7 @@ nocasematch-match() {
 # ${!var} syntax -- oil should replace this with associative arrays.
 # mksh has completely different behavior for this syntax.  Not worth testing.
 var-ref() {
-  sh-spec spec/var-ref.test.sh --osh-failures-allowed 4 \
+  sh-spec spec/var-ref.test.sh --osh-failures-allowed 1 \
     $BASH $OSH_LIST "$@"
 }