go/analysis: don't apply fixes to generated files

This change causes the analysis drivers unitchecker,
{single,multi}checker, and analysistest to discard any
fix any of whose edits is in a generated Go source file
(as defined by ast.IsGenerated).

gopls's analysis driver is not yet covered.

(It would be nice if more of the "fix" logic of analysistest
could be consolidated with the various checkers.)

+ test of analysistest (via modernize_test)
+ test of unitchecker

Fixes golang/go#75948

Change-Id: I96a4abf605657392a6dbb58b22a036dc8f9f6d44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/718505
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Hongxiang Jiang <hxjiang@golang.org>

Author: adonovan

go/analysis/analysistest/analysistest.go

@@ -8,6 +8,7 @@ package analysistest
 import (
 	"bytes"
 	"fmt"
+	"go/ast"
 	"go/format"
 	"go/token"
 	"go/types"
@@ -157,6 +158,8 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 		}
 	}
 
+	generated := make(map[*token.File]bool)
+
 	// Process each result (package) separately, matching up the suggested
 	// fixes into a diff, which we will compare to the .golden file.  We have
 	// to do this per-result in case a file appears in two packages, such as in
@@ -170,6 +173,16 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 	for _, result := range results {
 		act := result.Action
 
+		// Compute set of generated files.
+		for _, file := range internal.ActionPass(act).Files {
+			// Memoize, since there may be many actions
+			// for the same package (list of files).
+			tokFile := act.Package.Fset.File(file.Pos())
+			if _, seen := generated[tokFile]; !seen {
+				generated[tokFile] = ast.IsGenerated(file)
+			}
+		}
+
 		// For each fix, split its edits by file and convert to diff form.
 		var (
 			// fixEdits: message -> fixes -> filename -> edits
@@ -182,12 +195,21 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns
 		)
 		for _, diag := range act.Diagnostics {
 			// Fixes are validated upon creation in Pass.Report.
+		fixloop:
 			for _, fix := range diag.SuggestedFixes {
 				// Assert that lazy fixes have a Category (#65578, #65087).
 				if inTools && len(fix.TextEdits) == 0 && diag.Category == "" {
 					t.Errorf("missing Diagnostic.Category for SuggestedFix without TextEdits (gopls requires the category for the name of the fix command")
 				}
 
+				// Skip any fix that edits a generated file.
+				for _, edit := range fix.TextEdits {
+					file := act.Package.Fset.File(edit.Pos)
+					if generated[file] {
+						continue fixloop
+					}
+				}
+
 				// Convert edits to diff form.
 				// Group fixes by message and file.
 				edits := make(map[string][]diff.Edit)

go/analysis/internal/checker/checker.go

@@ -198,6 +198,7 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) {
 				fixActions[i] = driverutil.FixAction{
 					Name:         act.String(),
 					Pkg:          act.Package.Types,
+					Files:        act.Package.Syntax,
 					FileSet:      act.Package.Fset,
 					ReadFileFunc: pass.ReadFile,
 					Diagnostics:  act.Diagnostics,

go/analysis/passes/modernize/any.go

@@ -10,25 +10,19 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	"golang.org/x/tools/internal/versions"
 )
 
 var AnyAnalyzer = &analysis.Analyzer{
-	Name: "any",
-	Doc:  analyzerutil.MustExtractDoc(doc, "any"),
-	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
-		inspect.Analyzer,
-	},
-	Run: runAny,
-	URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#any",
+	Name:     "any",
+	Doc:      analyzerutil.MustExtractDoc(doc, "any"),
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      runAny,
+	URL:      "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#any",
 }
 
 // The any pass replaces interface{} with go1.18's 'any'.
 func runAny(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	for curFile := range filesUsingGoVersion(pass, versions.Go1_18) {
 		for curIface := range curFile.Preorder((*ast.InterfaceType)(nil)) {
 			iface := curIface.Node().(*ast.InterfaceType)

go/analysis/passes/modernize/bloop.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	typeindexanalyzer "golang.org/x/tools/internal/analysis/typeindex"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/moreiters"
@@ -29,7 +28,6 @@ var BLoopAnalyzer = &analysis.Analyzer{
 	Name: "bloop",
 	Doc:  analyzerutil.MustExtractDoc(doc, "bloop"),
 	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
 		inspect.Analyzer,
 		typeindexanalyzer.Analyzer,
 	},
@@ -46,8 +44,6 @@ var BLoopAnalyzer = &analysis.Analyzer{
 //	for i := 0; i < b.N; i++ {}  =>   for b.Loop() {}
 //	for range b.N {}
 func bloop(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	if !typesinternal.Imports(pass.Pkg, "testing") {
 		return nil, nil
 	}

go/analysis/passes/modernize/errorsastype.go

@@ -15,7 +15,6 @@ import (
 	"golang.org/x/tools/go/ast/edge"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	typeindexanalyzer "golang.org/x/tools/internal/analysis/typeindex"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/goplsexport"
@@ -29,7 +28,7 @@ var errorsastypeAnalyzer = &analysis.Analyzer{
 	Name:     "errorsastype",
 	Doc:      analyzerutil.MustExtractDoc(doc, "errorsastype"),
 	URL:      "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#errorsastype",
-	Requires: []*analysis.Analyzer{generated.Analyzer, typeindexanalyzer.Analyzer},
+	Requires: []*analysis.Analyzer{typeindexanalyzer.Analyzer},
 	Run:      errorsastype,
 }
 
@@ -79,8 +78,6 @@ func init() {
 //
 // - if errors.As(err, myerr) && othercond { ... }
 func errorsastype(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	var (
 		index = pass.ResultOf[typeindexanalyzer.Analyzer].(*typeindex.Index)
 		info  = pass.TypesInfo

go/analysis/passes/modernize/fmtappendf.go

@@ -14,7 +14,6 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/edge"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	typeindexanalyzer "golang.org/x/tools/internal/analysis/typeindex"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/typesinternal/typeindex"
@@ -25,7 +24,6 @@ var FmtAppendfAnalyzer = &analysis.Analyzer{
 	Name: "fmtappendf",
 	Doc:  analyzerutil.MustExtractDoc(doc, "fmtappendf"),
 	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
 		inspect.Analyzer,
 		typeindexanalyzer.Analyzer,
 	},
@@ -36,8 +34,6 @@ var FmtAppendfAnalyzer = &analysis.Analyzer{
 // The fmtappend function replaces []byte(fmt.Sprintf(...)) by
 // fmt.Appendf(nil, ...), and similarly for Sprint, Sprintln.
 func fmtappendf(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	index := pass.ResultOf[typeindexanalyzer.Analyzer].(*typeindex.Index)
 	for _, fn := range []types.Object{
 		index.Object("fmt", "Sprintf"),

go/analysis/passes/modernize/forvar.go

@@ -11,21 +11,17 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/refactor"
 	"golang.org/x/tools/internal/versions"
 )
 
 var ForVarAnalyzer = &analysis.Analyzer{
-	Name: "forvar",
-	Doc:  analyzerutil.MustExtractDoc(doc, "forvar"),
-	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
-		inspect.Analyzer,
-	},
-	Run: forvar,
-	URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#forvar",
+	Name:     "forvar",
+	Doc:      analyzerutil.MustExtractDoc(doc, "forvar"),
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      forvar,
+	URL:      "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#forvar",
 }
 
 // forvar offers to fix unnecessary copying of a for variable
@@ -43,8 +39,6 @@ var ForVarAnalyzer = &analysis.Analyzer{
 // is declared implicitly before executing the post statement and initialized to the
 // value of the previous iteration's variable at that moment.")
 func forvar(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	for curFile := range filesUsingGoVersion(pass, versions.Go1_22) {
 		for curLoop := range curFile.Preorder((*ast.RangeStmt)(nil)) {
 			loop := curLoop.Node().(*ast.RangeStmt)

go/analysis/passes/modernize/maps.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/refactor"
 	"golang.org/x/tools/internal/typeparams"
@@ -25,14 +24,11 @@ import (
 )
 
 var MapsLoopAnalyzer = &analysis.Analyzer{
-	Name: "mapsloop",
-	Doc:  analyzerutil.MustExtractDoc(doc, "mapsloop"),
-	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
-		inspect.Analyzer,
-	},
-	Run: mapsloop,
-	URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#mapsloop",
+	Name:     "mapsloop",
+	Doc:      analyzerutil.MustExtractDoc(doc, "mapsloop"),
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      mapsloop,
+	URL:      "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#mapsloop",
 }
 
 // The mapsloop pass offers to simplify a loop of map insertions:
@@ -56,8 +52,6 @@ var MapsLoopAnalyzer = &analysis.Analyzer{
 //	m = make(M)
 //	m = M{}
 func mapsloop(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	// Skip the analyzer in packages where its
 	// fixes would create an import cycle.
 	if within(pass, "maps", "bytes", "runtime") {

go/analysis/passes/modernize/minmax.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/tools/go/ast/edge"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	typeindexanalyzer "golang.org/x/tools/internal/analysis/typeindex"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/typeparams"
@@ -28,7 +27,6 @@ var MinMaxAnalyzer = &analysis.Analyzer{
 	Name: "minmax",
 	Doc:  analyzerutil.MustExtractDoc(doc, "minmax"),
 	Requires: []*analysis.Analyzer{
-		generated.Analyzer,
 		inspect.Analyzer,
 		typeindexanalyzer.Analyzer,
 	},
@@ -57,8 +55,6 @@ var MinMaxAnalyzer = &analysis.Analyzer{
 // - "x := a" or "x = a" or "var x = a" in pattern 2
 // - "x < b" or "a < b" in pattern 2
 func minmax(pass *analysis.Pass) (any, error) {
-	skipGenerated(pass)
-
 	// Check for user-defined min/max functions that can be removed
 	checkUserDefinedMinMax(pass)
 

go/analysis/passes/modernize/modernize.go

@@ -20,7 +20,6 @@ import (
 	"golang.org/x/tools/go/ast/edge"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/internal/analysis/analyzerutil"
-	"golang.org/x/tools/internal/analysis/generated"
 	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/moreiters"
 	"golang.org/x/tools/internal/packagepath"
@@ -59,18 +58,6 @@ var Suite = []*analysis.Analyzer{
 
 // -- helpers --
 
-// skipGenerated decorates pass.Report to suppress diagnostics in generated files.
-func skipGenerated(pass *analysis.Pass) {
-	report := pass.Report
-	pass.Report = func(diag analysis.Diagnostic) {
-		generated := pass.ResultOf[generated.Analyzer].(*generated.Result)
-		if generated.IsGenerated(diag.Pos) {
-			return // skip
-		}
-		report(diag)
-	}
-}
-
 // formatExprs formats a comma-separated list of expressions.
 func formatExprs(fset *token.FileSet, exprs []ast.Expr) string {
 	var buf strings.Builder