Rewrite #[derive(QueryableByName)] in derives2

Since `QueryableByName` is one of the more recently written derives, it should have been a really straightforward port. Unfortunately, the tests for this derive hit multiple rustc bugs - rust-lang/rust#47983 - rust-lang/rust#47311 I love what we were able to do with the error message here. We could even go so far as to have the `help` lines point at the struct itself for the `table_name` annotation if we want to. I also much prefer the workaround for rust-lang/rust#47311 in this PR to the one I did in #1529. I'll need to update that PR if this is merged first.
diesel-rs · Feb 4, 2018 · 1d4bf99 · 1d4bf99
1 parent 486792a
commit 1d4bf99
Show file tree

Hide file tree

Showing 25 changed files with 225 additions and 237 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -20,7 +20,6 @@ script:
     (cd diesel && travis-cargo test -- --no-default-features --features "extras $BACKEND")
   fi &&
   (cd diesel && travis-cargo test -- --no-default-features --features "extras with-deprecated $BACKEND") &&
-  (cd diesel_derives && travis-cargo test -- --features "diesel/$BACKEND") &&
   (cd diesel_derives2 && travis-cargo test -- --features "$BACKEND") &&
   if [[ "$TRAVIS_RUST_VERSION" == nightly* ]]; then
     (cd diesel_derives2 && travis-cargo test -- --features "nightly $BACKEND")
@@ -48,7 +47,6 @@ matrix:
     script:
     - (cd diesel && cargo rustc --no-default-features --features "lint unstable sqlite postgres mysql extras" -- -Zno-trans)
     - (cd diesel_cli && cargo rustc --no-default-features --features "lint sqlite postgres mysql" -- -Zno-trans)
-    - (cd diesel_derives && cargo rustc --no-default-features --features "lint dotenv sqlite postgres mysql" -- -Zno-trans)
     - (cd diesel_derives2 && cargo rustc --no-default-features --features "lint" -- -Zno-trans)
     - (cd diesel_migrations && cargo rustc --no-default-features --features "lint dotenv sqlite postgres mysql" -- -Zno-trans)
     - (cd diesel_infer_schema && cargo rustc --no-default-features --features "lint dotenv sqlite postgres mysql" -- -Zno-trans)

diff --git a/bin/check b/bin/check
@@ -7,10 +7,6 @@ check() {
     cargo rustc --quiet --features "lint $BACKENDS $1" -- -Zno-trans
 }
 
-(cd diesel_derives &&
-    check &&
-    echo "✔ derives crate looks good!")
-
 (cd diesel_derives2 &&
     check &&
     echo "✔ derives2 crate looks good!")

diff --git a/bin/test b/bin/test
@@ -26,15 +26,13 @@ else
   (cd diesel_infer_schema/infer_schema_macros && cargo test --features "$CLIPPY sqlite" $*)
   (cd diesel_infer_schema && cargo test --features "$CLIPPY sqlite" $*)
   (cd diesel_migrations && cargo test --features "$CLIPPY sqlite" $*)
-  (cd diesel_derives && cargo test --features "$CLIPPY diesel/sqlite" $*)
   (cd diesel_derives2 && cargo test --features "$CLIPPY sqlite" $*)
   (cd diesel_tests && cargo test --features "$CLIPPY sqlite" --no-default-features $*)
 
   (cd diesel_infer_schema/infer_schema_internals && cargo test --features "$CLIPPY postgres" $*)
   (cd diesel_infer_schema/infer_schema_macros && cargo test --features "$CLIPPY postgres" $*)
   (cd diesel_infer_schema && cargo test --features "$CLIPPY postgres" $*)
   (cd diesel_migrations && cargo test --features "$CLIPPY postgres" $*)
-  (cd diesel_derives && cargo test --features "$CLIPPY diesel/postgres" $*)
   (cd diesel_derives2 && cargo test --features "$CLIPPY postgres" $*)
   (cd diesel_cli && cargo test --features "$CLIPPY postgres" --no-default-features $*)
   (cd diesel_tests && cargo test --features "$CLIPPY postgres" --no-default-features $*)
@@ -44,7 +42,6 @@ else
   (cd diesel_infer_schema/infer_schema_macros && cargo test --features "$CLIPPY mysql" $*)
   (cd diesel_infer_schema && cargo test --features "$CLIPPY mysql" $*)
   (cd diesel_migrations && cargo test --features "$CLIPPY mysql" $*)
-  (cd diesel_derives && cargo test --features "$CLIPPY diesel/mysql" $*)
   (cd diesel_derives2 && cargo test --features "$CLIPPY mysql" $*)
   (cd diesel_cli && cargo test --features "$CLIPPY mysql" --no-default-features $*)
   (cd diesel_tests && cargo test --features "$CLIPPY mysql" --no-default-features $*)

diff --git a/..._tests/tests/compile-fail/queryable_by_name_requires_table_name_or_sql_type_annotation.rs b/..._tests/tests/compile-fail/queryable_by_name_requires_table_name_or_sql_type_annotation.rs
diff --git a/...el_compile_tests/tests/ui/queryable_by_name_requires_table_name_or_sql_type_annotation.rs b/...el_compile_tests/tests/ui/queryable_by_name_requires_table_name_or_sql_type_annotation.rs
@@ -0,0 +1,12 @@
+#[macro_use] extern crate diesel;
+
+#[derive(QueryableByName)]
+struct Foo {
+    foo: i32,
+    bar: String,
+}
+
+#[derive(QueryableByName)]
+struct Bar(i32, String);
+
+fn main() {}
diff --git a/...ompile_tests/tests/ui/queryable_by_name_requires_table_name_or_sql_type_annotation.stderr b/...ompile_tests/tests/ui/queryable_by_name_requires_table_name_or_sql_type_annotation.stderr
@@ -0,0 +1,46 @@
+error: Cannot determine the SQL type of foo
+ --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:5:5
+  |
+5 |     foo: i32,
+  |     ^^^
+  |
+  = help: Your struct must either be annotated with `#[table_name = "foo"]` or have all of its fields annotated with `#[sql_type = "Integer"]`
+
+error: Cannot determine the SQL type of bar
+ --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:6:5
+  |
+6 |     bar: String,
+  |     ^^^
+  |
+  = help: Your struct must either be annotated with `#[table_name = "foo"]` or have all of its fields annotated with `#[sql_type = "Integer"]`
+
+error: All fields of tuple structs must be annotated with `#[column_name]`
+  --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:10:12
+   |
+10 | struct Bar(i32, String);
+   |            ^^^
+
+error: Cannot determine the SQL type of field
+  --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:10:12
+   |
+10 | struct Bar(i32, String);
+   |            ^^^
+   |
+   = help: Your struct must either be annotated with `#[table_name = "foo"]` or have all of its fields annotated with `#[sql_type = "Integer"]`
+
+error: All fields of tuple structs must be annotated with `#[column_name]`
+  --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:10:17
+   |
+10 | struct Bar(i32, String);
+   |                 ^^^^^^
+
+error: Cannot determine the SQL type of field
+  --> $DIR/queryable_by_name_requires_table_name_or_sql_type_annotation.rs:10:17
+   |
+10 | struct Bar(i32, String);
+   |                 ^^^^^^
+   |
+   = help: Your struct must either be annotated with `#[table_name = "foo"]` or have all of its fields annotated with `#[sql_type = "Integer"]`
+
+error: aborting due to 6 previous errors
+
diff --git a/diesel_derives/Cargo.toml b/diesel_derives/Cargo.toml
@@ -29,9 +29,6 @@ postgres = []
 sqlite = []
 mysql = []
 
-[[test]]
-name = "tests"
-
 [badges]
 travis-ci = { repository = "diesel-rs/diesel" }
 appveyor = { repository = "diesel-rs/diesel" }
diff --git a/diesel_derives/src/attr.rs b/diesel_derives/src/attr.rs
@@ -36,31 +36,6 @@ impl Attr {
         }
     }
 
-    pub fn field_name(&self) -> &syn::Ident {
-        self.field_name.as_ref().unwrap_or(&self.field_position)
-    }
-
-    pub fn column_name(&self) -> &syn::Ident {
-        self.column_name
-            .as_ref()
-            .or_else(|| self.field_name.as_ref())
-            .expect(
-                "All fields of tuple structs must be annotated with `#[column_name=\"something\"]`",
-            )
-    }
-
-    pub fn sql_type(&self) -> Option<&syn::Ty> {
-        self.sql_type.as_ref()
-    }
-
-    pub fn has_flag<T>(&self, flag: &T) -> bool
-    where
-        T: ?Sized,
-        syn::Ident: PartialEq<T>,
-    {
-        self.flags.iter().any(|f| f == flag)
-    }
-
     fn field_kind(&self) -> &str {
         if is_option_ty(&self.ty) {
             "option"

diff --git a/diesel_derives/src/lib.rs b/diesel_derives/src/lib.rs
@@ -28,18 +28,12 @@ mod ast_builder;
 mod attr;
 mod insertable;
 mod model;
-mod queryable_by_name;
 mod sql_type;
 mod util;
 
 use proc_macro::TokenStream;
 use syn::parse_derive_input;
 
-#[proc_macro_derive(QueryableByName, attributes(table_name, column_name, sql_type, diesel))]
-pub fn derive_queryable_by_name(input: TokenStream) -> TokenStream {
-    expand_derive(input, queryable_by_name::derive)
-}
-
 #[proc_macro_derive(Insertable, attributes(table_name, column_name))]
 pub fn derive_insertable(input: TokenStream) -> TokenStream {
     expand_derive(input, insertable::derive_insertable)

diff --git a/diesel_derives/src/model.rs b/diesel_derives/src/model.rs
@@ -50,11 +50,6 @@ impl Model {
     pub fn has_table_name_annotation(&self) -> bool {
         self.table_name_from_annotation.is_some()
     }
-
-    pub fn dummy_const_name(&self, trait_name: &str) -> syn::Ident {
-        let model_name = self.name.as_ref().to_uppercase();
-        format!("_IMPL_{}_FOR_{}", trait_name, model_name).into()
-    }
 }
 
 pub fn infer_association_name(name: &str) -> String {

diff --git a/diesel_derives/src/queryable_by_name.rs b/diesel_derives/src/queryable_by_name.rs
diff --git a/diesel_derives/tests/test_helpers.rs b/diesel_derives/tests/test_helpers.rs
diff --git a/diesel_derives/tests/tests.rs b/diesel_derives/tests/tests.rs
diff --git a/diesel_derives2/src/as_changeset.rs b/diesel_derives2/src/as_changeset.rs
@@ -86,7 +86,7 @@ fn field_changeset_expr(
     table_name: syn::Ident,
     treat_none_as_null: bool,
 ) -> syn::Expr {
-    let field_access = &field.name;
+    let field_access = field.name.access();
     let column_name = field.column_name();
     if !treat_none_as_null && is_option_ty(&field.ty) {
         parse_quote!(self#field_access.as_ref().map(|x| #table_name::#column_name.eq(x)))

diff --git a/diesel_derives2/src/associations.rs b/diesel_derives2/src/associations.rs
@@ -39,7 +39,7 @@ fn derive_belongs_to(
 
     let foreign_key_field = model.find_column(foreign_key)?;
     let struct_name = model.name;
-    let foreign_key_access = &foreign_key_field.name;
+    let foreign_key_access = foreign_key_field.name.access();
     let foreign_key_ty = inner_of_option_ty(&foreign_key_field.ty);
     let table_name = model.table_name();