Skip to content

Commit

Permalink
[gopls-release-branch.0.16] gopls/internal/settings: move CodeLensSou…
Browse files Browse the repository at this point in the history 
…rce 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]>
  • Loading branch information
@adonovan @gopherbot
adonovan authored and gopherbot committed Jun 20, 2024
1 parent 6c3e23f commit cb3016b
Show file tree
Hide file tree
Showing 11 changed files with 210 additions and 220 deletions.
120 changes: 53 additions & 67 deletions gopls/doc/generate/generate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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
}
Expand All @@ -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":
Expand All @@ -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),
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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)
}
Expand All @@ -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")
}

Expand Down Expand Up @@ -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:]
}
Expand Down
6 changes: 3 additions & 3 deletions gopls/doc/settings.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/cmd/codelens.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 12 additions & 12 deletions gopls/internal/doc/api.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
]
Expand Down
13 changes: 7 additions & 6 deletions gopls/internal/golang/code_lens.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
Loading

0 comments on commit cb3016b

Please sign in to comment.