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 #12312

Merged
merged 2 commits into from Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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
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
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
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
293 changes: 293 additions & 0 deletions clippy_lints/src/legacy_numeric_constants.rs
@@ -0,0 +1,293 @@
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"]
pitaj marked this conversation as resolved.
Show resolved Hide resolved
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 {
matches!(
cx.tcx.get_diagnostic_name(did),
Some(
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
)
)
}

fn is_numeric_const(cx: &LateContext<'_>, did: DefId) -> bool {
matches!(
cx.tcx.get_diagnostic_name(did),
Some(
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
)
)
}

// 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 {
matches!(
cx.tcx.get_diagnostic_name(did),
Some(
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
)
)
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
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
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