It would be nice if the protoparse package could preserve comments and element position information and store them in the file descriptor's source_code_info, just like protoc does when writing a descriptor set and --include_source_info is given.

unmarshal map error，

Unmarshal body to inner.HelloResponse: unexpected EOF

There is a TODO in the mergeFrom method about the ugly way unrecognized extensions are converted. A fix landed in the proto runtime, golang/protobuf#483, so we can now "fix" the logic in protoreflect to be more efficient.

At least for Messages in the desc package (and probably for other types, but I'm not sure), the GetFullyQualifiedName() method currently returns values that lack a leading dot, so they aren't actually fully-qualified.

This is a twin issue to #150. Copy-pasting from there:

Leading dots are the way that a path specification is marked as fully-qualified, as documented in the parser: https://github.com/protocolbuffers/protobuf/blob/ba8692fbade4ba329cc4531e286ab5a8e0821d97/src/google/protobuf/compiler/parser.cc#L2157

This is also documented in FieldDecriptorProto.

The latest protoc adds support for reserved numbers and names for enum values.

This will need to be supported in the desc/protoparse package, so files that define these reserved numbers and names can be successfully parsed/compiled. They will also need to be supported in upcoming desc/builder package, to allow creation of enums with reserved ranges and also to not lose fidelity when converting from a descriptor (which may have reserved ranges defined) to a builder and then back.

This will have to wait until golang/protobuf is updated so that the generated Go code for descriptor.proto supports this new functionality. That likely won't happen until whenever this new protoc feature is actually released (probably v3.5).

I'm noticing a lot of repetition in my code due to wanting to set a field by name, resulting in this pattern appearing repeatedly:
myMessage.SetField(myMessage.FindFieldDescriptorByName("myfield"), myValue)

I've added some helper methods which changes the code to a neater:
myMessage.SetFieldByName("myfield", myvalue)

Example method added
var UnknownFieldNameError = errors.New("Unknown field name")
...
func (m *Message) SetFieldByName(name string, val interface{}) {
if err := m.TrySetFieldByName(name, val); err != nil {
panic(err.Error())
}
}

func (m *Message) TrySetFieldByName(name string, val interface{}) error {
fd := m.md.FindFieldByName(name)
if fd == nil {
return UnknownFieldNameError
}
return m.setField(fd, val)
}

I'm happy to extend that ByName functionality to the other methods (GetField, HasField, Clearfield etc and submit a PR, just checking if you agree with adding it before I do the work + tests.

A previously merged fix (#169) addressed the file path matching problem for loading linked-in local registered files (#147). It turns out, the same sort of thing is needed for remote files, when using server reflection.

The way the grpcreflect package interacts with the server reflection interface is to first use one of the various methods to get an "entry point" into the descriptors. But these returns one file descriptor at a time. So to link a file descriptor, we have to ask for its imports, and the way that is done is by name. It turns out that this can encounter the exact same kinds of problems: the name (and path) of a given file, for querying for the descriptor by name, may differ from how it is referenced by import statements in other files. So, when resolving a file, we ask for the observed import path, for which the server may reply with a "not found" error if the name and path don't match.

So we basically need an ImportResolver for the grpcreflect package -- when linking a file, it will try to map import paths to "canonical registered paths" when asking the remote server for that file.

Some of this logic is indirectly exercised via some of the tests (and the grpcurl project actually relies on these functions, and some of its tests exercise even more of the code).

But recently found bugs like addressed in #146 show that there are still some ugly corners in here. There are TODOs for test cases for these methods, and actually addressing these TODOs is rising in urgency.

The protoparse library fails when validating a proto3 file with a map field, when trying to validate the synthesized {FieldName}Entry message:

field FieldNameEntry.key: field has label LABEL_OPTIONAL, but proto3 should omit labels other than 'repeated'

You can repro this by adding a line to internal/testprotos/desc_test_proto3.proto like:

// Inserted after "snafu", or in any message.
map<string, string> fieldName = 5;

and then running the parser unit tests.

It looks like fixing would involve passing the map_entry value from MessageOptions down to the validateField function.

I'm happy to make that change if you think it's reasonable.

First of all I do apologise if I'm missing something very obvious or if I am misunderstanding or abusing things in this wonderful package.

I'm trying to setup a dynamic registry with a fetcher that is able to load protos from their source files without much success.

I've managed to get the dynamic registry to work when the fetcher manually constructs and returns a *ptype.Type but I'm struggling to figure out how to convert the result from the protoparse.Parser into a *ptype.Type. It seems to be dying trying to get the options, but I'm probably doing something very wrong here.

ps. Just a weird side-note, I cannot for the life of me debug this test using delve, it just dies with Failed to produce stack trace! -- will need to dig into this at some point :D

Thank you very much in advance for your help and also for this amazing package.
George

Testcase

package dynamic

import (
	"bytes"
	"errors"
	"fmt"
	"io"
	"io/ioutil"
	"testing"

	"github.com/davecgh/go-spew/spew"
	"github.com/golang/protobuf/proto"
	"github.com/jhump/protoreflect/desc/protoparse"
	"github.com/jhump/protoreflect/dynamic"
	"github.com/jhump/protoreflect/dynamic/msgregistry"
	"github.com/stretchr/testify/assert"
)

func TestSomethingWeird2(t *testing.T) {
	protoSource := `
		syntax = "proto2";
		package foo;
		message Bar {
			optional string abc = 1;
			optional int32 def = 2;
		}`

	registry := (&msgregistry.MessageRegistry{}).
		WithMessageFactory(dynamic.NewMessageFactoryWithDefaults())

	reader := func(filename string) (io.ReadCloser, error) {
		if filename != "https://test/foo.Bar" {
			return nil, errors.New("not found")
		}
		return ioutil.NopCloser(bytes.NewReader([]byte(protoSource))), nil
	}

	parser := &protoparse.Parser{
		Accessor: reader,
	}

	fetcher := func(url string, enum bool) (proto.Message, error) {
		fmt.Println(url)
		if url != "https://test/foo.Bar" {
			return nil, errors.New("not found")
		}

		fds, err := parser.ParseFiles(url)
		if err != nil {
			return nil, err
		}

		if len(fds) == 0 {
			return nil, errors.New("missing descs")
		}

		fooMessage := fds[0].FindMessage("foo.Bar")
		spew.Dump(fooMessage)

		fooPType := registry.MessageAsPType(fooMessage)
		spew.Dump(fooPType)

		return fooPType, nil
	}

	registry = registry.WithFetcher(fetcher)

	md, err := registry.FindMessageTypeByUrl("test/foo.Bar")
	assert.NoError(t, err)
	assert.NotNil(t, md)

	if md == nil {
		t.FailNow()
	}

	dm := dynamic.NewMessageFactoryWithDefaults().NewMessage(md).(*dynamic.Message)
	dm.SetFieldByNumber(1, "fubar")
	dm.SetFieldByNumber(2, int32(123))
	dm.SetFieldByNumber(1, "snafu")
	dm.SetFieldByNumber(2, int32(456))

	js, err := dm.MarshalJSON()
	assert.NoError(t, err)
	fmt.Println(string(js))
}

Result

=== RUN   TestSomethingWeird
(*desc.MessageDescriptor)(0xc0001ebb20)(name:"Bar" field:<name:"abc" number:1 label:LABEL_OPTIONAL type:TYPE_STRING json_name:"abc" > field:<name:"def" number:2 label:LABEL_OPTIONAL type:TYPE_INT32 json_name:"def" > )
--- FAIL: TestSomethingWeird2 (0.00s)
panic: reflect: call of reflect.Value.Type on zero Value [recovered]
	panic: reflect: call of reflect.Value.Type on zero Value

goroutine 19 [running]:
testing.tRunner.func1(0xc00014cb00)
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:792 +0x387
panic(0x1436720, 0xc0002184c0)
	/usr/local/Cellar/go/1.11/libexec/src/runtime/panic.go:513 +0x1b9
reflect.Value.Type(0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/Cellar/go/1.11/libexec/src/reflect/value.go:1713 +0x16c
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*MessageRegistry).options(0xc000149300, 0x1533bc0, 0x0, 0x10, 0x10, 0x149bea0)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/message_registry.go:596 +0x10c
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*MessageRegistry).fieldAsPType(0xc000149300, 0xc00020da40, 0x2)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/message_registry.go:456 +0x69
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*MessageRegistry).MessageAsPType(0xc000149300, 0xc0001ebb20, 0x1)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/message_registry.go:438 +0xcb
command-line-arguments.TestSomethingWeird2.func2(0xc0001e91c0, 0x14, 0x0, 0xc0001cef60, 0x30, 0x30, 0x1496840)
	/Users/geoah/src/nope.io/go/playground/dynamic/dynamic3_test.go:62 +0x33c
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*resolutionContext).addType(0xc000153dc0, 0xc0001e91c0, 0x14, 0xc0000bf9e0, 0x187b700, 0x0, 0x0)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/ptype_resolver.go:445 +0xbb
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*typeResolver).resolveUrlToMessageDescriptor(0xc000149308, 0xc0001e91c0, 0x14, 0x0, 0x0, 0x0)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/ptype_resolver.go:282 +0x278
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry.(*MessageRegistry).FindMessageTypeByUrl(0xc000149300, 0x14b7940, 0xc, 0x105e370, 0xc00004c720, 0xe4b2a72b)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/msgregistry/message_registry.go:165 +0x91
command-line-arguments.TestSomethingWeird2(0xc00014cb00)
	/Users/geoah/src/nope.io/go/playground/dynamic/dynamic3_test.go:70 +0x21d
testing.tRunner(0xc00014cb00, 0x14cebd0)
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:878 +0x353
FAIL	command-line-arguments	0.018s

Due to golang/protobuf#567, file descriptor registration in the protobuf runtime is error-prone. If a file uses a relative path in an import statement that does not precisely match the path given to protoc to generate that file's corresponding Go code, then linking of descriptors can fail.

A possible work-around is to provide an API for users to register aliases for file paths. For example, a file may have been compiled with protoc and thus registered at runtime as foo.proto, but another file might import the same file via import "misc/foo.proto"; (e.g. with misc/ added to path). The API would allow users to register "misc/foo.proto" as an alias for "foo.proto". Since registration is global, the aliases can probably safely be global, too (e.g. registered in a package variable map [protected by mutex for thread-safety] that is queried from any calls to desc.LoadFileDescriptor).

Hey! I love this library! Thank you so much for its existence!

I'm having a strange result calling desc.LoadMessageDescriptorForMessage on a dynamic message with a MessageDescriptor that came from builder. Here's a reproducible test case:

package prtc

import (
	"github.com/jhump/protoreflect/desc"
	"github.com/jhump/protoreflect/desc/builder"
	"github.com/jhump/protoreflect/dynamic"
	"testing"
)

func TestDescriptorGet(t *testing.T) {
	md, err := builder.NewMessage("Person").
		AddField(builder.NewField("name", builder.FieldTypeString())).
		Build()

	if err != nil {
		t.Fatalf("error building Person: %v", err)
	}

	msg := dynamic.NewMessage(md)
	msg.SetFieldByNumber(1, "Cailin")

	_, err = desc.LoadMessageDescriptorForMessage(msg)
	if err != nil {
		t.Fatalf("error loading Person's message descriptor: %v", err)
	}
}

And the output:

$ go test
--- FAIL: TestDescriptorGet (0.00s)
panic: Failed to get descriptor path for Person [recovered]
        panic: Failed to get descriptor path for Person

goroutine 5 [running]:
testing.tRunner.func1(0xc4200c00f0)
        /usr/local/opt/go/libexec/src/testing/testing.go:742 +0x29d
panic(0x11aa580, 0xc42000e650)
        /usr/local/opt/go/libexec/src/runtime/panic.go:505 +0x229
github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/dynamic.(*Message).Descriptor(0xc420088d80, 0xc420088660, 0x11f9d07, 0x6, 0x13320e0, 0x11f8de0, 0xc400000000)
        /Users/kivikakk/.go/src/github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/dynamic/dynamic_message.go:176 +0x7bc
github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/desc.loadMessageDescriptorForTypeLocked(0x11f9d07, 0x6, 0x13f3020, 0xc420088d80, 0xc420088d80, 0x0, 0x0)
        /Users/kivikakk/.go/src/github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/desc/descriptor.go:1785 +0x76
github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/desc.LoadMessageDescriptorForMessage(0x1221e00, 0xc420088d80, 0x0, 0x0, 0x0)
        /Users/kivikakk/.go/src/github.com/kivikakk/prtc/vendor/github.com/jhump/protoreflect/desc/descriptor.go:1765 +0x10b
github.com/kivikakk/prtc.TestDescriptorGet(0xc4200c00f0)
        /Users/kivikakk/.go/src/github.com/kivikakk/prtc/main_test.go:22 +0x18f
testing.tRunner(0xc4200c00f0, 0x12057e0)
        /usr/local/opt/go/libexec/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
        /usr/local/opt/go/libexec/src/testing/testing.go:824 +0x2e0
exit status 2
FAIL    github.com/kivikakk/prtc        0.010s

You can fetch this from github.com/kivikakk/prtc if it makes life easier.

I tried hacking the code in dynamic_message.go a bit to see what the deal is — this case here isn't finding anything:

This is because Person has no nested message types, but we start our search at m.md i.e. Person itself, and don't match anything with that name. I tried starting the search from m.md.GetFile() instead, just to see if it'd work, and it sure didn't; the path code presumes there'll be at least 2 items. And when I changed it to not presume that, … actually, wait a second:

j starts off as len(path). We try to read path[j]. Won't that always fail, as it'll be one past the end of the slice?

The well-known types are all, on the surface, just messages. They are all defined in the google.protobuf package (that is their protobuf package, not the name of the Go package). You can find their sources in the various .proto files in the golang/protobuf repo. (Note that the canonical source for these is in the main protobuf repo).

Currently, if a message that has Any fields (for example; also applies to all other well-known types) is unmarshalled into a dynamic message, those fields' values will be unmarshalled into child *dynamic.Message objects. This means one has to explicitly convert it to a generated Any struct before being able to use the facilities of the ptypes package.

Two possible approaches to improving the situation:

1. Always unmarshal well-known types into their corresponding generated structs (in the ptypes package and sub-packages). This is the simplest solution.
2. Provide a mechanism where the user can provide a known-type registry. This registry maps fully-qualified message names to their generated types. When unmarshalling a message, unmarshal into that type when possible, instead of always unmarshalling into a *dynamic.Message.

The latter approach is more complicated but also much more flexible. The dynamic message already supports setting field values to generated structs (vs. requiring they be set to instances of *dynamic.Message). The proposed change would allow this to automatically be done for certain message types during de-serialization. It would be trivial to provide an out-of-the-box known-type registry that represents the well-known types and one that knows about all linked generated message types (e.g. it could call through to proto.MessageType).

An additional concern with the Any type is mirroring the functionality in ptypes/any.go. That package provides a way to extract the represented message from an Any type. However, it only works with generated Go structs. A dynamic analog would need some sort of message registry, which maps fully-qualified message names to a *desc.MessageDescriptor for that type. Such a registry might look a lot like dynamic.ExtensionRegistry, except it stores message descriptors instead of extension field descriptors. This would allow a user to extract the represented message from an Any type as a *dynamic.Message.

If we go with option 2 above, then the message registry probably also wants a reference to a known-type registry, so that it can extract values from Any types using either known/generated Go structs or dynamic messages.

I have multiple instances a service and I would like to only have to resolve the service descriptor once and cache the symbols for use across different connections. Are there issues with sharing the symbol cache across different connections? Are there plans to allow users to plug in a cache on a per-connection basis?

After parsing proto files none of the descriptors (services, methods, fields, etc) have SourceCodeInfo, i.e. .GetSourceInfo() returns nil. I looked through the parser code and it seems like sourceCodeInfo parsing (generateSourceCodeInfo method) happens after files linking, which is too late, since all descriptors are already created.

Feature request. This may exist in a place I haven't seen yet.

It would be cool to be able to dynamically create or modify a FileDescriptor, then write out the new .proto definition.

This would be even cooler if protoparse-generated descriptors included source_code_info so that round-tripping modifications were possible without losing comment metadata.

I have an app working that is reading .proto files and is able to create new messages by parsing bytes using a factory.

This gives me a dynamic message that I can use for things such as person.GetFieldByName("id"), but when in a JavaScript context such as with goja, I'd like to be able to just do person.id.
So far, the only way I've found is to MarshalJSON the object in the Go code, cast it to a string and store it as an object in the script's context, then call JSON.parse() inside the script. Just wondering if anyone has recommendations for a better method.

A map-typed field does not correctly unmarshal from text; it seems to ignore all input.

Given a message like:

message TestMessage {
  map<string, int32> mapValue = 1;
}

A dynamic message should be able to parse the text:

mapValue: <key: "bar" value: 1>
mapValue: <key: "foo" value: 2>

Passing that text to proto.UnmarshalText with a dynamic message produces no change in the saved map data (but also returns no error).

Note that serialization (proto.MarshalText) produces correct output when given a message with a map value set.

Message definition:

message point {
	double x = 1;
	double y = 2;
	double z = 3;
}

Message instantiation:

msg := pointline.Point{ X : 12, Y : -12, Z : 0 } // note Z field is zero

Expected output from dynamic.MarshalJSON:

{"x":12,"y":-12,"z":0}

Got:

{"x":12,"y":-12}

First of all, thank you very much for your work on this lib. It's trully awesome and definitely something that is missing on Go. Given that, i've been struggling with the following task: how can i make dynamic messages from protos that import other protos?

Lets say i have Proto A, which imports Proto b in its file, i've already seen that simply trying to use the proto descriptor of A isn't enough, since it says not enough dependencies, so I wanted to know, is it already supported to - somehow - use descriptors to create dyn. messages of protos with import? And if not, what could I study in order to do so?

Again, thank you very much, this repo is awesome!

While using the latest code to use the Protoparse, I noticed that I am unable to compile due to an overflowing int value. Problem is specific to using GOARCH=386, it works ok on amd64.

I've been experimenting with adding fields to existing message descriptors on the fly, there seems no way in which to access the functionality without creating a shim to export the createFieldDescriptor function.

With that exposed, I can take a parent message descriptor + file descriptor and add a new field

alias := descriptor.FieldDescriptorProto_TYPE_DOUBLE
fdp := descriptor.FieldDescriptorProto{
Name: proto.String("examplename),
Number: proto.Int(index),
Label: descriptor.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: &alias,
}
return desc.CreateFieldDescriptor(fileDescriptor, parentDescriptor, "", &fdp)

This then gives me a FieldDescriptor I can further use and places it onto the parent message - index here has to be a suitable position e.g. not clashing with existing fields.

Is the functionality accessible in another form, or should this function be exported?

The lexer was only checking for \n line endings, which mean't that files with \r\n or \r (rarer) cause the lexer to throw syntax error: unexpected $unk

Added a \r check in the same place as the \n, tests then pass when I've altered the line endings.

#51

There are some other users of just \n but they did not cause the lexer to fail to parse the proto file / tests to fail.

Hi,
I need to parse a string of simple protobuf syntax.

Currently what I do is create a file, parse the file and delete the file. Are there any functionality in reflect that I can use?

Thanks

protoc produces the FileDescriptorSet in reverse topological sort order when the --descriptor_set_out and --include_imports are set.

This means the input file on the command line is last, preceded by its imports.

dynamic.CreateFileDescriptorFromSet expects the input file to be first, followed by imports. This required swapping the first and last element of the File field of FileDescriptorSet.

Close if you think the documented behavior is fine. This is just a note.

Input

message msg {
	option (someoption) = "bar";
	uint64 id = 1;
}

Output:

{,
  "id": 12,
}

I'm trying to wrap my head around how to get the message registry to fall back to well known types when using the fetcher.

When using a custom fetcher it tries to fetch the types that it doesn't know about, and then the parser tries to find their dependencies which in our case is google/protobuf/any.proto. (testcase A).

Normally when the parser asks the accessor for a type that it doesn't know about, it checks the well known types after that. (testcase B)

But this doesn't to be the case when I do this through the fetcher.

In both my test cases the accessor returns an error when it doesn't find something (tried with returning nil, nil as in some places in the lib with worst results hehe).

ps. Your fetcher tests seem to be able to get the duration wkt so I assume it's not a bug and that I'm just missing something or that I'm just doing wrong.

@jhump thank you again for your help and sorry for the weird questions and long tests. hopefully I'll get a better grasp of the lib and stop spamming :D

Testcase A

package dynamic

import (
	"bytes"
	"errors"
	"fmt"
	"io"
	"io/ioutil"
	"strings"
	"testing"

	"github.com/davecgh/go-spew/spew"
	"github.com/golang/protobuf/jsonpb"
	"github.com/golang/protobuf/proto"
	"github.com/jhump/protoreflect/desc/protoparse"
	"github.com/jhump/protoreflect/dynamic"
	"github.com/jhump/protoreflect/dynamic/msgregistry"
	"github.com/stretchr/testify/assert"
)

var ErrNotFound = errors.New("not found")

var files = map[string]string{
	"https://test/core.wrapper.proto": `
		syntax = "proto3";
		package core;
		import "google/protobuf/any.proto";
		message wrapper {
			string abc = 1;
			int32 def = 2;
			repeated google.protobuf.Any extras = 3;
		}`,
	"https://test/core.one.proto": `
		syntax = "proto3";
		package core;
		message one {
			string abc = 1;
			int32 def = 2;
		}`,
	"https://test/core.two.proto": `
		syntax = "proto3";
		package core;
		message two {
			string ghi = 1;
			int32 jkl = 2;
		}`,
}

func fqnFromURL(url string) string {
	// TODO replace with regex or find something in protoreflect that does this?
	ps := strings.Split(url, "/")
	fqn := ps[0]
	if i := len(ps); i > 1 {
		fqn = ps[i-1]
	}
	if strings.HasSuffix(fqn, ".proto") {
		fqn = fqn[:len(fqn)-6]
	}
	return fqn
}

func TestMessageRegistryWithFetcherAndWellKnown(t *testing.T) {
	registry := (&msgregistry.MessageRegistry{}).
		WithMessageFactory(dynamic.NewMessageFactoryWithDefaults())

	reader := func(name string) (io.ReadCloser, error) {
		fmt.Println("READ:", name)
		src, ok := files[name]
		if !ok {
			fmt.Println("____ not found")
			return nil, ErrNotFound
		}
		return ioutil.NopCloser(bytes.NewReader([]byte(src))), nil
	}

	parser := &protoparse.Parser{
		Accessor: reader,
	}

	fetcher := func(url string, enum bool) (proto.Message, error) {
		fmt.Println("FETCH", url)

		fds, err := parser.ParseFiles(url)
		if err == ErrNotFound {
			return nil, nil
		}

		if err != nil {
			return nil, err
		}

		if len(fds) == 0 {
			return nil, errors.New("missing descs")
		}

		lp := fqnFromURL(url)
		fooMessage := fds[0].FindMessage(lp)
		spew.Dump(fooMessage)

		fooPType := registry.MessageAsPType(fooMessage)
		spew.Dump(fooPType)

		return fooPType, nil
	}

	registry = registry.WithFetcher(fetcher)

	md, err := registry.FindMessageTypeByUrl("https://test/core.wrapper.proto")
	assert.NoError(t, err)
	assert.NotNil(t, md)

	wr := dynamic.NewMessageFactoryWithDefaults().NewMessage(md).(*dynamic.Message)

	js := `{"extras":[{"@type":"test/core.one","abc":"fubar","def":123},{"@type":"test/core.two","abc":"snafu","def":456}]}`

	jsu := jsonpb.Unmarshaler{
		AllowUnknownFields: true,
		AnyResolver:        registry,
	}

	err = jsu.Unmarshal(strings.NewReader(js), wr)
	assert.NoError(t, err)

	spew.Dump(wr)
}

Result A

=== RUN   TestMessageRegistryWithFetcherAndWellKnown
FETCH: https://test/core.wrapper.proto
READ: https://test/core.wrapper.proto
READ: google/protobuf/any.proto
____ not found
([]*desc.FileDescriptor) (len=1 cap=1) {
 (*desc.FileDescriptor)(0xc0001f2540)(name:"https://test/core.wrapper.proto" package:"core" dependency:"google/protobuf/any.proto" message_type:<name:"wrapper" field:<name:"abc" number:1 label:LABEL_OPTIONAL type:TYPE_STRING json_name:"abc" > field:<name:"def" number:2 label:LABEL_OPTIONAL type:TYPE_INT32 json_name:"def" > field:<name:"extras" number:3 label:LABEL_REPEATED type:TYPE_MESSAGE type_name:".google.protobuf.Any" json_name:"extras" > > syntax:"proto3" )
}
____ looking for https://test/core.wrapper.proto core.wrapper
(*desc.MessageDescriptor)(0xc0001f7860)(name:"wrapper" field:<name:"abc" number:1 label:LABEL_OPTIONAL type:TYPE_STRING json_name:"abc" > field:<name:"def" number:2 label:LABEL_OPTIONAL type:TYPE_INT32 json_name:"def" > field:<name:"extras" number:3 label:LABEL_REPEATED type:TYPE_MESSAGE type_name:".google.protobuf.Any" json_name:"extras" > )
FETCH: https://type.googleapis.com/google.protobuf.Any
READ: https://type.googleapis.com/google.protobuf.Any
____ not found
--- FAIL: TestMessageRegistryWithFetcherAndWellKnown (0.00s)
    dynamic4_test.go:118:
        	Error Trace:	dynamic4_test.go:118
        	Error:      	Received unexpected error:
        	            	Failed to locate type for https://type.googleapis.com/google.protobuf.Any
        	Test:       	TestMessageRegistryWithFetcherAndWellKnown
    dynamic4_test.go:119:
        	Error Trace:	dynamic4_test.go:119
        	Error:      	Expected value not to be nil.
        	Test:       	TestMessageRegistryWithFetcherAndWellKnown
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xb8 pc=0x11ee3d8]

goroutine 20 [running]:
testing.tRunner.func1(0xc000152b00)
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:792 +0x387
panic(0x144f520, 0x1893a00)
	/usr/local/Cellar/go/1.11/libexec/src/runtime/panic.go:513 +0x1b9
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/desc.(*MessageDescriptor).GetFullyQualifiedName(...)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/desc/descriptor.go:538
nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic.(*MessageFactory).NewMessage(0xc0002128d0, 0x0, 0x14b8340, 0x0)
	/Users/geoah/src/nope.io/go/playground/dynamic/vendor/github.com/jhump/protoreflect/dynamic/message_factory.go:64 +0x38
command-line-arguments.TestMessageRegistryWithFetcherAndWellKnown(0xc000152b00)
	/Users/geoah/src/nope.io/go/playground/dynamic/dynamic4_test.go:121 +0x2fd
testing.tRunner(0xc000152b00, 0x14e5430)
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:827 +0xbf
created by testing.(*T).Run
	/usr/local/Cellar/go/1.11/libexec/src/testing/testing.go:878 +0x353
FAIL	command-line-arguments	0.019s

Testcase B

package dynamic

import (
	"bytes"
	"errors"
	"fmt"
	"io"
	"io/ioutil"
	"testing"

	"github.com/jhump/protoreflect/desc/protoparse"
	"github.com/stretchr/testify/assert"
)

var parserFiles = map[string]string{
	"https://test/core.wrapper.proto": `
		syntax = "proto3";
		package core;
		import "google/protobuf/any.proto";
		message wrapper {
			string abc = 1;
			int32 def = 2;
			repeated google.protobuf.Any extras = 3;
		}`,
}

func TestParserWithAccessor(t *testing.T) {
	reader := func(name string) (io.ReadCloser, error) {
		fmt.Println("READ:", name)
		src, ok := parserFiles[name]
		if !ok {
			fmt.Println("____ not found")
			return nil, errors.New("not found")
		}
		return ioutil.NopCloser(bytes.NewReader([]byte(src))), nil
	}

	parser := &protoparse.Parser{
		Accessor: reader,
	}

	fds, err := parser.ParseFiles("https://test/core.wrapper.proto")
	assert.NoError(t, err)
	assert.Len(t, fds, 1)
}

Result B

=== RUN   TestParserWithAccessor
READ: https://test/core.wrapper.proto
READ: google/protobuf/any.proto
____ not found
--- PASS: TestParserWithAccessor (0.00s)
PASS
ok  	command-line-arguments	0.014s

hi,
I am trying to use
https://godoc.org/github.com/jhump/protoreflect/desc
https://godoc.org/github.com/jhump/protoreflect/dynamic

to unmarshal buffer([] byte) input to my photo file.

desc2,_ := desc.LoadMessageDescriptorForMessage(telemetry_top.TelemetryStream)
dy := dynamic.NewMessage(desc2)
fmt.Println(dy.Unmarshal(buf))

but when I run this I get -

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x61b869]

goroutine 1 [running]:
github.com/jhump/protoreflect/desc.(*MessageDescriptor).FindFieldByNumber(...)
/b/amohit/go/src/github.com/jhump/protoreflect/desc/descriptor.go:673
github.com/jhump/protoreflect/dynamic.(*Message).FindFieldDescriptor(0xc420055e98, 0x200000001, 0x0)
/b/amohit/go/src/github.com/jhump/protoreflect/dynamic/dynamic_message.go:244 +0x29
github.com/jhump/protoreflect/dynamic.(*Message).unmarshal(0xc420055e98, 0xc420055de0, 0x410100, 0x6cbc60, 0x0)
/b/amohit/go/src/github.com/jhump/protoreflect/dynamic/binary.go:407 +0x7b
github.com/jhump/protoreflect/dynamic.(*Message).UnmarshalMerge(0xc420055e98, 0xc4200be000, 0x499, 0xffff, 0xc42007d350, 0xffff)
/b/amohit/go/src/github.com/jhump/protoreflect/dynamic/binary.go:390 +0x89
github.com/jhump/protoreflect/dynamic.(*Message).Unmarshal(0xc420055e98, 0xc4200be000, 0x499, 0xffff, 0x0, 0x499)
/b/amohit/go/src/github.com/jhump/protoreflect/dynamic/binary.go:379 +0x75
main.Parse(0xc4200be000, 0x499, 0xffff)
/b/amohit/go/src/github.com/influxdata/telegraf/plugins/parsers/juniperUDP/server.go:68 +0xad
main.main()
/b/amohit/go/src/github.com/influxdata/telegraf/plugins/parsers/juniperUDP/server.go:54 +0x19d
exit status 2

Can you please provide an example on how to Unmarshal buffer([] bytes) input for photo.

Howdy @jhump!

Message definition:

message point_list {
	repeated double point = 1;
}

Message instantiation:

msg := &pointline.PointList{
	Point: []float64{ 1,2,3 },
}

Stack trace (some parts omitted):

panic: runtime error: comparing uncomparable type []interface {} [recovered]
	panic: runtime error: comparing uncomparable type []interface {}

goroutine 8 [running]:
panic(0x531fa0, 0xc420284680)
	/bin/src/runtime/panic.go:500 +0x1a1
testing.tRunner.func1(0xc420094540)
	/bin/src/testing/testing.go:579 +0x25d
panic(0x531fa0, 0xc420284680)
	/bin/src/runtime/panic.go:458 +0x243
/vendor/github.com/jhump/protoreflect/dynamic.(*Message).internalSetField(0xc42015fe60, 0xc42027bf10, 0x4f8260, 0xc420241e60)
	/vendor/github.com/jhump/protoreflect/dynamic/dynamic_message.go:362 +0x3df
/vendor/github.com/jhump/protoreflect/dynamic.(*Message).unmarshalKnownField(0xc42015fe60, 0xc42027bf10, 0xc42027bf02, 0xc42015fd80, 0xc420249f80, 0x3)
	/vendor/github.com/jhump/protoreflect/dynamic/binary.go:583 +0x9a2
/vendor/github.com/jhump/protoreflect/dynamic.(*Message).unmarshal(0xc42015fe60, 0xc42015fd80, 0x8a300, 0xc420094600, 0x0)
	/vendor/github.com/jhump/protoreflect/dynamic/binary.go:381 +0x171
/vendor/github.com/jhump/protoreflect/dynamic.(*Message).UnmarshalMerge(0xc42015fe60, 0xc4201cd9e0, 0x1a, 0x20, 0xc420266750, 0x3100)
	/vendor/github.com/jhump/protoreflect/dynamic/binary.go:354 +0x92
/vendor/github.com/jhump/protoreflect/dynamic.(*Message).Unmarshal(0xc42015fe60, 0xc4201cd9e0, 0x1a, 0x20, 0xc4202667f8, 0x3100)
	/vendor/github.com/jhump/protoreflect/dynamic/binary.go:347 +0x65

I have a proto file that extends google's MethodOptions to annotate methods with additional validation logic.

During debugging, I found an issue in protoreflect that causes the MethodOptions of a latter method to overwrite the MethodOptions of the previous method. The problem could have wider impact than just this specific case.

An example proto file is like this:

service User {
    rpc UserAuth(Request) returns (Response) {
        option (validator) = {
            authenticated: true
            permission: {
                action: login
                entity: "client"
            }
        };
    }

    rpc Get(Request) returns (Response) {
        option (validator) = {
            authenticated: true
            permission: {
                action: read
                entity: "user"
            }
        };
    }
}

In the parse result, the option of the second (Get) method overwrites the option for the first (UserAuth) method.

I have a fix. I will submit a PR to resolve this issue.

When unmarshaling a dynamic message from JSON, if it has an Any field whose type URL refers to a proto that is not linked into the program, but is known via MessageRegistry, unmarshaling fails.

MessageRegistry should provide some facility to marshal and unmarshal Any messages to JSON. Dynamic messages that contain Any messages should also provide some mechanism to resolve type URLs when marshaling to and unmarshaling from JSON.

I tried to modify the code to make it work：

       // parser.go:147
       // Add this Code to Fix.
	if p.IncludeSourceCodeInfo {
		for _, pr := range protos {
			pr.fd.SourceCodeInfo = pr.generateSourceCodeInfo()
		}
	}
	linkedProtos, err := newLinker(protos).linkFiles()
	if err != nil {
		return nil, err
	}
	fds := make([]*desc.FileDescriptor, len(filenames))
	for i, name := range filenames {
		fd := linkedProtos[name]
		//if p.IncludeSourceCodeInfo {
		//	pr := protos[name]
		//	fd.AsFileDescriptorProto().SourceCodeInfo = pr.generateSourceCodeInfo()
		//}
		fds[i] = fd
	}

The builders should both preserve comments as much as possible (when creating a builder from an existing descriptor that has comment info) and provide a mechanism for setting/changing element comments. These comments would be inserted into resulting file descriptors' source_code_info field.

This, combined with protoprint package in #84, would provide a nice model for generating proto source code.

I have a couple more ideas for performance improvements and am wondering how you feel about all of the following:

1. How do you feel about replacing some of the internal maps that are keyed on field number with slices? My profiling shows a large chunk of time spent in map_access routines that could probably be heavily reduced by using fixed size slices (i.e if a messages maximum field number is 100 then we would use a slice with size 101. I.E m.values could become []interface{}
2. Adding "typed" APIs so that I can do something like m.GetInt64FieldByFieldNumber(fieldNum). My profiling shows that the library spends a lot of time allocating (and collecting) interface{}. We could alleviate this a lot by adding support for typed APIs and then using union types internally for storing the data.
3. Allowing allocation of []byte to be controlled. Right now I have no control on how []byte fields will be allocated. It would be nice if I could configure my own allocation function or somehow indicate that I would rather the library take a subslice of the buffer its marshaling from instead of allocating a new one.
4. Probably the biggest improvement for me would be if I could unmarshal buffers in an iterative fashion (basically if I could do what the codedBuffer and m.unmarshal method do internally where I could have an iterator that reads the stream and gives me the value of each field one at a time. That would probably make a huge performance improvement for my workload.

I realize that these changes may not align with your vision for the library so just wanted to get your thoughts.

The current pattern through out the code in this repo is to propagate error values directly, without much in the way of transforming/wrapping.

The exception to this is the protoparse package, which now provides useful context by attaching locations in source code to each error message. But the desc and dynamic packages can provide very hard-to-decipher errors -- like when something happens deep in the bowels during de-serialization.

For example, if an error occurs that is related to data types/integrity, the message often lacks necessary context. Even if it indicates a field name, that is often insufficient since it could be working in a deeply nested message, and the field name alone provides little information for a user to act on.

I'd like to take a stab at re-doing the error propagation behavior throughout most of this repo to instead use the github.com/pkg/errors package for wrapping/chaining errors and providing additional context. That way, errors returned to user code have more context to assist with understanding and resolving those errors.

Hi!
Thank you very much for this excellent library!

I currently face a problem with the text (un)marshaler.
For example with this test.proto:

syntax = "proto3";

message Test {
  Inner foo = 1;
}

message Inner {
  string bar = 1;
}

When I decode 0x0A050A0362617A with protoc I get this:

$ protoc --decode Test test.proto < test.bin
foo {
  bar: "baz"
}

While I get this when marshaling the same message with MarshalText:

foo:<bar:"baz">

The fact that syntaxes differ is not a real problem since protoc can understand both:

$ diff <(protoc --encode Test test.proto <<< 'foo{bar:"baz"}') <(protoc --encode Test test.proto <<< 'foo:<bar:"baz">')

However, it appears that UnmarshalText is not able to handle the protoc format. I get this error:

Line 1, col 5: Expecting a colon ':'; instead got "{"

Please correct me if this is somehow the expected behavior, but based on my understanding of the Proto spec a repeatable field that is not set and one which is set to an empty array should be equal to each other because the default value is an empty array for any repeatable field.

I caught this issue with one of my property tests, but it seems that if you have two messages with a repeatable field and one you explicitly set the value to an empty array and the other you leave it untouched, dynamic.Equal() doesn't view the two fields as equal.

I've included a simple reproduction here: #180

Also I believe the issue has something to do with this line, but I could be wrong: https://github.com/jhump/protoreflect/blob/master/dynamic/dynamic_message.go#L719

If I replace IsProto3() with true the code behaves as expected

The code snippet below in parser.go has a bug. It doesn't try to import the file when it exists only on the 3rd import path or later.

accessor = func(name string) (io.ReadCloser, error) {
        var ret error
        for _, path := range paths {
                f, err := acc(filepath.Join(path, name))
                if err != nil && ret == nil {
                        ret = err
                        continue
                }
                return f, nil
        }
        return nil, ret
}

Proto files let you refer to messages defined in a parent package without having to qualify the message.

For example, if you have a foo.proto file like:

syntax = "proto3";
package foo;

message ParentMessage {
  string foo = 1;
};

You can refer to ParentMessage without using a package qualifier in a pkg/bar.proto file like:

syntax = "proto3";
package foo.pkg;

message ChildMessage {
  // No qualification needed here!
  ParentMessage msg = 1;
};

The proto parser will fail with a message like unknown type ParentMessage, while protoc will happily resolve this symbol.

Hello!

First off, I'd like to thank you for releasing such a well engineered library. I've been using it for the last few weeks to develop a P.O.C of a new feature for our open source time series database M3DB and I can tell that a lot of careful thought and design went into this library.

I was wondering if you would be open to me adding a method called MarshalInto() to the library that basically behaves exactly like Marshal, but instead of returning a byte slice it writes into a provided one (similar to how Write() methods usually work). I'm also open to some other interface, but the gist is that I need to do this Marshaling on the hot path of some of my code and I need to be able to pool the byte slices / control allocations.

Let me know what you think!

Cheers,
Richie

Hey,
First of all, you do a great job with this library, thanks!

I have one question about working example. Let's say I have only .proto file, can I send a message to a gRPC service using your library? Without generating code go .pb.go files?

I just can't understand stuff related to FileDescriptor.
Can you provide some example?

I have a fully-valid FileDescriptorSet created from protoc with --include_imports, that contains google/protobuf/duration.proto in the files list, and the message I try to operate on contains a google.protobuf.Duration field named duration. When I create a DynamicMessage with dynamic.NewMessage (or using the default message factory with the WKT resolver and NewDynamicMessage) from a MessageDescriptor from FindMessage from something returned from CreateFileDescriptorFromSet where the message is contained in the last file. When I try to unmarshal '{"duration":"10.000s"}', I get the error Bad input. Expecting start of JSON object: '{'. Instead got: 10.000s.. I believe this is related to the fact that dynamic/message_factory.go relies on reflection to see if there is a registered WKT message in the github.com/golang/protobuf globals, when this library shouldn't rely on the global maps within github.com/golang/protobuf at all (ie calls to proto.MessageType) - it makes this unusable for github.com/gogo/protobuf users, and belies the purpose of being able to do everything dynamically with FileDescriptorSets in the first place :-)

I know this is a really weak explanation, I'll try to create a test that reproduces this, but wanted to kick off the conversation.

Great library btw!

Sorry to file another one, but there's still an issue. I see that I can unmarshal Well-Known Types without an error now per #99, but the resulting *dynamic.Message does not seem to be properly formed. See https://github.com/jhump/protoreflect/compare/master...peter-edge:marshal-json-test?expand=1

$ git diff master
diff --git a/dynamic/json_test.go b/dynamic/json_test.go
index 6bca0c9..fdae69f 100644
--- a/dynamic/json_test.go
+++ b/dynamic/json_test.go
@@ -430,6 +430,10 @@ func TestJSONWellKnownTypeFromFileDescriptorSet(t *testing.T) {
        dm := NewMessage(md)
        err = dm.UnmarshalJSON([]byte(js))
        testutil.Ok(t, err)
+
+       jsonData, err := dm.MarshalJSON()
+       testutil.Ok(t, err)
+       testutil.Eq(t, `"30303.000040404s"`, string(jsonData))
 }

Result:

--- FAIL: TestJSONWellKnownTypeFromFileDescriptorSet (0.00s)
	asserts.go:55: dynamic.TestJSONWellKnownTypeFromFileDescriptorSet(dynamic/json_test.go:436): Expecting "30303.000040404s" (string), got "0.000s" (string)

What I did:

Called (*FileDescriptor).FindSymbol on a fully-qualified type name, .twitch.twirp.example.Size.

This fully-qualified type name is the one that a *ServiceDescriptorProto from protoc listed as a type name. Here's the text-encoded proto for an example service:

  service: <
    name: "Haberdasher"
    method: <
      name: "MakeHat"
      input_type: ".twitch.twirp.example.Size"
      output_type: ".twitch.twirp.example.Hat"
    >
  >

I called this on the same *FileDescriptor that contained this service definition.

What I expected:

I expected FindSymbol to return a descriptor for the .twitch.twirp.example.Size type.

What I saw instead:

I got nil.

I added logging to print out the (*FileDescriptor).symbols map, and saw that it had twitch.twirp.example.Size, but not .twitch.twirp.example.Size - note the leading dot.

Leading dots are the way that a path specification is marked as fully-qualified, as documented in the parser: https://github.com/protocolbuffers/protobuf/blob/ba8692fbade4ba329cc4531e286ab5a8e0821d97/src/google/protobuf/compiler/parser.cc#L2157

This is also documented in FieldDecriptorProto.

I wrote command line tool using protoreflect parser.
protoreflect is a good library!! thanks!!

Next I will write a same functionality protoc plugin.
protoc plugin have descriptor.FileDescriptorProto in CodeGeneratorRequest, but it dose not link...

I want to use following method because Link make to write code very easy and use same code command line tool and protoc plugin.

func (p Parser) Link([]*dpb.FileDescriptorProto) ([]*desc.FileDescriptor, error)  {

}

First of all, thanks for the package, it's been really useful for tool development!

I am using the desc/builder and desc/protoprint pkgs to generate a new proto file containing a service. I set a custom option for the service using similar code I see in the tests. But the printed proto file doesn't contain the custom service option. After looking through all of the tests, I'm guessing I'm doing something wrong, because it looks like there is protoprint test output containing service options.

I did a lot of print debugging and saw the custom option being passed around at various stages. I was able to get the option included in the printed proto file by making a change to the protoprint code here, adding dm.GetKnownExtensions() to the list of FieldDescriptors in the range. So that becomes:

for _, fldset := range [][]*desc.FieldDescriptor{md.GetFields(), mf.GetExtensionRegistry().AllExtensionsForType(md.GetFullyQualifiedName()), dm.GetKnownExtensions()} {

Any thoughts? Thanks in advance!

My code for adding a ServiceOption is:

opts := &descriptor.ServiceOptions{}
if err := proto.SetExtension(opts, annotations.E_DefaultHost, proto.String(host)); err != nil {
    return nil, nil, err
}

// srv is a ServiceBuilder here
srv.SetOptions(opts)

If you create a message with some fields dynamically added, the .GetKnownFields() method does not return them in the list - they are also not accessible by the .FindFieldDescriptorByName method.

If I store a map of the fieldDescriptors I created, I can use that to access them and set values on the message, so there is a workaround that way.

Hi!

Thanks for protoreflect, it's a super nice and helpful project.

I've just run into a problem using protoparse and a custom annotation we've got. A reproduction case is this:

package main

import (
	"fmt"
	"os"

	"github.com/jhump/protoreflect/desc/protoparse"
)

func main() {
	// this parses the .proto files from disk
	parser := protoparse.Parser{
		ImportPaths: []string{
			".",
			"/Users/stephan/Go/src/github.com/lyft/protoc-gen-validate", # needed, this contains validate/validate.proto, which is imported
		},
	}
	fds, err := parser.ParseFiles(os.Args[1:]...)
	if err != nil {
		fmt.Fprintf(os.Stderr, "parse proto files %v: %s", os.Args, err)
	}
	fmt.Println(fds)
}

and the proto file

syntax = "proto3";
package tests.kitchensink;

import "validate/validate.proto";

message String {
    string pattern  = 10 [(validate.rules) = {string: {pattern: "foo+(ba+r)?"}}];
}

running go run test.go string.proto gives me

string.proto:7:47: syntax error: unexpected _STRING, expecting _NAME or '}' or ',' or '['[]

(using that proto file with protoc-gen-validate works, though, so I suspect something missing in protoparse's grammer?)

Any hints? Happy to contribute, but I'd need some pointer on where to start....

Thanks!