diff --git a/CHANGELOG.md b/CHANGELOG.md index 2344a53cc2..154916a31c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Fix build failures for modules with a vendored `descriptor.proto`. - Fix LSP incorrectly reporting "edition '2024' not yet fully supported" errors. - Fix CEL compilation error messages in `buf lint` to use the structured error API instead of parsing cel-go's text output. diff --git a/go.mod b/go.mod index 3e9046418f..a8f5f6ba52 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,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.20260416105908-db11c79f9322 + github.com/bufbuild/protocompile v0.14.2-0.20260417153234-65c782f91a0b github.com/bufbuild/protoplugin v0.0.0-20260414125817-25d1d281b46b github.com/cli/browser v1.3.0 github.com/gofrs/flock v0.13.0 diff --git a/go.sum b/go.sum index 72196b695a..153f5acfcf 100644 --- a/go.sum +++ b/go.sum @@ -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.20260416105908-db11c79f9322 h1:0jR8G4rU/roKE9WY0Mh72a0t5GegneOgezPNdzqfI1M= -github.com/bufbuild/protocompile v0.14.2-0.20260416105908-db11c79f9322/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU= +github.com/bufbuild/protocompile v0.14.2-0.20260417153234-65c782f91a0b h1:0q84wQoejn+OL3iUOrOuYrkNfrzwwlcPwDnOhc1ztr8= +github.com/bufbuild/protocompile v0.14.2-0.20260417153234-65c782f91a0b/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU= github.com/bufbuild/protoplugin v0.0.0-20260414125817-25d1d281b46b h1:b7wvo9ZhjLzCp7tGbOUMvgtYTnd33zGSAmMxcdxMnhQ= github.com/bufbuild/protoplugin v0.0.0-20260414125817-25d1d281b46b/go.mod h1:c5D8gWRIZ2HLWO3gXYTtUfw/hbJyD8xikv2ooPxnklQ= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= diff --git a/private/buf/bufformat/formatter.go b/private/buf/bufformat/formatter.go index 81804c9d0e..8a36fa6f87 100644 --- a/private/buf/bufformat/formatter.go +++ b/private/buf/bufformat/formatter.go @@ -438,16 +438,7 @@ func (f *formatter) writeImport(importNode *ast.ImportNode, forceCompact, first // leading comments. f.writeStartMaybeCompact(importNode.Keyword, forceCompact, first && !f.importHasComment(importNode)) f.Space() - // We don't want to write the "public" and "weak" nodes - // if they aren't defined. One could be set, but never both. - switch { - case importNode.Public != nil: - f.writeInline(importNode.Public) - f.Space() - case importNode.Weak != nil: - f.writeInline(importNode.Weak) - f.Space() - case importNode.Modifier != nil: + if importNode.Modifier != nil { f.writeInline(importNode.Modifier) f.Space() } @@ -2507,8 +2498,7 @@ func (f *formatter) importHasComment(importNode *ast.ImportNode) bool { return f.nodeHasComment(importNode.Keyword) || f.nodeHasComment(importNode.Name) || f.nodeHasComment(importNode.Semicolon) || - f.nodeHasComment(importNode.Public) || - f.nodeHasComment(importNode.Weak) + f.nodeHasComment(importNode.Modifier) } func (f *formatter) nodeHasComment(node ast.Node) bool { @@ -2555,14 +2545,15 @@ func (n infoWithTrailingComments) TrailingComments() ast.Comments { // importSortOrder maps import types to a sort order number, so it can be compared and sorted. // Higher values sort first: `import`=3, `import public`=2, `import weak`=1. func importSortOrder(node *ast.ImportNode) int { - switch { - case node.Public != nil: - return 2 - case node.Weak != nil: - return 1 - default: - return 3 + if node.Modifier != nil { + switch node.Modifier.Val { + case "public": + return 2 + case "weak": + return 1 + } } + return 3 } // isOptionImport reports whether the import has the "option" modifier. diff --git a/private/buf/buflsp/deprecate.go b/private/buf/buflsp/deprecate.go index d7e4f7a71b..91d3e7b5cb 100644 --- a/private/buf/buflsp/deprecate.go +++ b/private/buf/buflsp/deprecate.go @@ -42,8 +42,8 @@ func (s *server) getDeprecateCodeAction( slog.Uint64("line", uint64(params.Range.Start.Line)), slog.Uint64("char", uint64(params.Range.Start.Character)), ) - if file.workspace == nil || file.ir == nil || !file.ir.Lowered() { - s.logger.DebugContext(ctx, "deprecate: no workspace or valid IR") + if file.workspace == nil || file.ir == nil { + s.logger.DebugContext(ctx, "deprecate: no workspace or IR") return nil } @@ -62,7 +62,7 @@ func (s *server) getDeprecateCodeAction( checker := newFullNameMatcher(fqnPrefix) edits := make(map[protocol.DocumentURI][]protocol.TextEdit) for _, wsFile := range file.workspace.PathToFile() { - if wsFile.ir == nil || !wsFile.ir.Lowered() { + if wsFile.ir == nil { continue } fileEdits := generateDeprecationEdits(wsFile, checker) @@ -191,7 +191,7 @@ func generateDeprecationEdits(file *file, checker *fullNameMatcher) []protocol.T // getPackageFQN extracts the package FQN components from a file. func getPackageFQN(file *file) ir.FullName { - if file.ir == nil || !file.ir.Lowered() { + if file.ir == nil { return "" } return file.ir.Package() diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 038f41dceb..3d19dd510c 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -56,11 +56,8 @@ type file struct { // Version is an opaque version identifier given to us by the LSP client. This // is used in the protocol to disambiguate which version of a file e.g. publishing // diagnostics or symbols an operating refers to. - version int32 - hasText bool // Whether this file has ever had text read into it. - // The local path of the descriptor.proto file used for compilation. This is stored for diagnostics. - descriptorProtoLocalPath string - + version int32 + hasText bool // Whether this file has ever had text read into it. workspace *workspace // May be nil. objectInfo storage.ObjectInfo // Info in the context of the workspace. @@ -283,10 +280,6 @@ func (f *file) RefreshIR(ctx context.Context) { for i, file := range files { file.ir = results[i].Value - if file.objectInfo.Path() == "google/protobuf/descriptor.proto" { - // Set local path for the descriptor.proto file used for diagnostics. - f.descriptorProtoLocalPath = file.objectInfo.LocalPath() - } if f != file { // Update symbols for imports. file.IndexSymbols(ctx) @@ -356,21 +349,6 @@ func (f *file) IndexSymbols(ctx context.Context) { return } - // The file has not completed lowering, so we cannot continue with symbol indexing here. - // To help the user better understand/diagnose this problem, we surface an additional - // diagnostic here. - if !f.ir.Lowered() { - f.diagnostics = append(f.diagnostics, protocol.Diagnostic{ - Source: serverName, - Severity: protocol.DiagnosticSeverityError, - Message: fmt.Sprintf(`The symbols for this file 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.`, - f.descriptorProtoLocalPath, - ), - }) - return - } - // Throw away all the old symbols and rebuild symbols unconditionally. This is because if // this file depends on a file that has since been modified, we may need to update references. f.symbols = nil diff --git a/private/buf/buflsp/folding_range.go b/private/buf/buflsp/folding_range.go index caeefb95e8..df9d471d2f 100644 --- a/private/buf/buflsp/folding_range.go +++ b/private/buf/buflsp/folding_range.go @@ -26,7 +26,7 @@ import ( // foldingRange generates folding ranges for a file. // It finds messages, services, enums, and multi-line comment blocks. func (s *server) foldingRange(file *file) []protocol.FoldingRange { - if file.ir == nil || !file.ir.Lowered() { + if file.ir == nil { return nil } diff --git a/private/bufpkg/bufimage/build_image.go b/private/bufpkg/bufimage/build_image.go index da5ed5a660..0ccee1972f 100644 --- a/private/bufpkg/bufimage/build_image.go +++ b/private/bufpkg/bufimage/build_image.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "log/slog" + "slices" descriptorv1 "buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go/buf/descriptor/v1" "buf.build/go/standard/xlog/xslog" @@ -167,17 +168,6 @@ func compileImage( } 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"), - ) - } - } - fds, resolver, err := irFilesToFileDescriptorSet(irFiles) if err != nil { return nil, err @@ -257,12 +247,6 @@ func BuildImageFromOpener( } irFiles := results[0].Value - for _, irFile := range irFiles { - if !irFile.Lowered() { - logger.WarnContext(ctx, "file not fully lowered", slog.String("path", irFile.Path())) - } - } - fds, resolver, err := irFilesToFileDescriptorSet(irFiles) if err != nil { return nil, diagnostics, err @@ -296,8 +280,21 @@ func irFilesToFileDescriptorSet(irFiles []*ir.File) (*descriptorpb.FileDescripto // Include Buf's descriptor proto alongside the compiled files so that // ReparseExtensions can recognise [descriptorv1.E_BufSourceCodeInfoExtension] // and convert unknown fields to typed extensions. - resolverFiles := []*descriptorpb.FileDescriptorProto{ - protodesc.ToFileDescriptorProto(descriptorv1.File_buf_descriptor_v1_descriptor_proto), + // + // We only prepend the buf descriptor proto when the compiled files do not already + // contain google/protobuf/descriptor.proto. When a vendored descriptor.proto is + // present in the compiled output, its FileDescriptorSet definition lacks extension + // ranges for the buf extension (field 536000000). protodesc.NewFiles validates + // that extension field numbers fall within declared extension ranges when the + // containing message is resolved (non-placeholder). Adding the buf descriptor + // proto alongside the vendored descriptor.proto causes this validation to fail. + var resolverFiles []*descriptorpb.FileDescriptorProto + if !slices.ContainsFunc(fds.File, func(file *descriptorpb.FileDescriptorProto) bool { + return file.GetName() == "google/protobuf/descriptor.proto" + }) { + resolverFiles = []*descriptorpb.FileDescriptorProto{ + protodesc.ToFileDescriptorProto(descriptorv1.File_buf_descriptor_v1_descriptor_proto), + } } resolverFiles = append(resolverFiles, fds.File...) resolver := protoencoding.NewLazyResolver(resolverFiles...)