Skip to content

Shared code in if and else #5234

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.

Sign up for GitHub

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

Jump to bottom
Closed
Luro02 opened this issue Feb 27, 2020 · 1 comment · Fixed by #6463
Closed

Shared code in if and else #5234

Luro02 opened this issue Feb 27, 2020 · 1 comment · Fixed by #6463
Assignees
@xFrednet
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-complexity Lint: Belongs in the complexity lint group

Comments

@Luro02
Copy link

Luro02 commented Feb 27, 2020 •
edited by rustbot
Loading

The lint would trigger if the if, else if and else clasues have the same code in all branches.

let mut result = "";
let is_something = 20;

if is_something == 20 {
    result = "hello";

    println!("yes");
} else {
    result = "hello";

    println!("nope!");
}

println!("{}", result);

could be optimized to

let mut result = "";
let is_something = 20;

result = "hello";

if is_something == 20 {
    println!("yes");
} else {
    println!("nope!");
}

println!("{}", result);

playground
@flip1995 flip1995 added L-complexity Lint: Belongs in the complexity lint group E-hard Call for participation: This a hard problem and requires more experience or effort to work on A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. and removed E-hard Call for participation: This a hard problem and requires more experience or effort to work on labels Mar 2, 2020
@xFrednet
Copy link
Member

I would like to work on this 🙃.

@rustbot claim

@xFrednet xFrednet mentioned this issue Dec 17, 2020
Merged
bors added a commit that referenced this issue Apr 5, 2021
@bors
Auto merge of #6463 - xFrednet:5234-shared-code-in-if-blocks, r=phansch
ead70c3 
New Lint: `branches_sharing_code`

This lint checks if all `if`-blocks contain some statements that are the same and can be moved out of the blocks to prevent code duplication. Here is an example:

```rust
let _ = if ... {
    println!("Start"); // <-- Lint for code duplication
    let _a = 99;
    println!("End"); // <-- Lint for code duplication
    false
} else {
    println!("Start");
    let _b = 17;
    println!("End");
    false
};
```

This could be written as:

```rust
println!("Start");

let _ = if ... {
    let _a = 99;
    false
} else {
    let _b = 17;
    false
};

println!("End");
```

---

This lint will get masked by the `IF_SAME_THEN_ELSE` lint. I think it makes more sense to only emit one lint per if block. This means that the folloing example:

```rust
if ... {
    let _a = 17;
} else {
    let _a = 17;
}
```

Will only trigger the `IF_SAME_THEN_ELSE` lint and not the `SHARED_CODE_IN_IF_BLOCKS` lint.

---

closes: #5234

changelog: Added a new lint: `branches_sharing_code`

And hello to the one that is writing the changelog for this release :D
bors added a commit that referenced this issue Apr 5, 2021
@bors
Auto merge of #6463 - xFrednet:5234-shared-code-in-if-blocks, r=phansch
d34cb85 
New Lint: `branches_sharing_code`

This lint checks if all `if`-blocks contain some statements that are the same and can be moved out of the blocks to prevent code duplication. Here is an example:

```rust
let _ = if ... {
    println!("Start"); // <-- Lint for code duplication
    let _a = 99;
    println!("End"); // <-- Lint for code duplication
    false
} else {
    println!("Start");
    let _b = 17;
    println!("End");
    false
};
```

This could be written as:

```rust
println!("Start");

let _ = if ... {
    let _a = 99;
    false
} else {
    let _b = 17;
    false
};

println!("End");
```

---

This lint will get masked by the `IF_SAME_THEN_ELSE` lint. I think it makes more sense to only emit one lint per if block. This means that the folloing example:

```rust
if ... {
    let _a = 17;
} else {
    let _a = 17;
}
```

Will only trigger the `IF_SAME_THEN_ELSE` lint and not the `SHARED_CODE_IN_IF_BLOCKS` lint.

---

closes: #5234

changelog: Added a new lint: `branches_sharing_code`

And hello to the one that is writing the changelog for this release :D
@bors bors closed this as completed in d91da40 Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xFrednet xFrednet

Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-complexity Lint: Belongs in the complexity lint group
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

New Lint: branches_sharing_code xFrednet/rust-clippy
3 participants
@flip1995 @xFrednet @Luro02