internal/lsp: make function extraction smarter
In the previous implementation, the extracted function would
sometimes include superfluous parameters and return values. It
might also unnecessarily initialize variables. This CL introduces
3 rules to limit this behavior. (1) a variable is not passed as a
parameter to the extracted function if its first use within the
function is its own redefinition. (2) a variable is not returned
from the extracted function if its first use after the function is its
own redefinition. (3) when possible, we redefine variables in the call
expression to the extracted function.
Change-Id: Ideb5a7eff8a1bf462c83271a2f043116ff5d8b76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244770
Run-TryBot: Josh Baum <joshbaum@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 4446741..3e16515 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -493,7 +493,15 @@
func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {
uri := start.URI()
- _, err := r.server.session.ViewOf(uri)
+ view, err := r.server.session.ViewOf(uri)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ snapshot, release := view.Snapshot()
+ defer release()
+
+ fh, err := snapshot.GetFile(r.ctx, uri)
if err != nil {
t.Fatal(err)
}
@@ -523,7 +531,11 @@
if len(actions) == 0 || len(actions) > 1 {
t.Fatalf("unexpected number of code actions, want 1, got %v", len(actions))
}
- res, err := applyTextDocumentEdits(r, actions[0].Edit.DocumentChanges)
+ edits, err := commandToEdits(r.ctx, snapshot, fh, rng, actions[0].Command.Command)
+ if err != nil {
+ t.Fatal(err)
+ }
+ res, err := applyTextDocumentEdits(r, edits)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/lsp/source/extract.go b/internal/lsp/source/extract.go
index c3593d9..06f293b 100644
--- a/internal/lsp/source/extract.go
+++ b/internal/lsp/source/extract.go
@@ -180,7 +180,7 @@
startParent := findParent(outer, start)
ast.Inspect(outer, func(n ast.Node) bool {
if n == nil {
- return true
+ return false
}
if n.Pos() < rng.Start || n.End() > rng.End {
return n.Pos() <= rng.End
@@ -194,7 +194,7 @@
return false
}
retStmts = append(retStmts, ret)
- return true
+ return false
})
if hasNonNestedReturn {
return nil, fmt.Errorf("extractFunction: selected block contains non-nested return")
@@ -205,7 +205,8 @@
// we must determine the signature of the extracted function. We will then replace
// the block with an assignment statement that calls the extracted function with
// the appropriate parameters and return values.
- free, vars, assigned := collectFreeVars(info, file, fileScope, pkgScope, rng, path[0])
+ free, vars, assigned, defined := collectFreeVars(
+ info, file, fileScope, pkgScope, rng, path[0])
var (
params, returns []ast.Expr // used when calling the extracted function
@@ -217,6 +218,28 @@
seenVars := make(map[types.Object]ast.Expr)
seenUninitialized := make(map[types.Object]struct{})
+ // Some variables on the left-hand side of our assignment statement may be free. If our
+ // selection begins in the same scope in which the free variable is defined, we can
+ // redefine it in our assignment statement. See the following example, where 'b' and
+ // 'err' (both free variables) can be redefined in the second funcCall() while maintaing
+ // correctness.
+ //
+ //
+ // Not Redefined:
+ //
+ // a, err := funcCall()
+ // var b int
+ // b, err = funcCall()
+ //
+ // Redefined:
+ //
+ // a, err := funcCall()
+ // b, err := funcCall()
+ //
+ // We track the number of free variables that can be redefined to maintain our preference
+ // of using "x, y, z := fn()" style assignment statements.
+ var canRedefineCount int
+
// Each identifier in the selected block must become (1) a parameter to the
// extracted function, (2) a return value of the extracted function, or (3) a local
// variable in the extracted function. Determine the outcome(s) for each variable
@@ -232,23 +255,32 @@
}
seenVars[obj] = typ
identifier := ast.NewIdent(obj.Name())
- // An identifier must meet two conditions to become a return value of the
- // extracted function. (1) it must be used at least once after the
- // selection (isUsed), and (2) its value must be initialized or reassigned
- // within the selection (isAssigned).
- isUsed := objUsed(obj, info, rng.End, obj.Parent().End())
+ // An identifier must meet three conditions to become a return value of the
+ // extracted function. (1) its value must be defined or reassigned within
+ // the selection (isAssigned), (2) it must be used at least once after the
+ // selection (isUsed), and (3) its first use after the selection
+ // cannot be its own reassignment or redefinition (objOverriden).
+ if obj.Parent() == nil {
+ return nil, fmt.Errorf("parent nil")
+ }
+ isUsed, firstUseAfter :=
+ objUsed(info, span.NewRange(fset, rng.End, obj.Parent().End()), obj)
_, isAssigned := assigned[obj]
_, isFree := free[obj]
- if isUsed && isAssigned {
+ if isAssigned && isUsed && !varOverridden(info, firstUseAfter, obj, isFree, outer) {
returnTypes = append(returnTypes, &ast.Field{Type: typ})
returns = append(returns, identifier)
if !isFree {
uninitialized = append(uninitialized, obj)
+ } else if obj.Parent().Pos() == startParent.Pos() {
+ canRedefineCount++
}
}
- // All free variables are parameters of and passed as arguments to the
- // extracted function.
- if isFree {
+ _, isDefined := defined[obj]
+ // An identifier must meet two conditions to become a parameter of the
+ // extracted function. (1) it must be free (isFree), and (2) its first
+ // use within the selection cannot be its own definition (isDefined).
+ if isFree && !isDefined {
params = append(params, identifier)
paramTypes = append(paramTypes, &ast.Field{
Names: []*ast.Ident{identifier},
@@ -371,13 +403,16 @@
}
// Construct the appropriate call to the extracted function.
- funName := generateAvailableIdentifier(rng.Start, file, path, info, "fn", 0)
- // If none of the variables on the left-hand side of the function call have
- // been initialized before the selection, we can use ':=' instead of '='.
+ // We must meet two conditions to use ":=" instead of '='. (1) there must be at least
+ // one variable on the lhs that is uninitailized (non-free) prior to the assignment.
+ // (2) all of the initialized (free) variables on the lhs must be able to be redefined.
sym := token.ASSIGN
- if len(uninitialized) == len(returns) {
+ canDefineCount := len(uninitialized) + canRedefineCount
+ canDefine := len(uninitialized)+len(retVars) > 0 && canDefineCount == len(returns)
+ if canDefine {
sym = token.DEFINE
}
+ funName := generateAvailableIdentifier(rng.Start, file, path, info, "fn", 0)
extractedFunCall := generateFuncCall(hasReturnValues, params,
append(returns, getNames(retVars)...), funName, sym)
@@ -392,11 +427,11 @@
}
// Create variable declarations for any identifiers that need to be initialized prior to
- // calling the extracted function.
- declarations, err := initializeVars(
- uninitialized, returns, retVars, seenUninitialized, seenVars)
- if err != nil {
- return nil, err
+ // calling the extracted function. We do not manually initialize variables if every return
+ // value is unitialized. We can use := to initialize the variables in this situation.
+ var declarations []ast.Stmt
+ if canDefineCount != len(returns) {
+ declarations = initializeVars(uninitialized, retVars, seenUninitialized, seenVars)
}
var declBuf, replaceBuf, newFuncBuf, ifBuf bytes.Buffer
@@ -509,7 +544,8 @@
// list of identifiers that may need to be returned by the extracted function.
// Some of the code in this function has been adapted from tools/cmd/guru/freevars.go.
func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope,
- pkgScope *types.Scope, rng span.Range, node ast.Node) (map[types.Object]struct{}, []types.Object, map[types.Object]struct{}) {
+ pkgScope *types.Scope, rng span.Range, node ast.Node) (map[types.Object]struct{},
+ []types.Object, map[types.Object]struct{}, map[types.Object]struct{}) {
// id returns non-nil if n denotes an object that is referenced by the span
// and defined either within the span or in the lexical environment. The bool
// return value acts as an indicator for where it was defined.
@@ -518,6 +554,9 @@
if obj == nil {
return info.Defs[n], false
}
+ if obj.Name() == "_" {
+ return nil, false // exclude objects denoting '_'
+ }
if _, ok := obj.(*types.PkgName); ok {
return nil, false // imported package
}
@@ -550,10 +589,11 @@
return nil, false
}
free := make(map[types.Object]struct{})
+ firstUseIn := make(map[types.Object]token.Pos)
var vars []types.Object
ast.Inspect(node, func(n ast.Node) bool {
if n == nil {
- return true
+ return false
}
if rng.Start <= n.Pos() && n.End() <= rng.End {
var obj types.Object
@@ -565,10 +605,15 @@
obj, isFree = sel(n)
prune = true
}
- if obj != nil && obj.Name() != "_" {
+ if obj != nil {
if isFree {
free[obj] = struct{}{}
}
+ // Find the first time that the object is used in the selection.
+ first, ok := firstUseIn[obj]
+ if !ok || n.Pos() < first {
+ firstUseIn[obj] = n.Pos()
+ }
vars = append(vars, obj)
if prune {
return false
@@ -589,9 +634,10 @@
// 4: z := x + a
//
assigned := make(map[types.Object]struct{})
+ defined := make(map[types.Object]struct{})
ast.Inspect(node, func(n ast.Node) bool {
if n == nil {
- return true
+ return false
}
if n.Pos() < rng.Start || n.End() > rng.End {
return n.Pos() <= rng.End
@@ -599,19 +645,43 @@
switch n := n.(type) {
case *ast.AssignStmt:
for _, assignment := range n.Lhs {
- if assignment, ok := assignment.(*ast.Ident); ok {
- obj, _ := id(assignment)
- if obj == nil {
+ lhs, ok := assignment.(*ast.Ident)
+ if !ok {
+ continue
+ }
+ obj, _ := id(lhs)
+ if obj == nil {
+ continue
+ }
+ assigned[obj] = struct{}{}
+ if n.Tok != token.DEFINE {
+ continue
+ }
+ // Find identifiers that are defined prior to being used
+ // elsewhere in the selection.
+ // TODO: Include identifiers that are assigned prior to being
+ // used elsewhere in the selection. Then, change the assignment
+ // to a definition in the extracted function.
+ if firstUseIn[obj] != lhs.Pos() {
+ continue
+ }
+ // Ensure that the object is not used in its own re-definition.
+ // For example:
+ // var f float64
+ // f, e := math.Frexp(f)
+ for _, expr := range n.Rhs {
+ if referencesObj(info, expr, obj) {
continue
}
- assigned[obj] = struct{}{}
+ defined[obj] = struct{}{}
+ break
}
}
return false
case *ast.DeclStmt:
gen, ok := n.Decl.(*ast.GenDecl)
if !ok {
- return true
+ return false
}
for _, spec := range gen.Specs {
vSpecs, ok := spec.(*ast.ValueSpec)
@@ -627,10 +697,39 @@
}
}
return false
+ case *ast.IncDecStmt:
+ if ident, ok := n.X.(*ast.Ident); !ok {
+ return false
+ } else if obj, _ := id(ident); obj == nil {
+ return false
+ } else {
+ assigned[obj] = struct{}{}
+ }
}
return true
})
- return free, vars, assigned
+ return free, vars, assigned, defined
+}
+
+// referencesObj checks whether the given object appears in the given expression.
+func referencesObj(info *types.Info, expr ast.Expr, obj types.Object) bool {
+ var hasObj bool
+ ast.Inspect(expr, func(n ast.Node) bool {
+ if n == nil {
+ return false
+ }
+ ident, ok := n.(*ast.Ident)
+ if !ok {
+ return true
+ }
+ objUse := info.Uses[ident]
+ if obj == objUse {
+ hasObj = true
+ return false
+ }
+ return false
+ })
+ return hasObj
}
// canExtractFunction reports whether the code in the given range can be extracted to a function.
@@ -671,7 +770,7 @@
var start, end ast.Node
ast.Inspect(outer, func(n ast.Node) bool {
if n == nil {
- return true
+ return false
}
// Do not override 'start' with a node that begins at the same location but is
// nested further from 'outer'.
@@ -689,14 +788,72 @@
return tok, path, outer, start, true, nil
}
-// objUsed checks if the object is used between the given positions.
-func objUsed(obj types.Object, info *types.Info, endSel token.Pos, endScope token.Pos) bool {
- for id, ob := range info.Uses {
- if obj == ob && endSel < id.Pos() && id.End() <= endScope {
- return true
+// objUsed checks if the object is used within the range. It returns the first occurence of
+// the object in the range, if it exists.
+func objUsed(info *types.Info, rng span.Range, obj types.Object) (bool, *ast.Ident) {
+ var firstUse *ast.Ident
+ for id, objUse := range info.Uses {
+ if obj != objUse {
+ continue
+ }
+ if id.Pos() < rng.Start || id.End() > rng.End {
+ continue
+ }
+ if firstUse == nil || id.Pos() < firstUse.Pos() {
+ firstUse = id
}
}
- return false
+ return firstUse != nil, firstUse
+}
+
+// varOverridden traverses the given AST node until we find the given identifier. Then, we
+// examine the occurrence of the given identifier and check for (1) whether the identifier
+// is being redefined. If the identifier is free, we also check for (2) whether the identifier
+// is being reassigned. We will not include an identifier in the return statement of the
+// extracted function if it meets one of the above conditions.
+func varOverridden(info *types.Info, firstUse *ast.Ident, obj types.Object, isFree bool, node ast.Node) bool {
+ var isOverriden bool
+ ast.Inspect(node, func(n ast.Node) bool {
+ if n == nil {
+ return false
+ }
+ assignment, ok := n.(*ast.AssignStmt)
+ if !ok {
+ return true
+ }
+ // A free variable is initialized prior to the selection. We can always reassign
+ // this variable after the selection because it has already been defined.
+ // Conversely, a non-free variable is initialized within the selection. Thus, we
+ // cannot reassign this variable after the selection unless it is initialized and
+ // returned by the extracted function.
+ if !isFree && assignment.Tok == token.ASSIGN {
+ return false
+ }
+ for _, assigned := range assignment.Lhs {
+ ident, ok := assigned.(*ast.Ident)
+ // Check if we found the first use of the identifier.
+ if !ok || ident != firstUse {
+ continue
+ }
+ objUse := info.Uses[ident]
+ if objUse == nil || objUse != obj {
+ continue
+ }
+ // Ensure that the object is not used in its own definition.
+ // For example:
+ // var f float64
+ // f, e := math.Frexp(f)
+ for _, expr := range assignment.Rhs {
+ if referencesObj(info, expr, obj) {
+ return false
+ }
+ }
+ isOverriden = true
+ return false
+ }
+ return false
+ })
+ return isOverriden
}
// parseExtraction generates an AST file from the given text. We then return the portion of the
@@ -794,11 +951,11 @@
zeroVals = append(zeroVals, ast.NewIdent("true"))
ast.Inspect(extractedBlock, func(n ast.Node) bool {
if n == nil {
- return true
+ return false
}
if n, ok := n.(*ast.ReturnStmt); ok {
n.Results = append(zeroVals, n.Results...)
- return true
+ return false
}
return true
})
@@ -830,21 +987,16 @@
// initializeVars creates variable declarations, if needed.
// Our preference is to replace the selected block with an "x, y, z := fn()" style
-// assignment statement. We can use this style when none of the variables in the
-// extracted function's return statement have already be initialized outside of the
-// selected block. However, for example, if z is already defined elsewhere, we
-// replace the selected block with:
+// assignment statement. We can use this style when all of the variables in the
+// extracted function's return statement are either not defined prior to the extracted block
+// or can be safely redefined. However, for example, if z is already defined
+// in a different scope, we replace the selected block with:
//
// var x int
// var y string
// x, y, z = fn()
-func initializeVars(uninitialized []types.Object, returns []ast.Expr, retVars []*returnVariable, seenUninitialized map[types.Object]struct{}, seenVars map[types.Object]ast.Expr) ([]ast.Stmt, error) {
+func initializeVars(uninitialized []types.Object, retVars []*returnVariable, seenUninitialized map[types.Object]struct{}, seenVars map[types.Object]ast.Expr) []ast.Stmt {
var declarations []ast.Stmt
- // We do not manually initialize variables if every return value is unitialized.
- // We can use := to initialize the variables in this situation.
- if len(uninitialized) == len(returns) {
- return declarations, nil
- }
for _, obj := range uninitialized {
if _, ok := seenUninitialized[obj]; ok {
continue
@@ -874,7 +1026,7 @@
}
declarations = append(declarations, &ast.DeclStmt{Decl: genDecl})
}
- return declarations, nil
+ return declarations
}
// getNames returns the names from the given list of returnVariable.
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go
index fc46f96..0c38011 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go
@@ -2,9 +2,9 @@
func _() {
a := 1
- a = 5 //@mark(s1, "a")
- a = a + 2 //@mark(e1, "2")
- //@extractfunc(s1, e1)
+ a = 5 //@mark(exSt0, "a")
+ a = a + 2 //@mark(exEn0, "2")
+ //@extractfunc(exSt0, exEn0)
b := a * 2
- var _ = 3 + 4
+ _ = 3 + 4
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go.golden
index b528db9..04caef2 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_args_returns.go.golden
@@ -3,10 +3,10 @@
func _() {
a := 1
- a = fn0(a) //@mark(e1, "2")
- //@extractfunc(s1, e1)
+ a = fn0(a) //@mark(exEn0, "2")
+ //@extractfunc(exSt0, exEn0)
b := a * 2
- var _ = 3 + 4
+ _ = 3 + 4
}
func fn0(a int) int {
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go
index 32cbcf1..b5b9efd 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go
@@ -1,7 +1,7 @@
package extract
func _() {
- a := 1 //@mark(s0, "a")
- var _ = 3 + 4 //@mark(e0, "4")
- //@extractfunc(s0, e0)
+ a := 1 //@mark(exSt1, "a")
+ _ = 3 + 4 //@mark(exEn1, "4")
+ //@extractfunc(exSt1, exEn1)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go.golden
index 5a8fb43..16a7863 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_basic.go.golden
@@ -2,12 +2,12 @@
package extract
func _() {
- x0() //@mark(e0, "4")
- //@extractfunc(s0, e0)
+ fn0() //@mark(exEn1, "4")
+ //@extractfunc(exSt1, exEn1)
}
-func x0() {
+func fn0() {
a := 1
- var _ = 3 + 4
+ _ = 3 + 4
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go
new file mode 100644
index 0000000..604f475
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go
@@ -0,0 +1,11 @@
+package extract
+
+import "strconv"
+
+func _() {
+ i, err := strconv.Atoi("1")
+ u, err := strconv.Atoi("2") //@extractfunc("u", ")")
+ if i == u || err == nil {
+ return
+ }
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go.golden
new file mode 100644
index 0000000..e739e66
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_redefine.go.golden
@@ -0,0 +1,18 @@
+-- functionextraction_extract_redefine_7_2 --
+package extract
+
+import "strconv"
+
+func _() {
+ i, err := strconv.Atoi("1")
+ u, err := fn0() //@extractfunc("u", ")")
+ if i == u || err == nil {
+ return
+ }
+}
+
+func fn0() (int, error) {
+ u, err := strconv.Atoi("2")
+ return u, err
+}
+
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go
index 01c8357..1ff24da 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go
@@ -2,9 +2,9 @@
func _() bool {
x := 1
- if x == 0 { //@mark(s0, "if")
+ if x == 0 { //@mark(exSt2, "if")
return true
- } //@mark(e0, "}")
+ } //@mark(exEn2, "}")
return false
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt2, exEn2)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go.golden
index 6e14a69..b1a27b7 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_basic.go.golden
@@ -3,15 +3,15 @@
func _() bool {
x := 1
- cond0, ret0 := x0(x)
+ cond0, ret0 := fn0(x)
if cond0 {
return ret0
- } //@mark(e0, "}")
+ } //@mark(exEn2, "}")
return false
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt2, exEn2)
}
-func x0(x int) (bool, bool) {
+func fn0(x int) (bool, bool) {
if x == 0 {
return true, true
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go
index af65602..605c5ec 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go
@@ -5,13 +5,13 @@
func _() (int, string, error) {
x := 1
y := "hello"
- z := "bye" //@mark(s0, "z")
+ z := "bye" //@mark(exSt3, "z")
if y == z {
return x, y, fmt.Errorf("same")
} else {
z = "hi"
return x, z, nil
- } //@mark(e0, "}")
+ } //@mark(exEn3, "}")
return x, z, nil
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt3, exEn3)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go.golden
index 0f56433..2fee5fb 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_complex.go.golden
@@ -6,15 +6,15 @@
func _() (int, string, error) {
x := 1
y := "hello"
- z, cond0, ret0, ret1, ret2 := x0(y, x)
+ z, cond0, ret0, ret1, ret2 := fn0(y, x)
if cond0 {
return ret0, ret1, ret2
- } //@mark(e0, "}")
+ } //@mark(exEn3, "}")
return x, z, nil
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt3, exEn3)
}
-func x0(y string, x int) (string, bool, int, string, error) {
+func fn0(y string, x int) (string, bool, int, string, error) {
z := "bye"
if y == z {
return "", true, x, y, fmt.Errorf("same")
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go
index 8e3171e..b3fb4fd 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go
@@ -4,10 +4,10 @@
func _() {
ast.Inspect(ast.NewIdent("a"), func(n ast.Node) bool {
- if n == nil { //@mark(s0, "if")
+ if n == nil { //@mark(exSt4, "if")
return true
- } //@mark(e0, "}")
+ } //@mark(exEn4, "}")
return false
})
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt4, exEn4)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go.golden
index 042ebab..6c4fe96 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_func_lit.go.golden
@@ -5,16 +5,16 @@
func _() {
ast.Inspect(ast.NewIdent("a"), func(n ast.Node) bool {
- cond0, ret0 := x0(n)
+ cond0, ret0 := fn0(n)
if cond0 {
return ret0
- } //@mark(e0, "}")
+ } //@mark(exEn4, "}")
return false
})
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt4, exEn4)
}
-func x0(n ast.Node) (bool, bool) {
+func fn0(n ast.Node) (bool, bool) {
if n == nil {
return true, true
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go
index 00fe065..c1994c1 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go
@@ -2,11 +2,11 @@
func _() string {
x := 1
- if x == 0 { //@mark(s0, "if")
+ if x == 0 { //@mark(exSt5, "if")
x = 3
return "a"
- } //@mark(e0, "}")
+ } //@mark(exEn5, "}")
x = 2
return "b"
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt5, exEn5)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go.golden
index f5f8f0f..40a9773 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_return_init.go.golden
@@ -3,22 +3,20 @@
func _() string {
x := 1
- var cond0 bool
- var ret0 string
- x, cond0, ret0 = fn0(x)
+ cond0, ret0 := fn0(x)
if cond0 {
return ret0
- } //@mark(e0, "}")
+ } //@mark(exEn5, "}")
x = 2
return "b"
- //@extractfunc(s0, e0)
+ //@extractfunc(exSt5, exEn5)
}
-func fn0(x int) (int, bool, string) {
+func fn0(x int) (bool, string) {
if x == 0 {
x = 3
- return 0, true, "a"
+ return true, "a"
}
- return x, false, ""
+ return false, ""
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go
index 1e33e13..da2c669 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go
@@ -2,8 +2,8 @@
func _() {
var a []int
- a = append(a, 2) //@mark(s4, "a")
- b := 4 //@mark(e4, "4")
- //@extractfunc(s4, e4)
+ a = append(a, 2) //@mark(exSt6, "a")
+ b := 4 //@mark(exEn6, "4")
+ //@extractfunc(exSt6, exEn6)
a = append(a, b)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go.golden
index 943e51f..d0b1d7a 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_initialization.go.golden
@@ -3,9 +3,8 @@
func _() {
var a []int
- var b int
- a, b = fn0(a) //@mark(e4, "4")
- //@extractfunc(s4, e4)
+ a, b := fn0(a) //@mark(exEn6, "4")
+ //@extractfunc(exSt6, exEn6)
a = append(a, b)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go
index 5f0d28f..264d680 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go
@@ -3,9 +3,9 @@
func _() {
var b []int
var a int
- a = 2 //@mark(s2, "a")
+ a = 2 //@mark(exSt7, "a")
b = []int{}
- b = append(b, a) //@mark(e2, ")")
+ b = append(b, a) //@mark(exEn7, ")")
b[0] = 1
- //@extractfunc(s2, e2)
+ //@extractfunc(exSt7, exEn7)
}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go.golden
index 6aa4c70..4c361ca 100644
--- a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_smart_return.go.golden
@@ -4,9 +4,9 @@
func _() {
var b []int
var a int
- b = fn0(a, b) //@mark(e2, ")")
+ b = fn0(a, b) //@mark(exEn7, ")")
b[0] = 1
- //@extractfunc(s2, e2)
+ //@extractfunc(exSt7, exEn7)
}
func fn0(a int, b []int) []int {
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go
new file mode 100644
index 0000000..a6eb1f8
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go
@@ -0,0 +1,14 @@
+package extract
+
+func _() {
+ var b []int
+ var a int
+ a := 2 //@mark(exSt8, "a")
+ b = []int{}
+ b = append(b, a) //@mark(exEn8, ")")
+ b[0] = 1
+ if a == 2 {
+ return
+ }
+ //@extractfunc(exSt8, exEn8)
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go.golden b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go.golden
new file mode 100644
index 0000000..f04c212
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/extract/extract_function/extract_unnecessary_param.go.golden
@@ -0,0 +1,21 @@
+-- functionextraction_extract_unnecessary_param_6_2 --
+package extract
+
+func _() {
+ var b []int
+ var a int
+ a, b = fn0(b) //@mark(exEn8, ")")
+ b[0] = 1
+ if a == 2 {
+ return
+ }
+ //@extractfunc(exSt8, exEn8)
+}
+
+func fn0(b []int) (int, []int) {
+ a := 2
+ b = []int{}
+ b = append(b, a)
+ return a, b
+}
+
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 019516e..91adf03 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -12,7 +12,7 @@
FormatCount = 6
ImportCount = 8
SuggestedFixCount = 31
-FunctionExtractionCount = 5
+FunctionExtractionCount = 11
DefinitionsCount = 63
TypeDefinitionsCount = 2
HighlightsCount = 69
