[gopls-release-branch.0.18] gopls: temporarily reinstate the "Structured" hover kind

As described in golang/go#71879, the removal of the experimental
"Structured" hover kind unexpectedly broke vim-go.

Reinstate support for this setting, with tests, so that we can proceed
with its deprecation more cautiously.

For golang/go#71879

Change-Id: I6d22852aa10126c84b66f4345fbbdcf4cefbd182
Reviewed-on: https://go-review.googlesource.com/c/tools/+/651238
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
(cherry picked from commit f2beb33)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/651239

Author: findleyr

gopls/doc/settings.md

@@ -428,6 +428,9 @@ Must be one of:
 * `"FullDocumentation"`
 * `"NoDocumentation"`
 * `"SingleLine"`
+* `"Structured"` is a misguided experimental setting that returns a JSON
+hover format. This setting should not be used, as it will be removed in a
+future release of gopls.
 * `"SynopsisDocumentation"`
 
 Default: `"FullDocumentation"`.

gopls/internal/doc/api.json

@@ -134,6 +134,10 @@
 						"Value": "\"SingleLine\"",
 						"Doc": ""
 					},
+					{
+						"Value": "\"Structured\"",
+						"Doc": "`\"Structured\"` is a misguided experimental setting that returns a JSON\nhover format. This setting should not be used, as it will be removed in a\nfuture release of gopls.\n"
+					},
 					{
 						"Value": "\"SynopsisDocumentation\"",
 						"Doc": ""

gopls/internal/golang/hover.go

@@ -7,6 +7,7 @@ package golang
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"go/ast"
 	"go/constant"
@@ -48,37 +49,47 @@ import (
 // It is formatted in one of several formats as determined by the
 // HoverKind setting.
 type hoverResult struct {
-	// synopsis is a single sentence synopsis of the symbol's documentation.
+	// The fields below are exported to define the JSON hover format.
+	// TODO(golang/go#70233): (re)remove support for JSON hover.
+
+	// Synopsis is a single sentence Synopsis of the symbol's documentation.
 	//
-	// TODO(adonovan): in what syntax? It (usually) comes from doc.synopsis,
+	// TODO(adonovan): in what syntax? It (usually) comes from doc.Synopsis,
 	// which produces "Text" form, but it may be fed to
 	// DocCommentToMarkdown, which expects doc comment syntax.
-	synopsis string
+	Synopsis string
 
-	// fullDocumentation is the symbol's full documentation.
-	fullDocumentation string
+	// FullDocumentation is the symbol's full documentation.
+	FullDocumentation string
 
-	// signature is the symbol's signature.
-	signature string
+	// Signature is the symbol's Signature.
+	Signature string
 
-	// singleLine is a single line describing the symbol.
+	// SingleLine is a single line describing the symbol.
 	// This is recommended only for use in clients that show a single line for hover.
-	singleLine string
+	SingleLine string
 
-	// symbolName is the human-readable name to use for the symbol in links.
-	symbolName string
+	// SymbolName is the human-readable name to use for the symbol in links.
+	SymbolName string
 
-	// linkPath is the path of the package enclosing the given symbol,
+	// LinkPath is the path of the package enclosing the given symbol,
 	// with the module portion (if any) replaced by "module@version".
 	//
 	// For example: "github.com/google/go-github/[email protected]/github".
 	//
-	// Use LinkTarget + "/" + linkPath + "#" + LinkAnchor to form a pkgsite URL.
-	linkPath string
+	// Use LinkTarget + "/" + LinkPath + "#" + LinkAnchor to form a pkgsite URL.
+	LinkPath string
 
-	// linkAnchor is the pkg.go.dev link anchor for the given symbol.
+	// LinkAnchor is the pkg.go.dev link anchor for the given symbol.
 	// For example, the "Node" part of "pkg.go.dev/go/ast#Node".
-	linkAnchor string
+	LinkAnchor string
+
+	// New fields go below, and are unexported. The existing
+	// exported fields are underspecified and have already
+	// constrained our movements too much. A detailed JSON
+	// interface might be nice, but it needs a design and a
+	// precise specification.
+	// TODO(golang/go#70233): (re)deprecate the JSON hover output.
 
 	// typeDecl is the declaration syntax for a type,
 	// or "" for a non-type.
@@ -284,9 +295,9 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
 		typesinternal.SetVarKind(v, typesinternal.LocalVar)
 		signature := types.ObjectString(v, qual)
 		return *hoverRange, &hoverResult{
-			signature:  signature,
-			singleLine: signature,
-			symbolName: v.Name(),
+			Signature:  signature,
+			SingleLine: signature,
+			SymbolName: v.Name(),
 		}, nil
 	}
 
@@ -615,13 +626,13 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro
 	}
 
 	return *hoverRange, &hoverResult{
-		synopsis:          doc.Synopsis(docText),
-		fullDocumentation: docText,
-		singleLine:        singleLineSignature,
-		symbolName:        linkName,
-		signature:         signature,
-		linkPath:          linkPath,
-		linkAnchor:        anchor,
+		Synopsis:          doc.Synopsis(docText),
+		FullDocumentation: docText,
+		SingleLine:        singleLineSignature,
+		SymbolName:        linkName,
+		Signature:         signature,
+		LinkPath:          linkPath,
+		LinkAnchor:        anchor,
 		typeDecl:          typeDecl,
 		methods:           methods,
 		promotedFields:    fields,
@@ -638,8 +649,8 @@ func hoverBuiltin(ctx context.Context, snapshot *cache.Snapshot, obj types.Objec
 	if obj.Name() == "Error" {
 		signature := obj.String()
 		return &hoverResult{
-			signature:  signature,
-			singleLine: signature,
+			Signature:  signature,
+			SingleLine: signature,
 			// TODO(rfindley): these are better than the current behavior.
 			// SymbolName: "(error).Error",
 			// LinkPath:   "builtin",
@@ -682,13 +693,13 @@ func hoverBuiltin(ctx context.Context, snapshot *cache.Snapshot, obj types.Objec
 
 	docText := comment.Text()
 	return &hoverResult{
-		synopsis:          doc.Synopsis(docText),
-		fullDocumentation: docText,
-		signature:         signature,
-		singleLine:        obj.String(),
-		symbolName:        obj.Name(),
-		linkPath:          "builtin",
-		linkAnchor:        obj.Name(),
+		Synopsis:          doc.Synopsis(docText),
+		FullDocumentation: docText,
+		Signature:         signature,
+		SingleLine:        obj.String(),
+		SymbolName:        obj.Name(),
+		LinkPath:          "builtin",
+		LinkAnchor:        obj.Name(),
 	}, nil
 }
 
@@ -740,9 +751,9 @@ func hoverImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Packa
 
 	docText := comment.Text()
 	return rng, &hoverResult{
-		signature:         "package " + string(impMetadata.Name),
-		synopsis:          doc.Synopsis(docText),
-		fullDocumentation: docText,
+		Signature:         "package " + string(impMetadata.Name),
+		Synopsis:          doc.Synopsis(docText),
+		FullDocumentation: docText,
 	}, nil
 }
 
@@ -798,9 +809,9 @@ func hoverPackageName(pkg *cache.Package, pgf *parsego.File) (protocol.Range, *h
 	}
 
 	return rng, &hoverResult{
-		signature:         "package " + string(pkg.Metadata().Name),
-		synopsis:          doc.Synopsis(docText),
-		fullDocumentation: docText,
+		Signature:         "package " + string(pkg.Metadata().Name),
+		Synopsis:          doc.Synopsis(docText),
+		FullDocumentation: docText,
 		footer:            footer,
 	}, nil
 }
@@ -926,8 +937,8 @@ func hoverLit(pgf *parsego.File, lit *ast.BasicLit, pos token.Pos) (protocol.Ran
 	}
 	hover := b.String()
 	return rng, &hoverResult{
-		synopsis:          hover,
-		fullDocumentation: hover,
+		Synopsis:          hover,
+		FullDocumentation: hover,
 	}, nil
 }
 
@@ -966,7 +977,7 @@ func hoverReturnStatement(pgf *parsego.File, path []ast.Node, ret *ast.ReturnStm
 	}
 	buf.WriteByte(')')
 	return rng, &hoverResult{
-		signature: buf.String(),
+		Signature: buf.String(),
 	}, nil
 }
 
@@ -1005,9 +1016,9 @@ func hoverEmbed(fh file.Handle, rng protocol.Range, pattern string) (protocol.Ra
 	}
 
 	res := &hoverResult{
-		signature:         fmt.Sprintf("Embedding %q", pattern),
-		synopsis:          s.String(),
-		fullDocumentation: s.String(),
+		Signature:         fmt.Sprintf("Embedding %q", pattern),
+		Synopsis:          s.String(),
+		FullDocumentation: s.String(),
 	}
 	return rng, res, nil
 }
@@ -1242,10 +1253,17 @@ func formatHover(h *hoverResult, options *settings.Options, pkgURL func(path Pac
 
 	switch options.HoverKind {
 	case settings.SingleLine:
-		return h.singleLine, nil
+		return h.SingleLine, nil
 
 	case settings.NoDocumentation:
-		return maybeFenced(h.signature), nil
+		return maybeFenced(h.Signature), nil
+
+	case settings.Structured:
+		b, err := json.Marshal(h)
+		if err != nil {
+			return "", err
+		}
+		return string(b), nil
 
 	case settings.SynopsisDocumentation, settings.FullDocumentation:
 		var sections [][]string // assembled below
@@ -1256,20 +1274,20 @@ func formatHover(h *hoverResult, options *settings.Options, pkgURL func(path Pac
 		// but not Signature, which is redundant (= TypeDecl + "\n" + Methods).
 		// For all other symbols, we display Signature;
 		// TypeDecl and Methods are empty.
-		// (Now that JSON is no more, we could rationalize this.)
+		// TODO(golang/go#70233): When JSON is no more, we could rationalize this.
 		if h.typeDecl != "" {
 			sections = append(sections, []string{maybeFenced(h.typeDecl)})
 		} else {
-			sections = append(sections, []string{maybeFenced(h.signature)})
+			sections = append(sections, []string{maybeFenced(h.Signature)})
 		}
 
 		// Doc section.
 		var doc string
 		switch options.HoverKind {
 		case settings.SynopsisDocumentation:
-			doc = h.synopsis
+			doc = h.Synopsis
 		case settings.FullDocumentation:
-			doc = h.fullDocumentation
+			doc = h.FullDocumentation
 		}
 		if options.PreferredContentFormat == protocol.Markdown {
 			doc = DocCommentToMarkdown(doc, options)
@@ -1392,34 +1410,34 @@ func StdSymbolOf(obj types.Object) *stdlib.Symbol {
 
 // If pkgURL is non-nil, it should be used to generate doc links.
 func formatLink(h *hoverResult, options *settings.Options, pkgURL func(path PackagePath, fragment string) protocol.URI) string {
-	if options.LinksInHover == settings.LinksInHover_None || h.linkPath == "" {
+	if options.LinksInHover == settings.LinksInHover_None || h.LinkPath == "" {
 		return ""
 	}
 	var url protocol.URI
 	var caption string
 	if pkgURL != nil { // LinksInHover == "gopls"
 		// Discard optional module version portion.
 		// (Ideally the hoverResult would retain the structure...)
-		path := h.linkPath
-		if module, versionDir, ok := strings.Cut(h.linkPath, "@"); ok {
+		path := h.LinkPath
+		if module, versionDir, ok := strings.Cut(h.LinkPath, "@"); ok {
 			// "module@version/dir"
 			path = module
 			if _, dir, ok := strings.Cut(versionDir, "/"); ok {
 				path += "/" + dir
 			}
 		}
-		url = pkgURL(PackagePath(path), h.linkAnchor)
+		url = pkgURL(PackagePath(path), h.LinkAnchor)
 		caption = "in gopls doc viewer"
 	} else {
 		if options.LinkTarget == "" {
 			return ""
 		}
-		url = cache.BuildLink(options.LinkTarget, h.linkPath, h.linkAnchor)
+		url = cache.BuildLink(options.LinkTarget, h.LinkPath, h.LinkAnchor)
 		caption = "on " + options.LinkTarget
 	}
 	switch options.PreferredContentFormat {
 	case protocol.Markdown:
-		return fmt.Sprintf("[`%s` %s](%s)", h.symbolName, caption, url)
+		return fmt.Sprintf("[`%s` %s](%s)", h.SymbolName, caption, url)
 	case protocol.PlainText:
 		return ""
 	default:

gopls/internal/settings/settings.go

@@ -798,6 +798,11 @@ const (
 	NoDocumentation       HoverKind = "NoDocumentation"
 	SynopsisDocumentation HoverKind = "SynopsisDocumentation"
 	FullDocumentation     HoverKind = "FullDocumentation"
+
+	// Structured is a misguided experimental setting that returns a JSON
+	// hover format. This setting should not be used, as it will be removed in a
+	// future release of gopls.
+	Structured HoverKind = "Structured"
 )
 
 type VulncheckMode string
@@ -1073,14 +1078,15 @@ func (o *Options) setOne(name string, value any) (applied []CounterPath, _ error
 			AllSymbolScope)
 
 	case "hoverKind":
-		if s, ok := value.(string); ok && strings.EqualFold(s, "structured") {
-			return nil, deprecatedError("the experimental hoverKind='structured' setting was removed in gopls/v0.18.0 (https://go.dev/issue/70233)")
-		}
+		// TODO(rfindley): reinstate the deprecation of Structured hover by making
+		// it a warning in gopls v0.N+1, and removing it in gopls v0.N+2.
 		return setEnum(&o.HoverKind, value,
 			NoDocumentation,
 			SingleLine,
 			SynopsisDocumentation,
-			FullDocumentation)
+			FullDocumentation,
+			Structured,
+		)
 
 	case "linkTarget":
 		return nil, setString(&o.LinkTarget, value)

gopls/internal/settings/settings_test.go

@@ -91,19 +91,19 @@ func TestOptions_Set(t *testing.T) {
 			},
 		},
 		{
-			name:      "hoverKind",
-			value:     "Structured",
-			wantError: true,
+			name:  "hoverKind",
+			value: "Structured",
+			// wantError: true, // TODO(rfindley): reinstate this error
 			check: func(o Options) bool {
-				return o.HoverKind == FullDocumentation
+				return o.HoverKind == Structured
 			},
 		},
 		{
-			name:      "ui.documentation.hoverKind",
-			value:     "Structured",
-			wantError: true,
+			name:  "ui.documentation.hoverKind",
+			value: "Structured",
+			// wantError: true, // TODO(rfindley): reinstate this error
 			check: func(o Options) bool {
-				return o.HoverKind == FullDocumentation
+				return o.HoverKind == Structured
 			},
 		},
 		{

gopls/internal/test/marker/testdata/hover/json.txt

@@ -0,0 +1,33 @@
+This test demonstrates support for "hoverKind": "Structured".
+
+Its size expectations assume a 64-bit machine.
+
+-- flags --
+-skip_goarch=386,arm
+
+-- go.mod --
+module example.com/p
+
+go 1.18
+
+-- settings.json --
+{
+	"hoverKind": "Structured"
+}
+-- p.go --
+package p
+
+// MyType is a type.
+type MyType struct { //@ hover("MyType", "MyType", MyType)
+	F int // a field
+	S string // a string field
+}
+
+// MyFunc is a function.
+func MyFunc(i int) string { //@ hover("MyFunc", "MyFunc", MyFunc)
+	return ""
+}
+-- @MyFunc --
+{"Synopsis":"MyFunc is a function.","FullDocumentation":"MyFunc is a function.\n","Signature":"func MyFunc(i int) string","SingleLine":"func MyFunc(i int) string","SymbolName":"p.MyFunc","LinkPath":"example.com/p","LinkAnchor":"MyFunc"}
+-- @MyType --
+{"Synopsis":"MyType is a type.","FullDocumentation":"MyType is a type.\n","Signature":"type MyType struct { // size=24 (0x18)\n\tF int    // a field\n\tS string // a string field\n}\n","SingleLine":"type MyType struct{F int; S string}","SymbolName":"p.MyType","LinkPath":"example.com/p","LinkAnchor":"MyType"}