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

enable clippy for tower-http and fix current issues #407

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@ jobs:
tool: [email protected]
- run: cargo fmt --all --check

clippy:
needs: check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
with:
components: clippy
- name: Install protoc
uses: taiki-e/install-action@v2
with:
tool: [email protected]
- run: cargo clippy --workspace --all-features --all-targets

deny-check:
name: cargo-deny check
runs-on: ubuntu-latest
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ members = [
"tower-http",
"examples/*",
]
resolver = "2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing final newline. Also an unrelated change, so may be worth a separate commit (though I think David always squash-merges anyways..)

Copy link
Contributor

@Oliboy50 Oliboy50 Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR should be rebased

at least because resolver = "2" has been added to Cargo.toml meanwhile

so it would be easier to review for David when he will come here...

Probably, but the bigger issue is first to ensure maintainers want to merge this in to begin with

if he comes here, indeed

2 changes: 1 addition & 1 deletion tower-http/src/classify/grpc_errors_as_failures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl fmt::Display for GrpcFailureClass {
}
}

#[allow(clippy::if_let_some_result)]
#[allow(clippy::match_result_ok)]
pub(crate) fn classify_grpc_metadata(
headers: &HeaderMap,
success_codes: GrpcCodeBitmask,
Expand Down
1 change: 1 addition & 0 deletions tower-http/src/compression/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ where
CompressionBody::new(BodyInner::zstd(WrapBody::new(body, self.quality)))
}
#[cfg(feature = "fs")]
#[allow(unreachable_patterns)]
(true, _) => {
// This should never happen because the `AcceptEncoding` struct which is used to determine
// `self.encoding` will only enable the different compression algorithms if the
Expand Down
2 changes: 1 addition & 1 deletion tower-http/src/compression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ mod tests {
let mut guard = self.0.write().unwrap();
let should_compress = *guard % 2 != 0;
*guard += 1;
dbg!(should_compress)
should_compress
}
}

Expand Down
64 changes: 32 additions & 32 deletions tower-http/src/content_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl QValue {
let mut c = s.chars();
// Parse "q=" (case-insensitively).
match c.next() {
Some('q') | Some('Q') => (),
Some('q' | 'Q') => (),
_ => return None,
};
match c.next() {
Expand Down Expand Up @@ -272,7 +272,7 @@ mod tests {
#[test]
fn no_accept_encoding_header() {
let encoding =
Encoding::from_headers(&http::HeaderMap::new(), SupportedEncodingsAll::default());
Encoding::from_headers(&http::HeaderMap::new(), SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);
}

Expand All @@ -283,7 +283,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Gzip, encoding);
}

Expand All @@ -294,7 +294,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip,br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -305,7 +305,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip,deflate,br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -316,7 +316,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5,br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -327,7 +327,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5,deflate,br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -342,7 +342,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -357,7 +357,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -376,7 +376,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("br"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -387,23 +387,23 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5,br;q=0.8"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.8,br;q=0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Gzip, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.995,br;q=0.999"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -414,31 +414,31 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5,deflate;q=0.6,br;q=0.8"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.8,deflate;q=0.6,br;q=0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Gzip, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.6,deflate;q=0.8,br;q=0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Deflate, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.995,deflate;q=0.997,br;q=0.999"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -449,7 +449,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("invalid,gzip"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Gzip, encoding);
}

Expand All @@ -460,23 +460,23 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0."),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0,br;q=0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -487,15 +487,15 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gZiP"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Gzip, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5,br;Q=0.8"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -506,7 +506,7 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static(" gzip\t; q=0.5 ,\tbr ;\tq=0.8\t"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Brotli, encoding);
}

Expand All @@ -517,15 +517,15 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q =0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q= 0.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);
}

Expand All @@ -536,47 +536,47 @@ mod tests {
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=-0.1"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=00.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=0.5000"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=.5"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=1.01"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);

let mut headers = http::HeaderMap::new();
headers.append(
http::header::ACCEPT_ENCODING,
http::HeaderValue::from_static("gzip;q=1.001"),
);
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll::default());
let encoding = Encoding::from_headers(&headers, SupportedEncodingsAll);
assert_eq!(Encoding::Identity, encoding);
}
}
6 changes: 6 additions & 0 deletions tower-http/src/cors/allow_private_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ impl AllowPrivateNetwork {
Self(AllowPrivateNetworkInner::Predicate(Arc::new(f)))
}

#[allow(clippy::borrow_interior_mutable_const)]
pub(super) fn to_header(
&self,
origin: Option<&HeaderValue>,
Expand All @@ -52,6 +53,7 @@ impl AllowPrivateNetwork {
const ALLOW_PRIVATE_NETWORK: HeaderName =
HeaderName::from_static("access-control-allow-private-network");

#[allow(clippy::declare_interior_mutable_const)]
const TRUE: HeaderValue = HeaderValue::from_static("true");

// Cheapest fallback: allow_private_network hasn't been set
Expand Down Expand Up @@ -110,6 +112,7 @@ impl Default for AllowPrivateNetworkInner {
}

#[cfg(test)]
#[allow(clippy::borrow_interior_mutable_const)]
mod tests {
use super::AllowPrivateNetwork;
use crate::cors::CorsLayer;
Expand All @@ -119,12 +122,15 @@ mod tests {
use tower::{BoxError, ServiceBuilder, ServiceExt};
use tower_service::Service;

#[allow(clippy::declare_interior_mutable_const)]
const REQUEST_PRIVATE_NETWORK: HeaderName =
HeaderName::from_static("access-control-request-private-network");

#[allow(clippy::declare_interior_mutable_const)]
const ALLOW_PRIVATE_NETWORK: HeaderName =
HeaderName::from_static("access-control-allow-private-network");

#[allow(clippy::declare_interior_mutable_const)]
const TRUE: HeaderValue = HeaderValue::from_static("true");

#[tokio::test]
Expand Down
3 changes: 1 addition & 2 deletions tower-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@
clippy::todo,
clippy::empty_enum,
clippy::enum_glob_use,
clippy::pub_enum_variant_names,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint deprecated?

clippy::mem_forget,
clippy::unused_self,
clippy::filter_map_next,
Expand Down Expand Up @@ -211,7 +210,7 @@
nonstandard_style,
missing_docs
)]
#![deny(unreachable_pub, private_in_public)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this lint got replaced by smaller individual lints? Probably worth enabling those / some of them.

#![deny(unreachable_pub)]
#![allow(
elided_lifetimes_in_paths,
// TODO: Remove this once the MSRV bumps to 1.42.0 or above.
Expand Down
Loading