[refactor] Consolidate a[x]=$v and (( a[x] = v ))

Also add failing test cases for Python-level TypeError.  We still need
more type checks.

Addresses issue #207.

osh/expr_eval.py

@@ -128,9 +128,6 @@ def EvalLhs(node, arith_ev, mem, spid, lookup_mode):
   """lhs_expr -> lvalue.
 
   Used for a=b and a[x]=b
-
-  TODO: Rationalize with expr_eval EvalLhsAndLookup, which is used for a+=b
-  and a[x]+=b.
   """
   assert isinstance(node, lhs_expr_t), node
 
@@ -152,6 +149,33 @@ def EvalLhs(node, arith_ev, mem, spid, lookup_mode):
   raise AssertionError(node.tag)
 
 
+def _EvalLhsArith(node, mem, arith_ev):
+  """lhs_expr -> lvalue.
+  
+  Very similar to EvalLhs above in core/cmd_exec.
+  """
+  assert isinstance(node, lhs_expr_t), node
+
+  if node.tag == lhs_expr_e.LhsName:  # (( i = 42 ))
+    lval = lvalue.LhsName(node.name)
+    # TODO: location info.  Use the = token?
+    #lval.spids.append(spid)
+    return lval
+
+  if node.tag == lhs_expr_e.LhsIndexedName:  # (( a[42] = 42 ))
+    # The index of StrArray needs to be coerced to int, but not the index of
+    # an AssocArray.
+    int_coerce = not mem.IsAssocArray(node.name, scope_e.Dynamic)
+    index = arith_ev.Eval(node.index, int_coerce=int_coerce)
+
+    lval = lvalue.LhsIndexedName(node.name, index)
+    # TODO: location info.  Use the = token?
+    #lval.spids.append(node.spids[0])
+    return lval
+
+  raise AssertionError(node.tag)
+
+
 def EvalLhsAndLookup(node, arith_ev, mem, exec_opts,
                      lookup_mode=scope_e.Dynamic):
   """Evaluate the operand for i++, a[0]++, i+=2, a[0]+=2 as an R-value.
@@ -336,32 +360,6 @@ def _EvalLhsAndLookupArith(self, node):
     i = self._ValToArithOrError(val, span_id=span_id)
     return i, lval
 
-  def _EvalLhsArith(self, node):
-    """lhs_expr -> lvalue.
-    
-    Very similar to _EvalLhs in core/cmd_exec.
-    """
-    assert isinstance(node, lhs_expr_t), node
-
-    if node.tag == lhs_expr_e.LhsName:  # (( i = 42 ))
-      lval = lvalue.LhsName(node.name)
-      # TODO: location info.  Use the = token?
-      #lval.spids.append(spid)
-      return lval
-
-    if node.tag == lhs_expr_e.LhsIndexedName:  # (( a[42] = 42 ))
-      # The index of StrArray needs to be coerced to int, but not the index of
-      # an AssocArray.
-      int_coerce = not self.mem.IsAssocArray(node.name, scope_e.Dynamic)
-      index = self.Eval(node.index, int_coerce=int_coerce)
-
-      lval = lvalue.LhsIndexedName(node.name, index)
-      # TODO: location info.  Use the = token?
-      #lval.spids.append(node.spids[0])
-      return lval
-
-    raise AssertionError(node.tag)
-
   def _Store(self, lval, new_int):
     val = value.Str(str(new_int))
     self.mem.SetVar(lval, val, (), scope_e.Dynamic)
@@ -372,7 +370,7 @@ def Eval(self, node, int_coerce=True):
       node: osh_ast.arith_expr
 
     Returns:
-      int or list of strings
+      int or list of strings (or dict?)
     """
     # OSH semantics: Variable NAMES cannot be formed dynamically; but INTEGERS
     # can.  ${foo:-3}4 is OK.  $? will be a compound word too, so we don't have
@@ -420,7 +418,7 @@ def Eval(self, node, int_coerce=True):
 
       if op_id == Id.Arith_Equal:
         rhs = self.Eval(node.right)
-        lval = self._EvalLhsArith(node.left)
+        lval = _EvalLhsArith(node.left, self.mem, self)
         self._Store(lval, rhs)
         return rhs
 

spec/array.test.sh

@@ -473,7 +473,7 @@ argv.py "${a[@]:15:2}"
 ## N-I mksh status: 1
 ## N-I mksh stdout-json: ""
 
-#### Using an array itself as the index
+#### Using an array itself as the index on LHS
 # TODO: Fix OSH crash.
 shopt -u strict-arith
 a[a]=42
@@ -483,6 +483,16 @@ argv "${a[@]}" "${a[0]}" "${a[42]}" "${a[99]}"
 ['42', '99', '42', '99', '']
 ## END
 
+#### Using an array itself as the index on RHS
+shopt -u strict-arith
+a=(1 2 3)
+(( x = a[a] ))
+echo $x
+## stdout-json: ""
+## BUG bash/mksh STDOUT:
+2
+## END
+
 #### a[$x$y] on LHS and RHS
 x=1
 y=2

spec/assoc.test.sh

@@ -338,11 +338,19 @@ echo $var
 42
 ## END
 
-#### (( A[5] += 1 ))
+#### (( A[5] += 42 ))
 declare -A A
+(( A[5] = 10 ))
 (( A[5] += 6 ))
 echo ${A[5]}
 ## STDOUT:
-6
+16
 ## END
 
+#### (( A[5] += 42 )) with empty cell
+declare -A A
+(( A[5] += 6 ))
+echo ${A[5]}
+## STDOUT:
+6
+## END

test/spec.sh

@@ -554,7 +554,7 @@ arith-context() {
 }
 
 array() {
-  sh-spec spec/array.test.sh --osh-failures-allowed 3 \
+  sh-spec spec/array.test.sh --osh-failures-allowed 4 \
     $BASH $MKSH $OSH_LIST "$@"
 }