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
17 changes: 13 additions & 4 deletions private/buf/buflsp/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +53 to +54
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was just checking where we Debugf in protocompile; this logic is the same as before, just updates the format to debug: similar to the other added types.

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()],
Expand Down
141 changes: 141 additions & 0 deletions private/buf/buflsp/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this sort of thing seems very valuable to include in our hover diagnostics, as it really points to how to solve the issue. (I do think another follow up of adding code actions to resolve these, where possible, would be great, though.)

},
},
},
{
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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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];
}
12 changes: 12 additions & 0 deletions private/buf/buflsp/testdata/diagnostics/enum_map_key.proto
Original file line number Diff line number Diff line change
@@ -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, string> status_map = 1;
}
8 changes: 8 additions & 0 deletions private/buf/buflsp/testdata/diagnostics/group_proto3.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto3";

package diagnostics.v1;

// Using group syntax in proto3 is not supported
message TestMessage {
group MyGroup = 1 {}
}
8 changes: 8 additions & 0 deletions private/buf/buflsp/testdata/diagnostics/invalid_map_key.proto
Original file line number Diff line number Diff line change
@@ -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<double, string> bad_map = 1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
syntax = "proto3";

package diagnostics.v1;

// Referencing a synthetic map entry type directly is not permitted
message Foo {
map<int32, int32> foo_bar = 1;
FooBarEntry bad_field = 2;
}
12 changes: 12 additions & 0 deletions private/buf/buflsp/testdata/diagnostics/map_typed_extension.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
syntax = "proto2";

package diagnostics.v1;

message TestMessage {
extensions 100 to 199;
}

// Extensions cannot have map-typed fields
extend TestMessage {
map<string, string> bad_extension = 100;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package diagnostics.v1;

syntax = "proto3";

message TestMessage {
string name = 1;
}
8 changes: 8 additions & 0 deletions private/buf/buflsp/testdata/diagnostics/unknown_type.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto3";

package diagnostics.v1;

// A field referencing a type that doesn't exist
message TestMessage {
UnknownType field = 1;
}
Loading