gopls/internal: prompt with full package path in preparerename

As part of implementing package move refactoring (rename on a package
that results in moving the package to a different directory), we
need to allow the user to provide us with a full package path. First,
we change PrepareRename to prompt the user with the full package path
instead of just the package name. For example, PrepareRename on package
golang would display "golang.org/x/tools/gopls/internal/golang" instead
of "golang".

For now, throw an error if the user attempts to rename a package to
a different directory.

Also update the marker test framework to handle directory renames.

For golang/go#57171

Change-Id: If2f7d1cfcbb9ce142f619b5ccb04843bca5d40f5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/708655
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: madelinekalil

gopls/doc/settings.md

@@ -269,6 +269,16 @@ and package declaration in a newly created Go file.
 
 Default: `true`.
 
+<a id='packageMove'></a>
+### `packageMove bool`
+
+**This setting is experimental and may be deleted.**
+
+packageMove enables PrepareRename to send the full package path
+and allows users to move a package via renaming.
+
+Default: `false`.
+
 <a id='completion'></a>
 ## Completion
 

gopls/internal/doc/api.json

@@ -2159,6 +2159,20 @@
 				"Hierarchy": "ui",
 				"DeprecationMessage": ""
 			},
+			{
+				"Name": "packageMove",
+				"Type": "bool",
+				"Doc": "packageMove enables PrepareRename to send the full package path\nand allows users to move a package via renaming.\n",
+				"EnumKeys": {
+					"ValueType": "",
+					"Keys": null
+				},
+				"EnumValues": null,
+				"Default": "false",
+				"Status": "experimental",
+				"Hierarchy": "ui",
+				"DeprecationMessage": ""
+			},
 			{
 				"Name": "local",
 				"Type": "string",

gopls/internal/golang/rename.go

@@ -214,9 +214,14 @@ func prepareRenamePackageName(ctx context.Context, snapshot *cache.Snapshot, pgf
 	if err != nil {
 		return nil, err
 	}
+
+	text := string(meta.Name)
+	if snapshot.Options().PackageMove {
+		text = string(meta.PkgPath)
+	}
 	return &PrepareItem{
 		Range: rng,
-		Text:  string(meta.Name),
+		Text:  text,
 	}, nil
 }
 
@@ -420,10 +425,6 @@ func Rename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle, pp pro
 		return edits, false, nil
 	}
 
-	if !isValidIdentifier(newName) {
-		return nil, false, fmt.Errorf("invalid identifier to rename: %q", newName)
-	}
-
 	// Cursor within package name declaration?
 	_, inPackageName, err := parsePackageNameDecl(ctx, snapshot, f, pp)
 	if err != nil {
@@ -433,8 +434,16 @@ func Rename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle, pp pro
 	var editMap map[protocol.DocumentURI][]diff.Edit
 	if inPackageName {
 		countRenamePackage.Inc()
+		if !isValidPackagePath(pkg.String(), newName) {
+			return nil, false, fmt.Errorf("invalid package path: %q (package moves are not yet supported, see go.dev/issue/57171)", newName)
+		}
+		// Only the last element of the path is required as input for [renamePackageName].
+		newName = path.Base(newName)
 		editMap, err = renamePackageName(ctx, snapshot, f, PackageName(newName))
 	} else {
+		if !isValidIdentifier(newName) {
+			return nil, false, fmt.Errorf("invalid identifier to rename: %q", newName)
+		}
 		editMap, err = renameOrdinary(ctx, snapshot, f.URI(), pp, newName)
 	}
 	if err != nil {
@@ -884,7 +893,6 @@ func renamePackageName(ctx context.Context, s *cache.Snapshot, f file.Handle, ne
 		return nil, err
 	}
 
-	// Update the last component of the file's enclosing directory.
 	oldBase := f.URI().DirPath()
 	newPkgDir := filepath.Join(filepath.Dir(oldBase), string(newName))
 

gopls/internal/golang/rename_check.go

@@ -914,6 +914,17 @@ func isValidIdentifier(id string) bool {
 	return token.Lookup(id) == token.IDENT
 }
 
+// isValidPackagePath reports whether newPath is a valid new path for the
+// package currently at oldPath. For now, we only support renames that
+// do not result in a package move.
+// TODO(mkalil): support package renames with arbitrary package paths, including
+// relative paths.
+func isValidPackagePath(oldPath, newPath string) bool {
+	// We prompt with the full package path, but some users may delete this and
+	// just enter a package identifier, which we should still support.
+	return isValidIdentifier(newPath) || filepath.Dir(oldPath) == filepath.Dir(newPath)
+}
+
 // isLocal reports whether obj is local to some function.
 // Precondition: not a struct field or interface method.
 func isLocal(obj types.Object) bool {

gopls/internal/server/rename.go

@@ -7,6 +7,7 @@ package server
 import (
 	"context"
 	"fmt"
+	"path"
 	"path/filepath"
 
 	"golang.org/x/tools/gopls/internal/file"
@@ -52,7 +53,7 @@ func (s *server) Rename(ctx context.Context, params *protocol.RenameParams) (*pr
 	if isPkgRenaming {
 		// Update the last component of the file's enclosing directory.
 		oldDir := fh.URI().DirPath()
-		newDir := filepath.Join(filepath.Dir(oldDir), params.NewName)
+		newDir := filepath.Join(filepath.Dir(oldDir), path.Base(params.NewName))
 		change := protocol.DocumentChangeRename(
 			protocol.URIFromPath(oldDir),
 			protocol.URIFromPath(newDir))

gopls/internal/settings/settings.go

@@ -250,6 +250,10 @@ type UIOptions struct {
 	// NewGoFileHeader enables automatic insertion of the copyright comment
 	// and package declaration in a newly created Go file.
 	NewGoFileHeader bool
+
+	// PackageMove enables PrepareRename to send the full package path
+	// and allows users to move a package via renaming.
+	PackageMove bool `status:"experimental"`
 }
 
 // A CodeLensSource identifies an (algorithmic) source of code lenses.
@@ -1361,6 +1365,9 @@ func (o *Options) setOne(name string, value any) (applied []CounterPath, _ error
 	case "mcpTools":
 		return setBoolMap(&o.MCPTools, value)
 
+	case "packageMove":
+		return setBool(&o.PackageMove, value)
+
 	// deprecated and renamed settings
 	//
 	// These should never be deleted: there is essentially no cost

gopls/internal/test/marker/marker_test.go

@@ -2091,17 +2091,43 @@ func changedFiles(env *integration.Env, changes []protocol.DocumentChange) (map[
 
 		case change.RenameFile != nil:
 			old := change.RenameFile.OldURI
-			m, err := read(old)
+			new := change.RenameFile.NewURI
+			info, err := os.Stat(old.Path())
 			if err != nil {
-				return nil, err // missing
+				return nil, err
 			}
-			write(old, nil)
+			if info.IsDir() {
+				// Walk through all the files in the old directory and copy
+				// their content to the new directory.
+				// TODO(mkalil): This currently only handles renaming the file's
+				// innermost directory. Need to handle renames of outer directories
+				// directories when implementing package move refactoring.
+				for _, file := range env.ListFiles(old.Path()) {
+					oldFile := protocol.URIFromPath(path.Join(old.Path(), path.Base(file)))
+					m, err := read(oldFile)
+					if err != nil {
+						return nil, err // missing
+					}
+					write(oldFile, nil)
 
-			new := change.RenameFile.NewURI
-			if _, err := read(old); err == nil {
-				return nil, fmt.Errorf("RenameFile: destination %s exists", new)
+					newFile := protocol.URIFromPath(path.Join(new.Path(), path.Base(file)))
+					if _, err := read(oldFile); err == nil {
+						return nil, fmt.Errorf("RenameFile: destination %s exists", new)
+					}
+					write(newFile, m.Content)
+				}
+			} else {
+				m, err := read(old)
+				if err != nil {
+					return nil, err // missing
+				}
+				write(old, nil)
+
+				if _, err := read(old); err == nil {
+					return nil, fmt.Errorf("RenameFile: destination %s exists", new)
+				}
+				write(new, m.Content)
 			}
-			write(new, m.Content)
 
 		case change.CreateFile != nil:
 			uri := change.CreateFile.URI

gopls/internal/test/marker/testdata/rename/packagedecl.txt

@@ -0,0 +1,26 @@
+This test verifies the behavior of renaming a package declaration.
+
+-- settings.json --
+{
+	"packageMove": true
+}
+
+-- flags --
+-ignore_extra_diags
+
+-- go.mod --
+module golang.org/lsptests/rename
+
+go 1.20
+-- one/one.go --
+package one //@ rename("one", "golang.org/lsptests/rename/one", sameName), rename("one", "golang.org/lsptests/rename/two", newNameSameDir), renameerr("one", "golang.org/lsptests/otherdir/one", re"invalid package path")
+
+-- @newNameSameDir/one/one.go --
+@@ -1,2 +0,0 @@
+-package one //@ rename("one", "golang.org/lsptests/rename/one", sameName), rename("one", "golang.org/lsptests/rename/two", newNameSameDir), renameerr("one", "golang.org/lsptests/otherdir/one", re"invalid package path")
+-
+-- @newNameSameDir/two/one.go --
+@@ -0,0 +1,2 @@
++package two //@ rename("one", "golang.org/lsptests/rename/one", sameName), rename("one", "golang.org/lsptests/rename/two", newNameSameDir), renameerr("one", "golang.org/lsptests/otherdir/one", re"invalid package path")
++
+-- @sameName/one/one.go --

gopls/internal/test/marker/testdata/rename/prepare_move.txt

@@ -0,0 +1,14 @@
+This test verifies the behavior of textDocument/prepareRename when the experimental package move setting is enabled.
+
+-- settings.json --
+{
+	"packageMove": true
+}
+
+-- go.mod --
+module golang.org/lsptests
+
+go 1.20
+
+-- b/b.go --
+package b //@ preparerename("b", "golang.org/lsptests/b")