Skip to content

Commit 538a6e9

Browse files
vikstrous2findleyr
authored andcommitted
gopls/internal/lsp/source: put context first in extracted functions
Put the context first when extracting functions/methods Fixes golang/go#60738 Change-Id: I04f86355ffa0c7b080a8aea82baf0947722b6440 GitHub-Last-Rev: 5267944 GitHub-Pull-Request: #440 Reviewed-on: https://go-review.googlesource.com/c/tools/+/503295 Reviewed-by: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Joedian Reid <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent ef12545 commit 538a6e9

File tree

6 files changed

+104
-3
lines changed

6 files changed

+104
-3
lines changed

gopls/internal/lsp/source/extract.go

+29
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,8 @@ func extractFunctionMethod(fset *token.FileSet, start, end token.Pos, src []byte
363363
}
364364
}
365365

366+
reorderParams(params, paramTypes)
367+
366368
// Find the function literal that encloses the selection. The enclosing function literal
367369
// may not be the enclosing function declaration (i.e. 'outer'). For example, in the
368370
// following block:
@@ -631,6 +633,33 @@ func extractFunctionMethod(fset *token.FileSet, start, end token.Pos, src []byte
631633
}, nil
632634
}
633635

636+
// isSelector reports if e is the selector expr <x>, <sel>.
637+
func isSelector(e ast.Expr, x, sel string) bool {
638+
selectorExpr, ok := e.(*ast.SelectorExpr)
639+
if !ok {
640+
return false
641+
}
642+
ident, ok := selectorExpr.X.(*ast.Ident)
643+
if !ok {
644+
return false
645+
}
646+
return ident.Name == x && selectorExpr.Sel.Name == sel
647+
}
648+
649+
// reorderParams reorders the given parameters in-place to follow common Go conventions.
650+
func reorderParams(params []ast.Expr, paramTypes []*ast.Field) {
651+
// Move Context parameter (if any) to front.
652+
for i, t := range paramTypes {
653+
if isSelector(t.Type, "context", "Context") {
654+
p, t := params[i], paramTypes[i]
655+
copy(params[1:], params[:i])
656+
copy(paramTypes[1:], paramTypes[:i])
657+
params[0], paramTypes[0] = p, t
658+
break
659+
}
660+
}
661+
}
662+
634663
// adjustRangeForCommentsAndWhiteSpace adjusts the given range to exclude unnecessary leading or
635664
// trailing whitespace characters from selection as well as leading or trailing comments.
636665
// In the following example, each line of the if statement is indented once. There are also two
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package extract
2+
3+
import "context"
4+
5+
type B struct {
6+
x int
7+
y int
8+
}
9+
10+
func (b *B) AddP(ctx context.Context) (int, error) {
11+
sum := b.x + b.y
12+
return sum, ctx.Err() //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
13+
}
14+
15+
func (b *B) LongList(ctx context.Context) (int, error) {
16+
p1 := 1
17+
p2 := 1
18+
p3 := 1
19+
return p1 + p2 + p3, ctx.Err() //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
20+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
-- methodextraction_extract_context_12_2 --
2+
package extract
3+
4+
import "context"
5+
6+
type B struct {
7+
x int
8+
y int
9+
}
10+
11+
func (b *B) AddP(ctx context.Context) (int, error) {
12+
sum := b.x + b.y
13+
return b.newMethod(ctx, sum) //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
14+
}
15+
16+
func (*B) newMethod(ctx context.Context, sum int) (int, error) {
17+
return sum, ctx.Err()
18+
}
19+
20+
func (b *B) LongList(ctx context.Context) (int, error) {
21+
p1 := 1
22+
p2 := 1
23+
p3 := 1
24+
return p1 + p2 + p3, ctx.Err() //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
25+
}
26+
27+
-- methodextraction_extract_context_19_2 --
28+
package extract
29+
30+
import "context"
31+
32+
type B struct {
33+
x int
34+
y int
35+
}
36+
37+
func (b *B) AddP(ctx context.Context) (int, error) {
38+
sum := b.x + b.y
39+
return sum, ctx.Err() //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
40+
}
41+
42+
func (b *B) LongList(ctx context.Context) (int, error) {
43+
p1 := 1
44+
p2 := 1
45+
p3 := 1
46+
return b.newMethod(ctx, p1, p2, p3) //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
47+
}
48+
49+
func (*B) newMethod(ctx context.Context, p1 int, p2 int, p3 int) (int, error) {
50+
return p1 + p2 + p3, ctx.Err()
51+
}
52+

gopls/internal/lsp/testdata/summary.txt.golden

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ DiagnosticsCount = 23
1212
FoldingRangesCount = 2
1313
SemanticTokenCount = 3
1414
SuggestedFixCount = 73
15-
MethodExtractionCount = 6
15+
MethodExtractionCount = 8
1616
DefinitionsCount = 46
1717
TypeDefinitionsCount = 18
1818
HighlightsCount = 70

gopls/internal/lsp/testdata/summary_go1.18.txt.golden

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ DiagnosticsCount = 23
1212
FoldingRangesCount = 2
1313
SemanticTokenCount = 3
1414
SuggestedFixCount = 79
15-
MethodExtractionCount = 6
15+
MethodExtractionCount = 8
1616
DefinitionsCount = 46
1717
TypeDefinitionsCount = 18
1818
HighlightsCount = 70

gopls/internal/lsp/testdata/summary_go1.21.txt.golden

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ DiagnosticsCount = 24
1212
FoldingRangesCount = 2
1313
SemanticTokenCount = 3
1414
SuggestedFixCount = 79
15-
MethodExtractionCount = 6
15+
MethodExtractionCount = 8
1616
DefinitionsCount = 46
1717
TypeDefinitionsCount = 18
1818
HighlightsCount = 70

0 commit comments

Comments
 (0)