Skip to content

Commit 9c087d9

Browse files
committed
internal/analysis/gofix: change "forward" back to "inline"
For golang/go#32816. Change-Id: I02605efe2ca4db4fbef68ae26a57cb793ad5bf56 Reviewed-on: https://go-review.googlesource.com/c/tools/+/647736 Reviewed-by: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 82317ce commit 9c087d9

File tree

9 files changed

+112
-146
lines changed

9 files changed

+112
-146
lines changed

gopls/doc/analyzers.md

+1-2
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,7 @@ Package documentation: [framepointer](https://pkg.go.dev/golang.org/x/tools/go/a
294294
## `gofix`: apply fixes based on go:fix comment directives
295295

296296

297-
The gofix analyzer inlines functions that are marked for inlining
298-
and forwards constants that are marked for forwarding.
297+
The gofix analyzer inlines functions and constants that are marked for inlining.
299298

300299
Default: on.
301300

gopls/doc/release/v0.18.0.md

+4-5
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,8 @@ instead.
7373

7474
## New `gofix` analyzer
7575

76-
Gopls now reports when a function call should be inlined or a use of a constant
77-
should be forwarded.
78-
These diagnostics and the associated code actions are triggered by "//go:fix"
76+
Gopls now reports when a function call or a use of a constant should be inlined.
77+
These diagnostics and the associated code actions are triggered by "//go:fix inline"
7978
directives at the function and constant definitions.
8079
(See [the go:fix proposal](https://go.dev/issue/32816).)
8180

@@ -90,10 +89,10 @@ func Square(x int) int { return Pow(x, 2) }
9089
If gopls sees a call to `intmath.Square` in your code, it will suggest inlining
9190
it, and will offer a code action to do so.
9291

93-
The same feature works for constants, only the directive is "//go:fix forward".
92+
The same feature works for constants.
9493
With a constant definition like this:
9594
```
96-
//go:fix forward
95+
//go:fix inline
9796
const Ptr = Pointer
9897
```
9998
gopls will suggest replacing `Ptr` in your code with `Pointer`.

gopls/internal/analysis/gofix/doc.go

+10-12
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@
44

55
/*
66
Package gofix defines an Analyzer that inlines calls to functions
7-
marked with a "//go:fix inline" doc comment,
8-
and forwards uses of constants
9-
marked with a "//go:fix forward" doc comment.
7+
and uses of constants
8+
marked with a "//go:fix inline" doc comment.
109
1110
# Analyzer gofix
1211
1312
gofix: apply fixes based on go:fix comment directives
1413
15-
The gofix analyzer inlines functions that are marked for inlining
16-
and forwards constants that are marked for forwarding.
14+
The gofix analyzer inlines functions and constants that are marked for inlining.
1715
1816
# Functions
1917
@@ -48,31 +46,31 @@ to enable automatic migration.
4846
4947
# Constants
5048
51-
Given a constant that is marked for forwarding, like this one:
49+
Given a constant that is marked for inlining, like this one:
5250
53-
//go:fix forward
51+
//go:fix inline
5452
const Ptr = Pointer
5553
5654
this analyzer will recommend that uses of Ptr should be replaced with Pointer.
5755
58-
As with inlining, forwarding can be used to replace deprecated constants and
56+
As with functions, inlining can be used to replace deprecated constants and
5957
constants in obsolete packages.
6058
61-
A constant definition can be marked for forwarding only if it refers to another
59+
A constant definition can be marked for inlining only if it refers to another
6260
named constant.
6361
64-
The "//go:fix forward" comment must appear before a single const declaration on its own,
62+
The "//go:fix inline" comment must appear before a single const declaration on its own,
6563
as above; before a const declaration that is part of a group, as in this case:
6664
6765
const (
6866
C = 1
69-
//go:fix forward
67+
//go:fix inline
7068
Ptr = Pointer
7169
)
7270
7371
or before a group, applying to every constant in the group:
7472
75-
//go:fix forward
73+
//go:fix inline
7674
const (
7775
Ptr = Pointer
7876
Val = Value

gopls/internal/analysis/gofix/gofix.go

+35-53
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ var Analyzer = &analysis.Analyzer{
3333
Doc: analysisinternal.MustExtractDoc(doc, "gofix"),
3434
URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/gofix",
3535
Run: run,
36-
FactTypes: []analysis.Fact{new(goFixInlineFuncFact), new(goFixForwardConstFact)},
36+
FactTypes: []analysis.Fact{new(goFixInlineFuncFact), new(goFixInlineConstFact)},
3737
Requires: []*analysis.Analyzer{inspect.Analyzer},
3838
}
3939

@@ -64,19 +64,14 @@ func run(pass *analysis.Pass) (any, error) {
6464
// comment (the syntax proposed by #32816),
6565
// and export a fact for each one.
6666
inlinableFuncs := make(map[*types.Func]*inline.Callee) // memoization of fact import (nil => no fact)
67-
forwardableConsts := make(map[*types.Const]*goFixForwardConstFact)
67+
inlinableConsts := make(map[*types.Const]*goFixInlineConstFact)
6868

6969
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
7070
nodeFilter := []ast.Node{(*ast.FuncDecl)(nil), (*ast.GenDecl)(nil)}
7171
inspect.Preorder(nodeFilter, func(n ast.Node) {
7272
switch decl := n.(type) {
7373
case *ast.FuncDecl:
74-
hasInline, hasForward := fixDirectives(decl.Doc)
75-
if hasForward {
76-
pass.Reportf(decl.Doc.Pos(), "use //go:fix inline for functions")
77-
return
78-
}
79-
if !hasInline {
74+
if !hasFixInline(decl.Doc) {
8075
return
8176
}
8277
content, err := readFile(decl)
@@ -97,20 +92,12 @@ func run(pass *analysis.Pass) (any, error) {
9792
if decl.Tok != token.CONST {
9893
return
9994
}
100-
declInline, declForward := fixDirectives(decl.Doc)
101-
if declInline {
102-
pass.Reportf(decl.Doc.Pos(), "use //go:fix forward for constants")
103-
return
104-
}
105-
// Accept forward directives on the entire decl as well as individual specs.
95+
declInline := hasFixInline(decl.Doc)
96+
// Accept inline directives on the entire decl as well as individual specs.
10697
for _, spec := range decl.Specs {
10798
spec := spec.(*ast.ValueSpec) // guaranteed by Tok == CONST
108-
specInline, specForward := fixDirectives(spec.Doc)
109-
if specInline {
110-
pass.Reportf(spec.Doc.Pos(), "use //go:fix forward for constants")
111-
return
112-
}
113-
if declForward || specForward {
99+
specInline := hasFixInline(spec.Doc)
100+
if declInline || specInline {
114101
for i, name := range spec.Names {
115102
if i >= len(spec.Values) {
116103
// Possible following an iota.
@@ -120,29 +107,29 @@ func run(pass *analysis.Pass) (any, error) {
120107
var rhsID *ast.Ident
121108
switch e := val.(type) {
122109
case *ast.Ident:
123-
// Constants defined with the predeclared iota cannot be forwarded.
110+
// Constants defined with the predeclared iota cannot be inlined.
124111
if pass.TypesInfo.Uses[e] == builtinIota {
125-
pass.Reportf(val.Pos(), "invalid //go:fix forward directive: const value is iota")
112+
pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is iota")
126113
continue
127114
}
128115
rhsID = e
129116
case *ast.SelectorExpr:
130117
rhsID = e.Sel
131118
default:
132-
pass.Reportf(val.Pos(), "invalid //go:fix forward directive: const value is not the name of another constant")
119+
pass.Reportf(val.Pos(), "invalid //go:fix inline directive: const value is not the name of another constant")
133120
continue
134121
}
135122
lhs := pass.TypesInfo.Defs[name].(*types.Const)
136123
rhs := pass.TypesInfo.Uses[rhsID].(*types.Const) // must be so in a well-typed program
137-
con := &goFixForwardConstFact{
124+
con := &goFixInlineConstFact{
138125
RHSName: rhs.Name(),
139126
RHSPkgName: rhs.Pkg().Name(),
140127
RHSPkgPath: rhs.Pkg().Path(),
141128
}
142129
if rhs.Pkg() == pass.Pkg {
143130
con.rhsObj = rhs
144131
}
145-
forwardableConsts[lhs] = con
132+
inlinableConsts[lhs] = con
146133
// Create a fact only if the LHS is exported and defined at top level.
147134
// We create a fact even if the RHS is non-exported,
148135
// so we can warn uses in other packages.
@@ -155,8 +142,8 @@ func run(pass *analysis.Pass) (any, error) {
155142
}
156143
})
157144

158-
// Pass 2. Inline each static call to an inlinable function,
159-
// and forward each reference to a forwardable constant.
145+
// Pass 2. Inline each static call to an inlinable function
146+
// and each reference to an inlinable constant.
160147
//
161148
// TODO(adonovan): handle multiple diffs that each add the same import.
162149
for cur := range cursor.Root(inspect).Preorder((*ast.CallExpr)(nil), (*ast.Ident)(nil)) {
@@ -231,14 +218,14 @@ func run(pass *analysis.Pass) (any, error) {
231218
}
232219

233220
case *ast.Ident:
234-
// If the identifier is a use of a forwardable constant, suggest forwarding it.
221+
// If the identifier is a use of an inlinable constant, suggest inlining it.
235222
if con, ok := pass.TypesInfo.Uses[n].(*types.Const); ok {
236-
fcon, ok := forwardableConsts[con]
223+
fcon, ok := inlinableConsts[con]
237224
if !ok {
238-
var fact goFixForwardConstFact
225+
var fact goFixInlineConstFact
239226
if pass.ImportObjectFact(con, &fact) {
240227
fcon = &fact
241-
forwardableConsts[con] = fcon
228+
inlinableConsts[con] = fcon
242229
}
243230
}
244231
if fcon == nil {
@@ -253,7 +240,7 @@ func run(pass *analysis.Pass) (any, error) {
253240
curFile := currentFile(cur)
254241

255242
// We have an identifier A here (n), possibly qualified by a package identifier (sel.X),
256-
// and a forwardable "const A = B" elsewhere (fcon).
243+
// and an inlinable "const A = B" elsewhere (fcon).
257244
// Consider replacing A with B.
258245

259246
// Check that the expression we are inlining (B) means the same thing
@@ -268,10 +255,10 @@ func run(pass *analysis.Pass) (any, error) {
268255
if obj == nil {
269256
// Should be impossible: if code at n can refer to the LHS,
270257
// it can refer to the RHS.
271-
panic(fmt.Sprintf("no object for forwardable const %s RHS %s", n.Name, fcon.RHSName))
258+
panic(fmt.Sprintf("no object for inlinable const %s RHS %s", n.Name, fcon.RHSName))
272259
}
273260
if obj != fcon.rhsObj {
274-
// "B" means something different here than at the forwardable const's scope.
261+
// "B" means something different here than at the inlinable const's scope.
275262
continue
276263
}
277264
}
@@ -304,9 +291,9 @@ func run(pass *analysis.Pass) (any, error) {
304291
pass.Report(analysis.Diagnostic{
305292
Pos: pos,
306293
End: end,
307-
Message: fmt.Sprintf("Constant %s should be forwarded", name),
294+
Message: fmt.Sprintf("Constant %s should be inlined", name),
308295
SuggestedFixes: []analysis.SuggestedFix{{
309-
Message: fmt.Sprintf("Forward constant %s", name),
296+
Message: fmt.Sprintf("Inline constant %s", name),
310297
TextEdits: edits,
311298
}},
312299
})
@@ -317,20 +304,15 @@ func run(pass *analysis.Pass) (any, error) {
317304
return nil, nil
318305
}
319306

320-
// fixDirectives reports the presence of "//go:fix inline" and "//go:fix forward"
321-
// directives in the comments.
322-
func fixDirectives(cg *ast.CommentGroup) (inline, forward bool) {
307+
// hasFixInline reports the presence of a "//go:fix inline" directive
308+
// in the comments.
309+
func hasFixInline(cg *ast.CommentGroup) bool {
323310
for _, d := range directives(cg) {
324-
if d.Tool == "go" && d.Name == "fix" {
325-
switch d.Args {
326-
case "inline":
327-
inline = true
328-
case "forward":
329-
forward = true
330-
}
311+
if d.Tool == "go" && d.Name == "fix" && d.Args == "inline" {
312+
return true
331313
}
332314
}
333-
return
315+
return false
334316
}
335317

336318
// A goFixInlineFuncFact is exported for each function marked "//go:fix inline".
@@ -340,21 +322,21 @@ type goFixInlineFuncFact struct{ Callee *inline.Callee }
340322
func (f *goFixInlineFuncFact) String() string { return "goFixInline " + f.Callee.String() }
341323
func (*goFixInlineFuncFact) AFact() {}
342324

343-
// A goFixForwardConstFact is exported for each constant marked "//go:fix forward".
344-
// It holds information about a forwardable constant. Gob-serializable.
345-
type goFixForwardConstFact struct {
325+
// A goFixInlineConstFact is exported for each constant marked "//go:fix inline".
326+
// It holds information about an inlinable constant. Gob-serializable.
327+
type goFixInlineConstFact struct {
346328
// Information about "const LHSName = RHSName".
347329
RHSName string
348330
RHSPkgPath string
349331
RHSPkgName string
350332
rhsObj types.Object // for current package
351333
}
352334

353-
func (c *goFixForwardConstFact) String() string {
354-
return fmt.Sprintf("goFixForward const %q.%s", c.RHSPkgPath, c.RHSName)
335+
func (c *goFixInlineConstFact) String() string {
336+
return fmt.Sprintf("goFixInline const %q.%s", c.RHSPkgPath, c.RHSName)
355337
}
356338

357-
func (*goFixForwardConstFact) AFact() {}
339+
func (*goFixInlineConstFact) AFact() {}
358340

359341
func discard(string, ...any) {}
360342

0 commit comments

Comments
 (0)