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

Stop inserting semicolon when extracting match arm #15235

Merged
merged 1 commit into from Jul 8, 2023

Conversation

omertuc
Copy link
Contributor

@omertuc omertuc commented Jul 8, 2023

Overview

Extracting a match arm value that has type unit into a function, when a
comma already follows the match arm value, results in an invalid (syntax
error) semicolon added between the newly generated function's generated
call and the comma.

Example

Running this extraction

fn main() {
    match () {
        _ => $0()$0,
    };
}

would lead to

fn main() {
    match () {
        _ => fun_name();,
    };
}

fn fun_name() {
}

Issue / Fix details

This happens because when there is no comma, rust-analyzer would simply
add the comma and wouldn't even try to evaluate whether it needs to add
a semicolon. But when the comma is there, it proceeds to evaluate
whether it needs to add a semicolon and it looks like the evaluation
logic erroneously ignores the possibility that we're in a match arm.
IIUC it never makes sense to add a semicolon when we're extracting from
a match arm value, so I've adjusted the logic to always decide against
adding a semicolon when we're in a match arm

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2023
Copy link
Contributor

@lowr lowr left a comment

Choose a reason for hiding this comment

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

Nice find! Looks good to me modulo one nit.

# Overview

Extracting a match arm value that has type unit into a function, when a
comma already follows the match arm value, results in an invalid (syntax
error) semicolon added between the newly generated function's generated
call and the comma.

# Example

Running this extraction

```rust
fn main() {
    match () {
        _ => $0()$0,
    };
}
```

would lead to

```rust
fn main() {
    match () {
        _ => fun_name();,
    };
}

fn fun_name() {
}
```

# Issue / Fix details

This happens because when there is no comma, rust-analyzer would simply
add the comma and wouldn't even try to evaluate whether it needs to add
a semicolon. But when the comma is there, it proceeds to evaluate
whether it needs to add a semicolon and it looks like the evaluation
logic erroneously ignores the possibility that we're in a match arm.
IIUC it never makes sense to add a semicolon when we're extracting from
a match arm value, so I've adjusted the logic to always decide against
adding a semicolon when we're in a match arm
@omertuc
Copy link
Contributor Author

omertuc commented Jul 8, 2023

Force pushed to apply suggestion

@lowr
Copy link
Contributor

lowr commented Jul 8, 2023

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

📌 Commit 9200d27 has been approved by lowr

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

⌛ Testing commit 9200d27 with merge fdb8aa2...

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

☀️ Test successful - checks-actions
Approved by: lowr
Pushing fdb8aa2 to master...

@bors bors merged commit fdb8aa2 into rust-lang:master Jul 8, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants