Skip to content

Commit

Permalink
Merge pull request #57 from mterwill/proper-namespacing
Browse files Browse the repository at this point in the history
fix: namespacing bug when importing other proto files
  • Loading branch information
sagikazarmark committed May 12, 2021
2 parents 309d13e + aad2309 commit a9dfbcf
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Fixes

- Fix segfault when input proto file does not specify options
- Fix namespacing bug when importing other proto files


## [0.7.1] - 2021-04-21
Expand Down
33 changes: 25 additions & 8 deletions protoc-gen-twirp_php/internal/php/func.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"strings"

"google.golang.org/protobuf/compiler/protogen"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/descriptorpb"
)

// Reserved PHP keywords that must be prefixed with something.
Expand All @@ -29,9 +31,15 @@ var reservedNames = []string{
//
// Created based on https://github.com/google/protobuf/blob/67952fab2c766ac5eacc15bb78e5af4039a3d398/src/google/protobuf/compiler/php/php_generator.cc#L137
func ClassNamePrefix(className string, file *protogen.File) string {
prefix := file.Proto.GetOptions().GetPhpClassPrefix()
if prefix != "" {
return prefix
return classNamePrefix(className, file.Desc)
}

func classNamePrefix(className string, file protoreflect.FileDescriptor) string {
if options, ok := file.Options().(*descriptorpb.FileOptions); ok {
prefix := options.GetPhpClassPrefix()
if prefix != "" {
return prefix
}
}

// Check if the class name matches a reserved name
Expand All @@ -46,7 +54,7 @@ func ClassNamePrefix(className string, file *protogen.File) string {

if isReserved {
// Google internal classes receive a different prefix
if file.Proto.GetPackage() == "google.protobuf" {
if file.Package() == "google.protobuf" {
return "GPB"
}

Expand All @@ -61,14 +69,18 @@ func ClassNamePrefix(className string, file *protogen.File) string {
// 1. Explicitly set namespace using the "php_namespace" option
// 2. Package name with dots replaced with backslashes and segments converted to title
func Namespace(file *protogen.File) string {
if options := file.Proto.GetOptions(); options != nil {
return namespace(file.Desc)
}

func namespace(file protoreflect.FileDescriptor) string {
if options, ok := file.Options().(*descriptorpb.FileOptions); ok {
// When there is a namespace option defined we use it
if options.PhpNamespace != nil {
return options.GetPhpNamespace()
}
}

return Name(file.Proto.GetPackage())
return Name(string(file.Package()))
}

// Path guesses the path of the file based on the (internally calculated) namespace.
Expand Down Expand Up @@ -96,7 +108,11 @@ func Name(s string) string {
//
// Created based on https://github.com/google/protobuf/blob/67952fab2c766ac5eacc15bb78e5af4039a3d398/src/google/protobuf/compiler/php/php_generator.cc#L195
func NamespacedName(className string, file *protogen.File) string {
ns := Namespace(file)
return namespacedName(className, file.Desc)
}

func namespacedName(className string, file protoreflect.FileDescriptor) string {
ns := namespace(file)

if ns == "" {
return className
Expand All @@ -115,6 +131,7 @@ func ServiceName(file *protogen.File, svc *protogen.Service) string {
// MessageName transforms a message name into a PHP compatible one.
func MessageName(file *protogen.File, message *protogen.Message) string {
className := string(message.Desc.Name())
parentFile := message.Desc.ParentFile()

return "\\" + NamespacedName(ClassNamePrefix(className, file)+className, file)
return "\\" + namespacedName(classNamePrefix(className, parentFile)+className, parentFile)
}

0 comments on commit a9dfbcf

Please sign in to comment.