internal/lsp: turn on completion documentation by default

This feature has been in an experimental state for a long enough time
that I think we can enable it by default at master.

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

internal/lsp/completion.go

@@ -26,7 +26,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara
 	candidates, surrounding, err := source.Completion(ctx, view, f, params.Position, source.CompletionOptions{
 		WantDeepCompletion:    !s.disableDeepCompletion,
 		WantFuzzyMatching:     !s.disableFuzzyMatching,
-		WantDocumentaton:      s.wantCompletionDocumentation,
+		NoDocumentation:       !s.wantCompletionDocumentation,
 		WantFullDocumentation: s.hoverKind == fullDocumentation,
 		WantUnimported:        s.wantUnimportedCompletions,
 	})

internal/lsp/general.go

@@ -273,9 +273,12 @@ func (s *Server) processConfig(ctx context.Context, view source.View, config int
 	}
 
 	// Check if the user wants documentation in completion items.
+	// This defaults to true.
+	s.wantCompletionDocumentation = true
 	if wantCompletionDocumentation, ok := c["wantCompletionDocumentation"].(bool); ok {
 		s.wantCompletionDocumentation = wantCompletionDocumentation
 	}
+
 	// Check if placeholders are enabled.
 	if usePlaceholders, ok := c["usePlaceholders"].(bool); ok {
 		s.usePlaceholders = usePlaceholders

internal/lsp/lsp_test.go

@@ -110,9 +110,9 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 		r.server.disableDeepCompletion = true
 		r.server.disableFuzzyMatching = true
 		r.server.wantUnimportedCompletions = false
-		r.server.wantCompletionDocumentation = false
 	}()
 
+	// Set this as a default.
 	r.server.wantCompletionDocumentation = true
 
 	for src, itemList := range data {

internal/lsp/source/completion.go

@@ -380,11 +380,13 @@ type candidate struct {
 }
 
 type CompletionOptions struct {
-	WantDeepCompletion    bool
+	WantDeepCompletion bool
-	WantDocumentaton      bool
+	WantFuzzyMatching  bool
+
+	WantUnimported bool
+
+	NoDocumentation       bool
 	WantFullDocumentation bool
-	WantUnimported        bool
-	WantFuzzyMatching     bool
 }
 
 // Completion returns a list of possible candidates for completion, given a

internal/lsp/source/completion_format.go

@@ -113,56 +113,56 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
 		plainSnippet:        plainSnippet,
 		placeholderSnippet:  placeholderSnippet,
 	}
-	// TODO(rstambler): Log errors when this feature is enabled.
+	// If the user doesn't want documentation for completion items.
-	if c.opts.WantDocumentaton {
+	if c.opts.NoDocumentation {
-		declRange, err := objToRange(c.ctx, c.view, obj)
+		return item, nil
-		if err != nil {
+	}
-			goto Return
+	declRange, err := objToRange(c.ctx, c.view, obj)
-		}
+	if err != nil {
-		pos := c.view.Session().Cache().FileSet().Position(declRange.spanRange.Start)
+		return item, nil
-		if !pos.IsValid() {
+	}
-			goto Return
+	pos := c.view.Session().Cache().FileSet().Position(declRange.spanRange.Start)
-		}
+	if !pos.IsValid() {
-		uri := span.FileURI(pos.Filename)
+		return item, nil
-		f, err := c.view.GetFile(c.ctx, uri)
+	}
-		if err != nil {
+	uri := span.FileURI(pos.Filename)
-			goto Return
+	f, err := c.view.GetFile(c.ctx, uri)
-		}
+	if err != nil {
-		gof, ok := f.(GoFile)
+		return item, nil
-		if !ok {
+	}
-			goto Return
+	gof, ok := f.(GoFile)
-		}
+	if !ok {
-		pkg, err := gof.GetCachedPackage(c.ctx)
+		return item, nil
-		if err != nil {
+	}
-			goto Return
+	pkg, err := gof.GetCachedPackage(c.ctx)
-		}
+	if err != nil {
-		var ph ParseGoHandle
+		return item, nil
-		for _, h := range pkg.GetHandles() {
+	}
-			if h.File().Identity().URI == gof.URI() {
+	var ph ParseGoHandle
-				ph = h
+	for _, h := range pkg.GetHandles() {
-			}
+		if h.File().Identity().URI == gof.URI() {
-		}
+			ph = h
-		if ph == nil {
-			goto Return
-		}
-		file, _ := ph.Cached(c.ctx)
-		if file == nil {
-			goto Return
-		}
-		ident, err := findIdentifier(c.ctx, c.view, gof, pkg, file, declRange.spanRange.Start)
-		if err != nil {
-			goto Return
-		}
-		hover, err := ident.Hover(c.ctx)
-		if err != nil {
-			goto Return
-		}
-		item.Documentation = hover.Synopsis
-		if c.opts.WantFullDocumentation {
-			item.Documentation = hover.FullDocumentation
 		}
 	}
-Return:
+	if ph == nil {
+		return item, nil
+	}
+	file, _ := ph.Cached(c.ctx)
+	if file == nil {
+		return item, nil
+	}
+	ident, err := findIdentifier(c.ctx, c.view, gof, pkg, file, declRange.spanRange.Start)
+	if err != nil {
+		return item, nil
+	}
+	hover, err := ident.Hover(c.ctx)
+	if err != nil {
+		return item, nil
+	}
+	item.Documentation = hover.Synopsis
+	if c.opts.WantFullDocumentation {
+		item.Documentation = hover.FullDocumentation
+	}
 	return item, nil
 }
 

internal/lsp/source/source_test.go

@@ -104,7 +104,6 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 		}, source.CompletionOptions{
 			WantDeepCompletion: deepComplete,
 			WantFuzzyMatching:  fuzzyMatch,
-			WantDocumentaton:   true,
 			WantUnimported:     unimported,
 		})
 		if err != nil {