refactor(macros): remove ResultExt and macro_result!()

This commit is contained in:
Austin Bonander 2020-07-19 21:25:08 -07:00 committed by Austin Bonander
parent d79335d955
commit 94aa698581
No known key found for this signature in database
GPG Key ID: 4E7DA63E66AFC37E
11 changed files with 156 additions and 196 deletions

View File

@ -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::<syn::Error>() {
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()
}
}
}

View File

@ -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<DB: DatabaseExt>(
) -> crate::Result<TokenStream> {
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::<Vec<_>>();
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<DB: DatabaseExt>(
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(&param_ty)
@ -72,14 +74,14 @@ pub fn quote_args<DB: DatabaseExt>(
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<T>`, 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<DB: DatabaseExt>(
}
};
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,
}
}

View File

@ -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<Ident>,
pub(super) arg_exprs: Vec<Expr>,
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,
})

View File

@ -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")]
{

View File

@ -100,16 +100,21 @@ pub fn quote_query_as<DB: DatabaseExt>(
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<DB: DatabaseExt>(
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),* })
})
}
}

View File

@ -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

View File

@ -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)
})
);

View File

@ -1,35 +0,0 @@
use crate::error::{Error, UnexpectedNullError};
pub trait ResultExt<T>: Sized {
fn try_unwrap_optional(self) -> crate::Result<T>;
}
impl<T> ResultExt<T> for crate::Result<T> {
fn try_unwrap_optional(self) -> crate::Result<T> {
self
}
}
impl<T> ResultExt<T> for crate::Result<Option<T>> {
fn try_unwrap_optional(self) -> crate::Result<T> {
self?.ok_or_else(|| crate::Error::Decode(UnexpectedNullError.into()))
}
}
impl<T> ResultExt<Option<T>> for crate::Result<T> {
fn try_unwrap_optional(self) -> crate::Result<Option<T>> {
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),
}
}
}

View File

@ -81,6 +81,10 @@ impl<'a> MatchBorrowExt for MatchBorrow<&'a [u8], Vec<u8>> {
type Matched = &'a [u8];
}
impl<'a, T> MatchBorrowExt for MatchBorrow<&'a T, T> {
type Matched = T;
}
impl<T, U> MatchBorrowExt for &'_ MatchBorrow<T, U> {
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();
}
}

View File

@ -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::<Postgres>().await?;
// this tests querying by a non-`Copy` type that doesn't have special reborrow semantics
let decimal = "1234".parse::<BigDecimal>()?;
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::<BigDecimal>::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

View File

@ -1 +1 @@
SELECT * from (VALUES (1, null)) accounts(id, name)
SELECT id "id!", name from (VALUES (1, null)) accounts(id, name)