[gopls-release-branch.0.16] gopls/internal/settings: move CodeLensSource from protocol

This enum is properly a setting, not a protocol feature.

The .md and api.json generator script assumes that all enums
are in the settings package, and consequently emitted incorrect
names for this type.

Generator changes in detail:
- remove the package.Load of both "settings" and "protocol",
  reverting part of an early change.
- remove loadAPI special case for "codelenses" not being an
  enum-keyed map, as it is one. (As a consequence, the enum
  values appear in api.json in alphabetical, not declaration,
  order. Also, the title portion of the codelend doc string
  is no longer discarded.)
- add lots of missing commentary.

This may seem like a large change at the 11th hour, but note:
- the only change to the production code is a renaming;
- the effects of the generator changes are entirely confined
  to settings.md and api.json.

Fixes golang/go#68057

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

Author: adonovan

gopls/doc/generate/generate.go

@@ -39,7 +39,6 @@ import (
 	"golang.org/x/tools/gopls/internal/doc"
 	"golang.org/x/tools/gopls/internal/golang"
 	"golang.org/x/tools/gopls/internal/mod"
-	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/protocol/command/commandmeta"
 	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/util/maps"
@@ -136,23 +135,12 @@ func loadAPI() (*doc.API, error) {
 		&packages.Config{
 			Mode: packages.NeedTypes | packages.NeedTypesInfo | packages.NeedSyntax | packages.NeedDeps,
 		},
-		"golang.org/x/tools/gopls/internal/settings", // for settings
-		"golang.org/x/tools/gopls/internal/protocol", // for lenses
+		"golang.org/x/tools/gopls/internal/settings",
 	)
 	if err != nil {
 		return nil, err
 	}
-	// TODO(adonovan): document at packages.Load that the result
-	// order does not match the pattern order.
-	var protocolPkg, settingsPkg *packages.Package
-	for _, pkg := range pkgs {
-		switch pkg.Types.Name() {
-		case "settings":
-			settingsPkg = pkg
-		case "protocol":
-			protocolPkg = pkg
-		}
-	}
+	settingsPkg := pkgs[0]
 
 	defaults := settings.DefaultOptions()
 	api := &doc.API{
@@ -164,7 +152,7 @@ func loadAPI() (*doc.API, error) {
 	if err != nil {
 		return nil, err
 	}
-	api.Lenses, err = loadLenses(protocolPkg, defaults.Codelenses)
+	api.Lenses, err = loadLenses(settingsPkg, defaults.Codelenses)
 	if err != nil {
 		return nil, err
 	}
@@ -185,8 +173,8 @@ func loadAPI() (*doc.API, error) {
 		catName := strings.TrimSuffix(category.Type().Name(), "Options")
 		api.Options[catName] = opts
 
-		// Hardcode the expected values for the analyses and code lenses
-		// settings, since their keys are not enums.
+		// Hardcode the expected values for the "analyses" and "hints" settings,
+		// since their map keys are strings, not enums.
 		for _, opt := range opts {
 			switch opt.Name {
 			case "analyses":
@@ -197,24 +185,9 @@ func loadAPI() (*doc.API, error) {
 						Default: strconv.FormatBool(a.Default),
 					})
 				}
-			case "codelenses":
-				// Hack: Lenses don't set default values, and we don't want to
-				// pass in the list of expected lenses to loadOptions. Instead,
-				// format the defaults using reflection here. The hackiest part
-				// is reversing lowercasing of the field name.
-				reflectField := category.FieldByName(upperFirst(opt.Name))
-				for _, l := range api.Lenses {
-					def, err := formatDefaultFromEnumBoolMap(reflectField, l.Lens)
-					if err != nil {
-						return nil, err
-					}
-					opt.EnumKeys.Keys = append(opt.EnumKeys.Keys, doc.EnumKey{
-						Name:    fmt.Sprintf("%q", l.Lens),
-						Doc:     l.Doc,
-						Default: def,
-					})
-				}
 			case "hints":
+				// TODO(adonovan): simplify InlayHints to use an enum,
+				// following CodeLensSource.
 				for _, a := range api.Hints {
 					opt.EnumKeys.Keys = append(opt.EnumKeys.Keys, doc.EnumKey{
 						Name:    fmt.Sprintf("%q", a.Name),
@@ -282,24 +255,48 @@ func loadOptions(category reflect.Value, optsType types.Object, pkg *packages.Pa
 			return nil, err
 		}
 
+		// Derive the doc-and-api.json type from the Go field type.
+		//
+		// In principle, we should use JSON nomenclature here
+		// (number, array, object, etc; see #68057), but in
+		// practice we use the Go type string ([]T, map[K]V,
+		// etc) with only one tweak: enumeration types are
+		// replaced by "enum", including when they appear as
+		// map keys.
+		//
+		// Notable edge cases:
+		// - any (e.g. in linksInHover) is really a sum of false | true | "internal".
+		// - time.Duration is really a string with a particular syntax.
 		typ := typesField.Type().String()
 		if _, ok := enums[typesField.Type()]; ok {
 			typ = "enum"
 		}
 		name := lowerFirst(typesField.Name())
 
+		// enum-keyed maps
 		var enumKeys doc.EnumKeys
 		if m, ok := typesField.Type().Underlying().(*types.Map); ok {
-			e, ok := enums[m.Key()]
+			values, ok := enums[m.Key()]
 			if ok {
-				typ = strings.Replace(typ, m.Key().String(), m.Key().Underlying().String(), 1)
-			}
-			keys, err := collectEnumKeys(name, m, reflectField, e)
-			if err != nil {
-				return nil, err
+				// Update type name: "map[CodeLensSource]T" -> "map[enum]T"
+				// hack: assumes key substring is unique!
+				typ = strings.Replace(typ, m.Key().String(), "enum", 1)
 			}
-			if keys != nil {
-				enumKeys = *keys
+
+			// Edge case: "analyses" is a string (not enum) keyed map,
+			// but its EnumKeys.ValueType was historically populated.
+			// (But not "env"; not sure why.)
+			if ok || name == "analyses" {
+				enumKeys.ValueType = m.Elem().String()
+
+				// For map[enum]T fields, gather the set of valid
+				// EnumKeys (from type information). If T=bool, also
+				// record the default value (from reflection).
+				keys, err := collectEnumKeys(m, reflectField, values)
+				if err != nil {
+					return nil, err
+				}
+				enumKeys.Keys = keys
 			}
 		}
 
@@ -350,20 +347,13 @@ func loadEnums(pkg *packages.Package) (map[types.Type][]doc.EnumValue, error) {
 	return enums, nil
 }
 
-func collectEnumKeys(name string, m *types.Map, reflectField reflect.Value, enumValues []doc.EnumValue) (*doc.EnumKeys, error) {
-	// Make sure the value type gets set for analyses and codelenses
-	// too.
-	if len(enumValues) == 0 && !hardcodedEnumKeys(name) {
-		return nil, nil
-	}
-	keys := &doc.EnumKeys{
-		ValueType: m.Elem().String(),
-	}
+func collectEnumKeys(m *types.Map, reflectField reflect.Value, enumValues []doc.EnumValue) ([]doc.EnumKey, error) {
 	// We can get default values for enum -> bool maps.
 	var isEnumBoolMap bool
 	if basic, ok := m.Elem().Underlying().(*types.Basic); ok && basic.Kind() == types.Bool {
 		isEnumBoolMap = true
 	}
+	var keys []doc.EnumKey
 	for _, v := range enumValues {
 		var def string
 		if isEnumBoolMap {
@@ -373,7 +363,7 @@ func collectEnumKeys(name string, m *types.Map, reflectField reflect.Value, enum
 				return nil, err
 			}
 		}
-		keys.Keys = append(keys.Keys, doc.EnumKey{
+		keys = append(keys, doc.EnumKey{
 			Name:    v.Value,
 			Doc:     v.Doc,
 			Default: def,
@@ -529,19 +519,19 @@ func structDoc(fields []*commandmeta.Field, level int) string {
 	return b.String()
 }
 
-// loadLenses combines the syntactic comments from the protocol
+// loadLenses combines the syntactic comments from the settings
 // package with the default values from settings.DefaultOptions(), and
 // returns a list of Code Lens descriptors.
-func loadLenses(protocolPkg *packages.Package, defaults map[protocol.CodeLensSource]bool) ([]*doc.Lens, error) {
+func loadLenses(settingsPkg *packages.Package, defaults map[settings.CodeLensSource]bool) ([]*doc.Lens, error) {
 	// Find the CodeLensSource enums among the files of the protocol package.
 	// Map each enum value to its doc comment.
 	enumDoc := make(map[string]string)
-	for _, f := range protocolPkg.Syntax {
+	for _, f := range settingsPkg.Syntax {
 		for _, decl := range f.Decls {
 			if decl, ok := decl.(*ast.GenDecl); ok && decl.Tok == token.CONST {
 				for _, spec := range decl.Specs {
 					spec := spec.(*ast.ValueSpec)
-					posn := safetoken.StartPosition(protocolPkg.Fset, spec.Pos())
+					posn := safetoken.StartPosition(settingsPkg.Fset, spec.Pos())
 					if id, ok := spec.Type.(*ast.Ident); ok && id.Name == "CodeLensSource" {
 						if len(spec.Names) != 1 || len(spec.Values) != 1 {
 							return nil, fmt.Errorf("%s: declare one CodeLensSource per line", posn)
@@ -566,7 +556,7 @@ func loadLenses(protocolPkg *packages.Package, defaults map[protocol.CodeLensSou
 
 	// Build list of Lens descriptors.
 	var lenses []*doc.Lens
-	addAll := func(sources map[protocol.CodeLensSource]cache.CodeLensSourceFunc, fileType string) error {
+	addAll := func(sources map[settings.CodeLensSource]cache.CodeLensSourceFunc, fileType string) error {
 		slice := maps.Keys(sources)
 		sort.Slice(slice, func(i, j int) bool { return slice[i] < slice[j] })
 		for _, source := range slice {
@@ -732,10 +722,6 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) {
 				buf.WriteString(opt.Doc)
 
 				// enums
-				//
-				// TODO(adonovan): `CodeLensSource` should be treated as an enum,
-				// but loadEnums considers only the `settings` package,
-				// not `protocol`.
 				write := func(name, doc string) {
 					if doc != "" {
 						unbroken := parBreakRE.ReplaceAllString(doc, "\\\n")
@@ -745,12 +731,14 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) {
 					}
 				}
 				if len(opt.EnumValues) > 0 && opt.Type == "enum" {
+					// enum as top-level type constructor
 					buf.WriteString("\nMust be one of:\n\n")
 					for _, val := range opt.EnumValues {
 						write(val.Value, val.Doc)
 					}
 				} else if len(opt.EnumKeys.Keys) > 0 && shouldShowEnumKeysInSettings(opt.Name) {
-					buf.WriteString("\nCan contain any of:\n\n")
+					// enum as map key (currently just "annotations")
+					buf.WriteString("\nEach enum must be one of:\n\n")
 					for _, val := range opt.EnumKeys.Keys {
 						write(val.Name, val.Doc)
 					}
@@ -772,7 +760,9 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) {
 var parBreakRE = regexp.MustCompile("\n{2,}")
 
 func shouldShowEnumKeysInSettings(name string) bool {
-	// These fields have too many possible options to print.
+	// These fields have too many possible options,
+	// or too voluminous documentation, to render as enums.
+	// Instead they each get their own page in the manual.
 	return !(name == "analyses" || name == "codelenses" || name == "hints")
 }
 
@@ -830,10 +820,6 @@ func collectGroups(opts []*doc.Option) []optionsGroup {
 	return groups
 }
 
-func hardcodedEnumKeys(name string) bool {
-	return name == "analyses" || name == "codelenses"
-}
-
 func capitalize(s string) string {
 	return string(unicode.ToUpper(rune(s[0]))) + s[1:]
 }

gopls/doc/settings.md

@@ -180,7 +180,7 @@ Default: `false`.
 ## UI
 
 <a id='codelenses'></a>
-### `codelenses` *map[golang.org/x/tools/gopls/internal/protocol.CodeLensSource]bool*
+### `codelenses` *map[enum]bool*
 
 codelenses overrides the enabled/disabled state of each of gopls'
 sources of [Code Lenses](codelenses.md).
@@ -325,14 +325,14 @@ These analyses are documented on
 Default: `false`.
 
 <a id='annotations'></a>
-### `annotations` *map[string]bool*
+### `annotations` *map[enum]bool*
 
 **This setting is experimental and may be deleted.**
 
 annotations specifies the various kinds of optimization diagnostics
 that should be reported by the gc_details command.
 
-Can contain any of:
+Each enum must be one of:
 
 * `"bounds"` controls bounds checking diagnostics.
 * `"escape"` controls diagnostics about escape choices.

gopls/internal/cmd/codelens.go

@@ -78,9 +78,9 @@ func (r *codelens) Run(ctx context.Context, args ...string) error {
 			origOptions(opts)
 		}
 		if opts.Codelenses == nil {
-			opts.Codelenses = make(map[protocol.CodeLensSource]bool)
+			opts.Codelenses = make(map[settings.CodeLensSource]bool)
 		}
-		opts.Codelenses[protocol.CodeLensTest] = true
+		opts.Codelenses[settings.CodeLensTest] = true
 	}
 
 	conn, err := r.app.connect(ctx)

gopls/internal/doc/api.json

@@ -646,7 +646,7 @@
 			},
 			{
 				"Name": "annotations",
-				"Type": "map[string]bool",
+				"Type": "map[enum]bool",
 				"Doc": "annotations specifies the various kinds of optimization diagnostics\nthat should be reported by the gc_details command.\n",
 				"EnumKeys": {
 					"ValueType": "bool",
@@ -799,49 +799,49 @@
 			},
 			{
 				"Name": "codelenses",
-				"Type": "map[golang.org/x/tools/gopls/internal/protocol.CodeLensSource]bool",
+				"Type": "map[enum]bool",
 				"Doc": "codelenses overrides the enabled/disabled state of each of gopls'\nsources of [Code Lenses](codelenses.md).\n\nExample Usage:\n\n```json5\n\"gopls\": {\n...\n  \"codelenses\": {\n    \"generate\": false,  // Don't show the `go generate` lens.\n    \"gc_details\": true  // Show a code lens toggling the display of gc's choices.\n  }\n...\n}\n```\n",
 				"EnumKeys": {
 					"ValueType": "bool",
 					"Keys": [
 						{
 							"Name": "\"gc_details\"",
-							"Doc": "\nThis codelens source causes the `package` declaration of\neach file to be annotated with a command to toggle the\nstate of the per-session variable that controls whether\noptimization decisions from the Go compiler (formerly known\nas \"gc\") should be displayed as diagnostics.\n\nOptimization decisions include:\n- whether a variable escapes, and how escape is inferred;\n- whether a nil-pointer check is implied or eliminated;\n- whether a function can be inlined.\n\nTODO(adonovan): this source is off by default because the\nannotation is annoying and because VS Code has a separate\n\"Toggle gc details\" command. Replace it with a Code Action\n(\"Source action...\").\n",
+							"Doc": "`\"gc_details\"`: Toggle display of Go compiler optimization decisions\n\nThis codelens source causes the `package` declaration of\neach file to be annotated with a command to toggle the\nstate of the per-session variable that controls whether\noptimization decisions from the Go compiler (formerly known\nas \"gc\") should be displayed as diagnostics.\n\nOptimization decisions include:\n- whether a variable escapes, and how escape is inferred;\n- whether a nil-pointer check is implied or eliminated;\n- whether a function can be inlined.\n\nTODO(adonovan): this source is off by default because the\nannotation is annoying and because VS Code has a separate\n\"Toggle gc details\" command. Replace it with a Code Action\n(\"Source action...\").\n",
 							"Default": "false"
 						},
 						{
 							"Name": "\"generate\"",
-							"Doc": "\nThis codelens source annotates any `//go:generate` comments\nwith commands to run `go generate` in this directory, on\nall directories recursively beneath this one.\n\nSee [Generating code](https://go.dev/blog/generate) for\nmore details.\n",
+							"Doc": "`\"generate\"`: Run `go generate`\n\nThis codelens source annotates any `//go:generate` comments\nwith commands to run `go generate` in this directory, on\nall directories recursively beneath this one.\n\nSee [Generating code](https://go.dev/blog/generate) for\nmore details.\n",
 							"Default": "true"
 						},
 						{
 							"Name": "\"regenerate_cgo\"",
-							"Doc": "\nThis codelens source annotates an `import \"C\"` declaration\nwith a command to re-run the [cgo\ncommand](https://pkg.go.dev/cmd/cgo) to regenerate the\ncorresponding Go declarations.\n\nUse this after editing the C code in comments attached to\nthe import, or in C header files included by it.\n",
+							"Doc": "`\"regenerate_cgo\"`: Re-generate cgo declarations\n\nThis codelens source annotates an `import \"C\"` declaration\nwith a command to re-run the [cgo\ncommand](https://pkg.go.dev/cmd/cgo) to regenerate the\ncorresponding Go declarations.\n\nUse this after editing the C code in comments attached to\nthe import, or in C header files included by it.\n",
 							"Default": "true"
 						},
 						{
-							"Name": "\"test\"",
-							"Doc": "\nThis codelens source annotates each `Test` and `Benchmark`\nfunction in a `*_test.go` file with a command to run it.\n\nThis source is off by default because VS Code has\na client-side custom UI for testing, and because progress\nnotifications are not a great UX for streamed test output.\nSee:\n- golang/go#67400 for a discussion of this feature.\n- https://github.com/joaotavora/eglot/discussions/1402\n  for an alternative approach.\n",
+							"Name": "\"run_govulncheck\"",
+							"Doc": "`\"run_govulncheck\"`: Run govulncheck\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run Govulncheck.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static\nanalysis tool that computes the set of functions reachable\nwithin your application, including dependencies;\nqueries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
 							"Default": "false"
 						},
 						{
-							"Name": "\"run_govulncheck\"",
-							"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run Govulncheck.\n\n[Govulncheck](https://go.dev/blog/vuln) is a static\nanalysis tool that computes the set of functions reachable\nwithin your application, including dependencies;\nqueries a database of known security vulnerabilities; and\nreports any potential problems it finds.\n",
+							"Name": "\"test\"",
+							"Doc": "`\"test\"`: Run tests and benchmarks\n\nThis codelens source annotates each `Test` and `Benchmark`\nfunction in a `*_test.go` file with a command to run it.\n\nThis source is off by default because VS Code has\na client-side custom UI for testing, and because progress\nnotifications are not a great UX for streamed test output.\nSee:\n- golang/go#67400 for a discussion of this feature.\n- https://github.com/joaotavora/eglot/discussions/1402\n  for an alternative approach.\n",
 							"Default": "false"
 						},
 						{
 							"Name": "\"tidy\"",
-							"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\ntidy`](https://go.dev/ref/mod#go-mod-tidy), which ensures\nthat the go.mod file matches the source code in the module.\n",
+							"Doc": "`\"tidy\"`: Tidy go.mod file\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\ntidy`](https://go.dev/ref/mod#go-mod-tidy), which ensures\nthat the go.mod file matches the source code in the module.\n",
 							"Default": "true"
 						},
 						{
 							"Name": "\"upgrade_dependency\"",
-							"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with commands to:\n\n- check for available upgrades,\n- upgrade direct dependencies, and\n- upgrade all dependencies transitively.\n",
+							"Doc": "`\"upgrade_dependency\"`: Update dependencies\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with commands to:\n\n- check for available upgrades,\n- upgrade direct dependencies, and\n- upgrade all dependencies transitively.\n",
 							"Default": "true"
 						},
 						{
 							"Name": "\"vendor\"",
-							"Doc": "\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\nvendor`](https://go.dev/ref/mod#go-mod-vendor), which\ncreates or updates the directory named `vendor` in the\nmodule root so that it contains an up-to-date copy of all\nnecessary package dependencies.\n",
+							"Doc": "`\"vendor\"`: Update vendor directory\n\nThis codelens source annotates the `module` directive in a\ngo.mod file with a command to run [`go mod\nvendor`](https://go.dev/ref/mod#go-mod-vendor), which\ncreates or updates the directory named `vendor` in the\nmodule root so that it contains an up-to-date copy of all\nnecessary package dependencies.\n",
 							"Default": "true"
 						}
 					]

gopls/internal/golang/code_lens.go

@@ -17,15 +17,16 @@ import (
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/protocol/command"
+	"golang.org/x/tools/gopls/internal/settings"
 )
 
 // CodeLensSources returns the supported sources of code lenses for Go files.
-func CodeLensSources() map[protocol.CodeLensSource]cache.CodeLensSourceFunc {
-	return map[protocol.CodeLensSource]cache.CodeLensSourceFunc{
-		protocol.CodeLensGenerate:      goGenerateCodeLens,    // commands: Generate
-		protocol.CodeLensTest:          runTestCodeLens,       // commands: Test
-		protocol.CodeLensRegenerateCgo: regenerateCgoLens,     // commands: RegenerateCgo
-		protocol.CodeLensGCDetails:     toggleDetailsCodeLens, // commands: GCDetails
+func CodeLensSources() map[settings.CodeLensSource]cache.CodeLensSourceFunc {
+	return map[settings.CodeLensSource]cache.CodeLensSourceFunc{
+		settings.CodeLensGenerate:      goGenerateCodeLens,    // commands: Generate
+		settings.CodeLensTest:          runTestCodeLens,       // commands: Test
+		settings.CodeLensRegenerateCgo: regenerateCgoLens,     // commands: RegenerateCgo
+		settings.CodeLensGCDetails:     toggleDetailsCodeLens, // commands: GCDetails
 	}
 }