thrift-gen: Use namespace to generate the output filename (#574)

The namespace was not being used everywhere in #559 -- extend the test
to create services so that all the tchan-*.go files are generated and
we can ensure it's created in the right directory.

testutils/data.go

@@ -37,8 +37,8 @@ var (
 
 func checkCacheSize(n int) {
 	// Start with a reasonably large cache.
-	if n < 8 {
-		n = 8
+	if n < 1024 {
+		n = 1024
 	}
 
 	randMut.RLock()

thrift/thrift-gen/compile_test.go

@@ -50,8 +50,8 @@ var (
 func TestMain(m *testing.M) {
 	exitCode := m.Run()
 
-	// If we created a fake GOPATH, we should clean it up.
-	if _testGoPath != "" {
+	// If we created a fake GOPATH, we should clean it up on success.
+	if _testGoPath != "" && exitCode == 0 {
 		os.RemoveAll(_testGoPath)
 	}
 
@@ -193,7 +193,7 @@ func TestExternalTemplate(t *testing.T) {
 		}
 
 		// Verify the contents of the extra file.
-		outFile := filepath.Join(dir, packageName(templateFile)+"-service_extend.go")
+		outFile := filepath.Join(dir, defaultPackageName(templateFile)+"-service_extend.go")
 		return verifyFileContents(outFile, expected)
 	}
 	if err := runTest(t, opts, checks); err != nil {
@@ -301,7 +301,7 @@ func checkDirectoryFiles(dir string, n int) error {
 
 func runBuildTest(t *testing.T, thriftFile string) error {
 	extraChecks := func(dir string) error {
-		return checkDirectoryFiles(filepath.Join(dir, packageName(thriftFile)), 4)
+		return checkDirectoryFiles(filepath.Join(dir, defaultPackageName(thriftFile)), 4)
 	}
 
 	opts := processOptions{InputFile: thriftFile}

thrift/thrift-gen/extends.go

@@ -45,6 +45,7 @@ func setExtends(state map[string]parseState) error {
 				searchFor = s.Extends
 			} else {
 				include := v.global.includes[parts[0]]
+				s.ExtendsPrefix = include.pkg + "."
 				searchServices = state[include.file].services
 				searchFor = parts[1]
 			}

thrift/thrift-gen/generate.go

@@ -71,7 +71,7 @@ func runThrift(inFile string, outDir string) error {
 	}
 
 	// Delete any existing generated code for this Thrift file.
-	genDir := filepath.Join(outDir, packageName(inFile))
+	genDir := filepath.Join(outDir, defaultPackageName(inFile))
 	if err := execCmd("rm", "-rf", genDir); err != nil {
 		return fmt.Errorf("failed to delete directory %s: %v", genDir, err)
 	}

thrift/thrift-gen/main.go

@@ -115,7 +115,7 @@ func processFile(opts processOptions) error {
 	}
 
 	for filename, v := range allParsed {
-		pkg := packageName(filename)
+		pkg := getNamespace(filename, v.ast)
 
 		for _, template := range allTemplates {
 			outputFile := filepath.Join(opts.OutputDir, pkg, template.outputFile(pkg))
@@ -180,14 +180,19 @@ func parseFile(inputFile string) (map[string]parseState, error) {
 	return allParsed, setExtends(allParsed)
 }
 
+func defaultPackageName(fullPath string) string {
+	filename := filepath.Base(fullPath)
+	file := strings.TrimSuffix(filename, filepath.Ext(filename))
+	return strings.ToLower(file)
+}
+
 func getNamespace(filename string, v *parser.Thrift) string {
 	if ns, ok := v.Namespaces["go"]; ok {
 		return ns
 	}
 
-	base := filepath.Base(filename)
-	base = strings.TrimSuffix(base, filepath.Ext(base))
-	return strings.ToLower(base)
+	// TODO(prashant): Remove any characters that are not valid in Go package names.
+	return defaultPackageName(filename)
 }
 
 func generateCode(outputFile string, template *Template, pkg string, state parseState) error {
@@ -212,13 +217,6 @@ func generateCode(outputFile string, template *Template, pkg string, state parse
 	return template.execute(outputFile, td)
 }
 
-func packageName(fullPath string) string {
-	// TODO(prashant): Remove any characters that are not valid in Go package names.
-	_, filename := filepath.Split(fullPath)
-	file := strings.TrimSuffix(filename, filepath.Ext(filename))
-	return strings.ToLower(file)
-}
-
 type stringSliceFlag []string
 
 func (s *stringSliceFlag) String() string {

thrift/thrift-gen/template.go

@@ -71,7 +71,7 @@ func parseTemplateFile(file string) (*Template, error) {
 		return nil, fmt.Errorf("failed to parse template in file %q: %v", file, err)
 	}
 
-	return &Template{packageName(file), t}, nil
+	return &Template{defaultPackageName(file), t}, nil
 }
 
 func contextType() string {

thrift/thrift-gen/test_files/include_test/namespace/a/shared.thrift

@@ -3,3 +3,7 @@ namespace go a_shared
 include "../b/shared.thrift"
 
 typedef shared.b_string a_string
+
+service AShared extends shared.BShared {
+	bool healthA()
+}

thrift/thrift-gen/test_files/include_test/namespace/b/shared.thrift

@@ -1,3 +1,7 @@
 namespace go b_shared
 
 typedef string b_string
+
+service BShared {
+	bool healthB()
+}

thrift/thrift-gen/test_files/include_test/namespace/namespace.thrift

@@ -1,5 +1,5 @@
 include "a/shared.thrift"
 
-service Foo {
+service Foo extends shared.AShared {
  void Foo(1: shared.a_string str)
 }

thrift/thrift-gen/wrap.go

@@ -37,8 +37,9 @@ type Service struct {
 	*parser.Service
 	state *State
 
-	// ExtendsService is not set in wrap, but in setExtends.
+	// ExtendsService and ExtendsPrefix are set in `setExtends`.
 	ExtendsService *Service
+	ExtendsPrefix  string
 
 	// methods is a cache of all methods.
 	methods []*Method
@@ -91,7 +92,7 @@ func (s *Service) HasExtends() bool {
 // ExtendsServicePrefix returns a package selector (if any) for the extended service.
 func (s *Service) ExtendsServicePrefix() string {
 	if dotIndex := strings.Index(s.Extends, "."); dotIndex > 0 {
-		return s.Extends[:dotIndex+1]
+		return s.ExtendsPrefix
 	}
 	return ""
 }