gopls/internal/analysis/modernize: replace interface{} with any

Also, modernize our tests in this manner.

Also, remove existing "useany" analyzer, which is subsumed by
this one. The fact that useany was off by default suggest that
perhaps modernize may need to be as well; but perhaps severity=INFO
will suffice.

Updates golang/go#70815

Change-Id: Iba1662993fb8a1c50b2d843ad9425bbddafc927f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/636276
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/doc/analyzers.md

@@ -444,9 +444,10 @@ This analyzer reports opportunities for simplifying and clarifying
 existing code by using more modern features of Go, such as:
 
   - replacing if/else conditional assignments by a call to the
-    built-in min or max functions.
+    built-in min or max functions added in go1.21;
   - replacing sort.Slice(x, func(i, j int) bool) { return s[i] < s[j] }
-    by slices.Sort(s).
+    by a call to slices.Sort(s), added in go1.21;
+  - replacing interface{} by the 'any' type added in go1.18;
 
 Default: on.
 
@@ -983,15 +984,6 @@ Default: on.
 
 Package documentation: [unusedwrite](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/unusedwrite)
 
-<a id='useany'></a>
-## `useany`: check for constraints that could be simplified to "any"
-
-
-
-Default: off. Enable by setting `"analyses": {"useany": true}`.
-
-Package documentation: [useany](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/useany)
-
 <a id='waitgroup'></a>
 ## `waitgroup`: check for misuses of sync.WaitGroup
 

gopls/doc/features/diagnostics.md

@@ -299,7 +299,7 @@ dorky details and deletia:
 
 - **Experimental analyzers**. Gopls has some analyzers that are not
   enabled by default, because they produce too high a rate of false
-  positives. For example, fieldalignment, shadow, useany.
+  positives. For example, fieldalignment, shadow.
 
 Note: fillstruct is not a real analyzer.
 

gopls/internal/analysis/modernize/doc.go

@@ -12,7 +12,8 @@
 // existing code by using more modern features of Go, such as:
 //
 //   - replacing if/else conditional assignments by a call to the
-//     built-in min or max functions.
+//     built-in min or max functions added in go1.21;
 //   - replacing sort.Slice(x, func(i, j int) bool) { return s[i] < s[j] }
-//     by slices.Sort(s).
+//     by a call to slices.Sort(s), added in go1.21;
+//   - replacing interface{} by the 'any' type added in go1.18;
 package modernize

gopls/internal/analysis/modernize/efaceany.go

@@ -0,0 +1,48 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+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'.
+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
+			}
+
+			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"),
+						},
+					},
+				}},
+			})
+		}
+	}
+}

gopls/internal/analysis/modernize/modernize.go

@@ -30,12 +30,34 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
+	// Decorate pass.Report to suppress diagnostics in generated files.
+	//
+	// TODO(adonovan): opt: do this more efficiently by interleaving
+	// the micro-passes (as described below) and preemptively skipping
+	// the entire subtree for each generated *ast.File.
+	{
+		// Gather information whether file is generated or not.
+		generated := make(map[*token.File]bool)
+		for _, file := range pass.Files {
+			if ast.IsGenerated(file) {
+				generated[pass.Fset.File(file.FileStart)] = true
+			}
+		}
+		report := pass.Report
+		pass.Report = func(diag analysis.Diagnostic) {
+			if _, ok := generated[pass.Fset.File(diag.Pos)]; ok {
+				return // skip checking if it's generated code
+			}
+			report(diag)
+		}
+	}
+
 	minmax(pass)
 	sortslice(pass)
+	efaceany(pass)
 
 	// TODO(adonovan): more modernizers here; see #70815.
-	// Consider interleaving passes with the same inspection
-	// criteria (e.g. CallExpr).
+	// TODO(adonovan): opt: interleave these micro-passes within a single inspection.
 
 	return nil, nil
 }

gopls/internal/analysis/modernize/modernize_test.go

@@ -14,5 +14,6 @@ import (
 func Test(t *testing.T) {
 	analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), modernize.Analyzer,
 		"minmax",
-		"sortslice")
+		"sortslice",
+		"efaceany")
 }

gopls/internal/analysis/modernize/testdata/src/efaceany/efaceany.go

@@ -0,0 +1,10 @@
+package efaceany
+
+func _(x interface{}) {} // want "interface{} can be replaced by any"
+
+func _() {
+	var x interface{} // want "interface{} can be replaced by any"
+	const any = 1
+	var y interface{} // nope: any is shadowed here
+	_, _ = x, y
+}

gopls/internal/analysis/modernize/testdata/src/efaceany/efaceany.go.golden

@@ -0,0 +1,10 @@
+package efaceany
+
+func _(x any) {} // want "interface{} can be replaced by any"
+
+func _() {
+	var x any // want "interface{} can be replaced by any"
+	const any = 1
+	var y interface{} // nope: any is shadowed here
+	_, _ = x, y
+}

gopls/internal/analysis/useany/testdata/src/a/a.go

@@ -467,7 +467,7 @@
 						},
 						{
 							"Name": "\"modernize\"",
-							"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n  - replacing if/else conditional assignments by a call to the\n    built-in min or max functions.\n  - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n    by slices.Sort(s).",
+							"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n  - replacing if/else conditional assignments by a call to the\n    built-in min or max functions added in go1.21;\n  - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n    by a call to slices.Sort(s), added in go1.21;\n  - replacing interface{} by the 'any' type added in go1.18;",
 							"Default": "true"
 						},
 						{
@@ -605,11 +605,6 @@
 							"Doc": "checks for unused writes\n\nThe analyzer reports instances of writes to struct fields and\narrays that are never read. Specifically, when a struct object\nor an array is copied, its elements are copied implicitly by\nthe compiler, and any element write to this copy does nothing\nwith the original object.\n\nFor example:\n\n\ttype T struct { x int }\n\n\tfunc f(input []T) {\n\t\tfor i, v := range input {  // v is a copy\n\t\t\tv.x = i  // unused write to field x\n\t\t}\n\t}\n\nAnother example is about non-pointer receiver:\n\n\ttype T struct { x int }\n\n\tfunc (t T) f() {  // t is a copy\n\t\tt.x = i  // unused write to field x\n\t}",
 							"Default": "true"
 						},
-						{
-							"Name": "\"useany\"",
-							"Doc": "check for constraints that could be simplified to \"any\"",
-							"Default": "false"
-						},
 						{
 							"Name": "\"waitgroup\"",
 							"Doc": "check for misuses of sync.WaitGroup\n\nThis analyzer detects mistaken calls to the (*sync.WaitGroup).Add\nmethod from inside a new goroutine, causing Add to race with Wait:\n\n\t// WRONG\n\tvar wg sync.WaitGroup\n\tgo func() {\n\t        wg.Add(1) // \"WaitGroup.Add called from inside new goroutine\"\n\t        defer wg.Done()\n\t        ...\n\t}()\n\twg.Wait() // (may return prematurely before new goroutine starts)\n\nThe correct code calls Add before starting the goroutine:\n\n\t// RIGHT\n\tvar wg sync.WaitGroup\n\twg.Add(1)\n\tgo func() {\n\t\tdefer wg.Done()\n\t\t...\n\t}()\n\twg.Wait()",
@@ -1138,7 +1133,7 @@
 		},
 		{
 			"Name": "modernize",
-			"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n  - replacing if/else conditional assignments by a call to the\n    built-in min or max functions.\n  - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n    by slices.Sort(s).",
+			"Doc": "simplify code by using modern constructs\n\nThis analyzer reports opportunities for simplifying and clarifying\nexisting code by using more modern features of Go, such as:\n\n  - replacing if/else conditional assignments by a call to the\n    built-in min or max functions added in go1.21;\n  - replacing sort.Slice(x, func(i, j int) bool) { return s[i] \u003c s[j] }\n    by a call to slices.Sort(s), added in go1.21;\n  - replacing interface{} by the 'any' type added in go1.18;",
 			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize",
 			"Default": true
 		},
@@ -1304,12 +1299,6 @@
 			"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/unusedwrite",
 			"Default": true
 		},
-		{
-			"Name": "useany",
-			"Doc": "check for constraints that could be simplified to \"any\"",
-			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/useany",
-			"Default": false
-		},
 		{
 			"Name": "waitgroup",
 			"Doc": "check for misuses of sync.WaitGroup\n\nThis analyzer detects mistaken calls to the (*sync.WaitGroup).Add\nmethod from inside a new goroutine, causing Add to race with Wait:\n\n\t// WRONG\n\tvar wg sync.WaitGroup\n\tgo func() {\n\t        wg.Add(1) // \"WaitGroup.Add called from inside new goroutine\"\n\t        defer wg.Done()\n\t        ...\n\t}()\n\twg.Wait() // (may return prematurely before new goroutine starts)\n\nThe correct code calls Add before starting the goroutine:\n\n\t// RIGHT\n\tvar wg sync.WaitGroup\n\twg.Add(1)\n\tgo func() {\n\t\tdefer wg.Done()\n\t\t...\n\t}()\n\twg.Wait()",

gopls/internal/analysis/useany/testdata/src/a/a.go.golden

@@ -58,7 +58,6 @@ import (
 	"golang.org/x/tools/gopls/internal/analysis/simplifyslice"
 	"golang.org/x/tools/gopls/internal/analysis/unusedparams"
 	"golang.org/x/tools/gopls/internal/analysis/unusedvariable"
-	"golang.org/x/tools/gopls/internal/analysis/useany"
 	"golang.org/x/tools/gopls/internal/analysis/yield"
 	"golang.org/x/tools/gopls/internal/protocol"
 )
@@ -162,7 +161,6 @@ func init() {
 
 		// disabled due to high false positives
 		{analyzer: shadow.Analyzer, enabled: false}, // very noisy
-		{analyzer: useany.Analyzer, enabled: false}, // never a bug
 		// fieldalignment is not even off-by-default; see #67762.
 
 		// "simplifiers": analyzers that offer mere style fixes