Skip to content

Commit

Permalink
thrift-gen: Add "namespace go" support.
Browse files Browse the repository at this point in the history
Currently, if "a.thrift" includes "b.thrift" but "b.thrift" has a
"namespace go" declaration, then the Apache Thrift generated code
uses the "namespace go" value as the package name. thrift-gen was
not respecting the "namespace go" and so the generated code would
not compile.

This will also help work around the following scenario. If we had:
- b/b.thrift which includes:
    b/shared.thrift
- c/c.thrift which includes
    c/shared.thrift

If a.thrift includes "b/b.thrift", everything is OK. As soon as it tries
to include "c/c.thrift", everything breaks since the "shared" package is
overwritten by the last compiled "shared.thrift". We can work around
this by adding a "namespace go" declaration to one of the shared.thrift
file to use a unique package for each of the shared.thrift files,
without having to rename the file (Which may break a lot of existing
code).

Fixes #101
  • Loading branch information
prashantv committed Jan 6, 2017
1 parent e8bf163 commit ff65066
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
13 changes: 6 additions & 7 deletions thrift/thrift-gen/include.go
Expand Up @@ -20,16 +20,13 @@

package main

import (
"strings"

"github.com/samuel/go-thrift/parser"
)
import "github.com/samuel/go-thrift/parser"

// Include represents a single include statement in the Thrift file.
type Include struct {
key string
file string
pkg string
}

// Import returns the go import to use for this package.
Expand All @@ -42,15 +39,17 @@ func (i *Include) Import() string {

// Package returns the package selector for this package.
func (i *Include) Package() string {
return strings.ToLower(i.key)
return i.pkg
}

func createIncludes(parsed *parser.Thrift) map[string]*Include {
func createIncludes(parsed *parser.Thrift, all map[string]parseState) map[string]*Include {
includes := make(map[string]*Include)
for k, v := range parsed.Includes {
included := all[v]
includes[k] = &Include{
key: k,
file: v,
pkg: included.namespace,
}
}
return includes
Expand Down
22 changes: 18 additions & 4 deletions thrift/thrift-gen/main.go
Expand Up @@ -129,9 +129,10 @@ func processFile(opts processOptions) error {
}

type parseState struct {
ast *parser.Thrift
global *State
services []*Service
ast *parser.Thrift
namespace string
global *State
services []*Service
}

// parseTemplates returns a list of Templates that must be rendered given the template files.
Expand Down Expand Up @@ -171,11 +172,24 @@ func parseFile(inputFile string) (map[string]parseState, error) {
if err != nil {
return nil, fmt.Errorf("wrap services failed: %v", err)
}
allParsed[filename] = parseState{v, state, services}

namespace := getNamespace(filename, v)
allParsed[filename] = parseState{v, namespace, state, services}
}
setIncludes(allParsed)
return allParsed, setExtends(allParsed)
}

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)
}

func generateCode(outputFile string, template *Template, pkg string, state parseState) error {
if outputFile == "" {
return fmt.Errorf("must speciy an output file")
Expand Down
@@ -0,0 +1,5 @@
namespace go a_shared

include "../b/shared.thrift"

typedef shared.b_string a_string
@@ -0,0 +1,3 @@
namespace go b_shared

typedef string b_string
@@ -0,0 +1,5 @@
include "a/shared.thrift"

service Foo {
void Foo(1: shared.a_string str)
}
8 changes: 7 additions & 1 deletion thrift/thrift-gen/typestate.go
Expand Up @@ -51,7 +51,13 @@ func newState(v *parser.Thrift, all map[string]parseState) *State {
typedefs[k] = i64Type
}

return &State{typedefs, createIncludes(v), all}
return &State{typedefs, nil, all}
}

func setIncludes(all map[string]parseState) {
for _, v := range all {
v.global.includes = createIncludes(v.ast, all)
}
}

func (s *State) isBasicType(thriftType string) bool {
Expand Down

0 comments on commit ff65066

Please sign in to comment.