Skip to content

Commit e8f3f64

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 e8f3f64

File tree

14 files changed

+984
-1
lines changed

14 files changed

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

0 commit comments

Comments
 (0)