Structured parse errors

[jparise]

idl.Parse() errors are always of the un-exported type idl.internal.parseError, which wraps a map of per-line parse error messages.

type parseError struct {
	Errors map[int][]string
}

It would be useful to expose these per-line error messages to the caller in a structured way. Otherwise, the only visibility callers have into parse errors is the multiline string provided by parseError.Error():

func (pe parseError) Error() string {
	var buffer bytes.Buffer
	buffer.WriteString("parse error\n")
	for line, msgs := range pe.Errors {
		for _, msg := range msgs {
			buffer.WriteString(fmt.Sprintf("  line %d: %s\n", line, msg))
		}
	}
	return buffer.String()
}

[abhinav]

Yeah, the error reporting is really old. I'm in favor of exposing this information. Perhaps something like this:

package idl

type ParseError struct{ Lines []LineError }

type LineError struct {
  Line int
  Err  error
}

It's been a while, but looking at lex.rl, what this will take modifying the following:

thriftrw-go/idl/internal/lex.rl

Line 346 in fe65efe

func (lex *lexer) Error(e string) {

To something like this:

func (lex *lexer) AppendError(err error)  {
  lex.parseFailed = true
  lex.errors = append(lex.errors, LineError{Line: lex.line, Err: err}
}

And updating all the call sites to it:

-lex.Error(err.Error())
+lex.AppendError(err)

If we define ParseError and LineError in idl/internal, then we'll need to export non-internal versions of them from the idl package. I'd like to avoid type ParseError = internal.ParseError because I think that that hurts documentation and discoverability, so maybe we'd have to re-define them in idl and map the internal.ParseError to the non-internal variants.