fix: only warn on ambiguous crates if the invocation relies on it

This commit is contained in:
Austin Bonander 2025-03-30 03:42:28 -07:00
parent 4e95bafb35
commit 732233a5da
4 changed files with 130 additions and 20 deletions

View File

@ -60,6 +60,10 @@ pub enum Error {
DateTimeCrateFeatureNotEnabled,
#[error("Cargo feature for configured `macros.preferred-crates.numeric` not enabled")]
NumericCrateFeatureNotEnabled,
#[error("multiple date-time types are possible; falling back to `{fallback}`")]
AmbiguousDateTimeType { fallback: &'static str },
#[error("multiple numeric types are possible; falling back to `{fallback}`")]
AmbiguousNumericType { fallback: &'static str },
}
/// An adapter for [`Value`] which attempts to decode the value and format it when printed using [`Debug`].
@ -195,12 +199,24 @@ macro_rules! impl_type_checking {
if matches!(preferred_crates.date_time, DateTimeCrate::Time | DateTimeCrate::Inferred) {
$(
if <$time_ty as sqlx_core::types::Type<$database>>::type_info() == *info {
if cfg!(feature = "chrono") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: $crate::select_input_type!($time_ty $(, $time_input)?),
});
}
return Ok($crate::select_input_type!($time_ty $(, $time_input)?));
}
)*
$(
if <$time_ty as sqlx_core::types::Type<$database>>::compatible(info) {
if cfg!(feature = "chrono") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: $crate::select_input_type!($time_ty $(, $time_input)?),
});
}
return Ok($crate::select_input_type!($time_ty $(, $time_input)?));
}
)*
@ -240,12 +256,24 @@ macro_rules! impl_type_checking {
if matches!(preferred_crates.numeric, NumericCrate::BigDecimal | NumericCrate::Inferred) {
$(
if <$bigdecimal_ty as sqlx_core::types::Type<$database>>::type_info() == *info {
if cfg!(feature = "rust_decimal") {
return Err($crate::type_checking::Error::AmbiguousNumericType {
fallback: $crate::select_input_type!($bigdecimal_ty $(, $bigdecimal_input)?),
});
}
return Ok($crate::select_input_type!($bigdecimal_ty $(, $bigdecimal_input)?));
}
)*
$(
if <$bigdecimal_ty as sqlx_core::types::Type<$database>>::compatible(info) {
if cfg!(feature = "rust_decimal") {
return Err($crate::type_checking::Error::AmbiguousNumericType {
fallback: $crate::select_input_type!($bigdecimal_ty $(, $bigdecimal_input)?),
});
}
return Ok($crate::select_input_type!($bigdecimal_ty $(, $bigdecimal_input)?));
}
)*
@ -311,12 +339,24 @@ macro_rules! impl_type_checking {
if matches!(preferred_crates.date_time, DateTimeCrate::Time | DateTimeCrate::Inferred) {
$(
if <$time_ty as sqlx_core::types::Type<$database>>::type_info() == *info {
if cfg!(feature = "chrono") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: stringify!($time_ty),
});
}
return Ok(stringify!($time_ty));
}
)*
$(
if <$time_ty as sqlx_core::types::Type<$database>>::compatible(info) {
if cfg!(feature = "chrono") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: stringify!($time_ty),
});
}
return Ok(stringify!($time_ty));
}
)*
@ -356,12 +396,24 @@ macro_rules! impl_type_checking {
if matches!(preferred_crates.numeric, NumericCrate::BigDecimal | NumericCrate::Inferred) {
$(
if <$bigdecimal_ty as sqlx_core::types::Type<$database>>::type_info() == *info {
if cfg!(feature = "rust_decimal") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: stringify!($bigdecimal_ty),
});
}
return Ok(stringify!($bigdecimal_ty));
}
)*
$(
if <$bigdecimal_ty as sqlx_core::types::Type<$database>>::compatible(info) {
if cfg!(feature = "rust_decimal") {
return Err($crate::type_checking::Error::AmbiguousDateTimeType {
fallback: stringify!($bigdecimal_ty),
});
}
return Ok(stringify!($bigdecimal_ty));
}
)*
@ -438,6 +490,24 @@ macro_rules! impl_type_checking {
)*
}
#[cfg(feature = "bigdecimal")]
{
$(
if <$bigdecimal_ty as sqlx_core::types::Type<$database>>::compatible(&info) {
return $crate::type_checking::FmtValue::debug::<$bigdecimal_ty>(value);
}
)*
}
#[cfg(feature = "rust_decimal")]
{
$(
if <$rust_decimal_ty as sqlx_core::types::Type<$database>>::compatible(&info) {
return $crate::type_checking::FmtValue::debug::<$rust_decimal_ty>(value);
}
)*
}
$(
$(#[$meta])?
if <$ty as sqlx_core::types::Type<$database>>::compatible(&info) {

View File

@ -1,11 +1,12 @@
use crate::database::DatabaseExt;
use crate::query::QueryMacroInput;
use crate::query::{QueryMacroInput, Warnings};
use either::Either;
use proc_macro2::TokenStream;
use quote::{format_ident, quote, quote_spanned};
use sqlx_core::config::Config;
use sqlx_core::describe::Describe;
use sqlx_core::type_checking;
use sqlx_core::type_checking::Error;
use sqlx_core::type_info::TypeInfo;
use syn::spanned::Spanned;
use syn::{Expr, ExprCast, ExprGroup, Type};
@ -15,6 +16,7 @@ use syn::{Expr, ExprCast, ExprGroup, Type};
pub fn quote_args<DB: DatabaseExt>(
input: &QueryMacroInput,
config: &Config,
warnings: &mut Warnings,
info: &Describe<DB>,
) -> crate::Result<TokenStream> {
let db_path = DB::db_path();
@ -59,7 +61,7 @@ pub fn quote_args<DB: DatabaseExt>(
return Ok(quote!());
}
let param_ty = get_param_type::<DB>(param_ty, config, i)?;
let param_ty = get_param_type::<DB>(param_ty, config, warnings, i)?;
Ok(quote_spanned!(expr.span() =>
// this shouldn't actually run
@ -107,6 +109,7 @@ pub fn quote_args<DB: DatabaseExt>(
fn get_param_type<DB: DatabaseExt>(
param_ty: &DB::TypeInfo,
config: &Config,
warnings: &mut Warnings,
i: usize,
) -> crate::Result<TokenStream> {
if let Some(type_override) = config.macros.type_override(param_ty.name()) {
@ -156,6 +159,15 @@ fn get_param_type<DB: DatabaseExt>(
(configured by `macros.preferred-crates.numeric` in sqlx.toml)",
)
}
Error::AmbiguousDateTimeType { fallback } => {
warnings.ambiguous_datetime = true;
return Ok(fallback.parse()?);
}
Error::AmbiguousNumericType { fallback } => {
warnings.ambiguous_numeric = true;
return Ok(fallback.parse()?);
}
};
Err(message.into())

View File

@ -241,6 +241,12 @@ impl<DB: Database> DescribeExt for Describe<DB> where
{
}
#[derive(Default)]
struct Warnings {
ambiguous_datetime: bool,
ambiguous_numeric: bool,
}
fn expand_with_data<DB: DatabaseExt>(
input: QueryMacroInput,
data: QueryData<DB>,
@ -267,7 +273,9 @@ where
}
}
let args_tokens = args::quote_args(&input, config, &data.describe)?;
let mut warnings = Warnings::default();
let args_tokens = args::quote_args(&input, config, &mut warnings, &data.describe)?;
let query_args = format_ident!("query_args");
@ -286,7 +294,7 @@ where
} else {
match input.record_type {
RecordType::Generated => {
let columns = output::columns_to_rust::<DB>(&data.describe, config)?;
let columns = output::columns_to_rust::<DB>(&data.describe, config, &mut warnings)?;
let record_name: Type = syn::parse_str("Record").unwrap();
@ -322,28 +330,32 @@ where
record_tokens
}
RecordType::Given(ref out_ty) => {
let columns = output::columns_to_rust::<DB>(&data.describe, config)?;
let columns = output::columns_to_rust::<DB>(&data.describe, config, &mut warnings)?;
output::quote_query_as::<DB>(&input, out_ty, &query_args, &columns)
}
RecordType::Scalar => {
output::quote_query_scalar::<DB>(&input, config, &query_args, &data.describe)?
}
RecordType::Scalar => output::quote_query_scalar::<DB>(
&input,
config,
&mut warnings,
&query_args,
&data.describe,
)?,
}
};
let mut warnings = TokenStream::new();
let mut warnings_out = TokenStream::new();
if config.macros.preferred_crates.date_time.is_inferred() {
if warnings.ambiguous_datetime {
// Warns if the date-time crate is inferred but both `chrono` and `time` are enabled
warnings.extend(quote! {
warnings_out.extend(quote! {
::sqlx::warn_on_ambiguous_inferred_date_time_crate();
});
}
if config.macros.preferred_crates.numeric.is_inferred() {
if warnings.ambiguous_numeric {
// Warns if the numeric crate is inferred but both `bigdecimal` and `rust_decimal` are enabled
warnings.extend(quote! {
warnings_out.extend(quote! {
::sqlx::warn_on_ambiguous_inferred_numeric_crate();
});
}
@ -354,7 +366,7 @@ where
{
use ::sqlx::Arguments as _;
#warnings
#warnings_out
#args_tokens

View File

@ -7,7 +7,7 @@ use sqlx_core::describe::Describe;
use crate::database::DatabaseExt;
use crate::query::QueryMacroInput;
use crate::query::{QueryMacroInput, Warnings};
use sqlx_core::config::Config;
use sqlx_core::type_checking;
use sqlx_core::type_checking::TypeChecking;
@ -82,15 +82,17 @@ impl Display for DisplayColumn<'_> {
pub fn columns_to_rust<DB: DatabaseExt>(
describe: &Describe<DB>,
config: &Config,
warnings: &mut Warnings,
) -> crate::Result<Vec<RustColumn>> {
(0..describe.columns().len())
.map(|i| column_to_rust(describe, config, i))
.map(|i| column_to_rust(describe, config, warnings, i))
.collect::<crate::Result<Vec<_>>>()
}
fn column_to_rust<DB: DatabaseExt>(
describe: &Describe<DB>,
config: &Config,
warnings: &mut Warnings,
i: usize,
) -> crate::Result<RustColumn> {
let column = &describe.columns()[i];
@ -116,7 +118,7 @@ fn column_to_rust<DB: DatabaseExt>(
(ColumnTypeOverride::Wildcard, true) => ColumnType::OptWildcard,
(ColumnTypeOverride::None, _) => {
let type_ = get_column_type::<DB>(config, i, column);
let type_ = get_column_type::<DB>(config, warnings, i, column);
if !nullable {
ColumnType::Exact(type_)
} else {
@ -204,6 +206,7 @@ pub fn quote_query_as<DB: DatabaseExt>(
pub fn quote_query_scalar<DB: DatabaseExt>(
input: &QueryMacroInput,
config: &Config,
warnings: &mut Warnings,
bind_args: &Ident,
describe: &Describe<DB>,
) -> crate::Result<TokenStream> {
@ -218,10 +221,10 @@ pub fn quote_query_scalar<DB: DatabaseExt>(
}
// attempt to parse a column override, otherwise fall back to the inferred type of the column
let ty = if let Ok(rust_col) = column_to_rust(describe, config, 0) {
let ty = if let Ok(rust_col) = column_to_rust(describe, config, warnings, 0) {
rust_col.type_.to_token_stream()
} else if input.checked {
let ty = get_column_type::<DB>(config, 0, &columns[0]);
let ty = get_column_type::<DB>(config, warnings, 0, &columns[0]);
if describe.nullable(0).unwrap_or(true) {
quote! { ::std::option::Option<#ty> }
} else {
@ -239,7 +242,12 @@ pub fn quote_query_scalar<DB: DatabaseExt>(
})
}
fn get_column_type<DB: DatabaseExt>(config: &Config, i: usize, column: &DB::Column) -> TokenStream {
fn get_column_type<DB: DatabaseExt>(
config: &Config,
warnings: &mut Warnings,
i: usize,
column: &DB::Column,
) -> TokenStream {
if let ColumnOrigin::Table(origin) = column.origin() {
if let Some(column_override) = config.macros.column_override(&origin.table, &origin.name) {
return column_override.parse().unwrap();
@ -322,6 +330,14 @@ fn get_column_type<DB: DatabaseExt>(config: &Config, i: usize, column: &DB::Colu
}
)
}
type_checking::Error::AmbiguousDateTimeType { fallback } => {
warnings.ambiguous_datetime = true;
return fallback.parse().unwrap();
}
type_checking::Error::AmbiguousNumericType { fallback } => {
warnings.ambiguous_numeric = true;
return fallback.parse().unwrap();
}
};
syn::Error::new(Span::call_site(), message).to_compile_error()