Skip to content

Conversation

@lapla-cogito
Copy link
Contributor

@lapla-cogito lapla-cogito commented Dec 11, 2024

fix #13411

The if_not_else lint can be fixed automatically, but the issue above reports that there is no implementation to do so. Therefore, this PR implements it.


changelog: [if_not_else]: make suggestions for modified code

@rustbot
Copy link
Collaborator

rustbot commented Dec 11, 2024

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 11, 2024
impl LateLintPass<'_> for IfNotElse {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if let ExprKind::If(cond, _, Some(els)) = e.kind
if let ExprKind::If(cond, inter, Some(els)) = e.kind
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The processing performed by the if statement is necessary to present the proposed modification in the make_sugg function.

@lapla-cogito lapla-cogito changed the title suggest if_not_else auto-fix if_not_else Dec 11, 2024
@lapla-cogito lapla-cogito force-pushed the suggest_if_else branch 4 times, most recently from 0322f5f to 654c288 Compare December 11, 2024 09:00
@lapla-cogito
Copy link
Contributor Author

r? clippy

@rustbot rustbot assigned llogiq and unassigned Alexendoo Dec 22, 2024
@llogiq
Copy link
Contributor

llogiq commented Dec 24, 2024

I'd like to see more test cases involving comments and annotations, just to be sure the suggestion reproduces them faithfully.

@lapla-cogito
Copy link
Contributor Author

@llogiq I added several test cases in 887aa26.

Comment on lines +32 to +46
if !(foo() && bla()) {
#[cfg(not(debug_assertions))]
println!("not debug");
#[cfg(debug_assertions)]
println!("debug");
if foo() {
println!("foo");
} else if bla() {
println!("bla");
} else {
println!("both false");
}
} else {
println!("both true");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering the following code, which is similar in structure to this one, except that the parts to be linted are nested:

if !(foo() && bla()) {
    if !foo() {
        println!("not foo");
    } else {
        println!("some output");
    }
} else {
    println!("both true");
}

Then, when I run cargo bless, I get a cannot replace slice of data that was already replaced error. But I don't think this is a problem since rustfix will apply the first fix and ignore the subsequent when put to practical use.

@llogiq
Copy link
Contributor

llogiq commented Dec 24, 2024

Thank you!

@llogiq llogiq added this pull request to the merge queue Dec 24, 2024
Merged via the queue into rust-lang:master with commit 85b6094 Dec 24, 2024
9 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.

auto-fix if_not_else

4 participants