From 6756e7383459ac35aec013e32c0f02da06afe9b1 Mon Sep 17 00:00:00 2001 From: Josh Baum Date: Fri, 7 Aug 2020 13:48:34 -0400 Subject: [PATCH] internal/lsp: fix bug in extract function when highlighting full line We did not adjust the range in extractFunction(). We only adjusted the range in canExtractFunction(). Change-Id: Idc1eab775988ab61be6df8b4afd4b1a36a8bb0ff Reviewed-on: https://go-review.googlesource.com/c/tools/+/247405 Run-TryBot: Josh Baum Reviewed-by: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/source/command.go | 2 +- internal/lsp/source/extract.go | 22 +++++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/internal/lsp/source/command.go b/internal/lsp/source/command.go index 117f3dbc92..e92bbaac69 100644 --- a/internal/lsp/source/command.go +++ b/internal/lsp/source/command.go @@ -123,7 +123,7 @@ var ( Title: "Extract to function", suggestedFixFn: extractFunction, appliesFn: func(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, _ *types.Package, info *types.Info) bool { - _, _, _, _, ok, _ := canExtractFunction(fset, rng, src, file, info) + _, _, _, _, _, ok, _ := canExtractFunction(fset, rng, src, file, info) return ok }, } diff --git a/internal/lsp/source/extract.go b/internal/lsp/source/extract.go index 06f293bb60..9076e8a5cb 100644 --- a/internal/lsp/source/extract.go +++ b/internal/lsp/source/extract.go @@ -156,7 +156,7 @@ type returnVariable struct { // of the function and insert this call as well as the extracted function into // their proper locations. func extractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, pkg *types.Package, info *types.Info) (*analysis.SuggestedFix, error) { - tok, path, outer, start, ok, err := canExtractFunction(fset, rng, src, file, info) + tok, path, rng, outer, start, ok, err := canExtractFunction(fset, rng, src, file, info) if !ok { return nil, fmt.Errorf("extractFunction: cannot extract %s: %v", fset.Position(rng.Start), err) @@ -733,25 +733,28 @@ func referencesObj(info *types.Info, expr ast.Expr, obj types.Object) bool { } // canExtractFunction reports whether the code in the given range can be extracted to a function. -func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, info *types.Info) (*token.File, []ast.Node, *ast.FuncDecl, ast.Node, bool, error) { +func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *ast.File, info *types.Info) (*token.File, []ast.Node, span.Range, *ast.FuncDecl, ast.Node, bool, error) { if rng.Start == rng.End { - return nil, nil, nil, nil, false, fmt.Errorf("start and end are equal") + return nil, nil, span.Range{}, nil, nil, false, + fmt.Errorf("start and end are equal") } tok := fset.File(file.Pos()) if tok == nil { - return nil, nil, nil, nil, false, + return nil, nil, span.Range{}, nil, nil, false, fmt.Errorf("no file for pos %v", fset.Position(file.Pos())) } rng = adjustRangeForWhitespace(rng, tok, src) path, _ := astutil.PathEnclosingInterval(file, rng.Start, rng.End) if len(path) == 0 { - return nil, nil, nil, nil, false, fmt.Errorf("no path enclosing interval") + return nil, nil, span.Range{}, nil, nil, false, + fmt.Errorf("no path enclosing interval") } // Node that encloses the selection must be a statement. // TODO: Support function extraction for an expression. _, ok := path[0].(ast.Stmt) if !ok { - return nil, nil, nil, nil, false, fmt.Errorf("node is not a statement") + return nil, nil, span.Range{}, nil, nil, false, + fmt.Errorf("node is not a statement") } // Find the function declaration that encloses the selection. @@ -763,7 +766,7 @@ func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *a } } if outer == nil { - return nil, nil, nil, nil, false, fmt.Errorf("no enclosing function") + return nil, nil, span.Range{}, nil, nil, false, fmt.Errorf("no enclosing function") } // Find the nodes at the start and end of the selection. @@ -783,9 +786,10 @@ func canExtractFunction(fset *token.FileSet, rng span.Range, src []byte, file *a return n.Pos() <= rng.End }) if start == nil || end == nil { - return nil, nil, nil, nil, false, fmt.Errorf("range does not map to AST nodes") + return nil, nil, span.Range{}, nil, nil, false, + fmt.Errorf("range does not map to AST nodes") } - return tok, path, outer, start, true, nil + return tok, path, rng, outer, start, true, nil } // objUsed checks if the object is used within the range. It returns the first occurence of