Skip to content

Commit 2c22b5f

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 3e218d1 commit 2c22b5f

File tree

14 files changed

+921
-1
lines changed

14 files changed

+921
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6496,6 +6496,7 @@ Released 2018-09-13
64966496
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
64976497
[`panicking_overflow_checks`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_overflow_checks
64986498
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
6499+
[`parsed_string_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#parsed_string_literals
64996500
[`partial_pub_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#partial_pub_fields
65006501
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
65016502
[`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
@@ -444,6 +444,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
444444
crate::methods::OPTION_MAP_OR_NONE_INFO,
445445
crate::methods::OR_FUN_CALL_INFO,
446446
crate::methods::OR_THEN_UNWRAP_INFO,
447+
crate::methods::PARSED_STRING_LITERALS_INFO,
447448
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
448449
crate::methods::PATH_ENDS_WITH_EXT_INFO,
449450
crate::methods::PTR_OFFSET_WITH_CAST_INFO,

clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![feature(f128)]
55
#![feature(f16)]
66
#![feature(if_let_guard)]
7+
#![feature(ip_as_octets)]
78
#![feature(iter_intersperse)]
89
#![feature(iter_partition_in_place)]
910
#![feature(never_type)]

clippy_lints/src/methods/mod.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ mod option_map_or_none;
8989
mod option_map_unwrap_or;
9090
mod or_fun_call;
9191
mod or_then_unwrap;
92+
mod parsed_string_literals;
9293
mod path_buf_push_overwrite;
9394
mod path_ends_with_ext;
9495
mod ptr_offset_with_cast;
@@ -4637,6 +4638,36 @@ declare_clippy_lint! {
46374638
"detects redundant calls to `Iterator::cloned`"
46384639
}
46394640

4641+
declare_clippy_lint! {
4642+
/// ### What it does
4643+
/// Checks for parsing string literals into types from the standard library
4644+
///
4645+
/// ### Why is this bad?
4646+
/// Parsing known values at runtime consumes resources and forces to
4647+
/// unwrap the `Ok()` variant returned by `parse()`.
4648+
///
4649+
/// ### Example
4650+
/// ```no_run
4651+
/// use std::net::Ipv4Addr;
4652+
///
4653+
/// let number = "123".parse::<u32>().unwrap();
4654+
/// let addr1: Ipv4Addr = "10.2.3.4".parse().unwrap();
4655+
/// let addr2: Ipv4Addr = "127.0.0.1".parse().unwrap();
4656+
/// ```
4657+
/// Use instead:
4658+
/// ```no_run
4659+
/// use std::net::Ipv4Addr;
4660+
///
4661+
/// let number = 123_u32;
4662+
/// let addr1: Ipv4Addr = Ipv4Addr::new(10, 2, 3, 4);
4663+
/// let addr2: Ipv4Addr = Ipv4Addr::LOCALHOST;
4664+
/// ```
4665+
#[clippy::version = "1.92.0"]
4666+
pub PARSED_STRING_LITERALS,
4667+
perf,
4668+
"known-correct literal IP address parsing"
4669+
}
4670+
46404671
#[expect(clippy::struct_excessive_bools)]
46414672
pub struct Methods {
46424673
avoid_breaking_exported_api: bool,
@@ -4818,6 +4849,7 @@ impl_lint_pass!(Methods => [
48184849
SWAP_WITH_TEMPORARY,
48194850
IP_CONSTANT,
48204851
REDUNDANT_ITER_CLONED,
4852+
PARSED_STRING_LITERALS,
48214853
]);
48224854

48234855
/// Extracts a method call name, args, and `Span` of the method name.
@@ -5542,6 +5574,9 @@ impl Methods {
55425574
Some((sym::or, recv, [or_arg], or_span, _)) => {
55435575
or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
55445576
},
5577+
Some((sym::parse, inner_recv, [], _, _)) => {
5578+
parsed_string_literals::check(cx, expr, inner_recv, recv, self.msrv);
5579+
},
55455580
_ => {},
55465581
}
55475582
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
use std::net::{Ipv4Addr, Ipv6Addr};
2+
3+
use clippy_utils::msrvs::{self, Msrv};
4+
use clippy_utils::source::snippet_with_applicability;
5+
use clippy_utils::sym;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{self as hir, AmbigArg, Expr};
8+
use rustc_lint::LateContext;
9+
use rustc_span::Symbol;
10+
11+
use super::maybe_emit_lint;
12+
13+
static IPV4_ENTITY: &str = "an IPv4 address";
14+
static IPV6_ENTITY: &str = "an IPv6 address";
15+
16+
pub(super) fn check(
17+
cx: &LateContext<'_>,
18+
expr: &Expr<'_>,
19+
lit: Symbol,
20+
method: Symbol,
21+
explicit_type: Option<hir::Ty<'_, AmbigArg>>,
22+
msrv: Msrv,
23+
) {
24+
let ipaddr_consts_available = msrv.meets(cx, msrvs::IPADDR_CONSTANTS);
25+
match method {
26+
sym::Ipv4Addr => {
27+
// Only use constants such as `Ipv4Addr::LOCALHOST` when the type has been explicitly given
28+
if let Some((sugg, typed_const, app)) = ipv4_subst(cx, lit, ipaddr_consts_available, explicit_type) {
29+
maybe_emit_lint(cx, expr, typed_const, IPV4_ENTITY, sugg, app);
30+
}
31+
},
32+
sym::Ipv6Addr => {
33+
// Only use constants such as `Ipv4Addr::LOCALHOST` when the type has been explicitly given
34+
if let Some((sugg, typed_const, app)) = ipv6_subst(cx, lit, ipaddr_consts_available, explicit_type) {
35+
maybe_emit_lint(cx, expr, typed_const, IPV6_ENTITY, sugg, app);
36+
}
37+
},
38+
sym::IpAddr => {
39+
if let Some((sugg, entity, app)) = ip_subst(cx, lit, explicit_type) {
40+
maybe_emit_lint(cx, expr, false, entity, sugg, app);
41+
}
42+
},
43+
_ => unreachable!(),
44+
}
45+
}
46+
47+
/// Suggests a replacement if `addr` is a correct IPv4 address, with:
48+
/// - the replacement string
49+
/// - a boolean that indicates if a typed constant is used
50+
/// - the applicability
51+
///
52+
/// The replacement will be `T::CONSTANT` if a constant is detected,
53+
/// where `T` is either `explicit_type` if provided, or `Ipv4Addr`
54+
/// otherwise.
55+
///
56+
/// In other cases, when the type has been explicitly given as `T`, the
57+
/// `T::new()` constructor will be used. If no type has been explicitly
58+
/// given, then `[u8; 4].into()` will be used as the context should
59+
/// already provide the proper information. This allows us not to use
60+
/// a type name which might not be available in the current scope.
61+
fn ipv4_subst(
62+
cx: &LateContext<'_>,
63+
addr: Symbol,
64+
with_consts: bool,
65+
explicit_type: Option<hir::Ty<'_, AmbigArg>>,
66+
) -> Option<(String, bool, Applicability)> {
67+
as_ipv4_octets(addr).map(|bytes| {
68+
if let Some(ty) = explicit_type {
69+
let mut app = Applicability::MachineApplicable;
70+
let ty = snippet_with_applicability(cx, ty.span, "_", &mut app);
71+
if with_consts && &bytes == Ipv4Addr::LOCALHOST.as_octets() {
72+
(format!("{ty}::LOCALHOST"), true, app)
73+
} else if with_consts && &bytes == Ipv4Addr::BROADCAST.as_octets() {
74+
(format!("{ty}::BROADCAST"), true, app)
75+
} else if with_consts && &bytes == Ipv4Addr::UNSPECIFIED.as_octets() {
76+
(format!("{ty}::UNSPECIFIED"), true, app)
77+
} else {
78+
(
79+
format!("{ty}::new({}, {}, {}, {})", bytes[0], bytes[1], bytes[2], bytes[3]),
80+
false,
81+
app,
82+
)
83+
}
84+
} else {
85+
(
86+
format!("[{}, {}, {}, {}].into()", bytes[0], bytes[1], bytes[2], bytes[3]),
87+
false,
88+
Applicability::MachineApplicable,
89+
)
90+
}
91+
})
92+
}
93+
94+
/// Try parsing `addr` as an IPv4 address and return its octets
95+
fn as_ipv4_octets(addr: Symbol) -> Option<[u8; 4]> {
96+
addr.as_str().parse::<Ipv4Addr>().ok().map(|addr| *addr.as_octets())
97+
}
98+
99+
/// Suggests a replacement if `addr` is a correct IPv6 address, with:
100+
/// - the replacement string
101+
/// - a boolean that indicates if a typed constant is used
102+
/// - the applicability
103+
///
104+
/// Replacement will either be:
105+
/// - `T::CONSTANT`
106+
/// - `Ipv6Addr::CONSTANT` if no `explicit_type` is defined
107+
/// - `T::new(…)`
108+
/// - `[u16; 8].into()` if no `explicit_type` is defined
109+
///
110+
/// See [`ipv4_subst()`] for more details.
111+
fn ipv6_subst(
112+
cx: &LateContext<'_>,
113+
addr: Symbol,
114+
with_consts: bool,
115+
explicit_type: Option<hir::Ty<'_, AmbigArg>>,
116+
) -> Option<(String, bool, Applicability)> {
117+
as_ipv6_segments(addr).map(|segments| {
118+
if let Some(ty) = explicit_type {
119+
let mut app = Applicability::MachineApplicable;
120+
let ty = snippet_with_applicability(cx, ty.span, "_", &mut app);
121+
if with_consts && segments == Ipv6Addr::LOCALHOST.segments() {
122+
(format!("{ty}::LOCALHOST"), true, app)
123+
} else if with_consts && explicit_type.is_some() && segments == Ipv6Addr::UNSPECIFIED.segments() {
124+
(format!("{ty}::UNSPECIFIED"), true, app)
125+
} else {
126+
(format!("{ty}::new({})", segments_to_string(&segments)), false, app)
127+
}
128+
} else {
129+
(
130+
format!("[{}].into()", segments_to_string(&segments)),
131+
false,
132+
Applicability::MachineApplicable,
133+
)
134+
}
135+
})
136+
}
137+
138+
/// Try parsing `addr` as an IPv6 address and return its 16-bit segments
139+
fn as_ipv6_segments(addr: Symbol) -> Option<[u16; 8]> {
140+
addr.as_str().parse().ok().as_ref().map(Ipv6Addr::segments)
141+
}
142+
143+
/// Return the `segments` separated by commas, in a common format for IPv6 addresses
144+
fn segments_to_string(segments: &[u16; 8]) -> String {
145+
segments
146+
.map(|n| if n < 2 { n.to_string() } else { format!("{n:#x}") })
147+
.join(", ")
148+
}
149+
150+
/// Suggests a replacement if `addr` is a correct IPv6 address, with:
151+
/// - the replacement string
152+
/// - the entity that was detected
153+
/// - the applicability
154+
///
155+
/// `explicit_type` refers to `IpAddr`, and not to the content of one of the variants
156+
/// (`IpAddr::V4` or `IpAddr::V6`). The use of constants from `Ipv4Addr` or `Ipv6Addr`
157+
/// will not be proposed because we do not know if those types are imported in the scope.
158+
fn ip_subst(
159+
cx: &LateContext<'_>,
160+
addr: Symbol,
161+
explicit_type: Option<hir::Ty<'_, AmbigArg>>,
162+
) -> Option<(String, &'static str, Applicability)> {
163+
let mut app = Applicability::MachineApplicable;
164+
let mut type_name = |ty: hir::Ty<'_, AmbigArg>| snippet_with_applicability(cx, ty.span, "_", &mut app);
165+
if let Some([a0, a1, a2, a3]) = as_ipv4_octets(addr) {
166+
Some((
167+
if let Some(ty) = explicit_type {
168+
format!("{}::V4([{a0}, {a1}, {a2}, {a3}].into())", type_name(ty))
169+
} else {
170+
format!("[{a0}, {a1}, {a2}, {a3}].into()")
171+
},
172+
IPV4_ENTITY,
173+
app,
174+
))
175+
} else if let Some(segments) = as_ipv6_segments(addr) {
176+
Some((
177+
if let Some(ty) = explicit_type {
178+
format!("{}::V6([{}].into())", type_name(ty), segments_to_string(&segments))
179+
} else {
180+
format!("[{}].into()", segments_to_string(&segments))
181+
},
182+
IPV6_ENTITY,
183+
app,
184+
))
185+
} else {
186+
None
187+
}
188+
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::msrvs::Msrv;
3+
use clippy_utils::source::SpanRangeExt as _;
4+
use clippy_utils::sym;
5+
use clippy_utils::ty::get_type_diagnostic_name;
6+
use rustc_ast::LitKind;
7+
use rustc_errors::Applicability;
8+
use rustc_hir::{self as hir, AmbigArg, Expr, ExprKind, GenericArg, Node};
9+
use rustc_lint::LateContext;
10+
11+
mod ip_addresses;
12+
mod primitive_types;
13+
14+
use super::PARSED_STRING_LITERALS;
15+
16+
/// Detects instances of `"literal".parse().unwrap()`:
17+
/// - `expr` is the whole expression
18+
/// - `recv` is the receiver of `parse()`
19+
/// - `parse_call` is the `parse()` method call
20+
/// - `msrv` is used for Rust version checking
21+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, parse_call: &Expr<'_>, msrv: Msrv) {
22+
if let ExprKind::Lit(lit) = recv.kind
23+
&& let LitKind::Str(lit, _) = lit.node
24+
{
25+
let ty = cx.typeck_results().expr_ty(expr);
26+
match get_type_diagnostic_name(cx, ty) {
27+
_ if ty.is_primitive() => {
28+
primitive_types::check(cx, expr, lit, ty, recv, type_from_parse(parse_call).is_some())
29+
},
30+
Some(method @ (sym::IpAddr | sym::Ipv4Addr | sym::Ipv6Addr)) => ip_addresses::check(
31+
cx,
32+
expr,
33+
lit,
34+
method,
35+
type_from_parse(parse_call).or_else(|| type_from_let(cx, expr)),
36+
msrv,
37+
),
38+
_ => (),
39+
}
40+
}
41+
}
42+
43+
/// Emit the lint if the length of `sugg` is no longer than the original `expr` span, or if `force`
44+
/// is set.
45+
fn maybe_emit_lint(
46+
cx: &LateContext<'_>,
47+
expr: &Expr<'_>,
48+
force: bool,
49+
entity: &str,
50+
sugg: String,
51+
applicability: Applicability,
52+
) {
53+
if force || expr.span.check_source_text(cx, |snip| snip.len() >= sugg.len()) {
54+
span_lint_and_sugg(
55+
cx,
56+
PARSED_STRING_LITERALS,
57+
expr.span,
58+
format!("unnecessary runtime parsing of {entity}"),
59+
"use",
60+
sugg,
61+
applicability,
62+
);
63+
}
64+
}
65+
66+
/// Returns `T` from the `parse::<T>(…)` call if present.
67+
fn type_from_parse<'hir>(parse_call: &'hir Expr<'_>) -> Option<hir::Ty<'hir, AmbigArg>> {
68+
if let ExprKind::MethodCall(parse, _, _, _) = parse_call.kind
69+
&& let [GenericArg::Type(ty)] = parse.args().args
70+
&& matches!(ty.kind, hir::TyKind::Path(_))
71+
{
72+
Some(**ty)
73+
} else {
74+
None
75+
}
76+
}
77+
78+
/// Returns `T` if `expr` is the initialization of `let …: T = expr`. This is used as an extra
79+
/// opportunity to use variant constructors when `T` denotes an `enum`.
80+
fn type_from_let<'hir>(cx: &'hir LateContext<'_>, expr: &'hir Expr<'_>) -> Option<hir::Ty<'hir, AmbigArg>> {
81+
if let Node::LetStmt(let_stmt) = cx.tcx.parent_hir_node(expr.hir_id)
82+
&& let Some(ty) = let_stmt.ty
83+
&& let Some(ty) = ty.try_as_ambig_ty()
84+
&& matches!(ty.kind, hir::TyKind::Path(_))
85+
{
86+
Some(*ty)
87+
} else {
88+
None
89+
}
90+
}

0 commit comments

Comments
 (0)