Improve error handling and make it more consistent.

The three cases where we catch util.ParseError are more consistent:

- interactive
- string input (osh -c)
- file input

I changed the ${x/} error to raise an exception to test the new style.

bin/oil.py

@@ -123,7 +123,14 @@ def InteractiveLoop(opts, ex, c_parser, arena):
      print('EOF')
      break
    else:
      node = c_parser.ParseCommandLine()
      try:
        node = c_parser.ParseCommandLine()
      except util.ParseError as e:
        ui.PrettyPrintError(e, arena)

        c_parser.Reset()
        c_parser.ResetInputObjects()
        continue
      #log('parsed node: %s', node)

      # Failed parse.
@@ -133,7 +140,7 @@ def InteractiveLoop(opts, ex, c_parser, arena):
      if not node:
        e = c_parser.Error()
        # NOTE: This is a bit verbose.
        ui.PrintErrorStack(e, arena, sys.stderr)
        ui.PrintErrorStack(e, arena)

        c_parser.Reset()
        c_parser.ResetInputObjects()
@@ -257,7 +264,7 @@ def OshMain(argv0, argv, login_shell):
        rc_node = rc_c_parser.ParseWholeFile()
        if not rc_node:
          err = rc_c_parser.Error()
          ui.PrintErrorStack(err, arena, sys.stderr)
          ui.PrintErrorStack(err, arena)
          return 2  # parse error is code 2
      finally:
        arena.PopSource()
@@ -328,14 +335,13 @@ def OshMain(argv0, argv, login_shell):
      node = c_parser.ParseWholeFile()
    except util.ParseError as e:
      ui.PrettyPrintError(e, arena, sys.stderr)
      print('parse error: %s' % e.UserErrorString(), file=sys.stderr)
      return 2
    else:
      # TODO: Remove this older form of error handling.
      if not node:
        err = c_parser.Error()
        assert err, err  # can't be empty
        ui.PrintErrorStack(err, arena, sys.stderr)
        ui.PrintErrorStack(err, arena)
        return 2  # parse error is code 2

    do_exec = True
@@ -500,14 +506,13 @@ def OshCommandMain(argv):
    node = c_parser.ParseWholeFile()
  except util.ParseError as e:
    ui.PrettyPrintError(e, arena, sys.stderr)
    print('parse error: %s' % e.UserErrorString(), file=sys.stderr)
    return 2
  else:
    # TODO: Remove this older form of error handling.
    if not node:
      err = c_parser.Error()
      assert err, err  # can't be empty
      ui.PrintErrorStack(err, arena, sys.stderr)
      ui.PrintErrorStack(err, arena)
      return 2  # parse error is code 2

  f.close()

core/ui.py

@@ -128,9 +128,10 @@ def PrettyPrintError(parse_error, arena, f=sys.stderr):
    return

  PrintFilenameAndLine(span_id, arena, f=f)
  print(parse_error.UserErrorString(), file=f)


def PrintErrorStack(error_stack, arena, f):
def PrintErrorStack(error_stack, arena, f=sys.stderr):
  """
  NOTE:
  - Parse errors always occur within a single arena.  Actually NO, you want to
@@ -141,6 +142,5 @@ def PrintErrorStack(error_stack, arena, f):
  # - parse errors happen at runtime because of 'source'
  #   - should there be a distinction then?
  for err in error_stack:
    PrettyPrintError(err, arena, f)
    print(err.UserErrorString(), file=f)
    PrettyPrintError(err, arena, f=f)
    print('---', file=f)

osh/word_parse.py

@@ -214,8 +214,8 @@ def _ReadPatSubVarOp(self, lex_mode):
        pat.parts.append(p)

    if len(pat.parts) == 0:
      self._BadToken("Pattern must not be empty: %r", token=self.cur_token)
      return None
      p_die('Pattern in ${x/pat/replace} must not be empty',
            token=self.cur_token)
    else:
      first_part = pat.parts[0]
      if first_part.tag == word_part_e.LiteralPart:

test/parse-errors.sh

@@ -32,6 +32,15 @@ cases-in-strings() {
  _error-case '$(( 1 +  ))'
  _error-case 'echo $( echo > >>  )'
  _error-case 'echo ${'

  # These are a little odd
  _error-case 'echo ${x/}'
  _error-case 'echo ${x//}'
  _error-case 'echo ${x///}'

  _error-case 'echo ${x/foo}'
  _error-case 'echo ${x//foo}'
  _error-case 'echo ${x///foo}'
}

# Cases in their own file