gopls/internal/analysis/modernize: check FileVersions throughout

This CL ensures that every modernizer sub-pass checks the effective
Go version of each file before inspecting that file, using
a version-filtering *ast.File iterator, something enabled by
Cursors.

It does increase indentation by a level though, so I split
some of the deeper function into outer and inner parts.

Change-Id: I2bcd059c4f52af5673a7d1e9f07ff9463dce96b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/638698
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/analysis/modernize/bloop.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/tools/go/types/typeutil"
 	"golang.org/x/tools/internal/astutil/cursor"
 	"golang.org/x/tools/internal/typesinternal"
-	"golang.org/x/tools/internal/versions"
 )
 
 // bloop updates benchmarks that use "for range b.N", replacing it
@@ -82,19 +81,13 @@ func bloop(pass *analysis.Pass) {
 
 	// Find all for/range statements.
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	filter := []ast.Node{
-		(*ast.File)(nil),
+	loops := []ast.Node{
 		(*ast.ForStmt)(nil),
 		(*ast.RangeStmt)(nil),
 	}
-	cursor.Root(inspect).Inspect(filter, func(cur cursor.Cursor, push bool) (descend bool) {
-		if push {
-			switch n := cur.Node().(type) {
-			case *ast.File:
-				if versions.Before(info.FileVersions[n], "go1.24") {
-					return false
-				}
-
+	for curFile := range filesUsing(inspect, info, "go1.24") {
+		for curLoop := range curFile.Preorder(loops...) {
+			switch n := curLoop.Node().(type) {
 			case *ast.ForStmt:
 				// for _; i < b.N; _ {}
 				if cmp, ok := n.Cond.(*ast.BinaryExpr); ok && cmp.Op == token.LSS {
@@ -108,7 +101,7 @@ func bloop(pass *analysis.Pass) {
 						//  for i := 0; i < b.N; i++ {
 						//    ...no references to i...
 						//  }
-						body, _ := cur.LastChild()
+						body, _ := curLoop.LastChild()
 						if assign, ok := n.Init.(*ast.AssignStmt); ok &&
 							assign.Tok == token.DEFINE &&
 							len(assign.Rhs) == 1 &&
@@ -129,7 +122,7 @@ func bloop(pass *analysis.Pass) {
 							Message:  "b.N can be modernized using b.Loop()",
 							SuggestedFixes: []analysis.SuggestedFix{{
 								Message:   "Replace b.N with b.Loop()",
-								TextEdits: edits(cur, sel.X, delStart, delEnd),
+								TextEdits: edits(curLoop, sel.X, delStart, delEnd),
 							}},
 						})
 					}
@@ -153,14 +146,13 @@ func bloop(pass *analysis.Pass) {
 						Message:  "b.N can be modernized using b.Loop()",
 						SuggestedFixes: []analysis.SuggestedFix{{
 							Message:   "Replace b.N with b.Loop()",
-							TextEdits: edits(cur, sel.X, n.Range, n.X.End()),
+							TextEdits: edits(curLoop, sel.X, n.Range, n.X.End()),
 						}},
 					})
 				}
 			}
 		}
-		return true
-	})
+	}
 }
 
 // isPtrToNamed reports whether t is type "*pkgpath.Name".

gopls/internal/analysis/modernize/efaceany.go

@@ -6,43 +6,45 @@ package modernize
 
 import (
 	"go/ast"
-	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 )
 
-// The efaceany pass replaces interface{} with 'any'.
+// The efaceany pass replaces interface{} with go1.18's 'any'.
 func efaceany(pass *analysis.Pass) {
-	// TODO(adonovan): opt: combine all these micro-passes into a single traversal.
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	for iface := range inspector.All[*ast.InterfaceType](inspect) {
-		if iface.Methods.NumFields() == 0 {
 
-			// TODO(adonovan): opt: record the enclosing file as we go.
-			f := enclosingFile(pass, iface.Pos())
-			scope := pass.TypesInfo.Scopes[f].Innermost(iface.Pos())
-			if _, obj := scope.LookupParent("any", iface.Pos()); obj != types.Universe.Lookup("any") {
-				continue // 'any' is shadowed
-			}
+	for curFile := range filesUsing(inspect, pass.TypesInfo, "go1.18") {
+		file := curFile.Node().(*ast.File)
+
+		for curIface := range curFile.Preorder((*ast.InterfaceType)(nil)) {
+			iface := curIface.Node().(*ast.InterfaceType)
 
-			pass.Report(analysis.Diagnostic{
-				Pos:      iface.Pos(),
-				End:      iface.End(),
-				Category: "efaceany",
-				Message:  "interface{} can be replaced by any",
-				SuggestedFixes: []analysis.SuggestedFix{{
-					Message: "Replace interface{} by any",
-					TextEdits: []analysis.TextEdit{
-						{
-							Pos:     iface.Pos(),
-							End:     iface.End(),
-							NewText: []byte("any"),
-						},
-					},
-				}},
-			})
+			if iface.Methods.NumFields() == 0 {
+				// Check that 'any' is not shadowed.
+				// TODO(adonovan): find scope using only local Cursor operations.
+				scope := pass.TypesInfo.Scopes[file].Innermost(iface.Pos())
+				if _, obj := scope.LookupParent("any", iface.Pos()); obj == builtinAny {
+					pass.Report(analysis.Diagnostic{
+						Pos:      iface.Pos(),
+						End:      iface.End(),
+						Category: "efaceany",
+						Message:  "interface{} can be replaced by any",
+						SuggestedFixes: []analysis.SuggestedFix{{
+							Message: "Replace interface{} by any",
+							TextEdits: []analysis.TextEdit{
+								{
+									Pos:     iface.Pos(),
+									End:     iface.End(),
+									NewText: []byte("any"),
+								},
+							},
+						}},
+					})
+				}
+			}
 		}
 	}
 }