diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 847a1ddd6ff0..f5ad09a1a5b7 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -24,6 +24,17 @@ pub type Result = result::Result>; /// /// This trait can be [derived](derive@Queryable) /// +/// ## How to resolve compiler errors while loading data from the database +/// +/// In case you getting uncomprehensable compiler errors while loading data +/// from the database you might want to consider using +/// [`#[derive(Selectable)]`](derive@crate::prelude::Selectable) + +/// `#[diesel(check_for_backend(YourBackendType))]` to check for mismatching fields at compile +/// time. This drastically improves the quality of the generated error messages by pointing +/// to concrete mismatches at field level. You need to specify the concrete database backend +/// this specific struct is indented to be used with, as otherwise rustc cannot correctly +/// identify the required deserialization implementation. +/// /// ## Interaction with `NULL`/`Option` /// [`Nullable`][crate::sql_types::Nullable] types can be queried into `Option`. /// This is valid for single fields, tuples, and structures with `Queryable`. @@ -409,6 +420,13 @@ pub use diesel_derives::QueryableByName; /// } /// } /// ``` +#[cfg_attr( + feature = "nightly-error-messages", + rustc_on_unimplemented( + message = "Cannot deserialize a value of the type `{A}` as `{Self}`", + note = "Double check your type mappings via the documentation of `{A}`" + ) +)] pub trait FromSql: Sized { /// See the trait documentation. fn from_sql(bytes: DB::RawValue<'_>) -> Result; diff --git a/diesel/src/query_dsl/mod.rs b/diesel/src/query_dsl/mod.rs index 09dabd851dd4..2d8f011a0de7 100644 --- a/diesel/src/query_dsl/mod.rs +++ b/diesel/src/query_dsl/mod.rs @@ -1450,6 +1450,19 @@ pub trait RunQueryDsl: Sized { /// [`execute`]: crate::query_dsl::RunQueryDsl::execute() /// [`sql_query`]: crate::sql_query() /// + /// ## How to resolve compiler errors while loading data from the database + /// + /// In case you getting uncomprehensable compiler errors while loading data + /// from the database into a type using [`#[derive(Queryable)]`](derive@crate::prelude::Queryable) + /// you might want to consider + /// using [`#[derive(Selectable)]`](derive@crate::prelude::Selectable) + + /// `#[diesel(check_for_backend(YourBackendType))]` + /// to check for mismatching fields at compile time. This drastically improves + /// the quality of the generated error messages by pointing to concrete type mismatches at + /// field level.You need to specify the concrete database backend + /// this specific struct is indented to be used with, as otherwise rustc cannot correctly + /// identify the required deserialization implementation. + /// /// # Examples /// /// ## Returning a single field diff --git a/diesel_compile_tests/tests/fail/derive/bad_primary_key.stderr b/diesel_compile_tests/tests/fail/derive/bad_primary_key.stderr index 165c191c9360..47086bf33846 100644 --- a/diesel_compile_tests/tests/fail/derive/bad_primary_key.stderr +++ b/diesel_compile_tests/tests/fail/derive/bad_primary_key.stderr @@ -15,6 +15,8 @@ error: unexpected end of input, expected parentheses | 26 | #[diesel(primary_key)] | ^ + | + = help: The correct format looks like `#[diesel(key1, key2)]` error: expected parentheses --> tests/fail/derive/bad_primary_key.rs:33:22 diff --git a/diesel_compile_tests/tests/fail/derive/unknown_attribute.stderr b/diesel_compile_tests/tests/fail/derive/unknown_attribute.stderr index 7dd0305d20f6..00c25a523190 100644 --- a/diesel_compile_tests/tests/fail/derive/unknown_attribute.stderr +++ b/diesel_compile_tests/tests/fail/derive/unknown_attribute.stderr @@ -1,11 +1,11 @@ -error: unknown attribute, expected one of `aggregate`, `not_sized`, `foreign_derive`, `table_name`, `sql_type`, `treat_none_as_default_value`, `treat_none_as_null`, `belongs_to`, `mysql_type`, `sqlite_type`, `postgres_type`, `primary_key` - --> $DIR/unknown_attribute.rs:5:10 +error: unknown attribute, expected one of `aggregate`, `not_sized`, `foreign_derive`, `table_name`, `sql_type`, `treat_none_as_default_value`, `treat_none_as_null`, `belongs_to`, `mysql_type`, `sqlite_type`, `postgres_type`, `primary_key`, `check_for_backend` + --> tests/fail/derive/unknown_attribute.rs:5:10 | 5 | #[diesel(what = true)] | ^^^^ error: unknown attribute, expected one of `embed`, `column_name`, `sql_type`, `serialize_as`, `deserialize_as`, `select_expression`, `select_expression_type` - --> $DIR/unknown_attribute.rs:12:14 + --> tests/fail/derive/unknown_attribute.rs:12:14 | 12 | #[diesel(what = true)] | ^^^^ diff --git a/diesel_compile_tests/tests/fail/derive_deprecated/deprecated_sql_type.stderr b/diesel_compile_tests/tests/fail/derive_deprecated/deprecated_sql_type.stderr index 4e2698e42dc5..f29a779f4d04 100644 --- a/diesel_compile_tests/tests/fail/derive_deprecated/deprecated_sql_type.stderr +++ b/diesel_compile_tests/tests/fail/derive_deprecated/deprecated_sql_type.stderr @@ -73,12 +73,13 @@ error[E0412]: cannot find type `foo` in this scope 27 | #[sql_type = "foo"] | ^^^^^ not found in this scope -error[E0277]: the trait bound `i32: FromSql<_, __DB>` is not satisfied +error[E0277]: Cannot deserialize a value of the type `_` as `i32` --> tests/fail/derive_deprecated/deprecated_sql_type.rs:25:10 | 25 | #[derive(QueryableByName)] | ^^^^^^^^^^^^^^^ the trait `FromSql<_, __DB>` is not implemented for `i32` | + = note: Double check your type mappings via the documentation of `_` = help: the following other types implement trait `FromSql`: > > diff --git a/diesel_compile_tests/tests/fail/select_carries_correct_result_type_info.stderr b/diesel_compile_tests/tests/fail/select_carries_correct_result_type_info.stderr index d686145a3691..504cf86a0419 100644 --- a/diesel_compile_tests/tests/fail/select_carries_correct_result_type_info.stderr +++ b/diesel_compile_tests/tests/fail/select_carries_correct_result_type_info.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `i32: FromSql` is not satisfied +error[E0277]: Cannot deserialize a value of the type `diesel::sql_types::Text` as `i32` --> tests/fail/select_carries_correct_result_type_info.rs:19:39 | 19 | let ids = select_name.load::(&mut connection); @@ -6,6 +6,7 @@ error[E0277]: the trait bound `i32: FromSql` is not | | | required by a bound introduced by this call | + = note: Double check your type mappings via the documentation of `diesel::sql_types::Text` = help: the following other types implement trait `FromSql`: > > @@ -20,7 +21,7 @@ note: required by a bound in `diesel::RunQueryDsl::load` | Self: LoadQuery<'query, Conn, U>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `RunQueryDsl::load` -error[E0277]: the trait bound `*const str: FromSql` is not satisfied +error[E0277]: Cannot deserialize a value of the type `diesel::sql_types::Integer` as `*const str` --> tests/fail/select_carries_correct_result_type_info.rs:20:42 | 20 | let names = select_id.load::(&mut connection); @@ -28,6 +29,7 @@ error[E0277]: the trait bound `*const str: FromSql`: <*const str as FromSql> <*const str as FromSql> diff --git a/diesel_compile_tests/tests/fail/select_sql_still_ensures_result_type.stderr b/diesel_compile_tests/tests/fail/select_sql_still_ensures_result_type.stderr index 9fd7499686f4..0c0802d0820c 100644 --- a/diesel_compile_tests/tests/fail/select_sql_still_ensures_result_type.stderr +++ b/diesel_compile_tests/tests/fail/select_sql_still_ensures_result_type.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `*const str: FromSql` is not satisfied +error[E0277]: Cannot deserialize a value of the type `BigInt` as `*const str` --> tests/fail/select_sql_still_ensures_result_type.rs:16:51 | 16 | let count = select_count.get_result::(&mut connection).unwrap(); @@ -6,6 +6,7 @@ error[E0277]: the trait bound `*const str: FromSql` is not satisfied | | | required by a bound introduced by this call | + = note: Double check your type mappings via the documentation of `BigInt` = help: the following other types implement trait `FromSql`: <*const str as FromSql> <*const str as FromSql> diff --git a/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs new file mode 100644 index 000000000000..d57365a5e0da --- /dev/null +++ b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs @@ -0,0 +1,39 @@ +extern crate diesel; + +use diesel::prelude::*; + +table! { + users { + id -> Integer, + name -> Text, + } +} + +#[derive(Selectable, Queryable)] +#[diesel(table_name = users)] +#[diesel(check_for_backend(diesel::pg::Pg))] +struct User { + id: String, + name: i32, +} + +#[derive(Selectable, Queryable)] +#[diesel(table_name = users)] +#[diesel(check_for_backend(diesel::pg::Pg))] +struct UserCorrect { + id: i32, + name: String, +} + +fn main() { + let mut conn = PgConnection::establish("...").unwrap(); + + users::table + .select(User::as_select()) + .load(&mut conn) + .unwrap(); + users::table + .select(UserCorrect::as_select()) + .load(&mut conn) + .unwrap(); +} diff --git a/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.stderr b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.stderr new file mode 100644 index 000000000000..e167f6f38fd6 --- /dev/null +++ b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.stderr @@ -0,0 +1,32 @@ +error[E0277]: Cannot deserialize a value of the type `diesel::sql_types::Text` as `i32` + --> tests/fail/selectable_with_typemisamatch.rs:17:11 + | +17 | name: i32, + | ^^^ the trait `FromSql` is not implemented for `i32` + | + = note: Double check your type mappings via the documentation of `diesel::sql_types::Text` + = help: the following other types implement trait `FromSql`: + > + > + > + = note: required for `i32` to implement `diesel::Queryable` + = note: required for `i32` to implement `FromSqlRow` + = help: see issue #48214 + = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable + +error[E0277]: Cannot deserialize a value of the type `diesel::sql_types::Integer` as `*const str` + --> tests/fail/selectable_with_typemisamatch.rs:16:9 + | +16 | id: String, + | ^^^^^^ the trait `FromSql` is not implemented for `*const str` + | + = note: Double check your type mappings via the documentation of `diesel::sql_types::Integer` + = help: the following other types implement trait `FromSql`: + <*const str as FromSql> + <*const str as FromSql> + <*const str as FromSql> + = note: required for `std::string::String` to implement `FromSql` + = note: required for `std::string::String` to implement `diesel::Queryable` + = note: required for `std::string::String` to implement `FromSqlRow` + = help: see issue #48214 + = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable diff --git a/diesel_derives/src/attrs.rs b/diesel_derives/src/attrs.rs index 9da55c748dea..414785f1cf35 100644 --- a/diesel_derives/src/attrs.rs +++ b/diesel_derives/src/attrs.rs @@ -8,7 +8,7 @@ use syn::parse::discouraged::Speculative; use syn::parse::{Parse, ParseStream, Parser, Result}; use syn::punctuated::Punctuated; use syn::token::Comma; -use syn::{parenthesized, Attribute, Ident, LitBool, LitStr, Path, Type, TypePath}; +use syn::{Attribute, Ident, LitBool, LitStr, Path, Type, TypePath}; use deprecated::ParseDeprecated; use parsers::{BelongsTo, MysqlType, PostgresType, SqliteType}; @@ -20,6 +20,7 @@ use util::{ }; use crate::field::SelectExpr; +use crate::util::{parse_paren_list, CHECK_FOR_BACKEND_NOTE}; pub struct AttributeSpanWrapper { pub item: T, @@ -123,7 +124,6 @@ impl Parse for FieldAttr { name, parse_eq(input, SELECT_EXPRESSION_TYPE_NOTE)?, )), - _ => unknown_attribute( &name, &[ @@ -170,6 +170,7 @@ pub enum StructAttr { SqliteType(Ident, SqliteType), PostgresType(Ident, PostgresType), PrimaryKey(Ident, Punctuated), + CheckForBackend(Ident, syn::punctuated::Punctuated), } impl Parse for StructAttr { @@ -212,11 +213,14 @@ impl Parse for StructAttr { name, parse_paren(input, POSTGRES_TYPE_NOTE)?, )), - "primary_key" => Ok(StructAttr::PrimaryKey(name, { - let content; - parenthesized!(content in input); - content.parse_terminated(Ident::parse)? - })), + "primary_key" => Ok(StructAttr::PrimaryKey( + name, + parse_paren_list(input, "key1, key2")?, + )), + "check_for_backend" => Ok(StructAttr::CheckForBackend( + name, + parse_paren_list(input, CHECK_FOR_BACKEND_NOTE)?, + )), _ => unknown_attribute( &name, @@ -233,6 +237,7 @@ impl Parse for StructAttr { "sqlite_type", "postgres_type", "primary_key", + "check_for_backend", ], ), } @@ -253,6 +258,7 @@ impl Spanned for StructAttr { | StructAttr::MysqlType(ident, _) | StructAttr::SqliteType(ident, _) | StructAttr::PostgresType(ident, _) + | StructAttr::CheckForBackend(ident, _) | StructAttr::PrimaryKey(ident, _) => ident.span(), } } diff --git a/diesel_derives/src/lib.rs b/diesel_derives/src/lib.rs index 9a84137666b3..bea281e47f72 100644 --- a/diesel_derives/src/lib.rs +++ b/diesel_derives/src/lib.rs @@ -1,4 +1,3 @@ -#![recursion_limit = "1024"] // Clippy lints #![allow( clippy::needless_doctest_main, @@ -455,7 +454,10 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream { /// **Note**: When this trait is derived, it will assume that __all fields on /// your struct__ matches __all fields in the query__, including the order and /// count. This means that field order is significant if you are using -/// `#[derive(Queryable)]`. __Field name has no effect__. +/// `#[derive(Queryable)]`. __Field name has no effect__. If you see errors while +/// loading data into a struct that derives `Queryable`: Consider using [`#[derive(Selectable)]`] +/// + `#[diesel(check_for_backend(YourBackendType))]` to check for mismatching fields at compile +/// time. /// /// To provide custom deserialization behavior for a field, you can use /// `#[diesel(deserialize_as = SomeType)]`. If this attribute is present, Diesel @@ -844,6 +846,16 @@ pub fn derive_queryable_by_name(input: TokenStream) -> TokenStream { /// If this attribute is not used, the type name converted to /// `snake_case` with an added `s` is used as table name. /// +/// ## Optional Type attributes +/// +/// * `#[diesel(check_for_backend(diesel::pg::Pg, diesel::mysql::Mysql))]`, instructs +/// the derive to generate additional code to identify potential type mismatches. +/// It accepts a list of backend types to check the types against. Using this option +/// will result in much better error messages in cases where some types in your `Queryable` +/// struct do not match. You need to specify the concrete database backend +/// this specific struct is indented to be used with, as otherwise rustc cannot correctly +/// identify the required deserialization implementation. +/// /// ## Field attributes /// /// * `#[diesel(column_name = some_column)]`, overrides the column name for diff --git a/diesel_derives/src/model.rs b/diesel_derives/src/model.rs index 0b10247e1951..847fb80049af 100644 --- a/diesel_derives/src/model.rs +++ b/diesel_derives/src/model.rs @@ -25,6 +25,7 @@ pub struct Model { pub mysql_type: Option, pub sqlite_type: Option, pub postgres_type: Option, + pub check_for_backend: Option>, fields: Vec, } @@ -61,6 +62,7 @@ impl Model { let mut mysql_type = None; let mut sqlite_type = None; let mut postgres_type = None; + let mut check_for_backend = None; for attr in parse_attributes(attrs) { match attr.item { @@ -80,6 +82,9 @@ impl Model { StructAttr::MysqlType(_, val) => mysql_type = Some(val), StructAttr::SqliteType(_, val) => sqlite_type = Some(val), StructAttr::PostgresType(_, val) => postgres_type = Some(val), + StructAttr::CheckForBackend(_, b) => { + check_for_backend = Some(b); + } } } @@ -100,6 +105,7 @@ impl Model { sqlite_type, postgres_type, fields: fields_from_item_data(fields), + check_for_backend, } } diff --git a/diesel_derives/src/selectable.rs b/diesel_derives/src/selectable.rs index 52d1ffe15a80..d799d711ae82 100644 --- a/diesel_derives/src/selectable.rs +++ b/diesel_derives/src/selectable.rs @@ -1,4 +1,5 @@ use proc_macro2::TokenStream; +use syn::spanned::Spanned; use syn::DeriveInput; use field::Field; @@ -28,9 +29,40 @@ pub fn derive(item: DeriveInput) -> TokenStream { let struct_name = &item.ident; - let field_columns_ty = model.fields().iter().map(|f| field_column_ty(f, &model)); + let field_columns_ty = model + .fields() + .iter() + .map(|f| field_column_ty(f, &model)) + .collect::>(); let field_columns_inst = model.fields().iter().map(|f| field_column_inst(f, &model)); + let check_function = if let Some(ref backends) = model.check_for_backend { + let field_check_bound = model + .fields() + .iter() + .zip(&field_columns_ty) + .filter(|(f, _)| !f.embed()) + .flat_map(|(f, ty)| { + backends.iter().map(move |b| { + let field_ty = to_field_ty_bound(&f.ty); + let span = field_ty.span(); + quote::quote_spanned! {span => + #field_ty: diesel::deserialize::FromSqlRow, #b> + } + }) + }); + Some(quote::quote! { + fn _check_field_compatibility() + where + #(#field_check_bound,)* + { + + } + }) + } else { + None + }; + wrap_in_dummy_mod(quote! { use diesel::expression::Selectable; @@ -44,9 +76,59 @@ pub fn derive(item: DeriveInput) -> TokenStream { (#(#field_columns_inst,)*) } } + + #check_function }) } +fn to_field_ty_bound(field_ty: &syn::Type) -> Option { + match field_ty { + syn::Type::Path(p) => { + if let syn::PathArguments::AngleBracketed(ref args) = + p.path.segments.last().unwrap().arguments + { + let lt = args + .args + .iter() + .filter_map(|f| { + if let syn::GenericArgument::Lifetime(lt) = f { + Some(lt) + } else { + None + } + }) + .collect::>(); + if lt.is_empty() { + Some(quote::quote! { + #field_ty + }) + } else if lt.len() == args.args.len() { + Some(quote::quote! { + for<#(#lt,)*> #field_ty + }) + } else { + // type parameters are not supported for checking + // for now + None + } + } else { + Some(quote::quote! { + #field_ty + }) + } + } + syn::Type::Reference(_r) => { + // references are not supported for checking for now + // + // (How ever you can even have references in a `Queryable` struct anyway) + None + } + field_ty => Some(quote::quote! { + #field_ty + }), + } +} + fn field_column_ty(field: &Field, model: &Model) -> TokenStream { if let Some(ref select_expression_type) = field.select_expression_type { let ty = &select_expression_type.item; @@ -62,8 +144,6 @@ fn field_column_ty(field: &Field, model: &Model) -> TokenStream { } fn field_column_inst(field: &Field, model: &Model) -> TokenStream { - use syn::spanned::Spanned; - if let Some(ref select_expression) = field.select_expression { let expr = &select_expression.item; let span = expr.span(); diff --git a/diesel_derives/src/util.rs b/diesel_derives/src/util.rs index 3ac60510cca1..42eeddb4c029 100644 --- a/diesel_derives/src/util.rs +++ b/diesel_derives/src/util.rs @@ -21,6 +21,7 @@ pub const SELECT_EXPRESSION_NOTE: &str = "select_expression = schema::table_name::column_name.is_not_null()"; pub const SELECT_EXPRESSION_TYPE_NOTE: &str = "select_expression_type = dsl::IsNotNull"; +pub const CHECK_FOR_BACKEND_NOTE: &str = "diesel::pg::Pg"; pub fn unknown_attribute(name: &Ident, valid: &[&str]) -> ! { let prefix = if valid.len() == 1 { "" } else { " one of" }; @@ -60,6 +61,23 @@ pub fn parse_paren(input: ParseStream, help: &str) -> Result { content.parse() } +pub fn parse_paren_list( + input: ParseStream, + help: &str, +) -> Result> { + if input.is_empty() { + abort!( + input.span(), + "unexpected end of input, expected parentheses"; + help = "The correct format looks like `#[diesel({})]`", help + ); + } + + let content; + parenthesized!(content in input); + content.parse_terminated(T::parse) +} + pub fn wrap_in_dummy_mod(item: TokenStream) -> TokenStream { quote! { #[allow(unused_imports)]