From 8fd0c8c026ea2cb263e3ace96ad742af3c073b37 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Wed, 29 Nov 2023 06:06:15 +0000 Subject: [PATCH 1/2] add clippy linting CI target --- .github/workflows/ci.yml | 17 ++++ Cargo.toml | 4 +- async-byte-channel/Cargo.toml | 2 + capnp-futures/Cargo.toml | 3 + capnp-rpc/Cargo.toml | 6 ++ capnp-rpc/src/rpc.rs | 122 +++++++++++++------------- capnp-rpc/test/test.rs | 6 +- capnp/Cargo.toml | 7 ++ capnp/src/private/layout.rs | 1 + capnpc/Cargo.toml | 3 + capnpc/test/dynamic.rs | 1 + example/addressbook/Cargo.toml | 3 + example/addressbook_send/Cargo.toml | 3 + example/fill_random_values/Cargo.toml | 5 +- example/wasm-hello-world/Cargo.toml | 3 + 15 files changed, 120 insertions(+), 66 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d7dede82..dbd270b3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,3 +137,20 @@ jobs: with: components: rustfmt - run: cargo fmt --all -- --check --unstable-features --error-on-unformatted + + clippy: + name: lint + runs-on: ubuntu-latest + steps: + - name: Install Cap'n Proto + run: | + export DEBIAN_FRONTEND=noninteractive + sudo apt-get install -y capnproto libcapnp-dev + + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@nightly + with: + components: clippy + - uses: actions-rs-plus/clippy-check@v2 + with: + args: --all --all-targets diff --git a/Cargo.toml b/Cargo.toml index aead52e9f..a2856900b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,4 +32,6 @@ members = [ default-members = [ "capnp", "capnpc", -] \ No newline at end of file +] + +[workspace.lints] diff --git a/async-byte-channel/Cargo.toml b/async-byte-channel/Cargo.toml index 892998943..262e1d707 100644 --- a/async-byte-channel/Cargo.toml +++ b/async-byte-channel/Cargo.toml @@ -12,3 +12,5 @@ version = "0.3.0" default-features = false features = ["std", "executor"] +[lints] +workspace = true diff --git a/capnp-futures/Cargo.toml b/capnp-futures/Cargo.toml index 0e95b5c1b..ebdb79b15 100644 --- a/capnp-futures/Cargo.toml +++ b/capnp-futures/Cargo.toml @@ -27,3 +27,6 @@ features = ["executor"] [dev-dependencies] capnp = { version = "0.18.0", path = "../capnp", features = ["quickcheck"] } quickcheck = "1" + +[lints] +workspace = true diff --git a/capnp-rpc/Cargo.toml b/capnp-rpc/Cargo.toml index c32b0c8e7..7a01570af 100644 --- a/capnp-rpc/Cargo.toml +++ b/capnp-rpc/Cargo.toml @@ -21,3 +21,9 @@ features = ["std"] [dependencies] capnp-futures = { version = "0.18.0", path = "../capnp-futures" } capnp = {version = "0.18.0", path = "../capnp"} + +#[lints] +#workspace = true + +[lints.clippy] +type_complexity = "allow" # this should be removed in future diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 18e3a685e..729d2395b 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -682,7 +682,7 @@ impl ConnectionState { fn send_unimplemented( connection_state: &Rc, - message: &Box, + message: &dyn crate::IncomingMessage, ) -> capnp::Result<()> { let mut out_message = connection_state.new_outgoing_message(50)?; // XXX size hint { @@ -1031,61 +1031,65 @@ impl ConnectionState { Some(ref mut question) => { question.is_awaiting_return = false; match question.self_ref { - Some(ref question_ref) => { - match ret.which()? { - return_::Results(results) => { - let cap_table = Self::receive_caps( - &connection_state, - results?.get_cap_table()?, - )?; - - let question_ref = - question_ref.upgrade().expect("dangling question ref?"); - let response = Response::new( - connection_state.clone(), - question_ref.clone(), - message, - cap_table, - ); - question_ref.borrow_mut().fulfill(Promise::ok(response)); - } - return_::Exception(e) => { - let tmp = - question_ref.upgrade().expect("dangling question ref?"); - tmp.borrow_mut().reject(remote_exception_to_error(e?)); - } - return_::Canceled(_) => { - Self::send_unimplemented(&connection_state, &message)?; - } - return_::ResultsSentElsewhere(_) => { - Self::send_unimplemented(&connection_state, &message)?; - } - return_::TakeFromOtherQuestion(id) => { - if let Some(answer) = - connection_state.answers.borrow_mut().slots.get_mut(&id) - { - if let Some(res) = answer.redirected_results.take() { - let tmp = question_ref - .upgrade() - .expect("dangling question ref?"); - tmp.borrow_mut().fulfill(res); - } else { - return Err(Error::failed("return.takeFromOtherQuestion referenced a call that \ - did not use sendResultsTo.yourself.".to_string())); - } + Some(ref question_ref) => match ret.which()? { + return_::Results(results) => { + let cap_table = Self::receive_caps( + &connection_state, + results?.get_cap_table()?, + )?; + + let question_ref = + question_ref.upgrade().expect("dangling question ref?"); + let response = Response::new( + connection_state.clone(), + question_ref.clone(), + message, + cap_table, + ); + question_ref.borrow_mut().fulfill(Promise::ok(response)); + } + return_::Exception(e) => { + let tmp = + question_ref.upgrade().expect("dangling question ref?"); + tmp.borrow_mut().reject(remote_exception_to_error(e?)); + } + return_::Canceled(_) => { + Self::send_unimplemented(&connection_state, message.as_ref())?; + } + return_::ResultsSentElsewhere(_) => { + Self::send_unimplemented(&connection_state, message.as_ref())?; + } + return_::TakeFromOtherQuestion(id) => { + if let Some(answer) = + connection_state.answers.borrow_mut().slots.get_mut(&id) + { + if let Some(res) = answer.redirected_results.take() { + let tmp = question_ref + .upgrade() + .expect("dangling question ref?"); + tmp.borrow_mut().fulfill(res); } else { - return Err(Error::failed("return.takeFromOtherQuestion had invalid answer ID.".to_string())); + return Err(Error::failed("return.takeFromOtherQuestion referenced a call that \ + did not use sendResultsTo.yourself.".to_string())); } - } - return_::AcceptFromThirdParty(_) => { - drop(questions); - Self::send_unimplemented(&connection_state, &message)?; + } else { + return Err(Error::failed( + "return.takeFromOtherQuestion had invalid answer ID." + .to_string(), + )); } } - } + return_::AcceptFromThirdParty(_) => { + drop(questions); + Self::send_unimplemented(&connection_state, message.as_ref())?; + } + }, None => { if let return_::TakeFromOtherQuestion(_) = ret.which()? { - return Self::send_unimplemented(&connection_state, &message); + return Self::send_unimplemented( + &connection_state, + message.as_ref(), + ); } // Looks like this question was canceled earlier, so `Finish` // was already sent, with `releaseResultCaps` set true so that @@ -1155,7 +1159,7 @@ impl ConnectionState { | message::ObsoleteDelete(_), ) | Err(::capnp::NotInSchema(_)) => { - Self::send_unimplemented(&connection_state, &message)?; + Self::send_unimplemented(&connection_state, message.as_ref())?; } } Ok(()) @@ -1283,8 +1287,7 @@ impl ConnectionState { } } - fn get_innermost_client(&self, client_ref: &Box) -> Box { - let mut client = client_ref.clone(); + fn get_innermost_client(&self, mut client: Box) -> Box { while let Some(inner) = client.get_resolved() { client = inner; } @@ -1314,7 +1317,7 @@ impl ConnectionState { match resolution_result { Ok(resolution) => { - let resolution = connection_state.get_innermost_client(&resolution); + let resolution = connection_state.get_innermost_client(resolution.clone()); let brand = resolution.get_brand(); @@ -1353,7 +1356,7 @@ impl ConnectionState { resolve.set_promise_id(export_id); let _export = Self::write_descriptor( &connection_state, - &resolution, + resolution, resolve.init_cap(), )?; } @@ -1378,11 +1381,10 @@ impl ConnectionState { fn write_descriptor( state: &Rc, - cap: &Box, + mut inner: Box, mut descriptor: cap_descriptor::Builder, ) -> ::capnp::Result> { // Find the innermost wrapped capability. - let mut inner = cap.clone(); while let Some(resolved) = inner.get_resolved() { inner = resolved; } @@ -1439,10 +1441,10 @@ impl ConnectionState { let mut exports = Vec::new(); for (idx, value) in cap_table.iter().enumerate() { match value { - Some(ref cap) => { + Some(cap) => { if let Some(export_id) = Self::write_descriptor( state, - cap, + cap.clone(), cap_table_builder.reborrow().get(idx as u32), ) .unwrap() @@ -2972,7 +2974,7 @@ impl Client { ConnectionState::write_descriptor( &self.connection_state.clone(), - &promise_client.borrow().cap.clone(), + promise_client.borrow().cap.clone(), descriptor, ) .unwrap() diff --git a/capnp-rpc/test/test.rs b/capnp-rpc/test/test.rs index f0d6268bd..37e822b4b 100644 --- a/capnp-rpc/test/test.rs +++ b/capnp-rpc/test/test.rs @@ -736,12 +736,10 @@ fn embargo_success() { call5.promise, ]) .map(|responses| { - let mut counter = 0; - for r in responses?.into_iter() { - if counter != r.get()?.get_n() { + for (counter, r) in responses?.into_iter().enumerate() { + if counter != r.get()?.get_n() as usize { return Err(Error::failed("calls arrived out of order".to_string())); } - counter += 1; } Ok(()) }) diff --git a/capnp/Cargo.toml b/capnp/Cargo.toml index 20c8c2da1..1643b10ad 100644 --- a/capnp/Cargo.toml +++ b/capnp/Cargo.toml @@ -40,3 +40,10 @@ std = ["embedded-io?/std"] # message readers to be `Sync`. Note that AtomicUsize is not supported by all # rustc targets. sync_reader = [] + +#[lints] +#workspace = true + +[lints.clippy] +type_complexity = "allow" # this should be removed in future +missing_safety_doc = "allow" # this should be removed in future diff --git a/capnp/src/private/layout.rs b/capnp/src/private/layout.rs index 985d58f0c..cf2793b5a 100644 --- a/capnp/src/private/layout.rs +++ b/capnp/src/private/layout.rs @@ -2146,6 +2146,7 @@ mod wire_helpers { } } + #[allow(clippy::too_many_arguments)] pub unsafe fn copy_pointer( dst_arena: &mut dyn BuilderArena, dst_segment_id: u32, diff --git a/capnpc/Cargo.toml b/capnpc/Cargo.toml index f2f915158..6d677d2d7 100644 --- a/capnpc/Cargo.toml +++ b/capnpc/Cargo.toml @@ -27,3 +27,6 @@ path = "src/capnpc-rust-bootstrap.rs" [dependencies.capnp] version = "0.18.0" path = "../capnp" + +[lints] +workspace = true diff --git a/capnpc/test/dynamic.rs b/capnpc/test/dynamic.rs index 60060b858..c2b84b1a7 100644 --- a/capnpc/test/dynamic.rs +++ b/capnpc/test/dynamic.rs @@ -156,6 +156,7 @@ fn test_generics() { let root: dynamic_value::Builder<'_> = root.into(); let mut root: dynamic_struct::Builder<'_> = root.downcast(); + #[allow(clippy::disallowed_names)] let foo = root.reborrow().get_named("foo").unwrap(); test_util::dynamic_init_test_message(foo.downcast()); diff --git a/example/addressbook/Cargo.toml b/example/addressbook/Cargo.toml index 00683b01a..42f75c252 100644 --- a/example/addressbook/Cargo.toml +++ b/example/addressbook/Cargo.toml @@ -16,3 +16,6 @@ path = "../../capnpc" [dependencies.capnp] path = "../../capnp" + +[lints] +workspace = true diff --git a/example/addressbook_send/Cargo.toml b/example/addressbook_send/Cargo.toml index bbb102cbe..ae4e07732 100644 --- a/example/addressbook_send/Cargo.toml +++ b/example/addressbook_send/Cargo.toml @@ -17,3 +17,6 @@ capnpc = { path = "../../capnpc" } [dependencies] capnp = { path = "../../capnp" } + +[lints] +workspace = true diff --git a/example/fill_random_values/Cargo.toml b/example/fill_random_values/Cargo.toml index 9915eef96..e968b4d74 100644 --- a/example/fill_random_values/Cargo.toml +++ b/example/fill_random_values/Cargo.toml @@ -25,4 +25,7 @@ path = "fill_addressbook.rs" [[bin]] name = "fill_shapes" -path = "fill_shapes.rs" \ No newline at end of file +path = "fill_shapes.rs" + +[lints] +workspace = true diff --git a/example/wasm-hello-world/Cargo.toml b/example/wasm-hello-world/Cargo.toml index 89cabe1aa..2b99fc9b6 100644 --- a/example/wasm-hello-world/Cargo.toml +++ b/example/wasm-hello-world/Cargo.toml @@ -16,3 +16,6 @@ path = "../../capnp" [build-dependencies.capnpc] path = "../../capnpc" + +[lints] +workspace = true From ac9e802ae50fdf1c6957ea75d5893950b7e7219a Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 3 Dec 2023 08:35:25 +0000 Subject: [PATCH 2/2] pin clippy version --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dbd270b3b..ddad4af65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,8 +148,9 @@ jobs: sudo apt-get install -y capnproto libcapnp-dev - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@nightly + - uses: dtolnay/rust-toolchain@master with: + toolchain: nightly-2023-11-30 components: clippy - uses: actions-rs-plus/clippy-check@v2 with: