x/tools/go/analysis/passes/fieldalignment: mutates AST

[adonovan]

Noticed in passing: the fieldalignment analyzer (which as of gopls/v0.17.0 is now used in x/tools only by its tests) potentially mutates the AST it is passed by the analysis framework:

x/tools/go/analysis/passes/fieldalignment/fieldalignment.go:

func fieldalignment(pass *analysis.Pass, node *ast.StructType, typ *types.Struct) {
	...
	for _, f := range node.Fields.List {
		// TODO: Preserve comment, for now get rid of them.
		//       See https://github.com/golang/go/issues/20744
		f.Comment = nil
		f.Doc = nil

No analyzer should do that. Similar mutations in other analyzers could be a cause of a variety of bugs, inconsistencies, and crashes in tools such as gopls.

We should consider developing the deepHash approach used in x/tools/internal/refactor/inline/inline_test.go so that we can dynamically check for unwanted AST mutations around certain operations such as Analyzer.Run. (Of course, it is not cheap, and is only as good as the test coverage.)

[gabyhelp]

Related Issues and Documentation

• x/tools/go/analysis/passes/fieldalignment: suggested fix producing garbled output #43233

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[aktau]

No longer a concrete issue since fieldalignment was removed in #67762. But a more generic approach for detecting analyzers that do this could be useful. So perhaps a different title.