You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by al...@apache.org on 2021/03/06 19:12:08 UTC
[thrift] branch master updated: Enable clippy in all Rust targets
and ensure that all existing code is clippy-clean (#2341)
This is an automated email from the ASF dual-hosted git repository.
allengeorge pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push:
new 99c3aa2 Enable clippy in all Rust targets and ensure that all existing code is clippy-clean (#2341)
99c3aa2 is described below
commit 99c3aa27e6f6daa062b905a65495315c0c2ded90
Author: Allen George <al...@apache.org>
AuthorDate: Sat Mar 6 14:11:56 2021 -0500
Enable clippy in all Rust targets and ensure that all existing code is clippy-clean (#2341)
Client: rs
---
compiler/cpp/src/thrift/generate/t_rs_generator.cc | 26 ++++++++++++----------
lib/rs/Makefile.am | 2 ++
lib/rs/test/Makefile.am | 1 +
test/rs/Makefile.am | 1 +
tutorial/rs/Makefile.am | 1 +
tutorial/rs/src/bin/tutorial_server.rs | 10 ++++-----
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 5f9791a..f14dd6f 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 dd1c03b..4abdff8 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 19056a6..017a2c4 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 afb2cad..78db5ee 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 4aa05da..13f6707 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 ad16ab6..ab6df57 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()),