From 50dc5facacc43ae2958cd2b0ab29db3b8023bef5 Mon Sep 17 00:00:00 2001 From: Stefan VanBuren Date: Thu, 9 Apr 2026 16:46:22 -0400 Subject: [PATCH] Extend diagnostics to include notes/help/debug Quick follow-up to #4439. In that PR, we mentioned that these [fields could become code actions][1], which is probably true with some refactoring / exporting of more of the underlying structure so we don't have just `string`s without more structured data. For now, it seems valuable to just expose these as additional information added to the hover states. See the added test-cases for how these look. [1]: https://github.com/bufbuild/buf/pull/4439#discussion_r3060536313 --- private/buf/buflsp/diagnostic.go | 17 ++- private/buf/buflsp/diagnostics_test.go | 141 ++++++++++++++++++ .../diagnostics/compact_option_colon.proto | 9 ++ .../testdata/diagnostics/enum_map_key.proto | 12 ++ .../testdata/diagnostics/group_proto3.proto | 8 + .../diagnostics/invalid_map_key.proto | 8 + .../diagnostics/map_entry_type_misuse.proto | 9 ++ .../diagnostics/map_typed_extension.proto | 12 ++ .../diagnostics/syntax_not_first.proto | 7 + .../testdata/diagnostics/unknown_type.proto | 8 + 10 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 private/buf/buflsp/testdata/diagnostics/compact_option_colon.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/enum_map_key.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/group_proto3.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/invalid_map_key.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/map_entry_type_misuse.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/map_typed_extension.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/syntax_not_first.proto create mode 100644 private/buf/buflsp/testdata/diagnostics/unknown_type.proto diff --git a/private/buf/buflsp/diagnostic.go b/private/buf/buflsp/diagnostic.go index 5f86e9f7c2..b54eddc267 100644 --- a/private/buf/buflsp/diagnostic.go +++ b/private/buf/buflsp/diagnostic.go @@ -43,12 +43,21 @@ var reportLevelToDiagnosticSeverity = map[report.Level]protocol.DiagnosticSeveri func reportDiagnosticToProtocolDiagnostic( reportDiagnostic report.Diagnostic, ) protocol.Diagnostic { - message := reportDiagnostic.Message() + parts := []string{reportDiagnostic.Message()} + for _, note := range reportDiagnostic.Notes() { + parts = append(parts, "note: "+note) + } + for _, help := range reportDiagnostic.Help() { + parts = append(parts, "help: "+help) + } + // Debug info is implementation-level detail; only include it for ICE where + // all available context helps diagnose the unexpected failure. if reportDiagnostic.Level() == report.ICE { - // Include notes for ICE - notes := append([]string{message}, reportDiagnostic.Notes()...) - message = strings.Join(notes, " ") + for _, debug := range reportDiagnostic.Debug() { + parts = append(parts, "debug: "+debug) + } } + message := strings.Join(parts, "\n") diagnostic := protocol.Diagnostic{ Source: serverName, Severity: reportLevelToDiagnosticSeverity[reportDiagnostic.Level()], diff --git a/private/buf/buflsp/diagnostics_test.go b/private/buf/buflsp/diagnostics_test.go index 88125c4424..de7fd0d681 100644 --- a/private/buf/buflsp/diagnostics_test.go +++ b/private/buf/buflsp/diagnostics_test.go @@ -237,6 +237,147 @@ func TestDiagnostics(t *testing.T) { }, }, }, + { + name: "invalid_map_key_type_with_help", + protoFile: "testdata/diagnostics/invalid_map_key.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 6, Character: 6}, + End: protocol.Position{Line: 6, Character: 12}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "expected map key type, found scalar type `double`\nhelp: valid map key types are integer types, `string`, and `bool`", + }, + }, + }, + { + name: "enum_map_key_type_with_multiple_helps", + protoFile: "testdata/diagnostics/enum_map_key.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 10, Character: 6}, + End: protocol.Position{Line: 10, Character: 12}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "expected map key type, found enum type `diagnostics.v1.Status`\nhelp: valid map key types are integer types, `string`, and `bool`\nhelp: counterintuitively, user-defined enum types cannot be used as keys", + }, + }, + }, + { + name: "unknown_type_with_scope_help", + protoFile: "testdata/diagnostics/unknown_type.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 6, Character: 2}, + End: protocol.Position{Line: 6, Character: 13}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "cannot find `UnknownType` in this scope\nhelp: the full name of this scope is `diagnostics.v1.TestMessage`", + }, + }, + }, + { + // group syntax is only valid in proto2; using it in proto3 produces + // an error with a note pointing to the correct syntax version. + name: "group_syntax_in_proto3_with_note", + protoFile: "testdata/diagnostics/group_proto3.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 6, Character: 2}, + End: protocol.Position{Line: 6, Character: 7}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "group syntax is not supported\nnote: group syntax is only available in proto2", + }, + }, + }, + { + // map-typed extensions are not supported; the error includes a help + // message suggesting a workaround. + name: "map_typed_extension_with_help", + protoFile: "testdata/diagnostics/map_typed_extension.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 10, Character: 2}, + End: protocol.Position{Line: 10, Character: 21}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "unsupported map-typed extension\nhelp: extensions cannot be map-typed; instead, define a message type with a map-typed field", + }, + }, + }, + { + // placing package before syntax triggers an error with a note explaining + // that syntax must be the first declaration. Without a valid syntax + // declaration, the parser falls back to proto2 mode and also produces a + // secondary error for proto3-style unqualified field declarations. + name: "syntax_not_first_with_note", + protoFile: "testdata/diagnostics/syntax_not_first.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 2, Character: 0}, + End: protocol.Position{Line: 2, Character: 18}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "unexpected `syntax` declaration\nnote: a `syntax` declaration must be the first declaration in a file", + }, + { + Range: protocol.Range{ + Start: protocol.Position{Line: 5, Character: 2}, + End: protocol.Position{Line: 5, Character: 8}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "unexpected type name\nnote: modifiers are required in proto2", + }, + }, + }, + { + // using a synthetic map entry type as a field type is not permitted; + // the error includes a help message explaining why. + name: "map_entry_type_misuse_with_help", + protoFile: "testdata/diagnostics/map_entry_type_misuse.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 7, Character: 2}, + End: protocol.Position{Line: 7, Character: 13}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "use of synthetic map entry type\nhelp: despite having a user-visible symbol, map entry types cannot be used as field types", + }, + }, + }, + { + // compact_option_colon uses ':' instead of '=' inside [...] options, + // which triggers a note explaining the correct syntax. + name: "compact_option_colon_with_note", + protoFile: "testdata/diagnostics/compact_option_colon.proto", + expectedDiagnostics: []protocol.Diagnostic{ + { + Range: protocol.Range{ + Start: protocol.Position{Line: 7, Character: 29}, + End: protocol.Position{Line: 7, Character: 30}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: "buf-lsp", + Message: "unexpected `:` in compact option\nnote: top-level `option` assignment uses `=`, not `:`", + }, + }, + }, } for _, tt := range tests { diff --git a/private/buf/buflsp/testdata/diagnostics/compact_option_colon.proto b/private/buf/buflsp/testdata/diagnostics/compact_option_colon.proto new file mode 100644 index 0000000000..938ed24597 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/compact_option_colon.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// Using ':' instead of '=' in a compact option triggers a note explaining the +// correct syntax for top-level option assignments. +message TestMessage { + string name = 1 [deprecated: true]; +} diff --git a/private/buf/buflsp/testdata/diagnostics/enum_map_key.proto b/private/buf/buflsp/testdata/diagnostics/enum_map_key.proto new file mode 100644 index 0000000000..dac7e17d62 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/enum_map_key.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// Using an enum type as a map key is not permitted +enum Status { + STATUS_UNSPECIFIED = 0; +} + +message TestMessage { + map status_map = 1; +} diff --git a/private/buf/buflsp/testdata/diagnostics/group_proto3.proto b/private/buf/buflsp/testdata/diagnostics/group_proto3.proto new file mode 100644 index 0000000000..5ba5acc292 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/group_proto3.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// Using group syntax in proto3 is not supported +message TestMessage { + group MyGroup = 1 {} +} diff --git a/private/buf/buflsp/testdata/diagnostics/invalid_map_key.proto b/private/buf/buflsp/testdata/diagnostics/invalid_map_key.proto new file mode 100644 index 0000000000..f3c7fe6bd6 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/invalid_map_key.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// A field with an invalid map key type (double is not a valid map key type) +message TestMessage { + map bad_map = 1; +} diff --git a/private/buf/buflsp/testdata/diagnostics/map_entry_type_misuse.proto b/private/buf/buflsp/testdata/diagnostics/map_entry_type_misuse.proto new file mode 100644 index 0000000000..b909174702 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/map_entry_type_misuse.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// Referencing a synthetic map entry type directly is not permitted +message Foo { + map foo_bar = 1; + FooBarEntry bad_field = 2; +} diff --git a/private/buf/buflsp/testdata/diagnostics/map_typed_extension.proto b/private/buf/buflsp/testdata/diagnostics/map_typed_extension.proto new file mode 100644 index 0000000000..aba4308d26 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/map_typed_extension.proto @@ -0,0 +1,12 @@ +syntax = "proto2"; + +package diagnostics.v1; + +message TestMessage { + extensions 100 to 199; +} + +// Extensions cannot have map-typed fields +extend TestMessage { + map bad_extension = 100; +} diff --git a/private/buf/buflsp/testdata/diagnostics/syntax_not_first.proto b/private/buf/buflsp/testdata/diagnostics/syntax_not_first.proto new file mode 100644 index 0000000000..04bf04c09f --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/syntax_not_first.proto @@ -0,0 +1,7 @@ +package diagnostics.v1; + +syntax = "proto3"; + +message TestMessage { + string name = 1; +} diff --git a/private/buf/buflsp/testdata/diagnostics/unknown_type.proto b/private/buf/buflsp/testdata/diagnostics/unknown_type.proto new file mode 100644 index 0000000000..a717165052 --- /dev/null +++ b/private/buf/buflsp/testdata/diagnostics/unknown_type.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package diagnostics.v1; + +// A field referencing a type that doesn't exist +message TestMessage { + UnknownType field = 1; +}