From 1bf28f7f1119d45473ee40e577980fc7859391ec Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Mon, 23 Mar 2026 16:50:14 +0200 Subject: [PATCH 01/13] 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 --- .github/workflows/test-lang-rust-clippy.yml | 2 +- avro/src/serde/ser_schema/mod.rs | 16 ++-- avro/tests/append_to_existing.rs | 4 +- avro_derive/build.rs | 2 +- avro_derive/src/attributes/avro.rs | 14 ++-- avro_derive/src/attributes/mod.rs | 6 +- avro_derive/src/case.rs | 8 +- avro_derive/src/enums/mod.rs | 2 +- avro_derive/src/enums/plain.rs | 4 +- avro_derive/src/lib.rs | 43 +++++------ avro_test_helper/src/data.rs | 4 +- avro_test_helper/src/lib.rs | 21 ++++-- avro_test_helper/src/logger.rs | 84 +++++++++++++++++++-- 13 files changed, 144 insertions(+), 66 deletions(-) diff --git a/.github/workflows/test-lang-rust-clippy.yml b/.github/workflows/test-lang-rust-clippy.yml index e5243f5e..33b6dec6 100644 --- a/.github/workflows/test-lang-rust-clippy.yml +++ b/.github/workflows/test-lang-rust-clippy.yml @@ -46,4 +46,4 @@ jobs: with: toolchain: ${{ matrix.rust }} components: clippy - - run: cargo clippy --all-features --all-targets + - run: cargo clippy --all-features --all-targets --workspace -- -Dclippy::cargo -Aclippy::multiple_crate_versions diff --git a/avro/src/serde/ser_schema/mod.rs b/avro/src/serde/ser_schema/mod.rs index fa9382f4..6f29b47d 100644 --- 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/tests/append_to_existing.rs b/avro/tests/append_to_existing.rs index ef5723ea..571c05c3 100644 --- a/avro/tests/append_to_existing.rs +++ b/avro/tests/append_to_existing.rs @@ -46,10 +46,8 @@ fn avro_3630_append_to_an_existing_file() -> TestResult { let new_bytes = writer.into_inner().expect("Cannot get the new bytes"); let reader = Reader::new(&*new_bytes).expect("Cannot read the new bytes"); - let mut i = 1; - for value in reader { + for (i, value) in (1..).zip(reader) { check(&value, i); - i += 1 } Ok(()) diff --git a/avro_derive/build.rs b/avro_derive/build.rs index 40e33af3..e3b221c6 100644 --- 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 +//! but that's currently not possible, see . #[rustversion::nightly] fn main() { diff --git a/avro_derive/src/attributes/avro.rs b/avro_derive/src/attributes/avro.rs index eb11b9e5..e0a1433c 100644 --- 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 index 27c1cbc3..66911d9e 100644 --- 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, + serde: Option<&String>, span: Span, ) -> Result { 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 { 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 index c9585621..9f8eff97 100644 --- 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 index e44f4a1a..1511f65e 100644 --- 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> { 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 index 2832c088..4fb0adbe 100644 --- 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> { 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 index 96f997ea..2fafa384 100644 --- a/avro_derive/src/lib.rs +++ b/avro_derive/src/lib.rs @@ -51,7 +51,7 @@ use crate::{ pub fn proc_macro_derive_avro_schema(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); derive_avro_schema(input) - .unwrap_or_else(to_compile_errors) + .unwrap_or_else(|errs| to_compile_errors(errs.as_slice())) .into() } @@ -68,16 +68,16 @@ fn derive_avro_schema(input: DeriveInput) -> Result 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 )]); } 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, + "e! { ::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 /// 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> } /// Stolen from serde -fn to_compile_errors(errors: Vec) -> proc_macro2::TokenStream { +fn to_compile_errors(errors: &[syn::Error]) -> proc_macro2::TokenStream { let compile_errors = errors.iter().map(syn::Error::to_compile_error); quote!(#(#compile_errors)*) } fn preserve_optional(op: Option) -> 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 index 3713e83e..b0999d70 100644 --- 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), ]; @@ -602,6 +602,7 @@ pub const LOCAL_TIMESTAMPMICROS_LOGICAL_TYPE: &[(&str, bool)] = &[ ), ]; +#[inline] pub fn examples() -> &'static Vec<(&'static str, bool)> { static EXAMPLES_ONCE: OnceLock> = OnceLock::new(); EXAMPLES_ONCE.get_or_init(|| { @@ -629,6 +630,7 @@ pub fn examples() -> &'static Vec<(&'static str, bool)> { }) } +#[inline] pub fn valid_examples() -> &'static Vec<(&'static str, bool)> { static VALID_EXAMPLES_ONCE: OnceLock> = OnceLock::new(); VALID_EXAMPLES_ONCE.get_or_init(|| examples().iter().copied().filter(|s| s.1).collect()) diff --git a/avro_test_helper/src/lib.rs b/avro_test_helper/src/lib.rs index 9b5248db..f0361872 100644 --- 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> = const { RefCell::new(Vec::new()) }; } -pub mod data; -pub mod logger; - #[cfg(not(target_arch = "wasm32"))] #[ctor] fn before_all() { @@ -50,19 +52,21 @@ fn after_all() { } /// A custom error type for tests. +#[non_exhaustive] #[derive(Debug)] pub struct TestError; /// A converter of any error into [`TestError`]. /// /// It is used to print better error messages in the tests. -/// Borrowed from +/// Borrowed from . // The Display bound is needed so that the `From` implementation doesn't // apply to `TestError` itself. -impl From for TestError { +impl From for TestError { #[track_caller] + #[inline] fn from(err: Err) -> Self { - panic!("{}: {:?}", std::any::type_name::(), err); + panic!("{}: {:?}", type_name::(), err); } } @@ -71,4 +75,5 @@ 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() {} +#[inline] +pub const fn init() {} diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 738cdeed..37b1eb00 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -53,6 +53,14 @@ fn test_logger() -> &'static TestLogger { }) } +/// Clears all log messages stored in the thread-local storage. +/// +/// # Panics +/// This function will panic if: +/// - The thread-local `LOG_MESSAGES` is already borrowed and cannot +/// be mutably borrowed. +/// +/// The panic includes a debug representation of the error encountered. pub fn clear_log_messages() { LOG_MESSAGES.with(|msgs| match msgs.try_borrow_mut() { Ok(mut log_messages) => log_messages.clear(), @@ -60,6 +68,32 @@ pub fn clear_log_messages() { }); } +/// Asserts that a specific log message has not been logged. +/// +/// This function checks the most recently logged message from a thread-local +/// storage and ensures that it does not match the provided `unexpected_message`. +/// If the message does match, the function will panic with an appropriate error +/// message indicating the unexpected log entry. +/// +/// # Arguments +/// - `unexpected_message`: A string slice that represents the log message +/// that is not expected to be logged. If this message matches the last +/// logged message, the function will panic. +/// +/// # Panics +/// - This function will panic if the `unexpected_message` matches the most +/// recently logged message. +/// +/// # Example +/// ```ignore +/// // Assume LOG_MESSAGES is set up to capture log messages. +/// log_message("Unexpected Error"); +/// assert_not_logged("Unexpected Error"); +/// // This will panic with the message: +/// // "The following log message should not have been logged: 'Unexpected Error'" +/// +/// assert_not_logged("Non-existent Message"); +/// // This will pass without issue since the message was not logged. #[track_caller] pub fn assert_not_logged(unexpected_message: &str) { LOG_MESSAGES.with(|msgs| match msgs.borrow().last() { @@ -70,6 +104,43 @@ pub fn assert_not_logged(unexpected_message: &str) { }); } +/// Asserts that the specified log message has been logged and removes it from +/// the stored log messages. +/// +/// # Parameters +/// - `expected_message`: A string slice that holds the log message to be asserted. +/// +/// # Panics +/// This function will panic if the `expected_message` has not been logged. The +/// panic message will indicate the missing log message. +/// +/// # Behavior +/// - The function verifies if the `expected_message` exists in the log messages +/// stored using the thread-local storage (`LOG_MESSAGES`). +/// - If the message is found, it is removed from the log messages and the function +/// completes successfully. +/// - If the message is not found, a panic is triggered with an explanatory message. +/// +/// # Usage +/// This function is typically used in unit tests or scenarios where it is important +/// to ensure specific messages are logged during execution. +/// +/// # Example +/// ```ignore +/// // Assuming LOG_MESSAGES is set up and some log messages have been recorded: +/// assert_logged("Expected log entry"); +/// ``` +/// +/// # Notes +/// - The function uses the `#[track_caller]` attribute to improve debugging by +/// providing more accurate information about the location of the panic in the +/// source code. +/// - The `LOG_MESSAGES` must be a thread-local data structure that supports +/// borrowing and mutating of a collection of string messages. +/// +/// # Thread Safety +/// This function relies on thread-local variables to manage logs for each thread +/// independently. #[track_caller] pub fn assert_logged(expected_message: &str) { let mut deleted = false; @@ -81,19 +152,20 @@ pub fn assert_logged(expected_message: &str) { } else { true } - }) + }); }); - if !deleted { - panic!("Expected log message has not been logged: '{expected_message}'"); - } + assert!( + deleted, + "Expected log message has not been logged: '{expected_message}'" + ); } pub(crate) fn install() { log::set_logger(test_logger()) - .map(|_| log::set_max_level(LevelFilter::Trace)) + .map(|()| log::set_max_level(LevelFilter::Trace)) .map_err(|err| { eprintln!("Failed to set the custom logger: {err:?}"); }) - .unwrap(); + .expect("Failed to set the custom TestLogger"); } From d2a38dd00bfd0890f14f99d14fb28dff8b9376fe Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 22:52:53 +0200 Subject: [PATCH 02/13] Simplify clear_log_messages() helper method Signed-off-by: Martin Tzvetanov Grigorov --- avro_test_helper/src/logger.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 37b1eb00..c08142a0 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -62,10 +62,7 @@ fn test_logger() -> &'static TestLogger { /// /// The panic includes a debug representation of the error encountered. pub fn clear_log_messages() { - LOG_MESSAGES.with(|msgs| match msgs.try_borrow_mut() { - Ok(mut log_messages) => log_messages.clear(), - Err(err) => panic!("Failed to clear log messages: {err:?}"), - }); + LOG_MESSAGES.with_borrow_mut(Vec::clear); } /// Asserts that a specific log message has not been logged. From 3a73087d55e5fe12cef692a964e05042950247e8 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 22:54:21 +0200 Subject: [PATCH 03/13] Move clippy settings to Cargo.toml Signed-off-by: Martin Tzvetanov Grigorov --- .github/workflows/test-lang-rust-clippy.yml | 8 ++++---- Cargo.toml | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-lang-rust-clippy.yml b/.github/workflows/test-lang-rust-clippy.yml index 33b6dec6..fab9458f 100644 --- 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@stable with: toolchain: ${{ matrix.rust }} components: clippy - - run: cargo clippy --all-features --all-targets --workspace -- -Dclippy::cargo -Aclippy::multiple_crate_versions + - run: cargo clippy --all-features --all-targets --workspace diff --git a/Cargo.toml b/Cargo.toml index 0ae7da48..d19b804a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,3 +60,5 @@ 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" From 1166b36cb8c49b59e1d9763c4f4adc8c4008048e Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:01:29 +0200 Subject: [PATCH 04/13] Simplify the docstrings for logging related helper methods Signed-off-by: Martin Tzvetanov Grigorov --- avro_test_helper/src/logger.rs | 91 +++++++++++----------------------- 1 file changed, 30 insertions(+), 61 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index c08142a0..959d1248 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -53,47 +53,33 @@ fn test_logger() -> &'static TestLogger { }) } -/// Clears all log messages stored in the thread-local storage. -/// -/// # Panics -/// This function will panic if: -/// - The thread-local `LOG_MESSAGES` is already borrowed and cannot -/// be mutably borrowed. -/// -/// The panic includes a debug representation of the error encountered. +/// Clears all log messages of this thread. pub fn clear_log_messages() { LOG_MESSAGES.with_borrow_mut(Vec::clear); } -/// Asserts that a specific log message has not been logged. -/// -/// This function checks the most recently logged message from a thread-local -/// storage and ensures that it does not match the provided `unexpected_message`. -/// If the message does match, the function will panic with an appropriate error -/// message indicating the unexpected log entry. -/// -/// # Arguments -/// - `unexpected_message`: A string slice that represents the log message -/// that is not expected to be logged. If this message matches the last -/// logged message, the function will panic. +/// Asserts that the message is not the last in the log for this thread. /// /// # Panics -/// - This function will panic if the `unexpected_message` matches the most -/// recently logged message. +/// Will panic if the provided message is an exact match for the last log message. /// /// # Example -/// ```ignore -/// // Assume LOG_MESSAGES is set up to capture log messages. -/// log_message("Unexpected Error"); -/// assert_not_logged("Unexpected Error"); -/// // This will panic with the message: -/// // "The following log message should not have been logged: 'Unexpected Error'" +/// ```should_panic +/// log::error("Something went wrong"); +/// log::error("Unexpected Error"); +/// +/// // This will not panic as it is not an exact match +/// assert_not_logged("No Unexpected Error"); +/// +/// // This will not panic as it is not the last log message +/// assert_not_logged("Something went wrong"); /// -/// assert_not_logged("Non-existent Message"); -/// // This will pass without issue since the message was not logged. +/// // This will panic +/// assert_not_logged("Unexpected Error"); +/// ``` #[track_caller] pub fn assert_not_logged(unexpected_message: &str) { - LOG_MESSAGES.with(|msgs| match msgs.borrow().last() { + LOG_MESSAGES.with_borrow(|msgs| match msgs.last() { Some(last_log) if last_log == unexpected_message => { panic!("The following log message should not have been logged: '{unexpected_message}'") } @@ -101,43 +87,26 @@ pub fn assert_not_logged(unexpected_message: &str) { }); } -/// Asserts that the specified log message has been logged and removes it from -/// the stored log messages. -/// -/// # Parameters -/// - `expected_message`: A string slice that holds the log message to be asserted. +/// Asserts that the message has been logged and removes it from the log of this thread. /// /// # Panics -/// This function will panic if the `expected_message` has not been logged. The -/// panic message will indicate the missing log message. -/// -/// # Behavior -/// - The function verifies if the `expected_message` exists in the log messages -/// stored using the thread-local storage (`LOG_MESSAGES`). -/// - If the message is found, it is removed from the log messages and the function -/// completes successfully. -/// - If the message is not found, a panic is triggered with an explanatory message. -/// -/// # Usage -/// This function is typically used in unit tests or scenarios where it is important -/// to ensure specific messages are logged during execution. +/// Will panic if the message does not appear in the log. /// /// # Example -/// ```ignore -/// // Assuming LOG_MESSAGES is set up and some log messages have been recorded: -/// assert_logged("Expected log entry"); -/// ``` +/// ```should_panic +/// log::error("Something went wrong"); +/// log::info("Something happened"); +/// log::error("Something went wrong"); /// -/// # Notes -/// - The function uses the `#[track_caller]` attribute to improve debugging by -/// providing more accurate information about the location of the panic in the -/// source code. -/// - The `LOG_MESSAGES` must be a thread-local data structure that supports -/// borrowing and mutating of a collection of string messages. +/// // This will not panic as the message was logged +/// assert_logged("Something went wrong"); /// -/// # Thread Safety -/// This function relies on thread-local variables to manage logs for each thread -/// independently. +/// // This will not panic as the message was logged +/// assert_logged("Something happened"); +/// +/// // This will panic as the first call removed the matching log messages +/// assert_logged("Something went wrong"); +/// ``` #[track_caller] pub fn assert_logged(expected_message: &str) { let mut deleted = false; From c6242797cbbcd8a3552267d5c610a417727057a7 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:08:08 +0200 Subject: [PATCH 05/13] Improve assert_not_logged() to check all messages, not just the last one Signed-off-by: Martin Tzvetanov Grigorov --- avro_test_helper/src/logger.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 959d1248..a64609f6 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -79,11 +79,12 @@ pub fn clear_log_messages() { /// ``` #[track_caller] pub fn assert_not_logged(unexpected_message: &str) { - LOG_MESSAGES.with_borrow(|msgs| match msgs.last() { - Some(last_log) if last_log == unexpected_message => { - panic!("The following log message should not have been logged: '{unexpected_message}'") - } - _ => (), + LOG_MESSAGES.with_borrow(|msgs| { + let is_logged = msgs.iter().any(|msg| msg == unexpected_message); + assert!( + !is_logged, + "The following log message should not have been logged: '{unexpected_message}'" + ); }); } @@ -110,8 +111,8 @@ pub fn assert_not_logged(unexpected_message: &str) { #[track_caller] pub fn assert_logged(expected_message: &str) { let mut deleted = false; - LOG_MESSAGES.with(|msgs| { - msgs.borrow_mut().retain(|msg| { + LOG_MESSAGES.with_borrow_mut(|msgs| { + msgs.retain(|msg| { if msg == expected_message { deleted = true; false From df04fd6286d9fcd0e877c3b132474457357c8853 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:20:29 +0200 Subject: [PATCH 06/13] 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 --- Cargo.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index d19b804a..ab8ab9af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,3 +62,10 @@ clippy.doc_markdown = "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" From dd117964795738361117d2328053ca0371fe5305 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:23:55 +0200 Subject: [PATCH 07/13] Fix docstring tests Signed-off-by: Martin Tzvetanov Grigorov --- avro_test_helper/src/logger.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index a64609f6..04396cd9 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -65,8 +65,10 @@ pub fn clear_log_messages() { /// /// # Example /// ```should_panic -/// log::error("Something went wrong"); -/// log::error("Unexpected Error"); +/// use apache_avro_test_helper::logger::assert_not_logged; +/// +/// log::error!("Something went wrong"); +/// log::error!("Unexpected Error"); /// /// // This will not panic as it is not an exact match /// assert_not_logged("No Unexpected Error"); @@ -95,9 +97,11 @@ pub fn assert_not_logged(unexpected_message: &str) { /// /// # Example /// ```should_panic -/// log::error("Something went wrong"); -/// log::info("Something happened"); -/// log::error("Something went wrong"); +/// use apache_avro_test_helper::logger::assert_logged; +/// +/// log::error!("Something went wrong"); +/// log::info!("Something happened"); +/// log::error!("Something went wrong"); /// /// // This will not panic as the message was logged /// assert_logged("Something went wrong"); From c38c0f8c3708f64a62682c184d6cbf56367eba64 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:25:10 +0200 Subject: [PATCH 08/13] 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 --- Cargo.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ab8ab9af..7b6c72d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,10 +62,10 @@ clippy.doc_markdown = "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" +# 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" From 8826f54997c34d69166f54513374ac1a1cdddcb0 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Tue, 24 Mar 2026 23:40:32 +0200 Subject: [PATCH 09/13] Improve assert_logged() to remove only one matching log message, not all Signed-off-by: Martin Tzvetanov Grigorov --- avro_test_helper/src/logger.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 04396cd9..79af9bbc 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -109,21 +109,20 @@ pub fn assert_not_logged(unexpected_message: &str) { /// // This will not panic as the message was logged /// assert_logged("Something happened"); /// -/// // This will panic as the first call removed the matching log messages +/// // This will not panic as the first call removed only the first matching log message +/// assert_logged("Something went wrong"); +/// +/// // This will panic as all matching log messages have been removed /// assert_logged("Something went wrong"); /// ``` #[track_caller] pub fn assert_logged(expected_message: &str) { let mut deleted = false; LOG_MESSAGES.with_borrow_mut(|msgs| { - msgs.retain(|msg| { - if msg == expected_message { - deleted = true; - false - } else { - true - } - }); + if let Some(pos) = msgs.iter().position(|msg| msg == expected_message) { + msgs.remove(pos); + deleted = true; + } }); assert!( From 5de1840cbdde46e5b3f604dfc45ad2858aef5b60 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Mon, 30 Mar 2026 17:01:03 +0300 Subject: [PATCH 10/13] Simplify by using `impl AsRef<[]>` Suggested-by: Kriskras99 --- avro_derive/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/avro_derive/src/lib.rs b/avro_derive/src/lib.rs index 2fafa384..d63ed8f4 100644 --- a/avro_derive/src/lib.rs +++ b/avro_derive/src/lib.rs @@ -51,7 +51,7 @@ use crate::{ pub fn proc_macro_derive_avro_schema(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = parse_macro_input!(input as DeriveInput); derive_avro_schema(input) - .unwrap_or_else(|errs| to_compile_errors(errs.as_slice())) + .unwrap_or_else(to_compile_errors) .into() } @@ -448,8 +448,8 @@ fn type_to_field_default_expr(ty: &Type) -> Result> } /// Stolen from serde -fn to_compile_errors(errors: &[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)*) } From 781e322a7e3b55614c835028c31a4d759465377c Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Mon, 30 Mar 2026 17:05:55 +0300 Subject: [PATCH 11/13] Remove `#[inline]`s Clippy asked to add them. Kriskras99 asked to remove them. --- avro_test_helper/src/data.rs | 2 -- avro_test_helper/src/lib.rs | 3 --- avro_test_helper/src/logger.rs | 1 - 3 files changed, 6 deletions(-) diff --git a/avro_test_helper/src/data.rs b/avro_test_helper/src/data.rs index b0999d70..18a4310e 100644 --- a/avro_test_helper/src/data.rs +++ b/avro_test_helper/src/data.rs @@ -602,7 +602,6 @@ pub const LOCAL_TIMESTAMPMICROS_LOGICAL_TYPE: &[(&str, bool)] = &[ ), ]; -#[inline] pub fn examples() -> &'static Vec<(&'static str, bool)> { static EXAMPLES_ONCE: OnceLock> = OnceLock::new(); EXAMPLES_ONCE.get_or_init(|| { @@ -630,7 +629,6 @@ pub fn examples() -> &'static Vec<(&'static str, bool)> { }) } -#[inline] pub fn valid_examples() -> &'static Vec<(&'static str, bool)> { static VALID_EXAMPLES_ONCE: OnceLock> = OnceLock::new(); VALID_EXAMPLES_ONCE.get_or_init(|| examples().iter().copied().filter(|s| s.1).collect()) diff --git a/avro_test_helper/src/lib.rs b/avro_test_helper/src/lib.rs index f0361872..686d96f1 100644 --- a/avro_test_helper/src/lib.rs +++ b/avro_test_helper/src/lib.rs @@ -52,7 +52,6 @@ fn after_all() { } /// A custom error type for tests. -#[non_exhaustive] #[derive(Debug)] pub struct TestError; @@ -64,7 +63,6 @@ pub struct TestError; // apply to `TestError` itself. impl From for TestError { #[track_caller] - #[inline] fn from(err: Err) -> Self { panic!("{}: {:?}", type_name::(), err); } @@ -75,5 +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. -#[inline] pub const fn init() {} diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 79af9bbc..52169d75 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -26,7 +26,6 @@ struct TestLogger { } impl Log for TestLogger { - #[inline] fn enabled(&self, _metadata: &Metadata) -> bool { true } From ab2a775b9b0ec8bcaf5d1066cc082939b0ddf5ef Mon Sep 17 00:00:00 2001 From: Martin Grigorov Date: Tue, 31 Mar 2026 09:10:43 +0300 Subject: [PATCH 12/13] Update the docstring for assert_not_logged Co-authored-by: Kriskras99 --- avro_test_helper/src/logger.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index 52169d75..fa2d5edd 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -57,24 +57,20 @@ pub fn clear_log_messages() { LOG_MESSAGES.with_borrow_mut(Vec::clear); } -/// Asserts that the message is not the last in the log for this thread. +/// Asserts that the message is not in the log for this thread. /// /// # Panics -/// Will panic if the provided message is an exact match for the last log message. +/// Will panic if the provided message is an exact match for any message in the log. /// /// # Example /// ```should_panic -/// use apache_avro_test_helper::logger::assert_not_logged; -/// -/// log::error!("Something went wrong"); +/// # use apache_avro_test_helper::logger::assert_not_logged; +/// # /// log::error!("Unexpected Error"); /// /// // This will not panic as it is not an exact match /// assert_not_logged("No Unexpected Error"); /// -/// // This will not panic as it is not the last log message -/// assert_not_logged("Something went wrong"); -/// /// // This will panic /// assert_not_logged("Unexpected Error"); /// ``` From b2069069dc9b835e885d27d45906230653df412a Mon Sep 17 00:00:00 2001 From: Martin Grigorov Date: Tue, 31 Mar 2026 09:11:08 +0300 Subject: [PATCH 13/13] Hide imports from the user Co-authored-by: Kriskras99 --- avro_test_helper/src/logger.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/avro_test_helper/src/logger.rs b/avro_test_helper/src/logger.rs index fa2d5edd..e3d13abf 100644 --- a/avro_test_helper/src/logger.rs +++ b/avro_test_helper/src/logger.rs @@ -92,8 +92,8 @@ pub fn assert_not_logged(unexpected_message: &str) { /// /// # Example /// ```should_panic -/// use apache_avro_test_helper::logger::assert_logged; -/// +/// # use apache_avro_test_helper::logger::assert_logged; +/// # /// log::error!("Something went wrong"); /// log::info!("Something happened"); /// log::error!("Something went wrong");