Skip to content

Commit 488f4dd

Browse files
samueltardieusamueltardieu
authored
fix legacy_numeric_constants suggestion when call is wrapped in parens (#15191)
Fixes #15008 changelog: [`legacy_numeric_constants`]: fix suggestion when call is inside parentheses like `(i32::max_value())`
2 parents b5e701e + 9457d64 commit 488f4dd

File tree

4 files changed

+172
-38
lines changed

4 files changed

+172
-38
lines changed

‎clippy_lints/src/legacy_numeric_constants.rs

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use clippy_config::Conf;
2-
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::is_from_proc_macro;
34
use clippy_utils::msrvs::{self, Msrv};
4-
use clippy_utils::{get_parent_expr, is_from_proc_macro};
5+
use clippy_utils::source::SpanRangeExt;
56
use hir::def_id::DefId;
67
use rustc_errors::Applicability;
78
use rustc_hir as hir;
@@ -102,39 +103,45 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
102103
}
103104

104105
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
105-
let ExprKind::Path(qpath) = &expr.kind else {
106-
return;
107-
};
108-
109106
// `std::<integer>::<CONST>` check
110-
let (span, sugg, msg) = if let QPath::Resolved(None, path) = qpath
107+
let (sugg, msg) = if let ExprKind::Path(qpath) = &expr.kind
108+
&& let QPath::Resolved(None, path) = qpath
111109
&& let Some(def_id) = path.res.opt_def_id()
112110
&& is_numeric_const(cx, def_id)
113-
&& let def_path = cx.get_def_path(def_id)
114-
&& let [.., mod_name, name] = &*def_path
111+
&& let [.., mod_name, name] = &*cx.get_def_path(def_id)
115112
// Skip linting if this usage looks identical to the associated constant,
116113
// since this would only require removing a `use` import (which is already linted).
117114
&& !is_numeric_const_path_canonical(path, [*mod_name, *name])
118115
{
119116
(
120-
expr.span,
121-
format!("{mod_name}::{name}"),
117+
vec![(expr.span, format!("{mod_name}::{name}"))],
122118
"usage of a legacy numeric constant",
123119
)
124120
// `<integer>::xxx_value` check
125-
} else if let QPath::TypeRelative(_, last_segment) = qpath
126-
&& let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id()
127-
&& let Some(par_expr) = get_parent_expr(cx, expr)
128-
&& let ExprKind::Call(_, []) = par_expr.kind
121+
} else if let ExprKind::Call(func, []) = &expr.kind
122+
&& let ExprKind::Path(qpath) = &func.kind
123+
&& let QPath::TypeRelative(ty, last_segment) = qpath
124+
&& let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
129125
&& is_integer_method(cx, def_id)
130126
{
131-
let name = last_segment.ident.name.as_str();
132-
133-
(
134-
last_segment.ident.span.with_hi(par_expr.span.hi()),
135-
name[..=2].to_ascii_uppercase(),
136-
"usage of a legacy numeric method",
137-
)
127+
let mut sugg = vec![
128+
// Replace the function name up to the end by the constant name
129+
(
130+
last_segment.ident.span.to(expr.span.shrink_to_hi()),
131+
last_segment.ident.name.as_str()[..=2].to_ascii_uppercase(),
132+
),
133+
];
134+
let before_span = expr.span.shrink_to_lo().until(ty.span);
135+
if !before_span.is_empty() {
136+
// Remove everything before the type name
137+
sugg.push((before_span, String::new()));
138+
}
139+
// Use `::` between the type name and the constant
140+
let between_span = ty.span.shrink_to_hi().until(last_segment.ident.span);
141+
if !between_span.check_source_text(cx, |s| s == "::") {
142+
sugg.push((between_span, String::from("::")));
143+
}
144+
(sugg, "usage of a legacy numeric method")
138145
} else {
139146
return;
140147
};
@@ -143,9 +150,8 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
143150
&& self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS)
144151
&& !is_from_proc_macro(cx, expr)
145152
{
146-
span_lint_hir_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.hir_id, span, msg, |diag| {
147-
diag.span_suggestion_verbose(
148-
span,
153+
span_lint_and_then(cx, LEGACY_NUMERIC_CONSTANTS, expr.span, msg, |diag| {
154+
diag.multipart_suggestion_verbose(
149155
"use the associated constant instead",
150156
sugg,
151157
Applicability::MaybeIncorrect,

‎tests/ui/legacy_numeric_constants.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,31 @@ fn main() {
7979
f64::consts::E;
8080
b!();
8181

82+
std::primitive::i32::MAX;
83+
//~^ ERROR: usage of a legacy numeric method
84+
//~| HELP: use the associated constant instead
8285
[(0, "", i128::MAX)];
8386
//~^ ERROR: usage of a legacy numeric constant
8487
//~| HELP: use the associated constant instead
88+
i32::MAX;
89+
//~^ ERROR: usage of a legacy numeric method
90+
//~| HELP: use the associated constant instead
91+
assert_eq!(0, -i32::MAX);
92+
//~^ ERROR: usage of a legacy numeric method
93+
//~| HELP: use the associated constant instead
94+
i128::MAX;
95+
//~^ ERROR: usage of a legacy numeric constant
96+
//~| HELP: use the associated constant instead
97+
u32::MAX;
98+
//~^ ERROR: usage of a legacy numeric method
99+
//~| HELP: use the associated constant instead
100+
i32::MAX;
101+
//~^ ERROR: usage of a legacy numeric method
102+
//~| HELP: use the associated constant instead
103+
type Ω = i32;
104+
Ω::MAX;
105+
//~^ ERROR: usage of a legacy numeric method
106+
//~| HELP: use the associated constant instead
85107
}
86108

87109
#[warn(clippy::legacy_numeric_constants)]

‎tests/ui/legacy_numeric_constants.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,31 @@ fn main() {
7979
f64::consts::E;
8080
b!();
8181

82+
<std::primitive::i32>::max_value();
83+
//~^ ERROR: usage of a legacy numeric method
84+
//~| HELP: use the associated constant instead
8285
[(0, "", std::i128::MAX)];
8386
//~^ ERROR: usage of a legacy numeric constant
8487
//~| HELP: use the associated constant instead
88+
(i32::max_value());
89+
//~^ ERROR: usage of a legacy numeric method
90+
//~| HELP: use the associated constant instead
91+
assert_eq!(0, -(i32::max_value()));
92+
//~^ ERROR: usage of a legacy numeric method
93+
//~| HELP: use the associated constant instead
94+
(std::i128::MAX);
95+
//~^ ERROR: usage of a legacy numeric constant
96+
//~| HELP: use the associated constant instead
97+
(<u32>::max_value());
98+
//~^ ERROR: usage of a legacy numeric method
99+
//~| HELP: use the associated constant instead
100+
((i32::max_value)());
101+
//~^ ERROR: usage of a legacy numeric method
102+
//~| HELP: use the associated constant instead
103+
type Ω = i32;
104+
Ω::max_value();
105+
//~^ ERROR: usage of a legacy numeric method
106+
//~| HELP: use the associated constant instead
85107
}
86108

87109
#[warn(clippy::legacy_numeric_constants)]

‎tests/ui/legacy_numeric_constants.stderr

Lines changed: 97 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ LL | u32::MAX;
7272
| +++++
7373

7474
error: usage of a legacy numeric method
75-
--> tests/ui/legacy_numeric_constants.rs:50:10
75+
--> tests/ui/legacy_numeric_constants.rs:50:5
7676
|
7777
LL | i32::max_value();
78-
| ^^^^^^^^^^^
78+
| ^^^^^^^^^^^^^^^^
7979
|
8080
help: use the associated constant instead
8181
|
@@ -84,10 +84,10 @@ LL + i32::MAX;
8484
|
8585

8686
error: usage of a legacy numeric method
87-
--> tests/ui/legacy_numeric_constants.rs:53:9
87+
--> tests/ui/legacy_numeric_constants.rs:53:5
8888
|
8989
LL | u8::max_value();
90-
| ^^^^^^^^^^^
90+
| ^^^^^^^^^^^^^^^
9191
|
9292
help: use the associated constant instead
9393
|
@@ -96,10 +96,10 @@ LL + u8::MAX;
9696
|
9797

9898
error: usage of a legacy numeric method
99-
--> tests/ui/legacy_numeric_constants.rs:56:9
99+
--> tests/ui/legacy_numeric_constants.rs:56:5
100100
|
101101
LL | u8::min_value();
102-
| ^^^^^^^^^^^
102+
| ^^^^^^^^^^^^^^^
103103
|
104104
help: use the associated constant instead
105105
|
@@ -120,10 +120,10 @@ LL + u8::MIN;
120120
|
121121

122122
error: usage of a legacy numeric method
123-
--> tests/ui/legacy_numeric_constants.rs:62:27
123+
--> tests/ui/legacy_numeric_constants.rs:62:5
124124
|
125125
LL | ::std::primitive::u8::min_value();
126-
| ^^^^^^^^^^^
126+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
127127
|
128128
help: use the associated constant instead
129129
|
@@ -132,10 +132,10 @@ LL + ::std::primitive::u8::MIN;
132132
|
133133

134134
error: usage of a legacy numeric method
135-
--> tests/ui/legacy_numeric_constants.rs:65:26
135+
--> tests/ui/legacy_numeric_constants.rs:65:5
136136
|
137137
LL | std::primitive::i32::max_value();
138-
| ^^^^^^^^^^^
138+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
139139
|
140140
help: use the associated constant instead
141141
|
@@ -171,8 +171,20 @@ LL - let x = std::u64::MAX;
171171
LL + let x = u64::MAX;
172172
|
173173

174+
error: usage of a legacy numeric method
175+
--> tests/ui/legacy_numeric_constants.rs:82:5
176+
|
177+
LL | <std::primitive::i32>::max_value();
178+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
179+
|
180+
help: use the associated constant instead
181+
|
182+
LL - <std::primitive::i32>::max_value();
183+
LL + std::primitive::i32::MAX;
184+
|
185+
174186
error: usage of a legacy numeric constant
175-
--> tests/ui/legacy_numeric_constants.rs:82:14
187+
--> tests/ui/legacy_numeric_constants.rs:85:14
176188
|
177189
LL | [(0, "", std::i128::MAX)];
178190
| ^^^^^^^^^^^^^^
@@ -183,8 +195,80 @@ LL - [(0, "", std::i128::MAX)];
183195
LL + [(0, "", i128::MAX)];
184196
|
185197

198+
error: usage of a legacy numeric method
199+
--> tests/ui/legacy_numeric_constants.rs:88:5
200+
|
201+
LL | (i32::max_value());
202+
| ^^^^^^^^^^^^^^^^^^
203+
|
204+
help: use the associated constant instead
205+
|
206+
LL - (i32::max_value());
207+
LL + i32::MAX;
208+
|
209+
210+
error: usage of a legacy numeric method
211+
--> tests/ui/legacy_numeric_constants.rs:91:20
212+
|
213+
LL | assert_eq!(0, -(i32::max_value()));
214+
| ^^^^^^^^^^^^^^^^^^
215+
|
216+
help: use the associated constant instead
217+
|
218+
LL - assert_eq!(0, -(i32::max_value()));
219+
LL + assert_eq!(0, -i32::MAX);
220+
|
221+
222+
error: usage of a legacy numeric constant
223+
--> tests/ui/legacy_numeric_constants.rs:94:5
224+
|
225+
LL | (std::i128::MAX);
226+
| ^^^^^^^^^^^^^^^^
227+
|
228+
help: use the associated constant instead
229+
|
230+
LL - (std::i128::MAX);
231+
LL + i128::MAX;
232+
|
233+
234+
error: usage of a legacy numeric method
235+
--> tests/ui/legacy_numeric_constants.rs:97:5
236+
|
237+
LL | (<u32>::max_value());
238+
| ^^^^^^^^^^^^^^^^^^^^
239+
|
240+
help: use the associated constant instead
241+
|
242+
LL - (<u32>::max_value());
243+
LL + u32::MAX;
244+
|
245+
246+
error: usage of a legacy numeric method
247+
--> tests/ui/legacy_numeric_constants.rs:100:5
248+
|
249+
LL | ((i32::max_value)());
250+
| ^^^^^^^^^^^^^^^^^^^^
251+
|
252+
help: use the associated constant instead
253+
|
254+
LL - ((i32::max_value)());
255+
LL + i32::MAX;
256+
|
257+
258+
error: usage of a legacy numeric method
259+
--> tests/ui/legacy_numeric_constants.rs:104:5
260+
|
261+
LL | Ω::max_value();
262+
| ^^^^^^^^^^^^^^
263+
|
264+
help: use the associated constant instead
265+
|
266+
LL - Ω::max_value();
267+
LL + Ω::MAX;
268+
|
269+
186270
error: usage of a legacy numeric constant
187-
--> tests/ui/legacy_numeric_constants.rs:116:5
271+
--> tests/ui/legacy_numeric_constants.rs:138:5
188272
|
189273
LL | std::u32::MAX;
190274
| ^^^^^^^^^^^^^
@@ -195,5 +279,5 @@ LL - std::u32::MAX;
195279
LL + u32::MAX;
196280
|
197281

198-
error: aborting due to 16 previous errors
282+
error: aborting due to 23 previous errors
199283

0 commit comments

Comments
 (0)