Skip to content

Commit

Permalink
gopls/internal/lsp/source: put context first in extracted functions
Browse files Browse the repository at this point in the history
Put the context first when extracting functions/methods

Fixes golang/go#60738
  • Loading branch information
vikstrous2 committed Jun 17, 2023
1 parent db6a81e commit a979900
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 3 deletions.
27 changes: 27 additions & 0 deletions gopls/internal/lsp/source/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ func extractFunctionMethod(fset *token.FileSet, start, end token.Pos, src []byte
}
}

reorderParams(params, paramTypes)

// Find the function literal that encloses the selection. The enclosing function literal
// may not be the enclosing function declaration (i.e. 'outer'). For example, in the
// following block:
Expand Down Expand Up @@ -631,6 +633,31 @@ func extractFunctionMethod(fset *token.FileSet, start, end token.Pos, src []byte
}, nil
}

// isSelector checks if e is the selector expr <x>, <sel>.
func isSelector(e ast.Expr, x, sel string) bool {
selectorExpr, ok := e.(*ast.SelectorExpr)
if !ok {
return false
}
ident, ok := selectorExpr.X.(*ast.Ident)
if !ok {
return false
}
return ident.Name == x && selectorExpr.Sel.Name == sel
}

// reorderParams reorders the given parameters in-place to follow common Go conventions.
func reorderParams(params []ast.Expr, paramTypes []*ast.Field) {
// If a context is found in the parameters, put it first.
for i, t := range paramTypes {
if isSelector(t.Type, "context", "Context") {
params[0], params[i] = params[i], params[0]
paramTypes[0], paramTypes[i] = paramTypes[i], paramTypes[0]
break
}
}
}

// adjustRangeForCommentsAndWhiteSpace adjusts the given range to exclude unnecessary leading or
// trailing whitespace characters from selection as well as leading or trailing comments.
// In the following example, each line of the if statement is indented once. There are also two
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package extract

import "context"

type A struct {
x int
y int
}

func (a *A) AddP(ctx context.Context) (int, error) {
sum := a.x + a.y
return sum, ctx.Err() //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- methodextraction_extract_context_12_2 --
package extract

import "context"

type A struct {
x int
y int
}

func (a *A) AddP(ctx context.Context) (int, error) {
sum := a.x + a.y
return a.newMethod(ctx, sum) //@extractmethod("return", "ctx.Err()"),extractfunc("return", "ctx.Err()")
}

func (*A) newMethod(ctx context.Context, sum int) (int, error) {
return sum, ctx.Err()
}

2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/summary.txt.golden
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ DiagnosticsCount = 23
FoldingRangesCount = 2
SemanticTokenCount = 3
SuggestedFixCount = 73
MethodExtractionCount = 6
MethodExtractionCount = 7
DefinitionsCount = 46
TypeDefinitionsCount = 18
HighlightsCount = 70
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/summary_go1.18.txt.golden
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ DiagnosticsCount = 23
FoldingRangesCount = 2
SemanticTokenCount = 3
SuggestedFixCount = 79
MethodExtractionCount = 6
MethodExtractionCount = 7
DefinitionsCount = 46
TypeDefinitionsCount = 18
HighlightsCount = 70
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/summary_go1.21.txt.golden
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ DiagnosticsCount = 24
FoldingRangesCount = 2
SemanticTokenCount = 3
SuggestedFixCount = 79
MethodExtractionCount = 6
MethodExtractionCount = 7
DefinitionsCount = 46
TypeDefinitionsCount = 18
HighlightsCount = 70
Expand Down

0 comments on commit a979900

Please sign in to comment.