gopls/internal/server: simplify FreeSymbols plumbing

Since only a view ID, not a View, is needed, we can simplify
the argument plumbing following the pattern used by Assembly.

Also, rename RenderPackageDoc to PackageDocHTML.

Change-Id: Ib12c26ff0960a3ba96a6b8e6872740dd8767dfbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/591157
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/doc/commands.md

@@ -325,15 +325,16 @@ Args:
 ```
 string,
 {
-	// The range's start position.
-	"start": {
-		"line": uint32,
-		"character": uint32,
-	},
-	// The range's end position.
-	"end": {
-		"line": uint32,
-		"character": uint32,
+	"uri": string,
+	"range": {
+		"start": {
+			"line": uint32,
+			"character": uint32,
+		},
+		"end": {
+			"line": uint32,
+			"character": uint32,
+		},
 	},
 }
 ```

gopls/internal/doc/api.json

@@ -1013,7 +1013,7 @@
 			"Command": "gopls.free_symbols",
 			"Title": "report free symbols referenced by the selection.",
 			"Doc": "This command is a query over a selected range of Go source\ncode. It reports the set of \"free\" symbols of the\nselection: the set of symbols that are referenced within\nthe selection but are declared outside of it. This\ninformation is useful for understanding at a glance what a\nblock of code depends on, perhaps as a precursor to\nextracting it into a separate function.",
-			"ArgDoc": "string,\n{\n\t// The range's start position.\n\t\"start\": {\n\t\t\"line\": uint32,\n\t\t\"character\": uint32,\n\t},\n\t// The range's end position.\n\t\"end\": {\n\t\t\"line\": uint32,\n\t\t\"character\": uint32,\n\t},\n}",
+			"ArgDoc": "string,\n{\n\t\"uri\": string,\n\t\"range\": {\n\t\t\"start\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t\t\"end\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t},\n}",
 			"ResultDoc": ""
 		},
 		{

gopls/internal/golang/codeaction.go

@@ -117,7 +117,8 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 		}
 
 		if want[protocol.GoFreeSymbols] && rng.End != rng.Start {
-			cmd, err := command.NewFreeSymbolsCommand("Show free symbols", pgf.URI, rng)
+			loc := protocol.Location{URI: pgf.URI, Range: rng}
+			cmd, err := command.NewFreeSymbolsCommand("Show free symbols", snapshot.View().ID(), loc)
 			if err != nil {
 				return nil, err
 			}

gopls/internal/golang/pkgdoc.go

@@ -62,7 +62,7 @@ type (
 	PosURLFunc = func(filename string, line, col8 int) protocol.URI
 )
 
-// RenderPackageDoc formats the package documentation page.
+// PackageDocHTML formats the package documentation page.
 //
 // The posURL function returns a URL that when visited, has the side
 // effect of causing gopls to direct the client editor to navigate to
@@ -70,9 +70,7 @@ type (
 //
 // The pkgURL function returns a URL for the documentation of the
 // specified package and symbol.
-//
-// TODO(adonovan): "Render" is a client-side verb; rename to PackageDocHTML.
-func RenderPackageDoc(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) ([]byte, error) {
+func PackageDocHTML(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) ([]byte, error) {
 	// We can't use doc.NewFromFiles (even with doc.PreserveAST
 	// mode) as it calls ast.NewPackage which assumes that each
 	// ast.File has an ast.Scope and resolves identifiers to

gopls/internal/protocol/command/command_gen.go

@@ -172,8 +172,8 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte
 		}
 		return s.FetchVulncheckResult(ctx, a0)
 	case FreeSymbols:
-		var a0 protocol.DocumentURI
-		var a1 protocol.Range
+		var a0 string
+		var a1 protocol.Location
 		if err := UnmarshalArgs(params.Arguments, &a0, &a1); err != nil {
 			return nil, err
 		}
@@ -444,7 +444,7 @@ func NewFetchVulncheckResultCommand(title string, a0 URIArg) (protocol.Command,
 	}, nil
 }
 
-func NewFreeSymbolsCommand(title string, a0 protocol.DocumentURI, a1 protocol.Range) (protocol.Command, error) {
+func NewFreeSymbolsCommand(title string, a0 string, a1 protocol.Location) (protocol.Command, error) {
 	args, err := MarshalArgs(a0, a1)
 	if err != nil {
 		return protocol.Command{}, err

gopls/internal/protocol/command/interface.go

@@ -251,7 +251,7 @@ type Interface interface {
 	// information is useful for understanding at a glance what a
 	// block of code depends on, perhaps as a precursor to
 	// extracting it into a separate function.
-	FreeSymbols(context.Context, protocol.DocumentURI, protocol.Range) error
+	FreeSymbols(ctx context.Context, viewID string, loc protocol.Location) error
 
 	// Assembly: Show disassembly of current function.
 	//

gopls/internal/server/command.go

@@ -1465,24 +1465,14 @@ func (c *commandHandler) Views(ctx context.Context) ([]command.View, error) {
 	return summaries, nil
 }
 
-func (c *commandHandler) FreeSymbols(ctx context.Context, uri protocol.DocumentURI, rng protocol.Range) error {
-	// TODO(adonovan): simplify, following Assembly, by putting the
-	// viewID in the command so that c.run isn't necessary.
-	// (freesymbolsURL needs only a viewID, not a view.)
-	return c.run(ctx, commandConfig{
-		forURI: uri,
-	}, func(ctx context.Context, deps commandDeps) error {
-		web, err := c.s.getWeb()
-		if err != nil {
-			return err
-		}
-		url := web.freesymbolsURL(deps.snapshot.View(), protocol.Location{
-			URI:   deps.fh.URI(),
-			Range: rng,
-		})
-		openClientBrowser(ctx, c.s.client, url)
-		return nil
-	})
+func (c *commandHandler) FreeSymbols(ctx context.Context, viewID string, loc protocol.Location) error {
+	web, err := c.s.getWeb()
+	if err != nil {
+		return err
+	}
+	url := web.freesymbolsURL(viewID, loc)
+	openClientBrowser(ctx, c.s.client, url)
+	return nil
 }
 
 func (c *commandHandler) Assembly(ctx context.Context, viewID, packageID, symbol string) error {

gopls/internal/server/server.go

@@ -350,7 +350,7 @@ func (s *server) initWeb() (*web, error) {
 		pkgURL := func(path golang.PackagePath, fragment string) protocol.URI {
 			return web.pkgURL(view, path, fragment)
 		}
-		content, err := golang.RenderPackageDoc(pkgs[0], web.openURL, pkgURL)
+		content, err := golang.PackageDocHTML(pkgs[0], web.openURL, pkgURL)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
@@ -494,7 +494,7 @@ func (w *web) pkgURL(v *cache.View, path golang.PackagePath, fragment string) pr
 
 // freesymbolsURL returns a /freesymbols URL for a report
 // on the free symbols referenced within the selection span (loc).
-func (w *web) freesymbolsURL(v *cache.View, loc protocol.Location) protocol.URI {
+func (w *web) freesymbolsURL(viewID string, loc protocol.Location) protocol.URI {
 	return w.url(
 		"freesymbols",
 		fmt.Sprintf("file=%s&range=%d:%d:%d:%d&view=%s",
@@ -503,7 +503,7 @@ func (w *web) freesymbolsURL(v *cache.View, loc protocol.Location) protocol.URI
 			loc.Range.Start.Character,
 			loc.Range.End.Line,
 			loc.Range.End.Character,
-			url.QueryEscape(v.ID())),
+			url.QueryEscape(viewID)),
 		"")
 }