@@ -26,48 +26,72 @@ import (
2626 "golang.org/x/tools/internal/typesinternal"
2727)
2828
29+ // extractVariable implements the refactor.extract.{variable,constant} CodeAction command.
2930func extractVariable (fset * token.FileSet , start , end token.Pos , src []byte , file * ast.File , pkg * types.Package , info * types.Info ) (* token.FileSet , * analysis.SuggestedFix , error ) {
3031 tokFile := fset .File (file .FileStart )
31- expr , path , ok , err := canExtractVariable (info , file , start , end )
32- if ! ok {
33- return nil , nil , fmt .Errorf ("extractVariable: cannot extract %s: %v" , safetoken .StartPosition (fset , start ), err )
32+ expr , path , err := canExtractVariable (info , file , start , end )
33+ if err != nil {
34+ return nil , nil , fmt .Errorf ("cannot extract %s: %v" , safetoken .StartPosition (fset , start ), err )
3435 }
36+ constant := info .Types [expr ].Value != nil
3537
3638 // Create new AST node for extracted expression.
3739 var lhsNames []string
3840 switch expr := expr .(type ) {
3941 case * ast.CallExpr :
4042 tup , ok := info .TypeOf (expr ).(* types.Tuple )
4143 if ! ok {
42- // If the call expression only has one return value, we can treat it the
43- // same as our standard extract variable case.
44- lhsName , _ := generateAvailableName (expr .Pos (), path , pkg , info , "x" , 0 )
45- lhsNames = append (lhsNames , lhsName )
46- break
47- }
48- idx := 0
49- for i := 0 ; i < tup .Len (); i ++ {
50- // Generate a unique variable for each return value.
51- var lhsName string
52- lhsName , idx = generateAvailableName (expr .Pos (), path , pkg , info , "x" , idx )
53- lhsNames = append (lhsNames , lhsName )
44+ // conversion or single-valued call:
45+ // treat it the same as our standard extract variable case.
46+ name , _ := generateAvailableName (expr .Pos (), path , pkg , info , "x" , 0 )
47+ lhsNames = append (lhsNames , name )
48+
49+ } else {
50+ // call with multiple results
51+ idx := 0
52+ for range tup .Len () {
53+ // Generate a unique variable for each result.
54+ var name string
55+ name , idx = generateAvailableName (expr .Pos (), path , pkg , info , "x" , idx )
56+ lhsNames = append (lhsNames , name )
57+ }
5458 }
5559
5660 default :
5761 // TODO: stricter rules for selectorExpr.
58- lhsName , _ := generateAvailableName (expr .Pos (), path , pkg , info , "x" , 0 )
59- lhsNames = append (lhsNames , lhsName )
62+ name , _ := generateAvailableName (expr .Pos (), path , pkg , info , "x" , 0 )
63+ lhsNames = append (lhsNames , name )
6064 }
6165
6266 // TODO: There is a bug here: for a variable declared in a labeled
6367 // switch/for statement it returns the for/switch statement itself
64- // which produces the below code which is a compiler error e.g.
65- // label:
66- // switch r1 := r() { ... break label ... }
68+ // which produces the below code which is a compiler error. e.g.
69+ // label:
70+ // switch r1 := r() { ... break label ... }
6771 // On extracting "r()" to a variable
68- // label:
69- // x := r()
70- // switch r1 := x { ... break label ... } // compiler error
72+ // label:
73+ // x := r()
74+ // switch r1 := x { ... break label ... } // compiler error
75+ //
76+ // TODO(golang/go#70563): Another bug: extracting the
77+ // expression to the recommended place may cause it to migrate
78+ // across one or more declarations that it references.
79+ //
80+ // Before:
81+ // if x := 1; cond {
82+ // } else if y := «x + 2»; cond {
83+ // }
84+ //
85+ // After:
86+ // x1 := x + 2 // error: undefined x
87+ // if x := 1; cond {
88+ // } else if y := x1; cond {
89+ // }
90+ //
91+ // TODO(golang/go#70665): Another bug (or limitation): this
92+ // operation fails at top-level:
93+ // package p
94+ // var x = «1 + 2» // error
7195 insertBeforeStmt := analysisinternal .StmtToInsertVarBefore (path )
7296 if insertBeforeStmt == nil {
7397 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
78102 }
79103 newLineIndent := "\n " + indent
80104
81- lhs := strings .Join (lhsNames , ", " )
82- assignStmt := & ast.AssignStmt {
83- Lhs : []ast.Expr {ast .NewIdent (lhs )},
84- Tok : token .DEFINE ,
85- Rhs : []ast.Expr {expr },
105+ // Create statement to declare extracted var/const.
106+ //
107+ // TODO(adonovan): beware the const decls are not valid short
108+ // statements, so if fixing #70563 causes
109+ // StmtToInsertVarBefore to evolve to permit declarations in
110+ // the "pre" part of an IfStmt, like so:
111+ // Before:
112+ // if cond {
113+ // } else if «1 + 2» > 0 {
114+ // }
115+ // After:
116+ // if x := 1 + 2; cond {
117+ // } else if x > 0 {
118+ // }
119+ // then it will need to become aware that this is invalid
120+ // for constants.
121+ //
122+ // Conversely, a short var decl stmt is not valid at top level,
123+ // so when we fix #70665, we'll need to use a var decl.
124+ var declStmt ast.Stmt
125+ if constant {
126+ // const x = expr
127+ declStmt = & ast.DeclStmt {
128+ Decl : & ast.GenDecl {
129+ Tok : token .CONST ,
130+ Specs : []ast.Spec {
131+ & ast.ValueSpec {
132+ Names : []* ast.Ident {ast .NewIdent (lhsNames [0 ])}, // there can be only one
133+ Values : []ast.Expr {expr },
134+ },
135+ },
136+ },
137+ }
138+
139+ } else {
140+ // var: x1, ... xn := expr
141+ var lhs []ast.Expr
142+ for _ , name := range lhsNames {
143+ lhs = append (lhs , ast .NewIdent (name ))
144+ }
145+ declStmt = & ast.AssignStmt {
146+ Tok : token .DEFINE ,
147+ Lhs : lhs ,
148+ Rhs : []ast.Expr {expr },
149+ }
86150 }
151+
152+ // Format and indent the declaration.
87153 var buf bytes.Buffer
88- if err := format .Node (& buf , fset , assignStmt ); err != nil {
154+ if err := format .Node (& buf , fset , declStmt ); err != nil {
89155 return nil , nil , err
90156 }
157+ // TODO(adonovan): not sound for `...` string literals containing newlines.
91158 assignment := strings .ReplaceAll (buf .String (), "\n " , newLineIndent ) + newLineIndent
92159
93160 return fset , & analysis.SuggestedFix {
@@ -100,39 +167,39 @@ func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, file
100167 {
101168 Pos : start ,
102169 End : end ,
103- NewText : []byte (lhs ),
170+ NewText : []byte (strings . Join ( lhsNames , ", " ) ),
104171 },
105172 },
106173 }, nil
107174}
108175
109176// canExtractVariable reports whether the code in the given range can be
110- // extracted to a variable.
111- func canExtractVariable (info * types.Info , file * ast.File , start , end token.Pos ) (ast.Expr , []ast.Node , bool , error ) {
177+ // extracted to a variable (or constant) .
178+ func canExtractVariable (info * types.Info , file * ast.File , start , end token.Pos ) (ast.Expr , []ast.Node , error ) {
112179 if start == end {
113- return nil , nil , false , fmt .Errorf ("empty selection" )
180+ return nil , nil , fmt .Errorf ("empty selection" )
114181 }
115182 path , exact := astutil .PathEnclosingInterval (file , start , end )
116183 if ! exact {
117- return nil , nil , false , fmt .Errorf ("selection is not an expression" )
184+ return nil , nil , fmt .Errorf ("selection is not an expression" )
118185 }
119186 if len (path ) == 0 {
120- return nil , nil , false , bug .Errorf ("no path enclosing interval" )
187+ return nil , nil , bug .Errorf ("no path enclosing interval" )
121188 }
122189 for _ , n := range path {
123190 if _ , ok := n .(* ast.ImportSpec ); ok {
124- return nil , nil , false , fmt .Errorf ("cannot extract variable in an import block" )
191+ return nil , nil , fmt .Errorf ("cannot extract variable or constant in an import block" )
125192 }
126193 }
127194 expr , ok := path [0 ].(ast.Expr )
128195 if ! ok {
129- return nil , nil , false , fmt .Errorf ("selection is not an expression" ) // e.g. statement
196+ return nil , nil , fmt .Errorf ("selection is not an expression" ) // e.g. statement
130197 }
131198 if tv , ok := info .Types [expr ]; ! ok || ! tv .IsValue () || tv .Type == nil || tv .HasOk () {
132199 // e.g. type, builtin, x.(type), 2-valued m[k], or ill-typed
133- return nil , nil , false , fmt .Errorf ("selection is not a single-valued expression" )
200+ return nil , nil , fmt .Errorf ("selection is not a single-valued expression" )
134201 }
135- return expr , path , true , nil
202+ return expr , path , nil
136203}
137204
138205// Calculate indentation for insertion.
0 commit comments