Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Jul 4, 2024
7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ resolver = "2"
homepage = "https://github.com/mozilla/neqo/"
repository = "https://github.com/mozilla/neqo/"
authors = ["The Neqo Authors <necko@mozilla.com>"]
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"
Expand All @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
3 changes: 3 additions & 0 deletions neqo-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion neqo-bin/benches/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {
Expand All @@ -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
Expand Down
10 changes: 4 additions & 6 deletions neqo-bin/src/client/http09.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
14 changes: 7 additions & 7 deletions neqo-bin/src/client/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
}

Expand Down
4 changes: 3 additions & 1 deletion neqo-bin/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
}
Expand Down
27 changes: 10 additions & 17 deletions neqo-bin/src/server/http09.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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)]
larseggert marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand All @@ -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()
Expand Down
20 changes: 8 additions & 12 deletions neqo-bin/src/server/http3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
} => {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
}
Expand Down Expand Up @@ -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];
Expand All @@ -238,7 +234,7 @@ impl ResponseData {
}
}

fn done(&self) -> bool {
const fn done(&self) -> bool {
self.remaining == 0
}
}
4 changes: 3 additions & 1 deletion neqo-bin/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions neqo-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions neqo-common/src/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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())
}
}
Expand Down Expand Up @@ -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);
}

Expand Down
Loading
Loading