diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 44467411ac..3e16515666 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -493,7 +493,15 @@ func commandToEdits(ctx context.Context, snapshot source.Snapshot, fh source.Ver 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 @@ func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span 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 c3593d9376..06f293bb60 100644 --- a/internal/lsp/source/extract.go +++ b/internal/lsp/source/extract.go @@ -180,7 +180,7 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. // 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. } 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. } // 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 @@ func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast. } // 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 @@ func findParent(start ast.Node, target ast.Node) ast.Node { // 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, // 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, 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 @@ func collectFreeVars(info *types.Info, file *ast.File, fileScope *types.Scope, } } 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 @@ func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *a 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 @@ func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *a 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 @@ func adjustReturnStatements(returnTypes []*ast.Field, seenVars map[types.Object] 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 @@ func generateFuncCall(hasReturnVals bool, params, returns []ast.Expr, name strin // 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 @@ func initializeVars(uninitialized []types.Object, returns []ast.Expr, retVars [] } 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 fc46f96883..0c380113dc 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 @@ package extract 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 b528db9274..04caef266b 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 @@ package extract 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 32cbcf10c3..b5b9efd6c1 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 5a8fb43854..16a786354e 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 0000000000..604f4757cc --- /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 0000000000..e739e66976 --- /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 01c8357c69..1ff24daebd 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 @@ package extract 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 6e14a69ddb..b1a27b75c3 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 @@ package extract 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 af65602c3a..605c5ec2e3 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 @@ import "fmt" 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 0f564339e4..2fee5fbea3 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 @@ import "fmt" 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 8e3171ea7a..b3fb4fd219 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 @@ import "go/ast" 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 042ebab474..6c4fe96fa4 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 @@ import "go/ast" 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 00fe065cd4..c1994c1c1f 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 @@ package extract 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 f5f8f0f404..40a9773c6b 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 @@ package extract 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 1e33e13c8a..da2c669a8d 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 @@ package extract 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 943e51f2d6..d0b1d7aef5 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 @@ package extract 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 5f0d28f7ff..264d680e20 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 @@ package extract 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 6aa4c70425..4c361ca0ea 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 @@ package extract 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 0000000000..a6eb1f8728 --- /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 0000000000..f04c21296a --- /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 019516eb3a..91adf03428 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -12,7 +12,7 @@ FoldingRangesCount = 2 FormatCount = 6 ImportCount = 8 SuggestedFixCount = 31 -FunctionExtractionCount = 5 +FunctionExtractionCount = 11 DefinitionsCount = 63 TypeDefinitionsCount = 2 HighlightsCount = 69