diff options
| -rw-r--r-- | compiler/cpp/src/thrift/generate/t_rs_generator.cc | 133 | ||||
| -rw-r--r-- | lib/rs/README.md | 47 | ||||
| -rw-r--r-- | lib/rs/test/src/bin/kitchen_sink_server.rs | 6 | ||||
| -rw-r--r-- | test/rs/src/bin/test_client.rs | 14 | ||||
| -rw-r--r-- | test/rs/src/bin/test_server.rs | 6 | ||||
| -rw-r--r-- | tutorial/rs/src/bin/tutorial_client.rs | 4 | ||||
| -rw-r--r-- | tutorial/rs/src/bin/tutorial_server.rs | 21 |
7 files changed, 159 insertions, 72 deletions
diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc index f55b7e0db..5f9791a9b 100644 --- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc @@ -116,7 +116,7 @@ private: // Write the impl block associated with the rust representation of an enum. This includes methods // to write the enum to a protocol, read it from a protocol, etc. - void render_enum_impl(const string& enum_name); + void render_enum_impl(t_enum* tenum, const string& enum_name); // Write a simple rust const value (ie. `pub const FOO: foo...`). void render_const_value(const string& name, t_type* ttype, t_const_value* tvalue); @@ -868,49 +868,62 @@ void t_rs_generator::generate_typedef(t_typedef* ttypedef) { void t_rs_generator::generate_enum(t_enum* tenum) { string enum_name(rust_camel_case(tenum->get_name())); render_enum_definition(tenum, enum_name); - render_enum_impl(enum_name); + render_enum_impl(tenum, enum_name); render_enum_conversion(tenum, enum_name); } void t_rs_generator::render_enum_definition(t_enum* tenum, const string& enum_name) { render_rustdoc((t_doc*) tenum); f_gen_ << "#[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]" << endl; - f_gen_ << "pub enum " << enum_name << " {" << endl; - indent_up(); - - vector<t_enum_value*> constants = tenum->get_constants(); - vector<t_enum_value*>::iterator constants_iter; - for (constants_iter = constants.begin(); constants_iter != constants.end(); ++constants_iter) { - t_enum_value* val = (*constants_iter); - render_rustdoc((t_doc*) val); - f_gen_ - << indent() - << rust_enum_variant_name(val->get_name()) - << " = " - << val->get_value() - << "," - << endl; - } - - indent_down(); - f_gen_ << "}" << endl; + f_gen_ << "pub struct " << enum_name << "(pub i32);" << endl; f_gen_ << endl; } -void t_rs_generator::render_enum_impl(const string& enum_name) { +void t_rs_generator::render_enum_impl(t_enum* tenum, const string& enum_name) { f_gen_ << "impl " << enum_name << " {" << endl; indent_up(); - // taking enum as 'self' here because Thrift enums - // are represented as Rust enums with integer values - // it's cheaper to copy the integer as opposed to - // taking a reference to the enum + vector<t_enum_value*> constants = tenum->get_constants(); + + // associated constants for each IDL-defined enum variant + { + vector<t_enum_value*>::iterator constants_iter; + for (constants_iter = constants.begin(); constants_iter != constants.end(); ++constants_iter) { + t_enum_value* val = (*constants_iter); + render_rustdoc((t_doc*) val); + f_gen_ + << indent() + << "pub const " << rust_enum_variant_name(val->get_name()) << ": " << enum_name + << " = " + << enum_name << "(" << val->get_value() << ")" + << ";" + << endl; + } + } + + // array containing all IDL-defined enum variants + { + f_gen_ << indent() << "pub const ENUM_VALUES: &'static [Self] = &[" << endl; + indent_up(); + vector<t_enum_value*>::iterator constants_iter; + for (constants_iter = constants.begin(); constants_iter != constants.end(); ++constants_iter) { + t_enum_value* val = (*constants_iter); + f_gen_ + << indent() + << "Self::" << rust_enum_variant_name(val->get_name()) + << "," + << endl; + } + indent_down(); + f_gen_ << indent() << "];" << endl; + } + f_gen_ << indent() - << "pub fn write_to_out_protocol(self, o_prot: &mut dyn TOutputProtocol) -> thrift::Result<()> {" + << "pub fn write_to_out_protocol(&self, o_prot: &mut dyn TOutputProtocol) -> thrift::Result<()> {" << endl; indent_up(); - f_gen_ << indent() << "o_prot.write_i32(self as i32)" << endl; + f_gen_ << indent() << "o_prot.write_i32(self.0)" << endl; indent_down(); f_gen_ << indent() << "}" << endl; @@ -919,10 +932,8 @@ void t_rs_generator::render_enum_impl(const string& enum_name) { << "pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<" << enum_name << "> {" << endl; indent_up(); - f_gen_ << indent() << "let enum_value = i_prot.read_i32()?;" << endl; - f_gen_ << indent() << enum_name << "::try_from(enum_value)"; - + f_gen_ << indent() << "Ok(" << enum_name << "::from(enum_value)" << ")" << endl; indent_down(); f_gen_ << indent() << "}" << endl; @@ -932,17 +943,13 @@ void t_rs_generator::render_enum_impl(const string& enum_name) { } void t_rs_generator::render_enum_conversion(t_enum* tenum, const string& enum_name) { - f_gen_ << "impl TryFrom<i32> for " << enum_name << " {" << endl; + // From trait: i32 -> ENUM_TYPE + f_gen_ << "impl From<i32> for " << enum_name << " {" << endl; indent_up(); - - f_gen_ << indent() << "type Error = thrift::Error;"; - - f_gen_ << indent() << "fn try_from(i: i32) -> Result<Self, Self::Error> {" << endl; + f_gen_ << indent() << "fn from(i: i32) -> Self {" << endl; indent_up(); - f_gen_ << indent() << "match i {" << endl; indent_up(); - vector<t_enum_value*> constants = tenum->get_constants(); vector<t_enum_value*>::iterator constants_iter; for (constants_iter = constants.begin(); constants_iter != constants.end(); ++constants_iter) { @@ -950,26 +957,50 @@ void t_rs_generator::render_enum_conversion(t_enum* tenum, const string& enum_na f_gen_ << indent() << val->get_value() - << " => Ok(" << enum_name << "::" << rust_enum_variant_name(val->get_name()) << ")," + << " => " << enum_name << "::" << rust_enum_variant_name(val->get_name()) << "," << endl; } - f_gen_ << indent() << "_ => {" << endl; - indent_up(); - render_thrift_error( - "Protocol", - "ProtocolError", - "ProtocolErrorKind::InvalidData", - "format!(\"cannot convert enum constant {} to " + enum_name + "\", i)" - ); + f_gen_ << indent() << "_ => " << enum_name << "(i)" << endl; indent_down(); - f_gen_ << indent() << "}," << endl; + f_gen_ << indent() << "}" << endl; + indent_down(); + f_gen_ << indent() << "}" << endl; + indent_down(); + f_gen_ << "}" << endl; + f_gen_ << endl; + // From trait: &i32 -> ENUM_TYPE + f_gen_ << "impl From<&i32> for " << enum_name << " {" << endl; + indent_up(); + f_gen_ << indent() << "fn from(i: &i32) -> Self {" << endl; + indent_up(); + f_gen_ << indent() << enum_name << "::from(*i)" << endl; indent_down(); f_gen_ << indent() << "}" << endl; + indent_down(); + f_gen_ << "}" << endl; + f_gen_ << endl; + // From trait: ENUM_TYPE -> int + f_gen_ << "impl From<" << enum_name << "> for i32 {" << endl; + indent_up(); + f_gen_ << indent() << "fn from(e: " << enum_name << ") -> i32 {" << endl; + indent_up(); + f_gen_ << indent() << "e.0" << endl; indent_down(); f_gen_ << indent() << "}" << endl; + indent_down(); + f_gen_ << "}" << endl; + f_gen_ << endl; + // From trait: &ENUM_TYPE -> int + f_gen_ << "impl From<&" << enum_name << "> for i32 {" << endl; + indent_up(); + f_gen_ << indent() << "fn from(e: &" << enum_name << ") -> i32 {" << endl; + indent_up(); + f_gen_ << indent() << "e.0" << endl; + indent_down(); + f_gen_ << indent() << "}" << endl; indent_down(); f_gen_ << "}" << endl; f_gen_ << endl; @@ -3295,9 +3326,11 @@ string t_rs_generator::rust_enum_variant_name(const string &name) { } if (all_uppercase) { - return capitalize(camelcase(lowercase(name))); + return name; } else { - return capitalize(camelcase(name)); + string modified_name(uppercase(underscore(name))); + string_replace(modified_name, "__", "_"); + return modified_name; } } diff --git a/lib/rs/README.md b/lib/rs/README.md index 555d219d2..1e10e35e3 100644 --- a/lib/rs/README.md +++ b/lib/rs/README.md @@ -46,6 +46,51 @@ It does not currently use any Rust 2018 features. Breaking changes are minimized. When they are made they will be outlined below with transition guidelines. +##### Thrift 0.15.0 + +* **[THRIFT-5314]** - Generate enums implementations that are forward compatible (i.e. don't error on unknown values) + + As a result of this change the Rust representation of an enum changes from a standard + Rust enum into a newtype struct with associated constants. + + For example: + + ```thrift + // THRIFT + enum Operation { + ADD, + SUBTRACT, + MULTIPLY, + DIVIDE, + } + ``` + + used to generate: + + ```rust + // OLD AUTO-GENERATED RUST + pub enum Operation { + Add, + Subtract, + Multiply, + Divide, + } + ``` + + It *now* generates: + + ```rust + // NEW AUTO-GENERATED RUST + pub struct Operation(pub i32); + + impl Operation { + pub const ADD: Operation = Operation(0); + pub const SUBTRACT: Operation = Operation(1); + pub const MULTIPLY: Operation = Operation(2); + pub const DIVIDE: Operation = Operation(3); + } + ``` + ##### Thrift 0.14.0 * **[THRIFT-5158]** - Rust library and generator now support Rust 2018 only. Required rust 1.40.0 or higher @@ -91,7 +136,9 @@ Breaking changes are minimized. When they are made they will be outlined below w DIVIDE, } ``` + It *now* generates: + ```rust // NEW AUTO-GENERATED RUST pub enum Operation { diff --git a/lib/rs/test/src/bin/kitchen_sink_server.rs b/lib/rs/test/src/bin/kitchen_sink_server.rs index 6df39e4bc..8b910b3bf 100644 --- a/lib/rs/test/src/bin/kitchen_sink_server.rs +++ b/lib/rs/test/src/bin/kitchen_sink_server.rs @@ -209,12 +209,12 @@ struct FullHandler; impl FullMealAndDrinksServiceSyncHandler for FullHandler { fn handle_full_meal_and_drinks(&self) -> thrift::Result<FullMealAndDrinks> { println!("full_meal_and_drinks: handling full meal and drinks call"); - Ok(FullMealAndDrinks::new(full_meal(), Drink::CanadianWhisky)) + Ok(FullMealAndDrinks::new(full_meal(), Drink::CANADIAN_WHISKY)) } fn handle_best_pie(&self) -> thrift::Result<Pie> { println!("full_meal_and_drinks: handling pie call"); - Ok(Pie::MississippiMud) // I prefer Pie::Pumpkin, but I have to check that casing works + Ok(Pie::MISSISSIPPI_MUD) // I prefer Pie::Pumpkin, but I have to check that casing works } } @@ -259,7 +259,7 @@ fn noodle() -> Noodle { } fn ramen() -> Ramen { - Ramen::new("Mr Ramen".to_owned(), 72, BrothType::Miso) + Ramen::new("Mr Ramen".to_owned(), 72, BrothType::MISO) } fn napkin() -> Napkin { diff --git a/test/rs/src/bin/test_client.rs b/test/rs/src/bin/test_client.rs index 3e20999ed..476f9eb25 100644 --- a/test/rs/src/bin/test_client.rs +++ b/test/rs/src/bin/test_client.rs @@ -206,7 +206,7 @@ fn make_thrift_calls( info!("testEnum"); { - verify_expected_result(thrift_test_client.test_enum(Numberz::Two), Numberz::Two)?; + verify_expected_result(thrift_test_client.test_enum(Numberz::TWO), Numberz::TWO)?; } info!("testBinary"); @@ -381,7 +381,7 @@ fn make_thrift_calls( }; verify_expected_result( - thrift_test_client.test_multi(1, -123_948, -19_234_123_981, m_snd, Numberz::Eight, 81), + thrift_test_client.test_multi(1, -123_948, -19_234_123_981, m_snd, Numberz::EIGHT, 81), s_cmp, )?; } @@ -395,8 +395,8 @@ fn make_thrift_calls( // } { let mut arg_map_usermap: BTreeMap<Numberz, i64> = BTreeMap::new(); - arg_map_usermap.insert(Numberz::One, 4289); - arg_map_usermap.insert(Numberz::Eight, 19); + arg_map_usermap.insert(Numberz::ONE, 4289); + arg_map_usermap.insert(Numberz::EIGHT, 19); let mut arg_vec_xtructs: Vec<Xtruct> = Vec::new(); arg_vec_xtructs.push( @@ -429,15 +429,15 @@ fn make_thrift_calls( user_map: Some(arg_map_usermap), xtructs: Some(arg_vec_xtructs), }; - s_cmp_nested_1.insert(Numberz::Two, insanity.clone()); - s_cmp_nested_1.insert(Numberz::Three, insanity.clone()); + s_cmp_nested_1.insert(Numberz::TWO, insanity.clone()); + s_cmp_nested_1.insert(Numberz::THREE, insanity.clone()); let mut s_cmp_nested_2: BTreeMap<Numberz, Insanity> = BTreeMap::new(); let empty_insanity = Insanity { user_map: Some(BTreeMap::new()), xtructs: Some(Vec::new()), }; - s_cmp_nested_2.insert(Numberz::Six, empty_insanity); + s_cmp_nested_2.insert(Numberz::SIX, empty_insanity); let mut s_cmp: BTreeMap<UserId, BTreeMap<Numberz, Insanity>> = BTreeMap::new(); s_cmp.insert(1 as UserId, s_cmp_nested_1); diff --git a/test/rs/src/bin/test_server.rs b/test/rs/src/bin/test_server.rs index 74be12d48..c1f31758a 100644 --- a/test/rs/src/bin/test_server.rs +++ b/test/rs/src/bin/test_server.rs @@ -268,15 +268,15 @@ impl ThriftTestSyncHandler for ThriftTestSyncHandlerImpl { ) -> thrift::Result<BTreeMap<UserId, BTreeMap<Numberz, Insanity>>> { info!("testInsanity({:?})", argument); let mut map_0: BTreeMap<Numberz, Insanity> = BTreeMap::new(); - map_0.insert(Numberz::Two, argument.clone()); - map_0.insert(Numberz::Three, argument); + map_0.insert(Numberz::TWO, argument.clone()); + map_0.insert(Numberz::THREE, argument); let mut map_1: BTreeMap<Numberz, Insanity> = BTreeMap::new(); let insanity = Insanity { user_map: None, xtructs: None, }; - map_1.insert(Numberz::Six, insanity); + map_1.insert(Numberz::SIX, insanity); let mut ret: BTreeMap<UserId, BTreeMap<Numberz, Insanity>> = BTreeMap::new(); ret.insert(1, map_0); diff --git a/tutorial/rs/src/bin/tutorial_client.rs b/tutorial/rs/src/bin/tutorial_client.rs index f7de23f06..4bf2ec098 100644 --- a/tutorial/rs/src/bin/tutorial_client.rs +++ b/tutorial/rs/src/bin/tutorial_client.rs @@ -67,7 +67,7 @@ fn run() -> thrift::Result<()> { let logid = 32; // let's do...a multiply! - let res = client.calculate(logid, Work::new(7, 8, Operation::Multiply, None))?; + let res = client.calculate(logid, Work::new(7, 8, Operation::MULTIPLY, None))?; println!("multiplied 7 and 8 and got {}", res); // let's get the log for it @@ -77,7 +77,7 @@ fn run() -> thrift::Result<()> { // ok - let's be bad :( // do a divide by 0 // logid doesn't matter; won't be recorded - let res = client.calculate(77, Work::new(2, 0, Operation::Divide, "we bad".to_owned())); + let res = client.calculate(77, Work::new(2, 0, Operation::DIVIDE, "we bad".to_owned())); // we should have gotten an exception back match res { diff --git a/tutorial/rs/src/bin/tutorial_server.rs b/tutorial/rs/src/bin/tutorial_server.rs index fbccb69c9..b6ed7a123 100644 --- a/tutorial/rs/src/bin/tutorial_server.rs +++ b/tutorial/rs/src/bin/tutorial_server.rs @@ -123,7 +123,7 @@ impl CalculatorSyncHandler for CalculatorServer { let res = if let Some(ref op) = w.op { if w.num1.is_none() || w.num2.is_none() { Err(InvalidOperation { - what_op: Some(*op as i32), + what_op: Some(op.into()), why: Some("no operands specified".to_owned()), }) } else { @@ -131,19 +131,26 @@ impl CalculatorSyncHandler for CalculatorServer { let num1 = w.num1.as_ref().expect("operands checked"); let num2 = w.num2.as_ref().expect("operands checked"); - match *op { - Operation::Add => Ok(num1 + num2), - Operation::Subtract => Ok(num1 - num2), - Operation::Multiply => Ok(num1 * num2), - Operation::Divide => { + match op { + &Operation::ADD => Ok(num1 + num2), + &Operation::SUBTRACT => Ok(num1 - num2), + &Operation::MULTIPLY => Ok(num1 * num2), + &Operation::DIVIDE => { if *num2 == 0 { Err(InvalidOperation { - what_op: Some(*op as i32), + what_op: Some(op.into()), why: Some("divide by 0".to_owned()), }) } else { Ok(num1 / num2) } + }, + _ => { + let op_val: i32 = op.into(); + Err(InvalidOperation { + what_op: Some(op_val), + why: Some(format!("unsupported operation type '{}'", op_val)), + }) } } } |