Skip to content

Commit 42a298b

Browse files
committed
parsed_string_literals: new lint
This lint detects parsing of string literals into primitive types or IP addresses when they are known correct.
1 parent 3da4c10 commit 42a298b

File tree

9 files changed

+498
-1
lines changed

9 files changed

+498
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6155,6 +6155,7 @@ Released 2018-09-13
61556155
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
61566156
[`panicking_overflow_checks`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_overflow_checks
61576157
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
6158+
[`parsed_string_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#parsed_string_literals
61586159
[`partial_pub_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#partial_pub_fields
61596160
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
61606161
[`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
437437
crate::methods::OPTION_MAP_OR_NONE_INFO,
438438
crate::methods::OR_FUN_CALL_INFO,
439439
crate::methods::OR_THEN_UNWRAP_INFO,
440+
crate::methods::PARSED_STRING_LITERALS_INFO,
440441
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
441442
crate::methods::PATH_ENDS_WITH_EXT_INFO,
442443
crate::methods::RANGE_ZIP_WITH_LEN_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
#![feature(array_windows)]
22
#![feature(box_patterns)]
3+
#![feature(cow_is_borrowed)]
34
#![feature(macro_metavar_expr_concat)]
45
#![feature(f128)]
56
#![feature(f16)]
67
#![feature(if_let_guard)]
8+
#![feature(ip_as_octets)]
79
#![feature(iter_intersperse)]
810
#![feature(iter_partition_in_place)]
911
#![feature(never_type)]

clippy_lints/src/methods/mod.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ mod option_map_or_none;
8888
mod option_map_unwrap_or;
8989
mod or_fun_call;
9090
mod or_then_unwrap;
91+
mod parsed_string_literals;
9192
mod path_buf_push_overwrite;
9293
mod path_ends_with_ext;
9394
mod range_zip_with_len;
@@ -4528,6 +4529,36 @@ declare_clippy_lint! {
45284529
"detect swap with a temporary value"
45294530
}
45304531

4532+
declare_clippy_lint! {
4533+
/// ### What it does
4534+
/// Checks for parsing string literals into types from the standard library
4535+
///
4536+
/// ### Why is this bad?
4537+
/// Parsing known values at runtime consumes resources and forces to
4538+
/// unwrap the `Ok()` variant returned by `parse()`.
4539+
///
4540+
/// ### Example
4541+
/// ```no_run
4542+
/// use std::net::Ipv4Addr;
4543+
///
4544+
/// let number = "123".parse::<u32>().unwrap();
4545+
/// let addr1: Ipv4Addr = "10.2.3.4".parse().unwrap();
4546+
/// let addr2: Ipv4Addr = "127.0.0.1".parse().unwrap();
4547+
/// ```
4548+
/// Use instead:
4549+
/// ```no_run
4550+
/// use std::net::Ipv4Addr;
4551+
///
4552+
/// let number = 123_u32;
4553+
/// let addr1: Ipv4Addr = Ipv4Addr::new(10, 2, 3, 4);
4554+
/// let addr2: Ipv4Addr = Ipv4Addr::LOCALHOST;
4555+
/// ```
4556+
#[clippy::version = "1.89.0"]
4557+
pub PARSED_STRING_LITERALS,
4558+
perf,
4559+
"known-correct literal IP address parsing"
4560+
}
4561+
45314562
#[expect(clippy::struct_excessive_bools)]
45324563
pub struct Methods {
45334564
avoid_breaking_exported_api: bool,
@@ -4706,6 +4737,7 @@ impl_lint_pass!(Methods => [
47064737
MANUAL_CONTAINS,
47074738
IO_OTHER_ERROR,
47084739
SWAP_WITH_TEMPORARY,
4740+
PARSED_STRING_LITERALS,
47094741
]);
47104742

47114743
/// Extracts a method call name, args, and `Span` of the method name.
@@ -5420,6 +5452,9 @@ impl Methods {
54205452
Some((sym::or, recv, [or_arg], or_span, _)) => {
54215453
or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
54225454
},
5455+
Some((sym::parse, recv, [], _, _)) => {
5456+
parsed_string_literals::check(cx, expr, recv, self.msrv);
5457+
},
54235458
_ => {},
54245459
}
54255460
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
use std::borrow::Cow;
2+
use std::fmt::Display;
3+
use std::net::{Ipv4Addr, Ipv6Addr};
4+
use std::str::FromStr;
5+
6+
use clippy_utils::diagnostics::span_lint_and_sugg;
7+
use clippy_utils::msrvs::{self, Msrv};
8+
use clippy_utils::source::{SpanRangeExt, str_literal_to_char_literal};
9+
use clippy_utils::sym;
10+
use clippy_utils::ty::is_type_diagnostic_item;
11+
use rustc_ast::LitKind;
12+
use rustc_errors::Applicability;
13+
use rustc_hir::{Expr, ExprKind};
14+
use rustc_lint::LateContext;
15+
use rustc_middle::ty::{self, Ty};
16+
use rustc_span::Symbol;
17+
18+
use super::PARSED_STRING_LITERALS;
19+
20+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, msrv: Msrv) {
21+
if let ExprKind::Lit(lit) = recv.kind
22+
&& let LitKind::Str(lit, _) = lit.node
23+
{
24+
let ty = cx.typeck_results().expr_ty(expr);
25+
if !check_primitive(cx, expr, lit, ty, recv) {
26+
check_ipaddr(cx, expr, lit, ty, msrv);
27+
}
28+
}
29+
}
30+
31+
fn check_primitive(cx: &LateContext<'_>, expr: &Expr<'_>, lit: Symbol, ty: Ty<'_>, strlit: &Expr<'_>) -> bool {
32+
macro_rules! number {
33+
($kind:ident, $expr:expr, $msg:expr, [$($subkind:ident => $ty:ident),*$(,)?]$(,)?) => {{
34+
match $expr {
35+
$(ty::$kind::$subkind => (try_parse::<$ty>(lit, Some(stringify!($ty))), $msg),)*
36+
#[allow(unreachable_patterns)]
37+
_ => return false,
38+
}
39+
}};
40+
}
41+
42+
let mut app = Applicability::MachineApplicable;
43+
if let (Some(subst), entity) = match ty.kind() {
44+
ty::Int(int_ty) => number!(IntTy, int_ty, "a signed integer",
45+
[Isize => isize, I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128]),
46+
ty::Uint(uint_ty) => number!(UintTy, uint_ty, "an unsigned integer",
47+
[Usize => usize, U8 => u8, U16 => u16, U32 => u32, U64 => u64, U128 => u128]),
48+
// FIXME: ignore `f16` and `f128` for now as they cannot use the default formatter
49+
ty::Float(float_ty) => number!(FloatTy, float_ty, "a real number",
50+
[F32 => f32, F64 => f64]),
51+
ty::Bool => (try_parse::<bool>(lit, None), "a boolean"),
52+
ty::Char => (str_literal_to_char_literal(cx, strlit, &mut app, false), "a character"),
53+
_ => return false,
54+
} {
55+
maybe_emit_lint(cx, expr, false, entity, subst.into(), app);
56+
}
57+
true
58+
}
59+
60+
fn check_ipaddr(cx: &LateContext<'_>, expr: &Expr<'_>, lit: Symbol, ty: Ty<'_>, msrv: Msrv) {
61+
static IPV4_ENTITY: &str = "an IPv4 address";
62+
static IPV6_ENTITY: &str = "an IPv6 address";
63+
let ipaddr_consts_available = || msrv.meets(cx, msrvs::IPADDR_CONSTANTS);
64+
if is_type_diagnostic_item(cx, ty, sym::Ipv4Addr)
65+
&& let Some(sugg) = ipv4_subst(lit, ipaddr_consts_available())
66+
{
67+
maybe_emit_lint(
68+
cx,
69+
expr,
70+
sugg.is_borrowed(),
71+
IPV4_ENTITY,
72+
sugg,
73+
Applicability::MaybeIncorrect,
74+
);
75+
} else if is_type_diagnostic_item(cx, ty, sym::Ipv6Addr)
76+
&& let Some(sugg) = ipv6_subst(lit, ipaddr_consts_available())
77+
{
78+
maybe_emit_lint(
79+
cx,
80+
expr,
81+
sugg.is_borrowed(),
82+
IPV6_ENTITY,
83+
sugg,
84+
Applicability::MaybeIncorrect,
85+
);
86+
} else if is_type_diagnostic_item(cx, ty, sym::IpAddr) {
87+
let with_consts = ipaddr_consts_available();
88+
if let Some(sugg) = ipv4_subst(lit, with_consts) {
89+
maybe_emit_lint(
90+
cx,
91+
expr,
92+
sugg.is_borrowed(),
93+
IPV4_ENTITY,
94+
format!("IpAddr::V4({sugg})").into(),
95+
Applicability::MaybeIncorrect,
96+
);
97+
} else if let Some(sugg) = ipv6_subst(lit, with_consts) {
98+
maybe_emit_lint(
99+
cx,
100+
expr,
101+
sugg.is_borrowed(),
102+
IPV6_ENTITY,
103+
format!("IpAddr::V6({sugg})").into(),
104+
Applicability::MaybeIncorrect,
105+
);
106+
}
107+
}
108+
}
109+
110+
/// Suggests a replacement if `addr` is a correct IPv4 address
111+
fn ipv4_subst(addr: Symbol, with_consts: bool) -> Option<Cow<'static, str>> {
112+
addr.as_str().parse().ok().map(|ipv4: Ipv4Addr| {
113+
if with_consts && ipv4.as_octets() == &[127, 0, 0, 1] {
114+
"Ipv4Addr::LOCALHOST".into()
115+
} else if with_consts && ipv4.is_broadcast() {
116+
"Ipv4Addr::BROADCAST".into()
117+
} else if with_consts && ipv4.is_unspecified() {
118+
"Ipv4Addr::UNSPECIFIED".into()
119+
} else {
120+
let ipv4 = ipv4.as_octets();
121+
format!("Ipv4Addr::new({}, {}, {}, {})", ipv4[0], ipv4[1], ipv4[2], ipv4[3]).into()
122+
}
123+
})
124+
}
125+
126+
/// Suggests a replacement if `addr` is a correct IPv6 address
127+
fn ipv6_subst(addr: Symbol, with_consts: bool) -> Option<Cow<'static, str>> {
128+
addr.as_str().parse().ok().map(|ipv6: Ipv6Addr| {
129+
if with_consts && ipv6.is_loopback() {
130+
"Ipv6Addr::LOCALHOST".into()
131+
} else if with_consts && ipv6.is_unspecified() {
132+
"Ipv6Addr::UNSPECIFIED".into()
133+
} else {
134+
format!(
135+
"Ipv6Addr::new([{}])",
136+
ipv6.segments()
137+
.map(|n| if n < 2 { n.to_string() } else { format!("{n:#x}") })
138+
.join(", ")
139+
)
140+
.into()
141+
}
142+
})
143+
}
144+
145+
fn try_parse<T: FromStr + Display>(lit: Symbol, suffix: Option<&str>) -> Option<String> {
146+
lit.as_str()
147+
.parse::<T>()
148+
.ok()
149+
.map(|_| suffix.map_or_else(|| lit.to_string(), |suffix| format!("{lit}_{suffix}")))
150+
}
151+
152+
/// Emit the lint if the length of `sugg` is no longer than the original `expr` span, or if `force`
153+
/// is set.
154+
fn maybe_emit_lint(
155+
cx: &LateContext<'_>,
156+
expr: &Expr<'_>,
157+
force: bool,
158+
entity: &str,
159+
sugg: Cow<'_, str>,
160+
applicability: Applicability,
161+
) {
162+
if force || expr.span.check_source_text(cx, |snip| snip.len() >= sugg.len()) {
163+
span_lint_and_sugg(
164+
cx,
165+
PARSED_STRING_LITERALS,
166+
expr.span,
167+
format!("unnecessary runtime parsing of {entity}"),
168+
"use",
169+
sugg.into(),
170+
applicability,
171+
);
172+
}
173+
}

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ msrv_aliases! {
6868
1,33,0 { UNDERSCORE_IMPORTS }
6969
1,32,0 { CONST_IS_POWER_OF_TWO }
7070
1,31,0 { OPTION_REPLACE }
71-
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
71+
1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES, IPADDR_CONSTANTS }
7272
1,29,0 { ITER_FLATTEN }
7373
1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
7474
1,27,0 { ITERATOR_TRY_FOLD }

tests/ui/parsed_string_literals.fixed

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#![warn(clippy::parsed_string_literals)]
2+
3+
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
4+
5+
fn main() {
6+
_ = Ipv4Addr::new(137, 194, 161, 2);
7+
//~^ parsed_string_literals
8+
9+
_ = Ipv4Addr::LOCALHOST;
10+
//~^ parsed_string_literals
11+
12+
_ = Ipv4Addr::BROADCAST;
13+
//~^ parsed_string_literals
14+
15+
_ = Ipv4Addr::UNSPECIFIED;
16+
//~^ parsed_string_literals
17+
18+
// Wrong address family
19+
_ = "::1".parse::<Ipv4Addr>().unwrap();
20+
_ = "127.0.0.1".parse::<Ipv6Addr>().unwrap();
21+
22+
_ = Ipv6Addr::LOCALHOST;
23+
//~^ parsed_string_literals
24+
25+
_ = Ipv6Addr::UNSPECIFIED;
26+
//~^ parsed_string_literals
27+
28+
_ = IpAddr::V6(Ipv6Addr::LOCALHOST);
29+
//~^ parsed_string_literals
30+
31+
_ = IpAddr::V6(Ipv6Addr::UNSPECIFIED);
32+
//~^ parsed_string_literals
33+
34+
// The substition text would be larger than the original and wouldn't use constants
35+
_ = "2a04:8ec0:0:47::131".parse::<Ipv6Addr>().unwrap();
36+
_ = "2a04:8ec0:0:47::131".parse::<IpAddr>().unwrap();
37+
38+
_ = true;
39+
//~^ parsed_string_literals
40+
_ = false;
41+
//~^ parsed_string_literals
42+
43+
let _: i64 = -17_i64;
44+
//~^ parsed_string_literals
45+
_ = 10_usize;
46+
//~^ parsed_string_literals
47+
_ = 1.23_f32;
48+
//~^ parsed_string_literals
49+
_ = 1.2300_f32;
50+
//~^ parsed_string_literals
51+
_ = 'c';
52+
//~^ parsed_string_literals
53+
_ = '"';
54+
//~^ parsed_string_literals
55+
_ = '\'';
56+
//~^ parsed_string_literals
57+
58+
// Check that the original form is preserved ('🦀' == '\u{1f980}')
59+
_ = '\u{1f980}';
60+
//~^ parsed_string_literals
61+
_ = '🦀';
62+
//~^ parsed_string_literals
63+
64+
// Do not lint invalid values
65+
_ = "-10".parse::<usize>().unwrap();
66+
67+
// Do not lint content or code coming from macros
68+
macro_rules! mac {
69+
(str) => {
70+
"10"
71+
};
72+
(parse $l:literal) => {
73+
$l.parse::<u32>().unwrap()
74+
};
75+
}
76+
_ = mac!(str).parse::<u32>().unwrap();
77+
_ = mac!(parse "10");
78+
}
79+
80+
#[clippy::msrv = "1.29"]
81+
fn msrv_under() {
82+
_ = "::".parse::<IpAddr>().unwrap();
83+
}

0 commit comments

Comments
 (0)