Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint [legacy_numeric_constants] #10997

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4956,6 +4956,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 @@ -151,6 +151,7 @@ The minimum rust version that the project supports
* [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds)
* [`tuple_array_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions)
* [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold)
* [`legacy_numeric_constants`](https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants)


## `cognitive-complexity-threshold`
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).expect("shift too high"))),
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 @@ -234,6 +234,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
24 changes: 12 additions & 12 deletions clippy_lints/src/implicit_saturating_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,18 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingAdd {

fn get_int_max(ty: Ty<'_>) -> Option<u128> {
match ty.peel_refs().kind() {
Int(IntTy::I8) => i8::max_value().try_into().ok(),
Int(IntTy::I16) => i16::max_value().try_into().ok(),
Int(IntTy::I32) => i32::max_value().try_into().ok(),
Int(IntTy::I64) => i64::max_value().try_into().ok(),
Int(IntTy::I128) => i128::max_value().try_into().ok(),
Int(IntTy::Isize) => isize::max_value().try_into().ok(),
Uint(UintTy::U8) => u8::max_value().try_into().ok(),
Uint(UintTy::U16) => u16::max_value().try_into().ok(),
Uint(UintTy::U32) => u32::max_value().try_into().ok(),
Uint(UintTy::U64) => u64::max_value().try_into().ok(),
Uint(UintTy::U128) => Some(u128::max_value()),
Uint(UintTy::Usize) => usize::max_value().try_into().ok(),
Int(IntTy::I8) => i8::MAX.try_into().ok(),
Int(IntTy::I16) => i16::MAX.try_into().ok(),
Int(IntTy::I32) => i32::MAX.try_into().ok(),
Int(IntTy::I64) => i64::MAX.try_into().ok(),
Int(IntTy::I128) => i128::MAX.try_into().ok(),
Int(IntTy::Isize) => isize::MAX.try_into().ok(),
Uint(UintTy::U8) => u8::MAX.try_into().ok(),
Uint(UintTy::U16) => u16::MAX.try_into().ok(),
Uint(UintTy::U32) => u32::MAX.try_into().ok(),
Uint(UintTy::U64) => u64::MAX.try_into().ok(),
Uint(UintTy::U128) => Some(u128::MAX),
Uint(UintTy::Usize) => usize::MAX.try_into().ok(),
_ => None,
}
}
Expand Down
223 changes: 223 additions & 0 deletions clippy_lints/src/legacy_numeric_constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::msrvs::{Msrv, NUMERIC_ASSOCIATED_CONSTANTS};
use clippy_utils::source::snippet_opt;
use clippy_utils::{get_parent_expr, is_from_proc_macro, last_path_segment, std_or_core};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{Expr, ExprKind, Item, ItemKind, PrimTy, QPath, TyKind, UseKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::nested_filter::OnlyBodies;
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, 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)
&& let ItemKind::Use(path, kind @ (UseKind::Single | UseKind::Glob)) = item.kind
// These modules are "TBD" deprecated, and the contents are too, so lint on the `use`
// statement directly
&& let def_path = cx.get_def_path(path.res[0].def_id())
&& is_path_in_numeric_module(&def_path, true)
{
let plurality = matches!(
kind,
UseKind::Glob | UseKind::Single if matches!(path.res[0], Res::Def(DefKind::Mod, _)),
);

span_lint_and_then(
cx,
LEGACY_NUMERIC_CONSTANTS,
path.span,
if plurality {
"importing legacy numeric constants"
} else {
"importing a legacy numeric constant"
},
|diag| {
if item.ident.name != kw::Underscore {
let msg = if plurality && let [.., module_name] = &*def_path {
format!("use the associated constants on `{module_name}` instead at their usage")
} else if let [.., module_name, name] = &*def_path {
format!("use the associated constant `{module_name}::{name}` instead at its usage")
} else {
return;
};

diag.help(msg);
}
},
);
}
}

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

cx.tcx.hir().visit_all_item_likes_in_crate(&mut V { cx, msrv });
}

extract_msrv_attr!(LateContext);
}

struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
msrv: &'a Msrv,
}

impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
type NestedFilter = OnlyBodies;

fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
walk_expr(self, expr);
let Self { cx, 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()
&& let path = cx.get_def_path(def_id)
&& is_path_in_numeric_module(&path, false)
&& let [.., module_name, name] = &*path
&& let Some(snippet) = snippet_opt(cx, expr.span)
&& let is_float_module = (*module_name == sym::f32 || *module_name == sym::f64)
// Skip linting if this usage looks identical to the associated constant, since this
// would only require removing a `use` import. We don't ignore ones from `f32` or `f64`, however.
&& let identical = snippet == format!("{module_name}::{name}")
&& (!identical || is_float_module)
{
(
expr.span,
if identical {
None
} else {
Some(format!("{module_name}::{name}"))
},
"usage of a legacy numeric constant",
)
// `<integer>::xxx_value` check
} else if let QPath::TypeRelative(ty, _) = qpath
&& let TyKind::Path(ty_qpath) = ty.kind
&& let Res::PrimTy(PrimTy::Int(_) | PrimTy::Uint(_)) = cx.qpath_res(&ty_qpath, ty.hir_id)
&& let last_segment = last_path_segment(&qpath)
&& let name = last_segment.ident.name.as_str()
&& (name == "max_value" || name == "min_value")
// Also remove the `()`
&& let Some(par_expr) = get_parent_expr(cx, expr)
&& let ExprKind::Call(_, _) = par_expr.kind
{
(
qpath.last_segment_span().with_hi(par_expr.span.hi()),
Some(name[..=2].to_ascii_uppercase()),
"usage of a legacy numeric method",
)
} else {
return;
};

if !is_from_proc_macro(cx, expr) {
span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
if let Some(sugg) = sugg {
diag.span_suggestion(
span,
"use the associated constant instead",
sugg,
Applicability::MaybeIncorrect,
);
} else if let Some(std_or_core) = std_or_core(cx)
&& let QPath::Resolved(None, path) = qpath
{
diag.help(format!(
"remove the import that brings `{std_or_core}::{}` into scope",
// Must be `<module>::<CONST>` if `needs_import_removed` is true yet is
// being linted
path.segments[0].ident.name,
));
}
});
}
}
}

fn is_path_in_numeric_module(path: &[Symbol], ignore_float_modules: bool) -> bool {
if let [
sym::core,
module @ (sym::u8
| sym::i8
| sym::u16
| sym::i16
| sym::u32
| sym::i32
| sym::u64
| sym::i64
| sym::u128
| sym::i128
| sym::usize
| sym::isize
| sym::f32
| sym::f64),
..,
] = path
&& !path.get(2).is_some_and(|&s| s == sym!(consts))
{
// If `ignore_float_modules` is `true`, return `None` for `_::f32` or `_::f64`, but not
// _::f64::MAX` or similar.
if ignore_float_modules && (*module == sym::f32 || *module == sym::f64) && path.get(2).is_none() {
return false;
}

return true;
}

false
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,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 @@ -1079,6 +1080,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
needless_raw_string_hashes_allow_one,
})
});
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
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,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.
/// 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, LEGACY_NUMERIC_CONSTANTS.
///
/// The minimum rust version that the project supports
(msrv: Option<String> = None),
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 @@ -103,9 +103,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
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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
1 change: 1 addition & 0 deletions tests/ui/checked_conversions.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![allow(
clippy::cast_lossless,
clippy::legacy_numeric_constants,
unused,
// Int::max_value will be deprecated in the future
deprecated,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/checked_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![allow(
clippy::cast_lossless,
clippy::legacy_numeric_constants,
unused,
// Int::max_value will be deprecated in the future
deprecated,
Expand Down
Loading