gopls/internal/cmd: check: print RelatedInformation

The check command previously ignored Diagnostic.RelatedInformation,
which confused me as I was trying to run an experiment related
to golang/go#64547. This change causes it print the related errors.

+ test

(Aside: there's a lot of weirdness in the way that
check.typeErrorsToDiagnostics constructs the response,
as you will see if you run the test on c.go not c2.go,
but that's not the test's concern.)

Change-Id: I14efbf8f2e47ac00c2e7d6eceeacbaaecab88213
Reviewed-on: https://go-review.googlesource.com/c/tools/+/576656
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/cache/check.go

@@ -1852,6 +1852,7 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, errs []types.Error, linkTarget
 	var result []*Diagnostic
 
 	// batch records diagnostics for a set of related types.Errors.
+	// (related[0] is the primary error.)
 	batch := func(related []types.Error) {
 		var diags []*Diagnostic
 		for i, e := range related {
@@ -1935,7 +1936,7 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, errs []types.Error, linkTarget
 				bug.Reportf("internal error: could not compute pos to range for %v: %v", e, err)
 				continue
 			}
-			msg := related[0].Msg
+			msg := related[0].Msg // primary
 			if i > 0 {
 				if supportsRelatedInformation {
 					msg += " (see details)"

gopls/internal/cmd/check.go

@@ -10,6 +10,8 @@ import (
 	"fmt"
 
 	"golang.org/x/tools/gopls/internal/protocol"
+	"golang.org/x/tools/gopls/internal/settings"
+	"golang.org/x/tools/gopls/internal/util/slices"
 )
 
 // check implements the check verb for gopls.
@@ -34,17 +36,30 @@ Example: show the diagnostic results of this file:
 // results to stdout.
 func (c *check) Run(ctx context.Context, args ...string) error {
 	if len(args) == 0 {
-		// no files, so no results
 		return nil
 	}
-	checking := map[protocol.DocumentURI]*cmdFile{}
-	var uris []protocol.DocumentURI
-	// now we ready to kick things off
+
+	// TODO(adonovan): formally, we are required to set this
+	// option if we want RelatedInformation, but it appears to
+	// have no effect on the server, even though the default is
+	// false. Investigate.
+	origOptions := c.app.options
+	c.app.options = func(opts *settings.Options) {
+		origOptions(opts)
+		opts.RelatedInformationSupported = true
+	}
+
 	conn, err := c.app.connect(ctx, nil)
 	if err != nil {
 		return err
 	}
 	defer conn.terminate(ctx)
+
+	// Open and diagnose the requested files.
+	var (
+		uris     []protocol.DocumentURI
+		checking = make(map[protocol.DocumentURI]*cmdFile)
+	)
 	for _, arg := range args {
 		uri := protocol.URIFromPath(arg)
 		uris = append(uris, uri)
@@ -57,16 +72,36 @@ func (c *check) Run(ctx context.Context, args ...string) error {
 	if err := conn.diagnoseFiles(ctx, uris); err != nil {
 		return err
 	}
-	conn.client.filesMu.Lock()
-	defer conn.client.filesMu.Unlock()
+
+	// print prints a single element of a diagnostic.
+	print := func(uri protocol.DocumentURI, rng protocol.Range, message string) error {
+		file, err := conn.openFile(ctx, uri)
+		if err != nil {
+			return err
+		}
+		spn, err := file.rangeSpan(rng)
+		if err != nil {
+			return fmt.Errorf("could not convert position %v for %q", rng, message)
+		}
+		fmt.Printf("%v: %v\n", spn, message)
+		return nil
+	}
 
 	for _, file := range checking {
-		for _, d := range file.diagnostics {
-			spn, err := file.rangeSpan(d.Range)
-			if err != nil {
-				return fmt.Errorf("Could not convert position %v for %q", d.Range, d.Message)
+		file.diagnosticsMu.Lock()
+		diags := slices.Clone(file.diagnostics)
+		file.diagnosticsMu.Unlock()
+
+		for _, diag := range diags {
+			if err := print(file.uri, diag.Range, diag.Message); err != nil {
+				return err
 			}
-			fmt.Printf("%v: %v\n", spn, d.Message)
+			for _, rel := range diag.RelatedInformation {
+				if err := print(rel.Location.URI, rel.Location.Range, "- "+rel.Message); err != nil {
+					return err
+				}
+			}
+
 		}
 	}
 	return nil

gopls/internal/cmd/cmd.go

@@ -399,15 +399,16 @@ type cmdClient struct {
 	app        *Application
 	onProgress func(*protocol.ProgressParams)
 
-	filesMu sync.Mutex // guards files map and each cmdFile.diagnostics
+	filesMu sync.Mutex // guards files map
 	files   map[protocol.DocumentURI]*cmdFile
 }
 
 type cmdFile struct {
-	uri         protocol.DocumentURI
-	mapper      *protocol.Mapper
-	err         error
-	diagnostics []protocol.Diagnostic
+	uri           protocol.DocumentURI
+	mapper        *protocol.Mapper
+	err           error
+	diagnosticsMu sync.Mutex
+	diagnostics   []protocol.Diagnostic
 }
 
 func newClient(app *Application, onProgress func(*protocol.ProgressParams)) *cmdClient {
@@ -595,9 +596,11 @@ func (c *cmdClient) PublishDiagnostics(ctx context.Context, p *protocol.PublishD
 	}
 
 	c.filesMu.Lock()
-	defer c.filesMu.Unlock()
-
 	file := c.getFile(p.URI)
+	c.filesMu.Unlock()
+
+	file.diagnosticsMu.Lock()
+	defer file.diagnosticsMu.Unlock()
 	file.diagnostics = append(file.diagnostics, p.Diagnostics...)
 
 	// Perform a crude in-place deduplication.

gopls/internal/cmd/integration_test.go

@@ -103,6 +103,12 @@ var _ = fmt.Sprintf("%s", 123)
 package a
 import "fmt"
 var _ = fmt.Sprintf("%d", "123")
+-- c/c.go --
+package c
+var C int
+-- c/c2.go --
+package c
+var C int
 `)
 
 	// no files
@@ -128,6 +134,14 @@ var _ = fmt.Sprintf("%d", "123")
 		res.checkStdout(`a.go:.* fmt.Sprintf format %s has arg 123 of wrong type int`)
 		res.checkStdout(`b.go:.* fmt.Sprintf format %d has arg "123" of wrong type string`)
 	}
+
+	// diagnostic with related information spanning files
+	{
+		res := gopls(t, tree, "check", "./c/c2.go")
+		res.checkExit(true)
+		res.checkStdout(`c2.go:2:5-6: C redeclared in this block`)
+		res.checkStdout(`c.go:2:5-6: - other declaration of C`)
+	}
 }
 
 // TestCallHierarchy tests the 'call_hierarchy' subcommand (../call_hierarchy.go).