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

Add support for if statements to suspicious_operation_groupings lint #6275

Open
Ryan1729 opened this issue Oct 31, 2020 · 1 comment
Open
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages

Comments

@Ryan1729
Copy link
Contributor

Ryan1729 commented Oct 31, 2020

The suggestions made in #6039 lead to the suspicious_operation_groupings lint which was initially added in #6086, a PR I wrote.

As part of those suggestions, the idea of making the lint trigger on buggy groups of if statements came up. The PR #6086 took a long time, so I figured it would be a good idea to postpone adding the if statement portion, as well as a particular kind of buggy expression. Below are some tests that I wrote to test those features, that would hopefully be useful if/when someone picks this up again.

fn multiple_comparison_types_and_unary_minus(s: &S) -> bool {
    s.a > s.c
    && s.a < -s.c
    && s.b > s.c // `s.c` should be `s.d` here.
    && s.b < -s.d
}

fn across_if_expression(
    s1: &mut S,
    s2: &mut S,
) -> i32 {
    if s1.a < s2.a {
        s1.a
    } else if s1.b < s2.b {
        s1.b
    } else if s1.c < s2.b { // `s2.b` should be `s2.c` here.
        s1.c
    } else if s1.d < s2.d {
        s1.d
    }
}
 
fn across_if_statements_in_an_expr(s1: &mut S, s2: &mut S) -> bool {
    {
        let mut changed = false;
        if s1.a < s2.a {
            s1.a = s2.a;
            changed = true;
        }
        if s1.b < s2.b {
            s1.b = s2.b;
            changed = true;
        }
        if s1.c < s2.b { // `s2.b` should be `s2.c` here.
            s1.c = s2.c;
            changed = true;
        }
        if s1.d < s2.d {
            s1.d = s2.d;
            changed = true;
        }
        changed
    }
}

fn across_if_statements(s1: &mut S, s2: &mut S) {
    if s1.a < s2.a {
        s1.a = s2.a;
    }

    if s1.b < s2.b {
        s1.b = s2.b;
     }

    if s1.c < s2.b { // `s2.b` should be `s2.c` here.
        s1.c = s2.c;
    }

    if s1.d < s2.d {
        s1.d = s2.d;
     }
}
 
fn across_if_expressions_with_a_leading_unary_minus(
    s1: &mut S,
    s2: &mut S,
) -> i32 {
    -(if s1.a < s2.a {
        s1.a
    } else if s1.b < s2.b {
        s1.b
    } else if s1.c < s2.b { // `s2.b` should be `s2.c` here.
        s1.c
    } else if s1.d < s2.d {
        s1.d
    })
}
@giraffate
Copy link
Contributor

@rustbot modify labels: +L-enhancement

@rustbot rustbot added the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages
Projects
None yet
Development

No branches or pull requests

3 participants