assert_eq! with a literal float should probably fire float_cmp_const not float_cmp · rust-lang/rust-clippy#6817

(11 comments) (0 reactions) (0 assignees)Rust (10,406 stars) (1,391 forks)batch import

C-bugI-false-positivegood first issue

Description

Example:

pub const RADIANS_PER_FURMAN: f64 = f64::consts::PI / 32_768_f64;
pub const FURMANS_PER_RADIAN: f64 = 32_768_f64 / f64::consts::PI;
pub const DEGREES_PER_FURMAN: f64 = 45_f64 / 8192_f64;
pub const FURMANS_PER_DEGREE: f64 = 8192_f64 / 45_f64;

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn constants_are_right() {
        // decimal expansions retrieved with WolframAlpha and truncated by clippy
        assert_eq!(RADIANS_PER_FURMAN, 0.000_095_873_799_242_852_57); // exact
        assert_eq!(FURMANS_PER_RADIAN, 10_430.378_350_470_453); // exact
        assert_eq!(DEGREES_PER_FURMAN, 0.005_493_164_062_5); // exact
        assert_eq!(FURMANS_PER_DEGREE, 182.044_444_444_444_45); // exact
    }
}

Here float_cmp fires. Given that this is comparing a const to a literal float, however, this should almost certainly be float_cmp_const instead (which is in the restriction group, as opposed to float_cmp in pedantic). (float_cmp used to be more aggressively linted, but both lints are allow by default nowadays.)

This seems like a reasonable usage of exact floating point equality: defining consts with the intuitive definition and asserting that the produced value is precise within ±½ ULP, by checking against an alternatively produced value.

(Yes, furmans are a real unit, even if unusual/whimsical. Furman is the name for 1/65536th of a revolution, such that one revolution divides evenly into u16.)

Contributor guide

Tech stack: rust
Domain: developer experiencetooling
Issue type: bug
Difficulty: 3
Estimated time: half day
Activity status: stale
Clarity: clear
Prerequisites: Rust programmingclippy lint internalsfloat comparison concepts
Newbie friendliness: 35
Research direction: The issue requires modifying the `float cmp` lint in clippy to detect when it is fired on `assert eq!` with a literal float and instead emit `float cmp const`. The relevant code is in `clippy lints/src/float cmp.rs` and `clippy lints/src/float cmp const.rs`. The fix should involve checking the context of the comparison and redirecting to the appropriate lint. Tests in `tests/ui/float cmp.rs` and `tests/ui/float cmp const.rs` should be updated to verify the new behavior.