[ysh] Properly implement setglobal

It skips locals that shadow globals.

This is issue #1841.

osh/cmd_eval.py

@@ -760,7 +760,7 @@ def _DoMutation(self, node):
 
             num_lhs = len(node.lhs)
             if num_lhs == 1:
-                lvals = [self.expr_ev.EvalLhsExpr(node.lhs[0])]
+                lvals = [self.expr_ev.EvalLhsExpr(node.lhs[0], which_scopes)]
                 rhs_vals = [right_val]
             else:
                 items = val_ops.ToList(
@@ -776,7 +776,8 @@ def _DoMutation(self, node):
                 lvals = []
                 rhs_vals = []
                 for i, lhs_val in enumerate(node.lhs):
-                    lvals.append(self.expr_ev.EvalLhsExpr(lhs_val))
+                    lvals.append(
+                        self.expr_ev.EvalLhsExpr(lhs_val, which_scopes))
                     rhs_vals.append(items[i])
 
             for i, lval in enumerate(lvals):
@@ -823,7 +824,7 @@ def _DoMutation(self, node):
             # Checked in the parser
             assert len(node.lhs) == 1
 
-            aug_lval = self.expr_ev.EvalLhsExpr(node.lhs[0])
+            aug_lval = self.expr_ev.EvalLhsExpr(node.lhs[0], which_scopes)
             val = self.expr_ev.EvalExpr(node.rhs, loc.Missing)
 
             self.expr_ev.EvalAugmented(aug_lval, val, node.op, which_scopes)

spec/ysh-scope.test.sh

@@ -1,4 +1,4 @@
-## oils_failures_allowed: 2
+## oils_failures_allowed: 0
 
 # Demonstrations for users.  Could go in docs.
 
@@ -332,65 +332,135 @@ shopt -s ysh:upgrade
 var g = {}
 
 proc mutate {
-  var g = {}  # shadows global var
+  var g = {'local': 1}  # shadows global var
 
-  echo 'hi from mutate'
   setglobal g.key = 'mutated'
   setglobal g['key2'] = 'mutated'
 
-
+  echo 'local that is ignored'
   pp line (g)
 }
 
-echo 'BEFORE mutate'
+echo 'BEFORE mutate global'
 pp line (g)
 
 mutate
 
-echo 'AFTER mutate'
+echo 'AFTER mutate global'
 pp line (g)
 
 ## STDOUT:
-BEFORE mutate
+BEFORE mutate global
 (Dict)   {}
-hi from mutate
-(Dict)   {"key":"mutated","key2":"mutated"}
-AFTER mutate
+local that is ignored
+(Dict)   {"local":1}
+AFTER mutate global
 (Dict)   {"key":"mutated","key2":"mutated"}
 ## END
 
 #### setglobal a[i] inside proc
-
 shopt -s ysh:upgrade
 
 var a = [0]
 
 proc mutate {
   var a = [1]  # shadows global var
 
-  echo 'hi from mutate'
+  echo 'local that is ignored'
   setglobal a[0] = 42
 
   pp line (a)
 }
 
-echo 'BEFORE mutate'
+echo 'BEFORE mutate global'
 pp line (a)
 
 mutate
 
-echo 'AFTER mutate'
+echo 'AFTER mutate global'
 pp line (a)
 
 ## STDOUT:
-BEFORE mutate
+BEFORE mutate global
 (List)   [0]
-hi from mutate
-(List)   [42]
-AFTER mutate
+local that is ignored
+(List)   [1]
+AFTER mutate global
 (List)   [42]
 ## END
 
+#### setglobal a[i] += and d.key +=
+shopt -s ysh:upgrade
+
+var mylist = [0]
+var mydict = {k: 0}
+
+proc mutate {
+  # these locals are ignored
+  var mylist = []
+  var mydict = {}
+
+  setglobal mylist[0] += 5
+  setglobal mydict['k'] += 5
+}
+
+mutate
+
+pp line (mylist)
+pp line (mydict)
+
+## STDOUT:
+(List)   [5]
+(Dict)   {"k":5}
+## END
+
+#### setglobal a[i] - i can be local or global
+shopt -s ysh:upgrade
+
+var mylist = [0, 1]
+var mydict = {k: 0, n: 1}
+
+var i = 0
+var key = 'k'
+
+proc mutate1 {
+  var mylist = []  # IGNORED
+  var mydict = {}  # IGNORED
+
+  var i = 1
+  var key = 'n'
+
+  setglobal mylist[i] = 11
+  setglobal mydict[key] = 11
+}
+
+# Same thing without locals
+proc mutate2 {
+  var mylist = []  # IGNORED
+  var mydict = {}  # IGNORED
+
+  setglobal mylist[i] = 22
+  setglobal mydict[key] = 22
+}
+
+mutate1
+
+pp line (mylist)
+pp line (mydict)
+echo
+
+mutate2
+
+pp line (mylist)
+pp line (mydict)
+
+## STDOUT:
+(List)   [0,11]
+(Dict)   {"k":0,"n":11}
+
+(List)   [22,11]
+(Dict)   {"k":22,"n":11}
+## END
 
 #### unset inside proc uses local scope
 shopt --set parse_brace

test/ysh-parse-errors.sh

@@ -518,10 +518,24 @@ test-float-literals() {
   _ysh-parse-error '= _42.0'
 }
 
-test-place-expr() {
-  _ysh-should-parse 'setvar x.y = 42'
+test-lhs-expr() {
+  _ysh-should-parse 'setvar x.y[z] = 42'
+  _ysh-should-parse 'setvar a[i][j] = 42'
+
+  _ysh-should-parse 'setvar a[i], d.key = 42, 43'
+  _ysh-parse-error 'setvar a[i], 3 = 42, 43'
+  _ysh-parse-error 'setvar a[i], {}["key"] = 42, 43'
+
   _ysh-parse-error 'setvar x+y = 42'
+
+  # method call
   _ysh-parse-error 'setvar x->y = 42'
+
+  # this is allowed
+  _ysh-should-parse 'setglobal a[{k:3}["k"]]  = 42'
+
+  _ysh-parse-error 'setglobal {}["k"] = 42'
+  _ysh-parse-error 'setglobal [1,2][0] = 42'
 }
 
 test-destructure() {

test/ysh-runtime-errors.sh

@@ -895,6 +895,28 @@ test-trim-utf8-error() {
 EOF
 }
 
+test-setglobal() {
+   _ysh-should-run '
+var a = [0]
+setglobal a[1-1] = 42
+pp line (a)
+   '
+
+   _ysh-expr-error '
+var a = [0]
+setglobal a[a.bad] = 42
+pp line (a)
+   '
+
+   _ysh-should-run '
+var d = {e:{f:0}}
+setglobal d.e.f = 42
+pp line (d)
+setglobal d.e.f += 1
+pp line (d)
+   '
+}
+
 soil-run-py() {
   run-test-funcs
 }