diff --git a/sqlx-macros/src/lib.rs b/sqlx-macros/src/lib.rs index e7d848d3..c6ac3b5b 100644 --- a/sqlx-macros/src/lib.rs +++ b/sqlx-macros/src/lib.rs @@ -16,15 +16,6 @@ mod database; mod derives; mod query; -fn macro_result(tokens: proc_macro2::TokenStream) -> TokenStream { - quote!( - macro_rules! macro_result { - ($($args:tt)*) => (#tokens) - } - ) - .into() -} - #[proc_macro] pub fn expand_query(input: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(input as query::QueryMacroInput); @@ -33,10 +24,10 @@ pub fn expand_query(input: TokenStream) -> TokenStream { Ok(ts) => ts.into(), Err(e) => { if let Some(parse_err) = e.downcast_ref::() { - macro_result(parse_err.to_compile_error()) + parse_err.to_compile_error().into() } else { let msg = e.to_string(); - macro_result(quote!(compile_error!(#msg))) + quote!(compile_error!(#msg)).into() } } } diff --git a/sqlx-macros/src/query/args.rs b/sqlx-macros/src/query/args.rs index 4b630a99..dac292b0 100644 --- a/sqlx-macros/src/query/args.rs +++ b/sqlx-macros/src/query/args.rs @@ -2,10 +2,10 @@ use crate::database::DatabaseExt; use crate::query::QueryMacroInput; use either::Either; use proc_macro2::TokenStream; -use quote::{quote, quote_spanned}; +use quote::{format_ident, quote, quote_spanned}; use sqlx_core::statement::StatementInfo; use syn::spanned::Spanned; -use syn::{Expr, Type}; +use syn::{Expr, ExprCast, ExprGroup, ExprType, Type}; /// Returns a tokenstream which typechecks the arguments passed to the macro /// and binds them to `DB::Arguments` with the ident `query_args`. @@ -15,13 +15,22 @@ pub fn quote_args( ) -> crate::Result { let db_path = DB::db_path(); - if input.arg_names.is_empty() { + if input.arg_exprs.is_empty() { return Ok(quote! { let query_args = <#db_path as sqlx::database::HasArguments>::Arguments::default(); }); } - let arg_name = &input.arg_names; + let arg_names = (0..input.arg_exprs.len()) + .map(|i| format_ident!("arg{}", i)) + .collect::>(); + + let arg_name = &arg_names; + let arg_expr = input.arg_exprs.iter().cloned().map(strip_wildcard); + + let arg_bindings = quote! { + #(let ref #arg_name = #arg_expr;)* + }; let args_check = match info.parameters() { None | Some(Either::Right(_)) => { @@ -32,19 +41,12 @@ pub fn quote_args( Some(Either::Left(params)) => { params .iter() - .zip(input.arg_names.iter().zip(&input.arg_exprs)) + .zip(arg_names.iter().zip(&input.arg_exprs)) .enumerate() .map(|(i, (param_ty, (name, expr)))| -> crate::Result<_> { let param_ty = match get_type_override(expr) { - // TODO: enable this in 1.45 when we can strip `as _` - // without stripping these we get some pretty nasty type errors - Some(Type::Infer(_)) => return Err( - syn::Error::new_spanned( - expr, - "casts to `_` are not allowed in bind parameters yet" - ).into() - ), // cast or type ascription will fail to compile if the type does not match + // and we strip casts to wildcard Some(_) => return Ok(quote!()), None => { DB::param_type_for_id(¶m_ty) @@ -72,14 +74,14 @@ pub fn quote_args( use sqlx::ty_match::{WrapSameExt as _, MatchBorrowExt as _}; // evaluate the expression only once in case it contains moves - let _expr = sqlx::ty_match::dupe_value(&$#name); + let _expr = sqlx::ty_match::dupe_value(#name); // if `_expr` is `Option`, get `Option<$ty>`, otherwise `$ty` let ty_check = sqlx::ty_match::WrapSame::<#param_ty, _>::new(&_expr).wrap_same(); // if `_expr` is `&str`, convert `String` to `&str` - let (mut ty_check, match_borrow) = sqlx::ty_match::MatchBorrow::new(ty_check, &_expr); + let (mut _ty_check, match_borrow) = sqlx::ty_match::MatchBorrow::new(ty_check, &_expr); - ty_check = match_borrow.match_borrow(); + _ty_check = match_borrow.match_borrow(); // this causes move-analysis to effectively ignore this block panic!(); @@ -90,13 +92,13 @@ pub fn quote_args( } }; - let args_count = input.arg_names.len(); + let args_count = input.arg_exprs.len(); Ok(quote! { + #arg_bindings + #args_check - // bind as a local expression, by-ref - #(let #arg_name = &$#arg_name;)* let mut query_args = <#db_path as sqlx::database::HasArguments>::Arguments::default(); query_args.reserve( #args_count, @@ -114,3 +116,36 @@ fn get_type_override(expr: &Expr) -> Option<&Type> { _ => None, } } + +fn strip_wildcard(expr: Expr) -> Expr { + match expr { + Expr::Group(ExprGroup { + attrs, + group_token, + expr, + }) => Expr::Group(ExprGroup { + attrs, + group_token, + expr: Box::new(strip_wildcard(*expr)), + }), + // type ascription syntax is experimental so we always strip it + Expr::Type(ExprType { expr, .. }) => *expr, + // we want to retain casts if they semantically matter + Expr::Cast(ExprCast { + attrs, + expr, + as_token, + ty, + }) => match *ty { + // cast to wildcard `_` will produce weird errors; we interpret it as taking the value as-is + Type::Infer(_) => *expr, + _ => Expr::Cast(ExprCast { + attrs, + expr, + as_token, + ty, + }), + }, + _ => expr, + } +} diff --git a/sqlx-macros/src/query/input.rs b/sqlx-macros/src/query/input.rs index 7cd060df..b7a70699 100644 --- a/sqlx-macros/src/query/input.rs +++ b/sqlx-macros/src/query/input.rs @@ -2,7 +2,6 @@ use std::env; use std::fs; use proc_macro2::{Ident, Span}; -use quote::format_ident; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; use syn::{Expr, LitBool, LitStr, Token}; @@ -17,8 +16,6 @@ pub struct QueryMacroInput { pub(super) record_type: RecordType, - // `arg0 .. argN` for N arguments - pub(super) arg_names: Vec, pub(super) arg_exprs: Vec, pub(super) checked: bool, @@ -82,15 +79,11 @@ impl Parse for QueryMacroInput { query_src.ok_or_else(|| input.error("expected `source` or `source_file` key"))?; let arg_exprs = args.unwrap_or_default(); - let arg_names = (0..arg_exprs.len()) - .map(|i| format_ident!("arg{}", i)) - .collect(); Ok(QueryMacroInput { src: src.resolve(src_span)?, src_span, record_type, - arg_names, arg_exprs, checked, }) diff --git a/sqlx-macros/src/query/mod.rs b/sqlx-macros/src/query/mod.rs index 04f2193b..b3ca1e15 100644 --- a/sqlx-macros/src/query/mod.rs +++ b/sqlx-macros/src/query/mod.rs @@ -195,10 +195,10 @@ where }; if let Some(num) = num_parameters { - if num != input.arg_names.len() { + if num != input.arg_exprs.len() { return Err(syn::Error::new( Span::call_site(), - format!("expected {} parameters, got {}", num, input.arg_names.len()), + format!("expected {} parameters, got {}", num, input.arg_exprs.len()), ) .into()); } @@ -260,19 +260,13 @@ where record_tokens }; - let arg_names = &input.arg_names; + let ret_tokens = quote! {{ + use sqlx::Arguments as _; - let ret_tokens = quote! { - macro_rules! macro_result { - (#($#arg_names:expr),*) => {{ - use sqlx::Arguments as _; + #args_tokens - #args_tokens - - #output - }} - } - }; + #output + }}; #[cfg(feature = "offline")] { diff --git a/sqlx-macros/src/query/output.rs b/sqlx-macros/src/query/output.rs index 249ec9d9..89ccd840 100644 --- a/sqlx-macros/src/query/output.rs +++ b/sqlx-macros/src/query/output.rs @@ -100,16 +100,21 @@ pub fn quote_query_as( match (input.checked, type_) { // we guarantee the type is valid so we can skip the runtime check (true, Some(type_)) => quote! { - #ident: row.try_get_unchecked::<#type_, _>(#i).try_unwrap_optional()? + // binding to a `let` avoids confusing errors about + // "try expression alternatives have incompatible types" + // it doesn't seem to hurt inference in the other branches + let #ident = row.try_get_unchecked::<#type_, _>(#i)?; }, // type was overridden to be a wildcard so we fallback to the runtime check - (true, None) => quote! ( #ident: row.try_get(#i)? ), + (true, None) => quote! ( let #ident = row.try_get(#i)?; ), // macro is the `_unchecked!()` variant so this will die in decoding if it's wrong - (false, _) => quote!( #ident: row.try_get_unchecked(#i)? ), + (false, _) => quote!( let #ident = row.try_get_unchecked(#i)?; ), } }, ); + let ident = columns.iter().map(|col| &col.ident); + let db_path = DB::db_path(); let row_path = DB::row_path(); let sql = &input.src; @@ -117,9 +122,10 @@ pub fn quote_query_as( quote! { sqlx::query_with::<#db_path, _>(#sql, #bind_args).try_map(|row: #row_path| { use sqlx::Row as _; - use sqlx::result_ext::ResultExt as _; - Ok(#out_ty { #(#instantiations),* }) + #(#instantiations)* + + Ok(#out_ty { #(#ident: #ident),* }) }) } } diff --git a/src/lib.rs b/src/lib.rs index 125342ba..08df37dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,10 +69,6 @@ mod macros; #[doc(hidden)] pub mod ty_match; -#[cfg(feature = "macros")] -#[doc(hidden)] -pub mod result_ext; - /// Conversions between Rust and SQL types. /// /// To see how each SQL type maps to a Rust type, see the corresponding `types` module for each diff --git a/src/macros.rs b/src/macros.rs index 6cb4e635..d74ef43c 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -136,7 +136,7 @@ /// Type overrides are also available for output columns, utilizing the SQL standard's support /// for arbitrary text in column names: /// -/// * selecting a column `foo as "foo!"` (Postgres / SQLite) or `` foo as `foo!` `` overrides +/// * selecting a column `foo as "foo!"` (Postgres / SQLite) or `` foo as `foo!` `` (MySQL) overrides /// inferred nullability and forces the column to be treated as `NOT NULL`; this is useful e.g. for /// selecting expressions in Postgres where we cannot infer nullability: /// @@ -154,7 +154,7 @@ /// # } /// /// ``` -/// * selecting a column `foo as "foo?"` (Postgres / SQLite) or `` foo as `foo?` `` overrides +/// * selecting a column `foo as "foo?"` (Postgres / SQLite) or `` foo as `foo?` `` (MySQL) overrides /// inferred nullability and forces the column to be treated as nullable; this is provided mainly /// for symmetry with `!`, but also because nullability inference currently has some holes and false /// negatives that may not be completely fixable without doing our own complex analysis on the given @@ -255,21 +255,20 @@ #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query ( - // by emitting a macro definition from our proc-macro containing the result tokens, - // we no longer have a need for `proc-macro-hack` + // in Rust 1.45 we can now invoke proc macros in expression position ($query:expr) => ({ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source = $query); - } - macro_result!() + $crate::sqlx_macros::expand_query!(source = $query) }); - ($query:expr, $($args:expr),*$(,)?) => ({ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source = $query, args = [$($args),*]); - } - macro_result!($($args),*) + // RFC: this semantically should be `$($args:expr),*` (with `$(,)?` to allow trailing comma) + // but that doesn't work in 1.45 because `expr` fragments get wrapped in a way that changes + // their hygiene, which is fixed in 1.46 so this is technically just a temp. workaround. + // My question is: do we care? + // I was hoping using the `expr` fragment might aid code completion but it doesn't in my + // experience, at least not with IntelliJ-Rust at the time of writing (version 0.3.126.3220-201) + // so really the only benefit is making the macros _slightly_ self-documenting, but it's + // not like it makes them magically understandable at-a-glance. + ($query:expr, $($args:tt)*) => ({ + $crate::sqlx_macros::expand_query!(source = $query, args = [$($args)*]) }) ); @@ -279,18 +278,10 @@ macro_rules! query ( #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_unchecked ( ($query:expr) => ({ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source = $query, checked = false); - } - macro_result!() + $crate::sqlx_macros::expand_query!(source = $query, checked = false) }); - ($query:expr, $($args:expr),*$(,)?) => ({ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source = $query, args = [$($args),*], checked = false); - } - macro_result!($($args),*) + ($query:expr, $($args:tt)*) => ({ + $crate::sqlx_macros::expand_query!(source = $query, args = [$($args)*], checked = false) }) ); @@ -339,19 +330,11 @@ macro_rules! query_unchecked ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_file ( - ($path:literal) => (#[allow(dead_code)]{ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source_file = $path); - } - macro_result!() + ($path:literal) => ({ + $crate::sqlx_macros::expand_query!(source_file = $path) }); - ($path:literal, $($args:expr),*$(,)?) => (#[allow(dead_code)]{ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source_file = $path, args = [$($args),*]); - } - macro_result!($($args),*) + ($path:literal, $($args:tt)*) => ({ + $crate::sqlx_macros::expand_query!(source_file = $path, args = [$($args)*]) }) ); @@ -360,19 +343,11 @@ macro_rules! query_file ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_file_unchecked ( - ($path:literal) => (#[allow(dead_code)]{ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source_file = $path, checked = false); - } - macro_result!() + ($path:literal) => ({ + $crate::sqlx_macros::expand_query!(source_file = $path, checked = false) }); - ($path:literal, $($args:expr),*$(,)?) => (#[allow(dead_code)]{ - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(source_file = $path, args = [$($args),*], checked = false); - } - macro_result!($($args),*) + ($path:literal, $($args:tt)*) => ({ + $crate::sqlx_macros::expand_query!(source_file = $path, args = [$($args)*], checked = false) }) ); @@ -461,19 +436,11 @@ macro_rules! query_file_unchecked ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_as ( - ($out_struct:path, $query:expr) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query); - } - macro_result!() + ($out_struct:path, $query:expr) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query) }); - ($out_struct:path, $query:expr, $($args:expr),*$(,)?) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, args = [$($args),*]); - } - macro_result!($($args),*) + ($out_struct:path, $query:expr, $($args:tt)*) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, args = [$($args)*]) }) ); @@ -513,19 +480,11 @@ macro_rules! query_as ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_file_as ( - ($out_struct:path, $path:literal) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path); - } - macro_result!() + ($out_struct:path, $path:literal) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path) }); - ($out_struct:path, $path:literal, $($args:tt),*$(,)?) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, args = [$($args),*]); - } - macro_result!($($args),*) + ($out_struct:path, $path:literal, $($args:tt)*) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, args = [$($args)*]) }) ); @@ -534,20 +493,12 @@ macro_rules! query_file_as ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_as_unchecked ( - ($out_struct:path, $query:expr) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, checked = false); - } - macro_result!() + ($out_struct:path, $query:expr) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, checked = false) }); - ($out_struct:path, $query:expr, $($args:expr),*$(,)?) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, args = [$($args),*], checked = false); - } - macro_result!($($args),*) + ($out_struct:path, $query:expr, $($args:tt)*) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source = $query, args = [$($args)*], checked = false) }) ); @@ -557,19 +508,11 @@ macro_rules! query_as_unchecked ( #[macro_export] #[cfg_attr(docsrs, doc(cfg(feature = "macros")))] macro_rules! query_file_as_unchecked ( - ($out_struct:path, $path:literal) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, checked = false); - } - macro_result!() + ($out_struct:path, $path:literal) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, checked = false) }); - ($out_struct:path, $path:literal, $($args:tt),*$(,)?) => (#[allow(dead_code)] { - #[macro_use] - mod _macro_result { - $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, args = [$($args),*], checked = false); - } - macro_result!($($args),*) + ($out_struct:path, $path:literal, $($args:tt)*) => ( { + $crate::sqlx_macros::expand_query!(record = $out_struct, source_file = $path, args = [$($args)*], checked = false) }) ); diff --git a/src/result_ext.rs b/src/result_ext.rs deleted file mode 100644 index ad57d1d6..00000000 --- a/src/result_ext.rs +++ /dev/null @@ -1,35 +0,0 @@ -use crate::error::{Error, UnexpectedNullError}; - -pub trait ResultExt: Sized { - fn try_unwrap_optional(self) -> crate::Result; -} - -impl ResultExt for crate::Result { - fn try_unwrap_optional(self) -> crate::Result { - self - } -} - -impl ResultExt for crate::Result> { - fn try_unwrap_optional(self) -> crate::Result { - self?.ok_or_else(|| crate::Error::Decode(UnexpectedNullError.into())) - } -} - -impl ResultExt> for crate::Result { - fn try_unwrap_optional(self) -> crate::Result> { - match self { - Ok(val) => Ok(Some(val)), - - Err(Error::Decode(error)) => { - if let Some(UnexpectedNullError) = error.downcast_ref() { - Ok(None) - } else { - Err(Error::Decode(error)) - } - } - - Err(e) => Err(e), - } - } -} diff --git a/src/ty_match.rs b/src/ty_match.rs index ebaae86e..6bed04ff 100644 --- a/src/ty_match.rs +++ b/src/ty_match.rs @@ -81,6 +81,10 @@ impl<'a> MatchBorrowExt for MatchBorrow<&'a [u8], Vec> { type Matched = &'a [u8]; } +impl<'a, T> MatchBorrowExt for MatchBorrow<&'a T, T> { + type Matched = T; +} + impl MatchBorrowExt for &'_ MatchBorrow { type Matched = U; } @@ -118,5 +122,8 @@ fn test_match_borrow() { if false { let (_, match_borrow) = MatchBorrow::new("", &String::new()); let _: &str = match_borrow.match_borrow(); + + let (_, match_borrow) = MatchBorrow::new(&&0i64, &0i64); + let _: i64 = match_borrow.match_borrow(); } } diff --git a/tests/postgres/macros.rs b/tests/postgres/macros.rs index 2d8adc7c..32d6cfcd 100644 --- a/tests/postgres/macros.rs +++ b/tests/postgres/macros.rs @@ -125,7 +125,7 @@ async fn test_query_as() -> anyhow::Result<()> { let name: Option<&str> = None; let account = sqlx::query_as!( Account, - "SELECT * from (VALUES (1, $1)) accounts(id, name)", + r#"SELECT id "id!", name from (VALUES (1, $1)) accounts(id, name)"#, name ) .fetch_one(&mut conn) @@ -150,7 +150,7 @@ async fn test_query_as_raw() -> anyhow::Result<()> { let account = sqlx::query_as!( RawAccount, - "SELECT * from (VALUES (1, null)) accounts(type, name)" + r#"SELECT type "type!", name from (VALUES (1, null)) accounts(type, name)"# ) .fetch_one(&mut conn) .await?; @@ -202,6 +202,36 @@ async fn query_by_string() -> anyhow::Result<()> { Ok(()) } +#[sqlx_macros::test] +#[cfg(feature = "bigdecimal")] +async fn query_by_bigdecimal() -> anyhow::Result<()> { + use sqlx_core::types::BigDecimal; + let mut conn = new::().await?; + + // this tests querying by a non-`Copy` type that doesn't have special reborrow semantics + + let decimal = "1234".parse::()?; + let ref tuple = ("Hello, world!".to_string(),); + + let result = sqlx::query!( + "SELECT * from (VALUES(1234.0)) decimals(decimal)\ + where decimal in ($1, $2, $3, $4, $5, $6, $7)", + decimal, // make sure we don't actually take ownership here + &decimal, // allow query-by-reference + Some(&decimal), + Some(&decimal), + Option::::None, + decimal.clone(), + tuple.0 // make sure we're not trying to move out of a field expression + ) + .fetch_one(&mut conn) + .await?; + + assert_eq!(result.decimal, Some(decimal)); + + Ok(()) +} + #[sqlx_macros::test] async fn test_nullable_err() -> anyhow::Result<()> { #[derive(Debug)] @@ -214,7 +244,7 @@ async fn test_nullable_err() -> anyhow::Result<()> { let err = sqlx::query_as!( Account, - "SELECT * from (VALUES (1, null::text)) accounts(id, name)" + r#"SELECT id "id!", name "name!" from (VALUES (1, null::text)) accounts(id, name)"# ) .fetch_one(&mut conn) .await diff --git a/tests/postgres/test-query.sql b/tests/postgres/test-query.sql index cb8ab010..157a02f6 100644 --- a/tests/postgres/test-query.sql +++ b/tests/postgres/test-query.sql @@ -1 +1 @@ -SELECT * from (VALUES (1, null)) accounts(id, name) +SELECT id "id!", name from (VALUES (1, null)) accounts(id, name)