bufbuild/protocompile

Tidy up error API for parser.Parse

Opened this issue · 0 comments

bufdev commented

The Godoc says:

// If the error returned is due to a syntax error in the source, then a non-nil
// AST is also returned. If the handler chooses to not abort the parse (e.g. the
// underlying error reporter returns nil instead of an error), the parser will
// attempt to recover and keep going. This allows multiple syntax errors to be
// reported in a single pass. And it also means that more of the AST can be
// populated (erroneous productions around the syntax error will of course be
// absent).

It would be nice if we could say something more concrete, such as If the error is due to a syntax error, errors.Is(err, something) will be true. For any other error, this is an error that will result in a nil AST. Right now, to handle errors from Parse, you're determining whether or not an error is a syntax error by using the nilness of the AST, whereas in my experience most Golang APIs use the error value to determine control flow here.