expand: make WriteEnviron.Set return an error

This mirrors os.Setenv, and more accurately represents the behavior of
setting a variable in the interpreter.

This is a breaking change in the expand package, so it won't be
backported to v2.

expand/arith.go

@@ -44,7 +44,9 @@ func Arithm(cfg *Config, expr syntax.ArithmExpr) (int, error) {
 			} else {
 				val--
 			}
-			cfg.envSet(name, strconv.Itoa(val))
+			if err := cfg.envSet(name, strconv.Itoa(val)); err != nil {
+				return 0, err
+			}
 			if x.Post {
 				return old, nil
 			}
@@ -139,7 +141,9 @@ func (cfg *Config) assgnArit(b *syntax.BinaryArithm) (int, error) {
 	case syntax.ShrAssgn:
 		val >>= uint(arg)
 	}
-	cfg.envSet(name, strconv.Itoa(val))
+	if err := cfg.envSet(name, strconv.Itoa(val)); err != nil {
+		return 0, err
+	}
 	return val, nil
 }
 

expand/environ.go

@@ -40,7 +40,10 @@ type WriteEnviron interface {
 	// attributes correctly. For example, changing an exported variable's
 	// value does not unexport it, and overwriting a name reference variable
 	// should modify its target.
-	Set(name string, vr Variable)
+	//
+	// An error may be returned if the operation is invalid, such as if the
+	// name is empty or if we're trying to overwrite a read-only variable.
+	Set(name string, vr Variable) error
 }
 
 // Variable describes a shell variable, which can have a number of attributes

expand/expand.go

@@ -116,13 +116,12 @@ func (cfg *Config) envGet(name string) string {
 	return cfg.Env.Get(name).String()
 }
 
-func (cfg *Config) envSet(name, value string) {
+func (cfg *Config) envSet(name, value string) error {
 	wenv, ok := cfg.Env.(WriteEnviron)
 	if !ok {
-		// TODO: we should probably error here
-		return
+		return fmt.Errorf("environment is read-only")
 	}
-	wenv.Set(name, Variable{Value: value})
+	return wenv.Set(name, Variable{Value: value})
 }
 
 // Literal expands a single shell word. It is similar to Fields, but the result

expand/param.go

@@ -193,7 +193,9 @@ func (cfg *Config) paramExp(pe *syntax.ParamExp) (string, error) {
 			fallthrough
 		case syntax.SubstColAssgn:
 			if str == "" {
-				cfg.envSet(name, arg)
+				if err := cfg.envSet(name, arg); err != nil {
+					return "", err
+				}
 				str = arg
 			}
 		case syntax.RemSmallPrefix, syntax.RemLargePrefix,

interp/interp.go

@@ -159,11 +159,14 @@ type expandEnv struct {
 	r *Runner
 }
 
+var _ expand.WriteEnviron = expandEnv{}
+
 func (e expandEnv) Get(name string) expand.Variable {
 	return e.r.lookupVar(name)
 }
-func (e expandEnv) Set(name string, vr expand.Variable) {
+func (e expandEnv) Set(name string, vr expand.Variable) error {
 	e.r.setVarInternal(name, vr)
+	return nil // TODO: return any errors
 }
 func (e expandEnv) Each(fn func(name string, vr expand.Variable) bool) {
 	e.r.Env.Each(fn)
@@ -800,7 +803,7 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
 		case *syntax.CStyleLoop:
 			r.arithm(y.Init)
 			for r.arithm(y.Cond) != 0 {
-				if r.loopStmtsBroken(ctx, x.Do) {
+				if r.exit != 0 || r.loopStmtsBroken(ctx, x.Do) {
 					break
 				}
 				r.arithm(y.Post)

interp/interp_test.go

@@ -524,6 +524,11 @@ var fileCases = []struct {
 		"for ((i=0; i<3; i++)); do echo $i; done",
 		"0\n1\n2\n",
 	},
+	// TODO: uncomment once expandEnv.Set starts returning errors
+	// {
+	// 	"readonly i; for ((i=0; i<3; i++)); do echo $i; done",
+	// 	"0\n1\n2\n",
+	// },
 	{
 		"for ((i=5; i>0; i--)); do echo $i; break; done",
 		"5\n",