Skip to content

clippy::comparison_chain false positive on PartialOrd #4827

Closed

Description

@timbodeit

The comparison_chain lint is defined as:

Checks comparison chains written with if that can be rewritten with match and cmp.

However it also triggers on comparison chains written with if who's compared type does not implement Ord and doesn't offer cmp.

One such example would be f64. This type implements PartialOrd but not Ord. It provides < and > operators without providing cmp.
While in theory the same concept of using match could also be applied to the result of partial_cmp, I personally find matching over Optional<Ordering> much less appealing than matching over Ordering.

Should we apply the same lint to all PartialOrd cases and change the documentation to make this the official desired behavior? Or should we consider this a bug and change the implementation, so that the lint only fires if cmp is available on the relevant type (as specified in the documentation)?

cc @james9909


Example implementation where clippy::comparison_chain fires, eventough cmp is not available:

fn f(x: f64, y: f64) {
    if x > y {
        a()
    } else if x < y {
        b()
    } else {
        c()
    }
}

Reproduced on:
clippy 0.0.212 (3aea860 2019-09-03)
clippy 0.0.212 (4e7e71b 2019-10-11)

Activity

  1. flip1995 commented on Nov 19, 2019

    @flip1995
    Member

    We should only lint for types, that implement Ord. It would be possible to implement the same lint for PartialOrd, but put it in the pedantic group.

  2. added
    good first issueThese issues are a good way to get started with Clippy
    C-enhancementCategory: Enhancement of lints, like adding more cases or adding help messages
    A-lintArea: New lints
    on Nov 19, 2019
  3. changed the title [-]`clippy::comparison_chain` false positive on PartialOrd or desired behavior[/-] [+]`clippy::comparison_chain` false positive on PartialOrd[/+] on Nov 19, 2019
  4. added 2 commits that reference this issue on Nov 23, 2019
  5. added a commit that references this issue on Nov 28, 2019
  6. added a commit that references this issue on Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintArea: New lintsC-enhancementCategory: Enhancement of lints, like adding more cases or adding help messagesgood first issueThese issues are a good way to get started with Clippy

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions