internal/lsp: use an enum for GC annotations settings

The annotations map should use an enum to indicate expected values. For
now, we reuse the EnumValues to expose the information in the settings.
Later, we'll create a separate EnumKeys field to expose this information
more correctly.

Also, adjust some of the logic that applies the settings because it was
incorrect.

Both gopls/doc/settings.md and internal/lsp/source/api_json.go are
generated files.

Updates golang/go#42961

Change-Id: Ifb032b70caaae73defe9a540df20d098d313e68e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280354
Trust: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Peter Weinberger <[email protected]>

Author: stamblerre

gopls/doc/generate.go

@@ -163,12 +163,21 @@ func loadOptions(category reflect.Value, pkg *packages.Package) ([]*source.Optio
 			typ = "enum"
 		}
 
+		// Track any maps whose keys are enums.
+		enumValues := enums[typesField.Type()]
+		if m, ok := typesField.Type().(*types.Map); ok {
+			if e, ok := enums[m.Key()]; ok {
+				enumValues = e
+				typ = strings.Replace(typ, m.Key().String(), m.Key().Underlying().String(), 1)
+			}
+		}
+
 		opts = append(opts, &source.OptionJSON{
 			Name:       lowerFirst(typesField.Name()),
 			Type:       typ,
 			Doc:        lowerFirst(astField.Doc.Text()),
 			Default:    string(defBytes),
-			EnumValues: enums[typesField.Type()],
+			EnumValues: enumValues,
 		})
 	}
 	return opts, nil
@@ -378,14 +387,23 @@ func rewriteSettings(doc []byte, api *source.APIJSON) ([]byte, error) {
 		for _, opt := range opts {
 			var enumValues strings.Builder
 			if len(opt.EnumValues) > 0 {
-				enumValues.WriteString("Must be one of:\n\n")
-				for _, val := range opt.EnumValues {
+				var msg string
+				if opt.Type == "enum" {
+					msg = "\nMust be one of:\n\n"
+				} else {
+					msg = "\nCan contain any of:\n\n"
+				}
+				enumValues.WriteString(msg)
+				for i, val := range opt.EnumValues {
 					if val.Doc != "" {
 						// Don't break the list item by starting a new paragraph.
 						unbroken := parBreakRE.ReplaceAllString(val.Doc, "\\\n")
-						fmt.Fprintf(&enumValues, " * %s\n", unbroken)
+						fmt.Fprintf(&enumValues, "* %s", unbroken)
 					} else {
-						fmt.Fprintf(&enumValues, " * `%s`\n", val.Value)
+						fmt.Fprintf(&enumValues, "* `%s`", val.Value)
+					}
+					if i < len(opt.EnumValues)-1 {
+						fmt.Fprint(&enumValues, "\n")
 					}
 				}
 			}

gopls/doc/settings.md

@@ -36,18 +36,18 @@ Default: `{}`.
 ### **hoverKind** *enum*
 hoverKind controls the information that appears in the hover text.
 SingleLine and Structured are intended for use only by authors of editor plugins.
+
 Must be one of:
 
- * `"FullDocumentation"`
- * `"NoDocumentation"`
- * `"SingleLine"`
- * `"Structured"` is an experimental setting that returns a structured hover format.
+* `"FullDocumentation"`
+* `"NoDocumentation"`
+* `"SingleLine"`
+* `"Structured"` is an experimental setting that returns a structured hover format.
 This format separates the signature from the documentation, so that the client
 can do more manipulation of these fields.\
 This should only be used by clients that support this behavior.
 
- * `"SynopsisDocumentation"`
-
+* `"SynopsisDocumentation"`
 
 Default: `"FullDocumentation"`.
 ### **usePlaceholders** *bool*
@@ -120,32 +120,32 @@ Default: `true`.
 ### **importShortcut** *enum*
 importShortcut specifies whether import statements should link to
 documentation or go to definitions.
-Must be one of:
 
- * `"Both"`
- * `"Definition"`
- * `"Link"`
+Must be one of:
 
+* `"Both"`
+* `"Definition"`
+* `"Link"`
 
 Default: `"Both"`.
 ### **matcher** *enum*
 matcher sets the algorithm that is used when calculating completion candidates.
-Must be one of:
 
- * `"CaseInsensitive"`
- * `"CaseSensitive"`
- * `"Fuzzy"`
+Must be one of:
 
+* `"CaseInsensitive"`
+* `"CaseSensitive"`
+* `"Fuzzy"`
 
 Default: `"Fuzzy"`.
 ### **symbolMatcher** *enum*
 symbolMatcher sets the algorithm that is used when finding workspace symbols.
-Must be one of:
 
- * `"CaseInsensitive"`
- * `"CaseSensitive"`
- * `"Fuzzy"`
+Must be one of:
 
+* `"CaseInsensitive"`
+* `"CaseSensitive"`
+* `"Fuzzy"`
 
 Default: `"Fuzzy"`.
 ### **symbolStyle** *enum*
@@ -159,21 +159,21 @@ Example Usage:
 ...
 }
 ```
+
 Must be one of:
 
- * `"Dynamic"` uses whichever qualifier results in the highest scoring
+* `"Dynamic"` uses whichever qualifier results in the highest scoring
 match for the given symbol query. Here a "qualifier" is any "/" or "."
 delimited suffix of the fully qualified symbol. i.e. "to/pkg.Foo.Field" or
 just "Foo.Field".
 
- * `"Full"` is fully qualified symbols, i.e.
+* `"Full"` is fully qualified symbols, i.e.
 "path/to/pkg.Foo.Field".
 
- * `"Package"` is package qualified symbols i.e.
+* `"Package"` is package qualified symbols i.e.
 "pkg.Foo.Field".
 
 
-
 Default: `"Dynamic"`.
 ### **directoryFilters** *[]string*
 directoryFilters can be used to exclude unwanted directories from the
@@ -198,15 +198,21 @@ The below settings are considered experimental. They may be deprecated or change
 
 <!-- BEGIN Experimental: DO NOT MANUALLY EDIT THIS SECTION -->
 ### **annotations** *map[string]bool*
-annotations suppress various kinds of optimization diagnostics
-that would be reported by the gc_details command.
- * noNilcheck suppresses display of nilchecks.
- * noEscape suppresses escape choices.
- * noInline suppresses inlining choices.
- * noBounds suppresses bounds checking diagnostics.
+annotations specifies the various kinds of optimization diagnostics
+that should be reported by the gc_details command.
 
+Can contain any of:
 
-Default: `{}`.
+* `"bounds"` controls bounds checking diagnostics.
+
+* `"escape"` controls diagnostics about escape choices.
+
+* `"inline"` controls diagnostics about inlining choices.
+
+* `"nil"` controls nil checks.
+
+
+Default: `{"bounds":true,"escape":true,"inline":true,"nil":true}`.
 ### **staticcheck** *bool*
 staticcheck enables additional analyses from staticcheck.io.
 

internal/lsp/source/api_json.go

@@ -19,6 +19,22 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
+type Annotation string
+
+const (
+	// Nil controls nil checks.
+	Nil Annotation = "nil"
+
+	// Escape controls diagnostics about escape choices.
+	Escape Annotation = "escape"
+
+	// Inline controls diagnostics about inlining choices.
+	Inline Annotation = "inline"
+
+	// Bounds controls bounds checking diagnostics.
+	Bounds Annotation = "bounds"
+)
+
 func GCOptimizationDetails(ctx context.Context, snapshot Snapshot, pkgDir span.URI) (map[VersionedFileIdentity][]*Diagnostic, error) {
 	outDir := filepath.Join(os.TempDir(), fmt.Sprintf("gopls-%d.details", os.Getpid()))
 
@@ -113,7 +129,7 @@ func parseDetailsFile(filename string, options *Options) (span.URI, []*Diagnosti
 		if msg != "" {
 			msg = fmt.Sprintf("%s(%s)", msg, d.Message)
 		}
-		if skipDiagnostic(msg, d.Source, options) {
+		if !showDiagnostic(msg, d.Source, options) {
 			continue
 		}
 		var related []RelatedInformation
@@ -138,24 +154,27 @@ func parseDetailsFile(filename string, options *Options) (span.URI, []*Diagnosti
 	return uri, diagnostics, nil
 }
 
-// skipDiagnostic reports whether a given diagnostic should be shown to the end
+// showDiagnostic reports whether a given diagnostic should be shown to the end
 // user, given the current options.
-func skipDiagnostic(msg, source string, o *Options) bool {
+func showDiagnostic(msg, source string, o *Options) bool {
 	if source != "go compiler" {
 		return false
 	}
+	if o.Annotations == nil {
+		return true
+	}
 	switch {
-	case o.Annotations["noInline"]:
-		return strings.HasPrefix(msg, "canInline") ||
-			strings.HasPrefix(msg, "cannotInline") ||
-			strings.HasPrefix(msg, "inlineCall")
-	case o.Annotations["noEscape"]:
-		return strings.HasPrefix(msg, "escape") || msg == "leak"
-	case o.Annotations["noNilcheck"]:
-		return strings.HasPrefix(msg, "nilcheck")
-	case o.Annotations["noBounds"]:
-		return strings.HasPrefix(msg, "isInBounds") ||
-			strings.HasPrefix(msg, "isSliceInBounds")
+	case strings.HasPrefix(msg, "canInline") ||
+		strings.HasPrefix(msg, "cannotInline") ||
+		strings.HasPrefix(msg, "inlineCall"):
+		return o.Annotations[Inline]
+	case strings.HasPrefix(msg, "escape") || msg == "leak":
+		return o.Annotations[Escape]
+	case strings.HasPrefix(msg, "nilcheck"):
+		return o.Annotations[Nil]
+	case strings.HasPrefix(msg, "isInBounds") ||
+		strings.HasPrefix(msg, "isSliceInBounds"):
+		return o.Annotations[Bounds]
 	}
 	return false
 }