Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-lang-rust-clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 7 additions & 9 deletions avro/src/serde/ser_schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
4 changes: 1 addition & 3 deletions avro/tests/append_to_existing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
2 changes: 1 addition & 1 deletion avro_derive/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
14 changes: 7 additions & 7 deletions avro_derive/src/attributes/avro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"#,
)
);
}
}
}
Expand All @@ -98,7 +98,7 @@ impl VariantAttributes {
span,
r#"`#[avro(rename = "..")]` is deprecated"#,
r#"Use `#[serde(rename = "..")]` instead"#,
)
);
}
}
}
Expand Down Expand Up @@ -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",
)
);
}
}
}
6 changes: 3 additions & 3 deletions avro_derive/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))]
Expand Down
8 changes: 5 additions & 3 deletions avro_derive/src/case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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('_', "-"),
}
Expand Down
2 changes: 1 addition & 1 deletion avro_derive/src/enums/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions avro_derive/src/enums/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 22 additions & 21 deletions avro_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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) => {
Expand All @@ -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(
Expand All @@ -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! {
Expand All @@ -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) {
Expand Down Expand Up @@ -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}
}
}

Expand Down
4 changes: 3 additions & 1 deletion avro_test_helper/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
];
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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())
Expand Down
21 changes: 13 additions & 8 deletions avro_test_helper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand All @@ -50,19 +52,21 @@ fn after_all() {
}

/// A custom error type for tests.
#[non_exhaustive]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding #[non_exhaustive] to the public TestError changes its public API (e.g., external users can no longer construct it), which can be a breaking change if avro_test_helper is consumed outside this workspace.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value:good-but-wont-fix; category:bug; feedback: The Augment AI reviewer is correct! The external users are not supposed to construct this error. It is a helper error that just unifies any other kind of errors, so that they could be used with TestResult in avro-rs' unit/IT tests.

#[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);
}
}

Expand All @@ -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() {}
Loading
Loading