More polish on 'test' errors.

Also, fix call site of BoolParser to not check return code.

core/test_builtin.py

@@ -75,7 +75,7 @@ def _TwoArgs(argv):
     # TODO:
     # - syntax error
     # - separate lookup by unary
-    util.p_die('Expected unary operator, got %r', a0)
+    util.p_die('Expected unary operator, got %r (2 args)', a0)
   child = ast.StringWord(Id.Word_Compound, a1)
   return ast.BoolUnary(unary_id, child)
 
@@ -109,7 +109,7 @@ def _ThreeArgs(argv):
   if a0 == '(' and a2 == ')':
     return _StringWordTest(a1)
 
-  util.p_die('Syntax error: binary operator expected')
+  util.p_die('Syntax error: binary operator expected, got %r (3 args)', a1)
 
 
 def Test(argv, need_right_bracket):
@@ -165,10 +165,10 @@ def Test(argv, need_right_bracket):
       bool_node = b_parser.ParseForBuiltin()
 
   except util.ParseError as e:
-    util.error("test: %s", e.UserErrorString())
     # TODO: There should be a nice method to print argv.  And some way to point
     # to the error.
-    log("Error parsing test expression: %s", argv)
+    log("Error parsing %s", argv)
+    util.error("test: %s", e.UserErrorString())
     return 2  # parse error is 2
 
   # mem: Don't need it for BASH_REMATCH?  Or I guess you could support it
@@ -185,7 +185,7 @@ def Test(argv, need_right_bracket):
     # e.g. [ -t xxx ]
     # TODO: Printing the location would be nice.
     util.error('test: %s', e.UserErrorString())
-    return 2
+    return 2  # because this is more like a parser error.
 
   status = 0 if b else 1
   return status

osh/bool_parse.py

@@ -120,6 +120,8 @@ def Parse(self):
     node = self.ParseExpr()
     if self.op_id != Id.Lit_DRightBracket:
       #p_die("Expected ]], got %r", self.cur_word, word=self.cur_word)
+      # NOTE: This might be better as unexpected token, since ]] doesn't always
+      # make sense.
       p_die('Expected ]]', word=self.cur_word)
     return node
 

osh/cmd_parse.py

@@ -1215,11 +1215,7 @@ def ParseDBracket(self):
 
     self._Next()  # skip [[
     b_parser = BoolParser(self.w_parser)
-    bnode = b_parser.Parse()
-    if not bnode:
-      error_stack = b_parser.Error()
-      self.error_stack.extend(error_stack)
-      return None
+    bnode = b_parser.Parse()  # May raise
     return ast.DBracket(bnode)
 
   def ParseDParen(self):

test/parse-errors.sh

@@ -165,6 +165,8 @@ test-builtin() {
   # Hm some of these errors are wonky.  Need positions.
   _error-case '[ x x ]'
 
+  _error-case '[ x x x ]'
+
   # -o tests if an option is enabled.
   #_error-case '[ -o x ]'
 }