Hyphenated file support

compile/compiler.go

@@ -22,6 +22,7 @@ package compile
 
 import (
 	"path/filepath"
+	"strings"
 
 	"go.uber.org/thriftrw/ast"
 	"go.uber.org/thriftrw/idl"
@@ -242,6 +243,7 @@ func (c compiler) gather(m *Module, prog *ast.Program) error {
 // include loads the file specified by the given include in the given Module.
 //
 // The path to the file is relative to the ThriftPath of the given module.
+// Including hyphenated file names will error.
 func (c compiler) include(m *Module, include *ast.Include) (*IncludedModule, error) {
 	if len(include.Name) > 0 {
 		// TODO(abg): Add support for include-as flag somewhere.
@@ -251,6 +253,13 @@ func (c compiler) include(m *Module, include *ast.Include) (*IncludedModule, err
 		}
 	}
 
+	if strings.Contains(fileBaseName(include.Path), "-") {
+		return nil, includeError{
+			Include: include,
+			Reason:  includeHyphenatedFileNameError{},
+		}
+	}
+
 	ipath := filepath.Join(filepath.Dir(m.ThriftPath), include.Path)
 	incM, err := c.load(ipath)
 	if err != nil {

compile/error.go

@@ -64,6 +64,14 @@ func (e includeAsDisabledError) Error() string {
 	return "include-as syntax is currently disabled"
 }
 
+// includeHyphenatedFileNameError is raised when the user attempts to
+// include hyphenated file names.
+type includeHyphenatedFileNameError struct{}
+
+func (e includeHyphenatedFileNameError) Error() string {
+	return "cannot include hyphenated Thrift files"
+}
+
 // includeError is raised when there is an error including another Thrift
 // file.
 type includeError struct {

gen/generate.go

@@ -165,6 +165,12 @@ func Generate(m *compile.Module, o *Options) error {
 	return nil
 }
 
+// normalizeFilePath replaces hyphens in the file name with underscores.
+func normalizeFilePath(p string) string {
+	normalizedName := strings.Replace(filepath.Base(p), "-", "_", -1)
+	return filepath.Join(filepath.Dir(p), normalizedName)
+}
+
 // ThriftPackageImporter determines import paths from a Thrift root.
 type ThriftPackageImporter interface {
 	// RelativePackage returns the import path for the top-level package of the
@@ -186,7 +192,7 @@ type thriftPackageImporter struct {
 }
 
 func (i thriftPackageImporter) RelativePackage(file string) (string, error) {
-	return filepath.Rel(i.ThriftRoot, strings.TrimSuffix(file, ".thrift"))
+	return filepath.Rel(i.ThriftRoot, strings.TrimSuffix(normalizeFilePath(file), ".thrift"))
 }
 
 func (i thriftPackageImporter) RelativeThriftFilePath(file string) (string, error) {
@@ -226,25 +232,25 @@ func generateModule(
 	builder *generateServiceBuilder,
 	o *Options,
 ) (outputFilepath string, contents []byte, err error) {
-	// packageRelPath is the path relative to outputDir into which we'll be
+	// thriftRelPath is the path relative to outputDir into which we'll be
 	// writing the package for this Thrift file. For $thriftRoot/foo/bar.thrift,
 	// packageRelPath is foo/bar, and packageDir is $outputDir/foo/bar. All
 	// files for bar.thrift will be written to the $outputDir/foo/bar/ tree. The
 	// package will be importable via $importPrefix/foo/bar.
-	packageRelPath, err := i.RelativePackage(m.ThriftPath)
+	thriftRelPath, err := i.RelativeThriftFilePath(m.ThriftPath)
 	if err != nil {
 		return "", nil, err
 	}
-
+	thriftRelPath = strings.TrimSuffix(thriftRelPath, ".thrift")
 	// TODO(abg): Prefer top-level package name from `namespace go` directive.
-	packageName := filepath.Base(packageRelPath)
+	outputFilename := filepath.Base(thriftRelPath)
 
 	// Output file name defaults to the package name.
-	outputFilename := packageName + ".go"
+	outputFilename = outputFilename + ".go"
 	if len(o.OutputFile) > 0 {
 		outputFilename = o.OutputFile
 	}
-	outputFilepath = filepath.Join(packageRelPath, outputFilename)
+	outputFilepath = filepath.Join(thriftRelPath, outputFilename)
 
 	// importPath is the full import path for the top-level package generated
 	// for this Thrift file.
@@ -253,10 +259,16 @@ func generateModule(
 		return "", nil, err
 	}
 
+	// converts package name from ab-def to ab_def for golang code generation
+	packageRelPath, err := i.RelativePackage(m.ThriftPath)
+	if err != nil {
+		return "", nil, err
+	}
+	normalizedPackageName := filepath.Base(packageRelPath)
 	g := NewGenerator(&GeneratorOptions{
 		Importer:    i,
 		ImportPath:  importPath,
-		PackageName: packageName,
+		PackageName: normalizedPackageName,
 		NoZap:       o.NoZap,
 	})
 

gen/generate_test.go

@@ -79,6 +79,62 @@ func TestGenerateWithRelativePaths(t *testing.T) {
 	}
 }
 
+func TestGenerateWithHyphenPaths(t *testing.T) {
+	outputDir, err := ioutil.TempDir("", "thriftrw-generate-test")
+	require.NoError(t, err)
+	defer os.RemoveAll(outputDir)
+
+	thriftRoot, err := os.Getwd()
+	require.NoError(t, err)
+
+	tests := []struct {
+		name          string
+		filepath      string
+		generateError string
+		compileError  string
+	}{
+		{
+			name:         "include with hyphens error",
+			filepath:     "internal/tests/thrift/nestedfiles_error/include_hyphen_files.thrift",
+			compileError: "cannot include hyphenated Thrift files",
+		},
+		{
+			name:     "normalization - hyphen files as top level file allowed",
+			filepath: "internal/tests/thrift/hyphenated-file.thrift",
+		},
+		{
+			name:     "normalization - non hyphen file with hyphen in the directory",
+			filepath: "internal/tests/thrift/hyphenated_file.thrift",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			module, err := compile.Compile(tt.filepath)
+			if tt.compileError != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.compileError)
+				return
+			}
+			require.NoError(t, err)
+			require.NotNil(t, module)
+
+			opt := &Options{
+				OutputDir:     outputDir,
+				PackagePrefix: "go.uber.org/thriftrw/gen",
+				ThriftRoot:    thriftRoot,
+			}
+			err = Generate(module, opt)
+			if tt.generateError != "" {
+				require.Error(t, err, "expected code generation with filepath %v to fail", tt.filepath)
+				assert.Contains(t, err.Error(), tt.generateError)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestGenerate(t *testing.T) {
 	var (
 		ts compile.TypeSpec = &compile.TypedefSpec{

gen/internal/tests/Makefile

@@ -18,4 +18,4 @@ nozap: thrift/nozap.thrift $(THRIFTRW)
 	$(THRIFTRW) --no-recurse --no-zap $<
 
 %: thrift/%.thrift $(THRIFTRW)
-	$(THRIFTRW) --no-recurse $<
+	$(THRIFTRW) $<