golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.64k stars 1.58k forks source link

anypb.Must #1522

Closed fsaintjacques closed 1 year ago

fsaintjacques commented 1 year ago

Is your feature request related to a problem? Please describe. This is a solution to make tests less verbose. I'd like to have the anypb.Must(src proto.Message) *Any function that follows the usual New(...) (T, err) / Must(...) T duality pattern for constructing object with panic if an error is encountered. While I can easily add this method myself (in my package), it would be nice to have it support directly by anypb. That would simplify tests like

assert.Equal(anypb.Must(&MyMessage{...}), actualMsg, protocmp...)

Describe the solution you'd like

diff --git a/cmd/protoc-gen-go/internal_gengo/well_known_types.go b/cmd/protoc-gen-go/internal_gengo/well_known_types.go
index 47c4fa1..2e3205f 100644
--- a/cmd/protoc-gen-go/internal_gengo/well_known_types.go
+++ b/cmd/protoc-gen-go/internal_gengo/well_known_types.go
@@ -345,6 +345,16 @@ func genMessageKnownFunctions(g *protogen.GeneratedFile, f *fileInfo, m *message
                g.P("}")
                g.P()

+               g.P("// Must marshals src into a new Any instance but panics on error.")
+               g.P("func Must(src ", protoPackage.Ident("Message"), ") (*Any) {")
+               g.P("   dst, err := New(src)")
+               g.P("   if err != nil {")
+               g.P("           panic(err)")
+               g.P("   }")
+               g.P("   return dst")
+               g.P("}")
+               g.P()
dsnet commented 1 year ago

I don't think there should be any more Must equivalent since you can now write your own generic wrapper that panics if the error is non-nil. For an example of one such function see must.Get.

Usage of it would look like:

must.Get(anypb.New(...))
fsaintjacques commented 1 year ago

Thank you, I didn't though of using generics, my brain is still not used to it.

puellanivis commented 1 year ago

Thank you, I didn't though of using generics, my brain is still not used to it.

I’d say this is still better than some of the generics code I’ve seen so far… :trollface: It seems like any new feature reaches a significant amount of adoption, and—dare I say—over-adopted. Finding the right level of use for any feature kind of requires a bunch of swings back and forth under-adoption and over-adoption until you eventually find a happy medium.

This is definitely a use case where generics is the right choice.