New / Updated Lint: Prefer sort_unstable over sort

[CryZe]

What it does

While there already is a similar lint stable_sort_primitive, this should be extended to all types, not just primitives. If you call the plain .sort() (importantly not the other variants), then typically all bits of a type contribute to the sorting. It is therefore impossible to differentiate equal elements after the fact, meaning there was no need for a stable sort in the first place. Only in exceedingly rare situations (which I wouldn't even have any example of) would calling the plain .sort() ever make sense over .sort_unstable(). Linting against all (maybe as an optional lint) and forcing users to explicitly #[allow(...)] the lint in those incredibly rare situations makes the most sense to me.

Lint Name

stable_sort_primitive

Category

perf

Advantage

• It's faster in most cases.

Drawbacks

• There are rare situations (of which I don't have an example) where it may be wrong to use sort_unstable.
• sort_unstable is not always faster.

Example

foo.sort();

Could be written as:

foo.sort_unstable();

[chansuke]

@rustbot claim

[ChrisJefferson]

Are you suggesting people clippy should suggest to always replace sort with sort_unstable?

I often want stable sorts.

If you want an example:

I store some extra data in classes which are not used as part of the comparator. However, I want repeatable results, to make testing easier, so I use stable_sort so this extra data is always sorted to the same place.

[Centri3]

Are you suggesting people clippy should suggest to always replace sort with sort_unstable?

"and forcing users to explicitly #[allow(...)] the lint in those incredibly rare situations makes the most sense to me."

Is there any reason you cannot #[allow] it across the entire test module? Like:

#[cfg(test)]
#[allow(clippy::stable_sort_primitive)]
mod tests {}

In the case it's testing a specific method, that happens to use sort_stable, that wouldn't exactly work (but you could still #[allow] it all the same, it'd just require it on the function instead)

(Maybe the lint should ignore modules annotated with #[cfg(test)].)