From bddb3720ba870cf1e2156ce9147e1b89a255c1dc Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Mon, 26 Sep 2022 18:17:59 -0400 Subject: [PATCH] gopls: deprecate three experimental features Deprecate the following features, which never made it out of experimental: - experimentalWorkspaceModule (golang/go#52897) - experimentalWatchedFileDelay (golang/go#55268) - experimentalUseInvalidMetadata (golang/go#54180) See the associated issues for rationale behind the deprecations. With this change, any users configuring these settings will get a ShowMessageRequest warning them of the deprecation. Fixes golang/go#52897 Fixes golang/go#55268 Fixes golang/go#54180 Change-Id: I49f178e68793df4e4f9edb63e9b03cad127c5f51 Reviewed-on: https://go-review.googlesource.com/c/tools/+/434640 TryBot-Result: Gopher Robot Reviewed-by: Alan Donovan Run-TryBot: Robert Findley gopls-CI: kokoro --- gopls/doc/settings.md | 12 ++- gopls/doc/workspace.md | 6 +- gopls/internal/lsp/source/api_json.go | 6 +- gopls/internal/lsp/source/options.go | 85 +++++++++++++------ .../regtest/misc/configuration_test.go | 26 +++++- 5 files changed, 103 insertions(+), 32 deletions(-) diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md index 58e97503f36..01b5d1a4da6 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -123,6 +123,9 @@ Default: `true`. experimentalWorkspaceModule opts a user into the experimental support for multi-module workspaces. +Deprecated: this feature is deprecated and will be removed in a future +version of gopls (https://go.dev/issue/55331). + Default: `false`. #### **experimentalPackageCacheKey** *bool* @@ -164,8 +167,10 @@ Default: `false`. experimentalUseInvalidMetadata enables gopls to fall back on outdated package metadata to provide editor features if the go command fails to -load packages for some reason (like an invalid go.mod file). This will -eventually be the default behavior, and this setting will be removed. +load packages for some reason (like an invalid go.mod file). + +Deprecated: this setting is deprecated and will be removed in a future +version of gopls (https://go.dev/issue/55333). Default: `false`. @@ -353,6 +358,9 @@ file system notifications. This option must be set to a valid duration string, for example `"100ms"`. +Deprecated: this setting is deprecated and will be removed in a future +version of gopls (https://go.dev/issue/55332) + Default: `"0s"`. #### Documentation diff --git a/gopls/doc/workspace.md b/gopls/doc/workspace.md index 734eaddbab9..cb166131fba 100644 --- a/gopls/doc/workspace.md +++ b/gopls/doc/workspace.md @@ -44,7 +44,11 @@ go work use tools tools/gopls ...followed by opening the `$WORK` directory in our editor. -#### Experimental workspace module (Go 1.17 and earlier) +#### DEPRECATED: Experimental workspace module (Go 1.17 and earlier) + +**This feature is deprecated and will be removed in future versions of gopls. +Please see [issue #52897](https://go.dev/issue/52897) for additional +information.** With earlier versions of Go, `gopls` can simulate multi-module workspaces by creating a synthetic module requiring the modules in the workspace root. diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go index 957319a3719..e40ecc2a16d 100755 --- a/gopls/internal/lsp/source/api_json.go +++ b/gopls/internal/lsp/source/api_json.go @@ -59,7 +59,7 @@ var GeneratedAPIJSON = &APIJSON{ { Name: "experimentalWorkspaceModule", Type: "bool", - Doc: "experimentalWorkspaceModule opts a user into the experimental support\nfor multi-module workspaces.\n", + Doc: "experimentalWorkspaceModule opts a user into the experimental support\nfor multi-module workspaces.\n\nDeprecated: this feature is deprecated and will be removed in a future\nversion of gopls (https://go.dev/issue/55331).\n", Default: "false", Status: "experimental", Hierarchy: "build", @@ -91,7 +91,7 @@ var GeneratedAPIJSON = &APIJSON{ { Name: "experimentalUseInvalidMetadata", Type: "bool", - Doc: "experimentalUseInvalidMetadata enables gopls to fall back on outdated\npackage metadata to provide editor features if the go command fails to\nload packages for some reason (like an invalid go.mod file). This will\neventually be the default behavior, and this setting will be removed.\n", + Doc: "experimentalUseInvalidMetadata enables gopls to fall back on outdated\npackage metadata to provide editor features if the go command fails to\nload packages for some reason (like an invalid go.mod file).\n\nDeprecated: this setting is deprecated and will be removed in a future\nversion of gopls (https://go.dev/issue/55333).\n", Default: "false", Status: "experimental", Hierarchy: "build", @@ -510,7 +510,7 @@ var GeneratedAPIJSON = &APIJSON{ { Name: "experimentalWatchedFileDelay", Type: "time.Duration", - Doc: "experimentalWatchedFileDelay controls the amount of time that gopls waits\nfor additional workspace/didChangeWatchedFiles notifications to arrive,\nbefore processing all such notifications in a single batch. This is\nintended for use by LSP clients that don't support their own batching of\nfile system notifications.\n\nThis option must be set to a valid duration string, for example `\"100ms\"`.\n", + Doc: "experimentalWatchedFileDelay controls the amount of time that gopls waits\nfor additional workspace/didChangeWatchedFiles notifications to arrive,\nbefore processing all such notifications in a single batch. This is\nintended for use by LSP clients that don't support their own batching of\nfile system notifications.\n\nThis option must be set to a valid duration string, for example `\"100ms\"`.\n\nDeprecated: this setting is deprecated and will be removed in a future\nversion of gopls (https://go.dev/issue/55332)\n", Default: "\"0s\"", Status: "experimental", Hierarchy: "ui.diagnostic", diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go index b15d7144bf6..110621c26b8 100644 --- a/gopls/internal/lsp/source/options.go +++ b/gopls/internal/lsp/source/options.go @@ -266,6 +266,9 @@ type BuildOptions struct { // ExperimentalWorkspaceModule opts a user into the experimental support // for multi-module workspaces. + // + // Deprecated: this feature is deprecated and will be removed in a future + // version of gopls (https://go.dev/issue/55331). ExperimentalWorkspaceModule bool `status:"experimental"` // ExperimentalPackageCacheKey controls whether to use a coarser cache key @@ -288,8 +291,10 @@ type BuildOptions struct { // ExperimentalUseInvalidMetadata enables gopls to fall back on outdated // package metadata to provide editor features if the go command fails to - // load packages for some reason (like an invalid go.mod file). This will - // eventually be the default behavior, and this setting will be removed. + // load packages for some reason (like an invalid go.mod file). + // + // Deprecated: this setting is deprecated and will be removed in a future + // version of gopls (https://go.dev/issue/55333). ExperimentalUseInvalidMetadata bool `status:"experimental"` } @@ -425,6 +430,9 @@ type DiagnosticOptions struct { // file system notifications. // // This option must be set to a valid duration string, for example `"100ms"`. + // + // Deprecated: this setting is deprecated and will be removed in a future + // version of gopls (https://go.dev/issue/55332) ExperimentalWatchedFileDelay time.Duration `status:"experimental"` } @@ -865,7 +873,7 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) result := OptionResult{Name: name, Value: value} if _, ok := seen[name]; ok { - result.errorf("duplicate configuration for %s", name) + result.parseErrorf("duplicate configuration for %s", name) } seen[name] = struct{}{} @@ -873,7 +881,7 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) case "env": menv, ok := value.(map[string]interface{}) if !ok { - result.errorf("invalid type %T, expect map", value) + result.parseErrorf("invalid type %T, expect map", value) break } if o.Env == nil { @@ -886,7 +894,7 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) case "buildFlags": iflags, ok := value.([]interface{}) if !ok { - result.errorf("invalid type %T, expect list", value) + result.parseErrorf("invalid type %T, expect list", value) break } flags := make([]string, 0, len(iflags)) @@ -897,14 +905,14 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) case "directoryFilters": ifilters, ok := value.([]interface{}) if !ok { - result.errorf("invalid type %T, expect list", value) + result.parseErrorf("invalid type %T, expect list", value) break } var filters []string for _, ifilter := range ifilters { filter, err := validateDirectoryFilter(fmt.Sprintf("%v", ifilter)) if err != nil { - result.errorf(err.Error()) + result.parseErrorf("%v", err) return result } filters = append(filters, strings.TrimRight(filepath.FromSlash(filter), "/")) @@ -1048,7 +1056,12 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) case "experimentalPostfixCompletions": result.setBool(&o.ExperimentalPostfixCompletions) - case "experimentalWorkspaceModule": // TODO(rfindley): suggest go.work on go1.18+ + case "experimentalWorkspaceModule": + const msg = "The experimentalWorkspaceModule feature has been replaced by go workspaces, " + + "and will be removed in a future version of gopls (https://go.dev/issue/55331). " + + "Please see https://github.com/golang/tools/blob/master/gopls/doc/workspace.md " + + "for information on setting up multi-module workspaces using go.work files." + result.softErrorf(msg) result.setBool(&o.ExperimentalWorkspaceModule) case "experimentalTemplateSupport": // TODO(pjw): remove after June 2022 @@ -1067,14 +1080,18 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) o.TemplateExtensions = nil break } - result.errorf(fmt.Sprintf("unexpected type %T not []string", value)) - case "experimentalDiagnosticsDelay", "diagnosticsDelay": - if name == "experimentalDiagnosticsDelay" { - result.deprecated("diagnosticsDelay") - } + result.parseErrorf("unexpected type %T not []string", value) + + case "experimentalDiagnosticsDelay": + result.deprecated("diagnosticsDelay") + + case "diagnosticsDelay": result.setDuration(&o.DiagnosticsDelay) case "experimentalWatchedFileDelay": + const msg = "The experimentalWatchedFileDelay setting is deprecated, and will " + + "be removed in a future version of gopls (https://go.dev/issue/55332)." + result.softErrorf(msg) result.setDuration(&o.ExperimentalWatchedFileDelay) case "experimentalPackageCacheKey": @@ -1087,6 +1104,9 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) result.setBool(&o.AllowImplicitNetworkAccess) case "experimentalUseInvalidMetadata": + const msg = "The experimentalUseInvalidMetadata setting is deprecated, and will be removed" + + "in a future version of gopls (https://go.dev/issue/55333)." + result.softErrorf(msg) result.setBool(&o.ExperimentalUseInvalidMetadata) case "allExperiments": @@ -1140,7 +1160,11 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{}) return result } -func (r *OptionResult) errorf(msg string, values ...interface{}) { +// parseErrorf reports an error parsing the current configuration value. +func (r *OptionResult) parseErrorf(msg string, values ...interface{}) { + if false { + _ = fmt.Sprintf(msg, values...) // this causes vet to check this like printf + } prefix := fmt.Sprintf("parsing setting %q: ", r.Name) r.Error = fmt.Errorf(prefix+msg, values...) } @@ -1154,6 +1178,16 @@ func (e *SoftError) Error() string { return e.msg } +// softErrorf reports an error that does not affect the functionality of gopls +// (a warning in the UI). +// The formatted message will be shown to the user unmodified. +func (r *OptionResult) softErrorf(format string, values ...interface{}) { + msg := fmt.Sprintf(format, values...) + r.Error = &SoftError{msg} +} + +// deprecated reports the current setting as deprecated. If 'replacement' is +// non-nil, it is suggested to the user. func (r *OptionResult) deprecated(replacement string) { msg := fmt.Sprintf("gopls setting %q is deprecated", r.Name) if replacement != "" { @@ -1162,6 +1196,7 @@ func (r *OptionResult) deprecated(replacement string) { r.Error = &SoftError{msg} } +// unexpected reports that the current setting is not known to gopls. func (r *OptionResult) unexpected() { r.Error = fmt.Errorf("unexpected gopls setting %q", r.Name) } @@ -1169,7 +1204,7 @@ func (r *OptionResult) unexpected() { func (r *OptionResult) asBool() (bool, bool) { b, ok := r.Value.(bool) if !ok { - r.errorf("invalid type %T, expect bool", r.Value) + r.parseErrorf("invalid type %T, expect bool", r.Value) return false, false } return b, true @@ -1185,7 +1220,7 @@ func (r *OptionResult) setDuration(d *time.Duration) { if v, ok := r.asString(); ok { parsed, err := time.ParseDuration(v) if err != nil { - r.errorf("failed to parse duration %q: %v", v, err) + r.parseErrorf("failed to parse duration %q: %v", v, err) return } *d = parsed @@ -1217,18 +1252,18 @@ func (r *OptionResult) setAnnotationMap(bm *map[Annotation]bool) { switch k { case "noEscape": m[Escape] = false - r.errorf(`"noEscape" is deprecated, set "Escape: false" instead`) + r.parseErrorf(`"noEscape" is deprecated, set "Escape: false" instead`) case "noNilcheck": m[Nil] = false - r.errorf(`"noNilcheck" is deprecated, set "Nil: false" instead`) + r.parseErrorf(`"noNilcheck" is deprecated, set "Nil: false" instead`) case "noInline": m[Inline] = false - r.errorf(`"noInline" is deprecated, set "Inline: false" instead`) + r.parseErrorf(`"noInline" is deprecated, set "Inline: false" instead`) case "noBounds": m[Bounds] = false - r.errorf(`"noBounds" is deprecated, set "Bounds: false" instead`) + r.parseErrorf(`"noBounds" is deprecated, set "Bounds: false" instead`) default: - r.errorf(err.Error()) + r.parseErrorf("%v", err) } continue } @@ -1240,7 +1275,7 @@ func (r *OptionResult) setAnnotationMap(bm *map[Annotation]bool) { func (r *OptionResult) asBoolMap() map[string]bool { all, ok := r.Value.(map[string]interface{}) if !ok { - r.errorf("invalid type %T for map[string]bool option", r.Value) + r.parseErrorf("invalid type %T for map[string]bool option", r.Value) return nil } m := make(map[string]bool) @@ -1248,7 +1283,7 @@ func (r *OptionResult) asBoolMap() map[string]bool { if enabled, ok := enabled.(bool); ok { m[a] = enabled } else { - r.errorf("invalid type %T for map key %q", enabled, a) + r.parseErrorf("invalid type %T for map key %q", enabled, a) return m } } @@ -1258,7 +1293,7 @@ func (r *OptionResult) asBoolMap() map[string]bool { func (r *OptionResult) asString() (string, bool) { b, ok := r.Value.(string) if !ok { - r.errorf("invalid type %T, expect string", r.Value) + r.parseErrorf("invalid type %T, expect string", r.Value) return "", false } return b, true @@ -1271,7 +1306,7 @@ func (r *OptionResult) asOneOf(options ...string) (string, bool) { } s, err := asOneOf(s, options...) if err != nil { - r.errorf(err.Error()) + r.parseErrorf("%v", err) } return s, err == nil } diff --git a/gopls/internal/regtest/misc/configuration_test.go b/gopls/internal/regtest/misc/configuration_test.go index 9cd0f0ff91e..a1d46f036c6 100644 --- a/gopls/internal/regtest/misc/configuration_test.go +++ b/gopls/internal/regtest/misc/configuration_test.go @@ -71,6 +71,30 @@ var FooErr = errors.New("foo") WithOptions( Settings{"staticcheck": true}, ).Run(t, files, func(t *testing.T, env *Env) { - env.Await(ShownMessage("staticcheck is not supported")) + env.Await( + OnceMet( + InitialWorkspaceLoad, + ShownMessage("staticcheck is not supported"), + ), + ) + }) +} + +func TestDeprecatedSettings(t *testing.T) { + WithOptions( + Settings{ + "experimentalUseInvalidMetadata": true, + "experimentalWatchedFileDelay": "1s", + "experimentalWorkspaceModule": true, + }, + ).Run(t, "", func(t *testing.T, env *Env) { + env.Await( + OnceMet( + InitialWorkspaceLoad, + ShownMessage("experimentalWorkspaceModule"), + ShownMessage("experimentalUseInvalidMetadata"), + ShownMessage("experimentalWatchedFileDelay"), + ), + ) }) }