Skip to content

Commit be8a3ab

Browse files
JarchoJarcho
committed
Add config options to ignore constant comparisons and comparisons to constants to float_cmp 
1 parent 70e5827 commit be8a3ab

File tree

16 files changed

+622
-330
lines changed

16 files changed

+622
-330
lines changed

‎clippy_config/src/conf.rs

+29
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,35 @@ define_Conf! {
587587
/// 2. Paths with any segment that containing the word 'prelude'
588588
/// are already allowed by default.
589589
(allowed_wildcard_imports: FxHashSet<String> = FxHashSet::default()),
590+
/// Lint: FLOAT_CMP
591+
///
592+
/// Whether to ignore comparisons to a named constnat
593+
///
594+
/// #### Example
595+
/// ```no_run
596+
/// const VALUE: f64 = 1.0;
597+
/// fn is_value(x: f64) -> bool {
598+
/// // Will warn if the config is `false`
599+
/// x == VALUE
600+
/// }
601+
/// ```
602+
(float_cmp_ignore_named_constants: bool = true),
603+
/// Lint: FLOAT_CMP
604+
///
605+
/// Whether to ignore comparisons which have a constant result.
606+
///
607+
/// #### Example
608+
/// ```no_run
609+
/// const fn f(x: f64) -> f64 {
610+
/// todo!()
611+
/// }
612+
///
613+
/// // Will warn if the config is `false`
614+
/// if f(1.0) == f(2.0) {
615+
/// // ...
616+
/// }
617+
/// ```
618+
(float_cmp_ignore_constant_comparisons: bool = true),
590619
}
591620

592621
/// Search for the configuration file.

‎clippy_lints/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
580580
pub_underscore_fields_behavior,
581581
ref allowed_duplicate_crates,
582582
allow_comparison_to_zero,
583+
float_cmp_ignore_named_constants,
584+
float_cmp_ignore_constant_comparisons,
583585

584586
blacklisted_names: _,
585587
cyclomatic_complexity_threshold: _,
@@ -982,6 +984,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
982984
Box::new(operators::Operators::new(
983985
verbose_bit_mask_threshold,
984986
allow_comparison_to_zero,
987+
float_cmp_ignore_named_constants,
988+
float_cmp_ignore_constant_comparisons,
985989
))
986990
});
987991
store.register_late_pass(|_| Box::<std_instead_of_core::StdReexports>::default());

‎clippy_lints/src/operators/float_cmp.rs

+42-41
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,54 @@
1-
use clippy_utils::consts::{constant_with_source, Constant};
1+
use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::get_item_name;
43
use clippy_utils::sugg::Sugg;
4+
use clippy_utils::visitors::is_const_evaluatable;
5+
use clippy_utils::{get_item_name, is_expr_named_const, peel_hir_expr_while};
56
use rustc_errors::Applicability;
6-
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
7+
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, UnOp};
78
use rustc_lint::LateContext;
89
use rustc_middle::ty;
910

10-
use super::{FLOAT_CMP, FLOAT_CMP_CONST};
11+
use super::{FloatCmpConfig, FLOAT_CMP};
1112

1213
pub(crate) fn check<'tcx>(
1314
cx: &LateContext<'tcx>,
15+
config: FloatCmpConfig,
1416
expr: &'tcx Expr<'_>,
1517
op: BinOpKind,
1618
left: &'tcx Expr<'_>,
1719
right: &'tcx Expr<'_>,
1820
) {
19-
if (op == BinOpKind::Eq || op == BinOpKind::Ne) && is_float(cx, left) {
20-
let left_is_local = match constant_with_source(cx, cx.typeck_results(), left) {
21-
Some((c, s)) if !is_allowed(&c) => s.is_local(),
22-
Some(_) => return,
23-
None => true,
24-
};
25-
let right_is_local = match constant_with_source(cx, cx.typeck_results(), right) {
26-
Some((c, s)) if !is_allowed(&c) => s.is_local(),
27-
Some(_) => return,
28-
None => true,
29-
};
30-
21+
if (op == BinOpKind::Eq || op == BinOpKind::Ne)
22+
&& is_float(cx, left)
3123
// Allow comparing the results of signum()
32-
if is_signum(cx, left) && is_signum(cx, right) {
24+
&& !(is_signum(cx, left) && is_signum(cx, right))
25+
{
26+
let left_c = constant(cx, cx.typeck_results(), left);
27+
let is_left_const = left_c.is_some();
28+
if left_c.is_some_and(|c| is_allowed(&c)) {
29+
return;
30+
}
31+
let right_c = constant(cx, cx.typeck_results(), right);
32+
let is_right_const = right_c.is_some();
33+
if right_c.is_some_and(|c| is_allowed(&c)) {
34+
return;
35+
}
36+
37+
if config.ignore_constant_comparisons
38+
&& (is_left_const || is_const_evaluatable(cx, left))
39+
&& (is_right_const || is_const_evaluatable(cx, right))
40+
{
41+
return;
42+
}
43+
44+
let peel_expr = |e: &'tcx Expr<'tcx>| match e.kind {
45+
ExprKind::Cast(e, _) | ExprKind::AddrOf(BorrowKind::Ref, _, e) => Some(e),
46+
_ => None,
47+
};
48+
if config.ignore_named_constants
49+
&& (is_expr_named_const(cx, peel_hir_expr_while(left, peel_expr))
50+
|| is_expr_named_const(cx, peel_hir_expr_while(right, peel_expr)))
51+
{
3352
return;
3453
}
3554

@@ -40,8 +59,12 @@ pub(crate) fn check<'tcx>(
4059
}
4160
}
4261
let is_comparing_arrays = is_array(cx, left) || is_array(cx, right);
43-
let (lint, msg) = get_lint_and_message(left_is_local && right_is_local, is_comparing_arrays);
44-
span_lint_and_then(cx, lint, expr.span, msg, |diag| {
62+
let msg = if is_comparing_arrays {
63+
"strict comparison of `f32` or `f64` arrays"
64+
} else {
65+
"strict comparison of `f32` or `f64`"
66+
};
67+
span_lint_and_then(cx, FLOAT_CMP, expr.span, msg, |diag| {
4568
let lhs = Sugg::hir(cx, left, "..");
4669
let rhs = Sugg::hir(cx, right, "..");
4770

@@ -62,28 +85,6 @@ pub(crate) fn check<'tcx>(
6285
}
6386
}
6487

65-
fn get_lint_and_message(is_local: bool, is_comparing_arrays: bool) -> (&'static rustc_lint::Lint, &'static str) {
66-
if is_local {
67-
(
68-
FLOAT_CMP,
69-
if is_comparing_arrays {
70-
"strict comparison of `f32` or `f64` arrays"
71-
} else {
72-
"strict comparison of `f32` or `f64`"
73-
},
74-
)
75-
} else {
76-
(
77-
FLOAT_CMP_CONST,
78-
if is_comparing_arrays {
79-
"strict comparison of `f32` or `f64` constant arrays"
80-
} else {
81-
"strict comparison of `f32` or `f64` constant"
82-
},
83-
)
84-
}
85-
}
86-
8788
fn is_allowed(val: &Constant<'_>) -> bool {
8889
match val {
8990
Constant::Ref(val) => is_allowed(val),

‎clippy_lints/src/operators/mod.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -768,10 +768,18 @@ declare_clippy_lint! {
768768
"explicit self-assignment"
769769
}
770770

771+
#[expect(clippy::struct_field_names)]
772+
#[derive(Clone, Copy)]
773+
pub struct FloatCmpConfig {
774+
pub ignore_named_constants: bool,
775+
pub ignore_constant_comparisons: bool,
776+
}
777+
771778
pub struct Operators {
772779
arithmetic_context: numeric_arithmetic::Context,
773780
verbose_bit_mask_threshold: u64,
774781
modulo_arithmetic_allow_comparison_to_zero: bool,
782+
float_cmp_config: FloatCmpConfig,
775783
}
776784
impl_lint_pass!(Operators => [
777785
ABSURD_EXTREME_COMPARISONS,
@@ -802,11 +810,20 @@ impl_lint_pass!(Operators => [
802810
SELF_ASSIGNMENT,
803811
]);
804812
impl Operators {
805-
pub fn new(verbose_bit_mask_threshold: u64, modulo_arithmetic_allow_comparison_to_zero: bool) -> Self {
813+
pub fn new(
814+
verbose_bit_mask_threshold: u64,
815+
modulo_arithmetic_allow_comparison_to_zero: bool,
816+
ignore_named_constants: bool,
817+
ignore_constant_comparisons: bool,
818+
) -> Self {
806819
Self {
807820
arithmetic_context: numeric_arithmetic::Context::default(),
808821
verbose_bit_mask_threshold,
809822
modulo_arithmetic_allow_comparison_to_zero,
823+
float_cmp_config: FloatCmpConfig {
824+
ignore_named_constants,
825+
ignore_constant_comparisons,
826+
},
810827
}
811828
}
812829
}
@@ -835,7 +852,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
835852
float_equality_without_abs::check(cx, e, op.node, lhs, rhs);
836853
integer_division::check(cx, e, op.node, lhs, rhs);
837854
cmp_owned::check(cx, op.node, lhs, rhs);
838-
float_cmp::check(cx, e, op.node, lhs, rhs);
855+
float_cmp::check(cx, self.float_cmp_config, e, op.node, lhs, rhs);
839856
modulo_one::check(cx, e, op.node, rhs);
840857
modulo_arithmetic::check(
841858
cx,

‎clippy_utils/src/lib.rs

+10
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,16 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool {
200200
cx.tcx.hir().is_inside_const_context(id)
201201
}
202202

203+
/// Checks if the expression is path to either a constant or an associated constant.
204+
pub fn is_expr_named_const<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
205+
matches!(&e.kind, ExprKind::Path(p)
206+
if matches!(
207+
cx.qpath_res(p, e.hir_id),
208+
Res::Def(DefKind::Const | DefKind::AssocConst, _)
209+
)
210+
)
211+
}
212+
203213
/// Checks if a `Res` refers to a constructor of a `LangItem`
204214
/// For example, use this to check whether a function call or a pattern is `Some(..)`.
205215
pub fn is_res_lang_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool {

‎tests/ui-toml/float_cmp_constant_comparisons/clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
float-cmp-ignore-constant-comparisons = false

‎tests/ui-toml/float_cmp_constant_comparisons/test.rs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//@no-rustfix
2+
3+
#![deny(clippy::float_cmp)]
4+
5+
fn main() {
6+
{
7+
const C: f64 = 1.0;
8+
fn f(x: f64) {
9+
let _ = x == C;
10+
}
11+
}
12+
{
13+
const fn f(x: f64) -> f64 {
14+
todo!()
15+
}
16+
let _ = f(1.0) == f(2.0);
17+
}
18+
}

‎tests/ui-toml/float_cmp_constant_comparisons/test.stderr

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: strict comparison of `f32` or `f64`
2+
--> $DIR/test.rs:16:17
3+
|
4+
LL | let _ = f(1.0) == f(2.0);
5+
| ^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(f(1.0) - f(2.0)).abs() < error_margin`
6+
|
7+
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
8+
note: the lint level is defined here
9+
--> $DIR/test.rs:3:9
10+
|
11+
LL | #![deny(clippy::float_cmp)]
12+
| ^^^^^^^^^^^^^^^^^
13+
14+
error: aborting due to 1 previous error
15+

‎tests/ui-toml/float_cmp_named_constants/clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
float-cmp-ignore-named-constants = false

‎tests/ui-toml/float_cmp_named_constants/test.rs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//@no-rustfix
2+
3+
#![deny(clippy::float_cmp)]
4+
5+
fn main() {
6+
{
7+
const C: f64 = 1.0;
8+
fn f(x: f64) {
9+
let _ = x == C;
10+
}
11+
}
12+
{
13+
const fn f(x: f64) -> f64 {
14+
todo!()
15+
}
16+
let _ = f(1.0) == f(2.0);
17+
}
18+
}

‎tests/ui-toml/float_cmp_named_constants/test.stderr

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: strict comparison of `f32` or `f64`
2+
--> $DIR/test.rs:9:21
3+
|
4+
LL | let _ = x == C;
5+
| ^^^^^^ help: consider comparing them within some margin of error: `(x - C).abs() < error_margin`
6+
|
7+
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
8+
note: the lint level is defined here
9+
--> $DIR/test.rs:3:9
10+
|
11+
LL | #![deny(clippy::float_cmp)]
12+
| ^^^^^^^^^^^^^^^^^
13+
14+
error: aborting due to 1 previous error
15+

‎tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
3838
enum-variant-name-threshold
3939
enum-variant-size-threshold
4040
excessive-nesting-threshold
41+
float-cmp-ignore-constant-comparisons
42+
float-cmp-ignore-named-constants
4143
future-size-threshold
4244
ignore-interior-mutability
4345
large-error-threshold
@@ -117,6 +119,8 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
117119
enum-variant-name-threshold
118120
enum-variant-size-threshold
119121
excessive-nesting-threshold
122+
float-cmp-ignore-constant-comparisons
123+
float-cmp-ignore-named-constants
120124
future-size-threshold
121125
ignore-interior-mutability
122126
large-error-threshold

0 commit comments

Comments
 (0)