Skip to content

Commit cebf879

Browse files
committed
Auto merge of #12312 - pitaj:legacy_numeric_constants, r=xFrednet
new lint `legacy_numeric_constants` Rework of #10997 - uses diagnostic items - does not lint imports of the float modules (`use std::f32`) - does not lint usage of float constants that look like `f32::MIN` I chose to make the float changes because the following pattern is actually pretty useful ```rust use std::f32; let omega = freq * 2 * f32::consts::PI; ``` and the float modules are not TBD-deprecated like the integer modules. Closes #10995 --- changelog: New lint [`legacy_numeric_constants`] [#12312](#12312)
2 parents e0e7ee1 + f2e91ab commit cebf879

22 files changed

+921
-37
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5379,6 +5379,7 @@ Released 2018-09-13
53795379
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
53805380
[`large_stack_frames`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames
53815381
[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value
5382+
[`legacy_numeric_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants
53825383
[`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
53835384
[`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
53845385
[`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return

book/src/lint_configuration.md

+1
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
616616
* [`if_then_some_else_none`](https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none)
617617
* [`index_refutable_slice`](https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice)
618618
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
619+
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
619620
* [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)
620621
* [`manual_c_str_literals`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals)
621622
* [`manual_clamp`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp)

clippy_config/src/conf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ define_Conf! {
262262
///
263263
/// Suppress lints whenever the suggested change would cause breakage for other crates.
264264
(avoid_breaking_exported_api: bool = true),
265-
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES.
265+
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS.
266266
///
267267
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
268268
#[default_text = ""]

clippy_config/src/msrvs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ msrv_aliases! {
3636
1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN }
3737
1,46,0 { CONST_IF_MATCH }
3838
1,45,0 { STR_STRIP_PREFIX }
39-
1,43,0 { LOG2_10, LOG10_2 }
39+
1,43,0 { LOG2_10, LOG10_2, NUMERIC_ASSOCIATED_CONSTANTS }
4040
1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS }
4141
1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE }
4242
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }

clippy_lints/src/casts/cast_possible_truncation.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
4141
})
4242
},
4343
BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right)
44-
.unwrap_or(u64::max_value())
44+
.unwrap_or(u64::MAX)
4545
.min(apply_reductions(cx, nbits, left, signed)),
4646
BinOpKind::Shr => apply_reductions(cx, nbits, left, signed)
4747
.saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or_default())),
@@ -56,7 +56,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
5656
} else {
5757
None
5858
};
59-
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
59+
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::MAX))
6060
},
6161
ExprKind::MethodCall(method, _, [lo, hi], _) => {
6262
if method.ident.as_str() == "clamp" {

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
254254
crate::large_include_file::LARGE_INCLUDE_FILE_INFO,
255255
crate::large_stack_arrays::LARGE_STACK_ARRAYS_INFO,
256256
crate::large_stack_frames::LARGE_STACK_FRAMES_INFO,
257+
crate::legacy_numeric_constants::LEGACY_NUMERIC_CONSTANTS_INFO,
257258
crate::len_zero::COMPARISON_TO_EMPTY_INFO,
258259
crate::len_zero::LEN_WITHOUT_IS_EMPTY_INFO,
259260
crate::len_zero::LEN_ZERO_INFO,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
use clippy_config::msrvs::{Msrv, NUMERIC_ASSOCIATED_CONSTANTS};
2+
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
3+
use clippy_utils::{get_parent_expr, is_from_proc_macro};
4+
use hir::def_id::DefId;
5+
use rustc_errors::{Applicability, SuggestionStyle};
6+
use rustc_hir as hir;
7+
use rustc_hir::{ExprKind, Item, ItemKind, QPath, UseKind};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::lint::in_external_macro;
10+
use rustc_session::impl_lint_pass;
11+
use rustc_span::symbol::kw;
12+
use rustc_span::{sym, Symbol};
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// Checks for usage of `<integer>::max_value()`, `std::<integer>::MAX`,
17+
/// `std::<float>::EPSILON`, etc.
18+
///
19+
/// ### Why is this bad?
20+
/// All of these have been superceded by the associated constants on their respective types,
21+
/// such as `i128::MAX`. These legacy items may be deprecated in a future version of rust.
22+
///
23+
/// ### Example
24+
/// ```rust
25+
/// let eps = std::f32::EPSILON;
26+
/// ```
27+
/// Use instead:
28+
/// ```rust
29+
/// let eps = f32::EPSILON;
30+
/// ```
31+
#[clippy::version = "1.72.0"]
32+
pub LEGACY_NUMERIC_CONSTANTS,
33+
style,
34+
"checks for usage of legacy std numeric constants and methods"
35+
}
36+
pub struct LegacyNumericConstants {
37+
msrv: Msrv,
38+
}
39+
40+
impl LegacyNumericConstants {
41+
#[must_use]
42+
pub fn new(msrv: Msrv) -> Self {
43+
Self { msrv }
44+
}
45+
}
46+
47+
impl_lint_pass!(LegacyNumericConstants => [LEGACY_NUMERIC_CONSTANTS]);
48+
49+
impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
50+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
51+
let Self { msrv } = self;
52+
53+
if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), item.span) {
54+
return;
55+
}
56+
57+
// Integer modules are "TBD" deprecated, and the contents are too,
58+
// so lint on the `use` statement directly.
59+
if let ItemKind::Use(path, kind @ (UseKind::Single | UseKind::Glob)) = item.kind
60+
&& let Some(def_id) = path.res[0].opt_def_id()
61+
{
62+
let module = if is_integer_module(cx, def_id) {
63+
true
64+
} else if is_numeric_const(cx, def_id) {
65+
false
66+
} else {
67+
return;
68+
};
69+
70+
span_lint_and_then(
71+
cx,
72+
LEGACY_NUMERIC_CONSTANTS,
73+
path.span,
74+
if module {
75+
"importing legacy numeric constants"
76+
} else {
77+
"importing a legacy numeric constant"
78+
},
79+
|diag| {
80+
if item.ident.name == kw::Underscore {
81+
diag.help("remove this import");
82+
return;
83+
}
84+
85+
let def_path = cx.get_def_path(def_id);
86+
87+
if module && let [.., module_name] = &*def_path {
88+
if kind == UseKind::Glob {
89+
diag.help(format!("remove this import and use associated constants `{module_name}::<CONST>` from the primitive type instead"));
90+
} else {
91+
diag.help("remove this import").note(format!(
92+
"then `{module_name}::<CONST>` will resolve to the respective associated constant"
93+
));
94+
}
95+
} else if let [.., module_name, name] = &*def_path {
96+
diag.help(
97+
format!("remove this import and use the associated constant `{module_name}::{name}` from the primitive type instead")
98+
);
99+
}
100+
},
101+
);
102+
}
103+
}
104+
105+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
106+
let Self { msrv } = self;
107+
108+
if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), expr.span) {
109+
return;
110+
}
111+
let ExprKind::Path(qpath) = expr.kind else {
112+
return;
113+
};
114+
115+
// `std::<integer>::<CONST>` check
116+
let (span, sugg, msg) = if let QPath::Resolved(None, path) = qpath
117+
&& let Some(def_id) = path.res.opt_def_id()
118+
&& is_numeric_const(cx, def_id)
119+
&& let def_path = cx.get_def_path(def_id)
120+
&& let [.., mod_name, name] = &*def_path
121+
// Skip linting if this usage looks identical to the associated constant,
122+
// since this would only require removing a `use` import (which is already linted).
123+
&& !is_numeric_const_path_canonical(path, [*mod_name, *name])
124+
{
125+
(
126+
expr.span,
127+
format!("{mod_name}::{name}"),
128+
"usage of a legacy numeric constant",
129+
)
130+
// `<integer>::xxx_value` check
131+
} else if let QPath::TypeRelative(_, last_segment) = qpath
132+
&& let Some(def_id) = cx.qpath_res(&qpath, expr.hir_id).opt_def_id()
133+
&& is_integer_method(cx, def_id)
134+
&& let Some(par_expr) = get_parent_expr(cx, expr)
135+
&& let ExprKind::Call(_, _) = par_expr.kind
136+
{
137+
let name = last_segment.ident.name.as_str();
138+
139+
(
140+
last_segment.ident.span.with_hi(par_expr.span.hi()),
141+
name[..=2].to_ascii_uppercase(),
142+
"usage of a legacy numeric method",
143+
)
144+
} else {
145+
return;
146+
};
147+
148+
if is_from_proc_macro(cx, expr) {
149+
return;
150+
}
151+
152+
span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
153+
diag.span_suggestion_with_style(
154+
span,
155+
"use the associated constant instead",
156+
sugg,
157+
Applicability::MaybeIncorrect,
158+
SuggestionStyle::ShowAlways,
159+
);
160+
});
161+
}
162+
163+
extract_msrv_attr!(LateContext);
164+
}
165+
166+
fn is_integer_module(cx: &LateContext<'_>, did: DefId) -> bool {
167+
matches!(
168+
cx.tcx.get_diagnostic_name(did),
169+
Some(
170+
sym::isize_legacy_mod
171+
| sym::i128_legacy_mod
172+
| sym::i64_legacy_mod
173+
| sym::i32_legacy_mod
174+
| sym::i16_legacy_mod
175+
| sym::i8_legacy_mod
176+
| sym::usize_legacy_mod
177+
| sym::u128_legacy_mod
178+
| sym::u64_legacy_mod
179+
| sym::u32_legacy_mod
180+
| sym::u16_legacy_mod
181+
| sym::u8_legacy_mod
182+
)
183+
)
184+
}
185+
186+
fn is_numeric_const(cx: &LateContext<'_>, did: DefId) -> bool {
187+
matches!(
188+
cx.tcx.get_diagnostic_name(did),
189+
Some(
190+
sym::isize_legacy_const_max
191+
| sym::isize_legacy_const_min
192+
| sym::i128_legacy_const_max
193+
| sym::i128_legacy_const_min
194+
| sym::i16_legacy_const_max
195+
| sym::i16_legacy_const_min
196+
| sym::i32_legacy_const_max
197+
| sym::i32_legacy_const_min
198+
| sym::i64_legacy_const_max
199+
| sym::i64_legacy_const_min
200+
| sym::i8_legacy_const_max
201+
| sym::i8_legacy_const_min
202+
| sym::usize_legacy_const_max
203+
| sym::usize_legacy_const_min
204+
| sym::u128_legacy_const_max
205+
| sym::u128_legacy_const_min
206+
| sym::u16_legacy_const_max
207+
| sym::u16_legacy_const_min
208+
| sym::u32_legacy_const_max
209+
| sym::u32_legacy_const_min
210+
| sym::u64_legacy_const_max
211+
| sym::u64_legacy_const_min
212+
| sym::u8_legacy_const_max
213+
| sym::u8_legacy_const_min
214+
| sym::f32_legacy_const_digits
215+
| sym::f32_legacy_const_epsilon
216+
| sym::f32_legacy_const_infinity
217+
| sym::f32_legacy_const_mantissa_dig
218+
| sym::f32_legacy_const_max
219+
| sym::f32_legacy_const_max_10_exp
220+
| sym::f32_legacy_const_max_exp
221+
| sym::f32_legacy_const_min
222+
| sym::f32_legacy_const_min_10_exp
223+
| sym::f32_legacy_const_min_exp
224+
| sym::f32_legacy_const_min_positive
225+
| sym::f32_legacy_const_nan
226+
| sym::f32_legacy_const_neg_infinity
227+
| sym::f32_legacy_const_radix
228+
| sym::f64_legacy_const_digits
229+
| sym::f64_legacy_const_epsilon
230+
| sym::f64_legacy_const_infinity
231+
| sym::f64_legacy_const_mantissa_dig
232+
| sym::f64_legacy_const_max
233+
| sym::f64_legacy_const_max_10_exp
234+
| sym::f64_legacy_const_max_exp
235+
| sym::f64_legacy_const_min
236+
| sym::f64_legacy_const_min_10_exp
237+
| sym::f64_legacy_const_min_exp
238+
| sym::f64_legacy_const_min_positive
239+
| sym::f64_legacy_const_nan
240+
| sym::f64_legacy_const_neg_infinity
241+
| sym::f64_legacy_const_radix
242+
)
243+
)
244+
}
245+
246+
// Whether path expression looks like `i32::MAX`
247+
fn is_numeric_const_path_canonical(expr_path: &hir::Path<'_>, [mod_name, name]: [Symbol; 2]) -> bool {
248+
let [
249+
hir::PathSegment {
250+
ident: one, args: None, ..
251+
},
252+
hir::PathSegment {
253+
ident: two, args: None, ..
254+
},
255+
] = expr_path.segments
256+
else {
257+
return false;
258+
};
259+
260+
one.name == mod_name && two.name == name
261+
}
262+
263+
fn is_integer_method(cx: &LateContext<'_>, did: DefId) -> bool {
264+
matches!(
265+
cx.tcx.get_diagnostic_name(did),
266+
Some(
267+
sym::isize_legacy_fn_max_value
268+
| sym::isize_legacy_fn_min_value
269+
| sym::i128_legacy_fn_max_value
270+
| sym::i128_legacy_fn_min_value
271+
| sym::i16_legacy_fn_max_value
272+
| sym::i16_legacy_fn_min_value
273+
| sym::i32_legacy_fn_max_value
274+
| sym::i32_legacy_fn_min_value
275+
| sym::i64_legacy_fn_max_value
276+
| sym::i64_legacy_fn_min_value
277+
| sym::i8_legacy_fn_max_value
278+
| sym::i8_legacy_fn_min_value
279+
| sym::usize_legacy_fn_max_value
280+
| sym::usize_legacy_fn_min_value
281+
| sym::u128_legacy_fn_max_value
282+
| sym::u128_legacy_fn_min_value
283+
| sym::u16_legacy_fn_max_value
284+
| sym::u16_legacy_fn_min_value
285+
| sym::u32_legacy_fn_max_value
286+
| sym::u32_legacy_fn_min_value
287+
| sym::u64_legacy_fn_max_value
288+
| sym::u64_legacy_fn_min_value
289+
| sym::u8_legacy_fn_max_value
290+
| sym::u8_legacy_fn_min_value
291+
)
292+
)
293+
}

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ mod large_futures;
189189
mod large_include_file;
190190
mod large_stack_arrays;
191191
mod large_stack_frames;
192+
mod legacy_numeric_constants;
192193
mod len_zero;
193194
mod let_if_seq;
194195
mod let_underscore;
@@ -1083,6 +1084,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10831084
allow_one_hash_in_raw_strings,
10841085
})
10851086
});
1087+
store.register_late_pass(move |_| Box::new(legacy_numeric_constants::LegacyNumericConstants::new(msrv())));
10861088
store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
10871089
store.register_early_pass(|| Box::new(visibility::Visibility));
10881090
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));

clippy_utils/src/check_proc_macro.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use rustc_hir::{
2424
use rustc_lint::{LateContext, LintContext};
2525
use rustc_middle::ty::TyCtxt;
2626
use rustc_session::Session;
27-
use rustc_span::symbol::Ident;
27+
use rustc_span::symbol::{kw, Ident};
2828
use rustc_span::{Span, Symbol};
2929
use rustc_target::spec::abi::Abi;
3030

@@ -99,9 +99,13 @@ fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) {
9999
let start = if ty.is_some() {
100100
Pat::Str("<")
101101
} else {
102-
path.segments
103-
.first()
104-
.map_or(Pat::Str(""), |seg| Pat::Sym(seg.ident.name))
102+
path.segments.first().map_or(Pat::Str(""), |seg| {
103+
if seg.ident.name == kw::PathRoot {
104+
Pat::Str("::")
105+
} else {
106+
Pat::Sym(seg.ident.name)
107+
}
108+
})
105109
};
106110
let end = path.segments.last().map_or(Pat::Str(""), |seg| {
107111
if seg.args.is_some() {

0 commit comments

Comments
 (0)