Don't skip semicolon if expressions follow

[CosmicHorrorDev]

Fixes #5797

This changes the logic for semicolon_for_stmt() to only allow skipping the semicolon on the last expression

Lingering question: I'm not sure whenast::Stmt gets used and I don't know of a way to check the docs for rustc_ast (doesn't seem to be included in the docs for cargo doc --open?), so I just skipped the last statement logic for that

[fee1-dead]

Can you check which places use this Rewrite impl? You could probably look at the errors if you remove this impl.

[CosmicHorrorDev]

Good idea! It looks like the three places that it gets used are (lines 520, 811, and 815)

rustfmt/src/expr.rs

Lines 518 to 520 in 9386b32

	if is_simple_block(context, block, attrs) {
	let expr_shape = shape.offset_left(last_line_width(prefix))?;
	let expr_str = block.stmts[0].rewrite(context, expr_shape)?;

rustfmt/src/expr.rs

Lines 802 to 815 in 9386b32

	if !is_simple_block(context, self.block, None)
	|| !is_simple_block(context, else_node, None)
	|| pat_expr_str.contains('\n')
	{
	return None;
	}
	let new_width = width.checked_sub(pat_expr_str.len() + fixed_cost)?;
	let expr = &self.block.stmts[0];
	let if_str = expr.rewrite(context, Shape::legacy(new_width, Indent::empty()))?;
	let new_width = new_width.checked_sub(if_str.len())?;
	let else_expr = &else_node.stmts[0];
	let else_str = else_expr.rewrite(context, Shape::legacy(new_width, Indent::empty()))?;

All three of those places currently gate on the containing block being a is_simple_block() (only an expr with no statements), so that wouldn't be true in cases that cause this issue. I doubt that that will always hold though, so it may be worth dropping impl Rewrite for ast::Stmt entirely for crate::stmt::Stmt since the only additional information needed is is_last which is known for all of those

[calebcartwright]

Perhaps instead of dropping the impl and having to update all the call sites to do the same things (e.g. create a stmt::Stmt) the rewrite could be updated to facilitate that translation?

[CosmicHorrorDev]

Any idea for how you would want that to look? I'm not sure of a way to handle it in a way that doesn't clash with rewrite for crate::stmt::Stmt already containing that information

[calebcartwright]

Ah yes, I suppose it might be difficult for callers to indicate whether it's the last statement through a rewrite call wouldn't it 😆.

I don't recall offhand if there's already a constructor function for the stmt::Stmt, but if not, would be good to add something that will encapsulate the creation of the stmt::Stmt wrapper and invokes format_stmt

[CosmicHorrorDev]

I've settled on adding a stmt::Stmt::from_simple_block(...) -> Option<Self> constructor to switch all the existing ast::Stmt rewriting over to

The only reason I didn't go for adding in a helper that also invokes format_stmt is because there is shaping that requires additional context between checking that it was a simple block and rewriting it, and I didn't want to have to switch to shaping before and then fixing up shaping on failures

Let me know what ya think

(aside: is this one of the repos where it's generally expected to rebase before the final merge?)

[fee1-dead]

(aside: is this one of the repos where it's generally expected to rebase before the final merge?)

I don't think you'd need to rebase on top of master before merging, but it would be nice if you could avoid having merge commits in your PR.

[CosmicHorrorDev]

Fair enough. The commit history should be clean now

[calebcartwright]

Wrt merge commits and up-to-date branches, I covered a decent amount of this in https://rust-lang.zulipchat.com/#narrow/stream/357797-t-rustfmt/topic/Workflow/near/367436178, but in general yes, we really try to avoid merge commits here (although they are a necessary evil when we do the subtree syncs)

We also have the branch protections configured to require the source branch be up to date, so yes rebasing is good. However, I do have the ability to bypass those and in cases like this I don't mind doing so.

Finally, thanks for the quick fix and making your first contribution 🎉 🎆