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>
diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go
index 44dae99..b20712e 100644
--- a/internal/lsp/cache/cache.go
+++ b/internal/lsp/cache/cache.go
@@ -76,7 +76,7 @@
s := &session{
cache: c,
id: strconv.FormatInt(index, 10),
- options: source.DefaultSessionOptions,
+ options: source.DefaultOptions,
overlays: make(map[span.URI]*overlay),
filesWatchMap: NewWatchMap(),
}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 6052976..ac004c6 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -28,7 +28,7 @@
cache *cache
id string
- options source.SessionOptions
+ options source.Options
viewMu sync.Mutex
views []*view
@@ -56,11 +56,11 @@
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 @@
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()
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 3a2f93f..5168ebd 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -30,7 +30,7 @@
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 @@
return v.folder
}
-func (v *view) Options() source.ViewOptions {
+func (v *view) Options() source.Options {
return v.options
}
diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go
index 3052840..ce04ca1 100644
--- a/internal/lsp/cmd/cmd.go
+++ b/internal/lsp/cmd/cmd.go
@@ -298,8 +298,7 @@
env[l[0]] = l[1]
}
results[i] = map[string]interface{}{
- "env": env,
- "noDocsOnHover": true,
+ "env": env,
}
}
return results, nil
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 7b33063..df2659f 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -51,7 +51,7 @@
}, 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
diff --git a/internal/lsp/general.go b/internal/lsp/general.go
index 69f973f..69f262a 100644
--- a/internal/lsp/general.go
+++ b/internal/lsp/general.go
@@ -7,6 +7,7 @@
import (
"bytes"
"context"
+ "fmt"
"os"
"path"
@@ -33,7 +34,7 @@
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 @@
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 @@
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
}
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 1b4b684..ce03f67 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -61,9 +61,8 @@
}
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)
}
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 120e170..2501a45 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -14,7 +14,8 @@
)
var (
- DefaultSessionOptions = SessionOptions{
+ DefaultOptions = Options{
+ Env: os.Environ(),
TextDocumentSyncKind: protocol.Incremental,
HoverKind: SynopsisDocumentation,
InsertTextFormat: protocol.PlainTextTextFormat,
@@ -32,15 +33,17 @@
Deep: false,
FuzzyMatching: false,
},
- DefaultViewOptions: ViewOptions{
- Env: os.Environ(),
- },
}
)
-type SessionOptions struct {
- Env []string
- BuildFlags []string
+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 @@
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 HoverKind int
-type Options interface {
- set(name string, value interface{}) OptionResult
-}
-
const (
SingleLine = HoverKind(iota)
NoDocumentation
@@ -104,8 +93,10 @@
type OptionResult struct {
Name string
Value interface{}
- State OptionState
Error error
+
+ State OptionState
+ Replacement string
}
type OptionState int
@@ -116,7 +107,7 @@
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 @@
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 @@
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 @@
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 @@
*b = v
}
}
-
-func (r *OptionResult) setNotBool(b *bool) {
- if v, ok := r.asBool(); ok {
- *b = !v
- }
-}
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 9734b79..36de197 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -49,10 +49,9 @@
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,
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 0480cc2..a9c92b3 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -155,7 +155,7 @@
// 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 @@
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 @@
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.
diff --git a/internal/lsp/workspace.go b/internal/lsp/workspace.go
index 27bdc16..42a252a 100644
--- a/internal/lsp/workspace.go
+++ b/internal/lsp/workspace.go
@@ -38,8 +38,8 @@
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
}
