New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find a good place to document this thing #290

Closed
sgrif opened this Issue Apr 20, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@sgrif
Member

sgrif commented Apr 20, 2016

TL;DR: UFCS can improve our error messages, and we should document this, but I'm not sure where the right place is.

Random trick for debugging when Diesel gives an unhelpful error message. Yesterday when I was implementing QueryId for our entire AST, I got the following error for a node I had missed:

tests/expressions/date_and_time.rs:35:10: 35:14 error: no method named `load` found for type `diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>>` in the current scope
tests/expressions/date_and_time.rs:35         .load::<i32>(&connection);
                                               ^~~~
tests/expressions/date_and_time.rs:35:10: 35:14 note: the method `load` exists but the following trait bounds were not satisfied: `_ : diesel::query_builder::QueryId`, `_ : diesel::query_builder::QueryId`, `&mut diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>> : diesel::query_builder::AsQuery`, `_ : diesel::query_builder::QueryFragment<_>`, `_ : diesel::query_builder::QueryId`, `&mut diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>> : diesel::query_builder::Query`, `&mut diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>> : diesel::query_builder::Query`, `&mut diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>> : diesel::query_builder::Query`, `&mut diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Lt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>> : diesel::query_builder::Query`
tests/expressions/date_and_time.rs:38:10: 38:14 error: no method named `load` found for type `diesel::query_builder::SelectStatement<diesel::types::Integer, expressions::date_and_time::has_timestamps::columns::id, expressions::date_and_time::has_timestamps::table, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::WhereClause<diesel::expression::predicates::Gt<expressions::date_and_time::has_timestamps::columns::created_at, diesel::expression::now>>>` in the current scope
tests/expressions/date_and_time.rs:38         .load::<i32>(&connection);

(This is much noisier when your terminal is soft wrapping).

Unfortunately, Rust doesn't give the best error messages when the error is a "no method named x" that results from a trait not being implemented properly. The error is much more clear if we switch to UFCS.

By changing the code from:

    let before_today = has_timestamps.select(id)
        .filter(created_at.lt(now))
        .load::<i32>(&connection);

to

    let query = has_timestamps.select(id)
        .filter(created_at.lt(now));
    let before_today = LoadDsl::load::<i32>(query, &connection);

the error message becomes:

tests/expressions/date_and_time.rs:35:24: 35:44 error: the trait bound `diesel::expression::now: diesel::query_builder::QueryId` is not satisfied [E0277]
tests/expressions/date_and_time.rs:35     let before_today = LoadDsl::load::<i32>(query, &connection);

Which gives the exact bound needed. This can be extremely useful for user code as well, though in those cases it's likely due to user error and not a missing constraint in Diesel (e.g. something missing SelectableExpression). We should document this trick somewhere prominent, but I'm not sure where.

@gregcline

This comment has been minimized.

gregcline commented Apr 30, 2016

Maybe it could go in the LoadDSL trait documentation here: http://docs.diesel.rs/diesel/prelude/trait.LoadDsl.html
I know I ended up there looking for my different options for executing queries, and it seems that this trick depends on decoupling the actual execution call from building the query.

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 30, 2016

LoadDsl feels like a really weird place to document it, since this affects
all traits not that one. It is a likely place people are to encounter the
problem, though

On Sat, Apr 30, 2016 at 5:00 AM gregcline notifications@github.com wrote:

Maybe it could go in the LoadDSL trait documentation here:
http://docs.diesel.rs/diesel/prelude/trait.LoadDsl.html
I know I ended up there looking for my different options for executing
queries, and it seems that this trick depends on decoupling the actual
execution call from building the query.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#290 (comment)

@killercup killercup added this to Contributors and internal docs in Documentation Feb 12, 2017

@lancecarlson

This comment has been minimized.

Contributor

lancecarlson commented Aug 15, 2017

@sgrif Perhaps it would make sense to put it on this page somewhere?

http://docs.diesel.rs/diesel/prelude/index.html

It's a tad but awkward here too but seems closer? If you think this may sure, I can whip something up. Maybe on the bottom? Or above traits?

@killercup @Eijebong, thoughts?

@lancecarlson

This comment has been minimized.

Contributor

lancecarlson commented Aug 15, 2017

Actually, I think it should go here:

http://docs.diesel.rs/diesel/index.html

Under a debugging section. I also think the main page should have more stuff. Rocket's main API docs page does a good job and mimicking their format would make diesel's API docs seem more inviting.

@sgrif

This comment has been minimized.

Member

sgrif commented Dec 4, 2017

I'm going to do the same thing that I did in #1343 to LoadDsl and friends which makes this trick obsolete.

@sgrif sgrif closed this Dec 4, 2017

@sgrif sgrif removed this from Contributor & internal docs in Documentation Dec 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment