[ 
https://issues.apache.org/jira/browse/THRIFT-4529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16683766#comment-16683766
 ] 

ASF GitHub Bot commented on THRIFT-4529:
----------------------------------------

jeking3 closed pull request #1625: THRIFT-4529: Rust enum variants are now 
camel-cased
URL: https://github.com/apache/thrift/pull/1625
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/CHANGES b/CHANGES
index 9533ad08f3..c24c2b2223 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,8 @@
 Apache Thrift Changelog
 
 Breaking Changes since 0.11.0 [for 0.12.0]:
---------------------------------------------------------------------------------
 
+--------------------------------------------------------------------------------
+    * [THRIFT-4529] - Rust enum variants are now camel-cased instead of 
uppercased to conform to Rust naming conventions
     * [THRIFT-4448] - Support for golang 1.6 and earlier has been dropped.
     * [THRIFT-4474] - PHP now uses the PSR-4 loader by default instead of 
class maps.
     * [THRIFT-4532] - method signatures changed in the compiler's 
t_oop_generator.
diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc 
b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
index 5cd67f6755..ae30a35715 100644
--- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
@@ -499,6 +499,9 @@ class t_rs_generator : public t_generator {
   // the server half of a Thrift service.
   string rust_sync_processor_impl_name(t_service *tservice);
 
+  // Return the variant name for an enum variant
+  string rust_enum_variant_name(const string& name);
+
   // Properly uppercase names for use in Rust.
   string rust_upper_case(const string& name);
 
@@ -873,7 +876,7 @@ void t_rs_generator::render_enum_definition(t_enum* tenum, 
const string& enum_na
     render_rustdoc((t_doc*) val);
     f_gen_
       << indent()
-      << uppercase(val->get_name())
+      << rust_enum_variant_name(val->get_name())
       << " = "
       << val->get_value()
       << ","
@@ -934,7 +937,7 @@ void t_rs_generator::render_enum_conversion(t_enum* tenum, 
const string& enum_na
     f_gen_
       << indent()
       << val->get_value()
-      << " => Ok(" << enum_name << "::" << uppercase(val->get_name()) << "),"
+      << " => Ok(" << enum_name << "::" << 
rust_enum_variant_name(val->get_name()) << "),"
       << endl;
   }
   f_gen_ << indent() << "_ => {" << endl;
@@ -3254,6 +3257,23 @@ string 
t_rs_generator::rust_sync_processor_impl_name(t_service *tservice) {
   return "T" + rust_camel_case(tservice->get_name()) + "ProcessFunctions";
 }
 
+string t_rs_generator::rust_enum_variant_name(const string &name) {
+  bool all_uppercase = true;
+
+  for (size_t i = 0; i < name.size(); i++) {
+    if (isalnum(name[i]) && islower(name[i])) {
+      all_uppercase = false;
+      break;
+    }
+  }
+
+  if (all_uppercase) {
+    return capitalize(camelcase(lowercase(name)));
+  } else {
+    return capitalize(camelcase(name));
+  }
+}
+
 string t_rs_generator::rust_upper_case(const string& name) {
   string str(uppercase(underscore(name)));
   string_replace(str, "__", "_");
diff --git a/lib/rs/README.md b/lib/rs/README.md
index 8b35eda95c..7c37a10bc6 100644
--- a/lib/rs/README.md
+++ b/lib/rs/README.md
@@ -37,6 +37,57 @@ Thrift compiler you're using.
 
 Full [Rustdoc](https://docs.rs/thrift/)
 
+## Compatibility
+
+The Rust library and auto-generated code targets Rust versions 1.28+.
+It does not currently use any Rust 2018 features.
+
+### Breaking Changes
+
+Breaking changes are minimized. When they are made they will be outlined below 
with transition guidelines.
+
+##### Thrift 0.12.0
+
+* **[THRIFT-4529]** - Rust enum variants are now camel-cased instead of 
uppercased to conform to Rust naming conventions
+
+    Previously, enum variants were uppercased in the auto-generated code.
+    For example, the following thrift enum:
+
+    ```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 enum Operation {
+       Add,
+       Subtract,
+       Multiply,
+       Divide,
+     }
+    ```
+    
+    You will have to change all enum variants in your code to use camel-cased 
names.
+    This should be a search and replace.
+
 ## Contributing
 
 Bug reports and PRs are always welcome! Please see the
diff --git a/lib/rs/test/src/bin/kitchen_sink_server.rs 
b/lib/rs/test/src/bin/kitchen_sink_server.rs
index ae7026288c..e2c4a271f2 100644
--- a/lib/rs/test/src/bin/kitchen_sink_server.rs
+++ b/lib/rs/test/src/bin/kitchen_sink_server.rs
@@ -17,23 +17,23 @@
 
 #[macro_use]
 extern crate clap;
-
 extern crate kitchen_sink;
 extern crate thrift;
 
+use thrift::protocol::{TBinaryInputProtocolFactory, 
TBinaryOutputProtocolFactory,
+                       TCompactInputProtocolFactory, 
TCompactOutputProtocolFactory,
+                       TInputProtocolFactory, TOutputProtocolFactory};
+use thrift::server::TServer;
+use thrift::transport::{TFramedReadTransportFactory, 
TFramedWriteTransportFactory,
+                        TReadTransportFactory, TWriteTransportFactory};
+
 use kitchen_sink::base_one::Noodle;
 use kitchen_sink::base_two::{BrothType, Napkin, NapkinServiceSyncHandler, 
Ramen, RamenServiceSyncHandler};
-use kitchen_sink::midlayer::{Dessert, Meal, MealServiceSyncHandler, 
MealServiceSyncProcessor};
+use kitchen_sink::midlayer::{Dessert, Meal, MealServiceSyncHandler, 
MealServiceSyncProcessor, Pie};
 use kitchen_sink::recursive;
 use kitchen_sink::ultimate::{Drink, FullMeal, FullMealAndDrinks,
                              FullMealAndDrinksServiceSyncProcessor, 
FullMealServiceSyncHandler};
 use kitchen_sink::ultimate::FullMealAndDrinksServiceSyncHandler;
-use thrift::protocol::{TBinaryInputProtocolFactory, 
TBinaryOutputProtocolFactory,
-                       TCompactInputProtocolFactory, 
TCompactOutputProtocolFactory,
-                       TInputProtocolFactory, TOutputProtocolFactory};
-use thrift::transport::{TFramedReadTransportFactory, 
TFramedWriteTransportFactory,
-                        TReadTransportFactory, TWriteTransportFactory};
-use thrift::server::TServer;
 
 fn main() {
     match run() {
@@ -207,7 +207,13 @@ struct FullHandler;
 
 impl FullMealAndDrinksServiceSyncHandler for FullHandler {
     fn handle_full_meal_and_drinks(&self) -> thrift::Result<FullMealAndDrinks> 
{
-        Ok(FullMealAndDrinks::new(full_meal(), Drink::WHISKEY))
+        println!("full_meal_and_drinks: handling full meal and drinks call");
+        Ok(FullMealAndDrinks::new(full_meal(), Drink::CanadianWhisky))
+    }
+
+    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
     }
 }
 
@@ -252,7 +258,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/lib/rs/test/thrifts/Midlayer.thrift 
b/lib/rs/test/thrifts/Midlayer.thrift
index cf1157c7ea..16ff49b0e3 100644
--- a/lib/rs/test/thrifts/Midlayer.thrift
+++ b/lib/rs/test/thrifts/Midlayer.thrift
@@ -46,6 +46,15 @@ const set<set<i32>> MyConstNestedSet = [
   [6, 7, 8]
 ]
 
+enum Pie {
+  PUMPKIN,
+  apple, // intentionally poorly cased
+  STRAWBERRY_RHUBARB,
+  Key_Lime, // intentionally poorly cased
+  coconut_Cream, // intentionally poorly cased
+  mississippi_mud, // intentionally poorly cased
+}
+
 struct Meal {
   1: Base_One.Noodle noodle
   2: Base_Two.Ramen ramen
diff --git a/lib/rs/test/thrifts/Ultimate.thrift 
b/lib/rs/test/thrifts/Ultimate.thrift
index 8154d912fd..72fa100a6a 100644
--- a/lib/rs/test/thrifts/Ultimate.thrift
+++ b/lib/rs/test/thrifts/Ultimate.thrift
@@ -27,6 +27,21 @@ enum Drink {
   WATER,
   WHISKEY,
   WINE,
+  scotch, // intentionally poorly cased
+  LATE_HARVEST_WINE,
+  India_Pale_Ale, // intentionally poorly cased
+  apple_cider, // intentially poorly cased
+  belgian_Ale, // intentionally poorly cased
+  Canadian_whisky, // intentionally poorly cased
+}
+
+const map<i8, Midlayer.Pie> RankedPies = {
+  1: Midlayer.Pie.PUMPKIN,
+  2: Midlayer.Pie.STRAWBERRY_RHUBARB,
+  3: Midlayer.Pie.apple,
+  4: Midlayer.Pie.mississippi_mud,
+  5: Midlayer.Pie.coconut_Cream,
+  6: Midlayer.Pie.Key_Lime,
 }
 
 struct FullMeal {
@@ -45,5 +60,7 @@ service FullMealService extends Midlayer.MealService {
 
 service FullMealAndDrinksService extends FullMealService {
   FullMealAndDrinks fullMealAndDrinks()
+
+  Midlayer.Pie bestPie()
 }
 
diff --git a/test/rs/src/bin/test_client.rs b/test/rs/src/bin/test_client.rs
index 297faf9181..29b5b88e88 100644
--- a/test/rs/src/bin/test_client.rs
+++ b/test/rs/src/bin/test_client.rs
@@ -216,7 +216,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");
@@ -391,7 +391,7 @@ fn make_thrift_calls(
         };
 
         verify_expected_result(
-            thrift_test_client.test_multi(1, -123948, -19234123981, m_snd, 
Numberz::EIGHT, 81),
+            thrift_test_client.test_multi(1, -123948, -19234123981, m_snd, 
Numberz::Eight, 81),
             s_cmp,
         )?;
     }
@@ -405,8 +405,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(
@@ -439,15 +439,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 1976bf4abf..81c1ec7046 100644
--- a/test/rs/src/bin/test_server.rs
+++ b/test/rs/src/bin/test_server.rs
@@ -273,15 +273,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.clone());
+        map_0.insert(Numberz::Two, argument.clone());
+        map_0.insert(Numberz::Three, argument.clone());
 
         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 24ab4be060..e7192b6164 100644
--- a/tutorial/rs/src/bin/tutorial_client.rs
+++ b/tutorial/rs/src/bin/tutorial_client.rs
@@ -71,7 +71,7 @@ fn run() -> thrift::Result<()> {
 
     // let's do...a multiply!
     let res = client
-        .calculate(logid, Work::new(7, 8, Operation::MULTIPLY, None))?;
+        .calculate(logid, Work::new(7, 8, Operation::Multiply, None))?;
     println!("multiplied 7 and 8 and got {}", res);
 
     // let's get the log for it
@@ -81,7 +81,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 8db8eed26d..171c4ce314 100644
--- a/tutorial/rs/src/bin/tutorial_server.rs
+++ b/tutorial/rs/src/bin/tutorial_server.rs
@@ -134,10 +134,10 @@ impl CalculatorSyncHandler for CalculatorServer {
                 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 => {
+                    Operation::Add => Ok(num1 + num2),
+                    Operation::Subtract => Ok(num1 - num2),
+                    Operation::Multiply => Ok(num1 * num2),
+                    Operation::Divide => {
                         if *num2 == 0 {
                             Err(
                                 InvalidOperation {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Rust generation should include #![allow(non_snake_case)] or force conform to 
> Rust style guidelines
> --------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-4529
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4529
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Rust - Compiler
>    Affects Versions: 0.11.0
>            Reporter: Joshua
>            Assignee: Allen George
>            Priority: Minor
>
> Without this, building a project using a thrift file meant for multiple 
> languages may end up with many compiler warnings similar to the following:
> {code:sh}
> warning: variant `EXAMPLE_NAME` should have a camel case name such as 
> `ExampleName`
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to