diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 847a1ddd6ff0..8bb1bccbf006 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -24,6 +24,15 @@ 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. +/// /// ## 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`. diff --git a/diesel/src/query_dsl/mod.rs b/diesel/src/query_dsl/mod.rs index 09dabd851dd4..de1bd6e06679 100644 --- a/diesel/src/query_dsl/mod.rs +++ b/diesel/src/query_dsl/mod.rs @@ -1450,6 +1450,17 @@ 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. + /// /// # 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/selectable_with_typemisamatch.rs b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs index b745bcd79fe6..d57365a5e0da 100644 --- a/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs +++ b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.rs @@ -11,16 +11,29 @@ table! { #[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(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 index 8d65851babbb..8c79d54f1a3d 100644 --- a/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.stderr +++ b/diesel_compile_tests/tests/fail/selectable_with_typemisamatch.stderr @@ -1,47 +1,7 @@ -error[E0277]: the trait bound `i32: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:16:11 - | -16 | name: i32, - | ^^^ the trait `FromSql` is not implemented for `i32` - | - = help: the following other types implement trait `FromSql`: - > - > - > - > - > - > - > - > - and 13 others - = note: required because of the requirements on the impl of `diesel::Queryable` for `i32` - = help: see issue #48214 - = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable - -error[E0277]: the trait bound `i32: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:16:11 - | -16 | name: i32, - | ^^^ the trait `FromSql` is not implemented for `i32` - | - = help: the following other types implement trait `FromSql`: - > - > - > - > - > - > - > - > - and 13 others - = note: required because of the requirements on the impl of `diesel::Queryable` for `i32` - = help: see issue #48214 - = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable - error[E0277]: the trait bound `i32: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:16:11 + --> tests/fail/selectable_with_typemisamatch.rs:17:11 | -16 | name: i32, +17 | name: i32, | ^^^ the trait `FromSql` is not implemented for `i32` | = help: the following other types implement trait `FromSql`: @@ -55,53 +15,14 @@ error[E0277]: the trait bound `i32: FromSql` is not > and 13 others = note: required because of the requirements on the impl of `diesel::Queryable` for `i32` - = help: see issue #48214 - = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable - -error[E0277]: the trait bound `*const str: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:15:9 - | -15 | id: String, - | ^^^^^^ the trait `FromSql` is not implemented for `*const str` - | - = help: the following other types implement trait `FromSql`: - <*const str as FromSql> - <*const str as FromSql> - <*const str as FromSql> - > - > - > - > - > - = note: required because of the requirements on the impl of `FromSql` for `std::string::String` - = note: required because of the requirements on the impl of `diesel::Queryable` for `std::string::String` - = help: see issue #48214 - = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable - -error[E0277]: the trait bound `*const str: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:15:9 - | -15 | id: String, - | ^^^^^^ the trait `FromSql` is not implemented for `*const str` - | - = help: the following other types implement trait `FromSql`: - <*const str as FromSql> - <*const str as FromSql> - <*const str as FromSql> - > - > - > - > - > - = note: required because of the requirements on the impl of `FromSql` for `std::string::String` - = note: required because of the requirements on the impl of `diesel::Queryable` for `std::string::String` + = note: required because of the requirements on the impl of `FromSqlRow` for `i32` = help: see issue #48214 = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable error[E0277]: the trait bound `*const str: FromSql` is not satisfied - --> tests/fail/selectable_with_typemisamatch.rs:15:9 + --> tests/fail/selectable_with_typemisamatch.rs:16:9 | -15 | id: String, +16 | id: String, | ^^^^^^ the trait `FromSql` is not implemented for `*const str` | = help: the following other types implement trait `FromSql`: @@ -115,5 +36,6 @@ error[E0277]: the trait bound `*const str: FromSql> = note: required because of the requirements on the impl of `FromSql` for `std::string::String` = note: required because of the requirements on the impl of `diesel::Queryable` for `std::string::String` + = note: required because of the requirements on the impl of `FromSqlRow` for `std::string::String` = 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..1fbe32678b0f 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,14 @@ 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. +/// /// ## 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 c1218ba5f328..ff54b70c0243 100644 --- a/diesel_derives/src/selectable.rs +++ b/diesel_derives/src/selectable.rs @@ -42,28 +42,19 @@ pub fn derive(item: DeriveInput) -> TokenStream { .zip(&field_columns_ty) .filter(|(f, _)| !f.embed()) .flat_map(|(f, ty)| { - let field_ty = to_field_ty_bound(&f.ty); - let span = field_ty.span(); - let mut out = Vec::new(); - if let Some(field_ty) = field_ty { - - if cfg!(feature = "postgres") { - out.push(quote::quote_spanned! {span=> - #field_ty: diesel::prelude::Queryable, diesel::pg::Pg> - }); - } - if cfg!(feature = "sqlite") { - out.push(quote::quote_spanned! {span=> - #field_ty: diesel::prelude::Queryable, diesel::sqlite::Sqlite> - }); - } - if cfg!(feature = "mysql") { - out.push(quote::quote_spanned! {span=> - #field_ty: diesel::prelude::Queryable, diesel::mysql::Mysql> - }); - } - } - out + model + .check_for_backend + .as_ref() + .into_iter() + .flat_map(move |d| { + let field_ty = to_field_ty_bound(&f.ty); + let span = field_ty.span(); + d.iter().map(move |b| { + quote::quote_spanned! {span => + #field_ty: diesel::deserialize::FromSqlRow, #b> + } + }) + }) }); wrap_in_dummy_mod(quote! { 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)]