From 0521b3f7c47b0a109fb3e161ed796f53005ba726 Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Tue, 31 May 2022 14:05:56 +0200 Subject: [PATCH 1/4] add clippy toml with msrv = 1.31 --- clippy.toml | 1 + 1 file changed, 1 insertion(+) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..3d30690f1 --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +msrv = "1.31.0" From 4b52024d747f0aa1c2bcc97ba6fc550f5d64847c Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Tue, 31 May 2022 14:06:04 +0200 Subject: [PATCH 2/4] apply clippy lints --- build.rs | 4 +- src/kv/key.rs | 41 +----------- src/lib.rs | 162 ++++++----------------------------------------- tests/filters.rs | 10 +-- tests/macros.rs | 8 +-- 5 files changed, 29 insertions(+), 196 deletions(-) diff --git a/build.rs b/build.rs index dd8d4e0f7..11c26a333 100644 --- a/build.rs +++ b/build.rs @@ -22,7 +22,7 @@ fn main() { } fn target_has_atomic_cas(target: &str) -> bool { - match &target[..] { + match target { "thumbv6m-none-eabi" | "msp430-none-elf" | "riscv32i-unknown-none-elf" @@ -32,7 +32,7 @@ fn target_has_atomic_cas(target: &str) -> bool { } fn target_has_atomics(target: &str) -> bool { - match &target[..] { + match target { "thumbv4t-none-eabi" | "msp430-none-elf" | "riscv32i-unknown-none-elf" diff --git a/src/kv/key.rs b/src/kv/key.rs index 762e08dcb..eda5288a3 100644 --- a/src/kv/key.rs +++ b/src/kv/key.rs @@ -1,9 +1,7 @@ //! Structured keys. use std::borrow::Borrow; -use std::cmp; use std::fmt; -use std::hash; /// A type that can be converted into a [`Key`](struct.Key.html). pub trait ToKey { @@ -33,7 +31,7 @@ impl ToKey for str { } /// A key in a structured key-value pair. -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Key<'k> { key: &'k str, } @@ -41,7 +39,7 @@ pub struct Key<'k> { impl<'k> Key<'k> { /// Get a key from a borrowed string. pub fn from_str(key: &'k str) -> Self { - Key { key: key } + Key { key } } /// Get a borrowed string from this key. @@ -50,47 +48,12 @@ impl<'k> Key<'k> { } } -impl<'k> fmt::Debug for Key<'k> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.key.fmt(f) - } -} - impl<'k> fmt::Display for Key<'k> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.key.fmt(f) } } -impl<'k> hash::Hash for Key<'k> { - fn hash(&self, state: &mut H) - where - H: hash::Hasher, - { - self.as_str().hash(state) - } -} - -impl<'k, 'ko> PartialEq> for Key<'k> { - fn eq(&self, other: &Key<'ko>) -> bool { - self.as_str().eq(other.as_str()) - } -} - -impl<'k> Eq for Key<'k> {} - -impl<'k, 'ko> PartialOrd> for Key<'k> { - fn partial_cmp(&self, other: &Key<'ko>) -> Option { - self.as_str().partial_cmp(other.as_str()) - } -} - -impl<'k> Ord for Key<'k> { - fn cmp(&self, other: &Self) -> cmp::Ordering { - self.as_str().cmp(other.as_str()) - } -} - impl<'k> AsRef for Key<'k> { fn as_ref(&self) -> &str { self.as_str() diff --git a/src/lib.rs b/src/lib.rs index 7043a6f12..2127f4805 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -421,7 +421,7 @@ static LEVEL_PARSE_ERROR: &str = /// [`log!`](macro.log.html), and comparing a `Level` directly to a /// [`LevelFilter`](enum.LevelFilter.html). #[repr(usize)] -#[derive(Copy, Eq, Debug, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] pub enum Level { /// The "error" level. /// @@ -448,20 +448,6 @@ pub enum Level { Trace, } -impl Clone for Level { - #[inline] - fn clone(&self) -> Level { - *self - } -} - -impl PartialEq for Level { - #[inline] - fn eq(&self, other: &Level) -> bool { - *self as usize == *other as usize - } -} - impl PartialEq for Level { #[inline] fn eq(&self, other: &LevelFilter) -> bool { @@ -469,65 +455,11 @@ impl PartialEq for Level { } } -impl PartialOrd for Level { - #[inline] - fn partial_cmp(&self, other: &Level) -> Option { - Some(self.cmp(other)) - } - - #[inline] - fn lt(&self, other: &Level) -> bool { - (*self as usize) < *other as usize - } - - #[inline] - fn le(&self, other: &Level) -> bool { - *self as usize <= *other as usize - } - - #[inline] - fn gt(&self, other: &Level) -> bool { - *self as usize > *other as usize - } - - #[inline] - fn ge(&self, other: &Level) -> bool { - *self as usize >= *other as usize - } -} - impl PartialOrd for Level { #[inline] fn partial_cmp(&self, other: &LevelFilter) -> Option { Some((*self as usize).cmp(&(*other as usize))) } - - #[inline] - fn lt(&self, other: &LevelFilter) -> bool { - (*self as usize) < *other as usize - } - - #[inline] - fn le(&self, other: &LevelFilter) -> bool { - *self as usize <= *other as usize - } - - #[inline] - fn gt(&self, other: &LevelFilter) -> bool { - *self as usize > *other as usize - } - - #[inline] - fn ge(&self, other: &LevelFilter) -> bool { - *self as usize >= *other as usize - } -} - -impl Ord for Level { - #[inline] - fn cmp(&self, other: &Level) -> cmp::Ordering { - (*self as usize).cmp(&(*other as usize)) - } } fn ok_or(t: Option, e: E) -> Result { @@ -637,7 +569,7 @@ impl Level { /// [`max_level()`]: fn.max_level.html /// [`set_max_level`]: fn.set_max_level.html #[repr(usize)] -#[derive(Copy, Eq, Debug, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] pub enum LevelFilter { /// A level lower than all log levels. Off, @@ -653,22 +585,6 @@ pub enum LevelFilter { Trace, } -// Deriving generates terrible impls of these traits - -impl Clone for LevelFilter { - #[inline] - fn clone(&self) -> LevelFilter { - *self - } -} - -impl PartialEq for LevelFilter { - #[inline] - fn eq(&self, other: &LevelFilter) -> bool { - *self as usize == *other as usize - } -} - impl PartialEq for LevelFilter { #[inline] fn eq(&self, other: &Level) -> bool { @@ -676,65 +592,11 @@ impl PartialEq for LevelFilter { } } -impl PartialOrd for LevelFilter { - #[inline] - fn partial_cmp(&self, other: &LevelFilter) -> Option { - Some(self.cmp(other)) - } - - #[inline] - fn lt(&self, other: &LevelFilter) -> bool { - (*self as usize) < *other as usize - } - - #[inline] - fn le(&self, other: &LevelFilter) -> bool { - *self as usize <= *other as usize - } - - #[inline] - fn gt(&self, other: &LevelFilter) -> bool { - *self as usize > *other as usize - } - - #[inline] - fn ge(&self, other: &LevelFilter) -> bool { - *self as usize >= *other as usize - } -} - impl PartialOrd for LevelFilter { #[inline] fn partial_cmp(&self, other: &Level) -> Option { Some((*self as usize).cmp(&(*other as usize))) } - - #[inline] - fn lt(&self, other: &Level) -> bool { - (*self as usize) < *other as usize - } - - #[inline] - fn le(&self, other: &Level) -> bool { - *self as usize <= *other as usize - } - - #[inline] - fn gt(&self, other: &Level) -> bool { - *self as usize > *other as usize - } - - #[inline] - fn ge(&self, other: &Level) -> bool { - *self as usize >= *other as usize - } -} - -impl Ord for LevelFilter { - #[inline] - fn cmp(&self, other: &LevelFilter) -> cmp::Ordering { - (*self as usize).cmp(&(*other as usize)) - } } impl FromStr for LevelFilter { @@ -1142,6 +1004,12 @@ impl<'a> RecordBuilder<'a> { } } +impl<'a> Default for RecordBuilder<'a> { + fn default() -> Self { + Self::new() + } +} + /// Metadata about a log message. /// /// # Use @@ -1265,6 +1133,12 @@ impl<'a> MetadataBuilder<'a> { } } +impl<'a> Default for MetadataBuilder<'a> { + fn default() -> Self { + Self::new() + } +} + /// A trait encapsulating the operations required of a logger. pub trait Log: Sync + Send { /// Determines if a log message with the specified metadata would be @@ -1307,10 +1181,10 @@ where } fn log(&self, record: &Record) { - (**self).log(record) + (**self).log(record); } fn flush(&self) { - (**self).flush() + (**self).flush(); } } @@ -1355,7 +1229,7 @@ where /// Note that `Trace` is the maximum level, because it provides the maximum amount of detail in the emitted logs. #[inline] pub fn set_max_level(level: LevelFilter) { - MAX_LOG_LEVEL_FILTER.store(level as usize, Ordering::Relaxed) + MAX_LOG_LEVEL_FILTER.store(level as usize, Ordering::Relaxed); } /// Returns the current maximum log level. @@ -1546,7 +1420,7 @@ impl error::Error for SetLoggerError {} /// /// [`from_str`]: https://doc.rust-lang.org/std/str/trait.FromStr.html#tymethod.from_str #[allow(missing_copy_implementations)] -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct ParseLevelError(()); impl fmt::Display for ParseLevelError { diff --git a/tests/filters.rs b/tests/filters.rs index 66d6f8cd0..fbbc435d8 100644 --- a/tests/filters.rs +++ b/tests/filters.rs @@ -69,15 +69,15 @@ fn main() { fn test(a: &State, filter: LevelFilter) { log::set_max_level(filter); error!(""); - last(&a, t(Level::Error, filter)); + last(a, t(Level::Error, filter)); warn!(""); - last(&a, t(Level::Warn, filter)); + last(a, t(Level::Warn, filter)); info!(""); - last(&a, t(Level::Info, filter)); + last(a, t(Level::Info, filter)); debug!(""); - last(&a, t(Level::Debug, filter)); + last(a, t(Level::Debug, filter)); trace!(""); - last(&a, t(Level::Trace, filter)); + last(a, t(Level::Trace, filter)); fn t(lvl: Level, filter: LevelFilter) -> Option { if lvl <= filter { diff --git a/tests/macros.rs b/tests/macros.rs index 22b7306fd..c48b16d26 100644 --- a/tests/macros.rs +++ b/tests/macros.rs @@ -75,18 +75,14 @@ fn named_args() { #[test] fn enabled() { for lvl in log::Level::iter() { - let _enabled = if log_enabled!(target: "my_target", lvl) { - true - } else { - false - }; + let _enabled = log_enabled!(target: "my_target", lvl); } } #[test] fn expr() { for lvl in log::Level::iter() { - let _ = log!(lvl, "hello"); + log!(lvl, "hello"); } } From b4a064c49f7b7ed6b0501347cada87c447db9964 Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Tue, 31 May 2022 14:12:46 +0200 Subject: [PATCH 3/4] add clippy to github workflow --- .github/workflows/main.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 860e9dfe9..2f6eeebe0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -60,6 +60,18 @@ jobs: rustup component add rustfmt - run: cargo fmt -- --check + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - name: Install Rust + run: | + rustup update stable --no-self-update + rustup default stable + rustup component add clippy + - run: cargo clippy --verbose + features: name: Feature check runs-on: ubuntu-latest From e68fa19bc15af13b0d5f6522fe80d9a6c6c0771c Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Wed, 1 Jun 2022 07:44:33 +0200 Subject: [PATCH 4/4] clarify comment about derive impls Co-authored-by: Ashley Mannix --- src/kv/key.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/kv/key.rs b/src/kv/key.rs index eda5288a3..a35338dcc 100644 --- a/src/kv/key.rs +++ b/src/kv/key.rs @@ -31,6 +31,8 @@ impl ToKey for str { } /// A key in a structured key-value pair. +// These impls must only be based on the as_str() representation of the key +// If a new field (such as an optional index) is added to the key they must not affect comparison #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Key<'k> { key: &'k str,