From 178e83bc9d6a1190667fd60e42deb7c461adabf2 Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Sat, 1 Jun 2019 00:08:57 -0400
Subject: [PATCH] internal/lsp: build the builtin package preemptively

This change fixes a regression introduced by the building the builtin
package on demand. Although this change increases the startup tasks of
gopls, it is necessary to ensure that we ignore diagnostics from
builtin.go.

Change-Id: I897e747a273056d70cecba486a74c75a736d8f80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179921
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
---
 internal/lsp/cache/session.go      |  4 ++++
 internal/lsp/cache/view.go         |  8 +++-----
 internal/lsp/source/diagnostics.go | 23 +++++++++++++++--------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 920eac24..1b4881bb 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -78,6 +78,10 @@ func (s *session) NewView(name string, folder span.URI) source.View {
 		},
 		ignoredURIs: make(map[span.URI]struct{}),
 	}
+	// Preemptively build the builtin package,
+	// so we immediately add builtin.go to the list of ignored files.
+	v.buildBuiltinPkg()
+
 	s.views = append(s.views, v)
 	// we always need to drop the view map
 	s.viewMap = make(map[span.URI]source.View)
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index b2bb3e61..2aef2237 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -177,15 +177,13 @@ func (v *view) BackgroundContext() context.Context {
 }
 
 func (v *view) BuiltinPackage() *ast.Package {
-	if v.builtinPkg == nil {
-		v.buildBuiltinPkg()
-	}
 	return v.builtinPkg
 }
 
+// buildBuiltinPkg builds the view's builtin package.
+// It assumes that the view is not active yet,
+// i.e. it has not been added to the session's list of views.
 func (v *view) buildBuiltinPkg() {
-	v.mu.Lock()
-	defer v.mu.Unlock()
 	cfg := *v.buildConfig()
 	pkgs, _ := packages.Load(&cfg, "builtin")
 	if len(pkgs) != 1 {
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index bd0cd2d7..01056019 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -59,16 +59,12 @@ func Diagnostics(ctx context.Context, v View, f GoFile) (map[span.URI][]Diagnost
 	// Prepare the reports we will send for the files in this package.
 	reports := make(map[span.URI][]Diagnostic)
 	for _, filename := range pkg.GetFilenames() {
-		uri := span.FileURI(filename)
-		if v.Ignore(uri) {
-			continue
-		}
-		reports[uri] = []Diagnostic{}
+		addReport(v, reports, span.FileURI(filename), nil)
 	}
 
 	// Prepare any additional reports for the errors in this package.
 	for _, pkgErr := range pkg.GetErrors() {
-		reports[packageErrorSpan(pkgErr).URI()] = []Diagnostic{}
+		addReport(v, reports, packageErrorSpan(pkgErr).URI(), nil)
 	}
 
 	// Run diagnostics for the package that this URI belongs to.
@@ -85,7 +81,7 @@ func Diagnostics(ctx context.Context, v View, f GoFile) (map[span.URI][]Diagnost
 			continue
 		}
 		for _, filename := range pkg.GetFilenames() {
-			reports[span.FileURI(filename)] = []Diagnostic{}
+			addReport(v, reports, span.FileURI(filename), nil)
 		}
 		diagnostics(ctx, v, pkg, reports)
 	}
@@ -143,7 +139,7 @@ func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]D
 		if diag.Category != "" {
 			category += "." + category
 		}
-		reports[s.URI()] = append(reports[s.URI()], Diagnostic{
+		addReport(v, reports, s.URI(), &Diagnostic{
 			Source:   category,
 			Span:     s,
 			Message:  diag.Message,
@@ -156,6 +152,17 @@ func analyses(ctx context.Context, v View, pkg Package, reports map[span.URI][]D
 	return nil
 }
 
+func addReport(v View, reports map[span.URI][]Diagnostic, uri span.URI, diagnostic *Diagnostic) {
+	if v.Ignore(uri) {
+		return
+	}
+	if diagnostic == nil {
+		reports[uri] = []Diagnostic{}
+	} else {
+		reports[uri] = append(reports[uri], *diagnostic)
+	}
+}
+
 // parseDiagnosticMessage attempts to parse a standard error message by stripping off the trailing error message.
 // Works only on errors where the message is prefixed by ": ".
 // e.g.: