Refine stderr

Author: Centri3

book/src/lint_configuration.md

@@ -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`

clippy_lints/src/legacy_numeric_constants.rs

@@ -1,22 +1,17 @@
-use clippy_utils::{
-    diagnostics::span_lint_and_then,
-    get_parent_expr, is_from_proc_macro, last_path_segment,
-    msrvs::{self, Msrv},
-};
-use itertools::Itertools;
-use rustc_data_structures::fx::FxHashMap;
+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::Res,
-    def_id::DefId,
-    intravisit::{walk_expr, Visitor},
-    Item, UseKind,
-};
-use rustc_hir::{Expr, ExprKind, ItemKind, PrimTy, QPath, TyKind};
+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, lint::in_external_macro};
+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::{sym, Span, Symbol};
+use rustc_span::symbol::kw;
+use rustc_span::{sym, Symbol};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -25,7 +20,7 @@ declare_clippy_lint! {
     ///
     /// ### Why is this bad?
     /// All of these have been superceded by the associated constants on their respective types,
-    /// such as `i128::MAX`. These legacy constants may be deprecated in a future version of rust.
+    /// such as `i128::MAX`. These legacy items may be deprecated in a future version of rust.
     ///
     /// ### Example
     /// ```rust
@@ -42,64 +37,64 @@ declare_clippy_lint! {
 }
 pub struct LegacyNumericConstants {
     msrv: Msrv,
-    use_stmts: FxHashMap<Symbol, Vec<Span>>,
-    glob_use_stmts: Vec<Span>,
 }
 
 impl LegacyNumericConstants {
     #[must_use]
     pub fn new(msrv: Msrv) -> Self {
-        Self {
-            msrv,
-            use_stmts: FxHashMap::default(),
-            glob_use_stmts: vec![],
-        }
+        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: _,
-            use_stmts,
-            glob_use_stmts,
-        } = self;
-
-        if !item.span.is_dummy() && let ItemKind::Use(path, kind) = item.kind {
-            match kind {
-                UseKind::Single => {
-                    for res in &path.res {
-                        if let Some(def_id) = res.opt_def_id()
-                            && let Some(module_name) = is_path_in_integral_module(cx, def_id)
-                            && let _ = use_stmts.insert(module_name, vec![])
-                            && let Some(use_stmts) = use_stmts.get_mut(&module_name)
-                        {
-                            use_stmts.push(item.span);
-                        }
+        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);
                     }
                 },
-                UseKind::Glob => glob_use_stmts.push(item.span),
-                UseKind::ListStem => {},
-            }
+            );
         }
     }
 
     fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
-        let Self {
-            msrv,
-            use_stmts,
-            glob_use_stmts,
-        } = self;
-
-        let mut v = V {
-            cx,
-            msrv,
-            use_stmts,
-            glob_use_stmts,
-        };
-        cx.tcx.hir().visit_all_item_likes_in_crate(&mut v);
+        let Self { msrv } = self;
+
+        cx.tcx.hir().visit_all_item_likes_in_crate(&mut V { cx, msrv });
     }
 
     extract_msrv_attr!(LateContext);
@@ -108,8 +103,6 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
 struct V<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     msrv: &'a Msrv,
-    use_stmts: &'a FxHashMap<Symbol, Vec<Span>>,
-    glob_use_stmts: &'a Vec<Span>,
 }
 
 impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
@@ -121,35 +114,36 @@ impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
 
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
         walk_expr(self, expr);
-        let Self {
-            cx,
-            msrv,
-            use_stmts,
-            glob_use_stmts,
-        } = *self;
-
-        if !msrv.meets(msrvs::STD_INTEGRAL_CONSTANTS) || in_external_macro(cx.sess(), expr.span) {
+        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, is_method, use_stmts) = if let QPath::Resolved(_, path) = qpath
+        let (span, sugg, msg) = if let QPath::Resolved(None, path) = qpath
             && let Some(def_id) = path.res.opt_def_id()
-            && let Some(name) = path.segments.iter().last().map(|segment| segment.ident.name)
-            && let Some(module_name) = is_path_in_integral_module(cx, 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,
-                format!("{module_name}::{name}"),
-                false,
-                if path.segments.get(0).is_some_and(|segment| segment.ident.name == module_name) {
-                    use_stmts.get(&module_name)
-                } else {
+                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
@@ -164,43 +158,40 @@ impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> {
         {
             (
                 qpath.last_segment_span().with_hi(par_expr.span.hi()),
-                name[..=2].to_ascii_uppercase(),
-                true,
-                None,
+                Some(name[..=2].to_ascii_uppercase()),
+                "usage of a legacy numeric method",
             )
         } else {
             return;
         };
 
         if !is_from_proc_macro(cx, expr) {
-            let msg = if is_method {
-                "usage of a legacy numeric method"
-            } else {
-                "usage of a legacy numeric constant"
-            };
-
-            span_lint_and_then(cx, LEGACY_NUMERIC_CONSTANTS, span, msg, |diag| {
-                let app = if use_stmts.is_none() {
-                    Applicability::MachineApplicable
-                } else {
-                    Applicability::MaybeIncorrect
-                };
-                diag.span_suggestion(span, "use the associated constant instead", sugg, app);
-                if let Some(use_stmts) = use_stmts {
-                    diag.span_note(
-                        use_stmts.iter().chain(glob_use_stmts).copied().collect_vec(),
-                        "you may need to remove one of the following `use` statements",
+            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_integral_module(cx: &LateContext<'_>, def_id: DefId) -> Option<Symbol> {
-    let path = cx.get_def_path(def_id);
+fn is_path_in_numeric_module(path: &[Symbol], ignore_float_modules: bool) -> bool {
     if let [
-        sym::core | sym::std,
+        sym::core,
         module @ (sym::u8
         | sym::i8
         | sym::u16
@@ -216,12 +207,17 @@ fn is_path_in_integral_module(cx: &LateContext<'_>, def_id: DefId) -> Option<Sym
         | sym::f32
         | sym::f64),
         ..,
-    ] = &*cx.get_def_path(def_id)
-        // So `use` statements like `std::f32` also work
-        && path.len() <= 3
+    ] = path
+        && !path.get(2).is_some_and(|&s| s == sym!(consts))
     {
-        return Some(*module);
+        // 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;
     }
 
-    None
+    false
 }

clippy_lints/src/lib.rs

@@ -1080,7 +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_integral_constants::LegacyIntegralConstants::new(msrv())));
+    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() }));

clippy_lints/src/utils/conf.rs

@@ -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),

clippy_utils/src/check_proc_macro.rs

@@ -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;
 
@@ -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() {

clippy_utils/src/msrvs.rs

@@ -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, STD_INTEGRAL_CONSTANTS }
+    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 }

tests/ui/legacy_numeric_constants.fixed

@@ -2,12 +2,14 @@
 //@aux-build:proc_macros.rs:proc-macro
 #![allow(clippy::no_effect, deprecated, unused)]
 #![warn(clippy::legacy_numeric_constants)]
+#![feature(lint_reasons)]
 
 #[macro_use]
 extern crate proc_macros;
 
 use std::u128 as _;
 pub mod a {
+    #![expect(clippy::legacy_numeric_constants)]
     pub use std::u128;
 }
 
@@ -17,6 +19,7 @@ fn main() {
     usize::MIN;
     u32::MAX;
     u32::MAX;
+    #![expect(clippy::legacy_numeric_constants)]
     use std::u32::MAX;
     u32::MAX;
     u32::MAX;