Rustfmt: Breaks associated_type_bounds

[dtolnay]

Input code:

#![feature(associated_type_bounds)]

fn f<I: Iterator<Item: Clone>>() {}

As of current rustfmt master (e0e2f0db537904b0efa78a106384e62852b806f0), rustfmt applies the following diff which makes the original code no longer compile:

- fn f<I: Iterator<Item: Clone>>() {}
+ fn f<I: Iterator<Item = Clone>>() {}

warning: trait objects without an explicit `dyn` are deprecated
 --> src/main.rs:3:25
  |
3 | fn f<I: Iterator<Item = Clone>>() {}
  |                         ^^^^^ help: use `dyn`: `dyn Clone`
  |
  = note: #[warn(bare_trait_objects)] on by default

error[E0277]: the size for values of type `(dyn std::clone::Clone + 'static)` cannot be known at compilation time
 --> src/main.rs:3:1
  |
3 | fn f<I: Iterator<Item = Clone>>() {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
  |
  = help: the trait `std::marker::Sized` is not implemented for `(dyn std::clone::Clone + 'static)`
  = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
  = note: required by `std::iter::Iterator`

error[E0038]: the trait `std::clone::Clone` cannot be made into an object
 --> src/main.rs:3:1
  |
3 | fn f<I: Iterator<Item = Clone>>() {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::clone::Clone` cannot be made into an object
  |
  = note: the trait cannot require that `Self : Sized`

[topecongiro]

https://github.com/rust-lang/rustfmt/blob/e0e2f0db537904b0efa78a106384e62852b806f0/src/types.rs#L172-L177

We need to use a correct separator based on the AssocTyConstraintKind.

[calebcartwright]

@topecongiro - I'd like to work on this one. The implementation looks relatively straightforward, but I have a few questions if that's okay 😄

What exactly is the expected behavior if the AssocTyConstraint is Bound?

We could conditionally use a : or = separator based on the AssocTyConstraintKind which would result in the below snippets. Technically both will compile, but I don't think it's what we'd want/expect (at least they both look odd from my perspective):

fn f<I: Iterator<Item : Clone>>() {} // type_punctuation_density = wide

fn f<I: Iterator<Item:Clone>>() {} // type_punctuation_density = compressed

Should we just skip the modification when the kind is Bound so that it remains unchanged?

fn f<I: Iterator<Item: Clone>>() {}

[topecongiro]

@calebcartwright Great, thank! Questions are always welcome :grin:

We should always add a space after : but not before it. In other words we do not need to take type_punctuation_density into account when AssocTyConstraint is Bound:

-fn f<I: Iterator<Item : Clone>>() {}
+fn f<I: Iterator<Item: Clone>>() {}

[calebcartwright]

That makes a lot of sense thanks! I believe I've got this fixed, will open a PR later today. With what I've got currently:

This:

fn f<I: Iterator<Item: Clone>>() {}
fn f<I: Iterator<Item : Clone>>() {}
fn f<I: Iterator<Item:Clone>>() {}

Would be formatted to:

fn f<I: Iterator<Item: Clone>>() {}
fn f<I: Iterator<Item: Clone>>() {}
fn f<I: Iterator<Item: Clone>>() {}

Assuming that's the correct/expected behavior, where should I add tests for that? Should they go under an issue-3657 directory under ./tests/source/ and ./tests/target/?

[scampi]

@calebcartwright tests would go in tests/source/ which is where you put the original code, then in tests/target/ where you put the expected formatting.

[scampi]

sorry I didn't your question got answered in the PR !