Skip to content

Commit ee8f138

Browse files
committed
go/analysis/passes/gofix: go:fix directive checker
Add an analyzer that checks the validity of go:fix directives. To avoid pulling the inlining framework into go vet, we split the gofix analyzers into three packages. The internal/gofix/findgofix package finds each `go:fix inline` directive, checks it for errors, and invokes a callback. The internal/gofix package uses findgofix with callbacks that register facts, and then does the inlining. The go/analysis/passes/gofix package uses findgofix to find errors, but takes no other action. vet_std_test passes, indicating that there were no findings in the standard library. A followup CL on the go repo will vendor x/tools at head and add this analyzer to go vet. Change-Id: Ib9ebe38cc719d7b3d85beccd76a9bedf8a2cd077 Reviewed-on: https://go-review.googlesource.com/c/tools/+/664176 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
1 parent 1494dfe commit ee8f138

File tree

9 files changed

+318
-147
lines changed

9 files changed

+318
-147
lines changed

go/analysis/passes/gofix/doc.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
/*
6+
Package gofix defines an Analyzer that checks "//go:fix inline" directives.
7+
See golang.org/x/tools/internal/gofix/doc.go for details.
8+
9+
# Analyzer gofixdirective
10+
11+
gofixdirective: validate uses of gofix comment directives
12+
13+
The gofixdirective analyzer checks "//go:fix inline" directives for correctness.
14+
15+
The proposal https://go.dev/issue/32816 introduces the "//go:fix" directives.
16+
17+
The analyzer checks for the following issues:
18+
19+
- A constant definition can be marked for inlining only if it refers to another
20+
named constant.
21+
22+
//go:fix inline
23+
const (
24+
a = 1 // error
25+
b = iota // error
26+
c = math.Pi // OK
27+
)
28+
29+
- A type definition can be marked for inlining only if it is an alias.
30+
31+
//go:fix inline
32+
type (
33+
T int // error
34+
A = int // OK
35+
)
36+
37+
- An alias whose right-hand side contains a non-literal array size
38+
cannot be marked for inlining.
39+
40+
const two = 2
41+
42+
//go:fix inline
43+
type (
44+
A = []int // OK
45+
B = [1]int // OK
46+
C = [two]int // error
47+
)
48+
*/
49+
package gofix

go/analysis/passes/gofix/gofix.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package gofix defines an analyzer that checks go:fix directives.
6+
package gofix
7+
8+
import (
9+
_ "embed"
10+
11+
"golang.org/x/tools/go/analysis"
12+
"golang.org/x/tools/go/analysis/passes/inspect"
13+
"golang.org/x/tools/go/ast/inspector"
14+
"golang.org/x/tools/internal/analysisinternal"
15+
"golang.org/x/tools/internal/astutil/cursor"
16+
"golang.org/x/tools/internal/gofix/findgofix"
17+
)
18+
19+
//go:embed doc.go
20+
var doc string
21+
22+
var Analyzer = &analysis.Analyzer{
23+
Name: "gofixdirective",
24+
Doc: analysisinternal.MustExtractDoc(doc, "gofixdirective"),
25+
URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/gofix",
26+
Run: run,
27+
Requires: []*analysis.Analyzer{inspect.Analyzer},
28+
}
29+
30+
func run(pass *analysis.Pass) (any, error) {
31+
root := cursor.Root(pass.ResultOf[inspect.Analyzer].(*inspector.Inspector))
32+
findgofix.Find(pass, root, nil)
33+
return nil, nil
34+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package gofix_test
6+
7+
import (
8+
"testing"
9+
10+
"golang.org/x/tools/go/analysis/analysistest"
11+
"golang.org/x/tools/go/analysis/passes/gofix"
12+
)
13+
14+
func Test(t *testing.T) {
15+
testdata := analysistest.TestData()
16+
analysistest.Run(t, testdata, gofix.Analyzer, "a")
17+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2023 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// This file contains tests for the gofix checker.
6+
7+
package a
8+
9+
const one = 1
10+
11+
//go:fix inline
12+
const (
13+
in3 = one
14+
in4 = one
15+
bad1 = 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
16+
)
17+
18+
//go:fix inline
19+
const in5,
20+
in6,
21+
bad2 = one, one,
22+
one + 1 // want `invalid //go:fix inline directive: const value is not the name of another constant`
23+
24+
//go:fix inline
25+
const (
26+
a = iota // want `invalid //go:fix inline directive: const value is iota`
27+
b
28+
in7 = one
29+
)
30+
31+
func shadow() {
32+
//go:fix inline
33+
const a = iota // want `invalid //go:fix inline directive: const value is iota`
34+
35+
const iota = 2
36+
37+
//go:fix inline
38+
const b = iota // not an error: iota is not the builtin
39+
}
40+
41+
// Type aliases
42+
43+
//go:fix inline
44+
type A int // want `invalid //go:fix inline directive: not a type alias`
45+
46+
//go:fix inline
47+
type E = map[[one]string][]int // want `invalid //go:fix inline directive: array types not supported`

go/analysis/unitchecker/vet_std_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"golang.org/x/tools/go/analysis/passes/directive"
2525
"golang.org/x/tools/go/analysis/passes/errorsas"
2626
"golang.org/x/tools/go/analysis/passes/framepointer"
27+
"golang.org/x/tools/go/analysis/passes/gofix"
2728
"golang.org/x/tools/go/analysis/passes/httpresponse"
2829
"golang.org/x/tools/go/analysis/passes/ifaceassert"
2930
"golang.org/x/tools/go/analysis/passes/loopclosure"
@@ -62,6 +63,7 @@ func vet() {
6263
directive.Analyzer,
6364
errorsas.Analyzer,
6465
framepointer.Analyzer,
66+
gofix.Analyzer,
6567
httpresponse.Analyzer,
6668
ifaceassert.Analyzer,
6769
loopclosure.Analyzer,

internal/gofix/doc.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
Package gofix defines an Analyzer that inlines calls to functions
77
and uses of constants
88
marked with a "//go:fix inline" directive.
9-
A second analyzer only checks uses of the directive.
109
1110
# Analyzer gofix
1211

internal/gofix/findgofix/findgofix.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package findgofix searches for and validates go:fix directives. The
6+
// internal/gofix package uses findgofix to perform inlining.
7+
// The go/analysis/passes/gofix package uses findgofix to check for problems
8+
// with go:fix directives.
9+
//
10+
// findgofix is separate from gofix to avoid depending on refactor/inline,
11+
// which is large.
12+
package findgofix
13+
14+
// This package is tested by internal/gofix.
15+
16+
import (
17+
"go/ast"
18+
"go/token"
19+
"go/types"
20+
21+
"golang.org/x/tools/go/analysis"
22+
internalastutil "golang.org/x/tools/internal/astutil"
23+
"golang.org/x/tools/internal/astutil/cursor"
24+
)
25+
26+
// A Handler handles language entities with go:fix directives.
27+
type Handler interface {
28+
HandleFunc(*ast.FuncDecl)
29+
HandleAlias(*ast.TypeSpec)
30+
HandleConst(name, rhs *ast.Ident)
31+
}
32+
33+
// Find finds functions and constants annotated with an appropriate "//go:fix"
34+
// comment (the syntax proposed by #32816), and calls handler methods for each one.
35+
// h may be nil.
36+
func Find(pass *analysis.Pass, root cursor.Cursor, h Handler) {
37+
for cur := range root.Preorder((*ast.FuncDecl)(nil), (*ast.GenDecl)(nil)) {
38+
switch decl := cur.Node().(type) {
39+
case *ast.FuncDecl:
40+
findFunc(decl, h)
41+
42+
case *ast.GenDecl:
43+
if decl.Tok != token.CONST && decl.Tok != token.TYPE {
44+
continue
45+
}
46+
declInline := hasFixInline(decl.Doc)
47+
// Accept inline directives on the entire decl as well as individual specs.
48+
for _, spec := range decl.Specs {
49+
switch spec := spec.(type) {
50+
case *ast.TypeSpec: // Tok == TYPE
51+
findAlias(pass, spec, declInline, h)
52+
53+
case *ast.ValueSpec: // Tok == CONST
54+
findConst(pass, spec, declInline, h)
55+
}
56+
}
57+
}
58+
}
59+
}
60+
61+
func findFunc(decl *ast.FuncDecl, h Handler) {
62+
if !hasFixInline(decl.Doc) {
63+
return
64+
}
65+
if h != nil {
66+
h.HandleFunc(decl)
67+
}
68+
}
69+
70+
func findAlias(pass *analysis.Pass, spec *ast.TypeSpec, declInline bool, h Handler) {
71+
if !declInline && !hasFixInline(spec.Doc) {
72+
return
73+
}
74+
if !spec.Assign.IsValid() {
75+
pass.Reportf(spec.Pos(), "invalid //go:fix inline directive: not a type alias")
76+
return
77+
}
78+
79+
// Disallow inlines of type expressions containing array types.
80+
// Given an array type like [N]int where N is a named constant, go/types provides
81+
// only the value of the constant as an int64. So inlining A in this code:
82+
//
83+
// const N = 5
84+
// type A = [N]int
85+
//
86+
// would result in [5]int, breaking the connection with N.
87+
for n := range ast.Preorder(spec.Type) {
88+
if ar, ok := n.(*ast.ArrayType); ok && ar.Len != nil {
89+
// Make an exception when the array length is a literal int.
90+
if lit, ok := ast.Unparen(ar.Len).(*ast.BasicLit); ok && lit.Kind == token.INT {
91+
continue
92+
}
93+
pass.Reportf(spec.Pos(), "invalid //go:fix inline directive: array types not supported")
94+
return
95+
}
96+
}
97+
if h != nil {
98+
h.HandleAlias(spec)
99+
}
100+
}
101+
102+
func findConst(pass *analysis.Pass, spec *ast.ValueSpec, declInline bool, h Handler) {
103+
specInline := hasFixInline(spec.Doc)
104+
if declInline || specInline {
105+
for i, nameIdent := range spec.Names {
106+
if i >= len(spec.Values) {
107+
// Possible following an iota.
108+
break
109+
}
110+
var rhsIdent *ast.Ident
111+
switch val := spec.Values[i].(type) {
112+
case *ast.Ident:
113+
// Constants defined with the predeclared iota cannot be inlined.
114+
if pass.TypesInfo.Uses[val] == builtinIota {
115+
pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is iota")
116+
return
117+
}
118+
rhsIdent = val
119+
case *ast.SelectorExpr:
120+
rhsIdent = val.Sel
121+
default:
122+
pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is not the name of another constant")
123+
return
124+
}
125+
if h != nil {
126+
h.HandleConst(nameIdent, rhsIdent)
127+
}
128+
}
129+
}
130+
}
131+
132+
// hasFixInline reports the presence of a "//go:fix inline" directive
133+
// in the comments.
134+
func hasFixInline(cg *ast.CommentGroup) bool {
135+
for _, d := range internalastutil.Directives(cg) {
136+
if d.Tool == "go" && d.Name == "fix" && d.Args == "inline" {
137+
return true
138+
}
139+
}
140+
return false
141+
}
142+
143+
var builtinIota = types.Universe.Lookup("iota")

0 commit comments

Comments
 (0)