THRIFT-4086: Use true type when generating field meta data (#2765)
Client: java
diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc
index ee98600..4259cf8 100644
--- a/compiler/cpp/src/thrift/generate/t_java_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc
@@ -3023,46 +3023,46 @@
}
void t_java_generator::generate_field_value_meta_data(std::ostream& out, t_type* type) {
+ t_type* ttype = get_true_type(type);
out << endl;
indent_up();
indent_up();
- t_type* ttype = get_true_type(type);
if (ttype->is_struct() || ttype->is_xception()) {
indent(out) << "new "
"org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType."
"STRUCT, "
- << type_name(type) << ".class";
- } else if (type->is_container()) {
- if (type->is_list()) {
+ << type_name(ttype) << ".class";
+ } else if (ttype->is_container()) {
+ if (ttype->is_list()) {
indent(out)
<< "new org.apache.thrift.meta_data.ListMetaData(org.apache.thrift.protocol.TType.LIST, ";
- t_type* elem_type = ((t_list*)type)->get_elem_type();
+ t_type* elem_type = ((t_list*)ttype)->get_elem_type();
generate_field_value_meta_data(out, elem_type);
- } else if (type->is_set()) {
+ } else if (ttype->is_set()) {
indent(out)
<< "new org.apache.thrift.meta_data.SetMetaData(org.apache.thrift.protocol.TType.SET, ";
- t_type* elem_type = ((t_set*)type)->get_elem_type();
+ t_type* elem_type = ((t_set*)ttype)->get_elem_type();
generate_field_value_meta_data(out, elem_type);
} else { // map
indent(out)
<< "new org.apache.thrift.meta_data.MapMetaData(org.apache.thrift.protocol.TType.MAP, ";
- t_type* key_type = ((t_map*)type)->get_key_type();
- t_type* val_type = ((t_map*)type)->get_val_type();
+ t_type* key_type = ((t_map*)ttype)->get_key_type();
+ t_type* val_type = ((t_map*)ttype)->get_val_type();
generate_field_value_meta_data(out, key_type);
out << ", ";
generate_field_value_meta_data(out, val_type);
}
- } else if (type->is_enum()) {
+ } else if (ttype->is_enum()) {
indent(out)
<< "new org.apache.thrift.meta_data.EnumMetaData(org.apache.thrift.protocol.TType.ENUM, "
- << type_name(type) << ".class";
+ << type_name(ttype) << ".class";
} else {
indent(out) << "new org.apache.thrift.meta_data.FieldValueMetaData("
- << get_java_type_string(type);
- if (type->is_typedef()) {
- indent(out) << ", \"" << ((t_typedef*)type)->get_symbolic() << "\"";
- } else if (type->is_binary()) {
+ << get_java_type_string(ttype);
+ if (ttype->is_binary()) {
indent(out) << ", true";
+ } else if (type->is_typedef()) {
+ indent(out) << ", \"" << ((t_typedef*)type)->get_symbolic() << "\"";
}
}
out << ")";
diff --git a/compiler/cpp/src/thrift/generate/t_oop_generator.h b/compiler/cpp/src/thrift/generate/t_oop_generator.h
index 2df1be4..8841962 100644
--- a/compiler/cpp/src/thrift/generate/t_oop_generator.h
+++ b/compiler/cpp/src/thrift/generate/t_oop_generator.h
@@ -70,7 +70,7 @@
}
virtual void generate_java_doc(std::ostream& out, t_field* field) {
- if (field->get_type()->is_enum()) {
+ if (get_true_type(field->get_type())->is_enum()) {
std::string combined_message = field->get_doc() + "\n@see "
+ get_enum_class_name(field->get_type());
generate_java_docstring_comment(out, combined_message);
diff --git a/lib/java/gradle/generateTestThrift.gradle b/lib/java/gradle/generateTestThrift.gradle
index 301812f..10bb16b 100644
--- a/lib/java/gradle/generateTestThrift.gradle
+++ b/lib/java/gradle/generateTestThrift.gradle
@@ -26,8 +26,8 @@
ext.genFullCamelSrc = file("$buildDir/gen-fullcamel")
ext.genOptionTypeJdk8Src = file("$buildDir/gen-option-type-jdk8")
ext.genUnsafeSrc = file("$buildDir/gen-unsafe")
-ext.genStructOrderTestASrc = file("$buildDir/resources/test/struct-order-test/a")
-ext.genStructOrderTestBSrc = file("$buildDir/resources/test/struct-order-test/b")
+ext.genDefinitionOrderTestASrc = file("$buildDir/resources/test/definition-order-test/a")
+ext.genDefinitionOrderTestBSrc = file("$buildDir/resources/test/definition-order-test/b")
// Add the generated code directories to the test source set
sourceSets {
@@ -146,12 +146,12 @@
thriftCompile(it, 'JavaAnnotationTest.thrift', 'java:annotations_as_metadata', genSrc)
}
-task generateJavaStructOrderTestJava(group: 'Build') {
- description = 'Generate structs defined in different order and add to build dir so we can compare them'
+task generateJavaDefinitionOrderTestJava(group: 'Build') {
+ description = 'Generate fields defined in different order and add to build dir so we can compare them'
generate.dependsOn it
ext.outputBuffer = new ByteArrayOutputStream()
- thriftCompile(it, 'JavaStructOrderA.thrift', 'java', genStructOrderTestASrc)
- thriftCompile(it, 'JavaStructOrderB.thrift', 'java', genStructOrderTestBSrc)
+ thriftCompile(it, 'JavaDefinitionOrderA.thrift', 'java', genDefinitionOrderTestASrc)
+ thriftCompile(it, 'JavaDefinitionOrderB.thrift', 'java', genDefinitionOrderTestBSrc)
}
diff --git a/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java b/lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java
similarity index 78%
rename from lib/java/src/test/java/org/apache/thrift/TestStructOrder.java
rename to lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java
index dd67d9b..51021b7 100644
--- a/lib/java/src/test/java/org/apache/thrift/TestStructOrder.java
+++ b/lib/java/src/test/java/org/apache/thrift/TestDefinitionOrder.java
@@ -26,20 +26,20 @@
import java.util.List;
import org.junit.jupiter.api.Test;
-// Tests that declaring structs in different order (esp. when they reference each other) generates
+// Tests that declaring fields in different order (esp. when they reference each other) generates
// identical code
-public class TestStructOrder {
+public class TestDefinitionOrder {
@Test
- public void testStructOrder() throws Exception {
- List<String> filenames = Arrays.asList("Parent.java", "Child.java");
+ public void testDefinitionOrder() throws Exception {
+ List<String> filenames = Arrays.asList("Parent.java", "Child.java", "MyEnum.java");
for (String fn : filenames) {
- String fnA = "struct-order-test/a/" + fn;
- String fnB = "struct-order-test/b/" + fn;
+ String fnA = "definition-order-test/a/" + fn;
+ String fnB = "definition-order-test/b/" + fn;
- try (InputStream isA = TestStructOrder.class.getClassLoader().getResourceAsStream(fnA);
- InputStream isB = TestStructOrder.class.getClassLoader().getResourceAsStream(fnB)) {
+ try (InputStream isA = TestDefinitionOrder.class.getClassLoader().getResourceAsStream(fnA);
+ InputStream isB = TestDefinitionOrder.class.getClassLoader().getResourceAsStream(fnB)) {
assertNotNull(isA, "Resource not found: " + fnA);
assertNotNull(isB, "Resource not found: " + fnB);
diff --git a/lib/java/src/test/resources/JavaStructOrderA.thrift b/lib/java/src/test/resources/JavaDefinitionOrderA.thrift
similarity index 64%
rename from lib/java/src/test/resources/JavaStructOrderA.thrift
rename to lib/java/src/test/resources/JavaDefinitionOrderA.thrift
index f00a710..9b42964 100644
--- a/lib/java/src/test/resources/JavaStructOrderA.thrift
+++ b/lib/java/src/test/resources/JavaDefinitionOrderA.thrift
@@ -22,7 +22,29 @@
1: required string Name
}
+typedef Parent MyParent
+typedef list<Parent> Parents
+
+enum MyEnum {
+ FOO = 1
+ BAR = 2
+}
+
+typedef i8 Age
+typedef MyEnum MyEnumV2
+typedef set<MyEnum> MyEnums
+typedef map<MyEnumV2, Parent> MyMapping
+typedef binary MyBinary
+
struct Child {
1: required string Name
- 2: required Parent Parent
+ 2: required Age Age
+ 3: required Parent Parent1
+ 4: required MyParent Parent2
+ 5: required Parents GrandParents
+ 6: required MyEnum MyEnum
+ 7: required MyEnumV2 MyEnumV2
+ 8: required MyEnums MyEnums
+ 9: required MyMapping MyMapping
+ 10: required MyBinary MyBinary
}
diff --git a/lib/java/src/test/resources/JavaStructOrderB.thrift b/lib/java/src/test/resources/JavaDefinitionOrderB.thrift
similarity index 68%
rename from lib/java/src/test/resources/JavaStructOrderB.thrift
rename to lib/java/src/test/resources/JavaDefinitionOrderB.thrift
index 4682668..0cccd97 100644
--- a/lib/java/src/test/resources/JavaStructOrderB.thrift
+++ b/lib/java/src/test/resources/JavaDefinitionOrderB.thrift
@@ -21,9 +21,30 @@
// fixing THRIFT-4086: Java compiler generates different meta data depending on order of structures in file
struct Child {
1: required string Name
- 2: required Parent Parent
+ 2: required Age Age
+ 3: required Parent Parent1
+ 4: required MyParent Parent2
+ 5: required Parents GrandParents
+ 6: required MyEnum MyEnum
+ 7: required MyEnumV2 MyEnumV2
+ 8: required MyEnums MyEnums
+ 9: required MyMapping MyMapping
+ 10: required MyBinary MyBinary
}
+typedef i8 Age
+typedef Parent MyParent
+typedef list<Parent> Parents
+typedef MyEnum MyEnumV2
+typedef set<MyEnum> MyEnums
+typedef map<MyEnumV2, Parent> MyMapping
+typedef binary MyBinary
+
struct Parent {
1: required string Name
}
+
+enum MyEnum {
+ FOO = 1
+ BAR = 2
+}