From e30313cc6b140e0a7f88e85bf9ec9440bfb4c82e Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Tue, 17 Jun 2025 17:21:44 -0700 Subject: [PATCH 1/5] fix `legacy_numeric_constants` suggestion when call is wrapped in parens --- clippy_lints/src/legacy_numeric_constants.rs | 8 +- tests/ui/legacy_numeric_constants.fixed | 15 ++++ tests/ui/legacy_numeric_constants.rs | 15 ++++ tests/ui/legacy_numeric_constants.stderr | 86 +++++++++++++++++--- 4 files changed, 107 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/legacy_numeric_constants.rs b/clippy_lints/src/legacy_numeric_constants.rs index b3c63f022d35..6a81db73edf0 100644 --- a/clippy_lints/src/legacy_numeric_constants.rs +++ b/clippy_lints/src/legacy_numeric_constants.rs @@ -122,17 +122,17 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { "usage of a legacy numeric constant", ) // `::xxx_value` check - } else if let QPath::TypeRelative(_, last_segment) = qpath + } else if let QPath::TypeRelative(mod_path, last_segment) = qpath && let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id() && let Some(par_expr) = get_parent_expr(cx, expr) && let ExprKind::Call(_, []) = par_expr.kind && is_integer_method(cx, def_id) { let name = last_segment.ident.name.as_str(); - + let mod_name = clippy_utils::source::snippet(cx, mod_path.span, "_"); ( - last_segment.ident.span.with_hi(par_expr.span.hi()), - name[..=2].to_ascii_uppercase(), + par_expr.span, + format!("{}::{}", mod_name, name[..=2].to_ascii_uppercase()), "usage of a legacy numeric method", ) } else { diff --git a/tests/ui/legacy_numeric_constants.fixed b/tests/ui/legacy_numeric_constants.fixed index 30bb549a9d65..5f3c3308de88 100644 --- a/tests/ui/legacy_numeric_constants.fixed +++ b/tests/ui/legacy_numeric_constants.fixed @@ -79,9 +79,24 @@ fn main() { f64::consts::E; b!(); + std::primitive::i32::MAX; + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead [(0, "", i128::MAX)]; //~^ ERROR: usage of a legacy numeric constant //~| HELP: use the associated constant instead + i32::MAX; + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + assert_eq!(0, -i32::MAX); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + i128::MAX; + //~^ ERROR: usage of a legacy numeric constant + //~| HELP: use the associated constant instead + u32::MAX; + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead } #[warn(clippy::legacy_numeric_constants)] diff --git a/tests/ui/legacy_numeric_constants.rs b/tests/ui/legacy_numeric_constants.rs index d3878199055f..0bb27532b41d 100644 --- a/tests/ui/legacy_numeric_constants.rs +++ b/tests/ui/legacy_numeric_constants.rs @@ -79,9 +79,24 @@ fn main() { f64::consts::E; b!(); + ::max_value(); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead [(0, "", std::i128::MAX)]; //~^ ERROR: usage of a legacy numeric constant //~| HELP: use the associated constant instead + (i32::max_value()); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + assert_eq!(0, -(i32::max_value())); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + (std::i128::MAX); + //~^ ERROR: usage of a legacy numeric constant + //~| HELP: use the associated constant instead + (::max_value()); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead } #[warn(clippy::legacy_numeric_constants)] diff --git a/tests/ui/legacy_numeric_constants.stderr b/tests/ui/legacy_numeric_constants.stderr index 4d69b8165a34..41e94fc8aaec 100644 --- a/tests/ui/legacy_numeric_constants.stderr +++ b/tests/ui/legacy_numeric_constants.stderr @@ -72,10 +72,10 @@ LL | u32::MAX; | +++++ error: usage of a legacy numeric method - --> tests/ui/legacy_numeric_constants.rs:50:10 + --> tests/ui/legacy_numeric_constants.rs:50:5 | LL | i32::max_value(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ | help: use the associated constant instead | @@ -84,10 +84,10 @@ LL + i32::MAX; | error: usage of a legacy numeric method - --> tests/ui/legacy_numeric_constants.rs:53:9 + --> tests/ui/legacy_numeric_constants.rs:53:5 | LL | u8::max_value(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | help: use the associated constant instead | @@ -96,10 +96,10 @@ LL + u8::MAX; | error: usage of a legacy numeric method - --> tests/ui/legacy_numeric_constants.rs:56:9 + --> tests/ui/legacy_numeric_constants.rs:56:5 | LL | u8::min_value(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | help: use the associated constant instead | @@ -120,10 +120,10 @@ LL + u8::MIN; | error: usage of a legacy numeric method - --> tests/ui/legacy_numeric_constants.rs:62:27 + --> tests/ui/legacy_numeric_constants.rs:62:5 | LL | ::std::primitive::u8::min_value(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: use the associated constant instead | @@ -132,10 +132,10 @@ LL + ::std::primitive::u8::MIN; | error: usage of a legacy numeric method - --> tests/ui/legacy_numeric_constants.rs:65:26 + --> tests/ui/legacy_numeric_constants.rs:65:5 | LL | std::primitive::i32::max_value(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: use the associated constant instead | @@ -171,8 +171,20 @@ LL - let x = std::u64::MAX; LL + let x = u64::MAX; | +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:82:5 + | +LL | ::max_value(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - ::max_value(); +LL + std::primitive::i32::MAX; + | + error: usage of a legacy numeric constant - --> tests/ui/legacy_numeric_constants.rs:82:14 + --> tests/ui/legacy_numeric_constants.rs:85:14 | LL | [(0, "", std::i128::MAX)]; | ^^^^^^^^^^^^^^ @@ -183,8 +195,56 @@ LL - [(0, "", std::i128::MAX)]; LL + [(0, "", i128::MAX)]; | +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:88:5 + | +LL | (i32::max_value()); + | ^^^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - (i32::max_value()); +LL + i32::MAX; + | + +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:91:20 + | +LL | assert_eq!(0, -(i32::max_value())); + | ^^^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - assert_eq!(0, -(i32::max_value())); +LL + assert_eq!(0, -i32::MAX); + | + +error: usage of a legacy numeric constant + --> tests/ui/legacy_numeric_constants.rs:94:5 + | +LL | (std::i128::MAX); + | ^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - (std::i128::MAX); +LL + i128::MAX; + | + +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:97:5 + | +LL | (::max_value()); + | ^^^^^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - (::max_value()); +LL + u32::MAX; + | + error: usage of a legacy numeric constant - --> tests/ui/legacy_numeric_constants.rs:116:5 + --> tests/ui/legacy_numeric_constants.rs:131:5 | LL | std::u32::MAX; | ^^^^^^^^^^^^^ @@ -195,5 +255,5 @@ LL - std::u32::MAX; LL + u32::MAX; | -error: aborting due to 16 previous errors +error: aborting due to 21 previous errors From ae6416aa866d455fb636934bf0911095e2c8cdea Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 10 Jul 2025 10:25:32 -0700 Subject: [PATCH 2/5] add extra legacy_numeric_constants test cases --- tests/ui/legacy_numeric_constants.fixed | 7 ++++++ tests/ui/legacy_numeric_constants.rs | 7 ++++++ tests/ui/legacy_numeric_constants.stderr | 28 ++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/tests/ui/legacy_numeric_constants.fixed b/tests/ui/legacy_numeric_constants.fixed index 5f3c3308de88..d90e7bec027c 100644 --- a/tests/ui/legacy_numeric_constants.fixed +++ b/tests/ui/legacy_numeric_constants.fixed @@ -97,6 +97,13 @@ fn main() { u32::MAX; //~^ ERROR: usage of a legacy numeric method //~| HELP: use the associated constant instead + i32::MAX; + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + type Ω = i32; + Ω::MAX; + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead } #[warn(clippy::legacy_numeric_constants)] diff --git a/tests/ui/legacy_numeric_constants.rs b/tests/ui/legacy_numeric_constants.rs index 0bb27532b41d..4a2ef3f70c21 100644 --- a/tests/ui/legacy_numeric_constants.rs +++ b/tests/ui/legacy_numeric_constants.rs @@ -97,6 +97,13 @@ fn main() { (::max_value()); //~^ ERROR: usage of a legacy numeric method //~| HELP: use the associated constant instead + ((i32::max_value)()); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead + type Ω = i32; + Ω::max_value(); + //~^ ERROR: usage of a legacy numeric method + //~| HELP: use the associated constant instead } #[warn(clippy::legacy_numeric_constants)] diff --git a/tests/ui/legacy_numeric_constants.stderr b/tests/ui/legacy_numeric_constants.stderr index 41e94fc8aaec..0b4f32e0abc3 100644 --- a/tests/ui/legacy_numeric_constants.stderr +++ b/tests/ui/legacy_numeric_constants.stderr @@ -243,8 +243,32 @@ LL - (::max_value()); LL + u32::MAX; | +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:100:5 + | +LL | ((i32::max_value)()); + | ^^^^^^^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - ((i32::max_value)()); +LL + i32::MAX; + | + +error: usage of a legacy numeric method + --> tests/ui/legacy_numeric_constants.rs:104:5 + | +LL | Ω::max_value(); + | ^^^^^^^^^^^^^^ + | +help: use the associated constant instead + | +LL - Ω::max_value(); +LL + Ω::MAX; + | + error: usage of a legacy numeric constant - --> tests/ui/legacy_numeric_constants.rs:131:5 + --> tests/ui/legacy_numeric_constants.rs:138:5 | LL | std::u32::MAX; | ^^^^^^^^^^^^^ @@ -255,5 +279,5 @@ LL - std::u32::MAX; LL + u32::MAX; | -error: aborting due to 21 previous errors +error: aborting due to 23 previous errors From 88ee494b385d17e12aa60f2dae307b643ebcf314 Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Thu, 10 Jul 2025 10:44:11 -0700 Subject: [PATCH 3/5] cleanup legacy_numeric_constants Removes an unnecessary use of `span_lint_hir_and_then()` where `span_lint_and_then()` would do. --- clippy_lints/src/legacy_numeric_constants.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/legacy_numeric_constants.rs b/clippy_lints/src/legacy_numeric_constants.rs index 6a81db73edf0..0768d037a738 100644 --- a/clippy_lints/src/legacy_numeric_constants.rs +++ b/clippy_lints/src/legacy_numeric_constants.rs @@ -1,5 +1,5 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::{get_parent_expr, is_from_proc_macro}; use hir::def_id::DefId; @@ -143,7 +143,7 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { && self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS) && !is_from_proc_macro(cx, expr) { - span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| { + span_lint_and_then(cx, LEGACY_NUMERIC_CONSTANTS, span, msg, |diag| { diag.span_suggestion_verbose( span, "use the associated constant instead", From 37ddd9f32dcd8bfff731f8bb148f2efca01a857c Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 10 Jul 2025 11:18:29 -0700 Subject: [PATCH 4/5] refactor legacy_numeric_constants to check calls instead of paths This also moves the lint to be posted on the call. --- clippy_lints/src/legacy_numeric_constants.rs | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/legacy_numeric_constants.rs b/clippy_lints/src/legacy_numeric_constants.rs index 0768d037a738..b9a235ecb8b7 100644 --- a/clippy_lints/src/legacy_numeric_constants.rs +++ b/clippy_lints/src/legacy_numeric_constants.rs @@ -1,7 +1,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::{get_parent_expr, is_from_proc_macro}; use hir::def_id::DefId; use rustc_errors::Applicability; use rustc_hir as hir; @@ -102,16 +102,12 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - let ExprKind::Path(qpath) = &expr.kind else { - return; - }; - // `std::::` check - let (span, sugg, msg) = if let QPath::Resolved(None, path) = qpath + let (span, sugg, msg) = if let ExprKind::Path(qpath) = &expr.kind + && 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 + && let [.., mod_name, name] = &*cx.get_def_path(def_id) // 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]) @@ -122,16 +118,16 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { "usage of a legacy numeric constant", ) // `::xxx_value` check - } else if let QPath::TypeRelative(mod_path, last_segment) = qpath - && let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id() - && let Some(par_expr) = get_parent_expr(cx, expr) - && let ExprKind::Call(_, []) = par_expr.kind + } else if let ExprKind::Call(func, []) = &expr.kind + && let ExprKind::Path(qpath) = &func.kind + && let QPath::TypeRelative(ty, last_segment) = qpath + && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() && is_integer_method(cx, def_id) { let name = last_segment.ident.name.as_str(); - let mod_name = clippy_utils::source::snippet(cx, mod_path.span, "_"); + let mod_name = clippy_utils::source::snippet(cx, ty.span, "_"); ( - par_expr.span, + expr.span, format!("{}::{}", mod_name, name[..=2].to_ascii_uppercase()), "usage of a legacy numeric method", ) From 9457d640b74fc5b1c0c5257658160dcd726bea72 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 10 Jul 2025 11:48:15 -0700 Subject: [PATCH 5/5] refactor legacy_numeric_constants to use multipart suggestion This removes the need for using source snippets in the replacement. --- clippy_lints/src/legacy_numeric_constants.rs | 36 +++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/legacy_numeric_constants.rs b/clippy_lints/src/legacy_numeric_constants.rs index b9a235ecb8b7..42c636505c01 100644 --- a/clippy_lints/src/legacy_numeric_constants.rs +++ b/clippy_lints/src/legacy_numeric_constants.rs @@ -2,6 +2,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::source::SpanRangeExt; use hir::def_id::DefId; use rustc_errors::Applicability; use rustc_hir as hir; @@ -103,7 +104,7 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { // `std::::` check - let (span, sugg, msg) = if let ExprKind::Path(qpath) = &expr.kind + let (sugg, msg) = if let ExprKind::Path(qpath) = &expr.kind && let QPath::Resolved(None, path) = qpath && let Some(def_id) = path.res.opt_def_id() && is_numeric_const(cx, def_id) @@ -113,8 +114,7 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { && !is_numeric_const_path_canonical(path, [*mod_name, *name]) { ( - expr.span, - format!("{mod_name}::{name}"), + vec![(expr.span, format!("{mod_name}::{name}"))], "usage of a legacy numeric constant", ) // `::xxx_value` check @@ -124,13 +124,24 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() && is_integer_method(cx, def_id) { - let name = last_segment.ident.name.as_str(); - let mod_name = clippy_utils::source::snippet(cx, ty.span, "_"); - ( - expr.span, - format!("{}::{}", mod_name, name[..=2].to_ascii_uppercase()), - "usage of a legacy numeric method", - ) + let mut sugg = vec![ + // Replace the function name up to the end by the constant name + ( + last_segment.ident.span.to(expr.span.shrink_to_hi()), + last_segment.ident.name.as_str()[..=2].to_ascii_uppercase(), + ), + ]; + let before_span = expr.span.shrink_to_lo().until(ty.span); + if !before_span.is_empty() { + // Remove everything before the type name + sugg.push((before_span, String::new())); + } + // Use `::` between the type name and the constant + let between_span = ty.span.shrink_to_hi().until(last_segment.ident.span); + if !between_span.check_source_text(cx, |s| s == "::") { + sugg.push((between_span, String::from("::"))); + } + (sugg, "usage of a legacy numeric method") } else { return; }; @@ -139,9 +150,8 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants { && self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS) && !is_from_proc_macro(cx, expr) { - span_lint_and_then(cx, LEGACY_NUMERIC_CONSTANTS, span, msg, |diag| { - diag.span_suggestion_verbose( - span, + span_lint_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.span, msg, |diag| { + diag.multipart_suggestion_verbose( "use the associated constant instead", sugg, Applicability::MaybeIncorrect,