From d25a30bc2ce13b428043713d18185c8a75daf8aa Mon Sep 17 00:00:00 2001 From: Michael Rodler Date: Thu, 8 Jun 2023 12:53:20 +0000 Subject: [PATCH 1/2] Enabled clippy in CI; added some safety comments; fixed some clippy warnings; --- .github/workflows/ci.yml | 11 +++++++++++ Cargo.toml | 2 +- clippy.toml | 1 + src/extensions.rs | 1 + src/header/map.rs | 20 ++++++++++++++++---- src/header/name.rs | 3 ++- src/header/value.rs | 5 +++++ src/lib.rs | 4 ++++ src/status.rs | 6 +++++- src/uri/mod.rs | 3 +++ src/uri/path.rs | 1 + 11 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 clippy.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13c79504..803743a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,6 +41,17 @@ jobs: if: matrix.benches run: cargo test --benches ${{ matrix.features }} + + clippy_check: + runs-on: ubuntu-latest + # Make sure CI fails on all warnings, including Clippy lints + env: + RUSTFLAGS: "-Dwarnings" + steps: + - uses: actions/checkout@v3 + - name: Run Clippy + run: cargo clippy --all-targets --all-features + msrv: name: Check MSRV runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 75eaae57..8f364c5d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ A set of types for representing HTTP requests and responses. keywords = ["http"] categories = ["web-programming"] edition = "2018" -# When updating this value, don't forget to also adjust the GitHub Actions config. +# When updating this value, don't forget to also adjust the clippy config. rust-version = "1.49.0" [workspace] diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..f3885f59 --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +msrv="1.49" diff --git a/src/extensions.rs b/src/extensions.rs index 7e815df7..942aa22e 100644 --- a/src/extensions.rs +++ b/src/extensions.rs @@ -60,6 +60,7 @@ impl Extensions { /// assert_eq!(ext.insert(9i32), Some(5i32)); /// ``` pub fn insert(&mut self, val: T) -> Option { + #[allow(clippy::box_default)] self.map .get_or_insert_with(|| Box::new(HashMap::default())) .insert(TypeId::of::(), Box::new(val)) diff --git a/src/header/map.rs b/src/header/map.rs index ed049dbc..a52d4e0a 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -961,6 +961,7 @@ impl HeaderMap { let entries = &mut self.entries[..] as *mut _; let extra_values = &mut self.extra_values as *mut _; let len = self.entries.len(); + // SAFETY: see comment above unsafe { self.entries.set_len(0); } Drain { @@ -2070,6 +2071,7 @@ impl<'a, T> Iterator for Iter<'a, T> { fn next(&mut self) -> Option { self.inner .next_unsafe() + // SAFETY: Iter invariant: only valid pointers .map(|(key, ptr)| (key, unsafe { &*ptr })) } @@ -2090,6 +2092,7 @@ impl<'a, T> IterMut<'a, T> { use self::Cursor::*; if self.cursor.is_none() { + // SAFETY: invariant dereferencing the self.map pointer is always safe if (self.entry + 1) >= unsafe { &*self.map }.entries.len() { return None; } @@ -2098,6 +2101,7 @@ impl<'a, T> IterMut<'a, T> { self.cursor = Some(Cursor::Head); } + // SAFETY: invariant dereferencing the self.map pointer is always safe let entry = unsafe { &(*self.map).entries[self.entry] }; match self.cursor.unwrap() { @@ -2106,6 +2110,7 @@ impl<'a, T> IterMut<'a, T> { Some((&entry.key, &entry.value as *const _ as *mut _)) } Values(idx) => { + // SAFETY: invariant dereferencing the self.map pointer is always safe let extra = unsafe { &(*self.map).extra_values[idx] }; match extra.next { @@ -2128,6 +2133,7 @@ impl<'a, T> Iterator for IterMut<'a, T> { } fn size_hint(&self) -> (usize, Option) { + // SAFETY: invariant dereferencing the self.map pointer is always safe let map = unsafe { &*self.map }; debug_assert!(map.entries.len() >= self.entry); @@ -2204,9 +2210,8 @@ impl<'a, T> Iterator for Drain<'a, T> { // Remove the extra value let raw_links = RawLinks(self.entries); - let extra = unsafe { - remove_extra_value(raw_links, &mut *self.extra_values, next) - }; + // SAFETY: dereferencing self.extra_values valid as long as self is alive. + let extra = remove_extra_value(raw_links, unsafe { &mut *self.extra_values } , next); match extra.next { Link::Extra(idx) => self.next = Some(idx), @@ -2224,6 +2229,8 @@ impl<'a, T> Iterator for Drain<'a, T> { self.idx += 1; + // SAFETY: pointer operation always valid, as `self` cannot outlive the HeaderMap it is + // referencing. unsafe { let entry = &(*self.entries)[idx]; @@ -2243,6 +2250,7 @@ impl<'a, T> Iterator for Drain<'a, T> { // For instance, extending a new `HeaderMap` wouldn't need to // reserve the upper-bound in `entries`, only the lower-bound. let lower = self.len - self.idx; + // SAFETY: dereferencing self.extra_values valid as long as self is alive. let upper = unsafe { (*self.extra_values).len() } + lower; (lower, Some(upper)) } @@ -2606,6 +2614,7 @@ impl<'a, T: 'a> Iterator for ValueIterMut<'a, T> { fn next(&mut self) -> Option { use self::Cursor::*; + // SAFETY: dereferencing self.map valid as long as self is alive. let entry = unsafe { &mut (*self.map).entries[self.index] }; match self.front { @@ -2626,6 +2635,7 @@ impl<'a, T: 'a> Iterator for ValueIterMut<'a, T> { Some(&mut entry.value) } Some(Values(idx)) => { + // SAFETY: dereferencing self.map valid as long as self is alive. let extra = unsafe { &mut (*self.map).extra_values[idx] }; if self.front == self.back { @@ -2649,6 +2659,7 @@ impl<'a, T: 'a> DoubleEndedIterator for ValueIterMut<'a, T> { fn next_back(&mut self) -> Option { use self::Cursor::*; + // SAFETY: dereferencing self.map valid as long as self is alive. let entry = unsafe { &mut (*self.map).entries[self.index] }; match self.back { @@ -2658,6 +2669,7 @@ impl<'a, T: 'a> DoubleEndedIterator for ValueIterMut<'a, T> { Some(&mut entry.value) } Some(Values(idx)) => { + // SAFETY: dereferencing self.map valid as long as self is alive. let extra = unsafe { &mut (*self.map).extra_values[idx] }; if self.front == self.back { @@ -2726,7 +2738,7 @@ impl Drop for IntoIter { // Ensure the iterator is consumed for _ in self.by_ref() {} - // All the values have already been yielded out. + // SAFETY: All the values have already been yielded out, once dropped. unsafe { self.extra_values.set_len(0); } diff --git a/src/header/name.rs b/src/header/name.rs index 6080cf08..5d32272d 100644 --- a/src/header/name.rs +++ b/src/header/name.rs @@ -87,8 +87,8 @@ macro_rules! standard_headers { #[inline] fn as_str(&self) -> &'static str { match *self { - // Safety: test_parse_standard_headers ensures these &[u8]s are &str-safe. $( + // SAFETY: test_parse_standard_headers ensures these &[u8]s are &str-safe. StandardHeader::$konst => unsafe { std::str::from_utf8_unchecked( $name_bytes ) }, )+ } @@ -1267,6 +1267,7 @@ impl HeaderName { i += 1; } } { + #[allow(clippy::no_effect)] ([] as [u8; 0])[0]; // Invalid header name } diff --git a/src/header/value.rs b/src/header/value.rs index bf05f16f..bca69628 100644 --- a/src/header/value.rs +++ b/src/header/value.rs @@ -85,6 +85,7 @@ impl HeaderValue { let mut i = 0; while i < bytes.len() { if !is_visible_ascii(bytes[i]) { + #[allow(clippy::no_effect)] ([] as [u8; 0])[0]; // Invalid header value } i += 1; @@ -191,6 +192,10 @@ impl HeaderValue { /// /// This function does NOT validate that illegal bytes are not contained /// within the buffer. + /// + /// ## Safety + /// + /// The caller must ensure that `src` contains only legal utf-8. pub unsafe fn from_maybe_shared_unchecked(src: T) -> HeaderValue where T: AsRef<[u8]> + 'static, diff --git a/src/lib.rs b/src/lib.rs index 38a4c2aa..edfca9c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -159,6 +159,10 @@ //! ``` #![deny(warnings, missing_docs, missing_debug_implementations)] +#![deny( + clippy::missing_safety_doc, + clippy::undocumented_unsafe_blocks +)] #[cfg(test)] #[macro_use] diff --git a/src/status.rs b/src/status.rs index d98d24c3..abdadd25 100644 --- a/src/status.rs +++ b/src/status.rs @@ -143,6 +143,8 @@ impl StatusCode { { &CODE_DIGITS[offset..offset+3] } #[cfg(not(debug_assertions))] + // SAFETY: we assume `StatusCode` is constructed in a way that self.0 (the numerical status + // code is >= 100 and also <= 1000, which makes any possible offset here valid. unsafe { CODE_DIGITS.get_unchecked(offset..offset+3) } } @@ -304,7 +306,9 @@ macro_rules! status_codes { impl StatusCode { $( $(#[$docs])* - pub const $konst: StatusCode = StatusCode(unsafe { NonZeroU16::new_unchecked($num) }); + pub const $konst: StatusCode = StatusCode( + // SAFETY: only called with constants + unsafe { NonZeroU16::new_unchecked($num) }); )+ } diff --git a/src/uri/mod.rs b/src/uri/mod.rs index 30be83b5..8cb6e216 100644 --- a/src/uri/mod.rs +++ b/src/uri/mod.rs @@ -837,6 +837,7 @@ fn parse_full(mut s: Bytes) -> Result { let _ = scheme.split_off(n); // Allocate the ByteStr + // SAFETY: previously verified by `Scheme2::parse` let val = unsafe { ByteStr::from_utf8_unchecked(scheme) }; Scheme2::Other(Box::new(val)) @@ -853,6 +854,7 @@ fn parse_full(mut s: Bytes) -> Result { } let authority = Authority { + // SAFETY: previously verified by `Authority::parse` data: unsafe { ByteStr::from_utf8_unchecked(s) }, }; @@ -870,6 +872,7 @@ fn parse_full(mut s: Bytes) -> Result { let authority = s.split_to(authority_end); let authority = Authority { + // SAFETY: previously verified by `Authority::parse` data: unsafe { ByteStr::from_utf8_unchecked(authority) }, }; diff --git a/src/uri/path.rs b/src/uri/path.rs index be2cb65c..0c390157 100644 --- a/src/uri/path.rs +++ b/src/uri/path.rs @@ -97,6 +97,7 @@ impl PathAndQuery { } Ok(PathAndQuery { + // Safety: previous iteration ensures that src is also valid utf-8 data: unsafe { ByteStr::from_utf8_unchecked(src) }, query: query, }) From 23c625738bb53f45d8ccff393c362848a65cc91f Mon Sep 17 00:00:00 2001 From: Michael Rodler Date: Thu, 8 Jun 2023 12:54:12 +0000 Subject: [PATCH 2/2] run clippy --fix --- src/byte_str.rs | 2 +- src/header/map.rs | 56 ++++++++++++++++++++-------------------- src/header/name.rs | 10 +++---- src/method.rs | 4 +-- src/request.rs | 4 +-- src/response.rs | 4 +-- src/status.rs | 6 ++--- src/uri/authority.rs | 17 ++++++------ src/uri/mod.rs | 30 ++++++++++----------- src/uri/path.rs | 12 ++++----- src/uri/scheme.rs | 4 +-- tests/header_map.rs | 2 +- tests/header_map_fuzz.rs | 27 ++++++++++--------- 13 files changed, 87 insertions(+), 91 deletions(-) diff --git a/src/byte_str.rs b/src/byte_str.rs index e83ff75d..c5d256b7 100644 --- a/src/byte_str.rs +++ b/src/byte_str.rs @@ -43,7 +43,7 @@ impl ByteStr { } } // Invariant: assumed by the safety requirements of this function. - ByteStr { bytes: bytes } + ByteStr { bytes } } } diff --git a/src/header/map.rs b/src/header/map.rs index a52d4e0a..f2b7e7a4 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -642,7 +642,7 @@ impl HeaderMap { assert!(cap <= MAX_SIZE, "header map reserve over max capacity"); assert!(cap != 0, "header map reserve overflowed"); - if self.entries.len() == 0 { + if self.entries.is_empty() { self.mask = cap as Size - 1; self.indices = vec![Pos::none(); cap].into_boxed_slice(); self.entries = Vec::with_capacity(usable_capacity(cap)); @@ -1082,22 +1082,22 @@ impl HeaderMap { danger, Entry::Vacant(VacantEntry { map: self, - hash: hash, + hash, key: key.into(), - probe: probe, - danger: danger, + probe, + danger, }), Entry::Occupied(OccupiedEntry { map: self, index: pos, - probe: probe, + probe, }), Entry::Vacant(VacantEntry { map: self, - hash: hash, + hash, key: key.into(), - probe: probe, - danger: danger, + probe, + danger, }) ) } @@ -1201,7 +1201,7 @@ impl HeaderMap { ValueDrain { first: Some(old), - next: next, + next, lt: PhantomData, } } @@ -1407,7 +1407,7 @@ impl HeaderMap { // backward shift deletion in self.indices // after probe, shift all non-ideally placed indices backward - if self.entries.len() > 0 { + if !self.entries.is_empty() { let mut last_probe = probe; let mut probe = probe + 1; @@ -1454,9 +1454,9 @@ impl HeaderMap { assert!(self.entries.len() < MAX_SIZE, "header map at capacity"); self.entries.push(Bucket { - hash: hash, - key: key, - value: value, + hash, + key, + value, links: None, }); } @@ -1851,7 +1851,7 @@ impl<'a, K, V, T> TryFrom<&'a HashMap> for HeaderMap type Error = Error; fn try_from(c: &'a HashMap) -> Result { - c.into_iter() + c.iter() .map(|(k, v)| -> crate::Result<(HeaderName, T)> { let name = TryFrom::try_from(k).map_err(Into::into)?; let value = TryFrom::try_from(v).map_err(Into::into)?; @@ -2038,7 +2038,7 @@ fn append_value( Some(links) => { let idx = extra.len(); extra.push(ExtraValue { - value: value, + value, prev: Link::Extra(links.tail), next: Link::Entry(entry_idx), }); @@ -2050,7 +2050,7 @@ fn append_value( None => { let idx = extra.len(); extra.push(ExtraValue { - value: value, + value, prev: Link::Entry(entry_idx), next: Link::Entry(entry_idx), }); @@ -2422,7 +2422,7 @@ impl<'a, T> VacantEntry<'a, T> { // Ensure that there is space in the map let index = self.map - .insert_phase_two(self.key, value.into(), self.hash, self.probe, self.danger); + .insert_phase_two(self.key, value, self.hash, self.probe, self.danger); &mut self.map.entries[index].value } @@ -2449,11 +2449,11 @@ impl<'a, T> VacantEntry<'a, T> { // Ensure that there is space in the map let index = self.map - .insert_phase_two(self.key, value.into(), self.hash, self.probe, self.danger); + .insert_phase_two(self.key, value, self.hash, self.probe, self.danger); OccupiedEntry { map: self.map, - index: index, + index, probe: self.probe, } } @@ -2863,7 +2863,7 @@ impl<'a, T> OccupiedEntry<'a, T> { /// assert_eq!("earth", map["host"]); /// ``` pub fn insert(&mut self, value: T) -> T { - self.map.insert_occupied(self.index, value.into()) + self.map.insert_occupied(self.index, value) } /// Sets the value of the entry. @@ -2889,7 +2889,7 @@ impl<'a, T> OccupiedEntry<'a, T> { /// assert_eq!("earth", map["host"]); /// ``` pub fn insert_mult(&mut self, value: T) -> ValueDrain<'_, T> { - self.map.insert_occupied_mult(self.index, value.into()) + self.map.insert_occupied_mult(self.index, value) } /// Insert the value into the entry. @@ -2916,7 +2916,7 @@ impl<'a, T> OccupiedEntry<'a, T> { pub fn append(&mut self, value: T) { let idx = self.index; let entry = &mut self.map.entries[idx]; - append_value(idx, entry, &mut self.map.extra_values, value.into()); + append_value(idx, entry, &mut self.map.extra_values, value); } /// Remove the entry from the map. @@ -3095,12 +3095,12 @@ impl<'a, T> Iterator for ValueDrain<'a, T> { // Exactly 1 (&Some(_), &None) => (1, Some(1)), // 1 + extras - (&Some(_), &Some(ref extras)) => { + (&Some(_), Some(extras)) => { let (l, u) = extras.size_hint(); (l + 1, u.map(|u| u + 1)) }, // Extras only - (&None, &Some(ref extras)) => extras.size_hint(), + (&None, Some(extras)) => extras.size_hint(), // No more (&None, &None) => (0, Some(0)), } @@ -3111,7 +3111,7 @@ impl<'a, T> FusedIterator for ValueDrain<'a, T> {} impl<'a, T> Drop for ValueDrain<'a, T> { fn drop(&mut self) { - while let Some(_) = self.next() {} + for _ in self.by_ref() {} } } @@ -3154,7 +3154,7 @@ impl Pos { debug_assert!(index < MAX_SIZE); Pos { index: index as Size, - hash: hash, + hash, } } @@ -3418,7 +3418,7 @@ mod as_header_name { } fn as_str(&self) -> &str { - ::as_str(*self) + ::as_str(self) } } @@ -3472,7 +3472,7 @@ mod as_header_name { } fn as_str(&self) -> &str { - *self + self } } diff --git a/src/header/name.rs b/src/header/name.rs index 5d32272d..c0a83261 100644 --- a/src/header/name.rs +++ b/src/header/name.rs @@ -1256,7 +1256,7 @@ impl HeaderName { }; } - if name_bytes.len() == 0 || name_bytes.len() > super::MAX_HEADER_NAME_LEN || { + if name_bytes.is_empty() || name_bytes.len() > super::MAX_HEADER_NAME_LEN || { let mut i = 0; loop { if i >= name_bytes.len() { @@ -1283,7 +1283,7 @@ impl HeaderName { pub fn as_str(&self) -> &str { match self.inner { Repr::Standard(v) => v.as_str(), - Repr::Custom(ref v) => &*v.0, + Repr::Custom(ref v) => &v.0, } } @@ -1516,8 +1516,8 @@ impl<'a> HdrName<'a> { HdrName { // Invariant (on MaybeLower): follows from the precondition inner: Repr::Custom(MaybeLower { - buf: buf, - lower: lower, + buf, + lower, }), } } @@ -1552,7 +1552,7 @@ impl<'a> From> for HeaderName { }, Repr::Custom(maybe_lower) => { if maybe_lower.lower { - let buf = Bytes::copy_from_slice(&maybe_lower.buf[..]); + let buf = Bytes::copy_from_slice(maybe_lower.buf); // Safety: the invariant on MaybeLower ensures buf is valid UTF-8. let byte_str = unsafe { ByteStr::from_utf8_unchecked(buf) }; diff --git a/src/method.rs b/src/method.rs index b7b3b357..c0c30b67 100644 --- a/src/method.rs +++ b/src/method.rs @@ -355,7 +355,7 @@ mod extension { } pub fn as_str(&self) -> &str { - // Safety: the invariant of AllocatedExtension ensures that self.0 + // SAFETY: the invariant of AllocatedExtension ensures that self.0 // contains valid UTF-8. unsafe {str::from_utf8_unchecked(&self.0)} } @@ -468,6 +468,6 @@ mod test { assert_eq!(Method::from_str("wOw!!").unwrap(), "wOw!!"); let long_method = "This_is_a_very_long_method.It_is_valid_but_unlikely."; - assert_eq!(Method::from_str(&long_method).unwrap(), long_method); + assert_eq!(Method::from_str(long_method).unwrap(), long_method); } } diff --git a/src/request.rs b/src/request.rs index 4481187c..42ea02bc 100644 --- a/src/request.rs +++ b/src/request.rs @@ -439,7 +439,7 @@ impl Request { pub fn new(body: T) -> Request { Request { head: Parts::new(), - body: body, + body, } } @@ -459,7 +459,7 @@ impl Request { pub fn from_parts(parts: Parts, body: T) -> Request { Request { head: parts, - body: body, + body, } } diff --git a/src/response.rs b/src/response.rs index da0fec98..3e9a49b5 100644 --- a/src/response.rs +++ b/src/response.rs @@ -251,7 +251,7 @@ impl Response { pub fn new(body: T) -> Response { Response { head: Parts::new(), - body: body, + body, } } @@ -274,7 +274,7 @@ impl Response { pub fn from_parts(parts: Parts, body: T) -> Response { Response { head: parts, - body: body, + body, } } diff --git a/src/status.rs b/src/status.rs index abdadd25..47fb2614 100644 --- a/src/status.rs +++ b/src/status.rs @@ -71,7 +71,7 @@ impl StatusCode { /// ``` #[inline] pub fn from_u16(src: u16) -> Result { - if src < 100 || src >= 1000 { + if !(100..1000).contains(&src) { return Err(InvalidStatusCode::new()); } @@ -265,7 +265,7 @@ impl FromStr for StatusCode { impl<'a> From<&'a StatusCode> for StatusCode { #[inline] fn from(t: &'a StatusCode) -> Self { - t.clone() + *t } } @@ -544,7 +544,7 @@ impl Error for InvalidStatusCode {} // A string of packed 3-ASCII-digit status code values for the supported range // of [100, 999] (900 codes, 2700 bytes). -const CODE_DIGITS: &'static str = "\ +const CODE_DIGITS: &str = "\ 100101102103104105106107108109110111112113114115116117118119\ 120121122123124125126127128129130131132133134135136137138139\ 140141142143144145146147148149150151152153154155156157158159\ diff --git a/src/uri/authority.rs b/src/uri/authority.rs index f41ddd19..7da5388a 100644 --- a/src/uri/authority.rs +++ b/src/uri/authority.rs @@ -238,7 +238,7 @@ impl Authority { pub fn port(&self) -> Option> { let bytes = self.as_str(); bytes - .rfind(":") + .rfind(':') .and_then(|i| Port::from_str(&bytes[i + 1..]).ok()) } @@ -253,7 +253,7 @@ impl Authority { /// assert_eq!(authority.port_u16(), Some(80)); /// ``` pub fn port_u16(&self) -> Option { - self.port().and_then(|p| Some(p.as_u16())) + self.port().map(|p| p.as_u16()) } /// Return a str representation of the authority @@ -434,7 +434,7 @@ impl<'a> TryFrom<&'a [u8]> for Authority { // Preconditon on create_authority: copy_from_slice() copies all of // bytes from the [u8] parameter into a new Bytes - create_authority(s, |s| Bytes::copy_from_slice(s)) + create_authority(s, Bytes::copy_from_slice) } } @@ -485,8 +485,7 @@ impl fmt::Display for Authority { } fn host(auth: &str) -> &str { - let host_port = auth - .rsplitn(2, '@') + let host_port = auth.rsplit('@') .next() .expect("split always has at least 1 item"); @@ -619,10 +618,10 @@ mod tests { #[test] fn compares_with_a_string() { let authority: Authority = "def.com".parse().unwrap(); - assert!(authority < "ghi.com".to_string()); - assert!("ghi.com".to_string() > authority); - assert!(authority > "abc.com".to_string()); - assert!("abc.com".to_string() < authority); + assert!(authority < *"ghi.com"); + assert!(*"ghi.com" > authority); + assert!(authority > *"abc.com"); + assert!(*"abc.com" < authority); } #[test] diff --git a/src/uri/mod.rs b/src/uri/mod.rs index 8cb6e216..76502c87 100644 --- a/src/uri/mod.rs +++ b/src/uri/mod.rs @@ -244,10 +244,8 @@ impl Uri { if src.path_and_query.is_none() { return Err(ErrorKind::PathAndQueryMissing.into()); } - } else { - if src.authority.is_some() && src.path_and_query.is_some() { - return Err(ErrorKind::SchemeMissing.into()); - } + } else if src.authority.is_some() && src.path_and_query.is_some() { + return Err(ErrorKind::SchemeMissing.into()); } let scheme = match src.scheme { @@ -268,9 +266,9 @@ impl Uri { }; Ok(Uri { - scheme: scheme, - authority: authority, - path_and_query: path_and_query, + scheme, + authority, + path_and_query, }) } @@ -321,7 +319,7 @@ impl Uri { return Ok(Uri { scheme: Scheme::empty(), - authority: authority, + authority, path_and_query: PathAndQuery::empty(), }); } @@ -650,7 +648,7 @@ impl Uri { /// assert_eq!(uri.port_u16(), Some(80)); /// ``` pub fn port_u16(&self) -> Option { - self.port().and_then(|p| Some(p.as_u16())) + self.port().map(|p| p.as_u16()) } /// Get the query string of this `Uri`, starting after the `?`. @@ -812,9 +810,9 @@ impl From for Parts { }; Parts { - scheme: scheme, - authority: authority, - path_and_query: path_and_query, + scheme, + authority, + path_and_query, _priv: (), } } @@ -860,7 +858,7 @@ fn parse_full(mut s: Bytes) -> Result { return Ok(Uri { scheme: scheme.into(), - authority: authority, + authority, path_and_query: PathAndQuery::empty(), }); } @@ -878,7 +876,7 @@ fn parse_full(mut s: Bytes) -> Result { Ok(Uri { scheme: scheme.into(), - authority: authority, + authority, path_and_query: PathAndQuery::from_shared(s)?, }) } @@ -968,8 +966,8 @@ impl PartialEq for Uri { } if let Some(query) = self.query() { - if other.len() == 0 { - return query.len() == 0; + if other.is_empty() { + return query.is_empty(); } if other[0] != b'?' { diff --git a/src/uri/path.rs b/src/uri/path.rs index 0c390157..ae3ac71a 100644 --- a/src/uri/path.rs +++ b/src/uri/path.rs @@ -99,7 +99,7 @@ impl PathAndQuery { Ok(PathAndQuery { // Safety: previous iteration ensures that src is also valid utf-8 data: unsafe { ByteStr::from_utf8_unchecked(src) }, - query: query, + query, }) } @@ -532,10 +532,10 @@ mod tests { #[test] fn compares_with_a_string() { let path_and_query: PathAndQuery = "/b/world&foo=bar".parse().unwrap(); - assert!(path_and_query < "/c/world&foo=bar".to_string()); - assert!("/c/world&foo=bar".to_string() > path_and_query); - assert!(path_and_query > "/a/world&foo=bar".to_string()); - assert!("/a/world&foo=bar".to_string() < path_and_query); + assert!(path_and_query < *"/c/world&foo=bar"); + assert!(*"/c/world&foo=bar" > path_and_query); + assert!(path_and_query > *"/a/world&foo=bar"); + assert!(*"/a/world&foo=bar" < path_and_query); } #[test] @@ -560,6 +560,6 @@ mod tests { } fn pq(s: &str) -> PathAndQuery { - s.parse().expect(&format!("parsing {}", s)) + s.parse().unwrap_or_else(|_| panic!("parsing {}", s)) } } diff --git a/src/uri/scheme.rs b/src/uri/scheme.rs index 682b11ee..156bfc7c 100644 --- a/src/uri/scheme.rs +++ b/src/uri/scheme.rs @@ -132,7 +132,7 @@ impl PartialEq for Scheme { match (&self.inner, &other.inner) { (&Standard(Http), &Standard(Http)) => true, (&Standard(Https), &Standard(Https)) => true, - (&Other(ref a), &Other(ref b)) => a.eq_ignore_ascii_case(b), + (Other(a), Other(b)) => a.eq_ignore_ascii_case(b), (&None, _) | (_, &None) => unreachable!(), _ => false, } @@ -358,6 +358,6 @@ mod test { } fn scheme(s: &str) -> Scheme { - s.parse().expect(&format!("Invalid scheme: {}", s)) + s.parse().unwrap_or_else(|_| panic!("Invalid scheme: {}", s)) } } diff --git a/tests/header_map.rs b/tests/header_map.rs index f2beba08..619ac124 100644 --- a/tests/header_map.rs +++ b/tests/header_map.rs @@ -327,7 +327,7 @@ fn custom_std(n: usize) -> Vec { .collect() } -const STD: &'static [HeaderName] = &[ +const STD: &[HeaderName] = &[ ACCEPT, ACCEPT_CHARSET, ACCEPT_ENCODING, diff --git a/tests/header_map_fuzz.rs b/tests/header_map_fuzz.rs index 68a8604f..fb0602ad 100644 --- a/tests/header_map_fuzz.rs +++ b/tests/header_map_fuzz.rs @@ -88,8 +88,8 @@ impl Fuzz { } Fuzz { - seed: seed, - steps: steps, + seed, + steps, reduce: 0, } } @@ -120,7 +120,7 @@ impl AltMap { let action = self.gen_action(weight, rng); Step { - action: action, + action, expect: self.clone(), } } @@ -156,9 +156,9 @@ impl AltMap { let old = self.insert(name.clone(), val.clone()); Action::Insert { - name: name, - val: val, - old: old, + name, + val, + old, } } @@ -167,8 +167,8 @@ impl AltMap { let val = self.remove(&name); Action::Remove { - name: name, - val: val, + name, + val, } } @@ -182,9 +182,9 @@ impl AltMap { vals.push(val.clone()); Action::Append { - name: name, - val: val, - ret: ret, + name, + val, + ret, } } @@ -263,7 +263,7 @@ impl Action { } fn gen_header_name(g: &mut StdRng) -> HeaderName { - const STANDARD_HEADERS: &'static [HeaderName] = &[ + const STANDARD_HEADERS: &[HeaderName] = &[ header::ACCEPT, header::ACCEPT_CHARSET, header::ACCEPT_ENCODING, @@ -364,10 +364,9 @@ fn gen_string(g: &mut StdRng, min: usize, max: usize) -> String { let bytes: Vec<_> = (min..max) .map(|_| { // Chars to pick from - b"ABCDEFGHIJKLMNOPQRSTUVabcdefghilpqrstuvwxyz----" + *b"ABCDEFGHIJKLMNOPQRSTUVabcdefghilpqrstuvwxyz----" .choose(g) .unwrap() - .clone() }) .collect();