Add a flag to help users better debug bad Queryable impls
#1615
Conversation
diesel/src/deserialize.rs
Outdated
| /// name -> Text, | ||
| /// hair_color -> Nullable<Text>, | ||
| /// created_at -> Timestamp, | ||
| /// updated_at -> Timestamp, |
There was a problem hiding this comment.
Shouldn't this be Nullable<Timestamp>
| /// | | ||
| /// 24| / updated_at: SystemTime, | ||
| /// | | ||
| /// = note: expected type `users::columns::created_at` |
There was a problem hiding this comment.
Some generated code to assert order like this:
let users::updated_at = users::all_columns.4
That code gets a span which points at the field definition as its source location
There was a problem hiding this comment.
Ohhh I didn't see the order was wrong :D
| use self::std::result::Result::Err; | ||
| use self::std::convert::Into; | ||
|
|
||
| Err("`#[check_types]` is only for debugging purposes".into()) |
There was a problem hiding this comment.
I think that should rather panic than return a Err, because this is clearly a programmer mistake.
(We could even think about using compile_error! if this typechecks correctly.)
There was a problem hiding this comment.
Nah, because you could have lots of code, add a column to you Queryable struct and still want the project to compile instead of throwing 20 000 compile errors telling you that #[check_types] is for debugging only.
| /// found type `users::columns::created_at` | ||
| /// ``` | ||
| /// | ||
| /// Keep in mind that `#[check_types]` is only for debugging purposes. The impl |
There was a problem hiding this comment.
We should mention somewhere here that this feature needs the unstable feature and a nightly compiler.
There was a problem hiding this comment.
@sgrif I do not see this anywhere mentioned.
Since the impl we generate is so generic, when a bad `Queryable` impl is given, the error doesn't occur until they actually try to load a query. This ends up being really non-local, and gives little to no feedback about which field is actually the problem. There is a new option called `#[check_types]` which completely changes how the impl is generated, in such a way that when the `unstable` feature is enabled the error will point at the specific field that is the problem. The actual message is the same, but where it points makes debugging much easier. This flag also generates code which validates that your fields are in the right order to match your columns. Unfortunately, we cannot have this generate an actual usable impl. Trait bounds not matching in types or where clauses will always result in an error pointing at the derive itself, which is no more helpful than where we started. For that reason, we can't actually have any usable `Row` type. So this flag is for debugging purposes only. This does not help with cases where the number of fields is incorrect.
| @@ -82,6 +82,11 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/ | |||
| rounding issues. This is primarily to support things like `avg(int_col)`, | |||
| which we define as returning `Numeric` | |||
|
|
|||
| * `#[check_types]` has been added to help debug missing `Queryable` impls. See | |||
There was a problem hiding this comment.
They're not missing, just wrong impls
There was a problem hiding this comment.
I worded it this way because the message the user gets is literally "Queryable<...> is not implemented"
|
That's now basically implemented by #3359 |
Since the impl we generate is so generic, when a bad
Queryableimpl isgiven, the error doesn't occur until they actually try to load a query.
This ends up being really non-local, and gives little to no feedback
about which field is actually the problem.
There is a new option called
#[check_types]which completely changeshow the impl is generated, in such a way that when the
unstablefeature is enabled the error will point at the specific field that is
the problem. The actual message is the same, but where it points makes
debugging much easier.
This flag also generates code which validates that your fields are in
the right order to match your columns.
Unfortunately, we cannot have this generate an actual usable impl.
Trait bounds not matching in types or where clauses will always result
in an error pointing at the derive itself, which is no more helpful than
where we started. For that reason, we can't actually have any usable
Rowtype. So this flag is for debugging purposes only.This does not help with cases where the number of fields is incorrect.