Redundant x_by and x_by_key

[clarfonthey]

For example, these:

x.sort_by(|l, r| l.cmp(r))
y.sort_by(|l, r| l.0.cmp(r.0))

could become:

x.sort();
y.sort_by_key(|z| z.0);

This also applies to binary_search_by and similar methods.

Perhaps we could also add a lint for adding _by but not _by_key when implementing a type.

Demonstrated on indexmap-rs/indexmap#55.

[bluss]

Just a reminder about the different conditions; sort_by is more general because it sometimes works when sort_by_key doesn't -- if the comparison value is a String, .sort_by_key would force you to clone the string for each comparison, that's as good as "doesn't work".

[clarfonthey]

@bluss I don't see why that'd be the case when &T also implements Ord if T: Ord

[bluss]

The compiler will tell you if you try 😉

[clarfonthey]

Oh! I see. It's a lifetime error in the original definition of the function.

fn sort_by_key<B, F>(&mut self, f: F) where
    B: Ord,
    F: FnMut(&T) -> B, 

Should be:

fn sort_by_key<'a, B, F>(&'a mut self, f: F) where
    B: Ord,
    F: FnMut(&'a T) -> B, 

[bluss]

Your definition is not valid - the elements move during sort.

[clarfonthey]

No, but the elements do not need to be moved, simply compared. I'll write something up tomorrow to demonstrate.

[clarfonthey]

…oh, I see the problem. K should really be K<'_>, and you're right that my definition is invalid. Larger issue regarding this: rust-lang/rust#34162