internal/lsp: determine diagnostics to show per-file, not per-package

Previously, if any one file in a package had a parse error, we would not
send type errors for any of the other files. Now we make this decision
per-file, so that, even if a file has parse errors, type errors in the
other files will be shown.

Fixes golang/go#32596

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

internal/lsp/source/diagnostics.go

@@ -67,7 +67,7 @@ func Diagnostics(ctx context.Context, v View, f GoFile, disabledAnalyses map[str
 		if err.Kind != packages.ListError {
 			continue
 		}
-		addReport(v, reports, listErrorSpan(err).URI(), nil)
+		addReport(v, reports, packagesErrorSpan(err).URI(), nil)
 	}
 
 	// Run diagnostics for the package that this URI belongs to.
@@ -91,42 +91,55 @@ func Diagnostics(ctx context.Context, v View, f GoFile, disabledAnalyses map[str
 	return reports, nil
 }
 
+type diagnosticSet struct {
+	listErrors, parseErrors, typeErrors []Diagnostic
+}
+
 func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI][]Diagnostic) bool {
-	var listErrors, parseErrors, typeErrors []packages.Error
+	diagSets := make(map[span.URI]*diagnosticSet)
 	for _, err := range pkg.GetErrors() {
-		switch err.Kind {
+		diag := Diagnostic{
-		case packages.ParseError:
+			Span:     packagesErrorSpan(err),
-			parseErrors = append(parseErrors, err)
+			Message:  err.Msg,
-		case packages.TypeError:
-			typeErrors = append(typeErrors, err)
-		default:
-			listErrors = append(listErrors, err)
-		}
-	}
-	// Don't report type errors if there are parse errors or list errors.
-	diags := typeErrors
-	if len(parseErrors) > 0 {
-		diags = parseErrors
-	} else if len(listErrors) > 0 {
-		diags = listErrors
-	}
-	for _, diag := range diags {
-		spn := listErrorSpan(diag)
-		if spn.IsPoint() && diag.Kind == packages.TypeError {
-			spn = pointToSpan(ctx, v, spn)
-		}
-		diagnostic := Diagnostic{
 			Source:   "LSP",
-			Span:     spn,
-			Message:  diag.Msg,
 			Severity: SeverityError,
 		}
-		if _, ok := reports[spn.URI()]; ok {
+		set, ok := diagSets[diag.Span.URI()]
-			reports[spn.URI()] = append(reports[spn.URI()], diagnostic)
+		if !ok {
+			set = &diagnosticSet{}
+			diagSets[diag.Span.URI()] = set
+		}
+		switch err.Kind {
+		case packages.ParseError:
+			set.parseErrors = append(set.parseErrors, diag)
+		case packages.TypeError:
+			if diag.Span.IsPoint() {
+				diag.Span = pointToSpan(ctx, v, diag.Span)
+			}
+			set.typeErrors = append(set.typeErrors, diag)
+		default:
+			set.listErrors = append(set.listErrors, diag)
 		}
 	}
-	// Returns true if we've sent non-empty diagnostics.
+	var nonEmptyDiagnostics bool // track if we actually send non-empty diagnostics
-	return len(diags) != 0
+	for uri, set := range diagSets {
+		// Don't report type errors if there are parse errors or list errors.
+		diags := set.typeErrors
+		if len(set.parseErrors) > 0 {
+			diags = set.parseErrors
+		} else if len(set.listErrors) > 0 {
+			diags = set.listErrors
+		}
+		if len(diags) > 0 {
+			nonEmptyDiagnostics = true
+		}
+		for _, diag := range diags {
+			if _, ok := reports[uri]; ok {
+				reports[uri] = append(reports[uri], diag)
+			}
+		}
+	}
+	return nonEmptyDiagnostics
 }
 
 func analyses(ctx context.Context, v View, pkg Package, disabledAnalyses map[string]struct{}, reports map[span.URI][]Diagnostic) error {
@@ -166,30 +179,31 @@ func addReport(v View, reports map[span.URI][]Diagnostic, uri span.URI, diagnost
 	}
 }
 
-// parseDiagnosticMessage attempts to parse a standard error message by stripping off the trailing error message.
+func packagesErrorSpan(err packages.Error) span.Span {
-// Works only on errors where the message is prefixed by ": ".
+	if err.Pos == "" {
-// e.g.:
+		return parseDiagnosticMessage(err.Msg)
+	}
+	return span.Parse(err.Pos)
+}
+
+// parseDiagnosticMessage attempts to parse a standard `go list` error message
+// by stripping off the trailing error message.
+//
+// It works only on errors whose message is prefixed by colon,
+// followed by a space (": "). For example:
+//
 //   attributes.go:13:1: expected 'package', found 'type'
+//
 func parseDiagnosticMessage(input string) span.Span {
 	input = strings.TrimSpace(input)
-
 	msgIndex := strings.Index(input, ": ")
 	if msgIndex < 0 {
 		return span.Parse(input)
 	}
-
 	return span.Parse(input[:msgIndex])
 }
 
-func listErrorSpan(pkgErr packages.Error) span.Span {
-	if pkgErr.Pos == "" {
-		return parseDiagnosticMessage(pkgErr.Msg)
-	}
-	return span.Parse(pkgErr.Pos)
-}
-
 func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span {
-	// Don't set a range if it's anything other than a type error.
 	f, err := v.GetFile(ctx, spn.URI())
 	if err != nil {
 		v.Session().Logger().Errorf(ctx, "Could find file for diagnostic: %v", spn.URI())