Enable clippy in all Rust targets and ensure that all existing code is clippy-clean (#2341)

Client: rs

6 files changed, 24 insertions, 17 deletions

diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
index 5f9791a9b..f14dd6f8c 100644
--- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
@@ -18,7 +18,6 @@
  */
 
 #include <string>
-#include <fstream>
 #include <iostream>
 
 #include "thrift/platform.h"
@@ -409,10 +408,10 @@ private:
   bool is_double(t_type* ttype);
 
   // Return a string representing the rust type given a `t_type`.
-  string to_rust_type(t_type* ttype, bool ordered_float = true);
+  string to_rust_type(t_type* ttype);
 
   // Return a string representing the `const` rust type given a `t_type`
-  string to_rust_const_type(t_type* ttype, bool ordered_float = true);
+  string to_rust_const_type(t_type* ttype);
 
   // Return a string representing the rift `protocol::TType` given a `t_type`.
   string to_rust_field_type_enum(t_type* ttype);
@@ -547,9 +546,15 @@ void t_rs_generator::render_attributes_and_includes() {
   f_gen_ << "#![allow(unused_extern_crates)]" << endl;
   // constructors take *all* struct parameters, which can trigger the "too many arguments" warning
   // some auto-gen'd types can be deeply nested. clippy recommends factoring them out which is hard to autogen
-  f_gen_ << "#![allow(clippy::too_many_arguments, clippy::type_complexity)]" << endl;
+  // FIXME: re-enable the 'vec_box' lint see: [THRIFT-5364](https://issues.apache.org/jira/browse/THRIFT-5364)
+  // This can happen because we automatically generate a Vec<Box<Type>> when the type is a typedef
+  // and it's a forward typedef. This (typedef + forward typedef) can happen in two situations:
+  // 1. When the type is recursive
+  // 2. When you define types out of order
+  f_gen_ << "#![allow(clippy::too_many_arguments, clippy::type_complexity, clippy::vec_box)]" << endl;
   // prevent rustfmt from running against this file
   // lines are too long, code is (thankfully!) not visual-indented, etc.
+  // can't use #[rustfmt::skip] see: https://github.com/rust-lang/rust/issues/54726
   f_gen_ << "#![cfg_attr(rustfmt, rustfmt_skip)]" << endl;
   f_gen_ << endl;
 
@@ -918,6 +923,7 @@ void t_rs_generator::render_enum_impl(t_enum* tenum, const string& enum_name) {
     f_gen_ << indent() << "];" << endl;
   }
 
+  f_gen_ << indent() << "#[allow(clippy::trivially_copy_pass_by_ref)]" << endl;
   f_gen_
     << indent()
     << "pub fn write_to_out_protocol(&self, o_prot: &mut dyn TOutputProtocol) -> thrift::Result<()> {"
@@ -3032,7 +3038,7 @@ bool t_rs_generator::is_double(t_type* ttype) {
   return false;
 }
 
-string t_rs_generator::to_rust_type(t_type* ttype, bool ordered_float) {
+string t_rs_generator::to_rust_type(t_type* ttype) {
   // ttype = get_true_type(ttype); <-- recurses through as many typedef layers as necessary
   if (ttype->is_base_type()) {
     t_base_type* tbase_type = ((t_base_type*)ttype);
@@ -3056,11 +3062,7 @@ string t_rs_generator::to_rust_type(t_type* ttype, bool ordered_float) {
     case t_base_type::TYPE_I64:
       return "i64";
     case t_base_type::TYPE_DOUBLE:
-      if (ordered_float) {
-        return "OrderedFloat<f64>";
-      } else {
-        return "f64";
-      }
+      return "OrderedFloat<f64>";
     }
   } else if (ttype->is_typedef()) {
     t_typedef* ttypedef = (t_typedef*)ttype;
@@ -3085,7 +3087,7 @@ string t_rs_generator::to_rust_type(t_type* ttype, bool ordered_float) {
   throw "cannot find rust type for " + ttype->get_name();
 }
 
-string t_rs_generator::to_rust_const_type(t_type* ttype, bool ordered_float) {
+string t_rs_generator::to_rust_const_type(t_type* ttype) {
   if (ttype->is_base_type()) {
     t_base_type* tbase_type = ((t_base_type*)ttype);
     if (tbase_type->get_base() == t_base_type::TYPE_STRING) {
@@ -3097,7 +3099,7 @@ string t_rs_generator::to_rust_const_type(t_type* ttype, bool ordered_float) {
     }
   }
 
-  return to_rust_type(ttype, ordered_float);
+  return to_rust_type(ttype);
 }
 
 string t_rs_generator::to_rust_field_type_enum(t_type* ttype) {
diff --git a/lib/rs/Makefile.am b/lib/rs/Makefile.am
index dd1c03bfa..4abdff844 100644
--- a/lib/rs/Makefile.am
+++ b/lib/rs/Makefile.am
@@ -32,10 +32,12 @@ install:
 
 check-local:
 	$(CARGO) fmt --all -- --check
+	$(CARGO) clippy --all -- -D warnings
 	$(CARGO) test
 
 all-local:
 	$(CARGO) fmt --all -- --check
+	$(CARGO) clippy --all -- -D warnings
 	$(CARGO) build
 
 clean-local:
diff --git a/lib/rs/test/Makefile.am b/lib/rs/test/Makefile.am
index 19056a65f..017a2c467 100644
--- a/lib/rs/test/Makefile.am
+++ b/lib/rs/test/Makefile.am
@@ -29,6 +29,7 @@ stubs: thrifts/Base_One.thrift thrifts/Base_Two.thrift thrifts/Midlayer.thrift t
 
 check: stubs
 	$(CARGO) fmt --all -- --check
+	$(CARGO) clippy --all -- -D warnings
 	$(CARGO) build
 	$(CARGO) test
 	[ -d bin ] || mkdir bin
diff --git a/test/rs/Makefile.am b/test/rs/Makefile.am
index afb2cad03..78db5ee0c 100644
--- a/test/rs/Makefile.am
+++ b/test/rs/Makefile.am
@@ -23,6 +23,7 @@ stubs: ../ThriftTest.thrift
 precross: stubs
 	$(CARGO) build
 	$(CARGO) fmt --all -- --check
+	$(CARGO) clippy --all -- -D warnings
 	[ -d bin ] || mkdir bin
 	cp target/debug/test_server bin/test_server
 	cp target/debug/test_client bin/test_client
diff --git a/tutorial/rs/Makefile.am b/tutorial/rs/Makefile.am
index 4aa05dada..13f670794 100644
--- a/tutorial/rs/Makefile.am
+++ b/tutorial/rs/Makefile.am
@@ -25,6 +25,7 @@ gen-rs/tutorial.rs gen-rs/shared.rs: $(top_srcdir)/tutorial/tutorial.thrift
 all-local: gen-rs/tutorial.rs
 	$(CARGO) build
 	$(CARGO) fmt --all -- --check
+	$(CARGO) clippy --all -- -D warnings
 	[ -d bin ] || mkdir bin
 	cp target/debug/tutorial_server bin/tutorial_server
 	cp target/debug/tutorial_client bin/tutorial_client
diff --git a/tutorial/rs/src/bin/tutorial_server.rs b/tutorial/rs/src/bin/tutorial_server.rs
index ad16ab6dc..ab6df57fb 100644
--- a/tutorial/rs/src/bin/tutorial_server.rs
+++ b/tutorial/rs/src/bin/tutorial_server.rs
@@ -131,11 +131,11 @@ 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.into()),