Skip to content

Commit

Permalink
gopls: add initial support for pull diagnostics
Browse files Browse the repository at this point in the history
Implement the scaffolding for pull diagnostics. For now, these are only
supported for Go files, only return parse/type errors for the narrowest
package in the default view, do not report related diagnostics, and do
not run analysis. All of these limitations can be fixed, but this
implementation should be sufficient for some end-to-end testing.

Since the implementation is incomplete, guard the server capability
behind a new internal "pullDiagnostics" setting.

Wire in pull diagnostics to the marker tests: if the server supports it
("pullDiagnostics": true), use pull diagnostics rather than awaiting to
collect the marker test diagnostics.

For golang/go#53275

Change-Id: If6d1c0838d69e43f187863adeca6a3bd5d9bb45d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/616835
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
  • Loading branch information
findleyr committed Oct 7, 2024
1 parent c19060b commit f21a1dc
Show file tree
Hide file tree
Showing 18 changed files with 339 additions and 115 deletions.
16 changes: 10 additions & 6 deletions gopls/doc/features/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ common mistakes.
Diagnostics come from two main sources: compilation errors and analysis findings.

- **Compilation errors** are those that you would obtain from running `go
build`. Gopls doesn't actually run the compiler; that would be too
build`. Gopls doesn't actually run the compiler; that would be too
slow. Instead it runs `go list` (when needed) to compute the
metadata of the compilation, then processes those packages in a similar
manner to the compiler front-end: reading, scanning, and parsing the
Expand Down Expand Up @@ -51,7 +51,7 @@ Diagnostics come from two main sources: compilation errors and analysis findings

## Recomputation of diagnostics

Diagnostics are automatically recomputed each time the source files
By default, diagnostics are automatically recomputed each time the source files
are edited.

Compilation errors in open files are updated after a very short delay
Expand All @@ -68,9 +68,12 @@ Alternatively, diagnostics may be triggered only after an edited file
is saved, using the
[`diagnosticsTrigger`](../settings.md#diagnosticsTrigger) setting.

Gopls does not currently support "pull-based" diagnostics, which are
computed synchronously when requested by the client; see golang/go#53275.

When initialized with `"pullDiagnostics": true`, gopls also supports
["pull diagnostics"](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics),
an alternative mechanism for recomputing diagnostics in which the client
requests diagnostics from gopls explicitly using the `textDocument/diagnostic`
request. This feature is off by default until the performance of pull
diagnostics is comparable to push diagnostics.

## Quick fixes

Expand All @@ -91,6 +94,7 @@ Suggested fixes that are indisputably safe are [code
actions](transformation.md#code-actions) whose kind is
`"source.fixAll"`.
Many client editors have a shortcut to apply all such fixes.

<!-- Note: each Code Action has exactly one kind, so a server
must offer each "safe" action twice, once with its usual kind
and once with kind "source.fixAll".
Expand All @@ -111,6 +115,7 @@ Settings:
the base URI for Go package links in the Diagnostic.CodeDescription field.

Client support:

- **VS Code**: Each diagnostic appears as a squiggly underline.
Hovering reveals the details, along with any suggested fixes.
- **Emacs + eglot**: Each diagnostic appears as a squiggly underline.
Expand All @@ -119,7 +124,6 @@ Client support:
- **Vim + coc.nvim**: ??
- **CLI**: `gopls check file.go`


<!-- Below we list any quick fixes (by their internal fix name)
that aren't analyzers. -->

Expand Down
13 changes: 11 additions & 2 deletions gopls/doc/release/v0.17.0.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@


# Configuration Changes

The `fieldalignment` analyzer, previously disabled by default, has
Expand Down Expand Up @@ -30,13 +28,24 @@ or by selecting a whole declaration or multiple declrations.
In order to avoid ambiguity and surprise about what to extract, some kinds
of paritial selection of a declration cannot invoke this code action.

## Pull diagnostics

When initialized with the option `"pullDiagnostics": true`, gopls will advertise support for the
`textDocument.diagnostic`
[client capability](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics),
which allows editors to request diagnostics directly from gopls using a
`textDocument/diagnostic` request, rather than wait for a
`textDocument/publishDiagnostics` notification. This feature is off by default
until the performance of pull diagnostics is comparable to push diagnostics.

## Standard library version information in Hover

Hovering over a standard library symbol now displays information about the first
Go release containing the symbol. For example, hovering over `errors.As` shows
"Added in go1.13".

## Semantic token modifiers of top-level constructor of types

The semantic tokens response now includes additional modifiers for the top-level
constructor of the type of each symbol:
`interface`, `struct`, `signature`, `pointer`, `array`, `map`, `slice`, `chan`, `string`, `number`, `bool`, and `invalid`.
Expand Down
26 changes: 26 additions & 0 deletions gopls/internal/cache/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
package cache

import (
"crypto/sha256"
"encoding/json"
"fmt"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
)
Expand Down Expand Up @@ -69,6 +71,30 @@ func (d *Diagnostic) String() string {
return fmt.Sprintf("%v: %s", d.Range, d.Message)
}

// Hash computes a hash to identify the diagnostic.
// The hash is for deduplicating within a file, so does not incorporate d.URI.
func (d *Diagnostic) Hash() file.Hash {
h := sha256.New()
for _, t := range d.Tags {
fmt.Fprintf(h, "tag: %s\n", t)
}
for _, r := range d.Related {
fmt.Fprintf(h, "related: %s %s %s\n", r.Location.URI, r.Message, r.Location.Range)
}
fmt.Fprintf(h, "code: %s\n", d.Code)
fmt.Fprintf(h, "codeHref: %s\n", d.CodeHref)
fmt.Fprintf(h, "message: %s\n", d.Message)
fmt.Fprintf(h, "range: %s\n", d.Range)
fmt.Fprintf(h, "severity: %s\n", d.Severity)
fmt.Fprintf(h, "source: %s\n", d.Source)
if d.BundledFixes != nil {
fmt.Fprintf(h, "fixes: %s\n", *d.BundledFixes)
}
var hash [sha256.Size]byte
h.Sum(hash[:0])
return hash
}

type DiagnosticSource string

const (
Expand Down
90 changes: 84 additions & 6 deletions gopls/internal/golang/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,36 @@ import (
"golang.org/x/tools/gopls/internal/util/moremaps"
)

// DiagnoseFile returns pull-based diagnostics for the given file.
func DiagnoseFile(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI) ([]*cache.Diagnostic, error) {
mp, err := NarrowestMetadataForFile(ctx, snapshot, uri)
if err != nil {
return nil, err
}

// TODO(rfindley): consider analysing the package concurrently to package
// diagnostics.

// Get package (list/parse/type check) diagnostics.
pkgDiags, err := snapshot.PackageDiagnostics(ctx, mp.ID)
if err != nil {
return nil, err
}
diags := pkgDiags[uri]

// Get analysis diagnostics.
analyzers := analyzers(snapshot.Options().Staticcheck)
pkgAnalysisDiags, err := snapshot.Analyze(ctx, map[PackageID]*metadata.Package{mp.ID: mp}, analyzers, nil)
if err != nil {
return nil, err
}
analysisDiags := moremaps.Group(pkgAnalysisDiags, byURI)[uri]

// Return the merged set of file diagnostics, combining type error analyses
// with type error diagnostics.
return CombineDiagnostics(diags, analysisDiags), nil
}

// Analyze reports go/analysis-framework diagnostics in the specified package.
//
// If the provided tracker is non-nil, it may be used to provide notifications
Expand All @@ -30,15 +60,63 @@ func Analyze(ctx context.Context, snapshot *cache.Snapshot, pkgIDs map[PackageID
return nil, ctx.Err()
}

analyzers := slices.Collect(maps.Values(settings.DefaultAnalyzers))
if snapshot.Options().Staticcheck {
analyzers = slices.AppendSeq(analyzers, maps.Values(settings.StaticcheckAnalyzers))
}

analyzers := analyzers(snapshot.Options().Staticcheck)
analysisDiagnostics, err := snapshot.Analyze(ctx, pkgIDs, analyzers, tracker)
if err != nil {
return nil, err
}
byURI := func(d *cache.Diagnostic) protocol.DocumentURI { return d.URI }
return moremaps.Group(analysisDiagnostics, byURI), nil
}

// byURI is used for grouping diagnostics.
func byURI(d *cache.Diagnostic) protocol.DocumentURI { return d.URI }

func analyzers(staticcheck bool) []*settings.Analyzer {
analyzers := slices.Collect(maps.Values(settings.DefaultAnalyzers))
if staticcheck {
analyzers = slices.AppendSeq(analyzers, maps.Values(settings.StaticcheckAnalyzers))
}
return analyzers
}

// CombineDiagnostics combines and filters list/parse/type diagnostics from
// tdiags with the analysis adiags, returning the resulting combined set.
//
// Type-error analyzers produce diagnostics that are redundant with type
// checker diagnostics, but more detailed (e.g. fixes). Rather than report two
// diagnostics for the same problem, we combine them by augmenting the
// type-checker diagnostic and discarding the analyzer diagnostic.
//
// If an analysis diagnostic has the same range and message as a
// list/parse/type diagnostic, the suggested fix information (et al) of the
// latter is merged into a copy of the former. This handles the case where a
// type-error analyzer suggests a fix to a type error, and avoids duplication.
//
// The arguments are not modified.
func CombineDiagnostics(tdiags []*cache.Diagnostic, adiags []*cache.Diagnostic) []*cache.Diagnostic {
// Build index of (list+parse+)type errors.
type key struct {
Range protocol.Range
message string
}
combined := make([]*cache.Diagnostic, len(tdiags))
index := make(map[key]int) // maps (Range,Message) to index in tdiags slice
for i, diag := range tdiags {
index[key{diag.Range, diag.Message}] = i
combined[i] = diag
}

// Filter out analysis diagnostics that match type errors,
// retaining their suggested fix (etc) fields.
for _, diag := range adiags {
if i, ok := index[key{diag.Range, diag.Message}]; ok {
copy := *tdiags[i]
copy.SuggestedFixes = diag.SuggestedFixes
copy.Tags = diag.Tags
combined[i] = &copy
continue
}
combined = append(combined, diag)
}
return combined
}
2 changes: 0 additions & 2 deletions gopls/internal/protocol/generate/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ var usedDisambiguate = make(map[string]bool)
var goplsType = map[string]string{
"And_RegOpt_textDocument_colorPresentation": "WorkDoneProgressOptionsAndTextDocumentRegistrationOptions",
"ConfigurationParams": "ParamConfiguration",
"DocumentDiagnosticParams": "string",
"DocumentDiagnosticReport": "string",
"DocumentUri": "DocumentURI",
"InitializeParams": "ParamInitialize",
"LSPAny": "interface{}",
Expand Down
10 changes: 10 additions & 0 deletions gopls/internal/protocol/tsprotocol.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions gopls/internal/protocol/tsserver.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f21a1dc

Please sign in to comment.