Skip to content

Commit 378bdfd

Browse files
xzbdmwxzbdmw
committed
add missing column
1 parent 53efd30 commit 378bdfd

File tree

6 files changed

+215
-66
lines changed

6 files changed

+215
-66
lines changed

‎gopls/doc/features/diagnostics.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,13 @@ func main() {
213213
}
214214
```
215215

216-
The quick fix would be:
216+
The quick fix would insert a declaration with a default
217+
value inferring its type from the context:
217218

218219
```go
219220
func main() {
220221
x := 42
221-
y :=
222+
y := 0
222223
min(x, y)
223224
}
224225
```

‎gopls/doc/release/v0.17.0.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ removed as needed.
3333

3434
The user can invoke this code action by selecting a function name, the keywords
3535
`func`, `const`, `var`, `type`, or by placing the caret on them without selecting,
36-
or by selecting a whole declaration or multiple declrations.
36+
or by selecting a whole declaration or multiple declarations.
3737

3838
In order to avoid ambiguity and surprise about what to extract, some kinds
39-
of paritial selection of a declration cannot invoke this code action.
39+
of paritial selection of a declaration cannot invoke this code action.
4040

4141
## Pull diagnostics
4242

‎gopls/internal/golang/undeclared.go‎

Lines changed: 102 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616

1717
"golang.org/x/tools/go/analysis"
1818
"golang.org/x/tools/go/ast/astutil"
19-
"golang.org/x/tools/gopls/internal/util/safetoken"
2019
"golang.org/x/tools/gopls/internal/util/typesutil"
2120
"golang.org/x/tools/internal/analysisinternal"
2221
"golang.org/x/tools/internal/typesinternal"
@@ -87,33 +86,118 @@ func CreateUndeclared(fset *token.FileSet, start, end token.Pos, content []byte,
8786
if isCallPosition(path) {
8887
return newFunctionDeclaration(path, file, pkg, info, fset)
8988
}
89+
var (
90+
firstRef *ast.Ident // We should insert the new declaration before the first occurrence of the undefined ident.
91+
assignTokPos token.Pos
92+
funcDecl = path[len(path)-2].(*ast.FuncDecl) // This is already ensured by [undeclaredFixTitle].
93+
parent = ast.Node(funcDecl)
94+
)
95+
// Search from enclosing FuncDecl to path[0], since we can not use := syntax outside function.
96+
// Adds the missing colon after the first undefined symbol
97+
// when it sits in lhs of an AssignStmt.
98+
ast.Inspect(funcDecl, func(n ast.Node) bool {
99+
if n == nil || firstRef != nil {
100+
return false
101+
}
102+
if n, ok := n.(*ast.Ident); ok && n.Name == ident.Name && info.ObjectOf(n) == nil {
103+
firstRef = n
104+
// Only consider adding colon at the first occurrence.
105+
if pos, ok := replaceableAssign(info, n, parent); ok {
106+
assignTokPos = pos
107+
return false
108+
}
109+
}
110+
parent = n
111+
return true
112+
})
113+
if assignTokPos.IsValid() {
114+
return fset, &analysis.SuggestedFix{
115+
TextEdits: []analysis.TextEdit{{
116+
Pos: assignTokPos,
117+
End: assignTokPos,
118+
NewText: []byte(":"),
119+
}},
120+
}, nil
121+
}
90122

91-
// Get the place to insert the new statement.
92-
insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(path)
123+
// firstRef should never be nil, at least one ident at cursor position should be found,
124+
// but be defensive.
125+
if firstRef == nil {
126+
return nil, nil, fmt.Errorf("no identifier found")
127+
}
128+
p, _ := astutil.PathEnclosingInterval(file, firstRef.Pos(), firstRef.Pos())
129+
insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(p)
93130
if insertBeforeStmt == nil {
94131
return nil, nil, fmt.Errorf("could not locate insertion point")
95132
}
96-
97-
insertBefore := safetoken.StartPosition(fset, insertBeforeStmt.Pos()).Offset
98-
99-
// Get the indent to add on the line after the new statement.
100-
// Since this will have a parse error, we can not use format.Source().
101-
contentBeforeStmt, indent := content[:insertBefore], "\n"
102-
if nl := bytes.LastIndex(contentBeforeStmt, []byte("\n")); nl != -1 {
103-
indent = string(contentBeforeStmt[nl:])
133+
indent, err := calculateIndentation(content, fset.File(file.FileStart), insertBeforeStmt)
134+
if err != nil {
135+
return nil, nil, err
136+
}
137+
typs := typesutil.TypesFromContext(info, path, start)
138+
if typs == nil {
139+
// Default to 0.
140+
typs = []types.Type{types.Typ[types.Int]}
141+
}
142+
assignStmt := &ast.AssignStmt{
143+
Lhs: []ast.Expr{ast.NewIdent(ident.Name)},
144+
Tok: token.DEFINE,
145+
Rhs: []ast.Expr{typesinternal.ZeroExpr(file, pkg, typs[0])},
146+
}
147+
var buf bytes.Buffer
148+
if err := format.Node(&buf, fset, assignStmt); err != nil {
149+
return nil, nil, err
104150
}
151+
newLineIndent := "\n" + indent
152+
assignment := strings.ReplaceAll(buf.String(), "\n", newLineIndent) + newLineIndent
105153

106-
// Create the new local variable statement.
107-
newStmt := fmt.Sprintf("%s := %s", ident.Name, indent)
108154
return fset, &analysis.SuggestedFix{
109-
TextEdits: []analysis.TextEdit{{
110-
Pos: insertBeforeStmt.Pos(),
111-
End: insertBeforeStmt.Pos(),
112-
NewText: []byte(newStmt),
113-
}},
155+
TextEdits: []analysis.TextEdit{
156+
{
157+
Pos: insertBeforeStmt.Pos(),
158+
End: insertBeforeStmt.Pos(),
159+
NewText: []byte(assignment),
160+
},
161+
},
114162
}, nil
115163
}
116164

165+
// replaceableAssign returns position of token.ASSIGN if ident meets the following conditions:
166+
// 1) parent node must be an *ast.AssignStmt with Tok set to token.ASSIGN.
167+
// 2) ident must not be self assignment.
168+
//
169+
// For example, we should not add a colon when
170+
// a = a + 1
171+
// ^ ^ cursor here
172+
func replaceableAssign(info *types.Info, ident *ast.Ident, parent ast.Node) (token.Pos, bool) {
173+
var pos token.Pos
174+
if assign, ok := parent.(*ast.AssignStmt); ok && assign.Tok == token.ASSIGN {
175+
for _, rhs := range assign.Rhs {
176+
if referencesIdent(info, rhs, ident) {
177+
return pos, false
178+
}
179+
}
180+
return assign.TokPos, true
181+
}
182+
return pos, false
183+
}
184+
185+
// referencesIdent checks whether the given undefined ident appears in the given expression.
186+
func referencesIdent(info *types.Info, expr ast.Expr, ident *ast.Ident) bool {
187+
var hasIdent bool
188+
ast.Inspect(expr, func(n ast.Node) bool {
189+
if n == nil {
190+
return false
191+
}
192+
if i, ok := n.(*ast.Ident); ok && i.Name == ident.Name && info.ObjectOf(i) == nil {
193+
hasIdent = true
194+
return false
195+
}
196+
return true
197+
})
198+
return hasIdent
199+
}
200+
117201
func newFunctionDeclaration(path []ast.Node, file *ast.File, pkg *types.Package, info *types.Info, fset *token.FileSet) (*token.FileSet, *analysis.SuggestedFix, error) {
118202
if len(path) < 3 {
119203
return nil, nil, fmt.Errorf("unexpected set of enclosing nodes: %v", path)

‎gopls/internal/test/marker/testdata/quickfix/undeclared.txt‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎gopls/internal/test/marker/testdata/quickfix/undeclared/undeclared_variable.txt‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
Tests of suggested fixes for "undeclared name" diagnostics,
2+
which are of ("compiler", "error") type.
3+
4+
-- flags --
5+
-ignore_extra_diags
6+
7+
-- go.mod --
8+
module example.com
9+
go 1.12
10+
11+
-- a.go --
12+
package undeclared_var
13+
14+
func a() {
15+
z, _ := 1+y, 11 //@quickfix("y", re"(undeclared name|undefined): y", a)
16+
_ = z
17+
}
18+
19+
-- @a/a.go --
20+
@@ -4 +4 @@
21+
+ y := 0
22+
-- b.go --
23+
package undeclared_var
24+
25+
func b() {
26+
if 100 < 90 {
27+
} else if 100 > n+2 { //@quickfix("n", re"(undeclared name|undefined): n", b)
28+
}
29+
}
30+
31+
-- @b/b.go --
32+
@@ -4 +4 @@
33+
+ n := 0
34+
-- c.go --
35+
package undeclared_var
36+
37+
func c() {
38+
for i < 200 { //@quickfix("i", re"(undeclared name|undefined): i", c)
39+
}
40+
r() //@diag("r", re"(undeclared name|undefined): r")
41+
}
42+
43+
-- @c/c.go --
44+
@@ -4 +4 @@
45+
+ i := 0
46+
-- add_colon.go --
47+
package undeclared_var
48+
49+
func addColon() {
50+
ac = 1 //@quickfix("ac", re"(undeclared name|undefined): ac", add_colon)
51+
}
52+
53+
-- @add_colon/add_colon.go --
54+
@@ -4 +4 @@
55+
- ac = 1 //@quickfix("ac", re"(undeclared name|undefined): ac", add_colon)
56+
+ ac := 1 //@quickfix("ac", re"(undeclared name|undefined): ac", add_colon)
57+
-- add_colon_first.go --
58+
package undeclared_var
59+
60+
func addColonAtFirstStmt() {
61+
ac = 1
62+
ac = 2
63+
ac = 3
64+
b := ac //@quickfix("ac", re"(undeclared name|undefined): ac", add_colon_first)
65+
}
66+
67+
-- @add_colon_first/add_colon_first.go --
68+
@@ -4 +4 @@
69+
- ac = 1
70+
+ ac := 1
71+
-- self_assign.go --
72+
package undeclared_var
73+
74+
func selfAssign() {
75+
ac = ac + 1
76+
ac = ac + 2 //@quickfix("ac", re"(undeclared name|undefined): ac", lhs)
77+
ac = ac + 3 //@quickfix("ac + 3", re"(undeclared name|undefined): ac", rhs)
78+
}
79+
80+
-- @lhs/self_assign.go --
81+
@@ -4 +4 @@
82+
+ ac := nil
83+
-- @rhs/self_assign.go --
84+
@@ -4 +4 @@
85+
+ ac := 0
86+
-- correct_type.go --
87+
package undeclared_var
88+
import "fmt"
89+
func selfAssign() {
90+
fmt.Printf(ac) //@quickfix("ac", re"(undeclared name|undefined): ac", string)
91+
}
92+
-- @string/correct_type.go --
93+
@@ -4 +4 @@
94+
+ ac := ""
95+
-- ignore.go --
96+
package undeclared_var
97+
import "fmt"
98+
type Foo struct {
99+
bar int
100+
}
101+
func selfAssign() {
102+
f := Foo{}
103+
b = f.bar
104+
c := bar //@quickfix("bar", re"(undeclared name|undefined): bar", ignore)
105+
}
106+
-- @ignore/ignore.go --
107+
@@ -9 +9 @@
108+
+ bar := nil

gopls/internal/test/marker/testdata/quickfix/undeclaredfunc.txt renamed to gopls/internal/test/marker/testdata/quickfix/undeclared/undeclaredfunc.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
This test checks the quick fix for "undeclared: f" that declares the
22
missing function. See #47558.
33

4-
TODO(adonovan): infer the result variables from the context (int, in this case).
5-
64
-- a.go --
75
package a
86

0 commit comments

Comments
 (0)