Skip to content

Add support for if statements to suspicious_operation_groupings lint #6275

Open
@Ryan1729

Description

@Ryan1729

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
    })
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-enhancementCategory: Enhancement of lints, like adding more cases or adding help messages

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions