Skip to content
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

fix: respect trailing semicolon setting for last statements #6182

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shekhirin
Copy link

Problem

#5797 fixed the trailing_semicolon rule, allowing it only for the last expression in the block. The side-effect of this change is that such snippet

fn main() {
    println!("{}", greet());
}

fn greet() -> String {
    return "Hello, b!".to_string();
}

will not get formatted into

fn main() {
    println!("{}", greet());
}

fn greet() -> String {
    return "Hello, b!".to_string()
}

(notice the ; in the greet function), even though return is the last expression here. It's happening because the implementation of is_last_expr() additionally checks for the type of expression for some reason unknown to me

rustfmt/src/stmt.rs

Lines 74 to 88 in 46e5f14

fn is_last_expr(&self) -> bool {
if !self.is_last {
return false;
}
match self.as_ast_node().kind {
ast::StmtKind::Expr(ref expr) => match expr.kind {
ast::ExprKind::Ret(..) | ast::ExprKind::Continue(..) | ast::ExprKind::Break(..) => {
false
}
_ => true,
},
_ => false,
}
}

Solution

This PR fixes that by checking only the is_last field of the Stmt struct when deciding whether to emit the semicolon.

fn main() {
    println!("{}", greet());
}

fn greet() -> String {
    return "Hello, b!".to_string();
}

fn foo() {}
fn main() {
    return;
    foo()
}
rustfmt git:(trailing-semicolon-last-stmt) cargo run --bin rustfmt -- --check -- ~/Projects/semicolon/src/main.rs
   Compiling rustfmt-nightly v1.7.0 (/Users/shekhirin/Projects/rustfmt)
    Finished dev [unoptimized + debuginfo] target(s) in 0.61s
     Running `target/debug/rustfmt --check -- /Users/shekhirin/Projects/semicolon/src/main.rs`
Diff in /Users/shekhirin/Projects/semicolon/src/main.rs:3:
 }
 
 fn greet() -> String {
-    return "Hello, b!".to_string();
+    return "Hello, b!".to_string()
 }
 
 fn foo() {}

As you can see, the example from the original PR is still not getting formatted, while the trailing comma for the last return statement gets removed.

@shekhirin shekhirin marked this pull request as draft June 4, 2024 16:14
@shekhirin shekhirin marked this pull request as ready for review June 4, 2024 16:17
@ytmimi
Copy link
Contributor

ytmimi commented Jun 4, 2024

Will probably be a little while before I'm able to review this, but I wanted to note that there's a semi related PR (#6149) that adds a new option to trailing_semicolon. It feels like this would be complimentary to that work, but I'm not 100% sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants