Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions private/buf/buflsp/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,41 @@ message TestMessage {
})
}

// TestDiagnosticsNoTransitiveLeak verifies that compile errors from a file
// imported by the opened file do not get republished under the opened file's
// URI. Diagnostics carry only a Range (no file path), so a leaked error lands
// at the importing file's same line/column, appearing as an "overlay".
func TestDiagnosticsNoTransitiveLeak(t *testing.T) {
t.Parallel()

synctest.Test(t, func(t *testing.T) {
importerPath, err := filepath.Abs("testdata/diagnostics/transitive_errors/importer.proto")
require.NoError(t, err)

_, testURI, capture := setupLSPServerWithDiagnostics(t, importerPath)

// Drain every goroutine in the bubble so the async RunChecks publish
// has definitely landed in the capture before we assert.
synctest.Wait()

diagnostics := capture.wait(t, testURI, time.Second, func(*protocol.PublishDiagnosticsParams) bool {
return true
})
require.NotNil(t, diagnostics)

// broken.proto has an `UnknownType` error at line 7 col 3. If the filter
// in buildImage regresses, that diagnostic resurfaces against
// importer.proto at the same (foreign) range.
for _, d := range diagnostics.Diagnostics {
assert.NotContains(
t, d.Message, "UnknownType",
"diagnostic from broken.proto leaked into importer.proto at %+v: %q",
d.Range, d.Message,
)
}
})
}

// diagnosticsCapture captures publishDiagnostics notifications from the LSP server.
type diagnosticsCapture struct {
mu sync.Mutex
Expand Down
21 changes: 20 additions & 1 deletion private/buf/buflsp/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,32 @@ func buildImage(
) (bufimage.Image, []protocol.Diagnostic) {
image, rpt, err := bufimage.BuildImageFromOpener(ctx, logger, opener, []string{path})

// Resolve the target file's path via the opener. The opener is keyed by the
// workspace-relative path, but its *source.File records an absolute path
// (the editor URI filename), which is what diagnostic primary spans carry.
var targetPath string
if targetFile, _ := opener.Open(path); targetFile != nil {
targetPath = targetFile.Path()
}

var diagnostics []protocol.Diagnostic
var hasErrors bool
for _, diagnostic := range rpt.Diagnostics {
if diagnostic.Primary().IsZero() || diagnostic.Level() > report.Error {
primary := diagnostic.Primary()
if primary.IsZero() || diagnostic.Level() > report.Error {
continue
}
// Track errors across the whole compilation so we skip linting an
// incomplete image below, even when the error is in a transitive
// import.
hasErrors = true
// Only surface diagnostics whose primary span is in the target file.
// Errors in transitively-compiled imports belong to those files'
// diagnostic streams; publishing them here would overlay them onto
// the current file at the wrong line and column.
if targetPath != "" && primary.Path() != targetPath {
continue
}
diagnostics = append(diagnostics, reportDiagnosticToProtocolDiagnostic(diagnostic))
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto3";

package transitive.v1;

message Broken {
// Reference to a type that does not exist anywhere in the workspace.
UnknownType field = 1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
version: v2
lint:
use:
- STANDARD
except:
- PACKAGE_DIRECTORY_MATCH
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
syntax = "proto3";

package transitive.v1;

import "broken.proto";

// This file is syntactically valid and type-correct on its own.
// Diagnostics produced by compiling broken.proto must not be attributed
// to this file's URI by the LSP.
message Importer {
Broken dep = 1;
}
Loading