rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.51k stars 1.55k forks source link

collapsible_if shouldn't apply if there is a comment before #798

Closed Zoxc closed 6 years ago

Zoxc commented 8 years ago

In this example there is comment which applies to both branches in the else block.

        if entry.end > entry.base {
            prev = entry_; // Go to the next entry
        } else {
            // No usable memory in this entry. Remove it from the list.

            if prev == st.list {
                st.list = entry.next;
                prev = st.list;
            } else {
                (*prev).next = entry.next;
            }
        }
joshtriplett commented 6 years ago

Similar example from rustc's tidy:

    // If this path is in-tree, we don't require it to be on the whitelist
    if must_be_on_whitelist {
        // If this dependency is not on the WHITELIST, add to bad set
        if !whitelist.contains(&krate.into()) {
            unapproved.insert(krate.into());
        }
    }

Both of the if statements have a comment, and collapsing them together would make this less clear.

lukasstevens commented 6 years ago

I'd like to work on this if there is still interest.

flip1995 commented 6 years ago

Great, it's all yours. Also have a look at #3228, for some more discussion about this lint.