Skip to content

Commit a1dac58

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 a1dac58

File tree

9 files changed

+457
-1
lines changed

9 files changed

+457
-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: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
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;
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) {
26+
check_ipaddr(cx, expr, lit, ty, msrv);
27+
}
28+
}
29+
}
30+
31+
fn check_primitive(cx: &LateContext<'_>, expr: &Expr<'_>, lit: Symbol, ty: Ty<'_>) -> 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+
if let (Some(subst), entity) = match ty.kind() {
43+
ty::Int(int_ty) => number!(IntTy, int_ty, "a signed integer",
44+
[Isize => isize, I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128]),
45+
ty::Uint(uint_ty) => number!(UintTy, uint_ty, "an unsigned integer",
46+
[Usize => usize, U8 => u8, U16 => u16, U32 => u32, U64 => u64, U128 => u128]),
47+
// FIXME: ignore `f16` and `f128` for now as they cannot use the default formatter
48+
ty::Float(float_ty) => number!(FloatTy, float_ty, "a real number",
49+
[F32 => f32, F64 => f64]),
50+
ty::Bool => (try_parse::<bool>(lit, None), "a boolean"),
51+
ty::Char if let Ok(ch) = lit.as_str().parse::<char>() => (
52+
Some(if ch == '\'' {
53+
r"'\''".to_owned()
54+
} else {
55+
format!("'{ch}'")
56+
}),
57+
"a character",
58+
),
59+
_ => return false,
60+
} {
61+
maybe_emit_lint(cx, expr, false, entity, subst.into());
62+
}
63+
true
64+
}
65+
66+
fn check_ipaddr(cx: &LateContext<'_>, expr: &Expr<'_>, lit: Symbol, ty: Ty<'_>, msrv: Msrv) {
67+
static IPV4_ENTITY: &str = "an IPv4 address";
68+
static IPV6_ENTITY: &str = "an IPv6 address";
69+
let ipaddr_consts_available = || msrv.meets(cx, msrvs::IPADDR_CONSTANTS);
70+
if is_type_diagnostic_item(cx, ty, sym::Ipv4Addr)
71+
&& let Some(sugg) = ipv4_subst(lit, ipaddr_consts_available())
72+
{
73+
maybe_emit_lint(cx, expr, sugg.is_borrowed(), IPV4_ENTITY, sugg);
74+
} else if is_type_diagnostic_item(cx, ty, sym::Ipv6Addr)
75+
&& let Some(sugg) = ipv6_subst(lit, ipaddr_consts_available())
76+
{
77+
maybe_emit_lint(cx, expr, sugg.is_borrowed(), IPV6_ENTITY, sugg);
78+
} else if is_type_diagnostic_item(cx, ty, sym::IpAddr) {
79+
let with_consts = ipaddr_consts_available();
80+
if let Some(sugg) = ipv4_subst(lit, with_consts) {
81+
maybe_emit_lint(
82+
cx,
83+
expr,
84+
sugg.is_borrowed(),
85+
IPV4_ENTITY,
86+
format!("IpAddr::V4({sugg})").into(),
87+
);
88+
} else if let Some(sugg) = ipv6_subst(lit, with_consts) {
89+
maybe_emit_lint(
90+
cx,
91+
expr,
92+
sugg.is_borrowed(),
93+
IPV6_ENTITY,
94+
format!("IpAddr::V6({sugg})").into(),
95+
);
96+
}
97+
}
98+
}
99+
100+
/// Suggests a replacement if `addr` is a correct IPv4 address
101+
fn ipv4_subst(addr: Symbol, with_consts: bool) -> Option<Cow<'static, str>> {
102+
addr.as_str().parse().ok().map(|ipv4: Ipv4Addr| {
103+
if with_consts && ipv4.as_octets() == &[127, 0, 0, 1] {
104+
"Ipv4Addr::LOCALHOST".into()
105+
} else if with_consts && ipv4.is_broadcast() {
106+
"Ipv4Addr::BROADCAST".into()
107+
} else if with_consts && ipv4.is_unspecified() {
108+
"Ipv4Addr::UNSPECIFIED".into()
109+
} else {
110+
let ipv4 = ipv4.as_octets();
111+
format!("Ipv4Addr::new({}, {}, {}, {})", ipv4[0], ipv4[1], ipv4[2], ipv4[3]).into()
112+
}
113+
})
114+
}
115+
116+
/// Suggests a replacement if `addr` is a correct IPv6 address
117+
fn ipv6_subst(addr: Symbol, with_consts: bool) -> Option<Cow<'static, str>> {
118+
addr.as_str().parse().ok().map(|ipv6: Ipv6Addr| {
119+
if with_consts && ipv6.is_loopback() {
120+
"Ipv6Addr::LOCALHOST".into()
121+
} else if with_consts && ipv6.is_unspecified() {
122+
"Ipv6Addr::UNSPECIFIED".into()
123+
} else {
124+
format!(
125+
"Ipv6Addr::new([{}])",
126+
ipv6.segments()
127+
.map(|n| if n < 2 { n.to_string() } else { format!("{n:#x}") })
128+
.join(", ")
129+
)
130+
.into()
131+
}
132+
})
133+
}
134+
135+
fn try_parse<T: FromStr + Display>(lit: Symbol, suffix: Option<&str>) -> Option<String> {
136+
lit.as_str()
137+
.parse::<T>()
138+
.ok()
139+
.map(|_| suffix.map_or_else(|| lit.to_string(), |suffix| format!("{lit}_{suffix}")))
140+
}
141+
142+
/// Emit the lint if the length of `sugg` is no longer than the original `expr` span, or if `force`
143+
/// is set.
144+
fn maybe_emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, force: bool, entity: &str, sugg: Cow<'_, str>) {
145+
if force || expr.span.check_source_text(cx, |snip| snip.len() >= sugg.len()) {
146+
span_lint_and_sugg(
147+
cx,
148+
PARSED_STRING_LITERALS,
149+
expr.span,
150+
format!("unnecessary runtime parsing of {entity}"),
151+
"use",
152+
sugg.into(),
153+
Applicability::MachineApplicable,
154+
);
155+
}
156+
}

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: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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+
// Do not lint invalid values
59+
_ = "-10".parse::<usize>().unwrap();
60+
61+
// Do not lint content or code coming from macros
62+
macro_rules! mac {
63+
(str) => {
64+
"10"
65+
};
66+
(parse $l:literal) => {
67+
$l.parse::<u32>().unwrap()
68+
};
69+
}
70+
_ = mac!(str).parse::<u32>().unwrap();
71+
_ = mac!(parse "10");
72+
}
73+
74+
#[clippy::msrv = "1.29"]
75+
fn msrv_under() {
76+
_ = "::".parse::<IpAddr>().unwrap();
77+
}

0 commit comments

Comments
 (0)