Merge tag 'v0.4.10' into feature/gogo

lyft · Jun 14, 2019 · c0cc61b · c0cc61b
2 parents 934ae55 + bb8c3f8
commit c0cc61b
Show file tree

Hide file tree

Showing 15 changed files with 39 additions and 48 deletions.
diff --git a/enum_test.go b/enum_test.go
@@ -19,11 +19,8 @@ func TestEnum_Name(t *testing.T) {
 func TestEnum_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	e := &enum{desc: &descriptor.EnumDescriptorProto{Name: proto.String("enum")}}
-	f := dummyFile()
-	f.addEnum(e)
-
-	assert.Equal(t, f.FullyQualifiedName()+".enum", e.FullyQualifiedName())
+	e := &enum{fqn: "enum"}
+	assert.Equal(t, e.fqn, e.FullyQualifiedName())
 }
 
 func TestEnum_Syntax(t *testing.T) {

diff --git a/enum_value_test.go b/enum_value_test.go
@@ -18,11 +18,8 @@ func TestEnumVal_Name(t *testing.T) {
 func TestEnumVal_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	ev := &enumVal{desc: &descriptor.EnumValueDescriptorProto{Name: proto.String("ev")}}
-	e := dummyEnum()
-	e.addValue(ev)
-
-	assert.Equal(t, e.FullyQualifiedName()+".ev", ev.FullyQualifiedName())
+	ev := &enumVal{fqn: "ev"}
+	assert.Equal(t, ev.fqn, ev.FullyQualifiedName())
 }
 
 func TestEnumVal_Syntax(t *testing.T) {

diff --git a/extension_test.go b/extension_test.go
@@ -5,19 +5,15 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/golang/protobuf/protoc-gen-go/descriptor"
-
 	"github.com/golang/protobuf/proto"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestExt_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	msg := dummyMsg()
-	e := &ext{parent: msg}
-	e.desc = &descriptor.FieldDescriptorProto{Name: proto.String("foo")}
-	assert.Equal(t, msg.FullyQualifiedName()+".foo", e.FullyQualifiedName())
+	e := &ext{fqn: "foo"}
+	assert.Equal(t, e.fqn, e.FullyQualifiedName())
 }
 
 func TestExt_Syntax(t *testing.T) {

diff --git a/field_test.go b/field_test.go
@@ -20,11 +20,8 @@ func TestField_Name(t *testing.T) {
 func TestField_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	f := &field{desc: &descriptor.FieldDescriptorProto{Name: proto.String("field")}}
-	m := dummyMsg()
-	m.addField(f)
-
-	assert.Equal(t, m.FullyQualifiedName()+".field", f.FullyQualifiedName())
+	f := &field{fqn: "field"}
+	assert.Equal(t, f.fqn, f.FullyQualifiedName())
 }
 
 func TestField_Syntax(t *testing.T) {

diff --git a/file_test.go b/file_test.go
@@ -23,11 +23,8 @@ func TestFile_Name(t *testing.T) {
 func TestFile_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	f := &file{desc: &descriptor.FileDescriptorProto{
-		Package: proto.String("foo"),
-	}}
-
-	assert.Equal(t, ".foo", f.FullyQualifiedName())
+	f := &file{fqn: "foo"}
+	assert.Equal(t, f.fqn, f.FullyQualifiedName())
 }
 
 func TestFile_Syntax(t *testing.T) {

diff --git a/lang/go/package.go b/lang/go/package.go
@@ -5,6 +5,8 @@ import (
 	"go/token"
 	"regexp"
 	"strings"
+	"unicode"
+	"unicode/utf8"
 
 	pgs "github.com/lyft/protoc-gen-star"
 	"github.com/lyft/protoc-gen-star/gogoproto"
@@ -95,6 +97,11 @@ func (c context) PackageName(node pgs.Node) pgs.Name {
 		pkg = "_" + pkg
 	}
 
+	// if package starts with digit, prefix with `_`
+	if r, _ := utf8.DecodeRuneInString(pkg); unicode.IsDigit(r) {
+		pkg = "_" + pkg
+	}
+
 	// package name is kosher
 	return pgs.Name(pkg)
 }
@@ -205,14 +212,14 @@ func (c context) optionPackage(e pgs.Entity) (path, pkg string) {
 	// go_package="example.com/foo/bar;baz" should have a package name of `baz`
 	if idx := strings.LastIndex(pkg, ";"); idx > -1 {
 		path = pkg[:idx]
-		pkg = pkg[idx+1:]
+		pkg = nonAlphaNumPattern.ReplaceAllString(pkg[idx+1:], "_")
 		return
 	}
 
 	// go_package="example.com/foo/bar" should have a package name of `bar`
 	if idx := strings.LastIndex(pkg, "/"); idx > -1 {
 		path = pkg
-		pkg = pkg[idx+1:]
+		pkg = nonAlphaNumPattern.ReplaceAllString(pkg[idx+1:], "_")
 		return
 	}
 

diff --git a/lang/go/package_test.go b/lang/go/package_test.go
@@ -25,6 +25,8 @@ func TestPackageName(t *testing.T) {
 		{"mapped", "unaffected"},             // M mapped params are ignored for build targets
 		{"import_path_mapped", "go_package"}, // mixed import_path and M parameters should lose to go_package
 		{"transitive_package", "foobar"},     // go_option gets picked up from other files if present
+		{"digit", "_2019fizz"},               // digit at the start are prefixed with _
+		{"path_dash", "path_dash"},           // if basename of go_package contains invalid characters, replace with _
 	}
 
 	for _, test := range tests {

diff --git a/lang/go/testdata/names/digit/digit.proto b/lang/go/testdata/names/digit/digit.proto
@@ -0,0 +1,5 @@
+syntax="proto3";
+
+package names.digit;
+option go_package = "2019fizz";
+
diff --git a/lang/go/testdata/names/digit/params b/lang/go/testdata/names/digit/params
diff --git a/lang/go/testdata/names/path_dash/params b/lang/go/testdata/names/path_dash/params
diff --git a/lang/go/testdata/names/path_dash/path_dash.proto b/lang/go/testdata/names/path_dash/path_dash.proto
@@ -0,0 +1,5 @@
+syntax="proto3";
+package names.path_dash;
+option go_package="example.com/path-dash";
+
+message PathDash {}
diff --git a/message_test.go b/message_test.go
@@ -22,11 +22,8 @@ func TestMsg_Name(t *testing.T) {
 func TestMsg_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	m := &msg{desc: &descriptor.DescriptorProto{Name: proto.String("msg")}}
-	f := dummyFile()
-	f.addMessage(m)
-
-	assert.Equal(t, f.FullyQualifiedName()+".msg", m.FullyQualifiedName())
+	m := &msg{fqn: "msg"}
+	assert.Equal(t, m.fqn, m.FullyQualifiedName())
 }
 
 func TestMsg_Syntax(t *testing.T) {

diff --git a/method_test.go b/method_test.go
@@ -19,11 +19,8 @@ func TestMethod_Name(t *testing.T) {
 func TestMethod_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	s := dummyService()
-	m := &method{desc: &descriptor.MethodDescriptorProto{Name: proto.String("fizz")}}
-	s.addMethod(m)
-
-	assert.Equal(t, s.FullyQualifiedName()+".fizz", m.FullyQualifiedName())
+	m := &method{fqn: "fizz"}
+	assert.Equal(t, m.fqn, m.FullyQualifiedName())
 }
 
 func TestMethod_Syntax(t *testing.T) {

diff --git a/oneof_test.go b/oneof_test.go
@@ -20,11 +20,8 @@ func TestOneof_Name(t *testing.T) {
 func TestOneOf_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	m := dummyMsg()
-	o := &oneof{desc: &descriptor.OneofDescriptorProto{Name: proto.String("one_of")}}
-	m.addOneOf(o)
-
-	assert.Equal(t, m.FullyQualifiedName()+".one_of", o.FullyQualifiedName())
+	o := &oneof{fqn: "one_of"}
+	assert.Equal(t, o.fqn, o.FullyQualifiedName())
 }
 
 func TestOneof_Syntax(t *testing.T) {

diff --git a/service_test.go b/service_test.go
@@ -20,11 +20,8 @@ func TestService_Name(t *testing.T) {
 func TestService_FullyQualifiedName(t *testing.T) {
 	t.Parallel()
 
-	f := dummyFile()
-	s := &service{desc: &descriptor.ServiceDescriptorProto{Name: proto.String("foo")}}
-	f.addService(s)
-
-	assert.Equal(t, f.FullyQualifiedName()+".foo", s.FullyQualifiedName())
+	s := &service{fqn: "foo"}
+	assert.Equal(t, s.fqn, s.FullyQualifiedName())
 }
 
 func TestService_Syntax(t *testing.T) {