Skip to content

Commit

Permalink
Fix clippy expect_used complains for derive(FromRow) macro (scyll…
Browse files Browse the repository at this point in the history
…adb#1077)

Get rid of `.expect` usage in `FromRow` macro by using arrays to detect problems during compile time.
  • Loading branch information
Mr-Leshiy authored Oct 1, 2024
1 parent 582d48e commit ea7c464
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 18 deletions.
15 changes: 15 additions & 0 deletions scylla-cql/src/frame/response/cql_to_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
// <https://rust-lang.github.io/rust-clippy/master/index.html#/expect_used>
#[deny(clippy::expect_used)]
#[test]
fn struct_from_row() {
#[derive(FromRow)]
Expand All @@ -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.
// <https://rust-lang.github.io/rust-clippy/master/index.html#/expect_used>
#[deny(clippy::expect_used)]
#[test]
fn struct_from_row_wrong_size() {
#[derive(FromRow, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -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.
// <https://rust-lang.github.io/rust-clippy/master/index.html#/expect_used>
#[deny(clippy::expect_used)]
#[test]
fn unnamed_struct_from_row() {
#[derive(FromRow)]
Expand Down
39 changes: 21 additions & 18 deletions scylla-macros/src/from_row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,20 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result<TokenStream,
// Generates a token that sets the values of struct fields: field_type::from_cql(...)
let (fill_struct_code, fields_count) = match struct_fields {
crate::parser::StructFields::Named(fields) => {
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.
// <https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/builtin/static.UNCONDITIONAL_PANIC.html>
let col_value = ::std::mem::take(&mut row_columns[#col_ix]);
<#field_type as FromCqlVal<::std::option::Option<CqlValue>>>::from_cql(col_value)
.map_err(|e| FromRowError::BadCqlVal {
err: e,
column: col_ix,
column: #col_ix,
})?
},
}
Expand All @@ -39,19 +40,19 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result<TokenStream,
(fill_struct_code, fields.named.len())
}
crate::parser::StructFields::Unnamed(fields) => {
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.
// <https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/builtin/static.UNCONDITIONAL_PANIC.html>
let col_value = ::std::mem::take(&mut row_columns[#col_ix]);
<#field_type as FromCqlVal<::std::option::Option<CqlValue>>>::from_cql(col_value)
.map_err(|e| FromRowError::BadCqlVal {
err: e,
column: col_ix,
column: #col_ix,
})?
},
}
Expand All @@ -71,15 +72,17 @@ pub(crate) fn from_row_derive(tokens_input: TokenStream) -> Result<TokenStream,
-> ::std::result::Result<Self, #path::FromRowError> {
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)
}
Expand Down

0 comments on commit ea7c464

Please sign in to comment.