diff --git a/experimental/report/diff.go b/experimental/report/diff.go index eb7dc166..a451908b 100644 --- a/experimental/report/diff.go +++ b/experimental/report/diff.go @@ -15,6 +15,7 @@ package report import ( + "slices" "sort" "strings" @@ -56,40 +57,44 @@ func (h hunk) bold(ss *styleSheet) string { } // hunkDiff computes edit hunks for a diff. -func hunkDiff(span Span, edits []Edit) []hunk { +func hunkDiff(span Span, edits []Edit) (Span, []hunk) { out := make([]hunk, 0, len(edits)*3+1) var prev int - src, offsets := offsetsForDiffing(span, edits) - for i, edit := range edits { - start := offsets[i][0] - end := offsets[i][1] - + span, edits = offsetsForDiffing(span, edits) + src := span.Text() + for _, edit := range edits { out = append(out, - hunk{hunkUnchanged, src[prev:start]}, - hunk{hunkDelete, src[start:end]}, + hunk{hunkUnchanged, src[prev:edit.Start]}, + hunk{hunkDelete, src[edit.Start:edit.End]}, hunk{hunkAdd, edit.Replace}, ) - prev = end + prev = edit.End } - return append(out, hunk{hunkUnchanged, src[prev:]}) + return span, append(out, hunk{hunkUnchanged, src[prev:]}) } // unifiedDiff computes whole-line hunks for this diff, for producing a unified // edit. // // Each slice will contain one or more lines that should be displayed together. -func unifiedDiff(span Span, edits []Edit) []hunk { +func unifiedDiff(span Span, edits []Edit) (Span, []hunk) { // Sort the edits such that they are ordered by starting offset. - src, offsets := offsetsForDiffing(span, edits) + span, edits = offsetsForDiffing(span, edits) + src := span.Text() sort.Slice(edits, func(i, j int) bool { - return offsets[i][0] < offsets[j][0] + return edits[i].Start < edits[j].End }) // Partition offsets into overlapping lines. That is, this connects together // all edit spans whose end and start are not separated by a newline. - prev := &offsets[0] - parts := slicesx.Partition(offsets, func(_, next *[2]int) bool { - if next == prev || !strings.Contains(src[prev[1]:next[0]], "\n") { + prev := &edits[0] + parts := slicesx.Partition(edits, func(_, next *Edit) bool { + if next == prev { + return false + } + + chunk := src[prev.End:next.Start] + if !strings.Contains(chunk, "\n") { return false } @@ -99,12 +104,12 @@ func unifiedDiff(span Span, edits []Edit) []hunk { var out []hunk var prevHunk int - parts(func(i int, offsets [][2]int) bool { + parts(func(_ int, edits []Edit) bool { // First, figure out the start and end of the modified region. - start, end := offsets[0][0], offsets[0][1] - for _, offset := range offsets[1:] { - start = min(start, offset[0]) - end = max(end, offset[1]) + start, end := edits[0].Start, edits[0].End + for _, edit := range edits[1:] { + start = min(start, edit.Start) + end = max(end, edit.End) } // Then, snap the region to be newline delimited. This is the unedited // lines. @@ -114,10 +119,10 @@ func unifiedDiff(span Span, edits []Edit) []hunk { // Now, apply the edits to original to produce the modified result. var buf strings.Builder prev := 0 - for j, offset := range offsets { - buf.WriteString(original[prev:offset[0]]) - buf.WriteString(edits[i+j].Replace) - prev = offset[1] + for _, edit := range edits { + buf.WriteString(original[prev : edit.Start-start]) + buf.WriteString(edit.Replace) + prev = edit.End - start } buf.WriteString(original[prev:]) @@ -131,20 +136,31 @@ func unifiedDiff(span Span, edits []Edit) []hunk { prevHunk = end return true }) - return append(out, hunk{hunkUnchanged, src[prevHunk:]}) + return span, append(out, hunk{hunkUnchanged, src[prevHunk:]}) } // offsetsForDiffing pre-calculates information needed for diffing: -// the line-snapped span, and the offsetsForDiffing of each edit as indices into -// that span. -func offsetsForDiffing(span Span, edits []Edit) (string, [][2]int) { - start, end := adjustLineOffsets(span.File.Text(), span.Start, span.End) - delta := span.Start - start - - offsets := make([][2]int, len(edits)) - for i, edit := range edits { - offsets[i] = [2]int{edit.Start + delta, edit.End + delta} +// the line-snapped span, and edits which are adjusted to conform to that +// span. +func offsetsForDiffing(span Span, edits []Edit) (Span, []Edit) { + edits = slices.Clone(edits) + var start, end int + for i := range edits { + e := &edits[i] + e.Start += span.Start + e.End += span.Start + if i == 0 { + start, end = e.Start, e.End + } else { + start, end = min(e.Start, start), max(e.End, end) + } + } + + start, end = adjustLineOffsets(span.File.Text(), start, end) + for i := range edits { + edits[i].Start -= start + edits[i].End -= start } - return span.File.Text()[start:end], offsets + return span.File.Span(start, end), edits } diff --git a/experimental/report/renderer.go b/experimental/report/renderer.go index 7114618f..fd5f7d48 100644 --- a/experimental/report/renderer.go +++ b/experimental/report/renderer.go @@ -25,6 +25,7 @@ import ( "unicode" "github.com/bufbuild/protocompile/internal/ext/slicesx" + "github.com/bufbuild/protocompile/internal/ext/stringsx" ) // Renderer configures a diagnostic rendering operation. @@ -246,7 +247,7 @@ func (r Renderer) diagnostic(report *Report, d Diagnostic) string { if i > 0 { out.WriteByte('\n') } - suggestion(snippets[0], locations[i][0].Line, lineBarWidth, &ss, &out) + suggestion(snippets[0], lineBarWidth, &ss, &out) return true } @@ -911,7 +912,7 @@ func renderSidebar(bars, lineno, slashAt int, ss *styleSheet, multis []*multilin } // suggestion renders a single suggestion window. -func suggestion(snip snippet, startLine int, lineBarWidth int, ss *styleSheet, out *strings.Builder) { +func suggestion(snip snippet, lineBarWidth int, ss *styleSheet, out *strings.Builder) { out.WriteString(ss.nAccent) padBy(out, lineBarWidth) out.WriteString("help: ") @@ -933,12 +934,29 @@ func suggestion(snip snippet, startLine int, lineBarWidth int, ss *styleSheet, o strings.Contains(snip.Span.Text(), "\n") if multiline { - aLine := startLine - bLine := startLine - for _, hunk := range unifiedDiff(snip.Span, snip.edits) { + span, hunks := unifiedDiff(snip.Span, snip.edits) + aLine := span.StartLoc().Line + bLine := aLine + for i, hunk := range hunks { + // Trim a single newline before and after hunk. This helps deal with + // cases where a newline gets duplicated across hunks of different + // type. + hunk.content, _ = strings.CutPrefix(hunk.content, "\n") + hunk.content, _ = strings.CutSuffix(hunk.content, "\n") + if hunk.content == "" { continue } + + // Skip addition lines that only contain whitespace, if the previous + // hunk was a deletion. This helps avoid cases where a whole line + // was deleted and some indentation was left over. + if prev, _ := slicesx.Get(hunks, i-1); prev.kind == hunkDelete && + hunk.kind == hunkAdd && + stringsx.EveryFunc(hunk.content, unicode.IsSpace) { + continue + } + for _, line := range strings.Split(hunk.content, "\n") { lineno := aLine if hunk.kind == '+' { @@ -954,12 +972,12 @@ func suggestion(snip snippet, startLine int, lineBarWidth int, ss *styleSheet, o ) switch hunk.kind { - case ' ': + case hunkUnchanged: aLine++ bLine++ - case '-': + case hunkDelete: aLine++ - case '+': + case hunkAdd: bLine++ } } @@ -972,8 +990,8 @@ func suggestion(snip snippet, startLine int, lineBarWidth int, ss *styleSheet, o return } - fmt.Fprintf(out, "\n%s%*d | ", ss.nAccent, lineBarWidth, startLine) - hunks := hunkDiff(snip.Span, snip.edits) + span, hunks := hunkDiff(snip.Span, snip.edits) + fmt.Fprintf(out, "\n%s%*d | ", ss.nAccent, lineBarWidth, span.StartLoc().Line) var column int for _, hunk := range hunks { if hunk.content == "" { diff --git a/experimental/report/testdata/suggestions.yaml b/experimental/report/testdata/suggestions.yaml index a31117e2..210b947a 100644 --- a/experimental/report/testdata/suggestions.yaml +++ b/experimental/report/testdata/suggestions.yaml @@ -104,4 +104,26 @@ diagnostics: replace: "{\n option " - start: 10 end: 12 - replace: ";\n }" \ No newline at end of file + replace: ";\n }" + + - message: 'delete some stuff' + level: LEVEL_ERROR + annotations: + - file: 0 + start: 38 + end: 153 + edits: + - start: 0 + end: 13 + - start: 114 + end: 115 + + - message: 'delete this method' + level: LEVEL_ERROR + annotations: + - file: 0 + start: 38 + end: 153 + edits: + - start: 59 + end: 113 \ No newline at end of file diff --git a/experimental/report/testdata/suggestions.yaml.color.txt b/experimental/report/testdata/suggestions.yaml.color.txt index 5dc0b6fb..f0161e23 100644 --- a/experimental/report/testdata/suggestions.yaml.color.txt +++ b/experimental/report/testdata/suggestions.yaml.color.txt @@ -46,5 +46,22 @@ ⟨blu⟩ 9 | ⟨b.grn⟩+⟨grn⟩ } ⟨blu⟩ | ⟨reset⟩ -⟨b.red⟩encountered 2 errors and 1 warning⟨reset⟩ +⟨b.red⟩error: delete some stuff⟨reset⟩ +⟨blu⟩ --> foo.proto:5:1 +⟨blu⟩ help: + | +⟨blu⟩ 5 | ⟨b.red⟩-⟨red⟩ service Foo { +⟨blu⟩ 6 | ⟨reset⟩ ⟨reset⟩ rpc Get(GetRequest) returns GetResponse; +⟨blu⟩ 7 | ⟨reset⟩ ⟨reset⟩ rpc Put(PutRequest) returns (PutResponse) [foo = bar]; +⟨blu⟩ 8 | ⟨b.red⟩-⟨red⟩ } +⟨blu⟩ | ⟨reset⟩ + +⟨b.red⟩error: delete this method⟨reset⟩ +⟨blu⟩ --> foo.proto:5:1 +⟨blu⟩ help: + | +⟨blu⟩ 7 | ⟨b.red⟩-⟨red⟩ rpc Put(PutRequest) returns (PutResponse) [foo = bar]; +⟨blu⟩ | ⟨reset⟩ + +⟨b.red⟩encountered 4 errors and 1 warning⟨reset⟩ ⟨reset⟩ \ No newline at end of file diff --git a/experimental/report/testdata/suggestions.yaml.fancy.txt b/experimental/report/testdata/suggestions.yaml.fancy.txt index 2a2c4a33..4dc5b31b 100644 --- a/experimental/report/testdata/suggestions.yaml.fancy.txt +++ b/experimental/report/testdata/suggestions.yaml.fancy.txt @@ -46,4 +46,21 @@ error: method options must go in a block 9 | + } | -encountered 2 errors and 1 warning +error: delete some stuff + --> foo.proto:5:1 + help: + | + 5 | - service Foo { + 6 | rpc Get(GetRequest) returns GetResponse; + 7 | rpc Put(PutRequest) returns (PutResponse) [foo = bar]; + 8 | - } + | + +error: delete this method + --> foo.proto:5:1 + help: + | + 7 | - rpc Put(PutRequest) returns (PutResponse) [foo = bar]; + | + +encountered 4 errors and 1 warning diff --git a/experimental/report/testdata/suggestions.yaml.simple.txt b/experimental/report/testdata/suggestions.yaml.simple.txt index 6915c543..864cc1ad 100644 --- a/experimental/report/testdata/suggestions.yaml.simple.txt +++ b/experimental/report/testdata/suggestions.yaml.simple.txt @@ -3,3 +3,5 @@ remark: foo.proto:1:10: let protocompile pick a syntax for you warning: foo.proto:5:9: services should have a `Service` suffix error: foo.proto:6:31: missing (...) around return type error: foo.proto:7:45: method options must go in a block +error: foo.proto:5:1: delete some stuff +error: foo.proto:5:1: delete this method diff --git a/internal/ext/iterx/iterx.go b/internal/ext/iterx/iterx.go index b001c442..5f12e0f5 100644 --- a/internal/ext/iterx/iterx.go +++ b/internal/ext/iterx/iterx.go @@ -40,6 +40,17 @@ func First[T any](seq iter.Seq[T]) (v T, ok bool) { return v, ok } +// All returns whether every element of an iterator satisfies the given +// predicate. Returns true if seq yields no values. +func All[T any](seq iter.Seq[T], p func(T) bool) bool { + all := true + seq(func(v T) bool { + all = p(v) + return all + }) + return all +} + // Map returns a new iterator applying f to each element of seq. func Map[T, U any](seq iter.Seq[T], f func(T) U) iter.Seq[U] { return func(yield func(U) bool) { diff --git a/internal/ext/stringsx/stringsx.go b/internal/ext/stringsx/stringsx.go new file mode 100644 index 00000000..f8200556 --- /dev/null +++ b/internal/ext/stringsx/stringsx.go @@ -0,0 +1,53 @@ +// Copyright 2020-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// package stringsx contains extensions to Go's package strings. +package stringsx + +import ( + "github.com/bufbuild/protocompile/internal/ext/iterx" + "github.com/bufbuild/protocompile/internal/ext/unsafex" + "github.com/bufbuild/protocompile/internal/iter" +) + +// EveryFunc verifies that all runes in the string satisfy the given predicate. +func EveryFunc(s string, p func(rune) bool) bool { + return iterx.All(Runes(s), p) +} + +// Runes returns an iterator over the runes in a string. +// +// Each non-UTF-8 byte in the string is yielded as a replacement character (U+FFFD). +func Runes(s string) iter.Seq[rune] { + return func(yield func(r rune) bool) { + for _, r := range s { + if !yield(r) { + return + } + } + } +} + +// Bytes returns an iterator over the bytes in a string. +func Bytes(s string) iter.Seq[byte] { + return func(yield func(byte) bool) { + for i := 0; i < len(s); i++ { + // Avoid performing a bounds check each loop step. + b := *unsafex.Add(unsafex.StringData(s), i) + if !yield(b) { + return + } + } + } +}