Skip to content

Commit

Permalink
Implement machine-applicable suggestions in diagnostics (#392)
Browse files Browse the repository at this point in the history
This PR adds `report.SuggestEdits`, which allows diagnostics to suggest
changes to an annotated span. This will be used for surfacing
quick-fixes in the LSP (for example, for suggesting deleting unused
imports, adding a `syntax` declaration, adding missing field numbers,
and so on).

The compiler is also able to render these as a simple diff, similar to
some of rustc's diagnostics. I've tried to keep the data model simple:
no diffing actually happens, the diagnostics are expected to generate
the diffs manually. All the renderer does is convert the edits into
hunks to show to the user. This PR contains examples in the lexer that
shows how it is meant to be used.
  • Loading branch information
mcy authored Jan 21, 2025
1 parent 2b8da30 commit bdba7cb
Show file tree
Hide file tree
Showing 17 changed files with 812 additions and 107 deletions.
45 changes: 30 additions & 15 deletions experimental/parser/diagnostics_number.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package parser

import (
"fmt"
"strings"

"github.com/bufbuild/protocompile/experimental/internal/taxa"
"github.com/bufbuild/protocompile/experimental/report"
Expand Down Expand Up @@ -62,23 +63,32 @@ func (e errInvalidBase) Diagnose(d *report.Diagnostic) {
base = fmt.Sprintf("base-%d", e.Base)
}

isFloat := taxa.IsFloat(e.Token)
if !isFloat && e.Base == 8 {
d.Apply(
report.Snippetf(e.Token, "replace `0o` with `0`"),
report.Notef("Protobuf does not support the `0o` prefix for octal literals"),
)
return
kind := taxa.Classify(e.Token)
if kind == taxa.Int {
switch e.Base {
case 2:
if value := e.Token.AsBigInt(); value != nil {
d.Apply(
report.SuggestEdits(e.Token, "use a hexadecimal literal instead", report.Edit{
Start: 0,
End: len(e.Token.Text()),
Replace: fmt.Sprintf("%#x", value),
}),
report.Notef("Protobuf does not support binary literals"),
)
return
}
case 8:
d.Apply(
report.SuggestEdits(e.Token, "remove the `o`", report.Edit{Start: 1, End: 2}),
report.Notef("octal literals are prefixed with `0`, not `0o`"),
)
return
}
}

kind := "integer"
if isFloat {
kind = "floating-point"
}

d.Apply(
report.Snippet(e.Token),
report.Notef("Protobuf does not support %s %s literals", base, kind),
report.Notef("Protobuf does not support %s %s", base, kind),
)
}

Expand All @@ -93,9 +103,14 @@ type errThousandsSep struct {

// Diagnose implements [report.Diagnose].
func (e errThousandsSep) Diagnose(d *report.Diagnostic) {
span := e.Token.Span()
d.Apply(
report.Message("%s contains underscores", taxa.Classify(e.Token)),
report.Snippet(e.Token),
report.SuggestEdits(e.Token, "remove these underscores", report.Edit{
Start: 0,
End: len(span.Text()),
Replace: strings.ReplaceAll(span.Text(), "_", ""),
}),
report.Notef("Protobuf does not support Go/Java/Rust-style thousands separators"),
)
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,38 @@
error: unsupported base for integer literal
--> testdata/lexer/numbers/exotic-base.proto:1:1
help: use a hexadecimal literal instead
|
1 | 0b1100101
| ^^^^^^^^^
= note: Protobuf does not support binary integer literals
1 | - 0b1100101
1 | + 0x65
|
= note: Protobuf does not support binary literals

error: unsupported base for integer literal
--> testdata/lexer/numbers/exotic-base.proto:2:1
help: use a hexadecimal literal instead
|
2 | 0B1010101
| ^^^^^^^^^
= note: Protobuf does not support binary integer literals
2 | - 0B1010101
2 | + 0x55
|
= note: Protobuf does not support binary literals

error: unsupported base for integer literal
--> testdata/lexer/numbers/exotic-base.proto:3:1
help: remove the `o`
|
3 | - 0o1234567
3 | + 01234567
|
3 | 0o1234567
| ^^^^^^^^^ replace `0o` with `0`
= note: Protobuf does not support the `0o` prefix for octal literals
= note: octal literals are prefixed with `0`, not `0o`

error: unsupported base for integer literal
--> testdata/lexer/numbers/exotic-base.proto:4:1
help: remove the `o`
|
4 | - 0O1234567
4 | + 01234567
|
4 | 0O1234567
| ^^^^^^^^^ replace `0o` with `0`
= note: Protobuf does not support the `0o` prefix for octal literals
= note: octal literals are prefixed with `0`, not `0o`

error: unexpected characters in floating-point literal
--> testdata/lexer/numbers/exotic-base.proto:6:1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,43 +1,55 @@
error: integer literal contains underscores
--> testdata/lexer/numbers/thousands.proto:1:1
help: remove these underscores
|
1 | - 1_000_000
1 | + 1000000
|
1 | 1_000_000
| ^^^^^^^^^
= note: Protobuf does not support Go/Java/Rust-style thousands separators

error: unsupported base for integer literal
--> testdata/lexer/numbers/thousands.proto:2:1
help: use a hexadecimal literal instead
|
2 | - 0b1_000_000
2 | + 0x40
|
2 | 0b1_000_000
| ^^^^^^^^^^^
= note: Protobuf does not support binary integer literals
= note: Protobuf does not support binary literals

error: unsupported base for integer literal
--> testdata/lexer/numbers/thousands.proto:3:1
help: remove the `o`
|
3 | - 0o1_000_000
3 | + 01_000_000
|
3 | 0o1_000_000
| ^^^^^^^^^^^ replace `0o` with `0`
= note: Protobuf does not support the `0o` prefix for octal literals
= note: octal literals are prefixed with `0`, not `0o`

error: integer literal contains underscores
--> testdata/lexer/numbers/thousands.proto:4:1
help: remove these underscores
|
4 | - 0x1_000_000
4 | + 0x1000000
|
4 | 0x1_000_000
| ^^^^^^^^^^^
= note: Protobuf does not support Go/Java/Rust-style thousands separators

error: integer literal contains underscores
--> testdata/lexer/numbers/thousands.proto:5:1
help: remove these underscores
|
5 | - 01_000_000
5 | + 01000000
|
5 | 01_000_000
| ^^^^^^^^^^
= note: Protobuf does not support Go/Java/Rust-style thousands separators

error: floating-point literal contains underscores
--> testdata/lexer/numbers/thousands.proto:6:1
help: remove these underscores
|
6 | - 1_000_000.00
6 | + 1000000.00
|
6 | 1_000_000.00
| ^^^^^^^^^^^^
= note: Protobuf does not support Go/Java/Rust-style thousands separators

encountered 6 errors
56 changes: 56 additions & 0 deletions experimental/report/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,34 @@ type Diagnostic struct {
notes, help, debug []string
}

// Edit is an edit to suggest on a snippet.
//
// See [SuggestEdits].
type Edit struct {
// The start and end offsets of the edit, relative the span of the snippet
// this edit is applied to (so, Start == 0 means the edit starts at the
// start of the span).
//
// An insertion without deletion is modeled by Start == End.
Start, End int

// Text to replace the content between Start and End with.
//
// A pure deletion is modeled by Replace == "".
Replace string
}

// IsDeletion returns whether this edit involves deleting part of the source
// text.
func (e Edit) IsDeletion() bool {
return e.Start < e.End
}

// IsInsertion returns whether this edit involves inserting new text.
func (e Edit) IsInsertion() bool {
return e.Replace != ""
}

// DiagnosticOption is an option that can be applied to a [Diagnostic].
//
// IsZero values passed to [Diagnostic.Apply] are ignored.
Expand Down Expand Up @@ -147,6 +175,28 @@ func Snippetf(at Spanner, format string, args ...any) DiagnosticOption {
}
}

// SuggestEdits is like [Snippet], but generates a snippet that contains
// machine-applicable suggestions.
//
// A snippet with suggestions will be displayed separately from other snippets.
// The message associated with the snippet will be prefixed with "help:" when
// rendered.
func SuggestEdits(at Spanner, message string, edits ...Edit) DiagnosticOption {
span := getSpan(at)
text := span.Text()
for _, edit := range edits {
// Force a bounds check here to make it easier to debug, instead of
// panicking in the renderer (or emitting an invalid report proto).
_ = text[edit.Start:edit.End]
}

return snippet{
Span: span,
message: message,
edits: edits,
}
}

// Notef returns a DiagnosticOption that provides the user with context about the
// diagnostic, after the annotations.
func Notef(format string, args ...any) DiagnosticOption {
Expand Down Expand Up @@ -185,6 +235,12 @@ type snippet struct {
// Whether this is a "primary" snippet, which is used for deciding whether or not
// to mark the snippet with the same color as the overall diagnostic.
primary bool

// Edits to include in this snippet. This causes this snippet to be rendered
// in its own window when it is non-empty, and no underline will appear for
// the overall span of the snippet. The message will still be used, as the
// title of the window.
edits []Edit
}

func (a snippet) apply(d *Diagnostic) {
Expand Down
Loading

0 comments on commit bdba7cb

Please sign in to comment.