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 <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>

Author: findleyr

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

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.

gopls/internal/lsp/source/api_json.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,15 +873,15 @@ 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{}{}
 
 	switch name {
 	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,14 +1196,15 @@ 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)
 }
 
 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,15 +1275,15 @@ 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)
 	for a, enabled := range all {
 		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
 }

gopls/internal/lsp/source/options.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"),
+			),
+		)
 	})
 }