Skip to content
Draft
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
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ linters:
- linters:
- containedctx
# we actually want to embed a context here
path: private/bufpkg/bufimage/parser_accessor_handler.go
path: private/bufpkg/bufimage/module_file_resolver.go
- linters:
- containedctx
# we actually want to embed a context here
Expand Down
24 changes: 23 additions & 1 deletion cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4493,7 +4493,29 @@ func TestProtoFileNoWorkspaceOrModule(t *testing.T) {
nil,
bufctl.ExitCodeFileAnnotation,
"", // no stdout
filepath.FromSlash(`testdata/protofileref/noworkspaceormodule/fail/import.proto:3:8:import "`)+`google/type/date.proto": file does not exist`,
fmt.Sprintf(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This (and all others) is likely a breaking change. I am not sure if this matters on stderr, I can't remember, but buf was designed to output errors in the standard file:line:column:message format, which is relied upon by many tools.

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.

Did a sweep, and I think it's mostly ALE -> stdout. That being said, this is still a valid concern and @stefanvanburen had a good suggestion, which I am fully supportive of:

  • Add a new error-format=pretty, which would provide the new error report format
  • Detect if the output is TTY, which we already do for logging, and if it is, then we default to error-format=pretty, and other wise, keep the default as error-format=text. This would keep CI's consistent, ALE, etc.
  • This would apply to anything that produces compiler diagnostics and file annotations, so I can do a bit of work to structure this for lint, breaking

Let me know if this approach seems sound to you!

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.

Discussed this offline, putting some notes here:

  • No need for a new format name, instead we'll just default to this report format for TTY (and keep the old format for non-TTY)
  • No changes to lint, breaking's stdout output -- this will only be used for stderr compiler output

`warning: missing `+"`package`"+` declaration
--> %[1]s
= note: not explicitly specifying a package places the file in the unnamed
package; using it strongly is discouraged

error: imported file does not exist
--> %[1]s:3:1
|
3 | import "google/type/date.proto";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here

error: cannot find `+"`google.type.Date`"+` in this scope
--> %[1]s:6:3
|
6 | google.type.Date date = 1;
| ^^^^^^^^^^^^^^^^ not found in this scope
|
= help: the full name of this scope is `+"`A`"+`

encountered 2 errors and 1 warning`,
filepath.FromSlash("testdata/protofileref/noworkspaceormodule/fail/import.proto"),
),
"build",
filepath.Join("testdata", "protofileref", "noworkspaceormodule", "fail", "import.proto"),
)
Expand Down
31 changes: 29 additions & 2 deletions cmd/buf/imports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package main

import (
"fmt"
"io"
"path/filepath"
"strings"
Expand Down Expand Up @@ -148,7 +149,16 @@ func TestInvalidNonexistentImport(t *testing.T) {
t.Parallel()
testRunStderrWithCache(
t, nil, bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/imports/failure/people/people/v1/people1.proto:5:8:import "nonexistent.proto": file does not exist`),
fmt.Sprintf(
`error: imported file does not exist
--> %s:5:1
|
5 | import "nonexistent.proto";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here
encountered 1 error`,
filepath.FromSlash("testdata/imports/failure/people/people/v1/people1.proto"),
),
"build",
filepath.Join("testdata", "imports", "failure", "people"),
)
Expand All @@ -158,7 +168,24 @@ func TestInvalidNonexistentImportFromDirectDep(t *testing.T) {
t.Parallel()
testRunStderrWithCache(
t, nil, bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/imports/failure/students/students/v1/students.proto:`)+`6:8:import "people/v1/people_nonexistent.proto": file does not exist`,
fmt.Sprintf(
`error: imported file does not exist
--> %[1]s:6:1
|
6 | import "people/v1/people_nonexistent.proto"; // but nonexistent file in explicit direct import
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here
error: cannot find `+"`people.v1.Person2`"+` in this scope
--> %[1]s:10:3
|
10 | people.v1.Person2 person2 = 2;
| ^^^^^^^^^^^^^^^^^ not found in this scope
|
= help: the full name of this scope is `+"`students.v1.Student`"+`
encountered 2 errors`,
filepath.FromSlash("testdata/imports/failure/students/students/v1/students.proto"),
),
"build",
filepath.Join("testdata", "imports", "failure", "students"),
)
Expand Down
2 changes: 1 addition & 1 deletion cmd/buf/internal/command/breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func run(
}
}
if len(allFileAnnotations) > 0 {
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...)
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...)
if err := bufanalysis.PrintFileAnnotationSet(
container.Stdout(),
allFileAnnotationSet,
Expand Down
2 changes: 1 addition & 1 deletion cmd/buf/internal/command/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func run(
}
}
if len(allFileAnnotations) > 0 {
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...)
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...)
if flags.ErrorFormat == "config-ignore-yaml" {
if err := bufcli.PrintFileAnnotationSetLintConfigIgnoreYAMLV1(
container.Stdout(),
Expand Down
46 changes: 42 additions & 4 deletions cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,30 @@ func TestWorkspaceDir(t *testing.T) {
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
)
dirImportError := fmt.Sprintf(`error: imported file does not exist
--> %[1]s:5:1
|
5 | import "request.proto";
| ^^^^^^^^^^^^^^^^^^^^^^^ imported here

error: cannot find %[2]s in this scope
--> %[1]s:8:5
|
8 | request.Request req = 1;
| ^^^^^^^^^^^^^^^ not found in this scope
|
= help: the full name of this scope is %[3]s

encountered 2 errors`,
filepath.FromSlash("testdata/workspace/success/"+baseDirPath+"/proto/rpc.proto"),
"`request.Request`",
"`example.RPC`",
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
dirImportError,
"",
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
Expand All @@ -156,7 +175,7 @@ func TestWorkspaceDir(t *testing.T) {
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
dirImportError,
"",
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
Expand Down Expand Up @@ -366,12 +385,31 @@ func TestWorkspaceDetached(t *testing.T) {
// we'd consider this a bug: you specified the proto directory, and no controlling workspace
// was discovered, therefore you build as if proto was the input directory, which results in
// request.proto not existing as an import.
detachedImportError := fmt.Sprintf(`error: imported file does not exist
--> %[1]s:5:1
|
5 | import "request.proto";
| ^^^^^^^^^^^^^^^^^^^^^^^ imported here

error: cannot find %[2]s in this scope
--> %[1]s:8:5
|
8 | request.Request req = 1;
| ^^^^^^^^^^^^^^^ not found in this scope
|
= help: the full name of this scope is %[3]s

encountered 2 errors`,
filepath.FromSlash("testdata/workspace/success/"+dirPath+"/proto/rpc.proto"),
"`request.Request`",
"`example.RPC`",
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
detachedImportError,
"build",
filepath.Join("testdata", "workspace", "success", dirPath, "proto"),
)
Expand All @@ -392,7 +430,7 @@ func TestWorkspaceDetached(t *testing.T) {
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
detachedImportError,
``,
"lint",
filepath.Join("testdata", "workspace", "success", dirPath, "proto"),
Expand Down
52 changes: 34 additions & 18 deletions cmd/buf/workspace_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package main

import (
"fmt"
"path/filepath"
"testing"

Expand All @@ -26,24 +27,39 @@ import (
func TestWorkspaceSymlinkFail(t *testing.T) {
t.Parallel()
// The workspace includes a symlink that isn't buildable.
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/fail/symlink/b/b.proto:5:8:import "c.proto": file does not exist`),
"build",
filepath.Join("testdata", "workspace", "fail", "symlink"),
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/fail/v2/symlink/b/b.proto:5:8:import "c.proto": file does not exist`),
"build",
filepath.Join("testdata", "workspace", "fail", "v2", "symlink"),
)
for _, dirPath := range []string{
"symlink",
"v2/symlink",
} {
symlinkImportError := fmt.Sprintf(`error: imported file does not exist
--> %[1]s:5:1
|
5 | import "c.proto";
| ^^^^^^^^^^^^^^^^^ imported here

error: cannot find %[2]s in this scope
--> %[1]s:8:5
|
8 | c.C c = 1;
| ^^^ not found in this scope
|
= help: the full name of this scope is %[3]s

encountered 2 errors`,
filepath.FromSlash("testdata/workspace/fail/"+dirPath+"/b/b.proto"),
"`c.C`",
"`b.B`",
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
symlinkImportError,
"build",
filepath.Join("testdata", "workspace", "fail", filepath.FromSlash(dirPath)),
)
}
}

func TestWorkspaceSymlink(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.25.7

require (
buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.11-20250718181942-e35f9b667443.1
buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go v1.36.11-20250109164928-1da0de137947.1
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.11-20260209202127-80ab13bee0bf.1
buf.build/gen/go/bufbuild/registry/connectrpc/go v1.19.1-20260126144947-819582968857.2
buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-819582968857.1
Expand All @@ -16,7 +17,7 @@ require (
buf.build/go/standard v0.1.1-0.20260325175353-2b287e071df5
connectrpc.com/connect v1.19.1
connectrpc.com/otelconnect v0.9.0
github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592
github.com/bufbuild/protocompile v0.14.2-0.20260325024419-a1c17f828947
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1
github.com/cli/browser v1.3.0
github.com/gofrs/flock v0.13.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6
github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/brianvoe/gofakeit/v6 v6.28.0 h1:Xib46XXuQfmlLS2EXRuJpqcw8St6qSZz75OUo0tgAW4=
github.com/brianvoe/gofakeit/v6 v6.28.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs=
github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592 h1:LP+okT4NAncx5csNnPka2j8rKcq1ekvrdQfQp3jD0TI=
github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592/go.mod h1:o4sxIWZ71DJt7/jan0/vi3XbGCQMTBk5KCo27vgT45Q=
github.com/bufbuild/protocompile v0.14.2-0.20260325024419-a1c17f828947 h1:v4a9aeJg8ZOnAy73cqroAYQczI+hGK2+oRfnjPzxHyc=
github.com/bufbuild/protocompile v0.14.2-0.20260325024419-a1c17f828947/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU=
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1 h1:V1xulAoqLqVg44rY97xOR+mQpD2N+GzhMHVwJ030WEU=
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1/go.mod h1:c5D8gWRIZ2HLWO3gXYTtUfw/hbJyD8xikv2ooPxnklQ=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
Expand Down
6 changes: 3 additions & 3 deletions private/buf/buflsp/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package buflsp

import (
"github.com/bufbuild/protocompile/experimental/report"
"github.com/bufbuild/protocompile/experimental/report/tags"
"github.com/bufbuild/protocompile/experimental/report/rtags"
"github.com/bufbuild/protocompile/experimental/source/length"
"go.lsp.dev/protocol"
)
Expand Down Expand Up @@ -61,11 +61,11 @@ func reportDiagnosticToProtocolDiagnostic(
}
}
switch reportDiagnostic.Tag() {
case tags.UnusedImport:
case rtags.UnusedImport:
diagnostic.Tags = []protocol.DiagnosticTag{
protocol.DiagnosticTagUnnecessary,
}
case tags.Deprecated:
case rtags.Deprecated:
diagnostic.Tags = []protocol.DiagnosticTag{
protocol.DiagnosticTagDeprecated,
}
Expand Down
6 changes: 3 additions & 3 deletions private/buf/buflsp/organize_imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

"github.com/bufbuild/protocompile/experimental/ast"
"github.com/bufbuild/protocompile/experimental/ir"
"github.com/bufbuild/protocompile/experimental/report/tags"
"github.com/bufbuild/protocompile/experimental/report/rtags"
"github.com/bufbuild/protocompile/experimental/seq"
"github.com/bufbuild/protocompile/experimental/source"
"github.com/bufbuild/protocompile/experimental/source/length"
Expand Down Expand Up @@ -57,7 +57,7 @@ func (s *server) getOrganizeImportsCodeAction(ctx context.Context, file *file) *
}

// Process UnknownSymbol diagnostics (missing imports)
if diag.Tag() == tags.UnknownSymbol {
if diag.Tag() == rtags.UnknownSymbol {
// Get the exact symbol name as written in the source
missingType := diag.Primary().Text()
if missingType != "" {
Expand All @@ -68,7 +68,7 @@ func (s *server) getOrganizeImportsCodeAction(ctx context.Context, file *file) *
}

// Process UnusedImport diagnostics (imports to remove)
if diag.Tag() == tags.UnusedImport {
if diag.Tag() == rtags.UnusedImport {
// The diagnostic text contains the import path
unusedImportPath := diag.Primary().Text()
if unusedImportPath != "" {
Expand Down
24 changes: 22 additions & 2 deletions private/bufpkg/bufanalysis/bufanalysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"io"
"strconv"
"strings"

"github.com/bufbuild/protocompile/experimental/report"
)

const (
Expand Down Expand Up @@ -207,14 +209,27 @@ type FileAnnotationSet interface {
// These will be deduplicated and sorted.
FileAnnotations() []FileAnnotation

// diagnosticReport returns the diagnostic [report.Report] for the [FileAnnotationSet],
// if set.
//
// This may be nil. If non-nil, it will be used as the output for
// [PrintFileAnnotationSet] when the format is set to "text", rendered
// with a [report.Renderer] configured by the given print options.
diagnosticReport() *report.Report

isFileAnnotationSet()
}

// NewFileAnnotationSet returns a new FileAnnotationSet.
//
// If len(fileAnnotations) is 0, this returns nil.
func NewFileAnnotationSet(fileAnnotations ...FileAnnotation) FileAnnotationSet {
return newFileAnnotationSet(fileAnnotations)
//
// The diagnosticReport is the [report.Report] from the compiler, if available.
// If non-nil, it will be rendered by [PrintFileAnnotationSet] when the format
// is "text". Otherwise, the individual file annotations will be used, same as
// all other print formats.
func NewFileAnnotationSet(diagnosticReport *report.Report, fileAnnotations ...FileAnnotation) FileAnnotationSet {
return newFileAnnotationSet(diagnosticReport, fileAnnotations)
}

// PrintFileAnnotationSet prints the file annotations separated by newlines.
Expand All @@ -226,6 +241,11 @@ func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSe

switch format {
case FormatText:
if diagnosticReport := fileAnnotationSet.diagnosticReport(); diagnosticReport != nil {
renderer := report.Renderer{}
_, _, err := renderer.Render(diagnosticReport, writer)
return err
}
return printAsText(writer, fileAnnotationSet.FileAnnotations())
case FormatJSON:
return printAsJSON(writer, fileAnnotationSet.FileAnnotations())
Expand Down
Loading
Loading