refactors

Author: golopot

gopls/doc/release/v0.16.0.md

@@ -94,18 +94,12 @@ TODO(dominikh/go-mode.el#436): add both of these to go-mode.el.
 
 ### Extract declarations to new file
 Gopls now offers another code action, "Extract declarations to new file",
-which moves selected code sections to a newly created file within the
-same package. The created filename is chosen as the first {function, type, 
-const, var} name encountered. In addition, import declarations are added or
-removed as needed.
-
-The user can invoke this code action by selecting a function name, the keywords
-`func`, `const`, `var`, `type`, or by placing the caret on them without selecting,
-or by selecting a whole declaration or multiple declrations.
-
-In order to avoid ambiguity and surprise about what to extract, some kinds
-of paritial selection of a declration cannot invoke this code action.
+which moves the selected declarations to a newly created file within the
+same package. The created file's name is based on the first symbol
+name encountered. Import declarations are added or removed as needed.
 
+You can find this action by selecting one or more complete declarations,
+or just the func, const, var, or type token of any top-level declaration.
 
 ### `unusedwrite` analyzer
 

gopls/internal/golang/codeaction.go

@@ -258,7 +258,7 @@ func getExtractCodeActions(pgf *parsego.File, rng protocol.Range, options *setti
 	if canExtractToNewFile(pgf, start, end) {
 		cmd, err := command.NewExtractToNewFileCommand(
 			"Extract declarations to new file",
-			command.ExtractToNewFileArgs{URI: pgf.URI, Range: rng},
+			protocol.Location{URI: pgf.URI, Range: rng},
 		)
 		if err != nil {
 			return nil, err

gopls/internal/golang/extracttofile.go

@@ -27,8 +27,6 @@ import (
 	"golang.org/x/tools/gopls/internal/util/typesutil"
 )
 
-var ErrDotImport = errors.New("\"extract to new file\" does not support files containing dot imports")
-
 // canExtractToNewFile reports whether the code in the given range can be extracted to a new file.
 func canExtractToNewFile(pgf *parsego.File, start, end token.Pos) bool {
 	_, _, _, ok := selectedToplevelDecls(pgf, start, end)
@@ -39,7 +37,7 @@ func canExtractToNewFile(pgf *parsego.File, start, end token.Pos) bool {
 // or deleted from the old file if the range is extracted to a new file.
 //
 // TODO: handle dot imports.
-func findImportEdits(file *ast.File, info *types.Info, start, end token.Pos) (adds []*ast.ImportSpec, deletes []*ast.ImportSpec, _ error) {
+func findImportEdits(file *ast.File, info *types.Info, start, end token.Pos) (adds, deletes []*ast.ImportSpec, _ error) {
 	// make a map from a pkgName to its references
 	pkgNameReferences := make(map[*types.PkgName][]*ast.Ident)
 	for ident, use := range info.Uses {
@@ -54,7 +52,8 @@ func findImportEdits(file *ast.File, info *types.Info, start, end token.Pos) (ad
 	// deleted from the original file.
 	for _, spec := range file.Imports {
 		if spec.Name != nil && spec.Name.Name == "." {
-			return nil, nil, ErrDotImport
+			// TODO: support dot imports.
+			return nil, nil, errors.New("\"extract to new file\" does not support files containing dot imports")
 		}
 		pkgName, ok := typesutil.ImportedPkgName(info, spec)
 		if !ok {
@@ -96,7 +95,7 @@ func ExtractToNewFile(ctx context.Context, snapshot *cache.Snapshot, fh file.Han
 
 	start, end, firstSymbol, ok := selectedToplevelDecls(pgf, start, end)
 	if !ok {
-		return nil, bug.Errorf("precondition unmet")
+		return nil, bug.Errorf("invalid selection")
 	}
 
 	// select trailing empty lines
@@ -105,7 +104,7 @@ func ExtractToNewFile(ctx context.Context, snapshot *cache.Snapshot, fh file.Han
 
 	replaceRange, err := pgf.PosRange(start, end)
 	if err != nil {
-		return nil, bug.Errorf("findRangeAndFilename returned invalid range: %v", err)
+		return nil, bug.Errorf("invalid range: %v", err)
 	}
 
 	adds, deletes, err := findImportEdits(pgf.File, pkg.TypesInfo(), start, end)
@@ -120,9 +119,9 @@ func ExtractToNewFile(ctx context.Context, snapshot *cache.Snapshot, fh file.Han
 	// For parenthesised declarations like `import ("fmt"\n "log")`
 	// we only remove the ImportSpec, because removing the whole declaration
 	// might remove other ImportsSpecs we don't want to touch.
-	parenthesisFreeImports := findParenthesisFreeImports(pgf)
+	unparenthesizedImports := unparenthesizedImports(pgf)
 	for _, importSpec := range deletes {
-		if decl := parenthesisFreeImports[importSpec]; decl != nil {
+		if decl := unparenthesizedImports[importSpec]; decl != nil {
 			importDeletes = append(importDeletes, removeNode(pgf, decl))
 		} else {
 			importDeletes = append(importDeletes, removeNode(pgf, importSpec))
@@ -143,7 +142,7 @@ func ExtractToNewFile(ctx context.Context, snapshot *cache.Snapshot, fh file.Han
 		buf.WriteString(")\n")
 	}
 
-	newFile, err := chooseNewFileURI(ctx, snapshot, pgf.URI.Dir().Path(), firstSymbol)
+	newFile, err := chooseNewFile(ctx, snapshot, pgf.URI.Dir().Path(), firstSymbol)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", errorPrefix, err)
 	}
@@ -162,27 +161,14 @@ func ExtractToNewFile(ctx context.Context, snapshot *cache.Snapshot, fh file.Han
 		// create a new file
 		protocol.DocumentChangeCreate(newFile.URI()),
 		// edit the created file
-		protocol.DocumentChangeEdit(&uriVersion{uri: newFile.URI(), version: 0}, []protocol.TextEdit{
+		protocol.DocumentChangeEdit(newFile, []protocol.TextEdit{
 			{Range: protocol.Range{}, NewText: string(newFileContent)},
 		})), nil
 }
 
-// uriVersion is the simplest struct that implements protocol.fileHandle.
-type uriVersion struct {
-	uri     protocol.DocumentURI
-	version int32
-}
-
-func (fh *uriVersion) URI() protocol.DocumentURI {
-	return fh.uri
-}
-func (fh *uriVersion) Version() int32 {
-	return fh.version
-}
-
-// chooseNewFileURI chooses a new filename in dir, based on the name of the
+// chooseNewFile chooses a new filename in dir, based on the name of the
 // first extracted symbol, and if necessary to disambiguate, a numeric suffix.
-func chooseNewFileURI(ctx context.Context, snapshot *cache.Snapshot, dir string, firstSymbol string) (file.Handle, error) {
+func chooseNewFile(ctx context.Context, snapshot *cache.Snapshot, dir string, firstSymbol string) (file.Handle, error) {
 	basename := strings.ToLower(firstSymbol)
 	newPath := protocol.URIFromPath(filepath.Join(dir, basename+".go"))
 	for count := 1; count < 5; count++ {
@@ -248,6 +234,7 @@ func selectedToplevelDecls(pgf *parsego.File, start, end token.Pos) (token.Pos,
 				return 0, 0, "", false
 			}
 			if id != nil && firstName == "" {
+				// may be "_"
 				firstName = id.Name
 			}
 			// extends selection to docs comments
@@ -277,23 +264,21 @@ func selectedToplevelDecls(pgf *parsego.File, start, end token.Pos) (token.Pos,
 	return start, end, firstName, true
 }
 
-func findParenthesisFreeImports(pgf *parsego.File) map[*ast.ImportSpec]*ast.GenDecl {
+// unparenthesizedImports returns a map from each unparenthesized ImportSpec
+// to its enclosing declaration (which may need to be deleted too).
+func unparenthesizedImports(pgf *parsego.File) map[*ast.ImportSpec]*ast.GenDecl {
 	decls := make(map[*ast.ImportSpec]*ast.GenDecl)
 	for _, decl := range pgf.File.Decls {
-		if g, ok := decl.(*ast.GenDecl); ok {
-			if !g.Lparen.IsValid() && len(g.Specs) > 0 {
-				if v, ok := g.Specs[0].(*ast.ImportSpec); ok {
-					decls[v] = g
-				}
-			}
+		if decl, ok := decl.(*ast.GenDecl); ok && decl.Tok == token.IMPORT && !decl.Lparen.IsValid() {
+			decls[decl.Specs[0].(*ast.ImportSpec)] = decl
 		}
 	}
 	return decls
 }
 
 // removeNode returns a TextEdit that removes the node.
 func removeNode(pgf *parsego.File, node ast.Node) protocol.TextEdit {
-	rng, err := pgf.PosRange(node.Pos(), node.End())
+	rng, err := pgf.NodeRange(node)
 	if err != nil {
 		bug.Reportf("removeNode: %v", err)
 	}

gopls/internal/protocol/command/command_gen.go

@@ -157,7 +157,7 @@ type Interface interface {
 	// ExtractToNewFile: Move selected declarations to a new file
 	//
 	// Used by the code action of the same name.
-	ExtractToNewFile(context.Context, ExtractToNewFileArgs) error
+	ExtractToNewFile(context.Context, protocol.Location) error
 
 	// StartDebugging: Start the gopls debug server
 	//
@@ -352,12 +352,6 @@ type AddImportArgs struct {
 	URI protocol.DocumentURI
 }
 
-type ExtractToNewFileArgs struct {
-	// URI of the file
-	URI   protocol.DocumentURI
-	Range protocol.Range
-}
-
 type ListKnownPackagesResult struct {
 	// Packages is a list of packages relative
 	// to the URIArg passed by the command request.

gopls/internal/protocol/command/interface.go

@@ -995,17 +995,13 @@ func (c *commandHandler) AddImport(ctx context.Context, args command.AddImportAr
 	})
 }
 
-func (c *commandHandler) ExtractToNewFile(ctx context.Context, args command.ExtractToNewFileArgs) error {
+func (c *commandHandler) ExtractToNewFile(ctx context.Context, args protocol.Location) error {
 	return c.run(ctx, commandConfig{
 		progress: "Extract to a new file",
 		forURI:   args.URI,
 	}, func(ctx context.Context, deps commandDeps) error {
 		edit, err := golang.ExtractToNewFile(ctx, deps.snapshot, deps.fh, args.Range)
 		if err != nil {
-			if errors.Is(err, golang.ErrDotImport) {
-				showMessage(ctx, c.s.client, protocol.Info, err.Error())
-				return nil
-			}
 			return err
 		}
 		resp, err := c.s.client.ApplyEdit(ctx, &protocol.ApplyWorkspaceEditParams{Edit: *edit})

gopls/internal/server/command.go

@@ -117,9 +117,8 @@ func (e *Env) SetBufferContent(name string, content string) {
 }
 
 // FileContent returns the file content for name that applies to the current
-// editing session: if the file is open, it returns its buffer content,
-// otherwise it returns on disk content, if the file does not exist,
-// it returns an empty string.
+// editing session: it returns the buffer content for an open file, the
+// on-disk content for an unopened file, or "" for a non-existent file.
 func (e *Env) FileContent(name string) string {
 	e.T.Helper()
 	text, ok := e.Editor.BufferText(name)

gopls/internal/test/integration/wrappers.go

@@ -71,12 +71,6 @@ package main
 import _ "fmt"
 func F() {} //@codeactionedit("func", "refactor.extract", blank_import)
 
--- dot_import.go --
-// This case is not yet implemented, it aborts and shows a message to the client. 
-package main
-import . "fmt"
-func F() { Println() } //@codeactionedit("func", "refactor.extract", dot_import)
-
 
 
 -- @blank_import/blank_import.go --