Diesel query fragment function return type auto inference

[Ten0]

Heyy!
More and more I found myself getting bored having to write return types for query fragment functions, so yesterday I decided to do the not-boring thing that would enable not being bored anymore.

Meet my prototype of diesel_auto_type!

// this compiles:
#[auto_type]
fn some_query_fragment(y: i32) -> _ {
	let x = 1; // ambiguous type
	let ((a,), (b,)): ((i32,), _) = ((x,), (y,)); // disambiguates x as a and showcases y's type being propagated to b
	dsl::not(abc::id.eq(a).or(abc::id.eq(b))) // Type of this is inferred and put as return type of the function
}

and more:
https://github.com/Ten0/diesel_auto_type/blob/master/tests/test.rs

It reads through the let assignments and expressions of the function to infer the return expression type using the basic convention of the existence of a corresponding helper type, same path as the function for functions, and PascalCase in the dsl module for methods.

Now there are some questions about how to move forward with this, I'm putting those as sub-threads below.

[Ten0]

So the first question is obviously: where should this live?

I can see 3 options (associated with their advantages):

1. As a separate diesel_auto_type crate owned by me or the diesel-rs organization

• Not much extra to commit on with respect to stability and maintenance
  • Notably this allows breaking changes
  • I've got darling and heck as dependencies in there - though both of these are very lightweight, >40M downloads and probably-already-in-the-tree dependencies, and we might benefit from using darling elsewhere - only darling is on the critical path because it depends on syn)
• It is so generic it currently isn't even specific to Diesel at all: it could be used for any domain-specific-language crate that follows the same conventions in a dsl module. (Actually that also means even if that was in Diesel there wouldn't be much extra to maintain either: we'd just need to make sure to respect the type alias casing conventions)

2. Within Diesel

• Super visible to users that it's now easy to factor query fragments
• We can use it to avoid having to specify select_expression_type when we specify select_expression in Selectable

3. As one/two separate crates owned by me & an additional dependency in Diesel (so used and exposed by Diesel, but most the non-specific inference code would be as a separate crate): one non-proc-macro auto_type_core with most of the syn logic, tailored to be parametrizable with ident casing conventions and dsl module path (so that it could match with Diesel) (alternately this one could also be owned by the diesel-rs organization), integration as a dependency within diesel_derives both in Selectable and as an auto_type attribute macro, and optionally an extra dsl_auto_type proc-macro crate owned by me in case somebody wants to ~infer its function return types for e.g. similar cases of large DSL types.

• Best of both worlds, but additions to that crate, if they are required, would require a more complex dev flow. (I'm going to test this on our codebase soon so I'll probably be soon reasonably-confident-enough that this won't be necessary often)

I think 3 would be my personal favorite here.

[Ten0]

EDIT: Question solved by #3662 merge

---

Second question is: how do we deal with inference around boolean expressions?

We have these implementations for boolean expressions:

diesel/diesel/src/expression_methods/bool_expression_methods.rs

Lines 41 to 46 in a4e6131

	fn and<T, ST>(self, other: T) -> dsl::And<Self, T, ST>
	where
	Self::SqlType: SqlType,
	ST: SqlType + TypedExpressionType,
	T: AsExpression<ST>,
	And<Self, T::Expression>: Expression,

diesel/diesel/src/expression/helper_types.rs

Lines 97 to 98 in a4e6131

	pub type And<Lhs, Rhs, ST = sql_types::Bool> =
	Grouped<super::operators::And<Lhs, AsExprOf<Rhs, ST>>>;

Latest design discussions on why we currently have this constraint: #2589 (comment)

So that currently won't work with our basic auto_type inference rule if b in a.eq(b) is an Expression<SqlType = Nullable<Bool>>: dsl::And<a, b> would be inferred when correct type would be dsl::And<a, b, Nullable<Bool>>

I found a solution for this that widens the type inference capabilities of the dsl::And/Or helper type:
#3662

It works with types that are Expressions and have a SqlType that is either Bool or Nullable<Bool>, with bool, and with custom user types provided they implement this trait (otherwise they would just have to specify manually).

That fixes our type inference issue with the helper types (it can now correctly be inferred in all cases), however that's technically a breaking change to this extent:

• If this trait isn't implemented for the other parameter of a.and(other) the user needs to implement it for it to work in helper type type inference.
• For this to actually break, a user would have to be using a custom type that has AsExpression<Bool> but not Expression<SqlType=Bool> as other and have written dsl::And/Or<x, ThatCustomType> without the explicit 3rd parameter somewhere, which seems very unlikely.
• I would estimate the use for diesel_auto_type and Selectable, as well as general usability improvement of dsl::And to be worth faking the non-breaking change.
• Alternately we could choose to keep that on master only until a hypothetical Diesel 3.

Side note: We could also choose to introduce a more general trait PreferredExpressionType: AsExpression<Self::SqlType> { SqlType } to be used in similar cases where type inference doesn't work through AsExpression (.into_sql()...), but that would invite us to implement it everywhere... Doesn't seem worth ATM.

[weiznich]

Good work 👍 That looks really interesting. I'm currently out for holidays, but I will come back with a longer response as soon as I'm back.

[weiznich]

So I had finally some time to look a bit closer on that implementation. Again: That's really good work 👍

I think it would be great to expose that as part of diesel itself, so variant 2 or 3. If you are interested in using this in other contexts as well we might implement it as separate crate in the main diesel repo, similar to what we have done with the diesel_table_syntax crate. This would at least from diesel's side allow a consistent dev-workflow.

I still need to review the code in depth and figure out some details, but that's something I can do as soon as we have a PR for that.