From 61b24083cf03354aeb61c16dceca74b0ff1541dc Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 3 Dec 2024 15:51:20 -0500 Subject: [PATCH] gopls/internal/golang: add "Extract constant" counterpart If the selection is a constant expression, instead of "Extract variable", gopls now offers "Extract constant", and generates a constant declaration. Regrettably, I noticed pending CL 564338 only while composing this CL description; this CL supersedes that one. Apologies for the redundant efforts. + Test, doc, relnote Fixes golang/go#37170 Updates golang/go#63852 Change-Id: I1376662b50820936de1e156413537e0bcc2292ff Reviewed-on: https://go-review.googlesource.com/c/tools/+/633257 Auto-Submit: Alan Donovan Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI --- gopls/doc/features/transformation.md | 8 +- gopls/doc/release/v0.17.0.md | 6 + gopls/internal/cmd/codeaction.go | 1 + gopls/internal/cmd/usage/codeaction.hlp | 1 + gopls/internal/golang/codeaction.go | 21 ++- gopls/internal/golang/extract.go | 145 +++++++++++++----- gopls/internal/golang/fix.go | 2 +- gopls/internal/settings/codeactionkind.go | 1 + gopls/internal/settings/default.go | 1 + .../codeaction/extract_variable-if.txt | 41 +++++ .../codeaction/extract_variable-inexact.txt | 10 +- .../testdata/codeaction/extract_variable.txt | 26 ++-- .../codeaction/extract_variable_resolve.txt | 26 ++-- internal/analysisinternal/analysis.go | 12 +- 14 files changed, 220 insertions(+), 81 deletions(-) create mode 100644 gopls/internal/test/marker/testdata/codeaction/extract_variable-if.txt diff --git a/gopls/doc/features/transformation.md b/gopls/doc/features/transformation.md index e0f48d6092e..bfc30eb21d0 100644 --- a/gopls/doc/features/transformation.md +++ b/gopls/doc/features/transformation.md @@ -69,6 +69,7 @@ Gopls supports the following code actions: - `source.test` (undocumented) - [`source.addTest`](#source.addTest) - [`gopls.doc.features`](README.md), which opens gopls' index of features in a browser +- [`refactor.extract.constant`](#extract) - [`refactor.extract.function`](#extract) - [`refactor.extract.method`](#extract) - [`refactor.extract.toNewFile`](#extract.toNewFile) @@ -358,6 +359,9 @@ newly created declaration that contains the selected code: ![Before extracting a var](../assets/extract-var-before.png) ![After extracting a var](../assets/extract-var-after.png) +- **`refactor.extract.constant** does the same thing for a constant + expression, introducing a local const declaration. + If the default name for the new declaration is already in use, gopls generates a fresh name. @@ -380,11 +384,9 @@ number of cases where it falls short, including: The following Extract features are planned for 2024 but not yet supported: -- **Extract constant** is a variant of "Extract variable" to be - offered when the expression is constant; see golang/go#37170. - **Extract parameter struct** will replace two or more parameters of a function by a struct type with one field per parameter; see golang/go#65552. - + - **Extract interface for type** will create a declaration of an interface type with all the methods of the selected concrete type; diff --git a/gopls/doc/release/v0.17.0.md b/gopls/doc/release/v0.17.0.md index dcd2ef06502..9728a317620 100644 --- a/gopls/doc/release/v0.17.0.md +++ b/gopls/doc/release/v0.17.0.md @@ -38,6 +38,12 @@ or by selecting a whole declaration or multiple declarations. In order to avoid ambiguity and surprise about what to extract, some kinds of paritial selection of a declaration cannot invoke this code action. +## Extract constant + +When the selection is a constant expression, gopls now offers "Extract +constant" instead of "Extract variable", and generates a `const` +declaration instead of a local variable. + ## Pull diagnostics When initialized with the option `"pullDiagnostics": true`, gopls will advertise support for the diff --git a/gopls/internal/cmd/codeaction.go b/gopls/internal/cmd/codeaction.go index c349c7ab653..2096a153681 100644 --- a/gopls/internal/cmd/codeaction.go +++ b/gopls/internal/cmd/codeaction.go @@ -51,6 +51,7 @@ Valid kinds include: quickfix refactor refactor.extract + refactor.extract.constant refactor.extract.function refactor.extract.method refactor.extract.toNewFile diff --git a/gopls/internal/cmd/usage/codeaction.hlp b/gopls/internal/cmd/usage/codeaction.hlp index 6d6923ef458..d7bfe3ea99e 100644 --- a/gopls/internal/cmd/usage/codeaction.hlp +++ b/gopls/internal/cmd/usage/codeaction.hlp @@ -22,6 +22,7 @@ Valid kinds include: quickfix refactor refactor.extract + refactor.extract.constant refactor.extract.function refactor.extract.method refactor.extract.toNewFile diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go index 691b54634a8..0a778ba758b 100644 --- a/gopls/internal/golang/codeaction.go +++ b/gopls/internal/golang/codeaction.go @@ -236,6 +236,7 @@ var codeActionProducers = [...]codeActionProducer{ {kind: settings.RefactorExtractFunction, fn: refactorExtractFunction}, {kind: settings.RefactorExtractMethod, fn: refactorExtractMethod}, {kind: settings.RefactorExtractToNewFile, fn: refactorExtractToNewFile}, + {kind: settings.RefactorExtractConstant, fn: refactorExtractVariable, needPkg: true}, {kind: settings.RefactorExtractVariable, fn: refactorExtractVariable, needPkg: true}, {kind: settings.RefactorInlineCall, fn: refactorInlineCall, needPkg: true}, {kind: settings.RefactorRewriteChangeQuote, fn: refactorRewriteChangeQuote}, @@ -462,11 +463,25 @@ func refactorExtractMethod(ctx context.Context, req *codeActionsRequest) error { return nil } -// refactorExtractVariable produces "Extract variable" code actions. +// refactorExtractVariable produces "Extract variable|constant" code actions. // See [extractVariable] for command implementation. func refactorExtractVariable(ctx context.Context, req *codeActionsRequest) error { - if _, _, ok, _ := canExtractVariable(req.pkg.TypesInfo(), req.pgf.File, req.start, req.end); ok { - req.addApplyFixAction("Extract variable", fixExtractVariable, req.loc) + info := req.pkg.TypesInfo() + if expr, _, err := canExtractVariable(info, req.pgf.File, req.start, req.end); err == nil { + // Offer one of refactor.extract.{constant,variable} + // based on the constness of the expression; this is a + // limitation of the codeActionProducers mechanism. + // Beware that future evolutions of the refactorings + // may make them diverge to become non-complementary, + // for example because "if const x = ...; y {" is illegal. + constant := info.Types[expr].Value != nil + if (req.kind == settings.RefactorExtractConstant) == constant { + title := "Extract variable" + if constant { + title = "Extract constant" + } + req.addApplyFixAction(title, fixExtractVariable, req.loc) + } } return nil } diff --git a/gopls/internal/golang/extract.go b/gopls/internal/golang/extract.go index 28ff3706412..27dc2d9e380 100644 --- a/gopls/internal/golang/extract.go +++ b/gopls/internal/golang/extract.go @@ -26,12 +26,14 @@ import ( "golang.org/x/tools/internal/typesinternal" ) +// extractVariable implements the refactor.extract.{variable,constant} CodeAction command. func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, file *ast.File, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) { tokFile := fset.File(file.FileStart) - expr, path, ok, err := canExtractVariable(info, file, start, end) - if !ok { - return nil, nil, fmt.Errorf("extractVariable: cannot extract %s: %v", safetoken.StartPosition(fset, start), err) + expr, path, err := canExtractVariable(info, file, start, end) + if err != nil { + return nil, nil, fmt.Errorf("cannot extract %s: %v", safetoken.StartPosition(fset, start), err) } + constant := info.Types[expr].Value != nil // Create new AST node for extracted expression. var lhsNames []string @@ -39,35 +41,57 @@ func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, file case *ast.CallExpr: tup, ok := info.TypeOf(expr).(*types.Tuple) if !ok { - // If the call expression only has one return value, we can treat it the - // same as our standard extract variable case. - lhsName, _ := generateAvailableName(expr.Pos(), path, pkg, info, "x", 0) - lhsNames = append(lhsNames, lhsName) - break - } - idx := 0 - for i := 0; i < tup.Len(); i++ { - // Generate a unique variable for each return value. - var lhsName string - lhsName, idx = generateAvailableName(expr.Pos(), path, pkg, info, "x", idx) - lhsNames = append(lhsNames, lhsName) + // conversion or single-valued call: + // treat it the same as our standard extract variable case. + name, _ := generateAvailableName(expr.Pos(), path, pkg, info, "x", 0) + lhsNames = append(lhsNames, name) + + } else { + // call with multiple results + idx := 0 + for range tup.Len() { + // Generate a unique variable for each result. + var name string + name, idx = generateAvailableName(expr.Pos(), path, pkg, info, "x", idx) + lhsNames = append(lhsNames, name) + } } default: // TODO: stricter rules for selectorExpr. - lhsName, _ := generateAvailableName(expr.Pos(), path, pkg, info, "x", 0) - lhsNames = append(lhsNames, lhsName) + name, _ := generateAvailableName(expr.Pos(), path, pkg, info, "x", 0) + lhsNames = append(lhsNames, name) } // TODO: There is a bug here: for a variable declared in a labeled // switch/for statement it returns the for/switch statement itself - // which produces the below code which is a compiler error e.g. - // label: - // switch r1 := r() { ... break label ... } + // which produces the below code which is a compiler error. e.g. + // label: + // switch r1 := r() { ... break label ... } // On extracting "r()" to a variable - // label: - // x := r() - // switch r1 := x { ... break label ... } // compiler error + // label: + // x := r() + // switch r1 := x { ... break label ... } // compiler error + // + // TODO(golang/go#70563): Another bug: extracting the + // expression to the recommended place may cause it to migrate + // across one or more declarations that it references. + // + // Before: + // if x := 1; cond { + // } else if y := «x + 2»; cond { + // } + // + // After: + // x1 := x + 2 // error: undefined x + // if x := 1; cond { + // } else if y := x1; cond { + // } + // + // TODO(golang/go#70665): Another bug (or limitation): this + // operation fails at top-level: + // package p + // var x = «1 + 2» // error insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(path) if insertBeforeStmt == nil { return nil, nil, fmt.Errorf("cannot find location to insert extraction") @@ -78,16 +102,59 @@ func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, file } newLineIndent := "\n" + indent - lhs := strings.Join(lhsNames, ", ") - assignStmt := &ast.AssignStmt{ - Lhs: []ast.Expr{ast.NewIdent(lhs)}, - Tok: token.DEFINE, - Rhs: []ast.Expr{expr}, + // Create statement to declare extracted var/const. + // + // TODO(adonovan): beware the const decls are not valid short + // statements, so if fixing #70563 causes + // StmtToInsertVarBefore to evolve to permit declarations in + // the "pre" part of an IfStmt, like so: + // Before: + // if cond { + // } else if «1 + 2» > 0 { + // } + // After: + // if x := 1 + 2; cond { + // } else if x > 0 { + // } + // then it will need to become aware that this is invalid + // for constants. + // + // Conversely, a short var decl stmt is not valid at top level, + // so when we fix #70665, we'll need to use a var decl. + var declStmt ast.Stmt + if constant { + // const x = expr + declStmt = &ast.DeclStmt{ + Decl: &ast.GenDecl{ + Tok: token.CONST, + Specs: []ast.Spec{ + &ast.ValueSpec{ + Names: []*ast.Ident{ast.NewIdent(lhsNames[0])}, // there can be only one + Values: []ast.Expr{expr}, + }, + }, + }, + } + + } else { + // var: x1, ... xn := expr + var lhs []ast.Expr + for _, name := range lhsNames { + lhs = append(lhs, ast.NewIdent(name)) + } + declStmt = &ast.AssignStmt{ + Tok: token.DEFINE, + Lhs: lhs, + Rhs: []ast.Expr{expr}, + } } + + // Format and indent the declaration. var buf bytes.Buffer - if err := format.Node(&buf, fset, assignStmt); err != nil { + if err := format.Node(&buf, fset, declStmt); err != nil { return nil, nil, err } + // TODO(adonovan): not sound for `...` string literals containing newlines. assignment := strings.ReplaceAll(buf.String(), "\n", newLineIndent) + newLineIndent return fset, &analysis.SuggestedFix{ @@ -100,39 +167,39 @@ func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, file { Pos: start, End: end, - NewText: []byte(lhs), + NewText: []byte(strings.Join(lhsNames, ", ")), }, }, }, nil } // canExtractVariable reports whether the code in the given range can be -// extracted to a variable. -func canExtractVariable(info *types.Info, file *ast.File, start, end token.Pos) (ast.Expr, []ast.Node, bool, error) { +// extracted to a variable (or constant). +func canExtractVariable(info *types.Info, file *ast.File, start, end token.Pos) (ast.Expr, []ast.Node, error) { if start == end { - return nil, nil, false, fmt.Errorf("empty selection") + return nil, nil, fmt.Errorf("empty selection") } path, exact := astutil.PathEnclosingInterval(file, start, end) if !exact { - return nil, nil, false, fmt.Errorf("selection is not an expression") + return nil, nil, fmt.Errorf("selection is not an expression") } if len(path) == 0 { - return nil, nil, false, bug.Errorf("no path enclosing interval") + return nil, nil, bug.Errorf("no path enclosing interval") } for _, n := range path { if _, ok := n.(*ast.ImportSpec); ok { - return nil, nil, false, fmt.Errorf("cannot extract variable in an import block") + return nil, nil, fmt.Errorf("cannot extract variable or constant in an import block") } } expr, ok := path[0].(ast.Expr) if !ok { - return nil, nil, false, fmt.Errorf("selection is not an expression") // e.g. statement + return nil, nil, fmt.Errorf("selection is not an expression") // e.g. statement } if tv, ok := info.Types[expr]; !ok || !tv.IsValue() || tv.Type == nil || tv.HasOk() { // e.g. type, builtin, x.(type), 2-valued m[k], or ill-typed - return nil, nil, false, fmt.Errorf("selection is not a single-valued expression") + return nil, nil, fmt.Errorf("selection is not a single-valued expression") } - return expr, path, true, nil + return expr, path, nil } // Calculate indentation for insertion. diff --git a/gopls/internal/golang/fix.go b/gopls/internal/golang/fix.go index 68110206add..f88343f029c 100644 --- a/gopls/internal/golang/fix.go +++ b/gopls/internal/golang/fix.go @@ -58,7 +58,7 @@ func singleFile(fixer1 singleFileFixer) fixer { // Names of ApplyFix.Fix created directly by the CodeAction handler. const ( - fixExtractVariable = "extract_variable" + fixExtractVariable = "extract_variable" // (or constant) fixExtractFunction = "extract_function" fixExtractMethod = "extract_method" fixInlineCall = "inline_call" diff --git a/gopls/internal/settings/codeactionkind.go b/gopls/internal/settings/codeactionkind.go index c0a8cb5f4e6..7bc4f4e4d66 100644 --- a/gopls/internal/settings/codeactionkind.go +++ b/gopls/internal/settings/codeactionkind.go @@ -99,6 +99,7 @@ const ( RefactorInlineCall protocol.CodeActionKind = "refactor.inline.call" // refactor.extract + RefactorExtractConstant protocol.CodeActionKind = "refactor.extract.constant" RefactorExtractFunction protocol.CodeActionKind = "refactor.extract.function" RefactorExtractMethod protocol.CodeActionKind = "refactor.extract.method" RefactorExtractVariable protocol.CodeActionKind = "refactor.extract.variable" diff --git a/gopls/internal/settings/default.go b/gopls/internal/settings/default.go index 25f3eae80f5..0354101f045 100644 --- a/gopls/internal/settings/default.go +++ b/gopls/internal/settings/default.go @@ -61,6 +61,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options { RefactorRewriteRemoveUnusedParam: true, RefactorRewriteSplitLines: true, RefactorInlineCall: true, + RefactorExtractConstant: true, RefactorExtractFunction: true, RefactorExtractMethod: true, RefactorExtractVariable: true, diff --git a/gopls/internal/test/marker/testdata/codeaction/extract_variable-if.txt b/gopls/internal/test/marker/testdata/codeaction/extract_variable-if.txt new file mode 100644 index 00000000000..cc4c599d748 --- /dev/null +++ b/gopls/internal/test/marker/testdata/codeaction/extract_variable-if.txt @@ -0,0 +1,41 @@ +This test checks the behavior of the 'extract variable/constant' code actions +when the optimal place for the new declaration is within the "if" statement, +like so: + + if x := 1 + 2 or y + y ; true { + } else if x > 0 { + } + +A future refactor.variable implementation that does this should avoid +using a 'const' declaration, which is not legal at that location. + +-- flags -- +-ignore_extra_diags + +-- a.go -- +package a + +func constant() { + if true { + } else if 1 + 2 > 0 { //@ codeaction("1 + 2", "refactor.extract.constant", edit=constant) + } +} + +func variable(y int) { + if true { + } else if y + y > 0 { //@ codeaction("y + y", "refactor.extract.variable", edit=variable) + } +} + +-- @constant/a.go -- +@@ -4 +4 @@ ++ const x = 1 + 2 +@@ -5 +6 @@ +- } else if 1 + 2 > 0 { //@ codeaction("1 + 2", "refactor.extract.constant", edit=constant) ++ } else if x > 0 { //@ codeaction("1 + 2", "refactor.extract.constant", edit=constant) +-- @variable/a.go -- +@@ -10 +10 @@ ++ x := y + y +@@ -11 +12 @@ +- } else if y + y > 0 { //@ codeaction("y + y", "refactor.extract.variable", edit=variable) ++ } else if x > 0 { //@ codeaction("y + y", "refactor.extract.variable", edit=variable) diff --git a/gopls/internal/test/marker/testdata/codeaction/extract_variable-inexact.txt b/gopls/internal/test/marker/testdata/codeaction/extract_variable-inexact.txt index 4eb859464a5..0a9cab949a5 100644 --- a/gopls/internal/test/marker/testdata/codeaction/extract_variable-inexact.txt +++ b/gopls/internal/test/marker/testdata/codeaction/extract_variable-inexact.txt @@ -1,4 +1,4 @@ -This test checks that extract variable permits: +This test checks that extract variable/constant permits: - extraneous whitespace in the selection - function literals - pointer dereference expressions @@ -8,7 +8,7 @@ This test checks that extract variable permits: package a func _(ptr *int) { - var _ = 1 + 2 + 3 //@codeaction("1 + 2 ", "refactor.extract.variable", edit=spaces) + var _ = 1 + 2 + 3 //@codeaction("1 + 2 ", "refactor.extract.constant", edit=spaces) var _ = func() {} //@codeaction("func() {}", "refactor.extract.variable", edit=funclit) var _ = *ptr //@codeaction("*ptr", "refactor.extract.variable", edit=ptr) var _ = (ptr) //@codeaction("(ptr)", "refactor.extract.variable", edit=paren) @@ -16,9 +16,9 @@ func _(ptr *int) { -- @spaces/a.go -- @@ -4 +4,2 @@ -- var _ = 1 + 2 + 3 //@codeaction("1 + 2 ", "refactor.extract.variable", edit=spaces) -+ x := 1 + 2 -+ var _ = x+ 3 //@codeaction("1 + 2 ", "refactor.extract.variable", edit=spaces) +- var _ = 1 + 2 + 3 //@codeaction("1 + 2 ", "refactor.extract.constant", edit=spaces) ++ const x = 1 + 2 ++ var _ = x+ 3 //@codeaction("1 + 2 ", "refactor.extract.constant", edit=spaces) -- @funclit/a.go -- @@ -5 +5,2 @@ - var _ = func() {} //@codeaction("func() {}", "refactor.extract.variable", edit=funclit) diff --git a/gopls/internal/test/marker/testdata/codeaction/extract_variable.txt b/gopls/internal/test/marker/testdata/codeaction/extract_variable.txt index 0842abf407c..e6f0f893848 100644 --- a/gopls/internal/test/marker/testdata/codeaction/extract_variable.txt +++ b/gopls/internal/test/marker/testdata/codeaction/extract_variable.txt @@ -1,4 +1,4 @@ -This test checks the behavior of the 'extract variable' code action. +This test checks the behavior of the 'extract variable/constant' code action. See extract_variable_resolve.txt for the same test with resolve support. -- flags -- @@ -8,20 +8,20 @@ See extract_variable_resolve.txt for the same test with resolve support. package extract func _() { - var _ = 1 + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) - var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) + var _ = 1 + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) + var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) } -- @basic_lit1/basic_lit.go -- @@ -4 +4,2 @@ -- var _ = 1 + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) -+ x := 1 -+ var _ = x + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) +- var _ = 1 + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) ++ const x = 1 ++ var _ = x + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) -- @basic_lit2/basic_lit.go -- @@ -5 +5,2 @@ -- var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) -+ x := 3 + 4 -+ var _ = x //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) +- var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) ++ const x = 3 + 4 ++ var _ = x //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) -- func_call.go -- package extract @@ -54,7 +54,7 @@ func _() { y := ast.CompositeLit{} //@codeaction("ast.CompositeLit{}", "refactor.extract.variable", edit=scope1) } if true { - x1 := !false //@codeaction("!false", "refactor.extract.variable", edit=scope2) + x1 := !false //@codeaction("!false", "refactor.extract.constant", edit=scope2) } } @@ -65,6 +65,6 @@ func _() { + y := x //@codeaction("ast.CompositeLit{}", "refactor.extract.variable", edit=scope1) -- @scope2/scope.go -- @@ -11 +11,2 @@ -- x1 := !false //@codeaction("!false", "refactor.extract.variable", edit=scope2) -+ x := !false -+ x1 := x //@codeaction("!false", "refactor.extract.variable", edit=scope2) +- x1 := !false //@codeaction("!false", "refactor.extract.constant", edit=scope2) ++ const x = !false ++ x1 := x //@codeaction("!false", "refactor.extract.constant", edit=scope2) diff --git a/gopls/internal/test/marker/testdata/codeaction/extract_variable_resolve.txt b/gopls/internal/test/marker/testdata/codeaction/extract_variable_resolve.txt index 7e444d84720..3ab9b2df0be 100644 --- a/gopls/internal/test/marker/testdata/codeaction/extract_variable_resolve.txt +++ b/gopls/internal/test/marker/testdata/codeaction/extract_variable_resolve.txt @@ -1,4 +1,4 @@ -This test checks the behavior of the 'extract variable' code action, with resolve support. +This test checks the behavior of the 'extract variable/constant' code action, with resolve support. See extract_variable.txt for the same test without resolve support. -- capabilities.json -- @@ -19,20 +19,20 @@ See extract_variable.txt for the same test without resolve support. package extract func _() { - var _ = 1 + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) - var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) + var _ = 1 + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) + var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) } -- @basic_lit1/basic_lit.go -- @@ -4 +4,2 @@ -- var _ = 1 + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) -+ x := 1 -+ var _ = x + 2 //@codeaction("1", "refactor.extract.variable", edit=basic_lit1) +- var _ = 1 + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) ++ const x = 1 ++ var _ = x + 2 //@codeaction("1", "refactor.extract.constant", edit=basic_lit1) -- @basic_lit2/basic_lit.go -- @@ -5 +5,2 @@ -- var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) -+ x := 3 + 4 -+ var _ = x //@codeaction("3 + 4", "refactor.extract.variable", edit=basic_lit2) +- var _ = 3 + 4 //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) ++ const x = 3 + 4 ++ var _ = x //@codeaction("3 + 4", "refactor.extract.constant", edit=basic_lit2) -- func_call.go -- package extract @@ -65,7 +65,7 @@ func _() { y := ast.CompositeLit{} //@codeaction("ast.CompositeLit{}", "refactor.extract.variable", edit=scope1) } if true { - x1 := !false //@codeaction("!false", "refactor.extract.variable", edit=scope2) + x1 := !false //@codeaction("!false", "refactor.extract.constant", edit=scope2) } } @@ -76,6 +76,6 @@ func _() { + y := x //@codeaction("ast.CompositeLit{}", "refactor.extract.variable", edit=scope1) -- @scope2/scope.go -- @@ -11 +11,2 @@ -- x1 := !false //@codeaction("!false", "refactor.extract.variable", edit=scope2) -+ x := !false -+ x1 := x //@codeaction("!false", "refactor.extract.variable", edit=scope2) +- x1 := !false //@codeaction("!false", "refactor.extract.constant", edit=scope2) ++ const x = !false ++ x1 := x //@codeaction("!false", "refactor.extract.constant", edit=scope2) diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go index 042b42e2c43..4190f6d478d 100644 --- a/internal/analysisinternal/analysis.go +++ b/internal/analysisinternal/analysis.go @@ -69,14 +69,18 @@ func TypeErrorEndPos(fset *token.FileSet, src []byte, start token.Pos) token.Pos // Some examples: // // Basic Example: -// z := 1 -// y := z + x +// +// z := 1 +// y := z + x +// // If x is undeclared, then this function would return `y := z + x`, so that we // can insert `x := ` on the line before `y := z + x`. // // If stmt example: -// if z == 1 { -// } else if z == y {} +// +// if z == 1 { +// } else if z == y {} +// // If y is undeclared, then this function would return `if z == 1 {`, because we cannot // insert a statement between an if and an else if statement. As a result, we need to find // the top of the if chain to insert `y := ` before.