From ea7c464bf1a3d10fadc1d67827dd17e1ee191b7d Mon Sep 17 00:00:00 2001 From: Alex Pozhylenkov Date: Tue, 1 Oct 2024 12:22:59 +0300 Subject: [PATCH] Fix clippy `expect_used` complains for `derive(FromRow)` macro (#1077) Get rid of `.expect` usage in `FromRow` macro by using arrays to detect problems during compile time. --- scylla-cql/src/frame/response/cql_to_rust.rs | 15 ++++++++ scylla-macros/src/from_row.rs | 39 +++++++++++--------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/scylla-cql/src/frame/response/cql_to_rust.rs b/scylla-cql/src/frame/response/cql_to_rust.rs index ec18ce0958..8f199b83eb 100644 --- a/scylla-cql/src/frame/response/cql_to_rust.rs +++ b/scylla-cql/src/frame/response/cql_to_rust.rs @@ -935,6 +935,11 @@ mod tests { ); } + // Enabling `expect_used` clippy lint, + // validates that `derive(FromRow)` macro definition does do not violates such rule under the hood. + // Could be removed after such rule will be applied for the whole crate. + // + #[deny(clippy::expect_used)] #[test] fn struct_from_row() { #[derive(FromRow)] @@ -959,6 +964,11 @@ mod tests { assert_eq!(my_row.c, Some(vec![1, 2])); } + // Enabling `expect_used` clippy lint, + // validates that `derive(FromRow)` macro definition does do not violates such rule under the hood. + // Could be removed after such rule will be applied for the whole crate. + // + #[deny(clippy::expect_used)] #[test] fn struct_from_row_wrong_size() { #[derive(FromRow, PartialEq, Eq, Debug)] @@ -998,6 +1008,11 @@ mod tests { ); } + // Enabling `expect_used` clippy lint, + // validates that `derive(FromRow)` macro definition does do not violates such rule under the hood. + // Could be removed after such rule will be applied for the whole crate. + // + #[deny(clippy::expect_used)] #[test] fn unnamed_struct_from_row() { #[derive(FromRow)] diff --git a/scylla-macros/src/from_row.rs b/scylla-macros/src/from_row.rs index 8bfcb9dce7..4a08c2a138 100644 --- a/scylla-macros/src/from_row.rs +++ b/scylla-macros/src/from_row.rs @@ -14,19 +14,20 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result { - let set_fields_code = fields.named.iter().map(|field| { + let set_fields_code = fields.named.iter().enumerate().map(|(col_ix, field)| { let field_name = &field.ident; let field_type = &field.ty; quote_spanned! {field.span() => #field_name: { - let (col_ix, col_value) = vals_iter - .next() - .expect("BUG: Size validated iterator did not contain the expected number of values"); + // To avoid unnecessary copy `std::mem::take` is used. + // Using explicit indexing operation is safe because `row_columns` is an array and `col_ix` is a litteral. + // + let col_value = ::std::mem::take(&mut row_columns[#col_ix]); <#field_type as FromCqlVal<::std::option::Option>>::from_cql(col_value) .map_err(|e| FromRowError::BadCqlVal { err: e, - column: col_ix, + column: #col_ix, })? }, } @@ -39,19 +40,19 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result { - let set_fields_code = fields.unnamed.iter().map(|field| { + let set_fields_code = fields.unnamed.iter().enumerate().map(|(col_ix, field)| { let field_type = &field.ty; quote_spanned! {field.span() => { - let (col_ix, col_value) = vals_iter - .next() - .expect("BUG: Size validated iterator did not contain the expected number of values"); - + // To avoid unnecessary copy `std::mem::take` is used. + // Using explicit indexing operation is safe because `row_columns` is an array and `col_ix` is a litteral. + // + let col_value = ::std::mem::take(&mut row_columns[#col_ix]); <#field_type as FromCqlVal<::std::option::Option>>::from_cql(col_value) .map_err(|e| FromRowError::BadCqlVal { err: e, - column: col_ix, + column: #col_ix, })? }, } @@ -71,15 +72,17 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result ::std::result::Result { use #path::{CqlValue, FromCqlVal, FromRow, FromRowError}; use ::std::result::Result::{Ok, Err}; + use ::std::convert::TryInto; use ::std::iter::{Iterator, IntoIterator}; - if #fields_count != row.columns.len() { - return Err(FromRowError::WrongRowSize { - expected: #fields_count, - actual: row.columns.len(), - }); - } - let mut vals_iter = row.columns.into_iter().enumerate(); + + let row_columns_len = row.columns.len(); + // An array used, to enable [uncoditional paniking](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/builtin/static.UNCONDITIONAL_PANIC.html) + // for "index out of range" issues and be able to catch them during the compile time. + let mut row_columns: [_; #fields_count] = row.columns.try_into().map_err(|_| FromRowError::WrongRowSize { + expected: #fields_count, + actual: row_columns_len, + })?; Ok(#struct_name #fill_struct_code) }