You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by mg...@apache.org on 2026/03/23 14:53:33 UTC
(avro-rs) branch pedantic-clippy created (now fc6d0db)
This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a change to branch pedantic-clippy
in repository https://gitbox.apache.org/repos/asf/avro-rs.git
at fc6d0db chore: Apply some suggestions by -Dclippy::pedantic
This branch includes the following new commits:
new fc6d0db chore: Apply some suggestions by -Dclippy::pedantic
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(avro-rs) 01/01: chore: Apply some suggestions by -Dclippy::pedantic
Posted by mg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a commit to branch pedantic-clippy
in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit fc6d0db8bb0f2244c4096b4991fbe70ee754c7ab
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
AuthorDate: Mon Mar 23 16:50:14 2026 +0200
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 e5243f5..33b6dec 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 fa9382f..6f29b47 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 ef5723e..571c05c 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 40e33af..e3b221c 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 <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
index eb11b9e..e0a1433 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 27c1cbc..66911d9 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<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
index c958562..9f8eff9 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 e44f4a1..1511f65 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<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
index 2832c08..4fb0adb 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<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
index 96f997e..2fafa38 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<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,
+ "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<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 {
+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<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
index 3713e83..b0999d7 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<Vec<(&'static str, bool)>> = 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<Vec<(&'static str, bool)>> = 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 9b5248d..f036187 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<Vec<String>> = 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 <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]
+ #[inline]
fn from(err: Err) -> Self {
- panic!("{}: {:?}", std::any::type_name::<Err>(), err);
+ panic!("{}: {:?}", type_name::<Err>(), 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 738cdee..f6c3b17 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
+/// ```
+/// // 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
+/// ```
+/// // 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");
}