Merge pull request #3359 from weiznich/try_error_messages

Add some additional checks to `#[derive(Selectable)]` to ensure field

diesel/src/deserialize.rs

@@ -24,6 +24,17 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 ///
 /// 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<A, DB: Backend>: Sized {
     /// See the trait documentation.
     fn from_sql(bytes: DB::RawValue<'_>) -> Result<Self>;

diesel/src/query_dsl/mod.rs

@@ -1450,6 +1450,19 @@ pub trait RunQueryDsl<Conn>: 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

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

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)]
    |              ^^^^

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<A, DB>`:
              <i32 as FromSql<diesel::sql_types::Integer, Mysql>>
              <i32 as FromSql<diesel::sql_types::Integer, Pg>>

diesel_compile_tests/tests/fail/select_carries_correct_result_type_info.stderr

@@ -1,11 +1,12 @@
-error[E0277]: the trait bound `i32: FromSql<diesel::sql_types::Text, _>` 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::<i32>(&mut connection);
    |                           ----        ^^^^^^^^^^^^^^^ the trait `FromSql<diesel::sql_types::Text, _>` is not implemented for `i32`
    |                           |
    |                           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<A, DB>`:
              <i32 as FromSql<diesel::sql_types::Integer, Mysql>>
              <i32 as FromSql<diesel::sql_types::Integer, Pg>>
@@ -20,14 +21,15 @@ 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<diesel::sql_types::Integer, _>` 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::<String>(&mut connection);
    |                           ----           ^^^^^^^^^^^^^^^ the trait `FromSql<diesel::sql_types::Integer, _>` is not implemented for `*const str`
    |                           |
    |                           required by a bound introduced by this call
    |
+   = note: Double check your type mappings via the documentation of `diesel::sql_types::Integer`
    = help: the following other types implement trait `FromSql<A, DB>`:
              <*const str as FromSql<diesel::sql_types::Text, Mysql>>
              <*const str as FromSql<diesel::sql_types::Text, Pg>>

diesel_compile_tests/tests/fail/select_sql_still_ensures_result_type.stderr

@@ -1,11 +1,12 @@
-error[E0277]: the trait bound `*const str: FromSql<BigInt, _>` 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::<String>(&mut connection).unwrap();
    |                              ----------           ^^^^^^^^^^^^^^^ the trait `FromSql<BigInt, _>` is not implemented for `*const str`
    |                              |
    |                              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<A, DB>`:
              <*const str as FromSql<diesel::sql_types::Text, Mysql>>
              <*const str as FromSql<diesel::sql_types::Text, Pg>>

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();
+}

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<diesel::sql_types::Text, Pg>` 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<A, DB>`:
+             <i32 as FromSql<diesel::sql_types::Integer, Mysql>>
+             <i32 as FromSql<diesel::sql_types::Integer, Pg>>
+             <i32 as FromSql<diesel::sql_types::Integer, Sqlite>>
+   = note: required for `i32` to implement `diesel::Queryable<diesel::sql_types::Text, Pg>`
+   = note: required for `i32` to implement `FromSqlRow<diesel::sql_types::Text, Pg>`
+   = 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<diesel::sql_types::Integer, Pg>` 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<A, DB>`:
+             <*const str as FromSql<diesel::sql_types::Text, Mysql>>
+             <*const str as FromSql<diesel::sql_types::Text, Pg>>
+             <*const str as FromSql<diesel::sql_types::Text, Sqlite>>
+   = note: required for `std::string::String` to implement `FromSql<diesel::sql_types::Integer, Pg>`
+   = note: required for `std::string::String` to implement `diesel::Queryable<diesel::sql_types::Integer, Pg>`
+   = note: required for `std::string::String` to implement `FromSqlRow<diesel::sql_types::Integer, Pg>`
+   = help: see issue #48214
+   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable

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<T> {
     pub item: T,
@@ -135,7 +136,6 @@ impl Parse for FieldAttr {
                 name,
                 parse_eq(input, SELECT_EXPRESSION_TYPE_NOTE)?,
             )),
-
             _ => unknown_attribute(
                 &name,
                 &[
@@ -182,6 +182,7 @@ pub enum StructAttr {
     SqliteType(Ident, SqliteType),
     PostgresType(Ident, PostgresType),
     PrimaryKey(Ident, Punctuated<Ident, Comma>),
+    CheckForBackend(Ident, syn::punctuated::Punctuated<TypePath, syn::Token![,]>),
 }
 
 impl Parse for StructAttr {
@@ -224,11 +225,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,
@@ -245,6 +249,7 @@ impl Parse for StructAttr {
                     "sqlite_type",
                     "postgres_type",
                     "primary_key",
+                    "check_for_backend",
                 ],
             ),
         }
@@ -265,6 +270,7 @@ impl Spanned for StructAttr {
             | StructAttr::MysqlType(ident, _)
             | StructAttr::SqliteType(ident, _)
             | StructAttr::PostgresType(ident, _)
+            | StructAttr::CheckForBackend(ident, _)
             | StructAttr::PrimaryKey(ident, _) => ident.span(),
         }
     }

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

diesel_derives/src/model.rs

@@ -25,6 +25,7 @@ pub struct Model {
     pub mysql_type: Option<MysqlType>,
     pub sqlite_type: Option<SqliteType>,
     pub postgres_type: Option<PostgresType>,
+    pub check_for_backend: Option<syn::punctuated::Punctuated<syn::TypePath, syn::Token![,]>>,
     fields: Vec<Field>,
 }
 
@@ -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,
         }
     }