diff --git a/private/buf/buflsp/diagnostics_test.go b/private/buf/buflsp/diagnostics_test.go index c355214501..e50e69d70f 100644 --- a/private/buf/buflsp/diagnostics_test.go +++ b/private/buf/buflsp/diagnostics_test.go @@ -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 diff --git a/private/buf/buflsp/image.go b/private/buf/buflsp/image.go index cbbcbd6be1..0264a38bd7 100644 --- a/private/buf/buflsp/image.go +++ b/private/buf/buflsp/image.go @@ -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)) } diff --git a/private/buf/buflsp/testdata/diagnostics/transitive_errors/broken.proto b/private/buf/buflsp/testdata/diagnostics/transitive_errors/broken.proto new file mode 100644 index 0000000000..1bb234dcab --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/transitive_errors/broken.proto @@ -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; +} diff --git a/private/buf/buflsp/testdata/diagnostics/transitive_errors/buf.yaml b/private/buf/buflsp/testdata/diagnostics/transitive_errors/buf.yaml new file mode 100644 index 0000000000..6fc5e4bb71 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/transitive_errors/buf.yaml @@ -0,0 +1,6 @@ +version: v2 +lint: + use: + - STANDARD + except: + - PACKAGE_DIRECTORY_MATCH diff --git a/private/buf/buflsp/testdata/diagnostics/transitive_errors/importer.proto b/private/buf/buflsp/testdata/diagnostics/transitive_errors/importer.proto new file mode 100644 index 0000000000..8266ef8af2 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/transitive_errors/importer.proto @@ -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; +}