valitydev/thrift
14
0
Fork 0
mirror of https://github.com/valitydev/thrift.git synced 2024-11-06 18:35:19 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

THRIFT-5389: Fix const generation for optional fields

Browse Source 
Client: go

The current compiler will generate uncompilable code when we use
optional enum and/or typedef'd types in a thrift constant.

This fixes the issue, also adds a test for that.
This commit is contained in:
Yuxuan 'fishy' Wang 2021-07-29 15:59:10 -07:00 committed by Yuxuan 'fishy' Wang
parent 68c0272a0a
commit f695535122
5 changed files with 361 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
compiler/cpp/src/thrift/generate/t_go_generator.cc
View File

@ -1102,6 +1102,10 @@ void t_go_generator::generate_const(t_const* tconst) {
* validate_types method in main.cc
*/
string t_go_generator::render_const_value(t_type* type, t_const_value* value, const string& name, bool opt) {
string typedef_opt_ptr;
if (type->is_typedef()) {
typedef_opt_ptr = type_name(type) + "Ptr";
}
type = get_true_type(type);
std::ostringstream out;
@ -1109,32 +1113,61 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
if (opt) {
out << "&(&struct{x ";
switch (tbase) {
case t_base_type::TYPE_BOOL:
out << "bool}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.BoolPtr";
}
out << "(";
out << (value->get_integer() > 0 ? "true" : "false");
break;
case t_base_type::TYPE_I8:
out << "int8}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.Int8Ptr";
}
out << "(";
out << value->get_integer();
break;
case t_base_type::TYPE_I16:
out << "int16}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.Int16Ptr";
}
out << "(";
out << value->get_integer();
break;
case t_base_type::TYPE_I32:
out << "int32}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.Int32Ptr";
}
out << "(";
out << value->get_integer();
break;
case t_base_type::TYPE_I64:
out << "int64}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.Int64Ptr";
}
out << "(";
out << value->get_integer();
break;
case t_base_type::TYPE_DOUBLE:
out << "float64}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.Float64Ptr";
}
out << "(";
if (value->get_type() == t_const_value::CV_INTEGER) {
out << value->get_integer();
} else {
@ -1143,14 +1176,19 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
break;
case t_base_type::TYPE_STRING:
out << "string}{";
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr;
} else {
out << "thrift.StringPtr";
}
out << "(";
out << '"' + get_escaped_string(value) + '"';
break;
default:
throw "compiler error: no const of base type " + t_base_type::t_base_name(tbase);
}
out << "}).x";
out << ")";
} else {
switch (tbase) {
case t_base_type::TYPE_STRING:
@ -1193,7 +1231,17 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
}
}
} else if (type->is_enum()) {
indent(out) << value->get_integer();
if (opt) {
if (typedef_opt_ptr != "") {
out << typedef_opt_ptr << "(";
} else {
out << type_name(type) << "Ptr(";
}
}
out << value->get_integer();
if (opt) {
out << ")";
}
} else if (type->is_struct() || type->is_xception()) {
out << "&" << publicize(type_name(type)) << "{";
indent_up();

111
lib/go/test/ConstOptionalField.thrift Normal file
View File

@ -0,0 +1,111 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
namespace go constoptionalfieldb
include "ConstOptionalFieldImport.thrift"
typedef ConstOptionalFieldImport.Foo TypedefBFoo
typedef bool TypedefBBool
typedef i8 TypedefBI8
typedef i16 TypedefBI16
typedef i32 TypedefBI32
typedef i64 TypedefBI64
typedef double TypedefBDouble
typedef string TypedefBString
typedef binary TypedefBBinary
struct Bar {
1: optional ConstOptionalFieldImport.Foo optFoo,
2: optional ConstOptionalFieldImport.TypedefAFoo aFoo,
3: optional TypedefBFoo bFoo,
4: optional bool optBool,
5: optional ConstOptionalFieldImport.TypedefABool aBool,
6: optional TypedefBBool bBool,
7: optional i8 optI8,
8: optional ConstOptionalFieldImport.TypedefAI8 aI8,
9: optional TypedefBI8 bI8,
10: optional i16 optI16,
11: optional ConstOptionalFieldImport.TypedefAI16 aI16,
12: optional TypedefBI16 bI16,
13: optional i32 optI32,
14: optional ConstOptionalFieldImport.TypedefAI32 aI32,
15: optional TypedefBI32 bI32,
16: optional i64 optI64,
17: optional ConstOptionalFieldImport.TypedefAI64 aI64,
18: optional TypedefBI64 bI64,
19: optional double optDouble,
20: optional ConstOptionalFieldImport.TypedefADouble aDouble,
21: optional TypedefBDouble bDouble,
22: optional string optString,
23: optional ConstOptionalFieldImport.TypedefAString aString,
24: optional TypedefBString bString,
25: optional binary optBinary,
26: optional ConstOptionalFieldImport.TypedefABinary aBinary,
27: optional TypedefBBinary bBinary,
}
const list<Bar> CONSTANTS = [
{
"optFoo": ConstOptionalFieldImport.Foo.One,
"aFoo": ConstOptionalFieldImport.Foo.One,
"bFoo": ConstOptionalFieldImport.Foo.One,
"optBool": true,
"aBool": true,
"bBool": true,
"optI8": 8,
"aI8": 8,
"bI8": 8,
"optI16": 16,
"aI16": 16,
"bI16": 16,
"optI32": 32,
"aI32": 32,
"bI32": 32,
"optI64": 64,
"aI64": 64,
"bI64": 64,
"optDouble": 1234,
"aDouble": 1234,
"bDouble": 1234,
"optString": "string",
"aString": "string",
"bString": "string",
"optBinary": "binary",
"aBinary": "binary",
"bBinary": "binary",
},
]

36
lib/go/test/ConstOptionalFieldImport.thrift Normal file
View File

@ -0,0 +1,36 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
namespace go constoptionalfielda
enum Foo {
One = 1,
Two = 2,
}
typedef Foo TypedefAFoo
typedef bool TypedefABool
typedef i8 TypedefAI8
typedef i16 TypedefAI16
typedef i32 TypedefAI32
typedef i64 TypedefAI64
typedef double TypedefADouble
typedef string TypedefAString
typedef binary TypedefABinary

7
lib/go/test/Makefile.am
View File

@ -48,7 +48,9 @@ gopath: $(THRIFT) $(THRIFTTEST) \
ConflictNamespaceServiceTest.thrift \
DuplicateImportsTest.thrift \
EqualsTest.thrift \
ConflictArgNamesTest.thrift
ConflictArgNamesTest.thrift \
ConstOptionalFieldImport.thrift \
ConstOptionalField.thrift
mkdir -p gopath/src
grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set<Insanity>' > ThriftTest.thrift
$(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift
@ -77,6 +79,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
$(THRIFT) $(THRIFTARGS) -r DuplicateImportsTest.thrift
$(THRIFT) $(THRIFTARGS) EqualsTest.thrift
$(THRIFT) $(THRIFTARGS) ConflictArgNamesTest.thrift
$(THRIFT) $(THRIFTARGS) -r ConstOptionalField.thrift
ln -nfs ../../tests gopath/src/tests
cp -r ./dontexportrwtest gopath/src
touch gopath
@ -121,6 +124,8 @@ EXTRA_DIST = \
ConflictNamespaceTestC.thrift \
ConflictNamespaceTestD.thrift \
ConflictNamespaceTestSuperThing.thrift \
ConstOptionalField.thrift \
ConstOptionalFieldImport.thrift \
DontExportRWTest.thrift \
DuplicateImportsTest.thrift \
ErrorTest.thrift \

150
lib/go/test/tests/const_optional_field_test.go Normal file
View File

@ -0,0 +1,150 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package tests
import (
"testing"
"github.com/apache/thrift/lib/go/test/gopath/src/constoptionalfielda"
"github.com/apache/thrift/lib/go/test/gopath/src/constoptionalfieldb"
)
func TestConstOptionalField(t *testing.T) {
c := constoptionalfieldb.CONSTANTS[0]
t.Run("foo", func(t *testing.T) {
const expected = constoptionalfielda.Foo_One
if *c.OptFoo != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptFoo)
}
if *c.AFoo != constoptionalfielda.TypedefAFoo(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.AFoo)
}
if *c.BFoo != constoptionalfieldb.TypedefBFoo(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BFoo)
}
})
t.Run("bool", func(t *testing.T) {
const expected = true
if *c.OptBool != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptBool)
}
if *c.ABool != constoptionalfielda.TypedefABool(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.ABool)
}
if *c.BBool != constoptionalfieldb.TypedefBBool(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BBool)
}
})
t.Run("i8", func(t *testing.T) {
const expected = 8
if *c.OptI8 != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptI8)
}
if *c.AI8 != constoptionalfielda.TypedefAI8(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.AI8)
}
if *c.BI8 != constoptionalfieldb.TypedefBI8(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BI8)
}
})
t.Run("i16", func(t *testing.T) {
const expected = 16
if *c.OptI16 != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptI16)
}
if *c.AI16 != constoptionalfielda.TypedefAI16(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.AI16)
}
if *c.BI16 != constoptionalfieldb.TypedefBI16(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BI16)
}
})
t.Run("i32", func(t *testing.T) {
const expected = 32
if *c.OptI32 != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptI32)
}
if *c.AI32 != constoptionalfielda.TypedefAI32(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.AI32)
}
if *c.BI32 != constoptionalfieldb.TypedefBI32(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BI32)
}
})
t.Run("i64", func(t *testing.T) {
const expected = 64
if *c.OptI64 != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptI64)
}
if *c.AI64 != constoptionalfielda.TypedefAI64(expected) {
t.Errorf("Typedef a expected %v, got %v", expected, *c.AI64)
}
if *c.BI64 != constoptionalfieldb.TypedefBI64(expected) {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BI64)
}
})
t.Run("double", func(t *testing.T) {
// To avoid the annoyance of comparing float numbers,
// we convert all floats to int in this test.
const expected = 1234
if int(*c.OptDouble) != expected {
t.Errorf("Expected %v, got %v", expected, *c.OptDouble)
}
if int(*c.ADouble) != expected {
t.Errorf("Typedef a expected %v, got %v", expected, *c.ADouble)
}
if int(*c.BDouble) != expected {
t.Errorf("Typedef b expected %v, got %v", expected, *c.BDouble)
}
})
t.Run("string", func(t *testing.T) {
const expected = "string"
if *c.OptString != expected {
t.Errorf("Expected %q, got %q", expected, *c.OptString)
}
if *c.AString != constoptionalfielda.TypedefAString(expected) {
t.Errorf("Typedef a expected %q, got %q", expected, *c.AString)
}
if *c.BString != constoptionalfieldb.TypedefBString(expected) {
t.Errorf("Typedef b expected %q, got %q", expected, *c.BString)
}
})
t.Run("binary", func(t *testing.T) {
const expected = "binary"
if string(c.OptBinary) != expected {
t.Errorf("Expected %q, got %q", expected, c.OptBinary)
}
if string(c.ABinary) != expected {
t.Errorf("Typedef a expected %q, got %q", expected, c.ABinary)
}
if string(c.BBinary) != expected {
t.Errorf("Typedef b expected %q, got %q", expected, c.BBinary)
}
})
}