internal/lsp: refactor and document options

This change cleans up the structure of the Options struct in order to
clearly delineate which options should be configurable for the user.
Follow-up work is needed to refactor the completion options to fit into
this structure, as well as to make sure that the name of the field is
the name of the setting. This will make it easier to generate markdown
documentation from the code.

Also, remove options that are no longer in-use and mark them as
deprecated.

Change-Id: Ib34ae25789e21b76077a564601e487fbebfc5f48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212519
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/general.go

@@ -92,7 +92,7 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitializ
 				TriggerCharacters: []string{"(", ","},
 			},
 			TextDocumentSync: &protocol.TextDocumentSyncOptions{
-				Change:    options.TextDocumentSyncKind,
+				Change:    protocol.Incremental,
 				OpenClose: true,
 				Save: protocol.SaveOptions{
 					IncludeText: false,
@@ -130,7 +130,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 		)
 	}
 
-	if options.WatchFileChanges && options.DynamicWatchedFilesSupported {
+	if options.DynamicWatchedFilesSupported {
 		registrations = append(registrations, protocol.Registration{
 			ID:     "workspace/didChangeWatchedFiles",
 			Method: "workspace/didChangeWatchedFiles",

internal/lsp/source/options.go

@@ -43,11 +43,22 @@ import (
 
 var (
 	DefaultOptions = Options{
-		Env:                    os.Environ(),
+		ClientOptions:       DefaultClientOptions,
-		TextDocumentSyncKind:   protocol.Incremental,
+		ServerOptions:       DefaultServerOptions,
-		HoverKind:              SynopsisDocumentation,
+		UserOptions:         DefaultUserOptions,
-		InsertTextFormat:       protocol.PlainTextTextFormat,
+		DebuggingOptions:    DefaultDebuggingOptions,
+		ExperimentalOptions: DefaultExperimentalOptions,
+		Hooks:               DefaultHooks,
+	}
+	DefaultClientOptions = ClientOptions{
+		InsertTextFormat:              protocol.SnippetTextFormat,
 		PreferredContentFormat:        protocol.Markdown,
+		ConfigurationSupported:        true,
+		DynamicConfigurationSupported: true,
+		DynamicWatchedFilesSupported:  true,
+		LineFoldingOnly:               false,
+	}
+	DefaultServerOptions = ServerOptions{
 		SupportedCodeActions: map[FileKind]map[protocol.CodeActionKind]bool{
 			Go: {
 				protocol.SourceOrganizeImports: true,
@@ -61,6 +72,10 @@ var (
 		SupportedCommands: []string{
 			"tidy", // for go.mod files
 		},
+	}
+	DefaultUserOptions = UserOptions{
+		Env:       os.Environ(),
+		HoverKind: SynopsisDocumentation,
 		Completion: CompletionOptions{
 			Documentation: true,
 			Deep:          true,
@@ -68,61 +83,84 @@ var (
 			Literal:       true,
 			Budget:        100 * time.Millisecond,
 		},
+		LinkTarget: "pkg.go.dev",
+	}
+	DefaultHooks = Hooks{
 		ComputeEdits: myers.ComputeEdits,
 		URLRegexp:    regexp.MustCompile(`(http|ftp|https)://([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])?`),
 		Analyzers:    defaultAnalyzers,
 		GoDiff:       true,
-		LinkTarget:   "pkg.go.dev",
+	}
+	DefaultExperimentalOptions = ExperimentalOptions{
 		TempModfile: false,
 	}
+	DefaultDebuggingOptions = DebuggingOptions{}
 )
 
 type Options struct {
-	// Env is the current set of environment overrides on this view.
+	ClientOptions
-	Env []string
+	ServerOptions
+	UserOptions
+	DebuggingOptions
+	ExperimentalOptions
+	Hooks
+}
 
-	// BuildFlags is used to adjust the build flags applied to the view.
+type ClientOptions struct {
-	BuildFlags []string
-
-	HoverKind        HoverKind
-	DisabledAnalyses map[string]struct{}
-
-	StaticCheck bool
-	GoDiff      bool
-
-	WatchFileChanges              bool
 	InsertTextFormat              protocol.InsertTextFormat
 	ConfigurationSupported        bool
 	DynamicConfigurationSupported bool
 	DynamicWatchedFilesSupported  bool
 	PreferredContentFormat        protocol.MarkupKind
 	LineFoldingOnly               bool
+}
 
+type ServerOptions struct {
 	SupportedCodeActions map[FileKind]map[protocol.CodeActionKind]bool
-
 	SupportedCommands    []string
+}
 
-	// TODO: Remove the option once we are certain there are no issues here.
+type UserOptions struct {
-	TextDocumentSyncKind protocol.TextDocumentSyncKind
+	// Env is the current set of environment overrides on this view.
+	Env []string
 
-	Completion CompletionOptions
+	// BuildFlags is used to adjust the build flags applied to the view.
+	BuildFlags []string
 
-	ComputeEdits diff.ComputeEdits
+	// HoverKind specifies the format of the content for hover requests.
-	URLRegexp    *regexp.Regexp
+	HoverKind HoverKind
 
-	Analyzers map[string]*analysis.Analyzer
+	// DisabledAnalyses specify analyses that the user would like to disable.
+	DisabledAnalyses map[string]struct{}
+
+	// StaticCheck enables additional analyses from staticcheck.io.
+	StaticCheck bool
+
+	// LinkTarget is the website used for documentation.
+	LinkTarget string
 
 	// LocalPrefix is used to specify goimports's -local behavior.
 	LocalPrefix string
 
-	VerboseOutput bool
+	Completion CompletionOptions
+}
 
+type Hooks struct {
+	GoDiff       bool
+	ComputeEdits diff.ComputeEdits
+	URLRegexp    *regexp.Regexp
+	Analyzers    map[string]*analysis.Analyzer
+}
+
+type ExperimentalOptions struct {
 	// WARNING: This configuration will be changed in the future.
 	// It only exists while this feature is under development.
 	// Disable use of the -modfile flag in Go 1.14.
 	TempModfile bool
+}
 
-	LinkTarget string
+type DebuggingOptions struct {
+	VerboseOutput bool
 }
 
 type CompletionOptions struct {
@@ -241,12 +279,6 @@ func (o *Options) set(name string, value interface{}) OptionResult {
 		}
 		o.BuildFlags = flags
 
-	case "noIncrementalSync":
-		if v, ok := result.asBool(); ok && v {
-			o.TextDocumentSyncKind = protocol.Full
-		}
-	case "watchFileChanges":
-		result.setBool(&o.WatchFileChanges)
 	case "completionDocumentation":
 		result.setBool(&o.Completion.Documentation)
 	case "usePlaceholders":
@@ -312,9 +344,6 @@ func (o *Options) set(name string, value interface{}) OptionResult {
 	case "staticcheck":
 		result.setBool(&o.StaticCheck)
 
-	case "go-diff":
-		result.setBool(&o.GoDiff)
-
 	case "local":
 		localPrefix, ok := value.(string)
 		if !ok {
@@ -349,6 +378,15 @@ func (o *Options) set(name string, value interface{}) OptionResult {
 		result.State = OptionDeprecated
 		result.Replacement = "completeUnimported"
 
+	case "noIncrementalSync":
+		result.State = OptionDeprecated
+
+	case "watchFileChanges":
+		result.State = OptionDeprecated
+
+	case "go-diff":
+		result.State = OptionDeprecated
+
 	default:
 		result.State = OptionUnexpected
 	}

internal/lsp/text_synchronization.go

@@ -132,11 +132,6 @@ func (s *Server) changedText(ctx context.Context, uri span.URI, changes []protoc
 		return []byte(changes[0].Text), nil
 	}
 
-	// We only accept an incremental change if that's what the server expects.
-	if s.session.Options().TextDocumentSyncKind == protocol.Full {
-		return nil, errors.Errorf("expected a full content change, received incremental changes for %s", uri)
-	}
-
 	return s.applyIncrementalChanges(ctx, uri, changes)
 }
 

internal/lsp/watched_files.go

@@ -20,9 +20,6 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did
 		ctx := telemetry.File.With(ctx, uri)
 
 		for _, view := range s.session.Views() {
-			if !view.Options().WatchFileChanges {
-				continue
-			}
 			action := toFileAction(change.Type)
 			switch action {
 			case source.Change, source.Create: