x/tools/go/packages: preserve information about original errors

[kujenga]

I was looking at migrating a tool off of the golang.org/x/tools/go/loader package, as it is seemingly deprecated for use with Go modules, switching it onto https://godoc.org/golang.org/x/tools/go/packages. I ran into a disparity between the two packages in the type of errors that are available to the user after loading in the packages of interest.

A loader.PackageInfo provides a struct field Errors []error, which allows one to do type introspection on the actual error returned from within the loading process.

In contrast, the packages.Package has an Errors []Error struct field, where all the errors from the underlying loading process have been one-way transformed into a new struct that loses information, or at least makes it harder to get at. That conversion process happens here: https://github.com/golang/tools/blob/83362c3779f5f48611068d488a03ea7bbaddc81e/go/packages/packages.go#L613-L660

The specific use case I have is to introspect certain types of types.Error values so that the tool can then go and attempt to fix those errors in an automated fashion. Since the types.Error gets stringified, that information is no longer readily available.

Proposal

My proposal is to augment the packages.Error type to include an Err error field that a user of the package can introspect further if desired.

// An Error describes a problem with a package's metadata, syntax, or types.
type Error struct {
	Pos  string // "file:line:col" or "file:line" or "" or "-"
	Msg  string
	Kind ErrorKind
    Err error
}

Alternative workarounds

One can write parsing logic for the current Error.Pos field based on what is currently done in the source code, but this seems unnecessary given that the underlying information was already present in a more structured form.

[josharian]

@matloob @alandonovan

[ianthehat]

The transformation is deliberately in place to allow for serialization between the host application and the build system driver when needed.
The serialization format is JSON, and serializing interface fields (like error) does not work.
If you need some specific information, please let us know and we will look at how to add it to the Error struct, but adding the underlying error directly is not going to be viable and I would encourage you to also not try to parse/detect the message text.
If you are looking into detecting errors and trying to automate fixes, you probably don't want to be using the errors out of go/packages directly for that anyway, you probably want to be writing an analysis.Analyzer, matloob is currently looking at adding more support for producing findings with "fixes" attached.

[bcmills]

The serialization format is JSON, and serializing interface fields (like error) does not work.

One alternative would be to try to encode the error using encoding/gob and embed the resulting blob in the JSON output. encoding/gob requires interface implementations to be passed to Register prior to encoding, but we could at least pre-register a common set of expected error types.

[kujenga]

@ianthehat Got it, thank you for the response! I was not aware of the serialization use case, and was also had not taken a look at the analysis package, thank you for pointing that out.

The serialization format is JSON, and serializing interface fields (like error) does not work.

I realize that you pointed out higher-level concerns as well, but in addition to the possible use of gob pointed out above, I think one possible alternative here be to do something like the following, and avoid serializing the error field:

Err error `json:"-"`

If you need some specific information, please let us know and we will look at how to add it to the Error struct

The specific information that my code is using currently is the numeric token.Pos value from the types.Error struct, as well as the *token.FileSet (though this is available from the packages.Package struct), which I make use of when walking the AST for the purposes of re-writing it to resolve various errors, for example propagating context.Context through multiple layers of APIs that need it because something within it now requires a context.