-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Format heterogeneous try blocks #152293
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
Format heterogeneous try blocks #152293
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -385,8 +385,33 @@ pub(crate) fn format_expr( | |
| )) | ||
| } | ||
| } | ||
| // FIXME: heterogeneous try blocks, which include a type so are harder to format | ||
| ast::ExprKind::TryBlock(_, Some(_)) => Err(RewriteError::Unknown), | ||
| ast::ExprKind::TryBlock(ref block, Some(ref ty)) => { | ||
| let keyword = "try bikeshed "; | ||
| // 2 = " {".len() | ||
| let ty_shape = shape | ||
| .shrink_left(keyword.len()) | ||
| .and_then(|shape| shape.sub_width(2)) | ||
| .max_width_error(shape.width, expr.span)?; | ||
| let ty_str = ty.rewrite_result(context, ty_shape)?; | ||
| let prefix = format!("{keyword}{ty_str} "); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, is there going to be an issue with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to handle cases where the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. I gave my opinion in the PR description. I don't think this is needed given "bikeshed" is temporary so whatever we choose might not be transposable to the final syntax. But I'm happy to do it if we believe it's useful.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still experimental so I think it's fine for now. T-style may want to write up some rules for this before stabilization though.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I thought I have linked it but apparently didn't. I also wrote a PR from that based on Zulip discussion: #152311. |
||
| if let rw @ Ok(_) = | ||
| rewrite_single_line_block(context, &prefix, block, Some(&expr.attrs), None, shape) | ||
| { | ||
| rw | ||
| } else { | ||
| let budget = shape.width.saturating_sub(prefix.len()); | ||
| Ok(format!( | ||
| "{prefix}{}", | ||
| rewrite_block( | ||
| block, | ||
| Some(&expr.attrs), | ||
| None, | ||
| context, | ||
| Shape::legacy(budget, shape.indent) | ||
| )? | ||
| )) | ||
| } | ||
| } | ||
| ast::ExprKind::Gen(capture_by, ref block, ref kind, _) => { | ||
| let mover = if matches!(capture_by, ast::CaptureBy::Value { .. }) { | ||
| "move " | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| // rustfmt-edition: 2018 | ||
| #![feature(try_blocks)] | ||
|
|
||
| fn main() -> Result<(), !> { | ||
| let _x: Option<_> = try { | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've also fixed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks I appreciate that 🙏🏼 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // rustfmt-edition: 2018 | ||
| #![feature(try_blocks_heterogeneous)] | ||
|
|
||
| fn main() -> Result<(), !> { | ||
| let _x = try bikeshed Option<_> { | ||
| 4 | ||
| }; | ||
|
|
||
| try bikeshed Result<_, _> {} | ||
| } | ||
|
|
||
| fn baz() -> Option<i32> { | ||
| if (1 == 1) { | ||
| return try bikeshed Option<i32> { | ||
| 5 | ||
| }; | ||
| } | ||
|
|
||
| // test | ||
| let x = try bikeshed Option<()> { | ||
| // try blocks are great | ||
| }; | ||
|
|
||
| let y = try bikeshed Option<i32> { | ||
| 6 | ||
| }; // comment | ||
|
|
||
| let x = try /* Invisible comment */ bikeshed Option<()> {}; | ||
| let x = try bikeshed /* Invisible comment */ Option<()> {}; | ||
| let x = try bikeshed Option<()> /* Invisible comment */ {}; | ||
|
|
||
| let x = try bikeshed Option<i32> { baz()?; baz()?; baz()?; 7 }; | ||
|
|
||
| let x = try bikeshed Foo<Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar> { 1 + 1 + 1 }; | ||
|
|
||
| let x = try bikeshed Foo<Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar> {}; | ||
|
|
||
| return None; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| // rustfmt-edition: 2018 | ||
| #![feature(try_blocks)] | ||
|
|
||
| fn main() -> Result<(), !> { | ||
| let _x: Option<_> = try { 4 }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // rustfmt-edition: 2018 | ||
| #![feature(try_blocks_heterogeneous)] | ||
|
|
||
| fn main() -> Result<(), !> { | ||
| let _x = try bikeshed Option<_> { 4 }; | ||
|
|
||
| try bikeshed Result<_, _> {} | ||
| } | ||
|
|
||
| fn baz() -> Option<i32> { | ||
| if (1 == 1) { | ||
| return try bikeshed Option<i32> { 5 }; | ||
| } | ||
|
|
||
| // test | ||
| let x = try bikeshed Option<()> { | ||
| // try blocks are great | ||
| }; | ||
|
|
||
| let y = try bikeshed Option<i32> { 6 }; // comment | ||
|
|
||
| let x = try /* Invisible comment */ bikeshed Option<()> {}; | ||
| let x = try bikeshed /* Invisible comment */ Option<()> {}; | ||
| let x = try bikeshed Option<()> /* Invisible comment */ {}; | ||
|
|
||
| let x = try bikeshed Option<i32> { | ||
| baz()?; | ||
| baz()?; | ||
| baz()?; | ||
| 7 | ||
| }; | ||
|
|
||
| let x = try bikeshed Foo<Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar> { | ||
| 1 + 1 + 1 | ||
| }; | ||
|
|
||
| let x = | ||
| try bikeshed Foo<Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar, Bar> {}; | ||
|
|
||
| return None; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there going to be an issue with
try /* comment */ bikeshed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not more than with
try_blocksapparently. It simply won't format. See the test case incatch.rs:If you do:
It won't format. I've added tests similar to the one of
catch.rs.