Skip to content

Commit

Permalink
chore: A few simple cleanups/lints
Browse files Browse the repository at this point in the history
I asked clippy to help out, to see if any simple improvements are available. I ignored most of the pedantic ones, but a few of them seemed reasonable.

* in a few places, the match with `_` is used, even though there is only one other option. I think it is better to explicitly list them all - so that if another enum value is added in the future, it won't compile until someone looks at each case and decides what should the proper value be. This way there won't be any surprising breakages due to changes in the enums.
* a few `use wildcard` inlinings
* some big numbers are easier to read with underscores
* `for` iterating without `.iter()`
  • Loading branch information
nyurik committed May 22, 2023
1 parent 06c8e5b commit 60292d5
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 62 deletions.
11 changes: 5 additions & 6 deletions examples/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@
#[tokio::main]
async fn main() -> Result<(), reqwest::Error> {
// Some simple CLI args requirements...
let url = match std::env::args().nth(1) {
Some(url) => url,
None => {
println!("No CLI URL provided, using default.");
"https://hyper.rs".into()
}
let url = if let Some(url) = std::env::args().nth(1) {
url
} else {
println!("No CLI URL provided, using default.");
"https://hyper.rs".into()
};

eprintln!("Fetching {:?}...", url);
Expand Down
4 changes: 2 additions & 2 deletions src/async_impl/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl Request {
None => None,
};
let mut req = Request::new(self.method().clone(), self.url().clone());
*req.timeout_mut() = self.timeout().cloned();
*req.timeout_mut() = self.timeout().copied();
*req.headers_mut() = self.headers().clone();
*req.version_mut() = self.version();
req.body = body;
Expand Down Expand Up @@ -196,7 +196,7 @@ impl RequestBuilder {
self.header_sensitive(key, value, false)
}

/// Add a `Header` to this Request with ability to define if header_value is sensitive.
/// Add a `Header` to this Request with ability to define if `header_value` is sensitive.
fn header_sensitive<K, V>(mut self, key: K, value: V, sensitive: bool) -> RequestBuilder
where
HeaderName: TryFrom<K>,
Expand Down
34 changes: 17 additions & 17 deletions src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,15 @@ impl<S: IntoUrl> IntoProxyScheme for S {
}
source = err.source();
}
if !presumed_to_have_scheme {
// the issue could have been caused by a missing scheme, so we try adding http://
let try_this = format!("http://{}", self.as_str());
try_this.into_url().map_err(|_| {
// return the original error
crate::error::builder(e)
})?
} else {
if presumed_to_have_scheme {
return Err(crate::error::builder(e));
}
// the issue could have been caused by a missing scheme, so we try adding http://
let try_this = format!("http://{}", self.as_str());
try_this.into_url().map_err(|_| {
// return the original error
crate::error::builder(e)
})?
}
};
ProxyScheme::parse(url)
Expand Down Expand Up @@ -330,7 +329,7 @@ impl Proxy {
.get("http")
.and_then(|s| s.maybe_http_auth())
.is_some(),
_ => false,
Intercept::Https(_) => false,
}
}

Expand All @@ -343,7 +342,7 @@ impl Proxy {
Intercept::Custom(custom) => {
custom.call(uri).and_then(|s| s.maybe_http_auth().cloned())
}
_ => None,
Intercept::Https(_) => None,
}
}

Expand Down Expand Up @@ -422,7 +421,7 @@ impl NoProxy {
Self::from_string(&raw)
}

/// Returns a new no-proxy configuration based on a no_proxy string (or `None` if no variables
/// Returns a new no-proxy configuration based on a `no_proxy` string (or `None` if no variables
/// are set)
/// The rules are as follows:
/// * The environment variable `NO_PROXY` is checked, if it is not set, `no_proxy` is checked
Expand Down Expand Up @@ -483,7 +482,7 @@ impl NoProxy {

impl IpMatcher {
fn contains(&self, addr: IpAddr) -> bool {
for ip in self.0.iter() {
for ip in &self.0 {
match ip {
Ip::Address(address) => {
if &addr == address {
Expand All @@ -507,7 +506,7 @@ impl DomainMatcher {
// * https://github.com/curl/curl/issues/1208
fn contains(&self, domain: &str) -> bool {
let domain_len = domain.len();
for d in self.0.iter() {
for d in &self.0 {
if d == domain || d.strip_prefix('.') == Some(domain) {
return true;
} else if domain.ends_with(d) {
Expand Down Expand Up @@ -743,7 +742,7 @@ impl Custom {
"{}://{}{}{}",
uri.scheme(),
uri.host(),
uri.port().map(|_| ":").unwrap_or(""),
uri.port().map_or("", |_| ":"),
uri.port().map(|p| p.to_string()).unwrap_or_default()
)
.parse()
Expand Down Expand Up @@ -1658,9 +1657,10 @@ mod test {

fn check_parse_error(url: &str, needle: url::ParseError) {
let error = Proxy::http(url).unwrap_err();
if !includes(&error, needle) {
panic!("{:?} expected; {:?}, {} found", needle, error, error);
}
assert!(
includes(&error, needle),
"{needle:?} expected; {error:?}, {error} found"
);
}

mod when_scheme_missing {
Expand Down
2 changes: 1 addition & 1 deletion tests/blocking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ mod support;
use http::header::CONTENT_TYPE;
use http::HeaderValue;
use std::collections::HashMap;
use support::*;
use support::server;

#[test]
fn test_response_text() {
Expand Down
2 changes: 1 addition & 1 deletion tests/brotli.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod support;
use std::io::Read;
use support::*;
use support::server;

#[tokio::test]
async fn brotli_response() {
Expand Down
8 changes: 5 additions & 3 deletions tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
mod support;

use futures_util::stream::StreamExt;
use support::server;

#[cfg(feature = "json")]
use http::header::CONTENT_TYPE;
use http::HeaderValue;
#[cfg(feature = "json")]
use std::collections::HashMap;
use support::*;

use reqwest::Client;

Expand Down Expand Up @@ -382,7 +384,7 @@ async fn test_allowed_methods() {
fn add_json_default_content_type_if_not_set_manually() {
let mut map = HashMap::new();
map.insert("body", "json");
let content_type = HeaderValue::from_static("application/vnd.api+json");
let content_type = http::HeaderValue::from_static("application/vnd.api+json");
let req = Client::new()
.post("https://google.com/")
.header(CONTENT_TYPE, &content_type)
Expand Down
4 changes: 2 additions & 2 deletions tests/cookie.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
mod support;
use support::*;
use support::server;

#[tokio::test]
async fn cookie_response_accessor() {
Expand Down Expand Up @@ -36,7 +36,7 @@ async fn cookie_response_accessor() {
assert_eq!(cookies[1].name(), "expires");
assert_eq!(
cookies[1].expires().unwrap(),
std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1445412480)
std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_445_412_480)
);

// path
Expand Down
2 changes: 1 addition & 1 deletion tests/deflate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod support;
use std::io::Write;
use support::*;
use support::server;

#[tokio::test]
async fn deflate_response() {
Expand Down
2 changes: 1 addition & 1 deletion tests/gzip.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
mod support;
use support::*;
use support::server;

use std::io::Write;

Expand Down
2 changes: 1 addition & 1 deletion tests/multipart.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![cfg(not(target_arch = "wasm32"))]
mod support;
use futures_util::stream::StreamExt;
use support::*;
use support::server;

#[tokio::test]
async fn text_part() {
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg(not(target_arch = "wasm32"))]
mod support;
use support::*;
use support::server;

use std::env;

Expand Down
39 changes: 20 additions & 19 deletions tests/redirect.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
#![cfg(not(target_arch = "wasm32"))]
mod support;
use futures_util::stream::StreamExt;
use support::*;
use hyper::Body;
use support::server;

#[tokio::test]
async fn test_redirect_301_and_302_and_303_changes_post_to_get() {
let client = reqwest::Client::new();
let codes = [301u16, 302, 303];

for &code in codes.iter() {
for &code in &codes {
let redirect = server::http(move |req| async move {
if req.method() == "POST" {
assert_eq!(req.uri(), &*format!("/{}", code));
http::Response::builder()
.status(code)
.header("location", "/dst")
.header("server", "test-redirect")
.body(Default::default())
.body(Body::default())
.unwrap()
} else {
assert_eq!(req.method(), "GET");

http::Response::builder()
.header("server", "test-dst")
.body(Default::default())
.body(Body::default())
.unwrap()
}
});
Expand All @@ -44,22 +45,22 @@ async fn test_redirect_301_and_302_and_303_changes_post_to_get() {
async fn test_redirect_307_and_308_tries_to_get_again() {
let client = reqwest::Client::new();
let codes = [307u16, 308];
for &code in codes.iter() {
for &code in &codes {
let redirect = server::http(move |req| async move {
assert_eq!(req.method(), "GET");
if req.uri() == &*format!("/{}", code) {
http::Response::builder()
.status(code)
.header("location", "/dst")
.header("server", "test-redirect")
.body(Default::default())
.body(Body::default())
.unwrap()
} else {
assert_eq!(req.uri(), "/dst");

http::Response::builder()
.header("server", "test-dst")
.body(Default::default())
.body(Body::default())
.unwrap()
}
});
Expand All @@ -81,7 +82,7 @@ async fn test_redirect_307_and_308_tries_to_post_again() {
let _ = env_logger::try_init();
let client = reqwest::Client::new();
let codes = [307u16, 308];
for &code in codes.iter() {
for &code in &codes {
let redirect = server::http(move |mut req| async move {
assert_eq!(req.method(), "POST");
assert_eq!(req.headers()["content-length"], "5");
Expand All @@ -94,14 +95,14 @@ async fn test_redirect_307_and_308_tries_to_post_again() {
.status(code)
.header("location", "/dst")
.header("server", "test-redirect")
.body(Default::default())
.body(Body::default())
.unwrap()
} else {
assert_eq!(req.uri(), "/dst");

http::Response::builder()
.header("server", "test-dst")
.body(Default::default())
.body(Body::default())
.unwrap()
}
});
Expand All @@ -123,7 +124,7 @@ async fn test_redirect_307_and_308_tries_to_post_again() {
fn test_redirect_307_does_not_try_if_reader_cannot_reset() {
let client = reqwest::blocking::Client::new();
let codes = [307u16, 308];
for &code in codes.iter() {
for &code in &codes {
let redirect = server::http(move |mut req| async move {
assert_eq!(req.method(), "POST");
assert_eq!(req.uri(), &*format!("/{}", code));
Expand All @@ -136,7 +137,7 @@ fn test_redirect_307_does_not_try_if_reader_cannot_reset() {
.status(code)
.header("location", "/dst")
.header("server", "test-redirect")
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand Down Expand Up @@ -179,7 +180,7 @@ async fn test_redirect_removes_sensitive_headers() {
http::Response::builder()
.status(302)
.header("location", format!("http://{}/end", end_addr))
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand All @@ -205,7 +206,7 @@ async fn test_redirect_policy_can_return_errors() {
http::Response::builder()
.status(302)
.header("location", "/loop")
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand All @@ -221,7 +222,7 @@ async fn test_redirect_policy_can_stop_redirects_without_an_error() {
http::Response::builder()
.status(302)
.header("location", "/dont")
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand All @@ -247,7 +248,7 @@ async fn test_referer_is_not_set_if_disabled() {
http::Response::builder()
.status(302)
.header("location", "/dst")
.body(Default::default())
.body(Body::default())
.unwrap()
} else {
assert_eq!(req.uri(), "/dst");
Expand All @@ -273,7 +274,7 @@ async fn test_invalid_location_stops_redirect_gh484() {
http::Response::builder()
.status(302)
.header("location", "http://www.yikes{KABOOM}")
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand All @@ -295,7 +296,7 @@ async fn test_redirect_302_with_set_cookies() {
.status(302)
.header("location", "/dst")
.header("set-cookie", "key=value")
.body(Default::default())
.body(Body::default())
.unwrap()
} else {
assert_eq!(req.uri(), "/dst");
Expand Down Expand Up @@ -325,7 +326,7 @@ async fn test_redirect_https_only_enforced_gh1312() {
http::Response::builder()
.status(302)
.header("location", "http://insecure")
.body(Default::default())
.body(Body::default())
.unwrap()
});

Expand Down

0 comments on commit 60292d5

Please sign in to comment.