Skip to content

Commit

Permalink
'rust.type' fix for typedef rhs
Browse files Browse the repository at this point in the history
Summary:
thrift like
```
typdef map<string, i64> map_t (rust.type="sorted_vector_map::SortedVectorMap")
struct T {
  1: map_t data;
}
```
should generate rust like,
```
pub map_t = ::std::collections::BTreeMap<String, i64>;

pub struct T {
  pub data: sorted_vector_map::SortedVectorMap<String, i64>,
}

impl<P> fbthrift::Serialize<P> for
  LocalImpl<sorted_vector_map::SortedVectorMap<
    ::std::string::String, ::std::primitive::i64>> {
  ...}

impl<P> fbthrift::Deserialize<P> for
  LocalImpl<sorted_vector_map::SortedVectorMap<
    ::std::string::String, ::std::primitive::i64>> {
  ...}
```
but until now this has not been the case [^1]. this diff fixes that.

[^1]: substitution of `SortedVectorMap<...>` for `crate::types::map_t` in the definition of `T` has been lacking.

Reviewed By: zertosh

Differential Revision: D51478881

fbshipit-source-id: 9bcd1d6a25a9b433d2136fb84aaa56d410074b57
  • Loading branch information
shayne-fletcher authored and facebook-github-bot committed Nov 22, 2023
1 parent e95097a commit 8fd5297
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 17 deletions.
11 changes: 10 additions & 1 deletion thrift/compiler/generate/t_mstch_rust_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,14 @@ FieldKind field_kind(const t_named& node) {
return FieldKind::Inline;
}

auto get_type_annotation(const t_type* type) {
std::string get_type_annotation(const t_type* type) {
return t_typedef::get_first_annotation(type, {"rust.type"});
}

bool has_type_annotation(const t_type* type) {
return !get_type_annotation(type).empty();
}

// For example `set<Value> (rust.type = "indexmap::IndexSet")` or `map<string,
// Value> (rust.type = "indexmap::IndexMap")`. Unlike for standard library
// collections, serialization impls for these types are not provided in the
Expand Down Expand Up @@ -1347,6 +1351,7 @@ class rust_mstch_type : public mstch_type {
{"type:rust_name_snake", &rust_mstch_type::rust_name_snake},
{"type:package", &rust_mstch_type::rust_package},
{"type:rust", &rust_mstch_type::rust_type},
{"type:type_annotation?", &rust_mstch_type::rust_type_annotation},
{"type:nonstandard?", &rust_mstch_type::rust_nonstandard},
{"type:has_adapter?", &rust_mstch_type::adapter},
});
Expand All @@ -1370,6 +1375,10 @@ class rust_mstch_type : public mstch_type {
}
return rust_type;
}
mstch::node rust_type_annotation() {
return has_type_annotation(type_) &&
!(type_->is_typedef() && type_->has_annotation("rust.newtype"));
}
mstch::node rust_nonstandard() {
return (
has_nonstandard_type_annotation(type_) &&
Expand Down
3 changes: 2 additions & 1 deletion thrift/compiler/generate/templates/rust/lib/type.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
limitations under the License.
}}{{#type:typedef?}}{{!
}}{{type:package}}::types::{{type:rust_name}}{{!
}}{{#type:type_annotation?}}{{>lib/rawtype}}{{/type:type_annotation?}}{{!
}}{{^type:type_annotation?}}{{type:package}}::types::{{type:rust_name}}{{/type:type_annotation?}}{{!
}}{{/type:typedef?}}{{!
}}{{^type:typedef?}}{{!
}}{{>lib/rawtype}}{{!
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions thrift/compiler/test/fixtures/types/gen-rust/lib.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions thrift/compiler/test/fixtures/types/gen-rust/types.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 41 additions & 2 deletions thrift/lib/rust/test/if.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ struct WrapString {
1: string data;
} (rust.exhaustive)

struct NonstandardCollectionTypes {
// The field types here are each of a form `typedef a (rust.type="c") b`. That
// is, are non-default types given as `rust.type` annotations where the
// annotations attach to the **left** type of the thrift `typedef` in which they
// appear.
struct TypedefNondefaultTypes_1 {
10: set<string> defaultset;
11: set_string_9948 btreeset;
12: set_string_3129 hashset;
Expand All @@ -41,6 +45,29 @@ struct NonstandardCollectionTypes {
31: binary_1562 bin_bytes;
}

// The field types here are each of a form `typedef a b (rust.type="c")`. That
// is, are non-default types given as `rust.type` annotations where the
// annotations attach to the **right** type of the thrift `typedef` in which
// they appear.
struct TypedefNondefaultTypes_2 {
10: set<string> defaultset;
11: set_string_9949 btreeset;
12: set_string_3130 hashset;
13: set_string_8509 indexset_a;
14: set_string_8509 indexset_b;
15: set_i64_4483 indexset_c;

20: map<string, string> defaultmap;
21: map_string_string_4845 btreemap;
22: map_string_string_2455 hashmap;
23: map_string_string_4180 indexmap_a;
24: map_string_string_4180 indexmap_b;
25: map_string_i64_1189 indexmap_c;

30: binary_460 bin_smallvec;
31: binary_1563 bin_bytes;
}

enum TestEnum {
FOO = 1,
BAR = 2,
Expand All @@ -62,7 +89,6 @@ struct TestBytesShared {
1: binary_1562 b;
}

// The following were automatically generated and may benefit from renaming.
typedef binary (rust.type = "Bytes") binary_1562
typedef binary (rust.type = "smallvec::SmallVec<[u8; 32]>") binary_459
typedef map<string, i64> (rust.type = "indexmap::IndexMap") map_string_i64_1188
Expand All @@ -75,3 +101,16 @@ typedef set<i64> (rust.type = "indexmap::IndexSet") set_i64_4482
typedef set<string> (rust.type = "HashSet") set_string_3129
typedef set<string> (rust.type = "indexmap::IndexSet") set_string_8508
typedef set<string> (rust.type = "BTreeSet") set_string_9948

typedef binary binary_1563 (rust.type = "Bytes")
typedef binary binary_460 (rust.type = "smallvec::SmallVec<[u8; 32]>")
typedef map<string, i64> map_string_i64_1189 (rust.type = "indexmap::IndexMap")
typedef map<string, string> map_string_string_2455 (rust.type = "HashMap")
typedef map<string, string> map_string_string_4180 (
rust.type = "indexmap::IndexMap",
)
typedef map<string, string> map_string_string_4845 (rust.type = "BTreeMap")
typedef set<i64> set_i64_4483 (rust.type = "indexmap::IndexSet")
typedef set<string> set_string_3130 (rust.type = "HashSet")
typedef set<string> set_string_8509 (rust.type = "indexmap::IndexSet")
typedef set<string> set_string_9949 (rust.type = "BTreeSet")
28 changes: 25 additions & 3 deletions thrift/lib/rust/test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,41 @@ use fbthrift::Serialize;
use fbthrift::ThriftEnum;
use indexmap::IndexMap;
use indexmap::IndexSet;
use interface::NonstandardCollectionTypes;
use interface::TestBytesShared;
use interface::TestEnum;
use interface::TestEnumEmpty;
use interface::TestSkipV1;
use interface::TestSkipV2;
use interface::TypedefNondefaultTypes_1;
use interface::TypedefNondefaultTypes_2;
use interface::WrapBinary;
use interface::WrapString;
use smallvec::SmallVec;

#[test]
fn test_nonstandard_collection_types() {
assert_round_trip(NonstandardCollectionTypes {
fn test_typedef_nondefault_types_1() {
assert_round_trip(TypedefNondefaultTypes_1 {
defaultset: make_btreeset(),
btreeset: make_btreeset(),
hashset: make_hashset(),
indexset_a: make_indexset(),
indexset_b: make_indexset(),
indexset_c: IndexSet::new(),
defaultmap: make_btreemap(),
btreemap: make_btreemap(),
hashmap: make_hashmap(),
indexmap_a: make_indexmap(),
indexmap_b: make_indexmap(),
indexmap_c: IndexMap::new(),
bin_smallvec: SmallVec::from(&b"smallvec"[..]),
bin_bytes: Bytes::from(&b"bytes"[..]),
..Default::default()
});
}

#[test]
fn test_typedef_non_default_types_2() {
assert_round_trip(TypedefNondefaultTypes_2 {
defaultset: make_btreeset(),
btreeset: make_btreeset(),
hashset: make_hashset(),
Expand Down

0 comments on commit 8fd5297

Please sign in to comment.