diff --git a/.golangci.yml b/.golangci.yml index d5ef0ac0f2..46aa05fb0b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -238,7 +238,7 @@ linters: - linters: - containedctx # we actually want to embed a context here - path: private/bufpkg/bufimage/parser_accessor_handler.go + path: private/bufpkg/bufimage/module_file_resolver.go - linters: - containedctx # we actually want to embed a context here diff --git a/cmd/buf/buf_test.go b/cmd/buf/buf_test.go index 4baac577e7..ad59db9382 100644 --- a/cmd/buf/buf_test.go +++ b/cmd/buf/buf_test.go @@ -4493,7 +4493,29 @@ func TestProtoFileNoWorkspaceOrModule(t *testing.T) { nil, bufctl.ExitCodeFileAnnotation, "", // no stdout - filepath.FromSlash(`testdata/protofileref/noworkspaceormodule/fail/import.proto:3:8:import "`)+`google/type/date.proto": file does not exist`, + fmt.Sprintf( + `warning: missing `+"`package`"+` declaration +--> %[1]s += note: not explicitly specifying a package places the file in the unnamed +package; using it strongly is discouraged + +error: imported file does not exist +--> %[1]s:3:1 +| +3 | import "google/type/date.proto"; +| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here + +error: cannot find `+"`google.type.Date`"+` in this scope +--> %[1]s:6:3 +| +6 | google.type.Date date = 1; +| ^^^^^^^^^^^^^^^^ not found in this scope +| += help: the full name of this scope is `+"`A`"+` + +encountered 2 errors and 1 warning`, + filepath.FromSlash("testdata/protofileref/noworkspaceormodule/fail/import.proto"), + ), "build", filepath.Join("testdata", "protofileref", "noworkspaceormodule", "fail", "import.proto"), ) diff --git a/cmd/buf/imports_test.go b/cmd/buf/imports_test.go index 7c5ea718de..0ba1f0b5a3 100644 --- a/cmd/buf/imports_test.go +++ b/cmd/buf/imports_test.go @@ -15,6 +15,7 @@ package main import ( + "fmt" "io" "path/filepath" "strings" @@ -148,7 +149,16 @@ func TestInvalidNonexistentImport(t *testing.T) { t.Parallel() testRunStderrWithCache( t, nil, bufctl.ExitCodeFileAnnotation, - filepath.FromSlash(`testdata/imports/failure/people/people/v1/people1.proto:5:8:import "nonexistent.proto": file does not exist`), + fmt.Sprintf( + `error: imported file does not exist +--> %s:5:1 +| +5 | import "nonexistent.proto"; +| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here + +encountered 1 error`, + filepath.FromSlash("testdata/imports/failure/people/people/v1/people1.proto"), + ), "build", filepath.Join("testdata", "imports", "failure", "people"), ) @@ -158,7 +168,24 @@ func TestInvalidNonexistentImportFromDirectDep(t *testing.T) { t.Parallel() testRunStderrWithCache( t, nil, bufctl.ExitCodeFileAnnotation, - filepath.FromSlash(`testdata/imports/failure/students/students/v1/students.proto:`)+`6:8:import "people/v1/people_nonexistent.proto": file does not exist`, + fmt.Sprintf( + `error: imported file does not exist +--> %[1]s:6:1 +| +6 | import "people/v1/people_nonexistent.proto"; // but nonexistent file in explicit direct import +| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ imported here + +error: cannot find `+"`people.v1.Person2`"+` in this scope +--> %[1]s:10:3 +| +10 | people.v1.Person2 person2 = 2; +| ^^^^^^^^^^^^^^^^^ not found in this scope +| += help: the full name of this scope is `+"`students.v1.Student`"+` + +encountered 2 errors`, + filepath.FromSlash("testdata/imports/failure/students/students/v1/students.proto"), + ), "build", filepath.Join("testdata", "imports", "failure", "students"), ) diff --git a/cmd/buf/internal/command/breaking/breaking.go b/cmd/buf/internal/command/breaking/breaking.go index 29ec1ed706..ff3a7969c8 100644 --- a/cmd/buf/internal/command/breaking/breaking.go +++ b/cmd/buf/internal/command/breaking/breaking.go @@ -364,7 +364,7 @@ func run( } } if len(allFileAnnotations) > 0 { - allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...) + allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...) if err := bufanalysis.PrintFileAnnotationSet( container.Stdout(), allFileAnnotationSet, diff --git a/cmd/buf/internal/command/lint/lint.go b/cmd/buf/internal/command/lint/lint.go index ab7421f571..938851d449 100644 --- a/cmd/buf/internal/command/lint/lint.go +++ b/cmd/buf/internal/command/lint/lint.go @@ -168,7 +168,7 @@ func run( } } if len(allFileAnnotations) > 0 { - allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...) + allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...) if flags.ErrorFormat == "config-ignore-yaml" { if err := bufcli.PrintFileAnnotationSetLintConfigIgnoreYAMLV1( container.Stdout(), diff --git a/cmd/buf/workspace_test.go b/cmd/buf/workspace_test.go index c75ef1030e..3518f40f51 100644 --- a/cmd/buf/workspace_test.go +++ b/cmd/buf/workspace_test.go @@ -141,11 +141,30 @@ func TestWorkspaceDir(t *testing.T) { "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), ) + dirImportError := fmt.Sprintf(`error: imported file does not exist +--> %[1]s:5:1 +| +5 | import "request.proto"; +| ^^^^^^^^^^^^^^^^^^^^^^^ imported here + +error: cannot find %[2]s in this scope +--> %[1]s:8:5 +| +8 | request.Request req = 1; +| ^^^^^^^^^^^^^^^ not found in this scope +| += help: the full name of this scope is %[3]s + +encountered 2 errors`, + filepath.FromSlash("testdata/workspace/success/"+baseDirPath+"/proto/rpc.proto"), + "`request.Request`", + "`example.RPC`", + ) testRunStdoutStderrNoWarn( t, nil, bufctl.ExitCodeFileAnnotation, - filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), + dirImportError, "", "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), @@ -156,7 +175,7 @@ func TestWorkspaceDir(t *testing.T) { t, nil, bufctl.ExitCodeFileAnnotation, - filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), + dirImportError, "", "lint", filepath.Join("testdata", "workspace", "success", baseDirPath), @@ -366,12 +385,31 @@ func TestWorkspaceDetached(t *testing.T) { // we'd consider this a bug: you specified the proto directory, and no controlling workspace // was discovered, therefore you build as if proto was the input directory, which results in // request.proto not existing as an import. + detachedImportError := fmt.Sprintf(`error: imported file does not exist +--> %[1]s:5:1 +| +5 | import "request.proto"; +| ^^^^^^^^^^^^^^^^^^^^^^^ imported here + +error: cannot find %[2]s in this scope +--> %[1]s:8:5 +| +8 | request.Request req = 1; +| ^^^^^^^^^^^^^^^ not found in this scope +| += help: the full name of this scope is %[3]s + +encountered 2 errors`, + filepath.FromSlash("testdata/workspace/success/"+dirPath+"/proto/rpc.proto"), + "`request.Request`", + "`example.RPC`", + ) testRunStdoutStderrNoWarn( t, nil, bufctl.ExitCodeFileAnnotation, ``, - filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), + detachedImportError, "build", filepath.Join("testdata", "workspace", "success", dirPath, "proto"), ) @@ -392,7 +430,7 @@ func TestWorkspaceDetached(t *testing.T) { t, nil, bufctl.ExitCodeFileAnnotation, - filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`), + detachedImportError, ``, "lint", filepath.Join("testdata", "workspace", "success", dirPath, "proto"), diff --git a/cmd/buf/workspace_unix_test.go b/cmd/buf/workspace_unix_test.go index a059fa7116..23b5a6c27d 100644 --- a/cmd/buf/workspace_unix_test.go +++ b/cmd/buf/workspace_unix_test.go @@ -17,6 +17,7 @@ package main import ( + "fmt" "path/filepath" "testing" @@ -26,24 +27,39 @@ import ( func TestWorkspaceSymlinkFail(t *testing.T) { t.Parallel() // The workspace includes a symlink that isn't buildable. - testRunStdoutStderrNoWarn( - t, - nil, - bufctl.ExitCodeFileAnnotation, - ``, - filepath.FromSlash(`testdata/workspace/fail/symlink/b/b.proto:5:8:import "c.proto": file does not exist`), - "build", - filepath.Join("testdata", "workspace", "fail", "symlink"), - ) - testRunStdoutStderrNoWarn( - t, - nil, - bufctl.ExitCodeFileAnnotation, - ``, - filepath.FromSlash(`testdata/workspace/fail/v2/symlink/b/b.proto:5:8:import "c.proto": file does not exist`), - "build", - filepath.Join("testdata", "workspace", "fail", "v2", "symlink"), - ) + for _, dirPath := range []string{ + "symlink", + "v2/symlink", + } { + symlinkImportError := fmt.Sprintf(`error: imported file does not exist +--> %[1]s:5:1 +| +5 | import "c.proto"; +| ^^^^^^^^^^^^^^^^^ imported here + +error: cannot find %[2]s in this scope +--> %[1]s:8:5 +| +8 | c.C c = 1; +| ^^^ not found in this scope +| += help: the full name of this scope is %[3]s + +encountered 2 errors`, + filepath.FromSlash("testdata/workspace/fail/"+dirPath+"/b/b.proto"), + "`c.C`", + "`b.B`", + ) + testRunStdoutStderrNoWarn( + t, + nil, + bufctl.ExitCodeFileAnnotation, + ``, + symlinkImportError, + "build", + filepath.Join("testdata", "workspace", "fail", filepath.FromSlash(dirPath)), + ) + } } func TestWorkspaceSymlink(t *testing.T) { diff --git a/go.mod b/go.mod index 2b39d3e114..571ee3ff9e 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25.7 require ( buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.11-20250718181942-e35f9b667443.1 + buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go v1.36.11-20250109164928-1da0de137947.1 buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.11-20260209202127-80ab13bee0bf.1 buf.build/gen/go/bufbuild/registry/connectrpc/go v1.19.1-20260126144947-819582968857.2 buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-819582968857.1 @@ -16,7 +17,7 @@ require ( buf.build/go/standard v0.1.1-0.20260320140628-2996a887cf13 connectrpc.com/connect v1.19.1 connectrpc.com/otelconnect v0.9.0 - github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592 + github.com/bufbuild/protocompile v0.14.2-0.20260323163443-aa1a63ac3dac 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 5657d8f1f6..c292a618a7 100644 --- a/go.sum +++ b/go.sum @@ -40,8 +40,8 @@ github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6 github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/brianvoe/gofakeit/v6 v6.28.0 h1:Xib46XXuQfmlLS2EXRuJpqcw8St6qSZz75OUo0tgAW4= github.com/brianvoe/gofakeit/v6 v6.28.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs= -github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592 h1:LP+okT4NAncx5csNnPka2j8rKcq1ekvrdQfQp3jD0TI= -github.com/bufbuild/protocompile v0.14.2-0.20260319203231-019757e4c592/go.mod h1:o4sxIWZ71DJt7/jan0/vi3XbGCQMTBk5KCo27vgT45Q= +github.com/bufbuild/protocompile v0.14.2-0.20260323163443-aa1a63ac3dac h1:aAuMdUyQnZtkFgb8gFDPy5ZZaNRbgOtVOU7Cy++8Ih4= +github.com/bufbuild/protocompile v0.14.2-0.20260323163443-aa1a63ac3dac/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/bufpkg/bufanalysis/bufanalysis.go b/private/bufpkg/bufanalysis/bufanalysis.go index 70ffb532ef..20bd92171e 100644 --- a/private/bufpkg/bufanalysis/bufanalysis.go +++ b/private/bufpkg/bufanalysis/bufanalysis.go @@ -19,6 +19,8 @@ import ( "io" "strconv" "strings" + + "github.com/bufbuild/protocompile/experimental/report" ) const ( @@ -207,14 +209,27 @@ type FileAnnotationSet interface { // These will be deduplicated and sorted. FileAnnotations() []FileAnnotation + // diagnosticReport returns the diagnostic [report.Report] for the [FileAnnotationSet], + // if set. + // + // This may be nil. If non-nil, it will be used as the output for + // [PrintFileAnnotationSet] when the format is set to "text", rendered + // with a [report.Renderer] configured by the given print options. + diagnosticReport() *report.Report + isFileAnnotationSet() } // NewFileAnnotationSet returns a new FileAnnotationSet. // // If len(fileAnnotations) is 0, this returns nil. -func NewFileAnnotationSet(fileAnnotations ...FileAnnotation) FileAnnotationSet { - return newFileAnnotationSet(fileAnnotations) +// +// The diagnosticReport is the [report.Report] from the compiler, if available. +// If non-nil, it will be rendered by [PrintFileAnnotationSet] when the format +// is "text". Otherwise, the individual file annotations will be used, same as +// all other print formats. +func NewFileAnnotationSet(diagnosticReport *report.Report, fileAnnotations ...FileAnnotation) FileAnnotationSet { + return newFileAnnotationSet(diagnosticReport, fileAnnotations) } // PrintFileAnnotationSet prints the file annotations separated by newlines. @@ -226,6 +241,11 @@ func PrintFileAnnotationSet(writer io.Writer, fileAnnotationSet FileAnnotationSe switch format { case FormatText: + if diagnosticReport := fileAnnotationSet.diagnosticReport(); diagnosticReport != nil { + renderer := report.Renderer{} + _, _, err := renderer.Render(diagnosticReport, writer) + return err + } return printAsText(writer, fileAnnotationSet.FileAnnotations()) case FormatJSON: return printAsJSON(writer, fileAnnotationSet.FileAnnotations()) diff --git a/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go b/private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting_test.go index 93700df24e..6c867b91c0 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(fileAnnotations...), "text") + err := bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, 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(fileAnnotations...), "json") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, 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(fileAnnotations...), "msvs") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, 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(fileAnnotations...), "junit") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), "junit") require.NoError(t, err) assert.Equal(t, ` @@ -98,6 +98,7 @@ path/to/file.proto(2,1) : error FOO : Hello. (buf-plugin-foo) err = bufanalysis.PrintFileAnnotationSet( sb, bufanalysis.NewFileAnnotationSet( + nil, append( fileAnnotations, newFileAnnotation( @@ -135,7 +136,7 @@ path/to/file.proto(2,1) : error FOO : Hello. (buf-plugin-foo) sb.String(), ) sb.Reset() - err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(fileAnnotations...), "gitlab-code-quality") + err = bufanalysis.PrintFileAnnotationSet(sb, bufanalysis.NewFileAnnotationSet(nil, 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 3b056a5fca..3fb8fcc3f1 100644 --- a/private/bufpkg/bufanalysis/file_annotation_set.go +++ b/private/bufpkg/bufanalysis/file_annotation_set.go @@ -19,18 +19,22 @@ import ( "sort" "strconv" "strings" + + "github.com/bufbuild/protocompile/experimental/report" ) type fileAnnotationSet struct { fileAnnotations []FileAnnotation + report *report.Report } -func newFileAnnotationSet(fileAnnotations []FileAnnotation) *fileAnnotationSet { +func newFileAnnotationSet(diagnosticReport *report.Report, fileAnnotations []FileAnnotation) *fileAnnotationSet { if len(fileAnnotations) == 0 { return nil } return &fileAnnotationSet{ fileAnnotations: deduplicateAndSortFileAnnotations(fileAnnotations), + report: diagnosticReport, } } @@ -49,6 +53,10 @@ 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 19a70e89a9..3965edd658 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(), - `% changed default value from %v to %v.`, + `%s 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 ea90ef0571..40e55026b8 100644 --- a/private/bufpkg/bufcheck/client.go +++ b/private/bufpkg/bufcheck/client.go @@ -163,6 +163,7 @@ func (c *client) Lint( return nil } return bufanalysis.NewFileAnnotationSet( + nil, annotationsToFileAnnotations( imageToPathToExternalPath( image, @@ -302,6 +303,7 @@ 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 428ec47b09..d2e095a02c 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 986e21e26d..01af50974b 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 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -205,28 +205,28 @@ enum Quz { 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -288,12 +288,13 @@ enum Quz { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -301,13 +302,12 @@ enum Quz { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -394,12 +394,13 @@ enum Quz { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -407,13 +408,12 @@ enum Quz { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -463,28 +463,28 @@ enum Quz { 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ @@ -562,28 +562,28 @@ enum Quz { 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ @@ -618,28 +618,28 @@ enum Quz { 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ @@ -702,28 +702,28 @@ enum Quz { 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ @@ -769,28 +769,28 @@ enum Quz { 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Baz.txtar index 40f13b8cbc..4495a519df 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 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar index ced7534362..707bdf89d3 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 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -213,28 +213,28 @@ service Svc { 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -296,12 +296,13 @@ service Svc { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -309,13 +310,12 @@ service Svc { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -402,12 +402,13 @@ service Svc { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -415,13 +416,12 @@ service Svc { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -471,28 +471,28 @@ service Svc { 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ @@ -570,28 +570,28 @@ service Svc { 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ @@ -626,28 +626,28 @@ service Svc { 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ @@ -710,28 +710,28 @@ service Svc { 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ @@ -777,28 +777,28 @@ service Svc { 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar index dd4025a192..5b467b10f0 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo+Ext.txtar @@ -435,18 +435,6 @@ 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, @@ -471,6 +459,18 @@ 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 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -649,28 +649,28 @@ extend google.protobuf.MessageOptions { 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -732,12 +732,13 @@ extend google.protobuf.MessageOptions { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -745,13 +746,12 @@ extend google.protobuf.MessageOptions { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -838,12 +838,13 @@ extend google.protobuf.MessageOptions { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -851,13 +852,12 @@ extend google.protobuf.MessageOptions { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -955,14 +955,14 @@ extend google.protobuf.MessageOptions { 3, 0, 6, - 0 + 0, + 2 ], "span": [ - 86, - 6, - 32 - ], - "leadingComments": " Keep if Foo + ext: comment on extension blah\n" + 84, + 11, + 14 + ] }, { "path": [ @@ -972,11 +972,11 @@ extend google.protobuf.MessageOptions { 0, 6, 0, - 2 + 4 ], "span": [ - 84, - 11, + 86, + 6, 14 ] }, @@ -987,14 +987,14 @@ extend google.protobuf.MessageOptions { 3, 0, 6, - 0, - 4 + 0 ], "span": [ 86, 6, - 14 - ] + 32 + ], + "leadingComments": " Keep if Foo + ext: comment on extension blah\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar index 9e8eff80b9..c81b682321 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 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -179,28 +179,28 @@ message Foo { 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -262,12 +262,13 @@ message Foo { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -275,13 +276,12 @@ message Foo { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -368,12 +368,13 @@ message Foo { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -381,13 +382,12 @@ message Foo { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/NestedFoo.txtar index 3c83914b74..fdcdd45ba3 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 + 0, + 4 ], "span": [ 69, 4, - 28 - ], - "leadingComments": " Keep if NestedFoo: comment on field uid\n" + 12 + ] }, { "path": [ @@ -341,14 +341,14 @@ message Foo { 3, 0, 2, - 0, - 4 + 0 ], "span": [ 69, 4, - 12 - ] + 28 + ], + "leadingComments": " Keep if NestedFoo: comment on field uid\n" }, { "path": [ @@ -405,14 +405,14 @@ message Foo { 3, 0, 2, - 1 + 1, + 4 ], "span": [ 71, 4, - 42 - ], - "leadingComments": " Keep if NestedFoo: comment on field meta\n" + 12 + ] }, { "path": [ @@ -421,14 +421,14 @@ message Foo { 3, 0, 2, - 1, - 4 + 1 ], "span": [ 71, 4, - 12 - ] + 42 + ], + "leadingComments": " Keep if NestedFoo: comment on field meta\n" }, { "path": [ @@ -485,14 +485,14 @@ message Foo { 3, 0, 2, - 2 + 2, + 4 ], "span": [ 73, 4, - 29 - ], - "leadingComments": " Keep if NestedFoo: comment on field state\n" + 12 + ] }, { "path": [ @@ -501,14 +501,14 @@ message Foo { 3, 0, 2, - 2, - 4 + 2 ], "span": [ 73, 4, - 12 - ] + 29 + ], + "leadingComments": " Keep if NestedFoo: comment on field state\n" }, { "path": [ @@ -600,14 +600,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": [ @@ -618,14 +618,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": [ @@ -654,14 +654,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": [ @@ -672,14 +672,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": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Quz.txtar index deee0b85bb..2dea92d579 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 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Svc.txtar index 7901a8ab25..31b8302205 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 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -215,28 +215,28 @@ service Svc { 4, 0, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 0, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -298,12 +298,13 @@ service Svc { 4, 0, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -311,13 +312,12 @@ service Svc { 4, 0, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -404,12 +404,13 @@ service Svc { 4, 0, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -417,13 +418,12 @@ service Svc { 4, 0, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -473,28 +473,28 @@ service Svc { 4, 1, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ 4, 1, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ @@ -572,28 +572,28 @@ service Svc { 4, 1, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ @@ -628,28 +628,28 @@ service Svc { 4, 1, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ 4, 1, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ @@ -712,28 +712,28 @@ service Svc { 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ @@ -779,28 +779,28 @@ service Svc { 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar index 49ed048f96..19af1f218f 100644 --- a/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar +++ b/private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/all.txtar @@ -717,18 +717,6 @@ 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, @@ -753,6 +741,18 @@ 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,18 +801,6 @@ 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, @@ -837,6 +825,18 @@ 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 + 0, + 4 ], "span": [ 33, 2, - 25 - ], - "leadingComments": " Keep if all: comment on field xyz\n" + 10 + ] }, { "path": [ 4, 0, 2, - 0, - 4 + 0 ], "span": [ 33, 2, - 10 - ] + 25 + ], + "leadingComments": " Keep if all: comment on field xyz\n" }, { "path": [ @@ -1028,28 +1028,28 @@ extend google.protobuf.ServiceOptions { 4, 1, 2, - 0 + 0, + 4 ], "span": [ 49, 2, - 27 - ], - "leadingComments": " Keep if Foo: comment on field name\n" + 10 + ] }, { "path": [ 4, 1, 2, - 0, - 4 + 0 ], "span": [ 49, 2, - 10 - ] + 27 + ], + "leadingComments": " Keep if Foo: comment on field name\n" }, { "path": [ @@ -1098,28 +1098,28 @@ extend google.protobuf.ServiceOptions { 4, 1, 2, - 1 + 1, + 4 ], "span": [ 51, 2, - 26 - ], - "leadingComments": " Keep if Foo: comment on field bits\n" + 10 + ] }, { "path": [ 4, 1, 2, - 1, - 4 + 1 ], "span": [ 51, 2, - 10 - ] + 26 + ], + "leadingComments": " Keep if Foo: comment on field bits\n" }, { "path": [ @@ -1181,12 +1181,13 @@ extend google.protobuf.ServiceOptions { 4, 1, 9, - 0 + 0, + 1 ], "span": [ 54, 11, - 19 + 13 ] }, { @@ -1194,13 +1195,12 @@ extend google.protobuf.ServiceOptions { 4, 1, 9, - 0, - 1 + 0 ], "span": [ 54, 11, - 13 + 19 ] }, { @@ -1287,12 +1287,13 @@ extend google.protobuf.ServiceOptions { 4, 1, 5, - 0 + 0, + 1 ], "span": [ 58, 13, - 23 + 16 ] }, { @@ -1300,13 +1301,12 @@ extend google.protobuf.ServiceOptions { 4, 1, 5, - 0, - 1 + 0 ], "span": [ 58, 13, - 16 + 23 ] }, { @@ -1390,14 +1390,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": [ @@ -1406,14 +1406,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": [ @@ -1470,14 +1470,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": [ @@ -1486,14 +1486,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": [ @@ -1550,14 +1550,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": [ @@ -1566,14 +1566,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": [ @@ -1665,14 +1665,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": [ @@ -1683,14 +1683,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": [ @@ -1719,14 +1719,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": [ @@ -1737,14 +1737,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": [ @@ -1787,14 +1787,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 6, - 0 + 0, + 2 ], "span": [ - 86, - 6, - 32 - ], - "leadingComments": " Keep if Foo + ext: comment on extension blah\n" + 84, + 11, + 14 + ] }, { "path": [ @@ -1804,11 +1804,11 @@ extend google.protobuf.ServiceOptions { 0, 6, 0, - 2 + 4 ], "span": [ - 84, - 11, + 86, + 6, 14 ] }, @@ -1819,14 +1819,14 @@ extend google.protobuf.ServiceOptions { 3, 0, 6, - 0, - 4 + 0 ], "span": [ 86, 6, - 14 - ] + 32 + ], + "leadingComments": " Keep if Foo + ext: comment on extension blah\n" }, { "path": [ @@ -1909,28 +1909,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 0 + 0, + 4 ], "span": [ 97, 2, - 23 - ], - "leadingComments": " Keep if Bar: comment on field foo\n" + 10 + ] }, { "path": [ 4, 2, 2, - 0, - 4 + 0 ], "span": [ 97, 2, - 10 - ] + 23 + ], + "leadingComments": " Keep if Bar: comment on field foo\n" }, { "path": [ @@ -2008,28 +2008,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 1 + 1, + 6 ], "span": [ 101, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field baz\n" + 7 + ] }, { "path": [ 4, 2, 2, - 1, - 6 + 1 ], "span": [ 101, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field baz\n" }, { "path": [ @@ -2064,28 +2064,28 @@ extend google.protobuf.ServiceOptions { 4, 2, 2, - 2 + 2, + 6 ], "span": [ 103, 4, - 16 - ], - "leadingComments": " Keep if Bar: comment on field quz\n" + 7 + ] }, { "path": [ 4, 2, 2, - 2, - 6 + 2 ], "span": [ 103, 4, - 7 - ] + 16 + ], + "leadingComments": " Keep if Bar: comment on field quz\n" }, { "path": [ @@ -2148,28 +2148,28 @@ extend google.protobuf.ServiceOptions { 5, 0, 2, - 0 + 0, + 1 ], "span": [ 113, 2, - 22 - ], - "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 0, 2, - 0, - 1 + 0 ], "span": [ 113, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Baz: comment on enum value BAZ_UNSPECIFIED\n" }, { "path": [ @@ -2215,28 +2215,28 @@ extend google.protobuf.ServiceOptions { 5, 1, 2, - 0 + 0, + 1 ], "span": [ 119, 2, - 22 - ], - "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" + 17 + ] }, { "path": [ 5, 1, 2, - 0, - 1 + 0 ], "span": [ 119, 2, - 17 - ] + 22 + ], + "leadingComments": " Keep if Quz: comment on enum value QUZ_UNSPECIFIED\n" }, { "path": [ @@ -2282,28 +2282,28 @@ extend google.protobuf.ServiceOptions { 4, 3, 2, - 0 + 0, + 4 ], "span": [ 125, 2, - 24 - ], - "leadingComments": " Keep if all: comment on field s\n" + 10 + ] }, { "path": [ 4, 3, 2, - 0, - 4 + 0 ], "span": [ 125, 2, - 10 - ] + 24 + ], + "leadingComments": " Keep if all: comment on field s\n" }, { "path": [ @@ -2377,28 +2377,28 @@ extend google.protobuf.ServiceOptions { 4, 4, 2, - 0 + 0, + 4 ], "span": [ 131, 2, - 24 - ], - "leadingComments": " Keep if all: comment on field t\n" + 10 + ] }, { "path": [ 4, 4, 2, - 0, - 4 + 0 ], "span": [ 131, 2, - 10 - ] + 24 + ], + "leadingComments": " Keep if all: comment on field t\n" }, { "path": [ diff --git a/private/bufpkg/bufimage/build_image.go b/private/bufpkg/bufimage/build_image.go index 8050511ede..85aefbd15b 100644 --- a/private/bufpkg/bufimage/build_image.go +++ b/private/bufpkg/bufimage/build_image.go @@ -19,23 +19,24 @@ 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" - "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" + "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" "google.golang.org/protobuf/types/dynamicpb" ) @@ -49,283 +50,228 @@ 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 no target files within the module after path filtering, this is an error. + // If we had 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) - buildResult := getBuildResult( + image, err := compileImage( ctx, - parserAccessorHandler, - paths, + bufmodule.ModuleReadBucketWithOnlyProtoFiles(moduleReadBucket), + bufmodule.FileInfoPaths(targetFileInfos), 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 } -func getBuildResult( +// compileImage compiles the [Image] for the given [bufmodule.ModuleReadBucket]. +func compileImage( ctx context.Context, - parserAccessorHandler *parserAccessorHandler, + moduleReadBucket bufmodule.ModuleReadBucket, paths []string, excludeSourceCodeInfo bool, noParallelism bool, -) *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 - } +) (Image, error) { + session := new(ir.Session) + moduleFileResolver := newModuleFileResolver(ctx, moduleReadBucket) + parallelism := thread.Parallelism() if noParallelism { parallelism = 1 } - 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) - }, - ), - } - // fileDescriptors are in the same order as paths per the documentation - compiledFiles, err := compiler.Compile(ctx, paths...) + + 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 { - 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) - } - 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)) + return nil, err + } + + 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 } - 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"), + 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()) + 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 + ), ) } - if len(compiledFiles) != len(paths) { - return newFailedBuildResult( - fmt.Errorf("expected FileDescriptors to be of length %d but was %d", len(paths), len(compiledFiles)), - ) + if len(fileAnnotations) > 0 { + return nil, bufanalysis.NewFileAnnotationSet(diagnostics, fileAnnotations...) } - 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), - ) - } + + // 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)) } - syntaxUnspecifiedFilenames := make(map[string]struct{}) - filenameToUnusedDependencyFilenames := make(map[string]map[string]struct{}) - for _, warningErrorWithPos := range warningErrorsWithPos { - maybeAddSyntaxUnspecified(syntaxUnspecifiedFilenames, warningErrorWithPos) - maybeAddUnusedImport(filenameToUnusedDependencyFilenames, warningErrorWithPos) + + if results[0].Fatal != nil { + return nil, results[0].Fatal } - return newBuildResult( - compiledFiles, - symbols, - syntaxUnspecifiedFilenames, - filenameToUnusedDependencyFilenames, + irFiles := results[0].Value + + 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 + } -// 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)) + // First unmarshal to get the descriptors + fds := new(descriptorpb.FileDescriptorSet) + if err := protoencoding.NewWireUnmarshaler(nil).Unmarshal(bytes, fds); err != nil { + return nil, err } - 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 + + // 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), } - // 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) + 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 } - sortedFileDescriptors = append(sortedFileDescriptors, fileDescriptor) } - return sortedFileDescriptors, nil + + return fileDescriptorSetToImage(resolver, moduleFileResolver, paths, fds, excludeSourceCodeInfo) } -// getImage gets the Image for the protoreflect.FileDescriptors. +// fileDescriptorSetToImage is a helper function that converts a [descriptorpb.FileDescriptorSet] +// to an [Image], preserving the order of the paths based on the module paths. // -// This mimics protoc's output order. -// This assumes checkAndSortFiles was called. -func getImage( - ctx context.Context, +// 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, excludeSourceCodeInfo bool, - sortedFiles linker.Files, - symbols *linker.Symbols, - parserAccessorHandler *parserAccessorHandler, - syntaxUnspecifiedFilenames map[string]struct{}, - filenameToUnusedDependencyFilenames map[string]map[string]struct{}, ) (Image, error) { - // 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{}{} + pathToDescriptor := make(map[string]*descriptorpb.FileDescriptorProto) + for _, fileDescriptor := range fds.File { + pathToDescriptor[fileDescriptor.GetName()] = fileDescriptor } var imageFiles []ImageFile var err error - alreadySeen := map[string]struct{}{} - for _, fileDescriptor := range sortedFiles { - imageFiles, err = getImageFilesRec( - ctx, + seen := make(map[string]struct{}) + nonImportPaths := xslices.ToStructMap(paths) + + for _, path := range paths { + imageFiles, err = getImageFilesForPath( + path, + pathToDescriptor, + moduleFileResolver, excludeSourceCodeInfo, - fileDescriptor, - parserAccessorHandler, - syntaxUnspecifiedFilenames, - filenameToUnusedDependencyFilenames, - alreadySeen, - nonImportFilenames, + seen, + nonImportPaths, imageFiles, ) + if err != nil { return nil, err } } - return newImage(imageFiles, false, newResolverForFiles(sortedFiles, symbols)) + + return newImage(imageFiles, false, resolver) } -func getImageFilesRec( - ctx context.Context, +func getImageFilesForPath( + path string, + pathToDescriptor map[string]*descriptorpb.FileDescriptorProto, + moduleFileResolver *moduleFileResolver, excludeSourceCodeInfo bool, - fileDescriptor protoreflect.FileDescriptor, - parserAccessorHandler *parserAccessorHandler, - syntaxUnspecifiedFilenames map[string]struct{}, - filenameToUnusedDependencyFilenames map[string]map[string]struct{}, - alreadySeen map[string]struct{}, + seen map[string]struct{}, nonImportFilenames map[string]struct{}, imageFiles []ImageFile, ) ([]ImageFile, error) { + fileDescriptor := pathToDescriptor[path] if fileDescriptor == nil { return nil, errors.New("nil FileDescriptor") } - path := fileDescriptor.Path() - if _, ok := alreadySeen[path]; ok { + + if _, ok := seen[path]; ok { return imageFiles, nil } - alreadySeen[path] = struct{}{} + seen[path] = struct{}{} - unusedDependencyFilenames, ok := filenameToUnusedDependencyFilenames[path] - var unusedDependencyIndexes []int32 - if ok { - unusedDependencyIndexes = make([]int32, 0, len(unusedDependencyFilenames)) - } var err error - 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, + for _, dependency := range fileDescriptor.Dependency { + imageFiles, err = getImageFilesForPath( dependency, - parserAccessorHandler, - syntaxUnspecifiedFilenames, - filenameToUnusedDependencyFilenames, - alreadySeen, + pathToDescriptor, + moduleFileResolver, + excludeSourceCodeInfo, + seen, nonImportFilenames, imageFiles, ) @@ -334,26 +280,13 @@ func getImageFilesRec( } } - 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] - _, syntaxUnspecified := syntaxUnspecifiedFilenames[path] - imageFile, err := NewImageFile( - fileDescriptorProto, - parserAccessorHandler.FullName(path), - parserAccessorHandler.CommitID(path), - // if empty, defaults to path - parserAccessorHandler.ExternalPath(path), - parserAccessorHandler.LocalPath(path), + + imageFile, err := fileDescriptorProtoToImageFile( + moduleFileResolver, + fileDescriptor, + excludeSourceCodeInfo, !isNotImport, - syntaxUnspecified, - unusedDependencyIndexes, ) if err != nil { return nil, err @@ -361,57 +294,53 @@ func getImageFilesRec( return append(imageFiles, imageFile), nil } -func maybeAddSyntaxUnspecified( - syntaxUnspecifiedFilenames map[string]struct{}, - errorWithPos reporter.ErrorWithPos, -) { - if errorWithPos.Unwrap() != parser.ErrNoSyntax { - return - } - syntaxUnspecifiedFilenames[errorWithPos.GetPosition().Filename] = struct{}{} -} +// 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 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 + 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() } - unusedImportFilenames[errorUnusedImport.UnusedImport()] = struct{}{} -} -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, + if excludeSourceCodeInfo { + fileDescriptor.SourceCodeInfo = nil } -} -func newFailedBuildResult(err error) *buildResult { - return &buildResult{Err: err} + return NewImageFile( + fileDescriptor, + moduleFileResolver.FullName(fileDescriptor.GetName()), + moduleFileResolver.CommitID(fileDescriptor.GetName()), + moduleFileResolver.ExternalPath(fileDescriptor.GetName()), + moduleFileResolver.LocalPath(fileDescriptor.GetName()), + isImport, + isSyntaxUnspecified, + unusedDependencyIndexes, + ) } type buildImageOptions struct { @@ -423,132 +352,18 @@ func newBuildImageOptions() *buildImageOptions { return &buildImageOptions{} } -// 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} +type fileInfo struct { + path string + externalPath string } -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 (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) +func (f *fileInfo) ExternalPath() string { + if f.externalPath != "" { + return f.externalPath } + return f.path } diff --git a/private/bufpkg/bufimage/build_image_test.go b/private/bufpkg/bufimage/build_image_test.go index 3154d6cfb9..dab6a62c79 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:27:field a.Baz.bat: option (a.foo).bat: field bat of a.Foo does not exist"), + filepath.FromSlash("testdata/customoptionserror1/b.proto:9:26:cannot find message field `bat` in `a.Foo`"), ) } @@ -246,7 +246,7 @@ func TestNotAMessageType(t *testing.T) { t, "notamessagetype", true, - filepath.FromSlash("testdata/notamessagetype/a.proto:9:11:method a.MyService.Foo: invalid request type: a.MyService.Foo is a method, not a message"), + filepath.FromSlash("testdata/notamessagetype/a.proto:9:11:expected message type, found service method `a.MyService.Foo`"), ) } @@ -256,8 +256,9 @@ func TestSpaceBetweenNumberAndID(t *testing.T) { t, "spacebetweennumberandid", true, - 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"), + 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"), ) } @@ -267,10 +268,8 @@ func TestCyclicImport(t *testing.T) { t, "cyclicimport", false, - // 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"), + fmt.Sprintf( + `%s:5:1:detected cyclic import while importing "a/a.proto"`, filepath.FromSlash("testdata/cyclicimport/b/b.proto"), ), ) @@ -278,18 +277,17 @@ 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: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`), + filepath.FromSlash("testdata/duplicatesyntheticoneofs/a1.proto:5:9:`Foo` declared multiple times"), ) } @@ -433,7 +431,7 @@ func testFileAnnotations(t *testing.T, relDirPath string, parallelism bool, want for i, annotation := range fileAnnotations { got[i] = annotation.String() } - require.Equal(t, len(want), len(got)) + assert.Equal(t, len(want), len(got)) for i := range want { options := strings.Split(want[i], "||") matched := false diff --git a/private/bufpkg/bufimage/module_file_resolver.go b/private/bufpkg/bufimage/module_file_resolver.go new file mode 100644 index 0000000000..0d51ff0003 --- /dev/null +++ b/private/bufpkg/bufimage/module_file_resolver.go @@ -0,0 +1,193 @@ +// 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(path, 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.LocalPath(), 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( + path string, + readObjectCloser storage.ReadObjectCloser, +) (*source.File, error) { + var buf strings.Builder + if _, err := io.Copy(&buf, readObjectCloser); err != nil { + return nil, err + } + 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 deleted file mode 100644 index 5c1b0adb90..0000000000 --- a/private/bufpkg/bufimage/parser_accessor_handler.go +++ /dev/null @@ -1,166 +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" - "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 fef0414f57..cbd546f020 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(fileAnnotations...), nil + return bufanalysis.NewFileAnnotationSet(nil, fileAnnotations...), nil } // FileAnnotationOption is an option when creating a FileAnnotation.