x/tools/go/{analysis,packages}: expose TypeErrors []types.Error fields

[adonovan]

The go/analysis package defines a standard interface between analysis drivers (e.g. go vet) and checkers (e.g. the printf format string checker) that allows static analyzers to be written once and run in a wide range of tools, environments, and build systems. Most analyzers assume well-typed code, but it is possible for an analyzer to indicate that it is safe to run despite type errors. The gopls tool makes extensive use of such analyzers, and in particular their ability to suggest a quick fix to the code that would repair a mistake. However, the go/analysis framework doesn't provide access to the errors detected during type checking, even to analyzers that have indicated they are safe to run in the presence of such errors. gopls currently uses its privileged location in the x/tools repo to obtain backdoor access to these fields, but we consider this a hack.

We propose to expose the list of type errors, alongside all the other existing results of type checking, in the go/analysis Pass structure, as a new field, TypeErrors []types.Error. Because this interface forms the contract between driver and analysis, this change implies that all drivers that support the RunDespiteErrors feature must in due course populate this field. In the meantime, we expect that third-party drivers that do not populate the new field may report fewer diagnostics or fixes from analyzers that rely on the new field but will otherwise behave gracefully.

For the go/packages-based driver to populate this field, it too would need to expose a similar field in its Packages struct. We propose a similar change there too.

The necessary code change can be seen in https://go.dev/cl/425095.

[findleyr]

Removing my assignment as this still goes through the proposal committee, even though it is an x/tools API change.

On first principles it makes sense to me that analyzers setting RunDespiteErrors receive the full results of type checking, including errors. However, I have concerns that this API encourages a structured interpretation of type-checker error messages, which should be discouraged. We interpret error messages in gopls, but this is a source of fragility and friction.

In short, I think we should do this -- gopls should not have privileged access to go/analysis internals -- but now is perhaps a good time to also revisit efforts aimed at adding additional structure to go/types.Error.

[gopherbot]

Change https://go.dev/cl/425095 mentions this issue: go/analysis: add Pass.TypeErrors field

[timothy-king]

If we do add structure to go/types.Error, the proposed change will forward additional information to the analyzers transparently. If we do not add more structure to go/types.Error, having access to this information still feels like 'better than nothing'. gopls using it (fragility and friction and all) feels like evidence of this value.

[timothy-king]

Can we think of a reason we would not be able to provide go/types.Errors in a consistent way across different drivers for analysis, internal/checker, go cmd, gopls, bazel, ... ? That could lead to inconsistent experiences across tools.

I have not thought of one, but maybe someone else can?

[adonovan]

@rsc I think this proposal is waiting for committee review, but do let me know if there's anything I need to do.

[adonovan]

Can we think of a reason we would not be able to provide go/types.Errors in a consistent way across different drivers for analysis, internal/checker, go cmd, gopls, bazel, ... ?

No, I cannot even imagine an obstacle here.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Does anyone object to accepting this proposal?

[adonovan]

Can't say for sure, but no-one was motivated to comment on the CL or proposal in the last eight weeks.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[adonovan]

The hour of victory is close at hand. ;-)

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group