diff --git a/private/buf/bufcurl/invoker_test.go b/private/buf/bufcurl/invoker_test.go index 948531987f..63f8b5794e 100644 --- a/private/buf/bufcurl/invoker_test.go +++ b/private/buf/bufcurl/invoker_test.go @@ -18,22 +18,38 @@ import ( "os" "testing" + "github.com/bufbuild/buf/private/buf/buftesting" "github.com/bufbuild/buf/private/pkg/protoencoding" - "github.com/bufbuild/protocompile" + "github.com/bufbuild/protocompile/experimental/incremental" + "github.com/bufbuild/protocompile/experimental/incremental/queries" + "github.com/bufbuild/protocompile/experimental/ir" + "github.com/bufbuild/protocompile/experimental/source" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/descriptorpb" ) func TestCountUnrecognized(t *testing.T) { t.Parallel() - descriptors, err := (&protocompile.Compiler{ - Resolver: &protocompile.SourceResolver{ - ImportPaths: []string{"./testdata"}, - }, - }).Compile(t.Context(), "test.proto") + results, _, err := incremental.Run(t.Context(), incremental.New(), queries.FDS{ + Opener: &source.Openers{&source.FS{FS: os.DirFS("./testdata")}, buftesting.WKTOpener()}, + Session: new(ir.Session), + Workspace: source.NewWorkspace("test.proto"), + }) + require.NoError(t, err) + require.Len(t, results, 1) + require.NoError(t, results[0].Fatal) + // fdp stores option values (e.g. MessageOptions.map_entry) as unknown bytes; + // a wire round-trip materializes them as typed fields so the resolver + // recognizes map fields as maps. Mirrors build_image.go's resolverForFDS. + fdsBytes, err := protoencoding.NewWireMarshaler().Marshal(results[0].Value) + require.NoError(t, err) + fds := new(descriptorpb.FileDescriptorSet) + require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(fdsBytes, fds)) + resolver, err := protoencoding.NewResolver(fds.File...) require.NoError(t, err) - msgType, err := descriptors.AsResolver().FindMessageByName("foo.bar.Message") + msgType, err := resolver.FindMessageByName("foo.bar.Message") require.NoError(t, err) msg := msgType.New() msgData, err := os.ReadFile("./testdata/testdata.txt") diff --git a/private/buf/buftesting/buftesting.go b/private/buf/buftesting/buftesting.go index 1b5557d5a7..76137a0e0d 100644 --- a/private/buf/buftesting/buftesting.go +++ b/private/buf/buftesting/buftesting.go @@ -16,6 +16,7 @@ package buftesting import ( "context" + "errors" "io" "log/slog" "net/http" @@ -25,10 +26,12 @@ import ( "github.com/bufbuild/buf/private/buf/bufprotoc" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/gen/data/datawkt" "github.com/bufbuild/buf/private/pkg/github/githubtesting" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/prototesting" "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/bufbuild/protocompile/experimental/source" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/descriptorpb" ) @@ -156,3 +159,33 @@ func GetProtocFilePathsErr(ctx context.Context, dirPath string, limit int) ([]st } return realFilePaths, nil } + +// WKTOpener returns a [source.Opener] backed by [datawkt.ReadBucket]. Tests that +// compile proto files referencing the well-known types — including the implicit +// descriptor.proto dependency requested by the experimental compiler — can chain +// this opener via [source.Openers] to provide WKT fallback resolution from the +// same in-process bucket production code uses. +func WKTOpener() source.Opener { + return datawktOpener{} +} + +type datawktOpener struct{} + +// Open implements [source.Opener]. +// +// Returns errors wrapping [fs.ErrNotExist] for unknown paths, which +// [source.Openers] uses to fall through to the next opener. +func (datawktOpener) Open(path string) (_ *source.File, retErr error) { + readObjectCloser, err := datawkt.ReadBucket.Get(context.Background(), path) + if err != nil { + return nil, err + } + defer func() { + retErr = errors.Join(retErr, readObjectCloser.Close()) + }() + data, err := io.ReadAll(readObjectCloser) + if err != nil { + return nil, err + } + return source.NewFile(path, string(data)), nil +} diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/field_default_test.go b/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/field_default_test.go index 57cdc1addf..5e1187262f 100644 --- a/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/field_default_test.go +++ b/private/bufpkg/bufcheck/bufcheckserver/internal/bufcheckserverhandle/field_default_test.go @@ -18,9 +18,15 @@ import ( "math" "testing" - "github.com/bufbuild/protocompile" + "github.com/bufbuild/buf/private/buf/buftesting" + "github.com/bufbuild/buf/private/pkg/protoencoding" + "github.com/bufbuild/protocompile/experimental/incremental" + "github.com/bufbuild/protocompile/experimental/incremental/queries" + "github.com/bufbuild/protocompile/experimental/ir" + "github.com/bufbuild/protocompile/experimental/source" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/descriptorpb" ) func TestDefaultsEqual(t *testing.T) { @@ -111,16 +117,27 @@ func TestGetDefault(t *testing.T) { V1 = 1; V123 = 123; }` - compiler := &protocompile.Compiler{ - Resolver: &protocompile.SourceResolver{ - Accessor: protocompile.SourceAccessorFromMap(map[string]string{ - "test.proto": testFile, - }), - }, - } - results, err := compiler.Compile(t.Context(), "test.proto") + opener := source.NewMap(nil) + opener.Add("test.proto", testFile) + results, _, err := incremental.Run(t.Context(), incremental.New(), queries.FDS{ + Opener: &source.Openers{opener, buftesting.WKTOpener()}, + Session: new(ir.Session), + Workspace: source.NewWorkspace("test.proto"), + }) + require.NoError(t, err) + require.Len(t, results, 1) + require.NoError(t, results[0].Fatal) + // fdp stores option values as unknown bytes; a wire round-trip + // materializes them as typed fields. Mirrors build_image.go. + fdsBytes, err := protoencoding.NewWireMarshaler().Marshal(results[0].Value) + require.NoError(t, err) + fds := new(descriptorpb.FileDescriptorSet) + require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(fdsBytes, fds)) + resolver, err := protoencoding.NewResolver(fds.File...) + require.NoError(t, err) + fd, err := resolver.FindFileByPath("test.proto") require.NoError(t, err) - msg := results[0].Messages().ByName("A") + msg := fd.Messages().ByName("A") assert.Equal(t, fieldDefault{ diff --git a/private/pkg/protosourcepath/protosourcepath_test.go b/private/pkg/protosourcepath/protosourcepath_test.go index 31403aedff..0d2e5be148 100644 --- a/private/pkg/protosourcepath/protosourcepath_test.go +++ b/private/pkg/protosourcepath/protosourcepath_test.go @@ -15,11 +15,19 @@ package protosourcepath import ( + "os" "testing" - "github.com/bufbuild/protocompile" + "github.com/bufbuild/buf/private/buf/buftesting" + "github.com/bufbuild/buf/private/pkg/protoencoding" + "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/source" "github.com/stretchr/testify/require" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/descriptorpb" ) func TestGetAssociatedSourcePathsProto2(t *testing.T) { @@ -376,14 +384,28 @@ func testGetAssociatedSourcePaths( sourcePathToExpectedAssociatedPaths map[string][]protoreflect.SourcePath, excludeChildAssociatedPaths bool, ) { - compiler := protocompile.Compiler{ - Resolver: &protocompile.SourceResolver{}, - SourceInfoMode: protocompile.SourceInfoStandard, - } - files, err := compiler.Compile(t.Context(), testFilePath) + var fdpOptions fdp.Options + fdpOptions.Apply(fdp.IncludeSourceCodeInfo(true)) + results, _, err := incremental.Run(t.Context(), incremental.New(), queries.FDS{ + Opener: &source.Openers{&source.FS{FS: os.DirFS(".")}, buftesting.WKTOpener()}, + Session: new(ir.Session), + Workspace: source.NewWorkspace(testFilePath), + Options: fdpOptions, + }) + require.NoError(t, err) + require.Len(t, results, 1) + require.NoError(t, results[0].Fatal) + // fdp stores option values as unknown bytes; a wire round-trip + // materializes them as typed fields. Mirrors build_image.go. + fdsBytes, err := protoencoding.NewWireMarshaler().Marshal(results[0].Value) + require.NoError(t, err) + fds := new(descriptorpb.FileDescriptorSet) + require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(fdsBytes, fds)) + resolver, err := protoencoding.NewResolver(fds.File...) + require.NoError(t, err) + fd, err := resolver.FindFileByPath(testFilePath) require.NoError(t, err) - require.Len(t, files, 1, "expect only one test file") - sourceLocations := files[0].SourceLocations() + sourceLocations := fd.SourceLocations() // SourceLocations are indexed starting from 1 for i := 1; i < sourceLocations.Len(); i++ { sourceLocation := sourceLocations.Get(i) diff --git a/private/pkg/protostat/protostat.go b/private/pkg/protostat/protostat.go index 5298255cca..a575a32c9a 100644 --- a/private/pkg/protostat/protostat.go +++ b/private/pkg/protostat/protostat.go @@ -18,9 +18,11 @@ import ( "context" "io" - "github.com/bufbuild/protocompile/ast" - "github.com/bufbuild/protocompile/parser" - "github.com/bufbuild/protocompile/reporter" + "github.com/bufbuild/protocompile/experimental/ast" + "github.com/bufbuild/protocompile/experimental/parser" + "github.com/bufbuild/protocompile/experimental/report" + "github.com/bufbuild/protocompile/experimental/seq" + "github.com/bufbuild/protocompile/experimental/source" ) // Stats represents some statistics about one or more Protobuf files. @@ -55,33 +57,28 @@ type FileWalker interface { // See the packages protostatos and protostatstorage for helpers for the // os and storage packages. func GetStats(ctx context.Context, fileWalker FileWalker) (*Stats, error) { - handler := reporter.NewHandler( - reporter.NewReporter( - func(reporter.ErrorWithPos) error { - // never aborts - return nil - }, - nil, - ), - ) statsBuilder := newStatsBuilder() if err := fileWalker.Walk( ctx, func(file io.Reader) error { - // This can return an error and non-nil AST. - // We do not need the filePath because we do not report errors. - astRoot, err := parser.Parse("", file, handler) - if astRoot == nil { - // No AST implies an I/O error trying to read the - // file contents. No stats to collect. + data, err := io.ReadAll(file) + if err != nil { return err } - if err != nil { - // There was a syntax error, but we still have a partial - // AST we can examine. - statsBuilder.FilesWithSyntaxErrors++ + r := &report.Report{Options: report.Options{SuppressWarnings: true}} + astFile, _ := parser.Parse("", source.NewFile("", string(data)), r) + if astFile == nil { + // Parse only returns a nil file in pathological cases; bail + // without recording stats, matching the legacy I/O-error path. + return nil + } + for _, d := range r.Diagnostics { + if d.Level() <= report.Error { + statsBuilder.FilesWithSyntaxErrors++ + break + } } - examineFile(statsBuilder, astRoot) + examineFile(statsBuilder, astFile) return nil }, ); err != nil { @@ -118,132 +115,154 @@ func MergeStats(statsSlice ...*Stats) *Stats { type statsBuilder struct { *Stats - packages map[ast.Identifier]struct{} + packages map[string]struct{} } func newStatsBuilder() *statsBuilder { return &statsBuilder{ Stats: &Stats{}, - packages: make(map[ast.Identifier]struct{}), + packages: make(map[string]struct{}), } } -func examineFile(statsBuilder *statsBuilder, fileNode *ast.FileNode) { +func examineFile(statsBuilder *statsBuilder, file *ast.File) { statsBuilder.Files++ - for _, decl := range fileNode.Decls { - switch decl := decl.(type) { - case *ast.PackageNode: - statsBuilder.packages[decl.Name.AsIdentifier()] = struct{}{} - case *ast.MessageNode: - examineMessage(statsBuilder, &decl.MessageBody, decl) - case *ast.EnumNode: - examineEnum(statsBuilder, decl) - case *ast.ExtendNode: - examineExtend(statsBuilder, decl) - case *ast.ServiceNode: + for decl := range seq.Values(file.Decls()) { + if pkg := decl.AsPackage(); !pkg.IsZero() { + statsBuilder.packages[pkg.Path().Canonicalized()] = struct{}{} + continue + } + def := decl.AsDef() + if def.IsZero() { + continue + } + switch def.Classify() { + case ast.DefKindMessage: + examineMessage(statsBuilder, def.AsMessage().Body, def) + case ast.DefKindEnum: + examineEnum(statsBuilder, def.AsEnum().Body, def) + case ast.DefKindExtend: + examineExtend(statsBuilder, def.AsExtend().Body) + case ast.DefKindService: statsBuilder.Services++ - for _, decl := range decl.Decls { - rpcNode, ok := decl.(*ast.RPCNode) - if ok { - statsBuilder.RPCs++ - statsBuilder.Types++ - if isDeprecated(rpcNode) { - statsBuilder.DeprecatedRPCs++ - } + for innerDecl := range seq.Values(def.AsService().Body.Decls()) { + innerDef := innerDecl.AsDef() + if innerDef.IsZero() || innerDef.Classify() != ast.DefKindMethod { + continue + } + statsBuilder.RPCs++ + statsBuilder.Types++ + if isDeprecated(innerDef) { + statsBuilder.DeprecatedRPCs++ } } } } } -// examineMessage examines a message body and updates stats. -// The node parameter is used to check for deprecated options, and can be a MessageNode or GroupNode. -func examineMessage(statsBuilder *statsBuilder, messageBody *ast.MessageBody, node ast.NodeWithOptions) { +// examineMessage examines a message or group body and updates stats. The def is the +// owning ast.DeclDef (message or group), used for the deprecation check. +func examineMessage(statsBuilder *statsBuilder, body ast.DeclBody, def ast.DeclDef) { statsBuilder.Messages++ statsBuilder.Types++ - if node != nil && isDeprecated(node) { + if isDeprecated(def) { statsBuilder.DeprecatedMessages++ } - for _, decl := range messageBody.Decls { - switch decl := decl.(type) { - case *ast.FieldNode, *ast.MapFieldNode: + for decl := range seq.Values(body.Decls()) { + innerDef := decl.AsDef() + if innerDef.IsZero() { + continue + } + switch innerDef.Classify() { + case ast.DefKindField: statsBuilder.Fields++ - case *ast.GroupNode: + case ast.DefKindGroup: statsBuilder.Fields++ - examineMessage(statsBuilder, &decl.MessageBody, decl) - case *ast.OneofNode: - for _, ooDecl := range decl.Decls { - switch ooDecl := ooDecl.(type) { - case *ast.FieldNode: + examineMessage(statsBuilder, innerDef.AsGroup().Body, innerDef) + case ast.DefKindOneof: + for ooDecl := range seq.Values(innerDef.AsOneof().Body.Decls()) { + ooDef := ooDecl.AsDef() + if ooDef.IsZero() { + continue + } + switch ooDef.Classify() { + case ast.DefKindField: statsBuilder.Fields++ - case *ast.GroupNode: + case ast.DefKindGroup: statsBuilder.Fields++ - examineMessage(statsBuilder, &ooDecl.MessageBody, ooDecl) + examineMessage(statsBuilder, ooDef.AsGroup().Body, ooDef) } } - case *ast.MessageNode: - examineMessage(statsBuilder, &decl.MessageBody, decl) - case *ast.EnumNode: - examineEnum(statsBuilder, decl) - case *ast.ExtendNode: - examineExtend(statsBuilder, decl) + case ast.DefKindMessage: + examineMessage(statsBuilder, innerDef.AsMessage().Body, innerDef) + case ast.DefKindEnum: + examineEnum(statsBuilder, innerDef.AsEnum().Body, innerDef) + case ast.DefKindExtend: + examineExtend(statsBuilder, innerDef.AsExtend().Body) } } } -func examineEnum(statsBuilder *statsBuilder, enumNode *ast.EnumNode) { +func examineEnum(statsBuilder *statsBuilder, body ast.DeclBody, def ast.DeclDef) { statsBuilder.Enums++ statsBuilder.Types++ - if isDeprecated(enumNode) { + if isDeprecated(def) { statsBuilder.DeprecatedEnums++ } - for _, decl := range enumNode.Decls { - _, ok := decl.(*ast.EnumValueNode) - if ok { + for decl := range seq.Values(body.Decls()) { + innerDef := decl.AsDef() + if !innerDef.IsZero() && innerDef.Classify() == ast.DefKindEnumValue { statsBuilder.EnumValues++ } } } -func examineExtend(statsBuilder *statsBuilder, extendNode *ast.ExtendNode) { - for _, decl := range extendNode.Decls { - switch decl := decl.(type) { - case *ast.FieldNode: +func examineExtend(statsBuilder *statsBuilder, body ast.DeclBody) { + for decl := range seq.Values(body.Decls()) { + innerDef := decl.AsDef() + if innerDef.IsZero() { + continue + } + switch innerDef.Classify() { + case ast.DefKindField: statsBuilder.Extensions++ - case *ast.GroupNode: + case ast.DefKindGroup: statsBuilder.Extensions++ - examineMessage(statsBuilder, &decl.MessageBody, decl) + examineMessage(statsBuilder, innerDef.AsGroup().Body, innerDef) } } } -func isDeprecated(node ast.NodeWithOptions) bool { - // GroupNode's Options can be nil. - if groupNode, ok := node.(*ast.GroupNode); ok && groupNode.Options == nil { +// isDeprecated reports whether def is marked `deprecated = true`, either via +// a compact option or a body-level `option deprecated = true;`. +func isDeprecated(def ast.DeclDef) bool { + for entry := range seq.Values(def.Options().Entries()) { + if entry.Path.IsIdents("deprecated") && exprIsTrue(entry.Value) { + return true + } + } + body := def.Body() + if body.IsZero() { return false } - deprecated := false - node.RangeOptions(func(opt *ast.OptionNode) bool { - // Check if this is the "deprecated" option (simple name, not extension) - if opt.Name == nil || len(opt.Name.Parts) != 1 { - return true // continue - } - part := opt.Name.Parts[0] - if part.IsExtension() { - return true // continue - } - if part.Value() != "deprecated" { - return true // continue + for decl := range seq.Values(body.Decls()) { + inner := decl.AsDef() + if inner.IsZero() || inner.Classify() != ast.DefKindOption { + continue } - // Check if the value is true - val := opt.Val.Value() - switch v := val.(type) { - case bool: - deprecated = v - case ast.Identifier: - deprecated = string(v) == "true" + opt := inner.AsOption() + if opt.Path.IsIdents("deprecated") && exprIsTrue(opt.Value) { + return true } - return false // stop iterating once we find deprecated option - }) - return deprecated + } + return false +} + +// exprIsTrue reports whether expr is the identifier `true`. +func exprIsTrue(expr ast.ExprAny) bool { + path := expr.AsPath() + if path.IsZero() { + return false + } + return path.Path.IsIdents("true") }