chore: Turn on more clippy lints, and fix the warnings (#1956)

* chore: Turn on more clippy lints, and fix the warnings * Fixes * Fixes * Fixes * Update neqo-transport/src/addr_valid.rs Co-authored-by: Martin Thomson <[email protected]> Signed-off-by: Lars Eggert <[email protected]> * Suggestions from @martinthomson * More from @martinthomson * More from @martinthomson * And more * Fixes after rebase * Better Cubic fix * let _ -> _ * Add reason to `#[allow(clippy::mutable_key_type)]` * fmt --------- Signed-off-by: Lars Eggert <[email protected]> Co-authored-by: Martin Thomson <[email protected]>
mozilla · Jul 4, 2024 · c209c43 · c209c43
1 parent 34a5126
commit c209c43
Show file tree

Hide file tree

Showing 140 changed files with 1,399 additions and 1,450 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -16,6 +16,10 @@ resolver = "2"
 homepage = "https://github.com/mozilla/neqo/"
 repository = "https://github.com/mozilla/neqo/"
 authors = ["The Neqo Authors <[email protected]>"]
+description = "Neqo, the Mozilla implementation of QUIC in Rust."
+keywords = ["quic", "http3", "neqo", "mozilla", "ietf", "firefox"]
+categories = ["network-programming", "web-programming"]
+readme = "README.md"
 version = "0.7.9"
 # Keep in sync with `.rustfmt.toml` `edition`.
 edition = "2021"
@@ -30,7 +34,10 @@ log = { version = "0.4", default-features = false }
 qlog = { version = "0.13", default-features = false }
 
 [workspace.lints.clippy]
+cargo = { level = "warn", priority = -1 }
+nursery = { level = "warn", priority = -1 }
 pedantic = { level = "warn", priority = -1 }
+multiple_crate_versions = "allow"
 
 [profile.release]
 lto = "fat"

diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
@@ -7,6 +7,10 @@ version.workspace = true
 edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
+description.workspace = true
+keywords.workspace = true
+categories.workspace = true
+readme.workspace = true
 
 [package.metadata]
 cargo-fuzz = true

diff --git a/fuzz/fuzz_targets/frame.rs b/fuzz/fuzz_targets/frame.rs
@@ -10,7 +10,7 @@ fuzz_target!(|data: &[u8]| {
 
     // Run the fuzzer
     let mut decoder = Decoder::new(data);
-    let _ = Frame::decode(&mut decoder);
+    _ = Frame::decode(&mut decoder);
 });
 
 #[cfg(any(not(fuzzing), windows))]

diff --git a/fuzz/fuzz_targets/packet.rs b/fuzz/fuzz_targets/packet.rs
@@ -14,7 +14,7 @@ fuzz_target!(|data: &[u8]| {
     neqo_crypto::init().unwrap();
 
     // Run the fuzzer
-    let _ = PublicPacket::decode(data, decoder);
+    _ = PublicPacket::decode(data, decoder);
 });
 
 #[cfg(any(not(fuzzing), windows))]

diff --git a/neqo-bin/Cargo.toml b/neqo-bin/Cargo.toml
@@ -8,6 +8,9 @@ version.workspace = true
 edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+readme.workspace = true
 
 [[bin]]
 name = "neqo-client"

diff --git a/neqo-bin/benches/main.rs b/neqo-bin/benches/main.rs
@@ -68,6 +68,7 @@ fn transfer(c: &mut Criterion) {
     done_sender.send(()).unwrap();
 }
 
+#[allow(clippy::redundant_pub_crate)] // Bug in clippy nursery? Not sure how this lint could fire here.
 fn spawn_server() -> tokio::sync::oneshot::Sender<()> {
     let (done_sender, mut done_receiver) = tokio::sync::oneshot::channel();
     std::thread::spawn(move || {
@@ -76,7 +77,7 @@ fn spawn_server() -> tokio::sync::oneshot::Sender<()> {
             tokio::select! {
                 _ = &mut done_receiver => {}
                 res = &mut server  => panic!("expect server not to terminate: {res:?}"),
-            }
+            };
         });
     });
     done_sender

diff --git a/neqo-bin/src/client/http09.rs b/neqo-bin/src/client/http09.rs
@@ -106,7 +106,7 @@ impl<'a> super::Handler for Handler<'a> {
     }
 }
 
-pub(crate) fn create_client(
+pub fn create_client(
     args: &Args,
     local_addr: SocketAddr,
     remote_addr: SocketAddr,
@@ -147,11 +147,9 @@ impl TryFrom<&State> for CloseState {
 
     fn try_from(value: &State) -> Result<Self, Self::Error> {
         let (state, error) = match value {
-            State::Closing { error, .. } | State::Draining { error, .. } => {
-                (CloseState::Closing, error)
-            }
-            State::Closed(error) => (CloseState::Closed, error),
-            _ => return Ok(CloseState::NotClosing),
+            State::Closing { error, .. } | State::Draining { error, .. } => (Self::Closing, error),
+            State::Closed(error) => (Self::Closed, error),
+            _ => return Ok(Self::NotClosing),
         };
 
         if error.is_error() {

diff --git a/neqo-bin/src/client/http3.rs b/neqo-bin/src/client/http3.rs
@@ -29,7 +29,7 @@ use url::Url;
 
 use super::{get_output_file, qlog_new, Args, CloseState, Res};
 
-pub(crate) struct Handler<'a> {
+pub struct Handler<'a> {
     #[allow(
         unknown_lints,
         clippy::struct_field_names,
@@ -62,7 +62,7 @@ impl<'a> Handler<'a> {
     }
 }
 
-pub(crate) fn create_client(
+pub fn create_client(
     args: &Args,
     local_addr: SocketAddr,
     remote_addr: SocketAddr,
@@ -110,13 +110,13 @@ impl TryFrom<Http3State> for CloseState {
 
     fn try_from(value: Http3State) -> Result<Self, Self::Error> {
         let (state, error) = match value {
-            Http3State::Closing(error) => (CloseState::Closing, error),
-            Http3State::Closed(error) => (CloseState::Closed, error),
-            _ => return Ok(CloseState::NotClosing),
+            Http3State::Closing(error) => (Self::Closing, error),
+            Http3State::Closed(error) => (Self::Closed, error),
+            _ => return Ok(Self::NotClosing),
         };
 
         if error.is_error() {
-            Err(error.clone())
+            Err(error)
         } else {
             Ok(state)
         }
@@ -452,7 +452,7 @@ impl<'a> UrlHandler<'a> {
         }
     }
 
-    fn done(&mut self) -> bool {
+    fn done(&self) -> bool {
         self.stream_handlers.is_empty() && self.url_queue.is_empty()
     }
 

diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs
@@ -4,6 +4,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![allow(clippy::future_not_send)]
+
 use std::{
     collections::{HashMap, VecDeque},
     fmt::{self, Display},
@@ -332,7 +334,7 @@ async fn ready(
     let socket_ready = Box::pin(socket.readable()).map_ok(|()| Ready::Socket);
     let timeout_ready = timeout
         .as_mut()
-        .map_or(Either::Right(futures::future::pending()), Either::Left)
+        .map_or_else(|| Either::Right(futures::future::pending()), Either::Left)
         .map(|()| Ok(Ready::Timeout));
     select(socket_ready, timeout_ready).await.factor_first().0
 }

diff --git a/neqo-bin/src/server/http09.rs b/neqo-bin/src/server/http09.rs
@@ -75,12 +75,7 @@ impl HttpServer {
         })
     }
 
-    fn save_partial(
-        &mut self,
-        stream_id: StreamId,
-        partial: Vec<u8>,
-        conn: &mut ActiveConnectionRef,
-    ) {
+    fn save_partial(&mut self, stream_id: StreamId, partial: Vec<u8>, conn: &ActiveConnectionRef) {
         let url_dbg = String::from_utf8(partial.clone())
             .unwrap_or_else(|_| format!("<invalid UTF-8: {}>", hex(&partial)));
         if partial.len() < 4096 {
@@ -92,12 +87,7 @@ impl HttpServer {
         }
     }
 
-    fn write(
-        &mut self,
-        stream_id: StreamId,
-        data: Option<Vec<u8>>,
-        conn: &mut ActiveConnectionRef,
-    ) {
+    fn write(&mut self, stream_id: StreamId, data: Option<Vec<u8>>, conn: &ActiveConnectionRef) {
         let resp = data.unwrap_or_else(|| Vec::from(&b"404 That request was nonsense\r\n"[..]));
         if let Some(stream_state) = self.write_state.get_mut(&stream_id) {
             match stream_state.data_to_send {
@@ -120,7 +110,7 @@ impl HttpServer {
         }
     }
 
-    fn stream_readable(&mut self, stream_id: StreamId, conn: &mut ActiveConnectionRef) {
+    fn stream_readable(&mut self, stream_id: StreamId, conn: &ActiveConnectionRef) {
         if !stream_id.is_client_initiated() || !stream_id.is_bidi() {
             qdebug!("Stream {} not client-initiated bidi, ignoring", stream_id);
             return;
@@ -176,7 +166,7 @@ impl HttpServer {
         self.write(stream_id, resp, conn);
     }
 
-    fn stream_writable(&mut self, stream_id: StreamId, conn: &mut ActiveConnectionRef) {
+    fn stream_writable(&mut self, stream_id: StreamId, conn: &ActiveConnectionRef) {
         match self.write_state.get_mut(&stream_id) {
             None => {
                 qwarn!("Unknown stream {stream_id}, ignoring event");
@@ -209,8 +199,11 @@ impl super::HttpServer for HttpServer {
     }
 
     fn process_events(&mut self, now: Instant) {
+        // `ActiveConnectionRef` `Hash` implementation doesn’t access any of the interior mutable
+        // types.
+        #[allow(clippy::mutable_key_type)]
         let active_conns = self.server.active_connections();
-        for mut acr in active_conns {
+        for acr in active_conns {
             loop {
                 let event = match acr.borrow_mut().next_event() {
                     None => break,
@@ -222,10 +215,10 @@ impl super::HttpServer for HttpServer {
                             .insert(stream_id, HttpStreamState::default());
                     }
                     ConnectionEvent::RecvStreamReadable { stream_id } => {
-                        self.stream_readable(stream_id, &mut acr);
+                        self.stream_readable(stream_id, &acr);
                     }
                     ConnectionEvent::SendStreamWritable { stream_id } => {
-                        self.stream_writable(stream_id, &mut acr);
+                        self.stream_writable(stream_id, &acr);
                     }
                     ConnectionEvent::StateChange(State::Connected) => {
                         acr.connection()

diff --git a/neqo-bin/src/server/http3.rs b/neqo-bin/src/server/http3.rs
@@ -91,7 +91,7 @@ impl super::HttpServer for HttpServer {
         while let Some(event) = self.server.next_event() {
             match event {
                 Http3ServerEvent::Headers {
-                    mut stream,
+                    stream,
                     headers,
                     fin,
                 } => {
@@ -138,17 +138,17 @@ impl super::HttpServer for HttpServer {
                             Header::new("content-length", response.remaining.to_string()),
                         ])
                         .unwrap();
-                    response.send(&mut stream);
+                    response.send(&stream);
                     if response.done() {
                         stream.stream_close_send().unwrap();
                     } else {
                         self.remaining_data.insert(stream.stream_id(), response);
                     }
                 }
-                Http3ServerEvent::DataWritable { mut stream } => {
+                Http3ServerEvent::DataWritable { stream } => {
                     if self.posts.get_mut(&stream).is_none() {
                         if let Some(remaining) = self.remaining_data.get_mut(&stream.stream_id()) {
-                            remaining.send(&mut stream);
+                            remaining.send(&stream);
                             if remaining.done() {
                                 self.remaining_data.remove(&stream.stream_id());
                                 stream.stream_close_send().unwrap();
@@ -157,11 +157,7 @@ impl super::HttpServer for HttpServer {
                     }
                 }
 
-                Http3ServerEvent::Data {
-                    mut stream,
-                    data,
-                    fin,
-                } => {
+                Http3ServerEvent::Data { stream, data, fin } => {
                     if let Some(received) = self.posts.get_mut(&stream) {
                         *received += data.len();
                     }
@@ -210,15 +206,15 @@ impl From<Vec<u8>> for ResponseData {
 }
 
 impl ResponseData {
-    fn repeat(buf: &'static [u8], total: usize) -> Self {
+    const fn repeat(buf: &'static [u8], total: usize) -> Self {
         Self {
             data: Cow::Borrowed(buf),
             offset: 0,
             remaining: total,
         }
     }
 
-    fn send(&mut self, stream: &mut Http3OrWebTransportStream) {
+    fn send(&mut self, stream: &Http3OrWebTransportStream) {
         while self.remaining > 0 {
             let end = min(self.data.len(), self.offset + self.remaining);
             let slice = &self.data[self.offset..end];
@@ -238,7 +234,7 @@ impl ResponseData {
         }
     }
 
-    fn done(&self) -> bool {
+    const fn done(&self) -> bool {
         self.remaining == 0
     }
 }
diff --git a/neqo-bin/src/server/mod.rs b/neqo-bin/src/server/mod.rs
@@ -4,6 +4,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#![allow(clippy::future_not_send)]
+
 use std::{
     cell::RefCell,
     fmt::{self, Display},
@@ -261,7 +263,7 @@ impl ServerRunner {
         let timeout_ready = self
             .timeout
             .as_mut()
-            .map_or(Either::Right(futures::future::pending()), Either::Left)
+            .map_or_else(|| Either::Right(futures::future::pending()), Either::Left)
             .map(|()| Ok(Ready::Timeout));
         select(sockets_ready, timeout_ready).await.factor_first().0
     }

diff --git a/neqo-common/Cargo.toml b/neqo-common/Cargo.toml
@@ -7,6 +7,10 @@ version.workspace = true
 edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
+description.workspace = true
+keywords.workspace = true
+categories.workspace = true
+readme.workspace = true
 
 [lints]
 workspace = true

diff --git a/neqo-common/src/codec.rs b/neqo-common/src/codec.rs
@@ -17,19 +17,19 @@ pub struct Decoder<'a> {
 impl<'a> Decoder<'a> {
     /// Make a new view of the provided slice.
     #[must_use]
-    pub fn new(buf: &[u8]) -> Decoder {
+    pub const fn new(buf: &[u8]) -> Decoder {
         Decoder { buf, offset: 0 }
     }
 
     /// Get the number of bytes remaining until the end.
     #[must_use]
-    pub fn remaining(&self) -> usize {
+    pub const fn remaining(&self) -> usize {
         self.buf.len() - self.offset
     }
 
     /// The number of bytes from the underlying slice that have been decoded.
     #[must_use]
-    pub fn offset(&self) -> usize {
+    pub const fn offset(&self) -> usize {
         self.offset
     }
 
@@ -73,7 +73,8 @@ impl<'a> Decoder<'a> {
     }
 
     /// Provides the next byte without moving the read position.
-    pub fn peek_byte(&mut self) -> Option<u8> {
+    #[must_use]
+    pub const fn peek_byte(&self) -> Option<u8> {
         if self.remaining() < 1 {
             None
         } else {
@@ -170,7 +171,7 @@ impl<'a> Debug for Decoder<'a> {
 
 impl<'a> From<&'a [u8]> for Decoder<'a> {
     #[must_use]
-    fn from(buf: &'a [u8]) -> Decoder<'a> {
+    fn from(buf: &'a [u8]) -> Self {
         Decoder::new(buf)
     }
 }
@@ -180,7 +181,7 @@ where
     T: AsRef<[u8]>,
 {
     #[must_use]
-    fn from(buf: &'a T) -> Decoder<'a> {
+    fn from(buf: &'a T) -> Self {
         Decoder::new(buf.as_ref())
     }
 }
@@ -632,7 +633,7 @@ mod tests {
 
     #[test]
     #[should_panic(expected = "Varint value too large")]
-    fn encoded_length_oob() {
+    const fn encoded_length_oob() {
         _ = Encoder::varint_len(1 << 62);
     }