diff --git a/.golangci.yml b/.golangci.yml index bd0974cfb3..b76568064d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -253,7 +253,7 @@ linters: - linters: - containedctx # we actually want to embed a context here - path: private/bufpkg/bufimage/module_file_resolver.go + path: private/bufpkg/bufimage/parser_accessor_handler.go - linters: - containedctx # we actually want to embed a context here @@ -449,13 +449,6 @@ linters: # term.IsTerminal. Safe on all supported platforms. path: private/pkg/slogapp/console.go text: "G115:" - - linters: - - gosec - # G115: uintptr->int conversion for passing a terminal file descriptor - # to term.IsTerminal, which requires int. Safe on all supported platforms; - # fd values fit in int. - path: private/buf/bufctl/controller.go - text: "G115:" - linters: - gosec # G602: false positive — the loop index i is bounded by the length of diff --git a/cmd/buf/buf_test.go b/cmd/buf/buf_test.go index 6f3c7e0ecd..4baac577e7 100644 --- a/cmd/buf/buf_test.go +++ b/cmd/buf/buf_test.go @@ -4493,10 +4493,7 @@ func TestProtoFileNoWorkspaceOrModule(t *testing.T) { nil, bufctl.ExitCodeFileAnnotation, "", // no stdout - fmt.Sprintf( - "%[1]s:3:1:imported file does not exist\n%[1]s:6:3:cannot find `google.type.Date` in this scope", - filepath.FromSlash("testdata/protofileref/noworkspaceormodule/fail/import.proto"), - ), + filepath.FromSlash(`testdata/protofileref/noworkspaceormodule/fail/import.proto:3:8:import "`)+`google/type/date.proto": file does not exist`, "build", filepath.Join("testdata", "protofileref", "noworkspaceormodule", "fail", "import.proto"), ) diff --git a/cmd/buf/imports_test.go b/cmd/buf/imports_test.go index d3788ee27c..7c5ea718de 100644 --- a/cmd/buf/imports_test.go +++ b/cmd/buf/imports_test.go @@ -15,7 +15,6 @@ package main import ( - "fmt" "io" "path/filepath" "strings" @@ -149,10 +148,7 @@ func TestInvalidNonexistentImport(t *testing.T) { t.Parallel() testRunStderrWithCache( t, nil, bufctl.ExitCodeFileAnnotation, - fmt.Sprintf( - "%[1]s:5:1:imported file does not exist", - filepath.FromSlash("testdata/imports/failure/people/people/v1/people1.proto"), - ), + filepath.FromSlash(`testdata/imports/failure/people/people/v1/people1.proto:5:8:import "nonexistent.proto": file does not exist`), "build", filepath.Join("testdata", "imports", "failure", "people"), ) @@ -162,10 +158,7 @@ func TestInvalidNonexistentImportFromDirectDep(t *testing.T) { t.Parallel() testRunStderrWithCache( t, nil, bufctl.ExitCodeFileAnnotation, - fmt.Sprintf( - "%[1]s:6:1:imported file does not exist\n%[1]s:10:3:cannot find `people.v1.Person2` in this scope", - filepath.FromSlash("testdata/imports/failure/students/students/v1/students.proto"), - ), + filepath.FromSlash(`testdata/imports/failure/students/students/v1/students.proto:`)+`6:8:import "people/v1/people_nonexistent.proto": file does not exist`, "build", filepath.Join("testdata", "imports", "failure", "students"), ) diff --git a/cmd/buf/internal/command/breaking/breaking.go b/cmd/buf/internal/command/breaking/breaking.go index 0f69e96f46..666e930e9f 100644 --- a/cmd/buf/internal/command/breaking/breaking.go +++ b/cmd/buf/internal/command/breaking/breaking.go @@ -181,7 +181,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), bufctl.WithFileAnnotationsToStdout(), ) if err != nil { @@ -369,7 +368,7 @@ func run( } } if len(allFileAnnotations) > 0 { - allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...) + allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...) if err := bufanalysis.PrintFileAnnotationSet( container.Stdout(), allFileAnnotationSet, diff --git a/cmd/buf/internal/command/build/build.go b/cmd/buf/internal/command/build/build.go index ed6189cd2b..d2b1cbdbe7 100644 --- a/cmd/buf/internal/command/build/build.go +++ b/cmd/buf/internal/command/build/build.go @@ -152,7 +152,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/internal/command/convert/convert.go b/cmd/buf/internal/command/convert/convert.go index f9040cee05..62d0d20feb 100644 --- a/cmd/buf/internal/command/convert/convert.go +++ b/cmd/buf/internal/command/convert/convert.go @@ -174,7 +174,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/internal/command/curl/curl.go b/cmd/buf/internal/command/curl/curl.go index 2e857295aa..bd651b87fb 100644 --- a/cmd/buf/internal/command/curl/curl.go +++ b/cmd/buf/internal/command/curl/curl.go @@ -1334,12 +1334,7 @@ func completeURLFromSchema(ctx context.Context, schemas []string, baseURL, rawPa return nil, cobra.ShellCompDirectiveNoFileComp } // Discard log output during shell completion. - container := appext.NewContainer( - nameContainer, - slog.New(slog.NewTextHandler(io.Discard, nil)), - appext.LogLevelError, // Doesn't matter, logs are discarded - appext.LogFormatText, // Doesn't matter, logs are discarded - ) + container := appext.NewContainer(nameContainer, slog.New(slog.NewTextHandler(io.Discard, nil))) controller, err := bufcli.NewController(container) if err != nil { reportError("%v", err) diff --git a/cmd/buf/internal/command/dep/depgraph/depgraph.go b/cmd/buf/internal/command/dep/depgraph/depgraph.go index c82e873e98..27a53955f6 100644 --- a/cmd/buf/internal/command/dep/depgraph/depgraph.go +++ b/cmd/buf/internal/command/dep/depgraph/depgraph.go @@ -155,7 +155,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/internal/command/format/format.go b/cmd/buf/internal/command/format/format.go index f8ce13fe04..60818e9adc 100644 --- a/cmd/buf/internal/command/format/format.go +++ b/cmd/buf/internal/command/format/format.go @@ -296,7 +296,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/internal/command/generate/generate.go b/cmd/buf/internal/command/generate/generate.go index 3504d6c5a1..f8526e28ee 100644 --- a/cmd/buf/internal/command/generate/generate.go +++ b/cmd/buf/internal/command/generate/generate.go @@ -534,7 +534,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/internal/command/lint/lint.go b/cmd/buf/internal/command/lint/lint.go index 571c0ef70e..0ff63f17cc 100644 --- a/cmd/buf/internal/command/lint/lint.go +++ b/cmd/buf/internal/command/lint/lint.go @@ -120,7 +120,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(controllerErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), bufctl.WithFileAnnotationsToStdout(), ) if err != nil { @@ -173,7 +172,7 @@ func run( } } if len(allFileAnnotations) > 0 { - allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...) + allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...) if flags.ErrorFormat == "config-ignore-yaml" { if err := bufcli.PrintFileAnnotationSetLintConfigIgnoreYAMLV1( container.Stdout(), diff --git a/cmd/buf/internal/command/push/push.go b/cmd/buf/internal/command/push/push.go index d3ae2882b6..c89c2fd268 100644 --- a/cmd/buf/internal/command/push/push.go +++ b/cmd/buf/internal/command/push/push.go @@ -312,7 +312,6 @@ func getBuildableWorkspace( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return nil, err diff --git a/cmd/buf/internal/command/source/sourceedit/sourceeditdeprecate/sourceeditdeprecate.go b/cmd/buf/internal/command/source/sourceedit/sourceeditdeprecate/sourceeditdeprecate.go index 3afbc21e84..a516ad1688 100644 --- a/cmd/buf/internal/command/source/sourceedit/sourceeditdeprecate/sourceeditdeprecate.go +++ b/cmd/buf/internal/command/source/sourceedit/sourceeditdeprecate/sourceeditdeprecate.go @@ -183,7 +183,6 @@ func run( container, bufctl.WithDisableSymlinks(flags.DisableSymlinks), bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat), - bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor), ) if err != nil { return err diff --git a/cmd/buf/workspace_test.go b/cmd/buf/workspace_test.go index 6398555883..c75ef1030e 100644 --- a/cmd/buf/workspace_test.go +++ b/cmd/buf/workspace_test.go @@ -141,15 +141,11 @@ func TestWorkspaceDir(t *testing.T) { "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), ) - dirImportError := fmt.Sprintf( - "%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `request.Request` in this scope", - filepath.FromSlash("testdata/workspace/success/"+baseDirPath+"/proto/rpc.proto"), - ) testRunStdoutStderrNoWarn( t, nil, bufctl.ExitCodeFileAnnotation, - dirImportError, + filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), "", "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), @@ -160,7 +156,7 @@ func TestWorkspaceDir(t *testing.T) { t, nil, bufctl.ExitCodeFileAnnotation, - dirImportError, + filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), "", "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), @@ -370,16 +366,12 @@ 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( - "%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `request.Request` in this scope", - filepath.FromSlash("testdata/workspace/success/"+dirPath+"/proto/rpc.proto"), - ) testRunStdoutStderrNoWarn( t, nil, bufctl.ExitCodeFileAnnotation, ``, - detachedImportError, + filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), "build", filepath.Join("testdata", "workspace", "success", dirPath, "proto"), ) @@ -400,7 +392,7 @@ func TestWorkspaceDetached(t *testing.T) { t, nil, bufctl.ExitCodeFileAnnotation, - detachedImportError, + filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), ``, "lint", filepath.Join("testdata", "workspace", "success", dirPath, "proto"), diff --git a/cmd/buf/workspace_unix_test.go b/cmd/buf/workspace_unix_test.go index a89c9a0069..a059fa7116 100644 --- a/cmd/buf/workspace_unix_test.go +++ b/cmd/buf/workspace_unix_test.go @@ -17,7 +17,6 @@ package main import ( - "fmt" "path/filepath" "testing" @@ -27,24 +26,24 @@ import ( func TestWorkspaceSymlinkFail(t *testing.T) { t.Parallel() // The workspace includes a symlink that isn't buildable. - for _, dirPath := range []string{ - "symlink", - "v2/symlink", - } { - symlinkImportError := fmt.Sprintf( - "%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `c.C` in this scope", - filepath.FromSlash("testdata/workspace/fail/"+dirPath+"/b/b.proto"), - ) - testRunStdoutStderrNoWarn( - t, - nil, - bufctl.ExitCodeFileAnnotation, - ``, - symlinkImportError, - "build", - filepath.Join("testdata", "workspace", "fail", filepath.FromSlash(dirPath)), - ) - } + 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"), + ) } func TestWorkspaceSymlink(t *testing.T) { diff --git a/go.mod b/go.mod index 32d030909e..61ea834837 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,10 @@ 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 - buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda + buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75 buf.build/go/bufplugin v0.9.0 buf.build/go/bufprivateusage v0.1.0 buf.build/go/protovalidate v1.1.3 @@ -18,7 +17,7 @@ require ( connectrpc.com/connect v1.19.1 connectrpc.com/grpcreflect v1.3.0 connectrpc.com/otelconnect v0.9.0 - github.com/bufbuild/protocompile v0.14.2-0.20260410160725-0b5e706465a9 + github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1 github.com/cli/browser v1.3.0 github.com/gofrs/flock v0.13.0 diff --git a/go.sum b/go.sum index da44195116..a63244d42b 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,8 @@ buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-81 buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-819582968857.1/go.mod h1:1JJi9jvOqRxSMa+JxiZSm57doB+db/1WYCIa2lHfc40= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.11-20241007202033-cf42259fcbfc.1 h1:iGPvEJltOXUMANWf0zajcRcbiOXLD90ZwPUFvbcuv6Q= buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.11-20241007202033-cf42259fcbfc.1/go.mod h1:nWVKKRA29zdt4uvkjka3i/y4mkrswyWwiu0TbdX0zts= -buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda h1:eysSyjrJtkxU1A/9+Kv+1Mwq9K6BYBw+STIOVsZ256Y= -buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda/go.mod h1:V32mBaPWsfq6REAeZvvs/rQl7ZCl9Dn7eW1BBrmH0GQ= +buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75 h1:wb0B8XpzzfGIekqUXw4kUAvkroAiVmAg3uIpgo5S8sM= +buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75/go.mod h1:0XVOYemubVbxNXVY0DnsVgWeGkcbbAvjDa1fmhBC+Wo= buf.build/go/bufplugin v0.9.0 h1:ktZJNP3If7ldcWVqh46XKeiYJVPxHQxCfjzVQDzZ/lo= buf.build/go/bufplugin v0.9.0/go.mod h1:Z0CxA3sKQ6EPz/Os4kJJneeRO6CjPeidtP1ABh5jPPY= buf.build/go/bufprivateusage v0.1.0 h1:SzCoCcmzS3zyXHEXHeSQhGI7OTkgtljoknLzsUz9Gg4= @@ -42,8 +42,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.20260410160725-0b5e706465a9 h1:7oOAVXCKsQ0LkyC5cr/EYKADCJIJLRnstt2bpo/ilOk= -github.com/bufbuild/protocompile v0.14.2-0.20260410160725-0b5e706465a9/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU= +github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a h1:mtZZcNBraTXNE1e/UG7Odlw8314ucNtRgnB2muW0/cs= +github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a/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= diff --git a/private/buf/bufcli/protoc_plugin.go b/private/buf/bufcli/protoc_plugin.go index 838635013f..151555696b 100644 --- a/private/buf/bufcli/protoc_plugin.go +++ b/private/buf/bufcli/protoc_plugin.go @@ -125,8 +125,6 @@ func NewAppextContainerForPluginEnv( return appext.NewContainer( nameContainer, logger, - logLevel, - logFormat, ), nil } diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 225ce86e86..126fea3df4 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -22,7 +22,6 @@ import ( "io/fs" "log/slog" "net/http" - "os" "slices" "sort" @@ -54,7 +53,6 @@ import ( "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/wasm" - "golang.org/x/term" "google.golang.org/protobuf/proto" ) @@ -215,11 +213,10 @@ type controller struct { policyDataProvider bufpolicy.PolicyDataProvider wktStore bufwktstore.Store - disableSymlinks bool - fileAnnotationErrorFormat string - fileAnnotationsToStdout bool - colorizedFileAnnotationSetDiagnosticReport bool - copyToInMemory bool + disableSymlinks bool + fileAnnotationErrorFormat string + fileAnnotationsToStdout bool + copyToInMemory bool storageosProvider storageos.Provider buffetchRefParser buffetch.RefParser @@ -1368,26 +1365,15 @@ func (c *controller) handleFileAnnotationSetRetError(retErrAddr *error) { return } var fileAnnotationSet bufanalysis.FileAnnotationSet - var printDiagnosticReport bool if errors.As(*retErrAddr, &fileAnnotationSet) { writer := c.container.Stderr() if c.fileAnnotationsToStdout { writer = c.container.Stdout() - } else { - // When writing to stderr, check if the input is TTY, if so, allow printing the - // diagnostic report. - // - // HACK: We need to check os.Stderr rather than writer, since c.container.Stderr() - // returns wrapped writer, which only implements io.Writer. A fix needs to be made - // upstream in order to address this. - printDiagnosticReport = term.IsTerminal(int(os.Stderr.Fd())) } if err := bufanalysis.PrintFileAnnotationSet( writer, fileAnnotationSet, c.fileAnnotationErrorFormat, - bufanalysis.WithPrintDiagnosticReport(printDiagnosticReport), - bufanalysis.WithRenderColorizedDiagnosticReport(c.colorizedFileAnnotationSetDiagnosticReport), ); err != nil { *retErrAddr = err return diff --git a/private/buf/bufctl/option.go b/private/buf/bufctl/option.go index c0d1bbd520..74760444df 100644 --- a/private/buf/bufctl/option.go +++ b/private/buf/bufctl/option.go @@ -42,14 +42,6 @@ func WithFileAnnotationsToStdout() ControllerOption { } } -// WithColorizedFileAnnotationSetDiagnosticReport returns a new ControllerOptions that sets -// whether or not to render a colorized diagnostic report for the FileAnnotationSet. -func WithColorizedFileAnnotationSetDiagnosticReport(colorizedFileAnnotationSetDiagnosticReport bool) ControllerOption { - return func(controller *controller) { - controller.colorizedFileAnnotationSetDiagnosticReport = colorizedFileAnnotationSetDiagnosticReport - } -} - // WithCopyToInMemory returns a new ControllerOption that copies to memory. func WithCopyToInMemory() ControllerOption { return func(controller *controller) { diff --git a/private/buf/buflsp/buf_yaml_lsp_test.go b/private/buf/buflsp/buf_yaml_lsp_test.go index 3699072a69..3a92589a03 100644 --- a/private/buf/buflsp/buf_yaml_lsp_test.go +++ b/private/buf/buflsp/buf_yaml_lsp_test.go @@ -66,7 +66,7 @@ func setupLSPServerForBufYAML( nameContainer, err := appext.NewNameContainer(appContainer, "buf-test") require.NoError(t, err) - appextContainer := appext.NewContainer(nameContainer, logger, appext.LogLevelInfo, appext.LogFormatText) + appextContainer := appext.NewContainer(nameContainer, logger) tmpDir := t.TempDir() storageBucket, err := storageos.NewProvider().NewReadWriteBucket(tmpDir) diff --git a/private/buf/buflsp/buflsp_test.go b/private/buf/buflsp/buflsp_test.go index f8720f8b04..00ca91b086 100644 --- a/private/buf/buflsp/buflsp_test.go +++ b/private/buf/buflsp/buflsp_test.go @@ -68,7 +68,7 @@ func setupLSPServer( nameContainer, err := appext.NewNameContainer(appContainer, "buf-test") require.NoError(t, err) - appextContainer := appext.NewContainer(nameContainer, logger, appext.LogLevelInfo, appext.LogFormatText) + appextContainer := appext.NewContainer(nameContainer, logger) graphProvider := bufmodule.NopGraphProvider moduleDataProvider := bufmodule.NopModuleDataProvider diff --git a/private/buf/buflsp/diagnostics_test.go b/private/buf/buflsp/diagnostics_test.go index c355214501..de7fd0d681 100644 --- a/private/buf/buflsp/diagnostics_test.go +++ b/private/buf/buflsp/diagnostics_test.go @@ -63,7 +63,7 @@ func setupLSPServerWithDiagnostics( nameContainer, err := appext.NewNameContainer(appContainer, "buf-test") require.NoError(t, err) - appextContainer := appext.NewContainer(nameContainer, logger, appext.LogLevelInfo, appext.LogFormatText) + appextContainer := appext.NewContainer(nameContainer, logger) graphProvider := bufmodule.NopGraphProvider moduleDataProvider := bufmodule.NopModuleDataProvider diff --git a/private/bufpkg/bufanalysis/bufanalysis.go b/private/bufpkg/bufanalysis/bufanalysis.go index 2e75f8ee88..70ffb532ef 100644 --- a/private/bufpkg/bufanalysis/bufanalysis.go +++ b/private/bufpkg/bufanalysis/bufanalysis.go @@ -19,8 +19,6 @@ import ( "io" "strconv" "strings" - - "github.com/bufbuild/protocompile/experimental/report" ) const ( @@ -209,59 +207,18 @@ 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. -// -// 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) -} - -// PrintFileAnnotationSetOption is an option for printing the FileAnnotationSet. -type PrintFileAnnotationSetOption func(*printFileAnnotationSetOptions) - -// WithPrintDiagnosticReport returns a new PrintFileAnnotationSetOption that sets whether -// or not to print the diagnostic report. -// -// The diagnostic report is only use for text outputs. -func WithPrintDiagnosticReport(printDiagnosticReport bool) PrintFileAnnotationSetOption { - return func(options *printFileAnnotationSetOptions) { - options.printDiagnosticReport = printDiagnosticReport - } -} - -// WithRenderColorizedDiagnosticReport returns a new PrintFileAnnotationSetOption that sets -// whether or not to render a colorized diagnostic report. -// -// The diagnostic report is only use for text outputs. -func WithRenderColorizedDiagnosticReport(colorizedDiagnosticReport bool) PrintFileAnnotationSetOption { - return func(options *printFileAnnotationSetOptions) { - options.colorizedDiagnosticReport = colorizedDiagnosticReport - } +func NewFileAnnotationSet(fileAnnotations ...FileAnnotation) FileAnnotationSet { + return newFileAnnotationSet(fileAnnotations) } // PrintFileAnnotationSet prints the file annotations separated by newlines. -func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSet, formatString string, options ...PrintFileAnnotationSetOption) error { - opts := &printFileAnnotationSetOptions{} - for _, option := range options { - option(opts) - } - +func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSet, formatString string) error { format, err := ParseFormat(formatString) if err != nil { return err @@ -269,17 +226,6 @@ func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSe switch format { case FormatText: - if diagnosticReport := fileAnnotationSet.diagnosticReport(); diagnosticReport != nil && opts.printDiagnosticReport { - // TODO: There is a follow-up effort to organize the way compiler warnings are - // handled vs. lint/breaking change rules. For now, only render the errors and - // suppress compiler warnings. - diagnosticReport.Options.SuppressWarnings = true - renderer := report.Renderer{ - Colorize: opts.colorizedDiagnosticReport, - } - _, _, err := renderer.Render(diagnosticReport, writer) - return err - } return printAsText(writer, fileAnnotationSet.FileAnnotations()) case FormatJSON: return printAsJSON(writer, fileAnnotationSet.FileAnnotations()) @@ -295,8 +241,3 @@ func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSe return fmt.Errorf("unknown FileAnnotation Format: %v", format) } } - -type printFileAnnotationSetOptions struct { - printDiagnosticReport bool - colorizedDiagnosticReport bool -} diff --git a/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go b/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go index 6c867b91c0..93700df24e 100644 --- a/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go +++ b/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go @@ -49,7 +49,7 @@ func TestBasic(t *testing.T) { ), } sb := &strings.Builder{} - err := bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "text") + err := bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "text") require.NoError(t, err) assert.Equal( t, @@ -59,7 +59,7 @@ path/to/file.proto:2:1:Hello. (buf-plugin-foo) sb.String(), ) sb.Reset() - err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "json") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "json") require.NoError(t, err) assert.Equal( t, @@ -69,7 +69,7 @@ path/to/file.proto:2:1:Hello. (buf-plugin-foo) sb.String(), ) sb.Reset() - err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "msvs") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "msvs") require.NoError(t, err) assert.Equal(t, `path/to/file.proto(1,1) : error FOO : Hello. @@ -78,7 +78,7 @@ path/to/file.proto(2,1) : error FOO : Hello. (buf-plugin-foo) sb.String(), ) sb.Reset() - err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "junit") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "junit") require.NoError(t, err) assert.Equal(t, ` @@ -98,7 +98,6 @@ path/to/file.proto(2,1) : error FOO : Hello. (buf-plugin-foo) err = bufanalysis.PrintFileAnnotationSet( sb, bufanalysis.NewFileAnnotationSet( - nil, append( fileAnnotations, newFileAnnotation( @@ -136,7 +135,7 @@ path/to/file.proto(2,1) : error FOO : Hello. (buf-plugin-foo) sb.String(), ) sb.Reset() - err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "gitlab-code-quality") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "gitlab-code-quality") require.NoError(t, err) assert.Equal(t, `[{"description":"Hello.","check_name":"FOO","fingerprint":"7fa769d9df9f6db3b793316aa485c307df262ece452189a1c434e77480e9a6a26759f7616faf70c654639075b2fd170d3b66eef686ad402c72b550305883a7b7","location":{"path":"path/to/file.proto","positions":{"begin":{"line":1},"end":{"line":1}}},"severity":"minor"},{"description":"Hello.","check_name":"FOO","fingerprint":"60eab160b8308bb2c5fb823e200a54d58749513e2f2ad28447584f7223812f106a746ab7bf5fa5493e2e320163f86b46d098cb398f1795715617a825665e2a89","location":{"path":"path/to/file.proto","positions":{"begin":{"line":2,"column":1},"end":{"line":2,"column":1}}},"severity":"minor"}] diff --git a/private/bufpkg/bufanalysis/file_annotation_set.go b/private/bufpkg/bufanalysis/file_annotation_set.go index 3fb8fcc3f1..3b056a5fca 100644 --- a/private/bufpkg/bufanalysis/file_annotation_set.go +++ b/private/bufpkg/bufanalysis/file_annotation_set.go @@ -19,22 +19,18 @@ import ( "sort" "strconv" "strings" - - "github.com/bufbuild/protocompile/experimental/report" ) type fileAnnotationSet struct { fileAnnotations []FileAnnotation - report *report.Report } -func newFileAnnotationSet(diagnosticReport *report.Report, fileAnnotations []FileAnnotation) *fileAnnotationSet { +func newFileAnnotationSet(fileAnnotations []FileAnnotation) *fileAnnotationSet { if len(fileAnnotations) == 0 { return nil } return &fileAnnotationSet{ fileAnnotations: deduplicateAndSortFileAnnotations(fileAnnotations), - report: diagnosticReport, } } @@ -53,10 +49,6 @@ func (f *fileAnnotationSet) String() string { return sb.String() } -func (f *fileAnnotationSet) diagnosticReport() *report.Report { - return f.report -} - func (f *fileAnnotationSet) Error() string { return f.String() } diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/breaking.go b/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/breaking.go index 3965edd658..19a70e89a9 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/breaking.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/breaking.go @@ -1677,7 +1677,7 @@ func handleBreakingFieldSameDefault( withBackupLocation(field.DefaultLocation(), field.Location()), withBackupLocation(previousField.DefaultLocation(), previousField.Location()), field.File().Path(), - `%s changed default value from %v to %v.`, + `% changed default value from %v to %v.`, fieldDescription(field), previousDefault.printable, currentDefault.printable, diff --git a/private/bufpkg/bufcheck/client.go b/private/bufpkg/bufcheck/client.go index 40e55026b8..ea90ef0571 100644 --- a/private/bufpkg/bufcheck/client.go +++ b/private/bufpkg/bufcheck/client.go @@ -163,7 +163,6 @@ func (c *client) Lint( return nil } return bufanalysis.NewFileAnnotationSet( - nil, annotationsToFileAnnotations( imageToPathToExternalPath( image, @@ -303,7 +302,6 @@ func (c *client) Breaking( return nil } return bufanalysis.NewFileAnnotationSet( - nil, annotationsToFileAnnotations( imageToPathToExternalPath( image, diff --git a/private/bufpkg/bufimage/bufimageutil/bufimageutil.go b/private/bufpkg/bufimage/bufimageutil/bufimageutil.go index d2e095a02c..428ec47b09 100644 --- a/private/bufpkg/bufimage/bufimageutil/bufimageutil.go +++ b/private/bufpkg/bufimage/bufimageutil/bufimageutil.go @@ -107,7 +107,7 @@ func WithAllowIncludeOfImportedType() ImageFilterOption { // For example, "google.protobuf.Any" or "buf.validate". Types may be nested, // and can be any package, message, enum, extension, service or method name. // -// If the type does not exist in the image, an error wrapping +// If the type does not exist in the image, an error wrapping // [ErrImageFilterTypeNotFound] will be returned. func WithIncludeTypes(typeNames ...string) ImageFilterOption { return func(opts *imageFilterOptions) { diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Bar.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Bar.txtar index 01af50974b..986e21e26d 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Bar.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Bar.txtar @@ -135,28 +135,28 @@ enum Quz { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -205,28 +205,28 @@ enum Quz { 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -288,13 +288,12 @@ enum Quz { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -302,12 +301,13 @@ enum Quz { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -394,13 +394,12 @@ enum Quz { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -408,12 +407,13 @@ enum Quz { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -463,28 +463,28 @@ enum Quz { 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ @@ -562,28 +562,28 @@ enum Quz { 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ @@ -618,28 +618,28 @@ enum Quz { 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ @@ -702,28 +702,28 @@ enum Quz { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ @@ -769,28 +769,28 @@ enum Quz { 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar index 4495a519df..40f13b8cbc 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar @@ -100,28 +100,28 @@ enum Baz { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar index 707bdf89d3..ced7534362 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar @@ -143,28 +143,28 @@ service Svc { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -213,28 +213,28 @@ service Svc { 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -296,13 +296,12 @@ service Svc { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -310,12 +309,13 @@ service Svc { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -402,13 +402,12 @@ service Svc { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -416,12 +415,13 @@ service Svc { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -471,28 +471,28 @@ service Svc { 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ @@ -570,28 +570,28 @@ service Svc { 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ @@ -626,28 +626,28 @@ service Svc { 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ @@ -710,28 +710,28 @@ service Svc { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ @@ -777,28 +777,28 @@ service Svc { 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar index 5b467b10f0..dd4025a192 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar @@ -435,6 +435,18 @@ extend google.protobuf.MessageOptions { ], "leadingComments": " Keep if ext: comment on extend block\n" }, + { + "path": [ + 7, + 0 + ], + "span": [ + 21, + 2, + 33 + ], + "leadingComments": " Keep if ext: comment on custom option bizniz\n" + }, { "path": [ 7, @@ -459,18 +471,6 @@ extend google.protobuf.MessageOptions { 10 ] }, - { - "path": [ - 7, - 0 - ], - "span": [ - 21, - 2, - 33 - ], - "leadingComments": " Keep if ext: comment on custom option bizniz\n" - }, { "path": [ 7, @@ -579,28 +579,28 @@ extend google.protobuf.MessageOptions { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -649,28 +649,28 @@ extend google.protobuf.MessageOptions { 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -732,13 +732,12 @@ extend google.protobuf.MessageOptions { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -746,12 +745,13 @@ extend google.protobuf.MessageOptions { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -838,13 +838,12 @@ extend google.protobuf.MessageOptions { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -852,12 +851,13 @@ extend google.protobuf.MessageOptions { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -955,14 +955,14 @@ extend google.protobuf.MessageOptions { 3, 0, 6, - 0, - 2 + 0 ], "span": [ - 84, - 11, - 14 - ] + 86, + 6, + 32 + ], + "leadingComments": " Keep if Foo + ext: comment on extension blah\n" }, { "path": [ @@ -972,11 +972,11 @@ extend google.protobuf.MessageOptions { 0, 6, 0, - 4 + 2 ], "span": [ - 86, - 6, + 84, + 11, 14 ] }, @@ -987,14 +987,14 @@ extend google.protobuf.MessageOptions { 3, 0, 6, - 0 + 0, + 4 ], "span": [ 86, 6, - 32 - ], - "leadingComments": " Keep if Foo + ext: comment on extension blah\n" + 14 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar index c81b682321..9e8eff80b9 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar @@ -109,28 +109,28 @@ message Foo { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -179,28 +179,28 @@ message Foo { 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -262,13 +262,12 @@ message Foo { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -276,12 +275,13 @@ message Foo { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -368,13 +368,12 @@ message Foo { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -382,12 +381,13 @@ message Foo { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar index fdcdd45ba3..3c83914b74 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar @@ -325,14 +325,14 @@ message Foo { 3, 0, 2, - 0, - 4 + 0 ], "span": [ 69, 4, - 12 - ] + 28 + ], + "leadingComments": " Keep if NestedFoo: comment on field uid\n" }, { "path": [ @@ -341,14 +341,14 @@ message Foo { 3, 0, 2, - 0 + 0, + 4 ], "span": [ 69, 4, - 28 - ], - "leadingComments": " Keep if NestedFoo: comment on field uid\n" + 12 + ] }, { "path": [ @@ -405,14 +405,14 @@ message Foo { 3, 0, 2, - 1, - 4 + 1 ], "span": [ 71, 4, - 12 - ] + 42 + ], + "leadingComments": " Keep if NestedFoo: comment on field meta\n" }, { "path": [ @@ -421,14 +421,14 @@ message Foo { 3, 0, 2, - 1 + 1, + 4 ], "span": [ 71, 4, - 42 - ], - "leadingComments": " Keep if NestedFoo: comment on field meta\n" + 12 + ] }, { "path": [ @@ -485,14 +485,14 @@ message Foo { 3, 0, 2, - 2, - 4 + 2 ], "span": [ 73, 4, - 12 - ] + 29 + ], + "leadingComments": " Keep if NestedFoo: comment on field state\n" }, { "path": [ @@ -501,14 +501,14 @@ message Foo { 3, 0, 2, - 2 + 2, + 4 ], "span": [ 73, 4, - 29 - ], - "leadingComments": " Keep if NestedFoo: comment on field state\n" + 12 + ] }, { "path": [ @@ -600,14 +600,14 @@ message Foo { 4, 0, 2, - 0, - 1 + 0 ], "span": [ 78, 6, - 23 - ] + 28 + ], + "leadingComments": " Keep if NestedFoo: comment on enum value STATE_UNSPECIFIED\n" }, { "path": [ @@ -618,14 +618,14 @@ message Foo { 4, 0, 2, - 0 + 0, + 1 ], "span": [ 78, 6, - 28 - ], - "leadingComments": " Keep if NestedFoo: comment on enum value STATE_UNSPECIFIED\n" + 23 + ] }, { "path": [ @@ -654,14 +654,14 @@ message Foo { 4, 0, 2, - 1, 1 ], "span": [ 80, 6, - 16 - ] + 21 + ], + "leadingComments": " Keep if NestedFoo: comment on enum value STATE_GOOD\n" }, { "path": [ @@ -672,14 +672,14 @@ message Foo { 4, 0, 2, + 1, 1 ], "span": [ 80, 6, - 21 - ], - "leadingComments": " Keep if NestedFoo: comment on enum value STATE_GOOD\n" + 16 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar index 2dea92d579..deee0b85bb 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar @@ -95,28 +95,28 @@ enum Quz { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar index 31b8302205..7901a8ab25 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar @@ -145,28 +145,28 @@ service Svc { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -215,28 +215,28 @@ service Svc { 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -298,13 +298,12 @@ service Svc { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -312,12 +311,13 @@ service Svc { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -404,13 +404,12 @@ service Svc { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -418,12 +417,13 @@ service Svc { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -473,28 +473,28 @@ service Svc { 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ @@ -572,28 +572,28 @@ service Svc { 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ @@ -628,28 +628,28 @@ service Svc { 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ @@ -712,28 +712,28 @@ service Svc { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ @@ -779,28 +779,28 @@ service Svc { 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar index 19af1f218f..49ed048f96 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar @@ -717,6 +717,18 @@ extend google.protobuf.ServiceOptions { ], "leadingComments": " Keep if ext: comment on extend block\n" }, + { + "path": [ + 7, + 0 + ], + "span": [ + 21, + 2, + 33 + ], + "leadingComments": " Keep if ext: comment on custom option bizniz\n" + }, { "path": [ 7, @@ -741,18 +753,6 @@ extend google.protobuf.ServiceOptions { 10 ] }, - { - "path": [ - 7, - 0 - ], - "span": [ - 21, - 2, - 33 - ], - "leadingComments": " Keep if ext: comment on custom option bizniz\n" - }, { "path": [ 7, @@ -801,6 +801,18 @@ extend google.protobuf.ServiceOptions { ], "leadingComments": " Keep if ext: comment on extend block\n" }, + { + "path": [ + 7, + 1 + ], + "span": [ + 27, + 2, + 35 + ], + "leadingComments": " Keep if ext + Svc: comment on custom option fizzbuzz\n" + }, { "path": [ 7, @@ -825,18 +837,6 @@ extend google.protobuf.ServiceOptions { 10 ] }, - { - "path": [ - 7, - 1 - ], - "span": [ - 27, - 2, - 35 - ], - "leadingComments": " Keep if ext + Svc: comment on custom option fizzbuzz\n" - }, { "path": [ 7, @@ -903,28 +903,28 @@ extend google.protobuf.ServiceOptions { 4, 0, 2, - 0, - 4 + 0 ], "span": [ 33, 2, - 10 - ] + 25 + ], + "leadingComments": " Keep if all: comment on field xyz\n" }, { "path": [ 4, 0, 2, - 0 + 0, + 4 ], "span": [ 33, 2, - 25 - ], - "leadingComments": " Keep if all: comment on field xyz\n" + 10 + ] }, { "path": [ @@ -1028,28 +1028,28 @@ extend google.protobuf.ServiceOptions { 4, 1, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ 4, 1, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ @@ -1098,28 +1098,28 @@ extend google.protobuf.ServiceOptions { 4, 1, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ 4, 1, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ @@ -1181,13 +1181,12 @@ extend google.protobuf.ServiceOptions { 4, 1, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -1195,12 +1194,13 @@ extend google.protobuf.ServiceOptions { 4, 1, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -1287,13 +1287,12 @@ extend google.protobuf.ServiceOptions { 4, 1, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -1301,12 +1300,13 @@ extend google.protobuf.ServiceOptions { 4, 1, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -1390,14 +1390,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 0, - 4 + 0 ], "span": [ 69, 4, - 12 - ] + 28 + ], + "leadingComments": " Keep if NestedFoo: comment on field uid\n" }, { "path": [ @@ -1406,14 +1406,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 0 + 0, + 4 ], "span": [ 69, 4, - 28 - ], - "leadingComments": " Keep if NestedFoo: comment on field uid\n" + 12 + ] }, { "path": [ @@ -1470,14 +1470,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 1, - 4 + 1 ], "span": [ 71, 4, - 12 - ] + 42 + ], + "leadingComments": " Keep if NestedFoo: comment on field meta\n" }, { "path": [ @@ -1486,14 +1486,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 1 + 1, + 4 ], "span": [ 71, 4, - 42 - ], - "leadingComments": " Keep if NestedFoo: comment on field meta\n" + 12 + ] }, { "path": [ @@ -1550,14 +1550,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 2, - 4 + 2 ], "span": [ 73, 4, - 12 - ] + 29 + ], + "leadingComments": " Keep if NestedFoo: comment on field state\n" }, { "path": [ @@ -1566,14 +1566,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 2, - 2 + 2, + 4 ], "span": [ 73, 4, - 29 - ], - "leadingComments": " Keep if NestedFoo: comment on field state\n" + 12 + ] }, { "path": [ @@ -1665,14 +1665,14 @@ extend google.protobuf.ServiceOptions { 4, 0, 2, - 0, - 1 + 0 ], "span": [ 78, 6, - 23 - ] + 28 + ], + "leadingComments": " Keep if NestedFoo: comment on enum value STATE_UNSPECIFIED\n" }, { "path": [ @@ -1683,14 +1683,14 @@ extend google.protobuf.ServiceOptions { 4, 0, 2, - 0 + 0, + 1 ], "span": [ 78, 6, - 28 - ], - "leadingComments": " Keep if NestedFoo: comment on enum value STATE_UNSPECIFIED\n" + 23 + ] }, { "path": [ @@ -1719,14 +1719,14 @@ extend google.protobuf.ServiceOptions { 4, 0, 2, - 1, 1 ], "span": [ 80, 6, - 16 - ] + 21 + ], + "leadingComments": " Keep if NestedFoo: comment on enum value STATE_GOOD\n" }, { "path": [ @@ -1737,14 +1737,14 @@ extend google.protobuf.ServiceOptions { 4, 0, 2, + 1, 1 ], "span": [ 80, 6, - 21 - ], - "leadingComments": " Keep if NestedFoo: comment on enum value STATE_GOOD\n" + 16 + ] }, { "path": [ @@ -1787,14 +1787,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 6, - 0, - 2 + 0 ], "span": [ - 84, - 11, - 14 - ] + 86, + 6, + 32 + ], + "leadingComments": " Keep if Foo + ext: comment on extension blah\n" }, { "path": [ @@ -1804,11 +1804,11 @@ extend google.protobuf.ServiceOptions { 0, 6, 0, - 4 + 2 ], "span": [ - 86, - 6, + 84, + 11, 14 ] }, @@ -1819,14 +1819,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 6, - 0 + 0, + 4 ], "span": [ 86, 6, - 32 - ], - "leadingComments": " Keep if Foo + ext: comment on extension blah\n" + 14 + ] }, { "path": [ @@ -1909,28 +1909,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ 4, 2, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ @@ -2008,28 +2008,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ 4, 2, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ @@ -2064,28 +2064,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ 4, 2, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ @@ -2148,28 +2148,28 @@ extend google.protobuf.ServiceOptions { 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ @@ -2215,28 +2215,28 @@ extend google.protobuf.ServiceOptions { 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ @@ -2282,28 +2282,28 @@ extend google.protobuf.ServiceOptions { 4, 3, 2, - 0, - 4 + 0 ], "span": [ 125, 2, - 10 - ] + 24 + ], + "leadingComments": " Keep if all: comment on field s\n" }, { "path": [ 4, 3, 2, - 0 + 0, + 4 ], "span": [ 125, 2, - 24 - ], - "leadingComments": " Keep if all: comment on field s\n" + 10 + ] }, { "path": [ @@ -2377,28 +2377,28 @@ extend google.protobuf.ServiceOptions { 4, 4, 2, - 0, - 4 + 0 ], "span": [ 131, 2, - 10 - ] + 24 + ], + "leadingComments": " Keep if all: comment on field t\n" }, { "path": [ 4, 4, 2, - 0 + 0, + 4 ], "span": [ 131, 2, - 24 - ], - "leadingComments": " Keep if all: comment on field t\n" + 10 + ] }, { "path": [ diff --git a/private/bufpkg/bufimage/build_image.go b/private/bufpkg/bufimage/build_image.go index ffb5a27190..8050511ede 100644 --- a/private/bufpkg/bufimage/build_image.go +++ b/private/bufpkg/bufimage/build_image.go @@ -19,24 +19,23 @@ import ( "errors" "fmt" "log/slog" + "math" + "strings" - descriptorv1 "buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go/buf/descriptor/v1" "buf.build/go/standard/xlog/xslog" - "buf.build/go/standard/xslices" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufprotocompile" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/thread" - "github.com/bufbuild/protocompile/experimental/fdp" - "github.com/bufbuild/protocompile/experimental/incremental" - "github.com/bufbuild/protocompile/experimental/incremental/queries" - "github.com/bufbuild/protocompile/experimental/ir" - "github.com/bufbuild/protocompile/experimental/report" - "github.com/bufbuild/protocompile/experimental/source" - "github.com/bufbuild/protocompile/experimental/source/length" - "google.golang.org/protobuf/reflect/protodesc" - "google.golang.org/protobuf/types/descriptorpb" + "github.com/bufbuild/protocompile" + "github.com/bufbuild/protocompile/linker" + "github.com/bufbuild/protocompile/parser" + "github.com/bufbuild/protocompile/protoutil" + "github.com/bufbuild/protocompile/reporter" + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/types/dynamicpb" ) @@ -50,243 +49,283 @@ func buildImage( defer xslog.DebugProfile(logger)() if !moduleReadBucket.ShouldBeSelfContained() { - return nil, syserror.New( - "passed a ModuleReadBucket to BuildImage that was not expected to be self-contained", - ) + return nil, syserror.New("passed a ModuleReadBucket to BuildImage that was not expected to be self-contained") } - + moduleReadBucket = bufmodule.ModuleReadBucketWithOnlyProtoFiles(moduleReadBucket) + parserAccessorHandler := newParserAccessorHandler(ctx, moduleReadBucket) targetFileInfos, err := bufmodule.GetTargetFileInfos(ctx, moduleReadBucket) if err != nil { return nil, err } - if len(targetFileInfos) == 0 { - // If we had no target files within the module after path filtering, this is an error. + // If we had no no target files within the module after path filtering, this is an error. // We could have a better user error than this. This gets back to the lack of allowNotExist. return nil, bufmodule.ErrNoTargetProtoFiles } + paths := bufmodule.FileInfoPaths(targetFileInfos) - image, err := compileImage( + buildResult := getBuildResult( ctx, - bufmodule.ModuleReadBucketWithOnlyProtoFiles(moduleReadBucket), - bufmodule.FileInfoPaths(targetFileInfos), + parserAccessorHandler, + paths, excludeSourceCodeInfo, noParallelism, ) + if buildResult.Err != nil { + return nil, buildResult.Err + } + sortedFiles, err := checkAndSortFiles(buildResult.Files, paths) + if err != nil { + return nil, err + } + image, err := getImage( + ctx, + excludeSourceCodeInfo, + sortedFiles, + buildResult.Symbols, + parserAccessorHandler, + buildResult.SyntaxUnspecifiedFilenames, + buildResult.FilenameToUnusedDependencyFilenames, + ) if err != nil { return nil, err } - return image, nil } -// compileImage compiles the [Image] for the given [bufmodule.ModuleReadBucket]. -func compileImage( +func getBuildResult( ctx context.Context, - moduleReadBucket bufmodule.ModuleReadBucket, + parserAccessorHandler *parserAccessorHandler, paths []string, excludeSourceCodeInfo bool, noParallelism bool, -) (Image, error) { - session := new(ir.Session) - moduleFileResolver := newModuleFileResolver(ctx, moduleReadBucket) - +) *buildResult { + var errorsWithPos []reporter.ErrorWithPos + var warningErrorsWithPos []reporter.ErrorWithPos + // With "extra option locations", buf can include more comments + // for an option value than protoc can. In particular, this allows + // it to preserve comments inside of message literals. + sourceInfoMode := protocompile.SourceInfoExtraOptionLocations + if excludeSourceCodeInfo { + sourceInfoMode = protocompile.SourceInfoNone + } parallelism := thread.Parallelism() if noParallelism { parallelism = 1 } - - exec := incremental.New( - incremental.WithParallelism(int64(parallelism)), - ) - results, diagnostics, err := incremental.Run( - ctx, - exec, - queries.Link{ - Opener: moduleFileResolver, - Session: session, - Workspace: source.NewWorkspace(paths...), - }, - ) - if err != nil { - return nil, err + symbols := &linker.Symbols{} + compiler := protocompile.Compiler{ + MaxParallelism: parallelism, + SourceInfoMode: sourceInfoMode, + Resolver: &protocompile.SourceResolver{Accessor: parserAccessorHandler.Open}, + Symbols: symbols, + Reporter: reporter.NewReporter( + func(errorWithPos reporter.ErrorWithPos) error { + errorsWithPos = append(errorsWithPos, errorWithPos) + return nil + }, + func(errorWithPos reporter.ErrorWithPos) { + warningErrorsWithPos = append(warningErrorsWithPos, errorWithPos) + }, + ), } - - var fileAnnotations []bufanalysis.FileAnnotation - for _, diagnostic := range diagnostics.Diagnostics { - primary := diagnostic.Primary() - if primary.IsZero() || diagnostic.Level() > report.Error { - // We only surface [report.Error] level or more sever diagnostics as build errors. - // Warnings will still be rendered in the diagnostics report if errors are found, - // but if there are no errors, then the warnings are not surfaced to the user. - // - // In the future, we should handle warnings and other checks in a unified way. - continue + // fileDescriptors are in the same order as paths per the documentation + compiledFiles, err := compiler.Compile(ctx, paths...) + if err != nil { + if err == reporter.ErrInvalidSource { + if len(errorsWithPos) == 0 { + return newFailedBuildResult( + errors.New("got invalid source error from parse but no errors reported"), + ) + } + fileAnnotationSet, err := bufprotocompile.FileAnnotationSetForErrorsWithPos( + errorsWithPos, + bufprotocompile.WithExternalPathResolver(parserAccessorHandler.ExternalPath), + ) + if err != nil { + return newFailedBuildResult(err) + } + return newFailedBuildResult(fileAnnotationSet) } - start := primary.Location(primary.Start, length.Bytes) - end := primary.Location(primary.End, length.Bytes) - - // We resolve the path and external path using moduleFileResolver, since the span - // uses the path set by moduleFileResolver, which is the moduleFile.LocalPath(). - path := moduleFileResolver.PathForLocalPath(primary.Path()) - if path == "" { - // If there is no path, fallback to using the path from the diagnostic span directly. - path = primary.Path() + if errorWithPos, ok := err.(reporter.ErrorWithPos); ok { + fileAnnotation, err := bufprotocompile.FileAnnotationForErrorWithPos( + errorWithPos, + bufprotocompile.WithExternalPathResolver(parserAccessorHandler.ExternalPath), + ) + if err != nil { + return newFailedBuildResult(err) + } + return newFailedBuildResult(bufanalysis.NewFileAnnotationSet(fileAnnotation)) } - fileAnnotations = append( - fileAnnotations, - bufanalysis.NewFileAnnotation( - &fileInfo{ - path: path, - externalPath: moduleFileResolver.ExternalPath(path), - }, - start.Line, - start.Column, - end.Line, - end.Column, - "COMPILE", - diagnostic.Message(), - "", // pluginName - "", // policyName - ), + return newFailedBuildResult(err) + } else if len(errorsWithPos) > 0 { + // https://github.com/jhump/protoreflect/pull/331 + return newFailedBuildResult( + errors.New("got no error from parse but errors reported"), ) } - if len(fileAnnotations) > 0 { - return nil, bufanalysis.NewFileAnnotationSet(diagnostics, fileAnnotations...) - } - - // Validate that there is a single result for all files - if len(results) != 1 { - return nil, fmt.Errorf("expected a single result from query, instead got: %d", len(results)) - } - - if results[0].Fatal != nil { - return nil, results[0].Fatal + if len(compiledFiles) != len(paths) { + return newFailedBuildResult( + fmt.Errorf("expected FileDescriptors to be of length %d but was %d", len(paths), len(compiledFiles)), + ) } - irFiles := results[0].Value - - // Validate that all files have completed lowering. If not, then symbols have not been - // properly resolved and cannot generate a valid file descriptor set. - for _, irFile := range irFiles { - if !irFile.Lowered() { - return nil, fmt.Errorf(`The symbols for file %q have not been fully resolved due to an invalid version of descriptor.proto, located at %q. This is likely due to a vendored descriptor.proto.`, - moduleFileResolver.ExternalPath(irFile.Path()), - moduleFileResolver.ExternalPath("google/protobuf/descriptor.proto"), + for i, fileDescriptor := range compiledFiles { + path := paths[i] + filename := fileDescriptor.Path() + // doing another rough verification + // NO LONGER NEED TO DO SUFFIX SINCE WE KNOW THE ROOT FILE NAME + if path != filename { + return newFailedBuildResult( + fmt.Errorf("expected fileDescriptor name %s to be a equal to %s", filename, path), ) } } - - bytes, err := fdp.DescriptorSetBytes( - irFiles, - // When compiling the [descriptorpb.FileDescriptorSet], we always include the source - // code info to get [descriptorv1.E_BufSourceCodeInfoExtension] information. The source - // code info may still be excluded from the final [Image] based on the options passed in. - fdp.IncludeSourceCodeInfo(true), - // This is needed for lint and breaking change detection annotations. - fdp.GenerateExtraOptionLocations(true), - ) - if err != nil { - return nil, err + syntaxUnspecifiedFilenames := make(map[string]struct{}) + filenameToUnusedDependencyFilenames := make(map[string]map[string]struct{}) + for _, warningErrorWithPos := range warningErrorsWithPos { + maybeAddSyntaxUnspecified(syntaxUnspecifiedFilenames, warningErrorWithPos) + maybeAddUnusedImport(filenameToUnusedDependencyFilenames, warningErrorWithPos) } + return newBuildResult( + compiledFiles, + symbols, + syntaxUnspecifiedFilenames, + filenameToUnusedDependencyFilenames, + ) +} - // First unmarshal to get the descriptors - fds := new(descriptorpb.FileDescriptorSet) - if err := protoencoding.NewWireUnmarshaler(nil).Unmarshal(bytes, fds); err != nil { - return nil, err +// We need to sort the files as they may/probably are out of order +// relative to input order after concurrent builds. This mimics the output +// order of protoc. +func checkAndSortFiles( + fileDescriptors linker.Files, + rootRelFilePaths []string, +) (linker.Files, error) { + if len(fileDescriptors) != len(rootRelFilePaths) { + return nil, fmt.Errorf("rootRelFilePath length was %d but FileDescriptor length was %d", len(rootRelFilePaths), len(fileDescriptors)) } - - // Create a resolver from the descriptors so extensions can be recognized. - // Specifically, we need to ensure that we are able to resolve the Buf-specific descriptor - // extensions for propagating compiler errors. In the future, we should have better - // integration with [report.Report] to handle warnings. - resolverFiles := []*descriptorpb.FileDescriptorProto{ - protodesc.ToFileDescriptorProto(descriptorv1.File_buf_descriptor_v1_descriptor_proto), + nameToFileDescriptor := make(map[string]linker.File, len(fileDescriptors)) + for _, fileDescriptor := range fileDescriptors { + name := fileDescriptor.Path() + if name == "" { + return nil, errors.New("no name on FileDescriptor") + } + if _, ok := nameToFileDescriptor[name]; ok { + return nil, fmt.Errorf("duplicate FileDescriptor: %s", name) + } + nameToFileDescriptor[name] = fileDescriptor } - resolverFiles = append(resolverFiles, fds.File...) - resolver := protoencoding.NewLazyResolver(resolverFiles...) - - // Reparse extensions with the resolver in all FileDescriptorProtos to convert unknown - // fields into recognized extensions - for _, fileDescriptor := range fds.File { - if err := protoencoding.ReparseExtensions(resolver, fileDescriptor.ProtoReflect()); err != nil { - return nil, err + // We now know that all FileDescriptors had unique names and the number of FileDescriptors + // is equal to the number of rootRelFilePaths. We also verified earlier that rootRelFilePaths + // has only unique values. Now we can put them in order. + sortedFileDescriptors := make(linker.Files, 0, len(fileDescriptors)) + for _, rootRelFilePath := range rootRelFilePaths { + fileDescriptor, ok := nameToFileDescriptor[rootRelFilePath] + if !ok { + return nil, fmt.Errorf("no FileDescriptor for rootRelFilePath: %q", rootRelFilePath) } + sortedFileDescriptors = append(sortedFileDescriptors, fileDescriptor) } - - return fileDescriptorSetToImage(resolver, moduleFileResolver, paths, fds, excludeSourceCodeInfo) + return sortedFileDescriptors, nil } -// fileDescriptorSetToImage is a helper function that converts a [descriptorpb.FileDescriptorSet] -// to an [Image], preserving the order of the paths based on the module paths. +// getImage gets the Image for the protoreflect.FileDescriptors. // -// Note that this iterates through the given paths and constructs the the [ImageFile]s -// based on that rather than using the file descriptor set compiled through the compiler. -// This is because there is a difference in the topological sorting algo used in the -// compiler vs. expected protoc ordering, and so for conformance reasons, we reconstruct -// the ordering of the [ImageFile]s. -func fileDescriptorSetToImage( - resolver protoencoding.Resolver, - moduleFileResolver *moduleFileResolver, - paths []string, - fds *descriptorpb.FileDescriptorSet, +// This mimics protoc's output order. +// This assumes checkAndSortFiles was called. +func getImage( + ctx context.Context, excludeSourceCodeInfo bool, + sortedFiles linker.Files, + symbols *linker.Symbols, + parserAccessorHandler *parserAccessorHandler, + syntaxUnspecifiedFilenames map[string]struct{}, + filenameToUnusedDependencyFilenames map[string]map[string]struct{}, ) (Image, error) { - pathToDescriptor := make(map[string]*descriptorpb.FileDescriptorProto) - for _, fileDescriptor := range fds.File { - pathToDescriptor[fileDescriptor.GetName()] = fileDescriptor + // if we aren't including imports, then we need a set of file names that + // are included so we can create a topologically sorted list w/out + // including imports that should not be present. + // + // if we are including imports, then we need to know what filenames + // are imports are what filenames are not + // all input protoreflect.FileDescriptors are not imports, we derive the imports + // from GetDependencies. + nonImportFilenames := map[string]struct{}{} + for _, fileDescriptor := range sortedFiles { + nonImportFilenames[fileDescriptor.Path()] = struct{}{} } var imageFiles []ImageFile var err error - seen := make(map[string]struct{}) - nonImportPaths := xslices.ToStructMap(paths) - - for _, path := range paths { - imageFiles, err = getImageFilesForPath( - path, - pathToDescriptor, - moduleFileResolver, + alreadySeen := map[string]struct{}{} + for _, fileDescriptor := range sortedFiles { + imageFiles, err = getImageFilesRec( + ctx, excludeSourceCodeInfo, - seen, - nonImportPaths, + fileDescriptor, + parserAccessorHandler, + syntaxUnspecifiedFilenames, + filenameToUnusedDependencyFilenames, + alreadySeen, + nonImportFilenames, imageFiles, ) - if err != nil { return nil, err } } - - return newImage(imageFiles, false, resolver) + return newImage(imageFiles, false, newResolverForFiles(sortedFiles, symbols)) } -func getImageFilesForPath( - path string, - pathToDescriptor map[string]*descriptorpb.FileDescriptorProto, - moduleFileResolver *moduleFileResolver, +func getImageFilesRec( + ctx context.Context, excludeSourceCodeInfo bool, - seen map[string]struct{}, + fileDescriptor protoreflect.FileDescriptor, + parserAccessorHandler *parserAccessorHandler, + syntaxUnspecifiedFilenames map[string]struct{}, + filenameToUnusedDependencyFilenames map[string]map[string]struct{}, + alreadySeen map[string]struct{}, nonImportFilenames map[string]struct{}, imageFiles []ImageFile, ) ([]ImageFile, error) { - fileDescriptor := pathToDescriptor[path] if fileDescriptor == nil { return nil, errors.New("nil FileDescriptor") } - - if _, ok := seen[path]; ok { + path := fileDescriptor.Path() + if _, ok := alreadySeen[path]; ok { return imageFiles, nil } - seen[path] = struct{}{} + alreadySeen[path] = struct{}{} + unusedDependencyFilenames, ok := filenameToUnusedDependencyFilenames[path] + var unusedDependencyIndexes []int32 + if ok { + unusedDependencyIndexes = make([]int32, 0, len(unusedDependencyFilenames)) + } var err error - for _, dependency := range fileDescriptor.Dependency { - imageFiles, err = getImageFilesForPath( - dependency, - pathToDescriptor, - moduleFileResolver, + for i := range fileDescriptor.Imports().Len() { + dependency := fileDescriptor.Imports().Get(i).FileDescriptor + if unusedDependencyFilenames != nil { + if _, ok := unusedDependencyFilenames[dependency.Path()]; ok { + // This should never happen, however we do a bounds check to ensure that we are + // doing a safe conversion for the index. + if i > math.MaxInt32 || i < math.MinInt32 { + return nil, fmt.Errorf("unused dependency index out-of-bounds for int32 conversion: %v", i) + } + unusedDependencyIndexes = append( + unusedDependencyIndexes, + int32(i), + ) + } + } + imageFiles, err = getImageFilesRec( + ctx, excludeSourceCodeInfo, - seen, + dependency, + parserAccessorHandler, + syntaxUnspecifiedFilenames, + filenameToUnusedDependencyFilenames, + alreadySeen, nonImportFilenames, imageFiles, ) @@ -295,13 +334,26 @@ func getImageFilesForPath( } } + fileDescriptorProto := protoutil.ProtoFromFileDescriptor(fileDescriptor) + if fileDescriptorProto == nil { + return nil, errors.New("nil FileDescriptorProto") + } + if excludeSourceCodeInfo { + // need to do this anyways as Parser does not respect this for FileDescriptorProtos + fileDescriptorProto.SourceCodeInfo = nil + } _, isNotImport := nonImportFilenames[path] - - imageFile, err := fileDescriptorProtoToImageFile( - moduleFileResolver, - fileDescriptor, - excludeSourceCodeInfo, + _, syntaxUnspecified := syntaxUnspecifiedFilenames[path] + imageFile, err := NewImageFile( + fileDescriptorProto, + parserAccessorHandler.FullName(path), + parserAccessorHandler.CommitID(path), + // if empty, defaults to path + parserAccessorHandler.ExternalPath(path), + parserAccessorHandler.LocalPath(path), !isNotImport, + syntaxUnspecified, + unusedDependencyIndexes, ) if err != nil { return nil, err @@ -309,53 +361,57 @@ func getImageFilesForPath( return append(imageFiles, imageFile), nil } -// fileDescriptorProtoToImageFile is a helper function that converts a [descriptorpb.FileDescriptorProto] -// to an [ImageFile]. -func fileDescriptorProtoToImageFile( - moduleFileResolver *moduleFileResolver, - fileDescriptor *descriptorpb.FileDescriptorProto, - excludeSourceCodeInfo bool, - isImport bool, -) (ImageFile, error) { - var ( - isSyntaxUnspecified bool - unusedDependencyIndexes []int32 - ) +func maybeAddSyntaxUnspecified( + syntaxUnspecifiedFilenames map[string]struct{}, + errorWithPos reporter.ErrorWithPos, +) { + if errorWithPos.Unwrap() != parser.ErrNoSyntax { + return + } + syntaxUnspecifiedFilenames[errorWithPos.GetPosition().Filename] = struct{}{} +} - sourceCodeInfo := fileDescriptor.GetSourceCodeInfo() - extensionDescriptor := descriptorv1.E_BufSourceCodeInfoExtension.TypeDescriptor() - if sourceCodeInfo.ProtoReflect().Has(extensionDescriptor) { - sourceCodeInfoExt := new(descriptorv1.SourceCodeInfoExtension) - switch sourceCodeInfoExtMessage := sourceCodeInfo.ProtoReflect().Get(extensionDescriptor).Message().Interface().(type) { - case *dynamicpb.Message: - bytes, err := protoencoding.NewWireMarshaler().Marshal(sourceCodeInfoExtMessage) - if err != nil { - return nil, err - } - if err := protoencoding.NewWireUnmarshaler(nil).Unmarshal(bytes, sourceCodeInfoExt); err != nil { - return nil, err - } - case *descriptorv1.SourceCodeInfoExtension: - sourceCodeInfoExt = sourceCodeInfoExtMessage - } - isSyntaxUnspecified = sourceCodeInfoExt.GetIsSyntaxUnspecified() - unusedDependencyIndexes = sourceCodeInfoExt.GetUnusedDependency() +func maybeAddUnusedImport( + filenameToUnusedImportFilenames map[string]map[string]struct{}, + errorWithPos reporter.ErrorWithPos, +) { + errorUnusedImport, ok := errorWithPos.Unwrap().(linker.ErrorUnusedImport) + if !ok { + return } + pos := errorWithPos.GetPosition() + unusedImportFilenames, ok := filenameToUnusedImportFilenames[pos.Filename] + if !ok { + unusedImportFilenames = make(map[string]struct{}) + filenameToUnusedImportFilenames[pos.Filename] = unusedImportFilenames + } + unusedImportFilenames[errorUnusedImport.UnusedImport()] = struct{}{} +} - if excludeSourceCodeInfo { - fileDescriptor.SourceCodeInfo = nil +type buildResult struct { + Files linker.Files + Symbols *linker.Symbols + SyntaxUnspecifiedFilenames map[string]struct{} + FilenameToUnusedDependencyFilenames map[string]map[string]struct{} + Err error +} + +func newBuildResult( + fileDescriptors linker.Files, + symbols *linker.Symbols, + syntaxUnspecifiedFilenames map[string]struct{}, + filenameToUnusedDependencyFilenames map[string]map[string]struct{}, +) *buildResult { + return &buildResult{ + Files: fileDescriptors, + Symbols: symbols, + SyntaxUnspecifiedFilenames: syntaxUnspecifiedFilenames, + FilenameToUnusedDependencyFilenames: filenameToUnusedDependencyFilenames, } +} - return NewImageFile( - fileDescriptor, - moduleFileResolver.FullName(fileDescriptor.GetName()), - moduleFileResolver.CommitID(fileDescriptor.GetName()), - moduleFileResolver.ExternalPath(fileDescriptor.GetName()), - moduleFileResolver.LocalPath(fileDescriptor.GetName()), - isImport, - isSyntaxUnspecified, - unusedDependencyIndexes, - ) +func newFailedBuildResult(err error) *buildResult { + return &buildResult{Err: err} } type buildImageOptions struct { @@ -367,18 +423,132 @@ func newBuildImageOptions() *buildImageOptions { return &buildImageOptions{} } -type fileInfo struct { - path string - externalPath string +// resolverForFiles implements protoencoding.Resolver and is backed +// by a linker.Files and the *linker.Symbols symbol table produced +// when compiling the files. The symbol table is used as an index +// for more efficient lookup. +type resolverForFiles struct { + pathToFile map[string]linker.File + symbols *linker.Symbols +} + +func newResolverForFiles(files linker.Files, symbols *linker.Symbols) protoencoding.Resolver { + // Expand the set of files so it includes the entire transitive graph + pathToFile := make(map[string]linker.File, len(files)) + for _, file := range files { + addFileToMapRec(pathToFile, file) + } + return &resolverForFiles{pathToFile: pathToFile, symbols: symbols} +} + +func (r *resolverForFiles) FindFileByPath(path string) (protoreflect.FileDescriptor, error) { + fileDescriptor, ok := r.pathToFile[path] + if !ok { + return nil, protoregistry.NotFound + } + return fileDescriptor, nil } -func (f *fileInfo) Path() string { - return f.path +func (r *resolverForFiles) FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error) { + span := r.symbols.Lookup(name) + if span == nil { + return nil, protoregistry.NotFound + } + descriptor := r.pathToFile[span.Start().Filename].FindDescriptorByName(name) + if descriptor == nil { + return nil, protoregistry.NotFound + } + return descriptor, nil } -func (f *fileInfo) ExternalPath() string { - if f.externalPath != "" { - return f.externalPath +func (r *resolverForFiles) FindExtensionByName(field protoreflect.FullName) (protoreflect.ExtensionType, error) { + descriptor, err := r.FindDescriptorByName(field) + if err != nil { + return nil, err + } + extensionDescriptor, ok := descriptor.(protoreflect.ExtensionDescriptor) + if !ok { + return nil, fmt.Errorf("%s is a %T, not a protoreflect.ExtensionDescriptor", field, descriptor) + } + if extensionTypeDescriptor, ok := extensionDescriptor.(protoreflect.ExtensionTypeDescriptor); ok { + return extensionTypeDescriptor.Type(), nil + } + return dynamicpb.NewExtensionType(extensionDescriptor), nil +} + +func (r *resolverForFiles) FindExtensionByNumber(message protoreflect.FullName, field protoreflect.FieldNumber) (protoreflect.ExtensionType, error) { + span := r.symbols.LookupExtension(message, field) + if span == nil { + return nil, protoregistry.NotFound + } + extensionDescriptor := findExtension(r.pathToFile[span.Start().Filename], message, field) + if extensionDescriptor == nil { + return nil, protoregistry.NotFound + } + if extensionTypeDescriptor, ok := extensionDescriptor.(protoreflect.ExtensionTypeDescriptor); ok { + return extensionTypeDescriptor.Type(), nil + } + return dynamicpb.NewExtensionType(extensionDescriptor), nil +} + +func (r *resolverForFiles) FindMessageByName(message protoreflect.FullName) (protoreflect.MessageType, error) { + descriptor, err := r.FindDescriptorByName(message) + if err != nil { + return nil, err + } + messageDescriptor, ok := descriptor.(protoreflect.MessageDescriptor) + if !ok { + return nil, fmt.Errorf("%s is a %T, not a protoreflect.MessageDescriptor", message, descriptor) + } + return dynamicpb.NewMessageType(messageDescriptor), nil +} + +func (r *resolverForFiles) FindMessageByURL(url string) (protoreflect.MessageType, error) { + pos := strings.LastIndexByte(url, '/') + return r.FindMessageByName(protoreflect.FullName(url[pos+1:])) +} + +func (r *resolverForFiles) FindEnumByName(enum protoreflect.FullName) (protoreflect.EnumType, error) { + descriptor, err := r.FindDescriptorByName(enum) + if err != nil { + return nil, err + } + enumDescriptor, ok := descriptor.(protoreflect.EnumDescriptor) + if !ok { + return nil, fmt.Errorf("%s is a %T, not a protoreflect.EnumDescriptor", enum, descriptor) + } + return dynamicpb.NewEnumType(enumDescriptor), nil +} + +type container interface { + Messages() protoreflect.MessageDescriptors + Extensions() protoreflect.ExtensionDescriptors +} + +func findExtension(d container, message protoreflect.FullName, field protoreflect.FieldNumber) protoreflect.ExtensionDescriptor { + extensions := d.Extensions() + for i, length := 0, extensions.Len(); i < length; i++ { + extension := extensions.Get(i) + if extension.Number() == field && extension.ContainingMessage().FullName() == message { + return extension + } + } + for i := range d.Messages().Len() { + if ext := findExtension(d.Messages().Get(i), message, field); ext != nil { + return ext + } + } + return nil // could not be found +} + +func addFileToMapRec(pathToFile map[string]linker.File, file linker.File) { + if _, alreadyAdded := pathToFile[file.Path()]; alreadyAdded { + return + } + pathToFile[file.Path()] = file + imports := file.Imports() + for i, length := 0, imports.Len(); i < length; i++ { + importedFile := file.FindImportByPath(imports.Get(i).Path()) + addFileToMapRec(pathToFile, importedFile) } - return f.path } diff --git a/private/bufpkg/bufimage/build_image_test.go b/private/bufpkg/bufimage/build_image_test.go index c4fba34aa7..3154d6cfb9 100644 --- a/private/bufpkg/bufimage/build_image_test.go +++ b/private/bufpkg/bufimage/build_image_test.go @@ -236,7 +236,7 @@ func TestCustomOptionsError1(t *testing.T) { t, "customoptionserror1", true, - filepath.FromSlash("testdata/customoptionserror1/b.proto:9:26:cannot find message field `bat` in `a.Foo`"), + filepath.FromSlash("testdata/customoptionserror1/b.proto:9:27:field a.Baz.bat: option (a.foo).bat: field bat of a.Foo does not exist"), ) } @@ -246,7 +246,7 @@ func TestNotAMessageType(t *testing.T) { t, "notamessagetype", true, - filepath.FromSlash("testdata/notamessagetype/a.proto:9:11:expected message type, found service method `a.MyService.Foo`"), + filepath.FromSlash("testdata/notamessagetype/a.proto:9:11:method a.MyService.Foo: invalid request type: a.MyService.Foo is a method, not a message"), ) } @@ -256,9 +256,8 @@ func TestSpaceBetweenNumberAndID(t *testing.T) { t, "spacebetweennumberandid", true, - filepath.FromSlash("testdata/spacebetweennumberandid/a.proto:6:14:field number out of range"), - filepath.FromSlash("testdata/spacebetweennumberandid/a.proto:6:16:invalid digit in decimal integer literal"), - filepath.FromSlash("testdata/spacebetweennumberandid/a.proto:6:19:unexpected `max` in extension range"), + filepath.FromSlash("testdata/spacebetweennumberandid/a.proto:6:14:invalid syntax in integer value: 10to"), + filepath.FromSlash("testdata/spacebetweennumberandid/a.proto:6:14:syntax error: unexpected error, expecting int literal"), ) } @@ -268,8 +267,10 @@ func TestCyclicImport(t *testing.T) { t, "cyclicimport", false, - fmt.Sprintf( - `%s:5:1:detected cyclic import while importing "a/a.proto"`, + // Since the compiler is multi-threaded, order of file compilation can happen one of two ways + fmt.Sprintf(`%s:5:8:cycle found in imports: "a/a.proto" -> "b/b.proto" -> "a/a.proto" + || %s:5:8:cycle found in imports: "b/b.proto" -> "a/a.proto" -> "b/b.proto"`, + filepath.FromSlash("testdata/cyclicimport/a/a.proto"), filepath.FromSlash("testdata/cyclicimport/b/b.proto"), ), ) @@ -277,17 +278,18 @@ func TestCyclicImport(t *testing.T) { func TestDuplicateSyntheticOneofs(t *testing.T) { // https://github.com/bufbuild/buf/issues/1071 - // - // However, this issue no longer applies to the new compiler, since the new compiler does - // not produce a symbol for the synthetic one-of types for its IR (these are generated - // for the file descriptor set on-demand). It also only surfaces duplicate symbol fqn's - // for the highest level, which is the message in the case of this test. t.Parallel() testFileAnnotations( t, "duplicatesyntheticoneofs", + // Since the compiler is multi-threaded, order of file compilation can happen one of two ways false, - filepath.FromSlash("testdata/duplicatesyntheticoneofs/a1.proto:5:9:`Foo` declared multiple times"), + filepath.FromSlash(`testdata/duplicatesyntheticoneofs/a1.proto:5:9:symbol "a.Foo" already defined at a2.proto:5:9 + || testdata/duplicatesyntheticoneofs/a2.proto:5:9:symbol "a.Foo" already defined at a1.proto:5:9`), + filepath.FromSlash(`testdata/duplicatesyntheticoneofs/a1.proto:6:19:symbol "a.Foo._bar" already defined at a2.proto:6:19 + || testdata/duplicatesyntheticoneofs/a2.proto:6:19:symbol "a.Foo._bar" already defined at a1.proto:6:19`), + filepath.FromSlash(`testdata/duplicatesyntheticoneofs/a1.proto:6:19:symbol "a.Foo.bar" already defined at a2.proto:6:19 + || testdata/duplicatesyntheticoneofs/a2.proto:6:19:symbol "a.Foo.bar" already defined at a1.proto:6:19`), ) } diff --git a/private/bufpkg/bufimage/module_file_resolver.go b/private/bufpkg/bufimage/module_file_resolver.go deleted file mode 100644 index d77c4cef06..0000000000 --- a/private/bufpkg/bufimage/module_file_resolver.go +++ /dev/null @@ -1,195 +0,0 @@ -// Copyright 2020-2026 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 bufimage - -import ( - "context" - "errors" - "fmt" - "io" - "io/fs" - "strings" - "sync" - - "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufparse" - "github.com/bufbuild/buf/private/gen/data/datawkt" - "github.com/bufbuild/buf/private/pkg/storage" - "github.com/bufbuild/protocompile/experimental/source" - "github.com/google/uuid" -) - -// moduleFileResolver resolves sources and module information for module files, including -// well-known types. This is used by the compiler and image building processes. -// [context.Context] is embedded in this case for [storage.ReadBucket] APIs. -type moduleFileResolver struct { - ctx context.Context - moduleReadBucket bufmodule.ModuleReadBucket - pathToExternalPath map[string]string - pathToLocalPath map[string]string - localPathToPath map[string]string - nonImportPaths map[string]struct{} - pathToFullName map[string]bufparse.FullName - pathToCommitID map[string]uuid.UUID - lock sync.RWMutex -} - -func newModuleFileResolver( - ctx context.Context, - moduleReadBucket bufmodule.ModuleReadBucket, -) *moduleFileResolver { - return &moduleFileResolver{ - ctx: ctx, - moduleReadBucket: moduleReadBucket, - pathToExternalPath: make(map[string]string), - pathToLocalPath: make(map[string]string), - localPathToPath: make(map[string]string), - nonImportPaths: make(map[string]struct{}), - pathToFullName: make(map[string]bufparse.FullName), - pathToCommitID: make(map[string]uuid.UUID), - } -} - -// Open opens the given path, and tracks the external path and import status. -// -// This implements [source.Opener]. -func (m *moduleFileResolver) Open(path string) (_ *source.File, retErr error) { - moduleFile, moduleErr := m.moduleReadBucket.GetFile(m.ctx, path) - if moduleErr != nil { - if !errors.Is(moduleErr, fs.ErrNotExist) { - return nil, moduleErr - } - if wktModuleFile, wktErr := datawkt.ReadBucket.Get(m.ctx, path); wktErr == nil { - if wktModuleFile.Path() != path { - // This should never happen, but just in case - return nil, fmt.Errorf("requested path %q but got %q", path, wktModuleFile.Path()) - } - if err := m.addPath(path, path, "", nil, uuid.Nil); err != nil { - return nil, err - } - return readObjectCloserToSourceFile(wktModuleFile) - } - return nil, moduleErr - } - defer func() { - retErr = errors.Join(retErr, moduleFile.Close()) - }() - if moduleFile.Path() != path { - // this should never happen, but just in case - return nil, fmt.Errorf("requested path %q but got %q", path, moduleFile.Path()) - } - if err := m.addPath( - path, - moduleFile.ExternalPath(), - moduleFile.LocalPath(), - moduleFile.Module().FullName(), - moduleFile.Module().CommitID(), - ); err != nil { - return nil, err - } - return readObjectCloserToSourceFile(moduleFile) -} - -// ExternalPath returns the external path for the input path. -// -// Returns the input path if the external path is not known. -func (m *moduleFileResolver) ExternalPath(path string) string { - m.lock.RLock() - defer m.lock.RUnlock() - if externalPath := m.pathToExternalPath[path]; externalPath != "" { - return externalPath - } - return path -} - -// LocalPath returns the local path for the input path if present. -func (m *moduleFileResolver) LocalPath(path string) string { - m.lock.RLock() - defer m.lock.RUnlock() - return m.pathToLocalPath[path] -} - -// PathForLocalPath returns the import path for the given local path if present. -func (m *moduleFileResolver) PathForLocalPath(localPath string) string { - m.lock.RLock() - defer m.lock.RUnlock() - return m.localPathToPath[localPath] -} - -// FullName returns nil if not available. -func (m *moduleFileResolver) FullName(path string) bufparse.FullName { - m.lock.RLock() - defer m.lock.RUnlock() - return m.pathToFullName[path] // nil is a valid value. -} - -// CommitID returns empty if not available. -func (m *moduleFileResolver) CommitID(path string) uuid.UUID { - m.lock.RLock() - defer m.lock.RUnlock() - return m.pathToCommitID[path] // empty is a valid value. -} - -func (m *moduleFileResolver) addPath( - path string, - externalPath string, - localPath string, - moduleFullName bufparse.FullName, - commitID uuid.UUID, -) error { - m.lock.Lock() - defer m.lock.Unlock() - existingExternalPath, ok := m.pathToExternalPath[path] - if ok { - if existingExternalPath != externalPath { - return fmt.Errorf("had external paths %q and %q for path %q", existingExternalPath, externalPath, path) - } - } else { - m.pathToExternalPath[path] = externalPath - } - if localPath != "" { - existingLocalPath, ok := m.pathToLocalPath[path] - if ok { - if existingLocalPath != localPath { - return fmt.Errorf("had local paths %q and %q for path %q", existingLocalPath, localPath, path) - } - } else { - m.pathToLocalPath[path] = localPath - m.localPathToPath[localPath] = path - } - } - if moduleFullName != nil { - m.pathToFullName[path] = moduleFullName - } - if commitID != uuid.Nil { - m.pathToCommitID[path] = commitID - } - return nil -} - -// readObjectCloserToSourceFile is a helper function that takes a given [storage.ReadObjectCloser] -// and returns the corresponding [*source.File]. -func readObjectCloserToSourceFile(readObjectCloser storage.ReadObjectCloser) (*source.File, error) { - var buf strings.Builder - if _, err := io.Copy(&buf, readObjectCloser); err != nil { - return nil, err - } - path := readObjectCloser.LocalPath() - if path == "" { - // Fallback to using Path() -- in-mem buckets, for example, will not have a LocalPath - path = readObjectCloser.Path() - } - return source.NewFile(path, buf.String()), nil -} diff --git a/private/bufpkg/bufimage/parser_accessor_handler.go b/private/bufpkg/bufimage/parser_accessor_handler.go new file mode 100644 index 0000000000..5c1b0adb90 --- /dev/null +++ b/private/bufpkg/bufimage/parser_accessor_handler.go @@ -0,0 +1,166 @@ +// Copyright 2020-2026 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 bufimage + +import ( + "context" + "errors" + "fmt" + "io" + "io/fs" + "sync" + + "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufparse" + "github.com/bufbuild/buf/private/gen/data/datawkt" + "github.com/google/uuid" +) + +type parserAccessorHandler struct { + ctx context.Context + moduleReadBucket bufmodule.ModuleReadBucket + pathToExternalPath map[string]string + pathToLocalPath map[string]string + nonImportPaths map[string]struct{} + pathToFullName map[string]bufparse.FullName + pathToCommitID map[string]uuid.UUID + lock sync.RWMutex +} + +func newParserAccessorHandler( + ctx context.Context, + moduleReadBucket bufmodule.ModuleReadBucket, +) *parserAccessorHandler { + return &parserAccessorHandler{ + ctx: ctx, + moduleReadBucket: moduleReadBucket, + pathToExternalPath: make(map[string]string), + pathToLocalPath: make(map[string]string), + nonImportPaths: make(map[string]struct{}), + pathToFullName: make(map[string]bufparse.FullName), + pathToCommitID: make(map[string]uuid.UUID), + } +} + +// Open opens the given path, and tracks the external path and import status. +// +// This function can be used as the accessor function for a protocompile.SourceResolver. +func (p *parserAccessorHandler) Open(path string) (_ io.ReadCloser, retErr error) { + moduleFile, moduleErr := p.moduleReadBucket.GetFile(p.ctx, path) + if moduleErr != nil { + if !errors.Is(moduleErr, fs.ErrNotExist) { + return nil, moduleErr + } + if wktModuleFile, wktErr := datawkt.ReadBucket.Get(p.ctx, path); wktErr == nil { + if wktModuleFile.Path() != path { + // this should never happen, but just in case + return nil, fmt.Errorf("parser accessor requested path %q but got %q", path, wktModuleFile.Path()) + } + if err := p.addPath(path, path, "", nil, uuid.Nil); err != nil { + return nil, err + } + return wktModuleFile, nil + } + return nil, moduleErr + } + defer func() { + if retErr != nil { + retErr = errors.Join(retErr, moduleFile.Close()) + } + }() + if moduleFile.Path() != path { + // this should never happen, but just in case + return nil, fmt.Errorf("parser accessor requested path %q but got %q", path, moduleFile.Path()) + } + if err := p.addPath( + path, + moduleFile.ExternalPath(), + moduleFile.LocalPath(), + moduleFile.Module().FullName(), + moduleFile.Module().CommitID(), + ); err != nil { + return nil, err + } + return moduleFile, nil +} + +// ExternalPath returns the external path for the input path. +// +// Returns the input path if the external path is not known. +func (p *parserAccessorHandler) ExternalPath(path string) string { + p.lock.RLock() + defer p.lock.RUnlock() + if externalPath := p.pathToExternalPath[path]; externalPath != "" { + return externalPath + } + return path +} + +// LocalPath returns the local path for the input path if present. +func (p *parserAccessorHandler) LocalPath(path string) string { + p.lock.RLock() + defer p.lock.RUnlock() + return p.pathToLocalPath[path] +} + +// FullName returns nil if not available. +func (p *parserAccessorHandler) FullName(path string) bufparse.FullName { + p.lock.RLock() + defer p.lock.RUnlock() + return p.pathToFullName[path] // nil is a valid value. +} + +// CommitID returns empty if not available. +func (p *parserAccessorHandler) CommitID(path string) uuid.UUID { + p.lock.RLock() + defer p.lock.RUnlock() + return p.pathToCommitID[path] // empty is a valid value. +} + +func (p *parserAccessorHandler) addPath( + path string, + externalPath string, + localPath string, + moduleFullName bufparse.FullName, + commitID uuid.UUID, +) error { + p.lock.Lock() + defer p.lock.Unlock() + existingExternalPath, ok := p.pathToExternalPath[path] + if ok { + if existingExternalPath != externalPath { + return fmt.Errorf("parser accessor had external paths %q and %q for path %q", existingExternalPath, externalPath, path) + } + } else { + p.pathToExternalPath[path] = externalPath + } + if localPath != "" { + existingLocalPath, ok := p.pathToLocalPath[path] + if ok { + if existingLocalPath != localPath { + return fmt.Errorf("parser accessor had local paths %q and %q for path %q", existingLocalPath, localPath, path) + } + } else { + p.pathToLocalPath[path] = localPath + } + } + if moduleFullName != nil { + p.pathToFullName[path] = moduleFullName + } + if commitID != uuid.Nil { + p.pathToCommitID[path] = commitID + } + return nil +} diff --git a/private/bufpkg/bufprotocompile/bufprotocompile.go b/private/bufpkg/bufprotocompile/bufprotocompile.go index cbd546f020..fef0414f57 100644 --- a/private/bufpkg/bufprotocompile/bufprotocompile.go +++ b/private/bufpkg/bufprotocompile/bufprotocompile.go @@ -111,7 +111,7 @@ func FileAnnotationSetForErrorsWithPos( if err != nil { return nil, err } - return bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), nil + return bufanalysis.NewFileAnnotationSet(fileAnnotations...), nil } // FileAnnotationOption is an option when creating a FileAnnotation.