v2/protobuilder doesn't automatically detect builders that point to the same descriptor (or path)

[nhatthm]

Problem Statement

I have this example

package main

import (
	"github.com/jhump/protoreflect/v2/protobuilder"
	"google.golang.org/protobuf/reflect/protoreflect"
	"google.golang.org/protobuf/types/known/wrapperspb"
)

func main() {
	dv := &wrapperspb.DoubleValue{}
	sv := &wrapperspb.StringValue{}

	mb := protobuilder.NewMessage("TestMessage")

	dvm, _ := protobuilder.FromMessage(dv.ProtoReflect().Descriptor())
	svm, _ := protobuilder.FromMessage(sv.ProtoReflect().Descriptor())
	doubleType := protobuilder.FieldTypeMessage(dvm)
	stringType := protobuilder.FieldTypeMessage(svm)

	mb.AddField(protobuilder.NewField("value_1", doubleType))
	mb.AddField(protobuilder.NewField("value_2", stringType))

	fb := protobuilder.NewFile("test.proto").
		SetSyntax(protoreflect.Proto3).
		AddMessage(mb)

	_, err := fb.Build()
	if err != nil {
		panic(err)
	}
}

When running this, I get

panic: proto: file "google/protobuf/wrappers.proto" is already registered

After debugging, the reason problem is due to

protoreflect/protobuilder/resolver.go

Lines 60 to 64 in 60df127

	b = getRoot(b)
	if fd, ok := r.resolvedRoots[b]; ok {
	return fd, nil
	}

The module can not detect that the file has been registered here because I have 2 different builders that point to the same file descriptor

protoreflect/protobuilder/resolver.go

Line 89 in 60df127

r.resolvedRoots[b] = fd

That's [somewhat] logically reasonable. However, it burdens end-users with determining and keeping track of file builders. It would be a great help to detect this problem inside the module.

Solution

Unfortunately, I'm not familiar with this module and the proto-reflection to propose any meaningful solutions.

My workaround at the moment

var (
	fileBuilderRegistry    = sync.Map{}
	messageBuilderRegistry = sync.Map{}
)

// RegisterFileBuilder registers a file builder.
func RegisterFileBuilder(fb *protobuilder.FileBuilder) {
	_, ok := fileBuilderRegistry.Load(fb.Path())
	if ok {
		return
	}

	fileBuilderRegistry.Store(fb.Path(), fb)

	for _, cb := range fb.Children() {
		if cb, ok := cb.(*protobuilder.MessageBuilder); ok {
			RegisterMessageBuilder(cb)
		}
	}
}

// RegisterMessageBuilder registers a message builder.
func RegisterMessageBuilder(mb *protobuilder.MessageBuilder) {
	key := string(mb.ParentFile().Package) + "." + string(mb.Name())

	_, ok := messageBuilderRegistry.Load(key)
	if ok {
		return
	}

	messageBuilderRegistry.Store(key, mb)
	RegisterFileBuilder(mb.ParentFile())
}

// FindMessageBuilder finds a message builder by name.
func FindMessageBuilder(name string) *protobuilder.MessageBuilder {
	mb, ok := messageBuilderRegistry.Load(name)
	if !ok {
		return nil
	}

	return mb.(*protobuilder.MessageBuilder) //nolint: forcetypeassert
}

// FromProtoMessage creates a new MessageBuilder from a proto message.
func FromProtoMessage(m proto.Message) (mb *protobuilder.MessageBuilder, err error) {
	mb = FindMessageBuilder(string(m.ProtoReflect().Descriptor().FullName()))
	if mb == nil {
		mb, err = protobuilder.FromMessage(m.ProtoReflect().Descriptor())
		if err != nil {
			return nil, err
		}

		RegisterMessageBuilder(mb)
	}

	return mb, nil
}