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 4, 2024
1 parent c2dd413 commit c5edb8e
Show file tree
Hide file tree
Showing 22 changed files with 839 additions and 37 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5321,6 +5321,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 @@ -615,6 +615,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.
/// 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, 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 @@ -35,7 +35,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 @@ -251,6 +251,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
210 changes: 210 additions & 0 deletions clippy_lints/src/legacy_numeric_constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
use clippy_config::msrvs::{Msrv, NUMERIC_ASSOCIATED_CONSTANTS};
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
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, SuggestionStyle};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{ExprKind, Item, ItemKind, PrimTy, QPath, TyKind, 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;
}

// These 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 def_path = cx.get_def_path(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 {
diag.help("remove this import");
return;
}

if plurality && let [.., module_name] = &*def_path {
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()
&& 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
{
(
last_segment.ident.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) {
return;
}

span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
if let Some(sugg) = sugg {
diag.span_suggestion_with_style(
span,
"use the associated constant instead",
sugg,
Applicability::MaybeIncorrect,
SuggestionStyle::ShowAlways,
);
} 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,
));
}
});
}

extract_msrv_attr!(LateContext);
}

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 @@ -184,6 +184,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 @@ -1076,6 +1077,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
2 changes: 1 addition & 1 deletion tests/ui-toml/absolute_paths/absolute_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//@revisions: allow_crates disallow_crates
//@[allow_crates] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/absolute_paths/allow_crates
//@[disallow_crates] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/absolute_paths/disallow_crates
#![allow(clippy::no_effect, unused)]
#![allow(clippy::no_effect, clippy::legacy_numeric_constants, unused)]
#![warn(clippy::absolute_paths)]
#![feature(decl_macro)]

Expand Down
1 change: 1 addition & 0 deletions tests/ui/checked_conversions.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![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
@@ -1,5 +1,6 @@
#![allow(
clippy::cast_lossless,
clippy::legacy_numeric_constants,
unused,
// Int::max_value will be deprecated in the future
deprecated,
Expand Down
Loading

0 comments on commit c5edb8e

Please sign in to comment.