From 423d2203b3386374cbd96d86055a62c797215313 Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Wed, 22 Aug 2018 23:15:35 -0700 Subject: [PATCH] More conversion from error codes to exceptions. - Remove None checks - Remove AddErrorContext calls - Make sure to tickle the errors from test/parse-errors.sh --- core/tdop.py | 11 ++----- osh/arith_parse_test.py | 11 ++----- osh/cmd_parse.py | 67 +++++++++++------------------------------ osh/cmd_parse_test.py | 5 +-- osh/word_parse.py | 39 +++++------------------- test/parse-errors.sh | 8 +++++ 6 files changed, 39 insertions(+), 102 deletions(-) diff --git a/core/tdop.py b/core/tdop.py index 052d45c0f9..1834aa2a46 100644 --- a/core/tdop.py +++ b/core/tdop.py @@ -259,7 +259,7 @@ def ParseUntil(self, rbp): """ # TODO: use Kind.Eof if self.op_id in (Id.Eof_Real, Id.Eof_RParen, Id.Eof_Backtick): - p_die('Unexpected end of input') + p_die('Unexpected end of input', word=self.cur_word) t = self.cur_word self.Next() # skip over the token, e.g. ! ~ + - @@ -287,13 +287,8 @@ def ParseUntil(self, rbp): return node def Parse(self): - try: - self.Next() # may raise ParseError - node = self.ParseUntil(0) - except util.ParseError as e: - self.error_stack.append(e) - return None - return node + self.Next() # may raise ParseError + return self.ParseUntil(0) class DynamicTdopParser(TdopParser): diff --git a/osh/arith_parse_test.py b/osh/arith_parse_test.py index 62b5651aa4..3476f7cc69 100755 --- a/osh/arith_parse_test.py +++ b/osh/arith_parse_test.py @@ -16,23 +16,16 @@ from core import word_eval from core import state from core import test_lib +from core import util from osh import parse_lib #from osh import arith_parse -class ExprSyntaxError(Exception): - pass - - def ParseAndEval(code_str): arena = test_lib.MakeArena('') w_parser, _ = parse_lib.MakeParserForCompletion(code_str, arena) anode = w_parser._ReadArithExpr() # need the right lex state? - - if not anode: - raise ExprSyntaxError("failed %s" % w_parser.Error()) - print('node:', anode) mem = state.Mem('', [], {}, None) @@ -55,7 +48,7 @@ def testEvalExpr(e, expected): def testSyntaxError(ex): try: actual = ParseAndEval(ex) - except ExprSyntaxError as e: + except util.ParseError as e: print(ex, '\t\t', e) else: raise AssertionError('Expected parse error: %r, got %r' % (ex, actual)) diff --git a/osh/cmd_parse.py b/osh/cmd_parse.py index e4042a986d..14d015523f 100644 --- a/osh/cmd_parse.py +++ b/osh/cmd_parse.py @@ -77,10 +77,6 @@ def ResetInputObjects(self): def Error(self): return self.error_stack - def AddErrorContext(self, msg, *args, **kwargs): - err = util.ParseError(msg, *args, **kwargs) - self.error_stack.append(err) - def GetCompletionState(self): return self.completion_stack @@ -124,10 +120,7 @@ def _MaybeReadHereDocs(self): # NOTE: There can be different kinds of parse errors in here docs. word = w_parser.ReadHereDocBody() - if not word: - self.AddErrorContext( - 'Error reading here doc body: %s', w_parser.Error()) - return False + assert word is not None h.body = word h.was_filled = True else: @@ -190,12 +183,10 @@ def _Eat(self, c_id): """ if not self._Peek(): return False - # TODO: It would be nicer to print the word type, right now we get a number + # TODO: Printing something like KW_Do is not friendly. We can map + # backwards using the _KEYWORDS list in osh/lex.py. if self.c_id != c_id: - self.AddErrorContext( - "Expected word type %s, got %s", c_id, self.cur_word, - word=self.cur_word) - return False + p_die("Expected word type %s", c_id, word=self.cur_word) self._Next() return True @@ -686,11 +677,7 @@ def _ParseForExprLoop(self): for (( init; cond; update )) for_sep? do_group """ node = self.w_parser.ReadForExpression() - if not node: - error_stack = self.w_parser.Error() - self.error_stack.extend(error_stack) - self.AddErrorContext("Parsing for expression failed") - return None + assert node is not None self._Next() if not self._Peek(): return None @@ -751,11 +738,8 @@ def _ParseForEachLoop(self): node.do_arg_iter = True # implicit for loop # do not advance - else: - # Hm I think these never happens? - self.AddErrorContext("Unexpected word in for loop: %s", self.cur_word, - word=self.cur_word) - return None + else: # for foo BAD + p_die('Unexpected word after for loop variable', word=self.cur_word) node.spids.extend((in_spid, semi_spid)) @@ -851,10 +835,8 @@ def ParseCaseItem(self): dsemi_spid = word.LeftMostSpanForWord(self.cur_word) self._Next() else: - # This never happens? - self.AddErrorContext('Expected DSEMI or ESAC, got %s', self.cur_word, - word=self.cur_word) - return None + # Happens on EOF + p_die('Expected ;; or esac', word=self.cur_word) if not self._NewlineOk(): return None @@ -1037,10 +1019,7 @@ def ParseCompoundCommand(self): return self.ParseDParen() # This never happens? - self.AddErrorContext( - "Expected a compound command (e.g. for while if case), got %s", - self.cur_word, word=self.cur_word) - return None + p_die('Unexpected word while parsing compound command', word=self.cur_word) def ParseFunctionBody(self, func): """ @@ -1185,15 +1164,8 @@ def ParseDBracket(self): def ParseDParen(self): maybe_error_word = self.cur_word self._Next() # skip (( - #print('1 ((', self.cur_word) anode = self.w_parser.ReadDParen() - if not anode: - error_stack = self.w_parser.Error() - self.error_stack.extend(error_stack) - self.AddErrorContext("Error parsing ((", word=maybe_error_word) - return None - - #print('2 ((', self.cur_word) + assert anode is not None return ast.DParen(anode) def ParseCommand(self): @@ -1241,11 +1213,11 @@ def ParseCommand(self): return self.ParseSimpleCommand() # echo foo - # Does this ever happen? - self.AddErrorContext( - "ParseCommand: Expected to parse a command, got %s", self.cur_word, - word=self.cur_word) - return None + if self.c_kind == Kind.Eof: + p_die("Unexpected EOF while parsing command", word=self.cur_word) + + # e.g. ) + p_die("Invalid word while parsing command", word=self.cur_word) def ParsePipeline(self): """ @@ -1286,7 +1258,6 @@ def ParsePipeline(self): child = self.ParseCommand() if not child: - self.AddErrorContext('Error parsing command after pipe') # TODO: Return partial pipeline here? All signatures shouldbe (ok, # node). Only the completion uses the node when ok is False. return None @@ -1399,9 +1370,8 @@ def ParseCommandLine(self): done = True else: - self.AddErrorContext( - 'ParseCommandLine: Unexpected token %s', self.cur_word) - return None + # Shouldn't happen? + assert False, 'ParseCommandLine: Unexpected word %s' % self.cur_word children.append(child) @@ -1450,7 +1420,6 @@ def ParseCommandTerm(self): child = self.ParseAndOr() if not child: - self.AddErrorContext('Error parsing AndOr in ParseCommandTerm') return None if not self._Peek(): return None diff --git a/osh/cmd_parse_test.py b/osh/cmd_parse_test.py index b9887bbb46..212295a73d 100755 --- a/osh/cmd_parse_test.py +++ b/osh/cmd_parse_test.py @@ -489,10 +489,7 @@ def testParsePipeline(self): self.assertEqual(3, len(node.children)) self.assertEqual(command_e.Pipeline, node.tag) - # Should be an error - _, c_parser = InitCommandParser('ls foo|') - self.assertEqual(None, c_parser.ParsePipeline()) - print(c_parser.Error()) + _assertParseMethod(self, 'ls foo|', 'ParsePipeline', expect_success=False) def testParsePipelineBash(self): node = assertParseCommandList(self, 'ls | cat |& cat') diff --git a/osh/word_parse.py b/osh/word_parse.py index 06fc173106..9a28e9a768 100644 --- a/osh/word_parse.py +++ b/osh/word_parse.py @@ -767,9 +767,7 @@ def _ReadArithExpr(self, do_next=True): # calls self.ReadWord(lex_mode_e.ARITH) a_parser = tdop.TdopParser(arith_parse.SPEC, self) anode = a_parser.Parse() - if not anode: - error_stack = a_parser.Error() - self.error_stack.extend(error_stack) + assert anode is not None return anode # could be None def _ReadArithSubPart(self): @@ -846,9 +844,7 @@ def ReadDParen(self): self.lexer.PushHint(Id.Op_RParen, Id.Op_DRightParen) anode = self._ReadArithExpr() - if not anode: - self.AddErrorContext("Error parsing dparen statement") - return None + assert anode is not None #print('xx ((', self.cur_token) if self.token_type != Id.Arith_RParen: @@ -866,52 +862,33 @@ def ReadDParen(self): return anode def ReadForExpression(self): - """Read ((i=0; i<5; ++i)) -- part of command context. - - """ + """Read ((i=0; i<5; ++i)) -- part of command context.""" # No PushHint because we're in arith state. #self.lexer.PushHint(Id.Op_RParen, Id.Op_DRightParen) self._Next(lex_mode_e.ARITH) # skip over (( self._Peek() - if self.token_type == Id.Arith_Semi: - #print('Got empty init') + if self.token_type == Id.Arith_Semi: # for (( ; i < 10; i++ )) init_node = None else: init_node = self._ReadArithExpr(do_next=False) - if not init_node: - self.AddErrorContext("Error parsing for init") - return None self._Next(lex_mode_e.ARITH) - #print('INIT',init_node) self._Peek() - if self.token_type == Id.Arith_Semi: - #print('Got empty condition') + if self.token_type == Id.Arith_Semi: # for (( ; ; i++ )) cond_node = None else: cond_node = self._ReadArithExpr(do_next=False) - if not cond_node: - self.AddErrorContext("Error parsing for cond") - return None self._Next(lex_mode_e.ARITH) - #print('COND',cond_node) self._Peek() - if self.token_type == Id.Arith_RParen: - #print('Got empty update') + if self.token_type == Id.Arith_RParen: # for (( ; ; )) update_node = None else: update_node = self._ReadArithExpr(do_next=False) - if not update_node: - self.AddErrorContext("Error parsing for update") - return None self._Next(lex_mode_e.ARITH) - #print('UPDATE',update_node) - #print('TT', self.cur_token) - # Second paren self._Peek() if self.token_type != Id.Arith_RParen: p_die('Expected ) to end for loop expression, got %r', @@ -1240,8 +1217,6 @@ def ReadHereDocBody(self): """ w = ast.CompoundWord() dq = self._ReadDoubleQuotedPart(here_doc=True) - if not dq: - self.AddErrorContext('Error parsing here doc body') - return False + assert dq is not None w.parts.append(dq) return w diff --git a/test/parse-errors.sh b/test/parse-errors.sh index e7ae860bfc..7b9cee1610 100755 --- a/test/parse-errors.sh +++ b/test/parse-errors.sh @@ -203,12 +203,20 @@ cmd-parse() { _error-case 'for $x in 1 2 3; do echo $i; done' _error-case 'for x.y in 1 2 3; do echo $i; done' _error-case 'for x in 1 2 3; &' + _error-case 'for foo BAD' _error-case 'x"y"() { echo hi; }' _error-case 'function x"y" { echo hi; }' _error-case '}' + + _error-case 'case foo in *) echo ' + + _error-case 'ls foo|' + _error-case 'ls foo&&' + + _error-case 'foo()' } redirect() {