From b153e554988a47030ff12e48102efba662fb5ac7 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Thu, 8 Sep 2022 13:42:51 -0400 Subject: [PATCH 1/5] feat: skip global registration for internal generated code --- features/protoc/main.go | 28 +++++++++++++++---- go.mod | 1 + go.sum | 2 ++ internal/testprotos/test3/test.pulsar.go | 2 ++ .../testprotos/test3/test_import.pulsar.go | 2 ++ .../testprotos/test3/test_nesting.pulsar.go | 2 ++ runtime/nullregistry.go | 23 +++++++++++++++ 7 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 runtime/nullregistry.go diff --git a/features/protoc/main.go b/features/protoc/main.go index 3cc5a1a..4c011d1 100644 --- a/features/protoc/main.go +++ b/features/protoc/main.go @@ -7,26 +7,31 @@ package protoc import ( "fmt" - "github.com/cosmos/cosmos-proto/features/protoc/genid" - "github.com/cosmos/cosmos-proto/generator" "go/ast" "go/parser" "go/token" - "google.golang.org/protobuf/encoding/protowire" - "google.golang.org/protobuf/proto" "math" "strconv" "strings" "unicode" "unicode/utf8" + "golang.org/x/exp/slices" + + "google.golang.org/protobuf/encoding/protowire" + "google.golang.org/protobuf/proto" + + "github.com/cosmos/cosmos-proto/features/protoc/genid" + "github.com/cosmos/cosmos-proto/generator" + pref "google.golang.org/protobuf/reflect/protoreflect" - "github.com/cosmos/cosmos-proto/features/protoc/version" "google.golang.org/protobuf/compiler/protogen" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/runtime/protoimpl" + "github.com/cosmos/cosmos-proto/features/protoc/version" + "google.golang.org/protobuf/types/descriptorpb" "google.golang.org/protobuf/types/pluginpb" ) @@ -60,6 +65,7 @@ var ( protojsonPackage goImportPath = protogen.GoImportPath("google.golang.org/protobuf/encoding/protojson") protoreflectPackage goImportPath = protogen.GoImportPath("google.golang.org/protobuf/reflect/protoreflect") protoregistryPackage goImportPath = protogen.GoImportPath("google.golang.org/protobuf/reflect/protoregistry") + runtimePackage = protogen.GoImportPath("github.com/cosmos/cosmos-proto/runtime") ) // GenerateFile generates the contents of a .pb.go file. @@ -360,6 +366,12 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *generator.GeneratedFile, } } + isInternal := false + pathParts := strings.Split(f.GoImportPath.String(), "/") + if slices.Contains(pathParts, "internal") { + isInternal = true + } + g.P("type x struct{}") g.P("out := ", protoimplPackage.Ident("TypeBuilder"), "{") g.P("File: ", protoimplPackage.Ident("DescBuilder"), "{") @@ -369,6 +381,9 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *generator.GeneratedFile, g.P("NumMessages: ", len(f.allMessages), ",") g.P("NumExtensions: ", len(f.allExtensions), ",") g.P("NumServices: ", len(f.Services), ",") + if isInternal { + g.P("FileRegistry: ", runtimePackage.Ident("NullRegistry"), "{},") + } g.P("},") g.P("GoTypes: ", goTypesVarName(f), ",") g.P("DependencyIndexes: ", depIdxsVarName(f), ",") @@ -381,6 +396,9 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *generator.GeneratedFile, if len(f.allExtensions) > 0 { g.P("ExtensionInfos: ", extensionTypesVarName(f), ",") } + if isInternal { + g.P("TypeRegistry: ", runtimePackage.Ident("NullRegistry"), "{},") + } g.P("}.Build()") g.P(f.GoDescriptorIdent, " = out.File") diff --git a/go.mod b/go.mod index 1a71a6a..c052266 100644 --- a/go.mod +++ b/go.mod @@ -12,5 +12,6 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 983e745..8358486 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,8 @@ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSS github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0 h1:17k44ji3KFYG94XS5QEFC8pyuOlMh3IoR+vkmTZmJJs= +golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.28.1 h1:d0NfwRgPtno5B1Wa6L2DAG+KivqkdutMf1UhdNx175w= diff --git a/internal/testprotos/test3/test.pulsar.go b/internal/testprotos/test3/test.pulsar.go index 1721a4a..2b0ee3e 100644 --- a/internal/testprotos/test3/test.pulsar.go +++ b/internal/testprotos/test3/test.pulsar.go @@ -12605,11 +12605,13 @@ func file_internal_testprotos_test3_test_proto_init() { NumMessages: 20, NumExtensions: 0, NumServices: 0, + FileRegistry: runtime.NullRegistry{}, }, GoTypes: file_internal_testprotos_test3_test_proto_goTypes, DependencyIndexes: file_internal_testprotos_test3_test_proto_depIdxs, EnumInfos: file_internal_testprotos_test3_test_proto_enumTypes, MessageInfos: file_internal_testprotos_test3_test_proto_msgTypes, + TypeRegistry: runtime.NullRegistry{}, }.Build() File_internal_testprotos_test3_test_proto = out.File file_internal_testprotos_test3_test_proto_rawDesc = nil diff --git a/internal/testprotos/test3/test_import.pulsar.go b/internal/testprotos/test3/test_import.pulsar.go index 06ba752..f9d5dfe 100644 --- a/internal/testprotos/test3/test_import.pulsar.go +++ b/internal/testprotos/test3/test_import.pulsar.go @@ -525,11 +525,13 @@ func file_internal_testprotos_test3_test_import_proto_init() { NumMessages: 1, NumExtensions: 0, NumServices: 0, + FileRegistry: runtime.NullRegistry{}, }, GoTypes: file_internal_testprotos_test3_test_import_proto_goTypes, DependencyIndexes: file_internal_testprotos_test3_test_import_proto_depIdxs, EnumInfos: file_internal_testprotos_test3_test_import_proto_enumTypes, MessageInfos: file_internal_testprotos_test3_test_import_proto_msgTypes, + TypeRegistry: runtime.NullRegistry{}, }.Build() File_internal_testprotos_test3_test_import_proto = out.File file_internal_testprotos_test3_test_import_proto_rawDesc = nil diff --git a/internal/testprotos/test3/test_nesting.pulsar.go b/internal/testprotos/test3/test_nesting.pulsar.go index 4547c29..52da9c7 100644 --- a/internal/testprotos/test3/test_nesting.pulsar.go +++ b/internal/testprotos/test3/test_nesting.pulsar.go @@ -2061,10 +2061,12 @@ func file_internal_testprotos_test3_test_nesting_proto_init() { NumMessages: 4, NumExtensions: 0, NumServices: 0, + FileRegistry: runtime.NullRegistry{}, }, GoTypes: file_internal_testprotos_test3_test_nesting_proto_goTypes, DependencyIndexes: file_internal_testprotos_test3_test_nesting_proto_depIdxs, MessageInfos: file_internal_testprotos_test3_test_nesting_proto_msgTypes, + TypeRegistry: runtime.NullRegistry{}, }.Build() File_internal_testprotos_test3_test_nesting_proto = out.File file_internal_testprotos_test3_test_nesting_proto_rawDesc = nil diff --git a/runtime/nullregistry.go b/runtime/nullregistry.go new file mode 100644 index 0000000..2933122 --- /dev/null +++ b/runtime/nullregistry.go @@ -0,0 +1,23 @@ +package runtime + +import ( + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" +) + +// NullRegistry is an implementation of the interfaces used to register generated +// types and file descriptors that does nothing. This is used to generated +// protobuf files in internal packages that are not registered with the global registry. +type NullRegistry struct{} + +func (NullRegistry) RegisterMessage(protoreflect.MessageType) error { return nil } +func (NullRegistry) RegisterEnum(protoreflect.EnumType) error { return nil } +func (NullRegistry) RegisterExtension(protoreflect.ExtensionType) error { return nil } +func (NullRegistry) RegisterFile(protoreflect.FileDescriptor) error { return nil } + +func (NullRegistry) FindFileByPath(path string) (protoreflect.FileDescriptor, error) { + return protoregistry.GlobalFiles.FindFileByPath(path) +} +func (NullRegistry) FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error) { + return protoregistry.GlobalFiles.FindDescriptorByName(name) +} From cf1389f0ce9786404b7af6f241a444b45416a318 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 21 May 2024 17:12:15 -0400 Subject: [PATCH 2/5] fixes --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 248c29c..d8fd04d 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.18 require ( github.com/google/go-cmp v0.6.0 github.com/stretchr/testify v1.9.0 + golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0 google.golang.org/protobuf v1.34.0 gotest.tools/v3 v3.5.1 pgregory.net/rapid v1.1.0 @@ -13,6 +14,5 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 5759b63..1eef86b 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0 h1:17k44ji3KFYG94XS5QEFC8pyuOlMh3IoR+vkmTZmJJs= +golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= google.golang.org/protobuf v1.34.0 h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4= google.golang.org/protobuf v1.34.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= From c7be699034999a922a370d6ee98dc65a00fd1373 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 21 May 2024 18:09:28 -0400 Subject: [PATCH 3/5] simplify, add test --- features/protoc/internal_test.go | 17 +++++++++++++++++ features/protoc/main.go | 12 +++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 features/protoc/internal_test.go diff --git a/features/protoc/internal_test.go b/features/protoc/internal_test.go new file mode 100644 index 0000000..025afa8 --- /dev/null +++ b/features/protoc/internal_test.go @@ -0,0 +1,17 @@ +package protoc + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestInternalPackage(t *testing.T) { + require.True(t, isInternalPackage("internal")) + require.True(t, isInternalPackage("example/internal")) + require.True(t, isInternalPackage("example.com/internal")) + require.True(t, isInternalPackage("example.com/internal/foo")) + require.False(t, isInternalPackage("example")) + require.False(t, isInternalPackage("example.com")) + require.False(t, isInternalPackage("example.com/false")) +} diff --git a/features/protoc/main.go b/features/protoc/main.go index 4c011d1..95abdc6 100644 --- a/features/protoc/main.go +++ b/features/protoc/main.go @@ -366,11 +366,7 @@ func genReflectFileDescriptor(gen *protogen.Plugin, g *generator.GeneratedFile, } } - isInternal := false - pathParts := strings.Split(f.GoImportPath.String(), "/") - if slices.Contains(pathParts, "internal") { - isInternal = true - } + isInternal := isInternalPackage(f.GoImportPath.String()) g.P("type x struct{}") g.P("out := ", protoimplPackage.Ident("TypeBuilder"), "{") @@ -2305,3 +2301,9 @@ func newFileInfo(file *protogen.File) *fileInfo { return f } + +// checks whether the package path is an internal package +func isInternalPackage(pkg string) bool { + pathParts := strings.Split(pkg, "/") + return slices.Contains(pathParts, "internal") +} From 66a110f1efb2c518bd3b23d445bb3ca4a9840577 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 21 May 2024 18:33:13 -0400 Subject: [PATCH 4/5] add tests --- features/protoc/internal_test.go | 2 +- internal/testprotos/test3/internal_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 internal/testprotos/test3/internal_test.go diff --git a/features/protoc/internal_test.go b/features/protoc/internal_test.go index 025afa8..2657f4d 100644 --- a/features/protoc/internal_test.go +++ b/features/protoc/internal_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestInternalPackage(t *testing.T) { +func TestIsInternalPackage(t *testing.T) { require.True(t, isInternalPackage("internal")) require.True(t, isInternalPackage("example/internal")) require.True(t, isInternalPackage("example.com/internal")) diff --git a/internal/testprotos/test3/internal_test.go b/internal/testprotos/test3/internal_test.go new file mode 100644 index 0000000..aeb6635 --- /dev/null +++ b/internal/testprotos/test3/internal_test.go @@ -0,0 +1,16 @@ +package test3 + +import ( + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/reflect/protoregistry" +) + +func TestInternalNotRegistered(t *testing.T) { + _, err := protoregistry.GlobalTypes.FindMessageByName((&TestAllTypes{}).ProtoReflect().Descriptor().FullName()) + require.Error(t, err) + + _, err = protoregistry.GlobalFiles.FindFileByPath("internal/testprotos/test3/test_import.proto") + require.Error(t, err) +} From 484e22f5456b3d2e967ae771f9f80c6a8e56ec62 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 25 Jun 2024 10:38:39 -0400 Subject: [PATCH 5/5] add CHANGELOG.md --- CHANGELOG.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..7d0d53f --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,41 @@ + + +# Changelog + +## [Unreleased] + +### Features + +* [#82](https://github.com/cosmos/cosmos-proto/pull/82) generated code in `internal` go packages is not registered with the global protobuf registry allowing modules to have internal protobuf generated types that are not exposed in public APIs and do not affect global protobuf registration. \ No newline at end of file