Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/tools/rustfmt/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ";
Copy link
Contributor

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?

Copy link
Contributor Author

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_blocks apparently. It simply won't format. See the test case in catch.rs:

    let x = try /* Invisible comment */ { foo()? };

If you do:

    let x = try /* Invisible comment */ { foo()?
    };

It won't format. I've added tests similar to the one of catch.rs.

// 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} ");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, is there going to be an issue with try bikeshed /* comment */ {ty_str}?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to handle cases where the type can't fit on the current line and needs to wraps to the next line? Something like:

try bikeshed
{ty_str} {
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 "
Expand Down
1 change: 1 addition & 0 deletions src/tools/rustfmt/tests/source/try_block.rs
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 {
Expand Down
39 changes: 39 additions & 0 deletions src/tools/rustfmt/tests/source/try_blocks_heterogeneous.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add #![feature(try_blocks_heterogeneous)] to these tests so that it's easier to tell what experimental feature they're for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also fixed try_block.rs.

Copy link
Contributor

Choose a reason for hiding this comment

The 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;
}
1 change: 1 addition & 0 deletions src/tools/rustfmt/tests/target/try_block.rs
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 };
Expand Down
41 changes: 41 additions & 0 deletions src/tools/rustfmt/tests/target/try_blocks_heterogeneous.rs
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;
}
Loading