[osh] Make ${!ref} with invalid ref non-fatal (#2204)

OSH is now like bash - it causes the command that contains the word to exit with status 1.

This is like the 'failglob' semantics.

* [osh] Add code comment about joining var ref with ${!a[@]-}
* [core] Add base class WordFailure for FailGlob and ValSubFailure

core/error.py

@@ -69,8 +69,8 @@ def __init__(self, msg, location):
         _ErrorWithLocation.__init__(self, msg, location)
 
 
-class FailGlob(_ErrorWithLocation):
-    """Raised when a glob matches nothing when failglob is set.
+class WordFailure(_ErrorWithLocation):
+    """Raised for failure of word evaluation
 
     Meant to be caught.
     """
@@ -80,6 +80,30 @@ def __init__(self, msg, location):
         _ErrorWithLocation.__init__(self, msg, location)
 
 
+class FailGlob(WordFailure):
+    """Raised when a glob matches nothing when failglob is set.
+
+    Meant to be caught.
+    """
+
+    def __init__(self, msg, location):
+        # type: (str, loc_t) -> None
+        WordFailure.__init__(self, 'failglob: ' + msg, location)
+
+
+class VarSubFailure(WordFailure):
+    """Raised when a variable substitution fails.  For example, this
+    is thrown with ${!ref} when the variable "ref" contains invalid
+    variable name such as ref="a b c".
+
+    Meant to be caught.
+    """
+
+    def __init__(self, msg, location):
+        # type: (str, loc_t) -> None
+        WordFailure.__init__(self, msg, location)
+
+
 class RedirectEval(_ErrorWithLocation):
     """Used in the CommandEvaluator.
 

osh/cmd_eval.py

@@ -1532,10 +1532,15 @@ def _DoRedirect(self, node, cmd_st):
         except error.RedirectEval as e:
             self.errfmt.PrettyPrintError(e)
             redirects = None
-        except error.FailGlob as e:  # e.g. echo hi > foo-*
+        except error.WordFailure as e:
+            # This happens e.g. with the following cases:
+            #
+            #   $ echo hi > foo-*   # with failglob (FailGlob)
+            #   $ echo > ${!undef}  # (VarSubFailure)
+            #
             if not e.HasLocation():
                 e.location = self.mem.GetFallbackLocation()
-            self.errfmt.PrettyPrintError(e, prefix='failglob: ')
+            self.errfmt.PrettyPrintError(e)
             redirects = None
 
         if redirects is None:
@@ -1890,12 +1895,12 @@ def _Execute(self, node):
         with vm.ctx_ProcessSub(self.shell_ex, process_sub_st):  # for wait()
             try:
                 status = self._Dispatch(node, cmd_st)
-            except error.FailGlob as e:
+            except error.WordFailure as e:  # e.g. echo hi > ${!undef}
                 if not e.HasLocation():  # Last resort!
                     e.location = self.mem.GetFallbackLocation()
-                self.errfmt.PrettyPrintError(e, prefix='failglob: ')
+                self.errfmt.PrettyPrintError(e)
                 status = 1  # another redirect word eval error
-                cmd_st.check_errexit = True  # failglob + errexit
+                cmd_st.check_errexit = True  # errexit for e.g. a=${!undef}
 
         # Now we've waited for process subs
 

osh/word_eval.py

@@ -893,6 +893,8 @@ def _EvalVarRef(self, val, blame_tok, quoted, vsub_state, vtest_place):
 
             elif case(value_e.BashArray):  # caught earlier but OK
                 val = cast(value.BashArray, UP_val)
+                # When there are more than one element in the array, this
+                # produces a wrong variable name containing spaces.
                 var_ref_str = ' '.join(bash_impl.BashArray_GetValues(val))
 
             elif case(value_e.BashAssoc):  # caught earlier but OK
@@ -902,7 +904,11 @@ def _EvalVarRef(self, val, blame_tok, quoted, vsub_state, vtest_place):
             else:
                 raise error.TypeErr(val, 'Var Ref op expected Str', blame_tok)
 
-        bvs_part = self.unsafe_arith.ParseVarRef(var_ref_str, blame_tok)
+        try:
+            bvs_part = self.unsafe_arith.ParseVarRef(var_ref_str, blame_tok)
+        except error.FatalRuntime as e:
+            raise error.VarSubFailure(e.msg, e.location)
+
         return self._VarRefValue(bvs_part, quoted, vsub_state, vtest_place)
 
     def _ApplyUnarySuffixOp(self, val, op):

spec/var-ref.test.sh

@@ -41,8 +41,6 @@ echo undef=${!undef}
 
 ## status: 1
 ## STDOUT:
-## END
-## OK bash STDOUT:
 NOUNSET
 ## END
 
@@ -451,12 +449,10 @@ check_indir "!a@"    "a aa"
 check_indir "#x"      1
 check_indir "x#y"
 check_indir "x/y/foo" foo
-check_indir "x@Q"    "'y'"
+check_indir "x@Q"     y
 echo done
-## status: 1
-## stdout-json: ""
-## OK bash status: 0
-## OK bash stdout: done
+## status: 0
+## stdout: done
 
 #### Bad var ref
 a='bad var name'
@@ -466,8 +462,6 @@ echo status=$?
 ## STDOUT:
 status=1
 ## END
-## OK osh stdout-json: ""
-## OK osh status: 1
 
 #### Bad var ref 2
 b='/'  # really bad
@@ -476,8 +470,6 @@ echo status=$?
 ## STDOUT:
 status=1
 ## END
-## OK osh stdout-json: ""
-## OK osh status: 1
 
 #### ${!OPTIND} (used by bash completion
 set -- a b c