Skip to content

Commit

Permalink
Rewrite #[derive(QueryableByName)] in derives2
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sgrif committed Feb 3, 2018
1 parent 8e5e59c commit ecea8fe
Show file tree
Hide file tree
Showing 25 changed files with 254 additions and 238 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions bin/check
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down
3 changes: 0 additions & 3 deletions bin/test
Original file line number Diff line number Diff line change
Expand Up @@ -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 $*)
Expand All @@ -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 $*)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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

3 changes: 0 additions & 3 deletions diesel_derives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ postgres = []
sqlite = []
mysql = []

[[test]]
name = "tests"

[badges]
travis-ci = { repository = "diesel-rs/diesel" }
appveyor = { repository = "diesel-rs/diesel" }
25 changes: 0 additions & 25 deletions diesel_derives/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 0 additions & 6 deletions diesel_derives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,12 @@ mod attr;
mod from_sql_row;
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)
Expand Down
5 changes: 0 additions & 5 deletions diesel_derives/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
76 changes: 0 additions & 76 deletions diesel_derives/src/queryable_by_name.rs

This file was deleted.

56 changes: 0 additions & 56 deletions diesel_derives/tests/test_helpers.rs

This file was deleted.

9 changes: 0 additions & 9 deletions diesel_derives/tests/tests.rs

This file was deleted.

2 changes: 1 addition & 1 deletion diesel_derives2/src/as_changeset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
2 changes: 1 addition & 1 deletion diesel_derives2/src/associations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Loading

0 comments on commit ecea8fe

Please sign in to comment.