From bc3478b32b2f6153b168906cb673db1ab24deb8a Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Fri, 13 Jan 2023 16:21:33 +0100 Subject: [PATCH 1/3] scylla-macros: add fully qualified names to all types in generated code To make the macros more hygienic, add full paths to types in the generated code, so that the macros will still work even if the user uses the same names with different meanings in their code. --- examples/user-defined-type.rs | 1 - scylla-macros/src/from_row.rs | 7 ++++--- scylla-macros/src/from_user_type.rs | 12 +++++++----- scylla-macros/src/into_user_type.rs | 8 ++++---- scylla-macros/src/value_list.rs | 2 +- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/examples/user-defined-type.rs b/examples/user-defined-type.rs index d369fdd43b..bbbc0466ec 100644 --- a/examples/user-defined-type.rs +++ b/examples/user-defined-type.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use scylla::cql_to_rust::FromCqlVal; use scylla::macros::{FromUserType, IntoUserType}; use scylla::{IntoTypedRows, Session, SessionBuilder}; use std::env; diff --git a/scylla-macros/src/from_row.rs b/scylla-macros/src/from_row.rs index 3a696c060f..233800601a 100644 --- a/scylla-macros/src/from_row.rs +++ b/scylla-macros/src/from_row.rs @@ -22,8 +22,8 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { .unwrap(); // vals_iter size is checked before this code is reached, so // it is safe to unwrap - <#field_type as FromCqlVal>>::from_cql(col_value) - .map_err(|e| FromRowError::BadCqlVal { + <#field_type as scylla::cql_to_rust::FromCqlVal<::std::option::Option>>::from_cql(col_value) + .map_err(|e| scylla::cql_to_rust::FromRowError::BadCqlVal { err: e, column: col_ix, })? @@ -35,9 +35,10 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { let generated = quote! { impl #impl_generics scylla::cql_to_rust::FromRow for #struct_name #ty_generics #where_clause { fn from_row(row: scylla::frame::response::result::Row) - -> Result { + -> ::std::result::Result { use scylla::frame::response::result::CqlValue; use scylla::cql_to_rust::{FromCqlVal, FromRow, FromRowError}; + use ::std::result::Result::{Ok, Err}; if #fields_count != row.columns.len() { return Err(FromRowError::WrongRowSize { diff --git a/scylla-macros/src/from_user_type.rs b/scylla-macros/src/from_user_type.rs index ef95a8cc83..41e70c85f8 100644 --- a/scylla-macros/src/from_user_type.rs +++ b/scylla-macros/src/from_user_type.rs @@ -16,9 +16,9 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { let field_type = &field.ty; quote_spanned! {field.span() => - #field_name: <#field_type as FromCqlVal>>::from_cql( + #field_name: <#field_type as scylla::cql_to_rust::FromCqlVal>>::from_cql( { - let received_field_name: Option<&String> = fields_iter + let received_field_name: Option<&::std::string::String> = fields_iter .peek() .map(|(ref name, _)| name); @@ -43,10 +43,12 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { }); let generated = quote! { - impl #impl_generics FromCqlVal for #struct_name #ty_generics #where_clause { + impl #impl_generics scylla::cql_to_rust::FromCqlVal for #struct_name #ty_generics #where_clause { fn from_cql(cql_val: scylla::frame::response::result::CqlValue) - -> Result { - use std::collections::BTreeMap; + -> ::std::result::Result { + use ::std::collections::BTreeMap; + use ::std::option::Option::{self, Some, None}; + use ::std::result::Result::{Ok, Err}; use scylla::cql_to_rust::{FromCqlVal, FromCqlValError}; use scylla::frame::response::result::CqlValue; diff --git a/scylla-macros/src/into_user_type.rs b/scylla-macros/src/into_user_type.rs index 3ae433fddb..0bc9351f28 100644 --- a/scylla-macros/src/into_user_type.rs +++ b/scylla-macros/src/into_user_type.rs @@ -14,17 +14,17 @@ pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { let field_name = &field.ident; quote_spanned! {field.span() => - <_ as Value>::serialize(&self.#field_name, buf) ?; + <_ as scylla::frame::value::Value>::serialize(&self.#field_name, buf) ?; } }); let generated = quote! { impl #impl_generics scylla::frame::value::Value for #struct_name #ty_generics #where_clause { - fn serialize(&self, buf: &mut Vec) -> std::result::Result<(), scylla::frame::value::ValueTooBig> { + fn serialize(&self, buf: &mut ::std::vec::Vec<::core::primitive::u8>) -> ::std::result::Result<(), scylla::frame::value::ValueTooBig> { use scylla::frame::value::{Value, ValueTooBig}; use scylla::macros::BufMut; use ::std::convert::TryInto; - + use ::core::primitive::{usize, i32}; // Reserve space to put serialized size in let total_size_index: usize = buf.len(); @@ -40,7 +40,7 @@ pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { let total_size_i32: i32 = total_size.try_into().map_err(|_| ValueTooBig) ?; buf[total_size_index..(total_size_index+4)].copy_from_slice(&total_size_i32.to_be_bytes()[..]); - Ok(()) + ::std::result::Result::Ok(()) } } }; diff --git a/scylla-macros/src/value_list.rs b/scylla-macros/src/value_list.rs index 4304ba222b..08e1794090 100644 --- a/scylla-macros/src/value_list.rs +++ b/scylla-macros/src/value_list.rs @@ -21,7 +21,7 @@ pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { result.add_value(&self.#field_name)?; )* - Ok(std::borrow::Cow::Owned(result)) + ::std::result::Result::Ok(::std::borrow::Cow::Owned(result)) } } }; From 6ee80a88be2b8f1b57262d84b95733f62f06dd24 Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Fri, 13 Jan 2023 16:26:35 +0100 Subject: [PATCH 2/3] scylla-macros: support renaming the scylla crate Currently, the macros will only work, if the code using them has a dependency on the scylla driver, or on the scylla_cql if it's renamed to scylla. This is caused by the fact that the paths referring to the scylla code are hardcoded to use a `scylla` crate. As a result, if a someone uses the `scylla_cql` crate without renaming, or renames `scylla_cql` or `scylla` to a name differen name than `scylla`, the macros will not work. Similarily, when another crate re-exports the macros, or uses them in their own macros, it can only work if the crate is used under the name `scylla` and it additionally re-exports parts of the `scylla_cql` code that are used in the macro under the same paths. This patch adds a `scylla_crate` helper attribute that allows users to set the crate name that will export the code that is used in the macros. All the symbols that are used there are also additionally exported in a `_macro_internal` module to make re-exporting important code segments more convenient. --- scylla-cql/src/lib.rs | 12 +++++++++ scylla-macros/Cargo.toml | 1 + scylla-macros/src/from_row.rs | 14 +++++----- scylla-macros/src/from_user_type.rs | 12 ++++----- scylla-macros/src/into_user_type.rs | 10 ++++---- scylla-macros/src/lib.rs | 8 +++--- scylla-macros/src/parser.rs | 40 +++++++++++++++++++++++++++++ scylla-macros/src/value_list.rs | 7 ++--- scylla/src/lib.rs | 5 ++++ 9 files changed, 84 insertions(+), 25 deletions(-) diff --git a/scylla-cql/src/lib.rs b/scylla-cql/src/lib.rs index af9f5f7aad..47b58b4f4e 100644 --- a/scylla-cql/src/lib.rs +++ b/scylla-cql/src/lib.rs @@ -7,3 +7,15 @@ pub use crate::frame::response::cql_to_rust; pub use crate::frame::response::cql_to_rust::FromRow; pub use crate::frame::types::Consistency; + +#[doc(hidden)] +pub mod _macro_internal { + pub use crate::frame::response::cql_to_rust::{ + FromCqlVal, FromCqlValError, FromRow, FromRowError, + }; + pub use crate::frame::response::result::{CqlValue, Row}; + pub use crate::frame::value::{ + SerializedResult, SerializedValues, Value, ValueList, ValueTooBig, + }; + pub use crate::macros::*; +} diff --git a/scylla-macros/Cargo.toml b/scylla-macros/Cargo.toml index 36a2c439a6..9c34a2a1cf 100644 --- a/scylla-macros/Cargo.toml +++ b/scylla-macros/Cargo.toml @@ -14,3 +14,4 @@ proc-macro = true [dependencies] syn = "1.0" quote = "1.0" +proc-macro2 = "1.0" \ No newline at end of file diff --git a/scylla-macros/src/from_row.rs b/scylla-macros/src/from_row.rs index 233800601a..c79b25dbb0 100644 --- a/scylla-macros/src/from_row.rs +++ b/scylla-macros/src/from_row.rs @@ -5,6 +5,7 @@ use syn::{spanned::Spanned, DeriveInput}; /// #[derive(FromRow)] derives FromRow for struct pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { let item = syn::parse::(tokens_input).expect("No DeriveInput"); + let path = crate::parser::get_path(&item).expect("No path"); let struct_fields = crate::parser::parse_named_fields(&item, "FromRow"); let struct_name = &item.ident; @@ -22,8 +23,8 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { .unwrap(); // vals_iter size is checked before this code is reached, so // it is safe to unwrap - <#field_type as scylla::cql_to_rust::FromCqlVal<::std::option::Option>>::from_cql(col_value) - .map_err(|e| scylla::cql_to_rust::FromRowError::BadCqlVal { + <#field_type as FromCqlVal<::std::option::Option>>::from_cql(col_value) + .map_err(|e| FromRowError::BadCqlVal { err: e, column: col_ix, })? @@ -33,11 +34,10 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { let fields_count = struct_fields.named.len(); let generated = quote! { - impl #impl_generics scylla::cql_to_rust::FromRow for #struct_name #ty_generics #where_clause { - fn from_row(row: scylla::frame::response::result::Row) - -> ::std::result::Result { - use scylla::frame::response::result::CqlValue; - use scylla::cql_to_rust::{FromCqlVal, FromRow, FromRowError}; + impl #impl_generics #path::FromRow for #struct_name #ty_generics #where_clause { + fn from_row(row: #path::Row) + -> ::std::result::Result { + use #path::{CqlValue, FromCqlVal, FromRow, FromRowError}; use ::std::result::Result::{Ok, Err}; if #fields_count != row.columns.len() { diff --git a/scylla-macros/src/from_user_type.rs b/scylla-macros/src/from_user_type.rs index 41e70c85f8..2c18088a3a 100644 --- a/scylla-macros/src/from_user_type.rs +++ b/scylla-macros/src/from_user_type.rs @@ -5,6 +5,7 @@ use syn::{spanned::Spanned, DeriveInput}; /// #[derive(FromUserType)] allows to parse a struct as User Defined Type pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { let item = syn::parse::(tokens_input).expect("No DeriveInput"); + let path = crate::parser::get_path(&item).expect("Couldn't get path to the scylla crate"); let struct_fields = crate::parser::parse_named_fields(&item, "FromUserType"); let struct_name = &item.ident; @@ -16,7 +17,7 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { let field_type = &field.ty; quote_spanned! {field.span() => - #field_name: <#field_type as scylla::cql_to_rust::FromCqlVal>>::from_cql( + #field_name: <#field_type as FromCqlVal<::std::option::Option>>::from_cql( { let received_field_name: Option<&::std::string::String> = fields_iter .peek() @@ -43,14 +44,13 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { }); let generated = quote! { - impl #impl_generics scylla::cql_to_rust::FromCqlVal for #struct_name #ty_generics #where_clause { - fn from_cql(cql_val: scylla::frame::response::result::CqlValue) - -> ::std::result::Result { + impl #impl_generics #path::FromCqlVal<#path::CqlValue> for #struct_name #ty_generics #where_clause { + fn from_cql(cql_val: #path::CqlValue) + -> ::std::result::Result { use ::std::collections::BTreeMap; use ::std::option::Option::{self, Some, None}; use ::std::result::Result::{Ok, Err}; - use scylla::cql_to_rust::{FromCqlVal, FromCqlValError}; - use scylla::frame::response::result::CqlValue; + use #path::{FromCqlVal, FromCqlValError, CqlValue}; // Interpret CqlValue as CQlValue::UserDefinedType let mut fields_iter = match cql_val { diff --git a/scylla-macros/src/into_user_type.rs b/scylla-macros/src/into_user_type.rs index 0bc9351f28..4adfaebd18 100644 --- a/scylla-macros/src/into_user_type.rs +++ b/scylla-macros/src/into_user_type.rs @@ -5,6 +5,7 @@ use syn::{spanned::Spanned, DeriveInput}; /// #[derive(IntoUserType)] allows to parse a struct as User Defined Type pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { let item = syn::parse::(tokens_input).expect("No DeriveInput"); + let path = crate::parser::get_path(&item).expect("No path"); let struct_fields = crate::parser::parse_named_fields(&item, "IntoUserType"); let struct_name = &item.ident; @@ -14,15 +15,14 @@ pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { let field_name = &field.ident; quote_spanned! {field.span() => - <_ as scylla::frame::value::Value>::serialize(&self.#field_name, buf) ?; + <_ as Value>::serialize(&self.#field_name, buf) ?; } }); let generated = quote! { - impl #impl_generics scylla::frame::value::Value for #struct_name #ty_generics #where_clause { - fn serialize(&self, buf: &mut ::std::vec::Vec<::core::primitive::u8>) -> ::std::result::Result<(), scylla::frame::value::ValueTooBig> { - use scylla::frame::value::{Value, ValueTooBig}; - use scylla::macros::BufMut; + impl #impl_generics #path::Value for #struct_name #ty_generics #where_clause { + fn serialize(&self, buf: &mut ::std::vec::Vec<::core::primitive::u8>) -> ::std::result::Result<(), #path::ValueTooBig> { + use #path::{BufMut, ValueTooBig, Value}; use ::std::convert::TryInto; use ::core::primitive::{usize, i32}; diff --git a/scylla-macros/src/lib.rs b/scylla-macros/src/lib.rs index d99a2588c9..f5ad28a26d 100644 --- a/scylla-macros/src/lib.rs +++ b/scylla-macros/src/lib.rs @@ -8,28 +8,28 @@ mod value_list; /// #[derive(FromRow)] derives FromRow for struct /// Works only on simple structs without generics etc -#[proc_macro_derive(FromRow)] +#[proc_macro_derive(FromRow, attributes(scylla_crate))] pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { from_row::from_row_derive(tokens_input) } /// #[derive(FromUserType)] allows to parse a struct as User Defined Type /// Works only on simple structs without generics etc -#[proc_macro_derive(FromUserType)] +#[proc_macro_derive(FromUserType, attributes(scylla_crate))] pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { from_user_type::from_user_type_derive(tokens_input) } /// #[derive(IntoUserType)] allows to parse a struct as User Defined Type /// Works only on simple structs without generics etc -#[proc_macro_derive(IntoUserType)] +#[proc_macro_derive(IntoUserType, attributes(scylla_crate))] pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { into_user_type::into_user_type_derive(tokens_input) } /// #[derive(ValueList)] derives ValueList for struct /// Works only on simple structs without generics etc -#[proc_macro_derive(ValueList)] +#[proc_macro_derive(ValueList, attributes(scylla_crate))] pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { value_list::value_list_derive(tokens_input) } diff --git a/scylla-macros/src/parser.rs b/scylla-macros/src/parser.rs index 2a55d850f8..0da54370c8 100644 --- a/scylla-macros/src/parser.rs +++ b/scylla-macros/src/parser.rs @@ -1,4 +1,5 @@ use syn::{Data, DeriveInput, Fields, FieldsNamed}; +use syn::{Lit, Meta}; /// Parses the tokens_input to a DeriveInput and returns the struct name from which it derives and /// the named fields @@ -17,3 +18,42 @@ pub(crate) fn parse_named_fields<'a>( _ => panic!("derive({}) works only on structs!", current_derive), } } + +pub(crate) fn get_path(input: &DeriveInput) -> Result { + let mut this_path: Option = None; + for attr in input.attrs.iter() { + if !attr.path.is_ident("scylla_crate") { + continue; + } + match attr.parse_meta() { + Ok(Meta::NameValue(meta_name_value)) => { + if let Lit::Str(lit_str) = &meta_name_value.lit { + let path_val = &lit_str.value().parse::().unwrap(); + if this_path.is_none() { + this_path = Some(quote::quote!(#path_val::_macro_internal)); + } else { + return Err(syn::Error::new_spanned( + &meta_name_value.lit, + "the `scylla_crate` attribute was set multiple times", + )); + } + } else { + return Err(syn::Error::new_spanned( + &meta_name_value.lit, + "the `scylla_crate` attribute should be a string literal", + )); + } + } + Ok(other) => { + return Err(syn::Error::new_spanned( + other, + "the `scylla_crate` attribute have a single value", + )); + } + Err(err) => { + return Err(err); + } + } + } + Ok(this_path.unwrap_or_else(|| quote::quote!(scylla::_macro_internal))) +} diff --git a/scylla-macros/src/value_list.rs b/scylla-macros/src/value_list.rs index 08e1794090..4b19083944 100644 --- a/scylla-macros/src/value_list.rs +++ b/scylla-macros/src/value_list.rs @@ -6,6 +6,7 @@ use syn::DeriveInput; /// which can be fed to the query directly. pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { let item = syn::parse::(tokens_input).expect("No DeriveInput"); + let path = crate::parser::get_path(&item).expect("No path"); let struct_fields = crate::parser::parse_named_fields(&item, "ValueList"); let struct_name = &item.ident; @@ -14,9 +15,9 @@ pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { let values_len = struct_fields.named.len(); let field_name = struct_fields.named.iter().map(|field| &field.ident); let generated = quote! { - impl #impl_generics scylla::frame::value::ValueList for #struct_name #ty_generics #where_clause { - fn serialized(&self) -> scylla::frame::value::SerializedResult { - let mut result = scylla::frame::value::SerializedValues::with_capacity(#values_len); + impl #impl_generics #path::ValueList for #struct_name #ty_generics #where_clause { + fn serialized(&self) -> #path::SerializedResult { + let mut result = #path::SerializedValues::with_capacity(#values_len); #( result.add_value(&self.#field_name)?; )* diff --git a/scylla/src/lib.rs b/scylla/src/lib.rs index 7bdb420725..a9698825ce 100644 --- a/scylla/src/lib.rs +++ b/scylla/src/lib.rs @@ -93,6 +93,11 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] +#[doc(hidden)] +pub mod _macro_internal { + pub use scylla_cql::_macro_internal::*; +} + pub use scylla_cql::frame; pub use scylla_cql::macros::{self, *}; From 8bf549367271f00d0bffd2220d028501314d8042 Mon Sep 17 00:00:00 2001 From: Wojciech Mitros Date: Mon, 16 Jan 2023 18:35:39 +0100 Subject: [PATCH 3/3] tests: assert that using macros in the renamed crate works Add a test that confirms that the macros work even if the crate was renamed. The test checks both scylla and scylla_cql crates. The test found missing `use ::std::iter...` statements, so they're also added. --- scylla-macros/src/from_row.rs | 1 + scylla-macros/src/from_user_type.rs | 1 + scylla/tests/hygiene.rs | 74 +++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 scylla/tests/hygiene.rs diff --git a/scylla-macros/src/from_row.rs b/scylla-macros/src/from_row.rs index c79b25dbb0..c4caa405b6 100644 --- a/scylla-macros/src/from_row.rs +++ b/scylla-macros/src/from_row.rs @@ -39,6 +39,7 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { -> ::std::result::Result { use #path::{CqlValue, FromCqlVal, FromRow, FromRowError}; use ::std::result::Result::{Ok, Err}; + use ::std::iter::{Iterator, IntoIterator}; if #fields_count != row.columns.len() { return Err(FromRowError::WrongRowSize { diff --git a/scylla-macros/src/from_user_type.rs b/scylla-macros/src/from_user_type.rs index 2c18088a3a..de3a2adb7d 100644 --- a/scylla-macros/src/from_user_type.rs +++ b/scylla-macros/src/from_user_type.rs @@ -51,6 +51,7 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { use ::std::option::Option::{self, Some, None}; use ::std::result::Result::{Ok, Err}; use #path::{FromCqlVal, FromCqlValError, CqlValue}; + use ::std::iter::{Iterator, IntoIterator}; // Interpret CqlValue as CQlValue::UserDefinedType let mut fields_iter = match cql_val { diff --git a/scylla/tests/hygiene.rs b/scylla/tests/hygiene.rs new file mode 100644 index 0000000000..6195bb0256 --- /dev/null +++ b/scylla/tests/hygiene.rs @@ -0,0 +1,74 @@ +#![no_implicit_prelude] + +// Macro that is given a crate name and tests it for hygiene +macro_rules! test_crate { + ($name:ident) => { + extern crate $name as _scylla; + + #[derive( + _scylla::macros::FromRow, + _scylla::macros::FromUserType, + _scylla::macros::IntoUserType, + _scylla::macros::ValueList, + PartialEq, + Debug, + )] + #[scylla_crate = "_scylla"] + struct TestStruct { + a: ::core::primitive::i32, + } + #[test] + fn test_rename() { + use _scylla::cql_to_rust::{FromCqlVal, FromRow}; + use _scylla::frame::response::result::CqlValue; + use _scylla::frame::value::{Value, ValueList}; + + pub fn derived() + where + T: FromRow + FromCqlVal + Value + ValueList, + { + } + derived::(); + } + #[test] + fn test_derives() { + use ::core::primitive::u8; + use ::std::assert_eq; + use ::std::option::Option::Some; + use ::std::vec::Vec; + use _scylla::_macro_internal::{CqlValue, Row, Value, ValueList}; + use _scylla::cql_to_rust::FromRow; + + let test_struct = TestStruct { a: 16 }; + fn get_row() -> Row { + Row { + columns: ::std::vec![Some(CqlValue::Int(16))], + } + } + + let st: TestStruct = FromRow::from_row(get_row()).unwrap(); + assert_eq!(st, test_struct); + + let udt = get_row().into_typed::().unwrap(); + assert_eq!(udt, test_struct); + + let mut buf = Vec::::new(); + test_struct.serialize(&mut buf).unwrap(); + let mut buf_assert = Vec::::new(); + let tuple_with_same_layout: (i32,) = (16,); + tuple_with_same_layout.serialize(&mut buf_assert).unwrap(); + assert_eq!(buf, buf_assert); + + let sv = test_struct.serialized().unwrap().into_owned(); + let sv2 = tuple_with_same_layout.serialized().unwrap().into_owned(); + assert_eq!(sv, sv2); + } + }; +} + +mod scylla_hygiene { + test_crate!(scylla); +} +mod scylla_cql_hygiene { + test_crate!(scylla_cql); +}