gopls/internal/analysis/gofix: add vet analyzer

Add a second analyzer that checks for valid go:fix inline directives
without suggesting changes.

Change-Id: I0b9ad3da79f554caef01dda66ef954c59718015d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/651656
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: jba

gopls/internal/analysis/gofix/doc.go

@@ -5,7 +5,8 @@
 /*
 Package gofix defines an Analyzer that inlines calls to functions
 and uses of constants
-marked with a "//go:fix inline" doc comment.
+marked with a "//go:fix inline" directive.
+A second analyzer only checks uses of the directive.
 
 # Analyzer gofix
 
@@ -81,5 +82,12 @@ The proposal https://go.dev/issue/32816 introduces the "//go:fix" directives.
 You can use this (officially unsupported) command to apply gofix fixes en masse:
 
 	$ go run golang.org/x/tools/gopls/internal/analysis/gofix/cmd/gofix@latest -test ./...
+
+# Analyzer gofixdirective
+
+gofixdirective: validate uses of gofix comment directives
+
+The gofixdirective analyzer checks "//go:fix inline" directives for correctness.
+See the documentation for the gofix analyzer for more about "/go:fix inline".
 */
 package gofix

gopls/internal/analysis/gofix/gofix.go

@@ -34,7 +34,20 @@ var Analyzer = &analysis.Analyzer{
 	Name: "gofix",
 	Doc:  analysisinternal.MustExtractDoc(doc, "gofix"),
 	URL:  "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/gofix",
-	Run:  run,
+	Run:  func(pass *analysis.Pass) (any, error) { return run(pass, true) },
+	FactTypes: []analysis.Fact{
+		(*goFixInlineFuncFact)(nil),
+		(*goFixInlineConstFact)(nil),
+		(*goFixInlineAliasFact)(nil),
+	},
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+}
+
+var DirectiveAnalyzer = &analysis.Analyzer{
+	Name: "gofixdirective",
+	Doc:  analysisinternal.MustExtractDoc(doc, "gofixdirective"),
+	URL:  "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/gofix",
+	Run:  func(pass *analysis.Pass) (any, error) { return run(pass, false) },
 	FactTypes: []analysis.Fact{
 		(*goFixInlineFuncFact)(nil),
 		(*goFixInlineConstFact)(nil),
@@ -46,6 +59,7 @@ var Analyzer = &analysis.Analyzer{
 // analyzer holds the state for this analysis.
 type analyzer struct {
 	pass *analysis.Pass
+	fix  bool // only suggest fixes if true; else, just check directives
 	root cursor.Cursor
 	// memoization of repeated calls for same file.
 	fileContent map[string][]byte
@@ -55,9 +69,10 @@ type analyzer struct {
 	inlinableAliases map[*types.TypeName]*goFixInlineAliasFact
 }
 
-func run(pass *analysis.Pass) (any, error) {
+func run(pass *analysis.Pass, fix bool) (any, error) {
 	a := &analyzer{
 		pass:             pass,
+		fix:              fix,
 		root:             cursor.Root(pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)),
 		fileContent:      make(map[string][]byte),
 		inlinableFuncs:   make(map[*types.Func]*inline.Callee),
@@ -256,6 +271,10 @@ func (a *analyzer) inlineCall(call *ast.CallExpr, cur cursor.Cursor) {
 			a.pass.Reportf(call.Lparen, "%v", err)
 			return
 		}
+		if !a.fix {
+			return
+		}
+
 		if res.Literalized {
 			// Users are not fond of inlinings that literalize
 			// f(x) to func() { ... }(), so avoid them.
@@ -533,6 +552,9 @@ func (a *analyzer) inlineConst(con *types.Const, cur cursor.Cursor) {
 
 // reportInline reports a diagnostic for fixing an inlinable name.
 func (a *analyzer) reportInline(kind, capKind string, ident ast.Expr, edits []analysis.TextEdit, newText string) {
+	if !a.fix {
+		return
+	}
 	edits = append(edits, analysis.TextEdit{
 		Pos:     ident.Pos(),
 		End:     ident.End(),

gopls/internal/analysis/gofix/gofix_test.go

@@ -22,6 +22,11 @@ func TestAnalyzer(t *testing.T) {
 	analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), Analyzer, "a", "b")
 }
 
+func TestDirectiveAnalyzer(t *testing.T) {
+	analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), DirectiveAnalyzer, "directive")
+
+}
+
 func TestTypesWithNames(t *testing.T) {
 	// Test setup inspired by internal/analysisinternal/addimport_test.go.
 	testenv.NeedsDefaultImporter(t)

gopls/internal/analysis/gofix/testdata/src/directive/directive.go

@@ -0,0 +1,63 @@
+package directive
+
+// Functions.
+
+func f() {
+	One()
+
+	new(T).Two()
+}
+
+type T struct{}
+
+//go:fix inline
+func One() int { return one } // want One:`goFixInline directive.One`
+
+const one = 1
+
+//go:fix inline
+func (T) Two() int { return 2 } // want Two:`goFixInline \(directive.T\).Two`
+
+// Constants.
+
+const Uno = 1
+
+//go:fix inline
+const In1 = Uno // want In1: `goFixInline const "directive".Uno`
+
+const (
+	no1 = one
+
+	//go:fix inline
+	In2 = one // want In2: `goFixInline const "directive".one`
+)
+
+//go:fix inline
+const bad1 = 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
+
+//go:fix inline
+const in5,
+	in6,
+	bad2 = one, one,
+	one + 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
+
+// Make sure we don't crash on iota consts, but still process the whole decl.
+//
+//go:fix inline
+const (
+	a = iota // want `invalid //go:fix inline directive: const value is iota`
+	b
+	in7 = one
+)
+
+const (
+	x = 1
+	//go:fix inline
+	in8 = x
+)
+
+//go:fix inline
+const in9 = iota // want `invalid //go:fix inline directive: const value is iota`
+
+//go:fix inline
+type E = map[[Uno]string][]*T // want `invalid //go:fix inline directive: array types not supported`

gopls/internal/analysis/gofix/testdata/src/directive/directive.go.golden

@@ -0,0 +1,71 @@
+package golden
+
+import "a/internal"
+
+// Functions.
+
+func f() {
+	One()
+
+	new(T).Two()
+}
+
+type T struct{}
+
+//go:fix inline
+func One() int { return one }
+
+const one = 1
+
+//go:fix inline
+func (T) Two() int { return 2 }
+
+// Constants.
+
+const Uno = 1
+
+//go:fix inline
+const In1 = Uno // want In1: `goFixInline const "a".Uno`
+
+const (
+	no1 = one
+
+	//go:fix inline
+	In2 = one // want In2: `goFixInline const "a".one`
+)
+
+//go:fix inline
+const bad1 = 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
+
+//go:fix inline
+const in5,
+	in6,
+	bad2 = one, one,
+	one + 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
+
+// Make sure we don't crash on iota consts, but still process the whole decl.
+//
+//go:fix inline
+const (
+	a = iota // want `invalid //go:fix inline directive: const value is iota`
+	b
+	in7 = one
+)
+
+const (
+	x = 1
+	//go:fix inline
+	in8 = x
+)
+
+//go:fix inline
+const a = iota // want `invalid //go:fix inline directive: const value is iota`
+
+//go:fix inline
+type E = map[[Uno]string][]*T // want `invalid //go:fix inline directive: array types not supported`
+
+// literal array lengths are OK
+//
+//go:fix inline
+type EL = map[[2]string][]*T // want EL: `goFixInline alias`
+