Improve the implementation

* Explicitly accept for which backend the relevant code is generated, which makes this an opt-in feature and allows the use with third party backends as WELL * Add documentation
diesel-rs · Feb 24, 2023 · 7541ace · 7541ace
1 parent d6d9260
commit 7541ace
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 121 deletions.
diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs
@@ -24,6 +24,15 @@ 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.
+///
 /// ## 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
@@ -1450,6 +1450,17 @@ 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.
+    ///
     /// # 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
@@ -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
@@ -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
@@ -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
@@ -1,47 +1,7 @@
-error[E0277]: the trait bound `i32: FromSql<diesel::sql_types::Text, Mysql>` is not satisfied
-  --> tests/fail/selectable_with_typemisamatch.rs:16:11
-   |
-16 |     name: i32,
-   |           ^^^ the trait `FromSql<diesel::sql_types::Text, Mysql>` is not implemented for `i32`
-   |
-   = help: the following other types implement trait `FromSql<A, DB>`:
-             <f32 as FromSql<diesel::sql_types::Float, Mysql>>
-             <f32 as FromSql<diesel::sql_types::Float, Pg>>
-             <f32 as FromSql<diesel::sql_types::Float, Sqlite>>
-             <f64 as FromSql<diesel::sql_types::Double, Mysql>>
-             <f64 as FromSql<diesel::sql_types::Double, Pg>>
-             <f64 as FromSql<diesel::sql_types::Double, Sqlite>>
-             <i16 as FromSql<diesel::sql_types::SmallInt, Mysql>>
-             <i16 as FromSql<diesel::sql_types::SmallInt, Pg>>
-           and 13 others
-   = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Text, Mysql>` for `i32`
-   = help: see issue #48214
-   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
-
-error[E0277]: the trait bound `i32: FromSql<diesel::sql_types::Text, Sqlite>` is not satisfied
-  --> tests/fail/selectable_with_typemisamatch.rs:16:11
-   |
-16 |     name: i32,
-   |           ^^^ the trait `FromSql<diesel::sql_types::Text, Sqlite>` is not implemented for `i32`
-   |
-   = help: the following other types implement trait `FromSql<A, DB>`:
-             <f32 as FromSql<diesel::sql_types::Float, Mysql>>
-             <f32 as FromSql<diesel::sql_types::Float, Pg>>
-             <f32 as FromSql<diesel::sql_types::Float, Sqlite>>
-             <f64 as FromSql<diesel::sql_types::Double, Mysql>>
-             <f64 as FromSql<diesel::sql_types::Double, Pg>>
-             <f64 as FromSql<diesel::sql_types::Double, Sqlite>>
-             <i16 as FromSql<diesel::sql_types::SmallInt, Mysql>>
-             <i16 as FromSql<diesel::sql_types::SmallInt, Pg>>
-           and 13 others
-   = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Text, Sqlite>` for `i32`
-   = help: see issue #48214
-   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
-
 error[E0277]: the trait bound `i32: FromSql<diesel::sql_types::Text, Pg>` 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<diesel::sql_types::Text, Pg>` is not implemented for `i32`
    |
    = help: the following other types implement trait `FromSql<A, DB>`:
@@ -55,53 +15,14 @@ error[E0277]: the trait bound `i32: FromSql<diesel::sql_types::Text, Pg>` is not
              <i16 as FromSql<diesel::sql_types::SmallInt, Pg>>
            and 13 others
    = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Text, Pg>` 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<diesel::sql_types::Integer, Mysql>` is not satisfied
-  --> tests/fail/selectable_with_typemisamatch.rs:15:9
-   |
-15 |     id: String,
-   |         ^^^^^^ the trait `FromSql<diesel::sql_types::Integer, Mysql>` is not implemented for `*const str`
-   |
-   = 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>>
-             <std::string::String as FromSql<ST, DB>>
-             <std::string::String as FromSql<TimestamptzSqlite, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Date, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Time, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Timestamp, Sqlite>>
-   = note: required because of the requirements on the impl of `FromSql<diesel::sql_types::Integer, Mysql>` for `std::string::String`
-   = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Integer, Mysql>` 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<diesel::sql_types::Integer, Sqlite>` is not satisfied
-  --> tests/fail/selectable_with_typemisamatch.rs:15:9
-   |
-15 |     id: String,
-   |         ^^^^^^ the trait `FromSql<diesel::sql_types::Integer, Sqlite>` is not implemented for `*const str`
-   |
-   = 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>>
-             <std::string::String as FromSql<ST, DB>>
-             <std::string::String as FromSql<TimestamptzSqlite, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Date, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Time, Sqlite>>
-             <std::string::String as FromSql<diesel::sql_types::Timestamp, Sqlite>>
-   = note: required because of the requirements on the impl of `FromSql<diesel::sql_types::Integer, Sqlite>` for `std::string::String`
-   = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Integer, Sqlite>` for `std::string::String`
+   = note: required because of the requirements on the impl of `FromSqlRow<diesel::sql_types::Text, Pg>` 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<diesel::sql_types::Integer, Pg>` 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<diesel::sql_types::Integer, Pg>` is not implemented for `*const str`
    |
    = help: the following other types implement trait `FromSql<A, DB>`:
@@ -115,5 +36,6 @@ error[E0277]: the trait bound `*const str: FromSql<diesel::sql_types::Integer, P
              <std::string::String as FromSql<diesel::sql_types::Timestamp, Sqlite>>
    = note: required because of the requirements on the impl of `FromSql<diesel::sql_types::Integer, Pg>` for `std::string::String`
    = note: required because of the requirements on the impl of `diesel::Queryable<diesel::sql_types::Integer, Pg>` for `std::string::String`
+   = note: required because of the requirements on the impl of `FromSqlRow<diesel::sql_types::Integer, Pg>` 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
@@ -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,
@@ -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<Ident, Comma>),
+    CheckForBackend(Ident, syn::punctuated::Punctuated<TypePath, syn::Token![,]>),
 }
 
 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
@@ -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
@@ -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,
         }
     }
 

diff --git 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::dsl::SqlTypeOf<#ty>, diesel::pg::Pg>
-                    });
-                }
-                if cfg!(feature = "sqlite") {
-                    out.push(quote::quote_spanned! {span=>
-                                                     #field_ty: diesel::prelude::Queryable<diesel::dsl::SqlTypeOf<#ty>, diesel::sqlite::Sqlite>
-                    });
-                }
-                if cfg!(feature = "mysql") {
-                    out.push(quote::quote_spanned! {span=>
-                                                     #field_ty: diesel::prelude::Queryable<diesel::dsl::SqlTypeOf<#ty>, 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<diesel::dsl::SqlTypeOf<#ty>, #b>
+                        }
+                    })
+                })
         });
 
     wrap_in_dummy_mod(quote! {