chore: Apply some suggestions by -Dclippy::pedantic (#514)

martin-g · Kriskras99 · web-flow · commit e2901d3cb644 · 2026-03-31T10:11:25.000+03:00
* chore: Apply some suggestions by -Dclippy::pedantic `-Dclippy::pedantic` is not added to test-lang-rust-clippy.yml because not all sugestions could be applied at the moment. Added `-Dclippy::cargo` to CI. `multiple_crate_versions` is needed because at the moment there are two versions of `heck` and `hashbrown` coming as transitive dependencies * Simplify clear_log_messages() helper method Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Move clippy settings to Cargo.toml Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Simplify the docstrings for logging related helper methods Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Improve assert_not_logged() to check all messages, not just the last one Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Add the clippy settings which were fixed to make clippy::pedantic happy But listing them explicitly here finds many more issues. Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Fix docstring tests Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Disable the extra lints now https://github.com/apache/avro-rs/actions/runs/23512928423/job/68438311919?pr=514 They will be re-enabled once #512 is merged. To avoid merge conflicts by editing many files in two PRs Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Improve assert_logged() to remove only one matching log message, not all Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> * Simplify by using `impl AsRef<[]>` Suggested-by: Kriskras99 * Remove `#[inline]`s Clippy asked to add them. Kriskras99 asked to remove them. * Update the docstring for assert_not_logged Co-authored-by: Kriskras99 <github@kriskras99.nl> * Hide imports from the user Co-authored-by: Kriskras99 <github@kriskras99.nl> --------- Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Co-authored-by: Kriskras99 <github@kriskras99.nl>
diff --git a/.github/workflows/test-lang-rust-clippy.yml b/.github/workflows/test-lang-rust-clippy.yml
@@ -19,9 +19,9 @@ name: Rust Clippy Check
 on:
   workflow_dispatch:
   push:
-    branches: [ main ]
+    branches: [main]
   pull_request:
-    branches: [ main ]
+    branches: [main]
 
 permissions:
   contents: read
@@ -39,11 +39,11 @@ jobs:
     strategy:
       matrix:
         rust:
-          - 'stable'
+          - "stable"
     steps:
       - uses: actions/checkout@v6
       - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable
         with:
           toolchain: ${{ matrix.rust }}
           components: clippy
-      - run: cargo clippy --all-features --all-targets
+      - run: cargo clippy --all-features --all-targets --workspace
diff --git a/Cargo.toml b/Cargo.toml
@@ -60,3 +60,12 @@ clippy.doc_markdown = "warn"
 #clippy.missing_errors_doc = "warn"
 #clippy.missing_panics_doc = "warn"
 rust.unexpected_cfgs = { level = "warn", check-cfg = ['cfg(nightly)'] }
+clippy.cargo = { level = "warn", priority = -1 }
+clippy.multiple_crate_versions = "allow"
+# clippy.needless_raw_string_hashes = "warn"
+# clippy.missing_panics_doc = "warn"
+# clippy.semicolon_if_nothing_returned = "warn"
+# clippy.manual_assert = "warn"
+# clippy.enum_glob_use = "warn"
+# clippy.needless_pass_by_value = "warn"
+# clippy.single_match_else = "warn"
diff --git a/avro/src/serde/ser_schema/mod.rs b/avro/src/serde/ser_schema/mod.rs
@@ -1645,15 +1645,13 @@ impl<'s, W: Write> SchemaAwareWriteSerializer<'s, W> {
             Schema::Union(union_schema) => {
                 for (i, variant_schema) in union_schema.schemas.iter().enumerate() {
                     match variant_schema {
-                        Schema::Record(inner) => {
-                            if inner.fields.len() == len {
-                                encode_int(i as i32, &mut *self.writer)?;
-                                return self.serialize_tuple_struct_with_schema(
-                                    name,
-                                    len,
-                                    variant_schema,
-                                );
-                            }
+                        Schema::Record(inner) if inner.fields.len() == len => {
+                            encode_int(i as i32, &mut *self.writer)?;
+                            return self.serialize_tuple_struct_with_schema(
+                                name,
+                                len,
+                                variant_schema,
+                            );
                         }
                         Schema::Array(_) | Schema::Ref { name: _ } => {
                             encode_int(i as i32, &mut *self.writer)?;
diff --git a/avro_derive/build.rs b/avro_derive/build.rs
@@ -18,7 +18,7 @@
 //! Set the `nightly` cfg value on nightly toolchains.
 //!
 //! We would prefer to just do `#![rustversion::attr(nightly, feature(proc_macro_diagnostic)]`
-//! but that's currently not possible, see <https://github.com/dtolnay/rustversion/issues/8>
+//! but that's currently not possible, see <https://github.com/dtolnay/rustversion/issues/8>.
 
 #[rustversion::nightly]
 fn main() {
diff --git a/avro_derive/src/attributes/avro.rs b/avro_derive/src/attributes/avro.rs
@@ -66,14 +66,14 @@ impl ContainerAttributes {
                 span,
                 r#"`#[avro(name = "...")]` is deprecated."#,
                 r#"Use `#[serde(rename = "...")]` instead."#,
-            )
+            );
         }
         if self.rename_all != RenameRule::None {
             super::warn(
                 span,
                 r#"`#[avro(rename_all = "..")]` is deprecated"#,
                 r#"Use `#[serde(rename_all = "..")]` instead"#,
-            )
+            );
         }
     }
 }
@@ -98,7 +98,7 @@ impl VariantAttributes {
                 span,
                 r#"`#[avro(rename = "..")]` is deprecated"#,
                 r#"Use `#[serde(rename = "..")]` instead"#,
-            )
+            );
         }
     }
 }
@@ -177,28 +177,28 @@ impl FieldAttributes {
                 span,
                 r#"`#[avro(alias = "..")]` is deprecated"#,
                 r#"Use `#[serde(alias = "..")]` instead"#,
-            )
+            );
         }
         if self.rename.is_some() {
             super::warn(
                 span,
                 r#"`#[avro(rename = "..")]` is deprecated"#,
                 r#"Use `#[serde(rename = "..")]` instead"#,
-            )
+            );
         }
         if self.skip {
             super::warn(
                 span,
                 "`#[avro(skip)]` is deprecated",
                 "Use `#[serde(skip)]` instead",
-            )
+            );
         }
         if self.flatten {
             super::warn(
                 span,
                 "`#[avro(flatten)]` is deprecated",
                 "Use `#[serde(flatten)]` instead",
-            )
+            );
         }
     }
 }
diff --git a/avro_derive/src/attributes/mod.rs b/avro_derive/src/attributes/mod.rs
@@ -202,7 +202,7 @@ pub enum With {
 impl With {
     fn from_avro_and_serde(
         avro: &avro::With,
-        serde: &Option<String>,
+        serde: Option<&String>,
         span: Span,
     ) -> Result<Self, syn::Error> {
         match &avro {
@@ -327,7 +327,7 @@ impl FieldOptions {
                 "`#[serde(skip_serializing)]` and `#[serde(skip_serializing_if)]` are incompatible with `#[avro(default = false)]`"
             ));
         }
-        let with = match With::from_avro_and_serde(&avro.with, &serde.with, span) {
+        let with = match With::from_avro_and_serde(&avro.with, serde.with.as_ref(), span) {
             Ok(with) => with,
             Err(error) => {
                 errors.push(error);
@@ -389,7 +389,7 @@ fn darling_to_syn(e: darling::Error) -> Vec<syn::Error> {
 fn warn(span: Span, message: &str, help: &str) {
     proc_macro::Diagnostic::spanned(span.unwrap(), proc_macro::Level::Warning, message)
         .help(help)
-        .emit()
+        .emit();
 }
 
 #[cfg(not(nightly))]
diff --git a/avro_derive/src/case.rs b/avro_derive/src/case.rs
@@ -21,7 +21,10 @@
 use darling::FromMeta;
 use syn::Lit;
 
-use self::RenameRule::*;
+use self::RenameRule::{
+    CamelCase, KebabCase, LowerCase, None, PascalCase, ScreamingKebabCase, ScreamingSnakeCase,
+    SnakeCase, UpperCase,
+};
 use std::fmt::{self, Debug, Display};
 
 /// The different possible ways to change case of fields in a struct, or variants in an enum.
@@ -111,7 +114,7 @@ impl RenameRule {
     pub fn apply_to_field(self, field: &str) -> String {
         match self {
             None | LowerCase | SnakeCase => field.to_owned(),
-            UpperCase => field.to_ascii_uppercase(),
+            UpperCase | ScreamingSnakeCase => field.to_ascii_uppercase(),
             PascalCase => {
                 let mut pascal = String::new();
                 let mut capitalize = true;
@@ -131,7 +134,6 @@ impl RenameRule {
                 let pascal = PascalCase.apply_to_field(field);
                 pascal[..1].to_ascii_lowercase() + &pascal[1..]
             }
-            ScreamingSnakeCase => field.to_ascii_uppercase(),
             KebabCase => field.replace('_', "-"),
             ScreamingKebabCase => ScreamingSnakeCase.apply_to_field(field).replace('_', "-"),
         }
diff --git a/avro_derive/src/enums/mod.rs b/avro_derive/src/enums/mod.rs
@@ -24,7 +24,7 @@ use syn::{Attribute, DataEnum, Fields, Meta};
 /// Generate a schema definition for a enum.
 pub fn get_data_enum_schema_def(
     container_attrs: &NamedTypeOptions,
-    data_enum: DataEnum,
+    data_enum: &DataEnum,
     ident_span: Span,
 ) -> Result<TokenStream, Vec<syn::Error>> {
     if data_enum.variants.iter().all(|v| Fields::Unit == v.fields) {
diff --git a/avro_derive/src/enums/plain.rs b/avro_derive/src/enums/plain.rs
@@ -26,13 +26,13 @@ use syn::{DataEnum, Fields};
 
 pub fn schema_def(
     container_attrs: &NamedTypeOptions,
-    data_enum: DataEnum,
+    data_enum: &DataEnum,
     ident_span: Span,
 ) -> Result<TokenStream, Vec<syn::Error>> {
     let doc = preserve_optional(container_attrs.doc.as_ref());
     let enum_aliases = aliases(&container_attrs.aliases);
     if data_enum.variants.iter().all(|v| Fields::Unit == v.fields) {
-        let default_value = default_enum_variant(&data_enum, ident_span)?;
+        let default_value = default_enum_variant(data_enum, ident_span)?;
         let default = preserve_optional(default_value);
         let mut symbols = Vec::new();
         for variant in &data_enum.variants {
diff --git a/avro_derive/src/lib.rs b/avro_derive/src/lib.rs
@@ -68,16 +68,16 @@ fn derive_avro_schema(input: DeriveInput) -> Result<TokenStream, Vec<syn::Error>
                 let (schema_def, record_fields) =
                     get_struct_schema_def(&named_type_options, data_struct, input.ident.span())?;
                 (
-                    handle_named_schemas(named_type_options.name, schema_def),
+                    handle_named_schemas(&named_type_options.name, &schema_def),
                     record_fields,
                 )
             };
             Ok(create_trait_definition(
-                input.ident,
+                &input.ident,
                 &input.generics,
-                get_schema_impl,
-                get_record_fields_impl,
-                named_type_options.default,
+                &get_schema_impl,
+                &get_record_fields_impl,
+                &named_type_options.default,
             ))
         }
         syn::Data::Enum(data_enum) => {
@@ -89,14 +89,14 @@ fn derive_avro_schema(input: DeriveInput) -> Result<TokenStream, Vec<syn::Error>
                 )]);
             }
             let schema_def =
-                get_data_enum_schema_def(&named_type_options, data_enum, input.ident.span())?;
-            let inner = handle_named_schemas(named_type_options.name, schema_def);
+                get_data_enum_schema_def(&named_type_options, &data_enum, input.ident.span())?;
+            let inner = handle_named_schemas(&named_type_options.name, &schema_def);
             Ok(create_trait_definition(
-                input.ident,
+                &input.ident,
                 &input.generics,
-                inner,
-                quote! { ::std::option::Option::None },
-                named_type_options.default,
+                &inner,
+                &quote! { ::std::option::Option::None },
+                &named_type_options.default,
             ))
         }
         syn::Data::Union(_) => Err(vec![syn::Error::new(
@@ -108,11 +108,11 @@ fn derive_avro_schema(input: DeriveInput) -> Result<TokenStream, Vec<syn::Error>
 
 /// Generate the trait definition with the correct generics
 fn create_trait_definition(
-    ident: Ident,
+    ident: &Ident,
     generics: &Generics,
-    get_schema_impl: TokenStream,
-    get_record_fields_impl: TokenStream,
-    field_default_impl: TokenStream,
+    get_schema_impl: &TokenStream,
+    get_record_fields_impl: &TokenStream,
+    field_default_impl: &TokenStream,
 ) -> TokenStream {
     let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
     quote! {
@@ -134,7 +134,7 @@ fn create_trait_definition(
 }
 
 /// Generate the code to check `named_schemas` if this schema already exist
-fn handle_named_schemas(full_schema_name: String, schema_def: TokenStream) -> TokenStream {
+fn handle_named_schemas(full_schema_name: &str, schema_def: &TokenStream) -> TokenStream {
     quote! {
         let name = ::apache_avro::schema::Name::new_with_enclosing_namespace(#full_schema_name, enclosing_namespace).expect(concat!("Unable to parse schema name ", #full_schema_name));
         if named_schemas.contains(&name) {
@@ -448,15 +448,16 @@ fn type_to_field_default_expr(ty: &Type) -> Result<TokenStream, Vec<syn::Error>>
 }
 
 /// Stolen from serde
-fn to_compile_errors(errors: Vec<syn::Error>) -> proc_macro2::TokenStream {
-    let compile_errors = errors.iter().map(syn::Error::to_compile_error);
+fn to_compile_errors(errors: impl AsRef<[syn::Error]>) -> proc_macro2::TokenStream {
+    let compile_errors = errors.as_ref().iter().map(syn::Error::to_compile_error);
     quote!(#(#compile_errors)*)
 }
 
 fn preserve_optional(op: Option<impl quote::ToTokens>) -> TokenStream {
-    match op {
-        Some(tt) => quote! {::std::option::Option::Some(#tt.into())},
-        None => quote! {::std::option::Option::None},
+    if let Some(tt) = op {
+        quote! {::std::option::Option::Some(#tt.into())}
+    } else {
+        quote! {::std::option::Option::None}
     }
 }
 
diff --git a/avro_test_helper/src/data.rs b/avro_test_helper/src/data.rs
@@ -37,7 +37,7 @@ pub const PRIMITIVE_EXAMPLES: &[(&str, bool)] = &[
     (r#""double""#, true),
     (r#"{"type": "double"}"#, true),
     (r#""true""#, false),
-    (r#"true"#, false),
+    ("true", false),
     (r#"{"no_type": "test"}"#, false),
     (r#"{"type": "panther"}"#, false),
 ];
diff --git a/avro_test_helper/src/lib.rs b/avro_test_helper/src/lib.rs
@@ -15,9 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
+pub mod data;
+pub mod logger;
+
+use core::any::type_name;
+use core::cell::RefCell;
+use core::fmt::{Debug, Display};
 #[cfg(not(target_arch = "wasm32"))]
 use ctor::{ctor, dtor};
-use std::cell::RefCell;
 
 thread_local! {
     // The unit tests run in parallel
@@ -26,9 +31,6 @@ thread_local! {
     pub(crate) static LOG_MESSAGES: RefCell<Vec<String>> = const { RefCell::new(Vec::new()) };
 }
 
-pub mod data;
-pub mod logger;
-
 #[cfg(not(target_arch = "wasm32"))]
 #[ctor]
 fn before_all() {
@@ -56,13 +58,13 @@ pub struct TestError;
 /// A converter of any error into [`TestError`].
 ///
 /// It is used to print better error messages in the tests.
-/// Borrowed from <https://bluxte.net/musings/2023/01/08/improving_failure_messages_rust_tests/>
+/// Borrowed from <https://bluxte.net/musings/2023/01/08/improving_failure_messages_rust_tests/>.
 // The Display bound is needed so that the `From` implementation doesn't
 // apply to `TestError` itself.
-impl<Err: std::fmt::Display + std::fmt::Debug> From<Err> for TestError {
+impl<Err: Display + Debug> From<Err> for TestError {
     #[track_caller]
     fn from(err: Err) -> Self {
-        panic!("{}: {:?}", std::any::type_name::<Err>(), err);
+        panic!("{}: {:?}", type_name::<Err>(), err);
     }
 }
 
@@ -71,4 +73,4 @@ pub type TestResult = Result<(), TestError>;
 /// Does nothing. Just loads the crate.
 /// Should be used in the integration tests, because they do not use [dev-dependencies]
 /// and do not auto-load this crate.
-pub fn init() {}
+pub const fn init() {}
diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs

Original file line number	Diff line number	Diff line change
`@@ -66,14 +66,14 @@ impl ContainerAttributes {`
`66`	`66`	`span,`
`67`	`67`	r#"`#[avro(name = "...")]` is deprecated."#,
`68`	`68`	r#"Use `#[serde(rename = "...")]` instead."#,
`69`		`- )`
	`69`	`+ );`
`70`	`70`	`}`
`71`	`71`	`if self.rename_all != RenameRule::None {`
`72`	`72`	`super::warn(`
`73`	`73`	`span,`
`74`	`74`	r#"`#[avro(rename_all = "..")]` is deprecated"#,
`75`	`75`	r#"Use `#[serde(rename_all = "..")]` instead"#,
`76`		`- )`
	`76`	`+ );`
`77`	`77`	`}`
`78`	`78`	`}`
`79`	`79`	`}`
`@@ -98,7 +98,7 @@ impl VariantAttributes {`
`98`	`98`	`span,`
`99`	`99`	r#"`#[avro(rename = "..")]` is deprecated"#,
`100`	`100`	r#"Use `#[serde(rename = "..")]` instead"#,
`101`		`- )`
	`101`	`+ );`
`102`	`102`	`}`
`103`	`103`	`}`
`104`	`104`	`}`
`@@ -177,28 +177,28 @@ impl FieldAttributes {`
`177`	`177`	`span,`
`178`	`178`	r#"`#[avro(alias = "..")]` is deprecated"#,
`179`	`179`	r#"Use `#[serde(alias = "..")]` instead"#,
`180`		`- )`
	`180`	`+ );`
`181`	`181`	`}`
`182`	`182`	`if self.rename.is_some() {`
`183`	`183`	`super::warn(`
`184`	`184`	`span,`
`185`	`185`	r#"`#[avro(rename = "..")]` is deprecated"#,
`186`	`186`	r#"Use `#[serde(rename = "..")]` instead"#,
`187`		`- )`
	`187`	`+ );`
`188`	`188`	`}`
`189`	`189`	`if self.skip {`
`190`	`190`	`super::warn(`
`191`	`191`	`span,`
`192`	`192`	"`#[avro(skip)]` is deprecated",
`193`	`193`	"Use `#[serde(skip)]` instead",
`194`		`- )`
	`194`	`+ );`
`195`	`195`	`}`
`196`	`196`	`if self.flatten {`
`197`	`197`	`super::warn(`
`198`	`198`	`span,`
`199`	`199`	"`#[avro(flatten)]` is deprecated",
`200`	`200`	"Use `#[serde(flatten)]` instead",
`201`		`- )`
	`201`	`+ );`
`202`	`202`	`}`
`203`	`203`	`}`
`204`	`204`	`}`