rust-lang / style-team

Home of the Rust style team
Apache License 2.0
454 stars 56 forks source link

Type Annotation in Binding vs Turbofish #90

Closed killercup closed 6 years ago

killercup commented 7 years ago

Are there any guidelines about type annotations on bindings and function/method calls?

In the review of a new guide for Diesel, we discussed whether we should write type annotations on let bindings or use the turbofish syntax to annotate the return type a a method call (chain). I think this is a much better place to discuss this than the Diesel PR 😄

Or concrete example is:

let query_result = users.load::<User>(&db_connection);

versus

let users_result: QueryResult<Vec<User>> = user.load(&db_connection);

(I omitted the unwrap/?).

My comment on the topic was:

Historically, people seem to like the name 'turbofish' more than its look. I personally prefer variable type annotations more than putting a turbofish at the end of a long chain of method calls, because you put the name right next to the type. But others may disagree, as the turbofish version of something like .load::<Post>(&c) or .parse::<i32>() can be read quite nicely as "load posts from connections" or "parse this as an integer".

jplatte commented 7 years ago

I personally use the turbofish for this kind of thing because it makes changes easier. So if I decide that I only care about the string representation of the result in your example,

-let query_result = users.load::<User>(&db_connection);
+let query_result_string = users.load::<User>(&db_connection).to_string();

is easier than

-let query_result: QueryResult<Vec<User>> = users.load(&db_connection);
+let query_result_string = users.load::<User>(&db_connection).to_string();

and even if it's not something generic like to_string, so you don't need to rewrite it to use a turbofish, you still have three changes to make with the version that uses a type annotation (variable name, variable type, function call) vs two (variable name, function call).

joshtriplett commented 7 years ago

I don't think the Rust style guide should mandate a particular option here, and in particular we certainly shouldn't attempt to translate from one to the other.

Speaking entirely in personal opinion, without my style team hat on, I personally have used both depending on circumstances. When I'm writing something like a collect call, I prefer to use things like let v: Vec<_> = ...; however, in the example posted in this issue, using the turbofish allows specifying fewer types. I don't think we can universally say one or the other will always be preferred.

killercup commented 7 years ago

we certainly shouldn't attempt to translate from one to the other.

In rustfmt? Agreed.

I personally use the turbofish for this kind of thing because it makes changes easier.

Oh, diff size is a very good point, @jplatte, and something I try to optimize for, but hadn't thought about.

I don't think the Rust style guide should mandate a particular option here

Perhaps—but thanks so much for your replies, guys! It just lead me to a mini-epiphany, and maybe even a good guideline:

Use the turbofish when you care about the content, and type annotation on the binding if you care about the container.

.collect is generic over the container, e.g. Vec<_>, or even Result<Vec<_>, _>. But Diesel's .load will always use Vec: It is generic over the type it will deserialize the data to, e.g., User or Post.

nrc commented 7 years ago

A third option (on nightly at least) is to use type ascription.

I try not to overthink this - if I can write an annotation on let I do so because it is less sigil soup. If it is not possible, I use the turbofish.

joshtriplett commented 7 years ago

@nrc I do greatly prefer the type ascription syntax. What's the status of stabilizing that?

nrc commented 7 years ago

What's the status of stabilizing that?

There's still a lot of open questions - it prevents a lot of other syntax we might want, so we need to be sure it is a good idea. There are also open questions about whether it should cause coercions.

joshtriplett commented 7 years ago

@nrc

it prevents a lot of other syntax we might want

Fair enough.

There are also open questions about whether it should cause coercions.

Aieee. That sounds problematic; I'd expect type ascription to be descriptive ("I expect this type, infer accordingly and error out if not possible"), but never lead to coercions. Got a link for where I can read more rather than using this issue for that?

nrc commented 7 years ago

I don't have a link to hand, sorry, it's been a while since there has been much discussion on this.

The counter-argument is that type ascription should do exactly the same thing as explicitly putting a type on let or passing a value to a function where the callee argument has an explicit type. It is also useful practically where you want a coercion (e.g., to create a DST but don't want to use as because you are not doing a potentially harmful cast).

joshtriplett commented 7 years ago

@nrc It may be that I'm getting confused by the term "coercions", and implicitness versus explicitness. It shouldn't do what as does; it seems like expression : type should have the same type behavior as let x: type = expression.

nrc commented 7 years ago

So, e.g., let x: &[_] = &[1, 2, 3] will coerce the rhs from &[i32; 3] to &[i32] (i.e., from the fixed size type to the DST), currently type ascription does not do that.

joshtriplett commented 7 years ago

@nrc Ah, I see. I'd expect type ascription to be able to do that, yes; &[1, 2, 3] : &[_] should work.

nrc commented 6 years ago

I don't think there is anything for us to do here. I'm happy not to recommend either.