internal/lsp: merge session and view options into one

This fixes the issue of config options not being applied.
Also, handle config errors and deprecation by showing a message to the
user.

Change-Id: I850d5303a7a1e301c97324060a87929710ee6700
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194682
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/cache.go

@@ -76,7 +76,7 @@ func (c *cache) NewSession(ctx context.Context) source.Session {
 	s := &session{
 		cache:         c,
 		id:            strconv.FormatInt(index, 10),
-		options:       source.DefaultSessionOptions,
+		options:       source.DefaultOptions,
 		overlays:      make(map[span.URI]*overlay),
 		filesWatchMap: NewWatchMap(),
 	}

internal/lsp/cache/session.go

@@ -28,7 +28,7 @@ type session struct {
 	cache *cache
 	id    string
 
-	options source.SessionOptions
+	options source.Options
 
 	viewMu  sync.Mutex
 	views   []*view
@@ -56,11 +56,11 @@ type overlay struct {
 	unchanged bool
 }
 
-func (s *session) Options() source.SessionOptions {
+func (s *session) Options() source.Options {
 	return s.options
 }
 
-func (s *session) SetOptions(options source.SessionOptions) {
+func (s *session) SetOptions(options source.Options) {
 	s.options = options
 }
 
@@ -79,7 +79,7 @@ func (s *session) Cache() source.Cache {
 	return s.cache
 }
 
-func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.ViewOptions) source.View {
+func (s *session) NewView(ctx context.Context, name string, folder span.URI, options source.Options) source.View {
 	index := atomic.AddInt64(&viewIndex, 1)
 	s.viewMu.Lock()
 	defer s.viewMu.Unlock()

internal/lsp/cache/view.go

@@ -30,7 +30,7 @@ type view struct {
 	session *session
 	id      string
 
-	options source.ViewOptions
+	options source.Options
 
 	// mu protects all mutable state of the view.
 	mu sync.Mutex
@@ -113,7 +113,7 @@ func (v *view) Folder() span.URI {
 	return v.folder
 }
 
-func (v *view) Options() source.ViewOptions {
+func (v *view) Options() source.Options {
 	return v.options
 }
 

internal/lsp/cmd/cmd.go

@@ -299,7 +299,6 @@ func (c *cmdClient) Configuration(ctx context.Context, p *protocol.ParamConfig)
 		}
 		results[i] = map[string]interface{}{
 			"env": env,
-			"noDocsOnHover": true,
 		}
 	}
 	return results, nil

internal/lsp/completion.go

@@ -51,7 +51,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	}, nil
 }
 
-func (s *Server) toProtocolCompletionItems(candidates []source.CompletionItem, rng protocol.Range, options source.SessionOptions) []protocol.CompletionItem {
+func (s *Server) toProtocolCompletionItems(candidates []source.CompletionItem, rng protocol.Range, options source.Options) []protocol.CompletionItem {
 	var (
 		items                  = make([]protocol.CompletionItem, 0, len(candidates))
 		numDeepCompletionsSeen int

internal/lsp/general.go

@@ -7,6 +7,7 @@ package lsp
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path"
 
@@ -33,7 +34,7 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitia) (
 	options := s.session.Options()
 	defer func() { s.session.SetOptions(options) }()
 
-	//TODO: handle the options results
+	// TODO: Handle results here.
 	source.SetOptions(&options, params.InitializationOptions)
 	options.ForClientCapabilities(params.Capabilities)
 
@@ -172,7 +173,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa
 	return nil
 }
 
-func (s *Server) fetchConfig(ctx context.Context, name string, folder span.URI, vo *source.ViewOptions) error {
+func (s *Server) fetchConfig(ctx context.Context, name string, folder span.URI, o *source.Options) error {
 	if !s.session.Options().ConfigurationSupported {
 		return nil
 	}
@@ -193,8 +194,31 @@ func (s *Server) fetchConfig(ctx context.Context, name string, folder span.URI,
 		return err
 	}
 	for _, config := range configs {
-		//TODO: handle the options results
-		source.SetOptions(vo, config)
+		results := source.SetOptions(o, config)
+		for _, result := range results {
+			if result.Error != nil {
+				s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+					Type:    protocol.Error,
+					Message: result.Error.Error(),
+				})
+			}
+			switch result.State {
+			case source.OptionUnexpected:
+				s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+					Type:    protocol.Error,
+					Message: fmt.Sprintf("unexpected config %s", result.Name),
+				})
+			case source.OptionDeprecated:
+				msg := fmt.Sprintf("config %s is deprecated", result.Name)
+				if result.Replacement != "" {
+					msg = fmt.Sprintf("%s, use %s instead", msg, result.Replacement)
+				}
+				s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+					Type:    protocol.Warning,
+					Message: msg,
+				})
+			}
+		}
 	}
 	return nil
 }

internal/lsp/lsp_test.go

@@ -61,9 +61,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	}
 	options.HoverKind = source.SynopsisDocumentation
 	session.SetOptions(options)
-	vo := options.DefaultViewOptions
-	vo.Env = data.Config.Env
-	session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), vo)
+	options.Env = data.Config.Env
+	session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), options)
 	for filename, content := range data.Config.Overlay {
 		session.SetOverlay(span.FileURI(filename), content)
 	}

internal/lsp/source/options.go

@@ -14,7 +14,8 @@ import (
 )
 
 var (
-	DefaultSessionOptions = SessionOptions{
+	DefaultOptions = Options{
+		Env:                    os.Environ(),
 		TextDocumentSyncKind:   protocol.Incremental,
 		HoverKind:              SynopsisDocumentation,
 		InsertTextFormat:       protocol.PlainTextTextFormat,
@@ -32,15 +33,17 @@ var (
 			Deep:          false,
 			FuzzyMatching: false,
 		},
-		DefaultViewOptions: ViewOptions{
-			Env: os.Environ(),
-		},
 	}
 )
 
-type SessionOptions struct {
+type Options struct {
+
+	// Env is the current set of environment overrides on this view.
 	Env []string
+
+	// BuildFlags is used to adjust the build flags applied to the view.
 	BuildFlags []string
+
 	HoverKind        HoverKind
 	DisabledAnalyses map[string]struct{}
 
@@ -58,16 +61,6 @@ type SessionOptions struct {
 	TextDocumentSyncKind protocol.TextDocumentSyncKind
 
 	Completion CompletionOptions
-
-	DefaultViewOptions ViewOptions
-}
-
-type ViewOptions struct {
-	// Env is the current set of environment overrides on this view.
-	Env []string
-
-	// BuildFlags is used to adjust the build flags applied to the view.
-	BuildFlags []string
 }
 
 type CompletionOptions struct {
@@ -81,10 +74,6 @@ type CompletionOptions struct {
 
 type HoverKind int
 
-type Options interface {
-	set(name string, value interface{}) OptionResult
-}
-
 const (
 	SingleLine = HoverKind(iota)
 	NoDocumentation
@@ -104,8 +93,10 @@ type OptionResults []OptionResult
 type OptionResult struct {
 	Name  string
 	Value interface{}
-	State OptionState
 	Error error
+
+	State       OptionState
+	Replacement string
 }
 
 type OptionState int
@@ -116,7 +107,7 @@ const (
 	OptionUnexpected
 )
 
-func SetOptions(options Options, opts interface{}) OptionResults {
+func SetOptions(options *Options, opts interface{}) OptionResults {
 	var results OptionResults
 	switch opts := opts.(type) {
 	case nil:
@@ -133,7 +124,7 @@ func SetOptions(options Options, opts interface{}) OptionResults {
 	return results
 }
 
-func (o *SessionOptions) ForClientCapabilities(caps protocol.ClientCapabilities) {
+func (o *Options) ForClientCapabilities(caps protocol.ClientCapabilities) {
 	// Check if the client supports snippets in completion items.
 	if caps.TextDocument.Completion.CompletionItem != nil &&
 		caps.TextDocument.Completion.CompletionItem.SnippetSupport {
@@ -152,24 +143,46 @@ func (o *SessionOptions) ForClientCapabilities(caps protocol.ClientCapabilities)
 	o.LineFoldingOnly = caps.TextDocument.FoldingRange.LineFoldingOnly
 }
 
-func (o *SessionOptions) set(name string, value interface{}) OptionResult {
+func (o *Options) set(name string, value interface{}) OptionResult {
 	result := OptionResult{Name: name, Value: value}
 	switch name {
+	case "env":
+		menv, ok := value.(map[string]interface{})
+		if !ok {
+			result.errorf("invalid config gopls.env type %T", value)
+			break
+		}
+		for k, v := range menv {
+			o.Env = append(o.Env, fmt.Sprintf("%s=%s", k, v))
+		}
+
+	case "buildFlags":
+		iflags, ok := value.([]interface{})
+		if !ok {
+			result.errorf("invalid config gopls.buildFlags type %T", value)
+			break
+		}
+		flags := make([]string, 0, len(iflags))
+		for _, flag := range iflags {
+			flags = append(flags, fmt.Sprintf("%s", flag))
+		}
+		o.BuildFlags = flags
+
 	case "noIncrementalSync":
 		if v, ok := result.asBool(); ok && v {
 			o.TextDocumentSyncKind = protocol.Full
 		}
 	case "watchFileChanges":
 		result.setBool(&o.WatchFileChanges)
-	case "wantCompletionDocumentation":
+	case "completionDocumentation":
 		result.setBool(&o.Completion.Documentation)
 	case "usePlaceholders":
 		result.setBool(&o.Completion.Placeholders)
-	case "disableDeepCompletion":
-		result.setNotBool(&o.Completion.Deep)
-	case "disableFuzzyMatching":
-		result.setNotBool(&o.Completion.FuzzyMatching)
-	case "wantUnimportedCompletions":
+	case "deepCompletion":
+		result.setBool(&o.Completion.Deep)
+	case "fuzzyMatching":
+		result.setBool(&o.Completion.FuzzyMatching)
+	case "completeUnimported":
 		result.setBool(&o.Completion.Unimported)
 
 	case "hoverKind":
@@ -204,39 +217,25 @@ func (o *SessionOptions) set(name string, value interface{}) OptionResult {
 			o.DisabledAnalyses[fmt.Sprint(a)] = struct{}{}
 		}
 
+	// Deprecated settings.
 	case "wantSuggestedFixes":
 		result.State = OptionDeprecated
 
-	default:
-		return o.DefaultViewOptions.set(name, value)
-	}
-	return result
-}
+	case "disableDeepCompletion":
+		result.State = OptionDeprecated
+		result.Replacement = "deepCompletion"
 
-func (o *ViewOptions) set(name string, value interface{}) OptionResult {
-	result := OptionResult{Name: name, Value: value}
-	switch name {
-	case "env":
-		menv, ok := value.(map[string]interface{})
-		if !ok {
-			result.errorf("invalid config gopls.env type %T", value)
-			break
-		}
-		for k, v := range menv {
-			o.Env = append(o.Env, fmt.Sprintf("%s=%s", k, v))
-		}
+	case "disableFuzzyMatching":
+		result.State = OptionDeprecated
+		result.Replacement = "fuzzyMatching"
 
-	case "buildFlags":
-		iflags, ok := value.([]interface{})
-		if !ok {
-			result.errorf("invalid config gopls.buildFlags type %T", value)
-			break
-		}
-		flags := make([]string, 0, len(iflags))
-		for _, flag := range iflags {
-			flags = append(flags, fmt.Sprintf("%s", flag))
-		}
-		o.BuildFlags = flags
+	case "wantCompletionDocumentation":
+		result.State = OptionDeprecated
+		result.Replacement = "completionDocumentation"
+
+	case "wantUnimportedCompletions":
+		result.State = OptionDeprecated
+		result.Replacement = "completeUnimported"
 
 	default:
 		result.State = OptionUnexpected
@@ -262,9 +261,3 @@ func (r *OptionResult) setBool(b *bool) {
 		*b = v
 	}
 }
-
-func (r *OptionResult) setNotBool(b *bool) {
-	if v, ok := r.asBool(); ok {
-		*b = !v
-	}
-}

internal/lsp/source/source_test.go

@@ -49,10 +49,9 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
 	cache := cache.New()
 	session := cache.NewSession(ctx)
 	options := session.Options()
-	vo := options.DefaultViewOptions
-	vo.Env = data.Config.Env
+	options.Env = data.Config.Env
 	r := &runner{
-		view: session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), vo),
+		view: session.NewView(ctx, "source_test", span.FileURI(data.Config.Dir), options),
 		data: data,
 		ctx:  ctx,
 	}

internal/lsp/source/view.go

@@ -155,7 +155,7 @@ type Cache interface {
 // A session may have many active views at any given time.
 type Session interface {
 	// NewView creates a new View and returns it.
-	NewView(ctx context.Context, name string, folder span.URI, options ViewOptions) View
+	NewView(ctx context.Context, name string, folder span.URI, options Options) View
 
 	// Cache returns the cache that created this session.
 	Cache() Cache
@@ -196,10 +196,10 @@ type Session interface {
 	DidChangeOutOfBand(ctx context.Context, f GoFile, change protocol.FileChangeType)
 
 	// Options returns a copy of the SessionOptions for this session.
-	Options() SessionOptions
+	Options() Options
 
 	// SetOptions sets the options of this session to new values.
-	SetOptions(SessionOptions)
+	SetOptions(Options)
 }
 
 // View represents a single workspace.
@@ -246,7 +246,7 @@ type View interface {
 	RunProcessEnvFunc(ctx context.Context, fn func(*imports.Options) error, opts *imports.Options) error
 
 	// Options returns a copy of the ViewOptions for this view.
-	Options() ViewOptions
+	Options() Options
 }
 
 // File represents a source file of any type.

internal/lsp/workspace.go

@@ -38,8 +38,8 @@ func (s *Server) addView(ctx context.Context, name string, uri span.URI) error {
 		return errors.Errorf("addView called before server initialized")
 	}
 
-	viewOptions := s.session.Options().DefaultViewOptions
-	s.fetchConfig(ctx, name, uri, &viewOptions)
-	s.session.NewView(ctx, name, uri, viewOptions)
+	options := s.session.Options()
+	s.fetchConfig(ctx, name, uri, &options)
+	s.session.NewView(ctx, name, uri, options)
 	return nil
 }