chore!: Enable and fix a few pedantic and nursery clippy lints (#1186)

* chore: First round of pedantic clippy lints

* clippy::too_many_lines

* clippy::implicit_hasher

* Move clippy::doc_link_with_quotes

* clippy::use_self

* clippy::or_fun_call

* clippy::derive_partial_eq_without_eq

* Also run for all other crates

* changelog

* Move restriction lints

* Silence explicit_deref_methods at calling location

* Document why we can't deny needless_continue

* Fix new code

* Fix new code
gp

* Update CHANGELOG.md

* Fix lints in new code

* Apply suggestions from code review

Co-authored-by: Xenia <xeniafischer@hotmail.de>

---------

Co-authored-by: Xenia <xeniafischer@hotmail.de>

Author: sbernauer

Cargo.toml

@@ -90,6 +90,30 @@ x509-cert = { version = "0.2.5", features = ["builder"] }
 zeroize = "1.8.1"
 
 [workspace.lints.clippy]
+# Enable all pedantic lints (with lower priority so individual lints can override)
+pedantic = { level = "deny", priority = -1 }
+
+# Pedantic lints we don't enforce (yet)
+doc_markdown = "allow"
+missing_errors_doc = "allow"
+must_use_candidate = "allow"
+return_self_not_must_use = "allow"
+missing_panics_doc = "allow"
+cast_possible_truncation = "allow"
+float_cmp = "allow"
+cast_sign_loss = "allow"
+cast_precision_loss = "allow"
+unchecked_time_subtraction = "allow"
+# We should be able to deny this, but it lint's on code generated by darling, raised https://github.com/TedDriggs/darling/pull/429
+needless_continue = "allow"
+
+# Additional nursery lints we enforce
+use_self = "deny"
+or_fun_call = "deny"
+derive_partial_eq_without_eq = "deny"
+unnecessary_struct_initialization = "deny"
+
+# Additional restriction lints we enforce
 unwrap_in_result = "deny"
 unwrap_used = "deny"
 panic = "deny"

crates/k8s-version/src/api_version/darling.rs

@@ -18,17 +18,17 @@ mod test {
     use super::*;
     use crate::{Level, Version};
 
-    fn parse_meta(tokens: proc_macro2::TokenStream) -> ::std::result::Result<syn::Meta, String> {
+    fn parse_meta(tokens: &proc_macro2::TokenStream) -> syn::Meta {
         let attribute: syn::Attribute = syn::parse_quote!(#[#tokens]);
-        Ok(attribute.meta)
+        attribute.meta
     }
 
     #[rstest]
     #[case(quote!(ignore = "extensions/v1beta1"), ApiVersion { group: Some("extensions".parse().unwrap()), version: Version { major: 1, level: Some(Level::Beta(1)) } })]
     #[case(quote!(ignore = "v1beta1"), ApiVersion { group: None, version: Version { major: 1, level: Some(Level::Beta(1)) } })]
     #[case(quote!(ignore = "v1"), ApiVersion { group: None, version: Version { major: 1, level: None } })]
     fn from_meta(#[case] input: proc_macro2::TokenStream, #[case] expected: ApiVersion) {
-        let meta = parse_meta(input).expect("valid attribute tokens");
+        let meta = parse_meta(&input);
         let api_version = ApiVersion::from_meta(&meta).expect("version must parse from attribute");
         assert_eq!(api_version, expected);
     }

crates/k8s-version/src/api_version/mod.rs

@@ -12,7 +12,7 @@ mod darling;
 
 /// Error variants which can be encountered when creating a new [`ApiVersion`]
 /// from unparsed input.
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, PartialEq, Eq, Snafu)]
 pub enum ParseApiVersionError {
     #[snafu(display("failed to parse version"))]
     ParseVersion { source: ParseVersionError },
@@ -87,10 +87,7 @@ impl ApiVersion {
     /// Try to create a new Kubernetes API version based on the unvalidated
     /// `group` string.
     pub fn try_new(group: Option<&str>, version: Version) -> Result<Self, ParseApiVersionError> {
-        let group = group
-            .map(|g| g.parse())
-            .transpose()
-            .context(ParseGroupSnafu)?;
+        let group = group.map(str::parse).transpose().context(ParseGroupSnafu)?;
 
         Ok(Self { group, version })
     }

crates/k8s-version/src/group.rs

@@ -12,7 +12,7 @@ static API_GROUP_REGEX: LazyLock<Regex> = LazyLock::new(|| {
 
 /// Error variants which can be encountered when creating a new [`Group`] from
 /// unparsed input.
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, PartialEq, Eq, Snafu)]
 pub enum ParseGroupError {
     #[snafu(display("group must not be empty"))]
     Empty,

crates/k8s-version/src/level/darling.rs

@@ -17,17 +17,17 @@ mod tests {
 
     use super::*;
 
-    fn parse_meta(tokens: proc_macro2::TokenStream) -> ::std::result::Result<syn::Meta, String> {
+    fn parse_meta(tokens: &proc_macro2::TokenStream) -> syn::Meta {
         let attribute: syn::Attribute = syn::parse_quote!(#[#tokens]);
-        Ok(attribute.meta)
+        attribute.meta
     }
 
     #[rstest]
     #[case(quote!(ignore = "alpha12"), Level::Alpha(12))]
     #[case(quote!(ignore = "alpha1"), Level::Alpha(1))]
     #[case(quote!(ignore = "beta1"), Level::Beta(1))]
     fn from_meta(#[case] input: proc_macro2::TokenStream, #[case] expected: Level) {
-        let meta = parse_meta(input).expect("valid attribute tokens");
+        let meta = parse_meta(&input);
         let version = Level::from_meta(&meta).expect("level must parse from attribute");
         assert_eq!(version, expected);
     }

crates/k8s-version/src/level/mod.rs

@@ -22,7 +22,7 @@ static LEVEL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
 
 /// Error variants which can be encountered when creating a new [`Level`] from
 /// unparsed input.
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, PartialEq, Eq, Snafu)]
 pub enum ParseLevelError {
     #[snafu(display("invalid level format, expected alpha<VERSION>|beta<VERSION>"))]
     InvalidFormat,
@@ -87,13 +87,13 @@ impl PartialOrd for Level {
 impl Ord for Level {
     fn cmp(&self, other: &Self) -> Ordering {
         match self {
-            Level::Alpha(lhs) => match other {
-                Level::Alpha(rhs) => lhs.cmp(rhs),
-                Level::Beta(_) => Ordering::Less,
+            Self::Alpha(lhs) => match other {
+                Self::Alpha(rhs) => lhs.cmp(rhs),
+                Self::Beta(_) => Ordering::Less,
             },
-            Level::Beta(lhs) => match other {
-                Level::Alpha(_) => Ordering::Greater,
-                Level::Beta(rhs) => lhs.cmp(rhs),
+            Self::Beta(lhs) => match other {
+                Self::Alpha(_) => Ordering::Greater,
+                Self::Beta(rhs) => lhs.cmp(rhs),
             },
         }
     }
@@ -103,12 +103,12 @@ impl<T> Add<T> for Level
 where
     T: Into<u64>,
 {
-    type Output = Level;
+    type Output = Self;
 
     fn add(self, rhs: T) -> Self::Output {
         match self {
-            Level::Alpha(lhs) => Level::Alpha(lhs + rhs.into()),
-            Level::Beta(lhs) => Level::Beta(lhs + rhs.into()),
+            Self::Alpha(lhs) => Self::Alpha(lhs + rhs.into()),
+            Self::Beta(lhs) => Self::Beta(lhs + rhs.into()),
         }
     }
 }
@@ -119,8 +119,7 @@ where
 {
     fn add_assign(&mut self, rhs: T) {
         match self {
-            Level::Alpha(lhs) => *lhs + rhs.into(),
-            Level::Beta(lhs) => *lhs + rhs.into(),
+            Self::Alpha(lhs) | Self::Beta(lhs) => *lhs + rhs.into(),
         };
     }
 }
@@ -129,12 +128,12 @@ impl<T> Sub<T> for Level
 where
     T: Into<u64>,
 {
-    type Output = Level;
+    type Output = Self;
 
     fn sub(self, rhs: T) -> Self::Output {
         match self {
-            Level::Alpha(lhs) => Level::Alpha(lhs - rhs.into()),
-            Level::Beta(lhs) => Level::Beta(lhs - rhs.into()),
+            Self::Alpha(lhs) => Self::Alpha(lhs - rhs.into()),
+            Self::Beta(lhs) => Self::Beta(lhs - rhs.into()),
         }
     }
 }
@@ -145,17 +144,16 @@ where
 {
     fn sub_assign(&mut self, rhs: T) {
         match self {
-            Level::Alpha(lhs) => *lhs - rhs.into(),
-            Level::Beta(lhs) => *lhs - rhs.into(),
+            Self::Alpha(lhs) | Self::Beta(lhs) => *lhs - rhs.into(),
         };
     }
 }
 
 impl Display for Level {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Level::Alpha(alpha) => write!(f, "alpha{}", alpha),
-            Level::Beta(beta) => write!(f, "beta{}", beta),
+            Self::Alpha(alpha) => write!(f, "alpha{alpha}"),
+            Self::Beta(beta) => write!(f, "beta{beta}"),
         }
     }
 }
@@ -181,11 +179,11 @@ mod test {
 
     #[apply(ord_cases)]
     fn ord(input: Level, other: Level, expected: Ordering) {
-        assert_eq!(input.cmp(&other), expected)
+        assert_eq!(input.cmp(&other), expected);
     }
 
     #[apply(ord_cases)]
     fn partial_ord(input: Level, other: Level, expected: Ordering) {
-        assert_eq!(input.partial_cmp(&other), Some(expected))
+        assert_eq!(input.partial_cmp(&other), Some(expected));
     }
 }

crates/k8s-version/src/version/darling.rs

@@ -18,9 +18,9 @@ mod tests {
     use super::*;
     use crate::Level;
 
-    fn parse_meta(tokens: proc_macro2::TokenStream) -> ::std::result::Result<syn::Meta, String> {
+    fn parse_meta(tokens: &proc_macro2::TokenStream) -> syn::Meta {
         let attribute: syn::Attribute = syn::parse_quote!(#[#tokens]);
-        Ok(attribute.meta)
+        attribute.meta
     }
 
     #[cfg(feature = "darling")]
@@ -30,7 +30,7 @@ mod tests {
     #[case(quote!(ignore = "v1beta1"), Version { major: 1, level: Some(Level::Beta(1)) })]
     #[case(quote!(ignore = "v1"), Version { major: 1, level: None })]
     fn from_meta(#[case] input: proc_macro2::TokenStream, #[case] expected: Version) {
-        let meta = parse_meta(input).expect("valid attribute tokens");
+        let meta = parse_meta(&input);
         let version = Version::from_meta(&meta).expect("version must parse from attribute");
         assert_eq!(version, expected);
     }

crates/k8s-version/src/version/mod.rs

@@ -18,7 +18,7 @@ static VERSION_REGEX: LazyLock<Regex> = LazyLock::new(|| {
 
 /// Error variants which can be encountered when creating a new [`Version`] from
 /// unparsed input.
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, PartialEq, Eq, Snafu)]
 pub enum ParseVersionError {
     #[snafu(display(
         "invalid version format. Input is empty, contains non-ASCII characters or contains more than 63 characters"
@@ -153,16 +153,16 @@ mod test {
     #[case("", ParseVersionError::InvalidFormat)]
     fn invalid_version(#[case] input: &str, #[case] error: ParseVersionError) {
         let err = Version::from_str(input).expect_err("invalid Kubernetes version");
-        assert_eq!(err, error)
+        assert_eq!(err, error);
     }
 
     #[apply(ord_cases)]
     fn ord(input: Version, other: Version, expected: Ordering) {
-        assert_eq!(input.cmp(&other), expected)
+        assert_eq!(input.cmp(&other), expected);
     }
 
     #[apply(ord_cases)]
     fn partial_ord(input: Version, other: Version, expected: Ordering) {
-        assert_eq!(input.partial_cmp(&other), Some(expected))
+        assert_eq!(input.partial_cmp(&other), Some(expected));
     }
 }

crates/stackable-certs/src/ca/mod.rs

@@ -415,7 +415,7 @@ where
     /// Kubernetes [`Secret`]. Common keys are `ca.crt` and `ca.key`.
     #[instrument(name = "create_certificate_authority_from_k8s_secret", skip(secret))]
     pub fn from_secret(
-        secret: Secret,
+        secret: &Secret,
         key_certificate: &str,
         key_private_key: &str,
     ) -> Result<Self, SecretError<S::Error>> {
@@ -424,27 +424,27 @@ where
         }
 
         let data = secret.data.as_ref().with_context(|| NoSecretDataSnafu {
-            secret: ObjectRef::from_obj(&secret),
+            secret: ObjectRef::from_obj(secret),
         })?;
 
         debug!("retrieving certificate data from secret via key {key_certificate:?}");
         let certificate_data =
             data.get(key_certificate)
                 .with_context(|| NoCertificateDataSnafu {
-                    secret: ObjectRef::from_obj(&secret),
+                    secret: ObjectRef::from_obj(secret),
                 })?;
 
         let certificate = x509_cert::Certificate::load_pem_chain(&certificate_data.0)
             .with_context(|_| ReadChainSnafu {
-                secret: ObjectRef::from_obj(&secret),
+                secret: ObjectRef::from_obj(secret),
             })?
             .remove(0);
 
         debug!("retrieving private key data from secret via key {key_certificate:?}");
         let private_key_data =
             data.get(key_private_key)
                 .with_context(|| NoPrivateKeyDataSnafu {
-                    secret: ObjectRef::from_obj(&secret),
+                    secret: ObjectRef::from_obj(secret),
                 })?;
 
         let private_key_data =
@@ -472,15 +472,15 @@ where
         key_private_key: &str,
         client: Client,
     ) -> Result<Self, SecretError<S::Error>> {
-        let secret_api = Api::namespaced(client, &secret_ref.namespace);
+        let secret_api: Api<Secret> = Api::namespaced(client, &secret_ref.namespace);
         let secret = secret_api
             .get(&secret_ref.name)
             .await
             .with_context(|_| GetSecretSnafu {
                 secret_ref: secret_ref.to_owned(),
             })?;
 
-        Self::from_secret(secret, key_certificate, key_private_key)
+        Self::from_secret(&secret, key_certificate, key_private_key)
     }
 
     /// Returns the ca certificate.

crates/stackable-certs/src/keys/ecdsa.rs

@@ -10,7 +10,7 @@ use crate::keys::CertificateKeypair;
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-#[derive(Debug, PartialEq, Snafu)]
+#[derive(Debug, PartialEq, Eq, Snafu)]
 pub enum Error {
     #[snafu(context(false))]
     SerializeKeyToPem { source: x509_cert::spki::Error },