twitter / scrooge

A Thrift parser/generator
http://twitter.github.io/scrooge/
Apache License 2.0
793 stars 247 forks source link

feat: support fully-qualified type name for fields #321

Closed chenzhuoyu closed 4 years ago

chenzhuoyu commented 4 years ago

Problem

Currently scrooge-generator does not support more than one dot in thrift file name.

This might be related to issue #44 , but since it's been 7 years, I decided to fix it myself.

Suppose we have a thrift file with the name com.whatever.product.module1.thrift:

struct Foo {
    1: i32 foo_field;
}

... and another thrift file that includes com.whatever.product.module1.thrift:

include "com.whatever.product.module1.thrift"
struct Bar {
    1: com.whatever.product.module1.Foo bar_field;
}

This doesn't work with the current version of scrooge-generator.

Solution

I found something in code that appears to be assuming the base part of a file name to be a SimpleID, which in fact isn't the case. Changing the scopePrefix to Identifier & other related places fixes this problem.

dotordogh commented 4 years ago

Thank you @chenzhuoyu! Please also add a test for this as well to ensure consistent behavior in the future.

mosesn commented 4 years ago

@chenzhuoyu have you had a chance to take a stab at adding a test case?

chenzhuoyu commented 4 years ago

@mosesn @dotordogh Test case added

bryce-anderson commented 4 years ago

Closed by 4fad471b63fde8022ca846624e37d8a6d387020c. Thanks!