add clippy linting CI target (#458)

* add clippy linting CI target * pin clippy version
capnproto · Dec 3, 2023 · eaad5e5 · eaad5e5
1 parent cc223f5
commit eaad5e5
Show file tree

Hide file tree

Showing 15 changed files with 121 additions and 66 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -137,3 +137,21 @@ 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@master
+        with:
+          toolchain: nightly-2023-11-30
+          components: clippy
+      - uses: actions-rs-plus/clippy-check@v2
+        with:
+          args: --all --all-targets
diff --git a/Cargo.toml b/Cargo.toml
@@ -32,4 +32,6 @@ members = [
 default-members = [
     "capnp",
     "capnpc",
-]
+]
+
+[workspace.lints]
diff --git 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
@@ -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
@@ -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
@@ -682,7 +682,7 @@ impl<VatId> ConnectionState<VatId> {
 
     fn send_unimplemented(
         connection_state: &Rc<Self>,
-        message: &Box<dyn crate::IncomingMessage>,
+        message: &dyn crate::IncomingMessage,
     ) -> capnp::Result<()> {
         let mut out_message = connection_state.new_outgoing_message(50)?; // XXX size hint
         {
@@ -1031,61 +1031,65 @@ impl<VatId> ConnectionState<VatId> {
                     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<VatId> ConnectionState<VatId> {
                 | 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<VatId> ConnectionState<VatId> {
         }
     }
 
-    fn get_innermost_client(&self, client_ref: &Box<dyn ClientHook>) -> Box<dyn ClientHook> {
-        let mut client = client_ref.clone();
+    fn get_innermost_client(&self, mut client: Box<dyn ClientHook>) -> Box<dyn ClientHook> {
         while let Some(inner) = client.get_resolved() {
             client = inner;
         }
@@ -1314,7 +1317,7 @@ impl<VatId> ConnectionState<VatId> {
 
             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<VatId> ConnectionState<VatId> {
                         resolve.set_promise_id(export_id);
                         let _export = Self::write_descriptor(
                             &connection_state,
-                            &resolution,
+                            resolution,
                             resolve.init_cap(),
                         )?;
                     }
@@ -1378,11 +1381,10 @@ impl<VatId> ConnectionState<VatId> {
 
     fn write_descriptor(
         state: &Rc<Self>,
-        cap: &Box<dyn ClientHook>,
+        mut inner: Box<dyn ClientHook>,
         mut descriptor: cap_descriptor::Builder,
     ) -> ::capnp::Result<Option<ExportId>> {
         // Find the innermost wrapped capability.
-        let mut inner = cap.clone();
         while let Some(resolved) = inner.get_resolved() {
             inner = resolved;
         }
@@ -1439,10 +1441,10 @@ impl<VatId> ConnectionState<VatId> {
         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<VatId> Client<VatId> {
 
                 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -25,4 +25,7 @@ path = "fill_addressbook.rs"
 
 [[bin]]
 name = "fill_shapes"
-path = "fill_shapes.rs"
+path = "fill_shapes.rs"
+
+[lints]
+workspace = true
diff --git 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