Skip to content

Commit

Permalink
new lint legacy_numeric_constants
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 authored and pitaj committed Mar 21, 2024
1 parent 9d6f416 commit 0c392d9
Show file tree
Hide file tree
Showing 22 changed files with 921 additions and 37 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5379,6 +5379,7 @@ Released 2018-09-13
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
[`large_stack_frames`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames
[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value
[`legacy_numeric_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants
[`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
[`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
[`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
* [`if_then_some_else_none`](https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none)
* [`index_refutable_slice`](https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice)
* [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)
* [`manual_bits`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits)
* [`manual_c_str_literals`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals)
* [`manual_clamp`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp)
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// 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.
/// 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.
///
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
#[default_text = ""]
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ msrv_aliases! {
1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN }
1,46,0 { CONST_IF_MATCH }
1,45,0 { STR_STRIP_PREFIX }
1,43,0 { LOG2_10, LOG10_2 }
1,43,0 { LOG2_10, LOG10_2, NUMERIC_ASSOCIATED_CONSTANTS }
1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS }
1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE }
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
})
},
BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right)
.unwrap_or(u64::max_value())
.unwrap_or(u64::MAX)
.min(apply_reductions(cx, nbits, left, signed)),
BinOpKind::Shr => apply_reductions(cx, nbits, left, signed)
.saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or_default())),
Expand All @@ -56,7 +56,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
} else {
None
};
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::MAX))
},
ExprKind::MethodCall(method, _, [lo, hi], _) => {
if method.ident.as_str() == "clamp" {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::large_include_file::LARGE_INCLUDE_FILE_INFO,
crate::large_stack_arrays::LARGE_STACK_ARRAYS_INFO,
crate::large_stack_frames::LARGE_STACK_FRAMES_INFO,
crate::legacy_numeric_constants::LEGACY_NUMERIC_CONSTANTS_INFO,
crate::len_zero::COMPARISON_TO_EMPTY_INFO,
crate::len_zero::LEN_WITHOUT_IS_EMPTY_INFO,
crate::len_zero::LEN_ZERO_INFO,
Expand Down
290 changes: 290 additions & 0 deletions clippy_lints/src/legacy_numeric_constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,290 @@
use clippy_config::msrvs::{Msrv, NUMERIC_ASSOCIATED_CONSTANTS};
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::{get_parent_expr, is_from_proc_macro};
use hir::def_id::DefId;
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir as hir;
use rustc_hir::{ExprKind, Item, ItemKind, QPath, UseKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::impl_lint_pass;
use rustc_span::symbol::kw;
use rustc_span::{sym, Symbol};

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `<integer>::max_value()`, `std::<integer>::MAX`,
/// `std::<float>::EPSILON`, etc.
///
/// ### Why is this bad?
/// All of these have been superceded by the associated constants on their respective types,
/// such as `i128::MAX`. These legacy items may be deprecated in a future version of rust.
///
/// ### Example
/// ```rust
/// let eps = std::f32::EPSILON;
/// ```
/// Use instead:
/// ```rust
/// let eps = f32::EPSILON;
/// ```
#[clippy::version = "1.72.0"]
pub LEGACY_NUMERIC_CONSTANTS,
style,
"checks for usage of legacy std numeric constants and methods"
}
pub struct LegacyNumericConstants {
msrv: Msrv,
}

impl LegacyNumericConstants {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
}
}

impl_lint_pass!(LegacyNumericConstants => [LEGACY_NUMERIC_CONSTANTS]);

impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Self { msrv } = self;

if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), item.span) {
return;
}

// Integer modules are "TBD" deprecated, and the contents are too,
// so lint on the `use` statement directly.
if let ItemKind::Use(path, kind @ (UseKind::Single | UseKind::Glob)) = item.kind
&& let Some(def_id) = path.res[0].opt_def_id()
{
let module = if is_integer_module(cx, def_id) {
true
} else if is_numeric_const(cx, def_id) {
false
} else {
return;
};

span_lint_and_then(
cx,
LEGACY_NUMERIC_CONSTANTS,
path.span,
if module {
"importing legacy numeric constants"
} else {
"importing a legacy numeric constant"
},
|diag| {
if item.ident.name == kw::Underscore {
diag.help("remove this import");
return;
}

let def_path = cx.get_def_path(def_id);

if module && let [.., module_name] = &*def_path {
if kind == UseKind::Glob {
diag.help(format!("remove this import and use associated constants `{module_name}::<CONST>` from the primitive type instead"));
} else {
diag.help("remove this import").note(format!(
"then `{module_name}::<CONST>` will resolve to the respective associated constant"
));
}
} else if let [.., module_name, name] = &*def_path {
diag.help(
format!("remove this import and use the associated constant `{module_name}::{name}` from the primitive type instead")
);
}
},
);
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
let Self { msrv } = self;

if !msrv.meets(NUMERIC_ASSOCIATED_CONSTANTS) || in_external_macro(cx.sess(), expr.span) {
return;
}
let ExprKind::Path(qpath) = expr.kind else {
return;
};

// `std::<integer>::<CONST>` check
let (span, sugg, msg) = if let QPath::Resolved(None, path) = qpath
&& let Some(def_id) = path.res.opt_def_id()
&& is_numeric_const(cx, def_id)
&& let def_path = cx.get_def_path(def_id)
&& let [.., mod_name, name] = &*def_path
// Skip linting if this usage looks identical to the associated constant,
// since this would only require removing a `use` import (which is already linted).
&& !is_numeric_const_path_canonical(path, [*mod_name, *name])
{
(
expr.span,
format!("{mod_name}::{name}"),
"usage of a legacy numeric constant",
)
// `<integer>::xxx_value` check
} else if let QPath::TypeRelative(_, last_segment) = qpath
&& let Some(def_id) = cx.qpath_res(&qpath, expr.hir_id).opt_def_id()
&& is_integer_method(cx, def_id)
&& let Some(par_expr) = get_parent_expr(cx, expr)
&& let ExprKind::Call(_, _) = par_expr.kind
{
let name = last_segment.ident.name.as_str();

(
last_segment.ident.span.with_hi(par_expr.span.hi()),
name[..=2].to_ascii_uppercase(),
"usage of a legacy numeric method",
)
} else {
return;
};

if is_from_proc_macro(cx, expr) {
return;
}

span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
diag.span_suggestion_with_style(
span,
"use the associated constant instead",
sugg,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
});
}

extract_msrv_attr!(LateContext);
}

fn is_integer_module(cx: &LateContext<'_>, did: DefId) -> bool {
[
sym::isize_legacy_mod,
sym::i128_legacy_mod,
sym::i64_legacy_mod,
sym::i32_legacy_mod,
sym::i16_legacy_mod,
sym::i8_legacy_mod,
sym::usize_legacy_mod,
sym::u128_legacy_mod,
sym::u64_legacy_mod,
sym::u32_legacy_mod,
sym::u16_legacy_mod,
sym::u8_legacy_mod,
]
.iter()
.any(|&name| cx.tcx.is_diagnostic_item(name, did))
}

fn is_numeric_const(cx: &LateContext<'_>, did: DefId) -> bool {
[
sym::isize_legacy_const_max,
sym::isize_legacy_const_min,
sym::i128_legacy_const_max,
sym::i128_legacy_const_min,
sym::i16_legacy_const_max,
sym::i16_legacy_const_min,
sym::i32_legacy_const_max,
sym::i32_legacy_const_min,
sym::i64_legacy_const_max,
sym::i64_legacy_const_min,
sym::i8_legacy_const_max,
sym::i8_legacy_const_min,
sym::usize_legacy_const_max,
sym::usize_legacy_const_min,
sym::u128_legacy_const_max,
sym::u128_legacy_const_min,
sym::u16_legacy_const_max,
sym::u16_legacy_const_min,
sym::u32_legacy_const_max,
sym::u32_legacy_const_min,
sym::u64_legacy_const_max,
sym::u64_legacy_const_min,
sym::u8_legacy_const_max,
sym::u8_legacy_const_min,
sym::f32_legacy_const_digits,
sym::f32_legacy_const_epsilon,
sym::f32_legacy_const_infinity,
sym::f32_legacy_const_mantissa_dig,
sym::f32_legacy_const_max,
sym::f32_legacy_const_max_10_exp,
sym::f32_legacy_const_max_exp,
sym::f32_legacy_const_min,
sym::f32_legacy_const_min_10_exp,
sym::f32_legacy_const_min_exp,
sym::f32_legacy_const_min_positive,
sym::f32_legacy_const_nan,
sym::f32_legacy_const_neg_infinity,
sym::f32_legacy_const_radix,
sym::f64_legacy_const_digits,
sym::f64_legacy_const_epsilon,
sym::f64_legacy_const_infinity,
sym::f64_legacy_const_mantissa_dig,
sym::f64_legacy_const_max,
sym::f64_legacy_const_max_10_exp,
sym::f64_legacy_const_max_exp,
sym::f64_legacy_const_min,
sym::f64_legacy_const_min_10_exp,
sym::f64_legacy_const_min_exp,
sym::f64_legacy_const_min_positive,
sym::f64_legacy_const_nan,
sym::f64_legacy_const_neg_infinity,
sym::f64_legacy_const_radix,
]
.iter()
.any(|&name| cx.tcx.is_diagnostic_item(name, did))
}

// Whether path expression looks like `i32::MAX`
fn is_numeric_const_path_canonical(expr_path: &hir::Path<'_>, [mod_name, name]: [Symbol; 2]) -> bool {
let [
hir::PathSegment {
ident: one, args: None, ..
},
hir::PathSegment {
ident: two, args: None, ..
},
] = expr_path.segments
else {
return false;
};

one.name == mod_name && two.name == name
}

fn is_integer_method(cx: &LateContext<'_>, did: DefId) -> bool {
[
sym::isize_legacy_fn_max_value,
sym::isize_legacy_fn_min_value,
sym::i128_legacy_fn_max_value,
sym::i128_legacy_fn_min_value,
sym::i16_legacy_fn_max_value,
sym::i16_legacy_fn_min_value,
sym::i32_legacy_fn_max_value,
sym::i32_legacy_fn_min_value,
sym::i64_legacy_fn_max_value,
sym::i64_legacy_fn_min_value,
sym::i8_legacy_fn_max_value,
sym::i8_legacy_fn_min_value,
sym::usize_legacy_fn_max_value,
sym::usize_legacy_fn_min_value,
sym::u128_legacy_fn_max_value,
sym::u128_legacy_fn_min_value,
sym::u16_legacy_fn_max_value,
sym::u16_legacy_fn_min_value,
sym::u32_legacy_fn_max_value,
sym::u32_legacy_fn_min_value,
sym::u64_legacy_fn_max_value,
sym::u64_legacy_fn_min_value,
sym::u8_legacy_fn_max_value,
sym::u8_legacy_fn_min_value,
]
.iter()
.any(|&name| cx.tcx.is_diagnostic_item(name, did))
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ mod large_futures;
mod large_include_file;
mod large_stack_arrays;
mod large_stack_frames;
mod legacy_numeric_constants;
mod len_zero;
mod let_if_seq;
mod let_underscore;
Expand Down Expand Up @@ -1080,6 +1081,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
allow_one_hash_in_raw_strings,
})
});
store.register_late_pass(move |_| Box::new(legacy_numeric_constants::LegacyNumericConstants::new(msrv())));
store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
store.register_early_pass(|| Box::new(visibility::Visibility));
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));
Expand Down
12 changes: 8 additions & 4 deletions clippy_utils/src/check_proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_hir::{
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::TyCtxt;
use rustc_session::Session;
use rustc_span::symbol::Ident;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{Span, Symbol};
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -99,9 +99,13 @@ fn qpath_search_pat(path: &QPath<'_>) -> (Pat, Pat) {
let start = if ty.is_some() {
Pat::Str("<")
} else {
path.segments
.first()
.map_or(Pat::Str(""), |seg| Pat::Sym(seg.ident.name))
path.segments.first().map_or(Pat::Str(""), |seg| {
if seg.ident.name == kw::PathRoot {
Pat::Str("::")
} else {
Pat::Sym(seg.ident.name)
}
})
};
let end = path.segments.last().map_or(Pat::Str(""), |seg| {
if seg.args.is_some() {
Expand Down
Loading

0 comments on commit 0c392d9

Please sign in to comment.