Format trailing where clauses in type aliases #5887

compiler-errors · 2023-08-16T18:12:53Z

Self-explanatory. cc rust-lang/rust#114901

This implementation is probably super busted. I have no idea what most of these options in rustfmt mean, lol.

calebcartwright · 2023-08-27T19:57:21Z

I'd like to hold off on merging this till the next-next sync & release (assuming we are still on track to get let-chains in quickly) given the diff it'll introduce on stable.

Probably worth having common (and uncommon) comments in the test snippets too

ytmimi · 2023-08-28T18:26:34Z

I'd like to hold off on merging this till the next-next sync & release (assuming we are still on track to get let-chains in quickly) given the diff it'll introduce on stable.

Sounds good. I think we're still on track to get that out soon. I'll likely have that PR ready later this week. Given that I'll probably hold off on reviewing this until after the next sync.

fee1-dead

implementation looks fine.

fee1-dead · 2023-12-01T03:13:39Z

src/items.rs

+    let rhs_hi = ty
+        .as_ref()
+        .map_or(where_clauses.0.1.hi(), |ty| ty.span.hi());


Suggested change

let rhs_hi = ty

.as_ref()

.map_or(where_clauses.0.1.hi(), |ty| ty.span.hi());

let rhs_hi = ty_opt.map_or(where_clauses.0.1, |ty| ty.span).hi();

ytmimi · 2023-12-04T23:41:53Z

tests/target/type-alias-where-clauses.rs

+type Foo
+    = E
+where
+    F: G,
+    H: I;


@compiler-errors is there documentation in the style guide that you can link to for this?

I'm wondering why this wouldn't be formatted like:

type Foo = E where F: G, H: I;

rust-lang/rust#114901

Specifically:

When there is a trailing where clause after the type, and no where clause
present before the type, break before the = and indent. Then break before the
where keyword and format the clauses normally, e.g.,

// With only a trailing where clause type VeryLongType<T, U> = AnEvenLongerType<T, U, Foo<T>> where T: U::AnAssociatedType, U: SomeBound;

Thank you. That's exactly what I was looking for

ytmimi · 2023-12-04T23:42:33Z

tests/target/type-alias-where-clauses.rs

+type Foo
+where
+    A: B,
+    C: D,
+= E
+where
+    F: G,
+    H: I;


This seems like an interesting case. Is it valid to have two where clauses?

No, but it's not denied by the parser, so I think rustfmt shouldn't explode when we have it.

I appreciate the clarification on this

so I think rustfmt shouldn't explode when we have it.

I'd like to adopt this as a new motto for us 🤣

calebcartwright · 2023-12-09T19:08:05Z

@ytmimi do you think we should pull this in as part of the next release? I imagine this would need to be prefaced with a blog post and some associated lead time due to the intro of formatting being applied for the first time?

ytmimi

I've been taking a look at this for a while now and the implementation makes sense to me. I was trying to think of ways to simplify / improve what you've got here, but I don't think I can come up with any. I think we're good to move forward with the PR

ytmimi · 2023-12-10T02:48:39Z

@ytmimi do you think we should pull this in as part of the next release? I imagine this would need to be prefaced with a blog post and some associated lead time due to the intro of formatting being applied for the first time?

@calebcartwright Yeah, I think we could pull this in for the next release. When did you want to do the sync? I'm still up for taking on the subtree push. Maybe we can talk it over on Zulip and find a time that works.

ytmimi · 2024-01-25T23:41:15Z

I'm expecting to see some changes now that we actually format trailing where clauses, but let's see what the Diff-Check reveals.

Edit: The job failed but includes a lot of unrelated changes. Going to rebase and run it agin.

ytmimi · 2024-01-26T01:53:42Z

Okay, This Diff-Check job shows the diffs I was expecting. @compiler-errors can when you have a chance, can you review the logs and double check that things are formatted here as expected based on the formatting rules you implemented.

For your convenience I'm including the diffs inline:
note the output is a diff of diffs, so it might be a little noisy, by the highlighting should help identify the changes

Diff in /tmp/rust-lang-rust-f0yK2vou/library/core/src/ops/async_function.rs:73:
     where
         <F as FnOnce<A>>::Output: Future,
     {
-        type CallFuture<'a> = <F as FnOnce<A>>::Output where Self: 'a;
+        type CallFuture<'a>
+            = <F as FnOnce<A>>::Output
+        where
+            Self: 'a;
 
         extern "rust-call" fn async_call(&self, args: A) -> Self::CallFuture<'_> {
             self.call(args)

Diff in /tmp/rust-lang-rust-f0yK2vou/library/core/src/ops/async_function.rs:85:
     where
         <F as FnOnce<A>>::Output: Future,
     {
-        type CallMutFuture<'a> = <F as FnOnce<A>>::Output where Self: 'a;
+        type CallMutFuture<'a>
+            = <F as FnOnce<A>>::Output
+        where
+            Self: 'a;
 
         extern "rust-call" fn async_call_mut(&mut self, args: A) -> Self::CallMutFuture<'_> {
             self.call_mut(args)

Diff in /tmp/rust-lang-rust-f0yK2vou/compiler/rustc_query_system/src/query/caches.rs:32:
 pub struct DefaultCacheSelector<K>(PhantomData<K>);
 
 impl<'tcx, K: Eq + Hash, V: 'tcx> CacheSelector<'tcx, V> for DefaultCacheSelector<K> {
-    type Cache = DefaultCache<K, V>
+    type Cache
+        = DefaultCache<K, V>
     where
         V: Copy;
 }

Diff in /tmp/rust-lang-rust-f0yK2vou/compiler/rustc_query_system/src/query/caches.rs:84:
 pub struct SingleCacheSelector;
 
 impl<'tcx, V: 'tcx> CacheSelector<'tcx, V> for SingleCacheSelector {
-    type Cache = SingleCache<V>
+    type Cache
+        = SingleCache<V>
     where
         V: Copy;
 }

Diff in /tmp/rust-lang-rust-f0yK2vou/compiler/rustc_query_system/src/query/caches.rs:126:
 pub struct VecCacheSelector<K>(PhantomData<K>);
 
 impl<'tcx, K: Idx, V: 'tcx> CacheSelector<'tcx, V> for VecCacheSelector<K> {
-    type Cache = VecCache<K, V>
+    type Cache
+        = VecCache<K, V>
     where
         V: Copy;
 }

Diff in /tmp/rust-lang-rust-f0yK2vou/compiler/rustc_query_system/src/query/caches.rs:177:
 pub struct DefIdCacheSelector;
 
 impl<'tcx, V: 'tcx> CacheSelector<'tcx, V> for DefIdCacheSelector {
-    type Cache = DefIdCache<V>
+    type Cache
+        = DefIdCache<V>
     where
         V: Copy;
 }

Diff in /tmp/rust-lang-rust-f0yK2vou/src/librustdoc/clean/types.rs:1059:
 }
 
 impl AttributesExt for [(Cow<'_, ast::Attribute>, Option<DefId>)] {
-    type AttributeIterator<'a> = impl Iterator<Item = ast::NestedMetaItem> + 'a
-        where Self: 'a;
-    type Attributes<'a> = impl Iterator<Item = &'a ast::Attribute> + 'a
-        where Self: 'a;
+    type AttributeIterator<'a>
+        = impl Iterator<Item = ast::NestedMetaItem> + 'a
+    where
+        Self: 'a;
+    type Attributes<'a>
+        = impl Iterator<Item = &'a ast::Attribute> + 'a
+    where
+        Self: 'a;
 
     fn lists<'a>(&'a self, name: Symbol) -> Self::AttributeIterator<'a> {
         AttributesExt::iter(self)
@ -7[54](https://github.com/rust-lang/rustfmt/actions/runs/7661954853/job/20882324738#step:4:55)9,0 +7[63](https://github.com/rust-lang/rustfmt/actions/runs/7661954853/job/20882324738#step:4:64)3,7 @@ Diff in /tmp/rust-lang-rust-f0yK2vou/src/tools/rust-analyzer/crates/parser/test_

Diff in /tmp/rust-lang-rust-f0yK2vou/src/tools/rust-analyzer/crates/parser/test_data/parser/inline/ok/0012_type_item_where_clause.rs:1:
-type Foo = () where Foo: Copy;
+type Foo
+    = ()
+where
+    Foo: Copy;

compiler-errors · 2024-01-26T01:56:24Z

@ytmimi: Yes, that formatting looks correct. When there is a where clause present, there's always a line break, then = and the type, then a break, then the where clauses.

ytmimi · 2024-01-26T03:12:22Z

@ytmimi do you think we should pull this in as part of the next release? I imagine this would need to be prefaced with a blog post and some associated lead time due to the intro of formatting being applied for the first time?

I think all the implementation work for this is done on the rustfmt side!

@calebcartwright I missed the fact that you suggested we get a blog post together for this. I'd be happy to draft something in preparation for the release. Is the idea to have the blog post go out before the release?

ytmimi · 2024-01-26T03:14:34Z

@ytmimi: Yes, that formatting looks correct. When there is a where clause present, there's always a line break, then = and the type, then a break, then the where clauses.

Awesome, thank you for confirming! I'm already anticipating someone asking for a single-line formatting option 😅

compiler-errors · 2024-06-06T13:39:07Z

@calebcartwright @ytmimi: It's been almost a year since this PR was opened. Is there any chance we could finally get this merged? What's the plan on doing the next rustfmt->rust sync?

calebcartwright · 2024-06-26T22:16:27Z

@ytmimi thoughts on how to label this one? it isn't technically part of the 2024 edition and can/should go before, but it is definitely something that can't slip past the 2024 edition

ytmimi · 2024-06-27T01:20:49Z

Agreed that this one should get in no later than the 2024 edition, and really as soon as we can. Just brainstorming here, but maybe we can label it something like pre-2024-must-have

src/items.rs

calebcartwright · 2024-07-26T23:20:46Z

What I think I'd like to do is get this merged such that it's timed to land with the addition of the style_edition option so that we can have a single blog post highlighting both

calebcartwright · 2024-09-12T03:19:16Z

@compiler-errors i hate to ask but would you have bandwidth to rebase? if not i understand (i know how busy you are) and i can grab your commits and pull them into a new branch but just wanted to check with you first

compiler-errors · 2024-09-12T10:33:49Z

@calebcartwright: Done. Sorry, was asleep by the time you sent that message otherwise I'd've done it sooner 😆

calebcartwright · 2024-09-12T15:04:08Z

ugh 😞 i clicked the wrong button and added a merge commit instead of rebasing

compiler-errors · 2024-09-12T15:04:20Z

Do you want me to rebase?

compiler-errors · 2024-09-12T15:06:09Z

Well, I went ahead and did it anyways.

calebcartwright · 2024-09-12T15:27:11Z

Thank you 🙏

rust-lang/rustfmt#5887

rustbot added the pr-not-reviewed label Aug 16, 2023

compiler-errors changed the title ~~Format trailing where clauses in rustfmt~~ Format trailing where clauses in type aliases Aug 16, 2023

compiler-errors force-pushed the trailing-wc branch from a3b562b to 73cb507 Compare August 27, 2023 20:19

fee1-dead approved these changes Dec 1, 2023

View reviewed changes

ytmimi reviewed Dec 4, 2023

View reviewed changes

ytmimi approved these changes Dec 10, 2023

View reviewed changes

ytmimi added pr-ready-to-merge release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Dec 10, 2023

ytmimi force-pushed the trailing-wc branch from 73cb507 to cdc4172 Compare January 25, 2024 23:56

fmease mentioned this pull request Feb 8, 2024

Don't remove const generic when using #[feature(generic_const_items)] #5996

Open

compiler-errors force-pushed the trailing-wc branch from cdc4172 to 49931f0 Compare June 27, 2024 01:26

fmease reviewed Jun 27, 2024

View reviewed changes

src/items.rs Outdated Show resolved Hide resolved

ytmimi mentioned this pull request Aug 12, 2024

Cannot handle generic associated type with where statement #6274

Closed

ytmimi linked an issue Aug 13, 2024 that may be closed by this pull request

Cannot handle generic associated type with where statement #6274

Closed

compiler-errors force-pushed the trailing-wc branch from 49931f0 to 5dd874e Compare September 12, 2024 10:30

compiler-errors added 2 commits September 12, 2024 11:05

Format trailing where clauses in rustfmt

4634072

Add tests showing how comments are handled

21314d1

compiler-errors force-pushed the trailing-wc branch from cf88459 to 21314d1 Compare September 12, 2024 15:06

calebcartwright merged commit d6d9e76 into rust-lang:master Sep 12, 2024
26 checks passed

compiler-errors deleted the trailing-wc branch September 12, 2024 15:37

ytmimi removed the release-notes Needs an associated changelog entry label Sep 20, 2024

bugadani mentioned this pull request Sep 21, 2024

Apply rustfmt fix esp-rs/esp-hal#2210

Merged

robertbastian mentioned this pull request Sep 23, 2024

cargo +nightly fmt unicode-org/icu4x#5572

Merged

robertbastian added a commit to unicode-org/icu4x that referenced this pull request Sep 23, 2024

cargo +nightly fmt (#5572)

53a5566

rust-lang/rustfmt#5887

ytmimi mentioned this pull request Oct 29, 2024

where clauses in GAT impls aren't formatted #5580

Closed

nicoburns mentioned this pull request Nov 7, 2024

Formatting of associated types with GATs very verbose #6387

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Format trailing where clauses in type aliases #5887

Format trailing where clauses in type aliases #5887

compiler-errors commented Aug 16, 2023

calebcartwright commented Aug 27, 2023

ytmimi commented Aug 28, 2023

fee1-dead left a comment

fee1-dead Dec 1, 2023 •

edited

Loading

ytmimi Dec 4, 2023

compiler-errors Dec 5, 2023

compiler-errors Dec 5, 2023

ytmimi Dec 5, 2023

ytmimi Dec 4, 2023

compiler-errors Dec 5, 2023

ytmimi Dec 5, 2023

calebcartwright Dec 6, 2023

calebcartwright commented Dec 9, 2023

ytmimi left a comment

ytmimi commented Dec 10, 2023

ytmimi commented Jan 25, 2024 •

edited

Loading

ytmimi commented Jan 26, 2024

compiler-errors commented Jan 26, 2024 •

edited

Loading

ytmimi commented Jan 26, 2024

ytmimi commented Jan 26, 2024

compiler-errors commented Jun 6, 2024

calebcartwright commented Jun 26, 2024

ytmimi commented Jun 27, 2024

calebcartwright commented Jul 26, 2024

calebcartwright commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

calebcartwright commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

calebcartwright commented Sep 12, 2024

Format trailing where clauses in type aliases #5887

Format trailing where clauses in type aliases #5887

Conversation

compiler-errors commented Aug 16, 2023

calebcartwright commented Aug 27, 2023

ytmimi commented Aug 28, 2023

fee1-dead left a comment

Choose a reason for hiding this comment

fee1-dead Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calebcartwright commented Dec 9, 2023

ytmimi left a comment

Choose a reason for hiding this comment

ytmimi commented Dec 10, 2023

ytmimi commented Jan 25, 2024 • edited Loading

ytmimi commented Jan 26, 2024

compiler-errors commented Jan 26, 2024 • edited Loading

ytmimi commented Jan 26, 2024

ytmimi commented Jan 26, 2024

compiler-errors commented Jun 6, 2024

calebcartwright commented Jun 26, 2024

ytmimi commented Jun 27, 2024

calebcartwright commented Jul 26, 2024

calebcartwright commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

calebcartwright commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

compiler-errors commented Sep 12, 2024

calebcartwright commented Sep 12, 2024

fee1-dead Dec 1, 2023 •

edited

Loading

ytmimi commented Jan 25, 2024 •

edited

Loading

compiler-errors commented Jan 26, 2024 •

edited

Loading