Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6496,6 +6496,7 @@ Released 2018-09-13
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
[`panicking_overflow_checks`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_overflow_checks
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
[`parsed_string_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#parsed_string_literals
[`partial_pub_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#partial_pub_fields
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
[`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::methods::OPTION_MAP_OR_NONE_INFO,
crate::methods::OR_FUN_CALL_INFO,
crate::methods::OR_THEN_UNWRAP_INFO,
crate::methods::PARSED_STRING_LITERALS_INFO,
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
crate::methods::PATH_ENDS_WITH_EXT_INFO,
crate::methods::PTR_OFFSET_WITH_CAST_INFO,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#![feature(f128)]
#![feature(f16)]
#![feature(if_let_guard)]
#![feature(ip_as_octets)]
#![feature(iter_intersperse)]
#![feature(iter_partition_in_place)]
#![feature(never_type)]
Expand Down
35 changes: 35 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ mod option_map_or_none;
mod option_map_unwrap_or;
mod or_fun_call;
mod or_then_unwrap;
mod parsed_string_literals;
mod path_buf_push_overwrite;
mod path_ends_with_ext;
mod ptr_offset_with_cast;
Expand Down Expand Up @@ -4639,6 +4640,36 @@ declare_clippy_lint! {
"detects redundant calls to `Iterator::cloned`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for parsing string literals into types from the standard library
///
/// ### Why is this bad?
/// Parsing known values at runtime consumes resources and forces to
/// unwrap the `Ok()` variant returned by `parse()`.
///
/// ### Example
/// ```no_run
/// use std::net::Ipv4Addr;
///
/// let number = "123".parse::<u32>().unwrap();
/// let addr1: Ipv4Addr = "10.2.3.4".parse().unwrap();
/// let addr2: Ipv4Addr = "127.0.0.1".parse().unwrap();
/// ```
/// Use instead:
/// ```no_run
/// use std::net::Ipv4Addr;
///
/// let number = 123_u32;
/// let addr1: Ipv4Addr = Ipv4Addr::new(10, 2, 3, 4);
/// let addr2: Ipv4Addr = Ipv4Addr::LOCALHOST;
/// ```
#[clippy::version = "1.92.0"]
pub PARSED_STRING_LITERALS,
perf,
"known-correct literal IP address parsing"
}

#[expect(clippy::struct_excessive_bools)]
pub struct Methods {
avoid_breaking_exported_api: bool,
Expand Down Expand Up @@ -4820,6 +4851,7 @@ impl_lint_pass!(Methods => [
SWAP_WITH_TEMPORARY,
IP_CONSTANT,
REDUNDANT_ITER_CLONED,
PARSED_STRING_LITERALS,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -5482,6 +5514,9 @@ impl Methods {
Some((sym::or, recv, [or_arg], or_span, _)) => {
or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
},
Some((sym::parse, inner_recv, [], _, _)) => {
parsed_string_literals::check(cx, expr, inner_recv, recv, self.msrv);
},
_ => {},
}
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
Expand Down
174 changes: 174 additions & 0 deletions clippy_lints/src/methods/parsed_string_literals/ip_addresses.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
use std::net::{Ipv4Addr, Ipv6Addr};

use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::SpanRangeExt as _;
use clippy_utils::sym;
use rustc_hir::{Expr, QPath};
use rustc_lint::LateContext;
use rustc_span::Symbol;

use super::maybe_emit_lint;

static IPV4_ENTITY: &str = "an IPv4 address";
static IPV6_ENTITY: &str = "an IPv6 address";

pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
lit: Symbol,
method: Symbol,
explicit_type: Option<QPath<'_>>,
msrv: Msrv,
) {
let ipaddr_consts_available = msrv.meets(cx, msrvs::IPADDR_CONSTANTS);
match method {
sym::Ipv4Addr => {
// Only use constants such as `Ipv4Addr::LOCALHOST` when the type has been explicitly given
if let Some((sugg, typed_const)) = ipv4_subst(cx, lit, ipaddr_consts_available, explicit_type) {
maybe_emit_lint(cx, expr, typed_const, IPV4_ENTITY, sugg);
}
},
sym::Ipv6Addr => {
// Only use constants such as `Ipv4Addr::LOCALHOST` when the type has been explicitly given
if let Some((sugg, typed_const)) = ipv6_subst(cx, lit, ipaddr_consts_available, explicit_type) {
maybe_emit_lint(cx, expr, typed_const, IPV6_ENTITY, sugg);
}
},
sym::IpAddr => {
if let Some((sugg, entity)) = ip_subst(cx, lit, explicit_type) {
maybe_emit_lint(cx, expr, false, entity, sugg);
}
},
_ => unreachable!(),
}
}

/// Suggests a replacement if `addr` is a correct IPv4 address, with:
/// - the replacement string
/// - a boolean that indicates if a typed constant is used
///
/// The replacement will be `T::CONSTANT` if a constant is detected,
/// where `T` is either `explicit_type` if provided, or `Ipv4Addr`
/// otherwise.
///
/// In other cases, when the type has been explicitly given as `T`, the
/// `T::new()` constructor will be used. If no type has been explicitly
/// given, then `[u8; 4].into()` will be used as the context should
/// already provide the proper information. This allows us not to use
/// a type name which might not be available in the current scope.
fn ipv4_subst(
cx: &LateContext<'_>,
addr: Symbol,
with_consts: bool,
explicit_type: Option<QPath<'_>>,
) -> Option<(String, bool)> {
as_ipv4_octets(addr).and_then(|bytes| {
if let Some(qpath) = explicit_type {
qpath.span().with_source_text(cx, |ty| {
if with_consts && &bytes == Ipv4Addr::LOCALHOST.as_octets() {
(format!("{ty}::LOCALHOST"), true)
} else if with_consts && &bytes == Ipv4Addr::BROADCAST.as_octets() {
(format!("{ty}::BROADCAST"), true)
} else if with_consts && &bytes == Ipv4Addr::UNSPECIFIED.as_octets() {
(format!("{ty}::UNSPECIFIED"), true)
} else {
(
format!("{ty}::new({}, {}, {}, {})", bytes[0], bytes[1], bytes[2], bytes[3]),
false,
)
}
})
} else {
Some((
format!("[{}, {}, {}, {}].into()", bytes[0], bytes[1], bytes[2], bytes[3]),
false,
))
}
})
}

/// Try parsing `addr` as an IPv4 address and return its octets
fn as_ipv4_octets(addr: Symbol) -> Option<[u8; 4]> {
addr.as_str().parse::<Ipv4Addr>().ok().map(|addr| *addr.as_octets())
}

/// Suggests a replacement if `addr` is a correct IPv6 address, with:
/// - the replacement string
/// - a boolean that indicates if a typed constant is used
///
/// Replacement will either be:
/// - `T::CONSTANT`
/// - `Ipv6Addr::CONSTANT` if no `explicit_type` is defined
/// - `T::new(…)`
/// - `[u16; 8].into()` if no `explicit_type` is defined
///
/// See [`ipv4_subst()`] for more details.
fn ipv6_subst(
cx: &LateContext<'_>,
addr: Symbol,
with_consts: bool,
explicit_type: Option<QPath<'_>>,
) -> Option<(String, bool)> {
as_ipv6_segments(addr).and_then(|segments| {
if let Some(qpath) = explicit_type {
qpath.span().with_source_text(cx, |ty| {
if with_consts && segments == Ipv6Addr::LOCALHOST.segments() {
(format!("{ty}::LOCALHOST"), true)
} else if with_consts && explicit_type.is_some() && segments == Ipv6Addr::UNSPECIFIED.segments() {
(format!("{ty}::UNSPECIFIED"), true)
} else {
(format!("{ty}::new({})", segments_to_string(&segments)), false)
}
})
} else {
Some((format!("[{}].into()", segments_to_string(&segments)), false))
}
})
}

/// Try parsing `addr` as an IPv6 address and return its 16-bit segments
fn as_ipv6_segments(addr: Symbol) -> Option<[u16; 8]> {
addr.as_str().parse().ok().as_ref().map(Ipv6Addr::segments)
}

/// Return the `segments` separated by commas, in a common format for IPv6 addresses
fn segments_to_string(segments: &[u16; 8]) -> String {
segments
.map(|n| if n < 2 { n.to_string() } else { format!("{n:#x}") })
.join(", ")
}

/// Suggests a replacement if `addr` is a correct IPv6 address, with:
/// - the replacement string
/// - the entity that was detected
///
/// `explicit_type` refers to `IpAddr`, and not to the content of one of the variants
/// (`IpAddr::V4` or `IpAddr::V6`). The use of constants from `Ipv4Addr` or `Ipv6Addr`
/// will not be proposed because we do not know if those types are imported in the scope.
fn ip_subst(cx: &LateContext<'_>, addr: Symbol, explicit_type: Option<QPath<'_>>) -> Option<(String, &'static str)> {
if let Some([a0, a1, a2, a3]) = as_ipv4_octets(addr) {
Some((
if let Some(qpath) = explicit_type {
qpath
.span()
.with_source_text(cx, |ty| format!("{ty}::V4([{a0}, {a1}, {a2}, {a3}].into())"))?
} else {
format!("[{a0}, {a1}, {a2}, {a3}].into()")
},
IPV4_ENTITY,
))
} else if let Some(segments) = as_ipv6_segments(addr) {
Some((
if let Some(qpath) = explicit_type {
qpath
.span()
.with_source_text(cx, |ty| format!("{ty}::V6([{}].into())", segments_to_string(&segments)))?
} else {
format!("[{}].into()", segments_to_string(&segments))
},
IPV6_ENTITY,
))
} else {
None
}
}
81 changes: 81 additions & 0 deletions clippy_lints/src/methods/parsed_string_literals/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::msrvs::Msrv;
use clippy_utils::source::SpanRangeExt as _;
use clippy_utils::sym;
use clippy_utils::ty::get_type_diagnostic_name;
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{self as hir, Expr, ExprKind, GenericArg, Node, QPath};
use rustc_lint::LateContext;

mod ip_addresses;
mod primitive_types;

use super::PARSED_STRING_LITERALS;

/// Detects instances of `"literal".parse().unwrap()`:
/// - `expr` is the whole expression
/// - `recv` is the receiver of `parse()`
/// - `parse_call` is the `parse()` method call
/// - `msrv` is used for Rust version checking
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, parse_call: &Expr<'_>, msrv: Msrv) {
if let ExprKind::Lit(lit) = recv.kind
&& let LitKind::Str(lit, _) = lit.node
{
let ty = cx.typeck_results().expr_ty(expr);
match get_type_diagnostic_name(cx, ty) {
_ if ty.is_primitive() => primitive_types::check(cx, expr, lit, ty, recv, type_from_parse(parse_call)),
Some(method @ (sym::IpAddr | sym::Ipv4Addr | sym::Ipv6Addr)) => ip_addresses::check(
cx,
expr,
lit,
method,
type_from_parse(parse_call).or_else(|| type_from_let(cx, expr)),
msrv,
),
_ => (),
}
}
}

/// Emit the lint if the length of `sugg` is no longer than the original `expr` span, or if `force`
/// is set.
fn maybe_emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, force: bool, entity: &str, sugg: String) {
if force || expr.span.check_source_text(cx, |snip| snip.len() >= sugg.len()) {
span_lint_and_sugg(
cx,
PARSED_STRING_LITERALS,
expr.span,
format!("unnecessary runtime parsing of {entity}"),
"use",
sugg,
Applicability::MachineApplicable,
);
}
}

/// Returns `T` from the `parse::<T>(…)` call if present.
fn type_from_parse<'hir>(parse_call: &'hir Expr<'_>) -> Option<QPath<'hir>> {
if let ExprKind::MethodCall(parse, _, _, _) = parse_call.kind
&& let [GenericArg::Type(ty)] = parse.args().args
&& let hir::TyKind::Path(qpath) = ty.kind
{
Some(qpath)
} else {
None
}
}

/// Returns `T` if `expr` is the initialization of `let …: T = expr`. This is used as an extra
/// opportunity to use variant constructors when `T` denotes an `enum`.
fn type_from_let<'hir>(cx: &'hir LateContext<'_>, expr: &'hir Expr<'_>) -> Option<QPath<'hir>> {
if let Node::LetStmt(let_stmt) = cx.tcx.parent_hir_node(expr.hir_id)
&& let Some(ty) = let_stmt.ty
&& let Some(ty) = ty.try_as_ambig_ty()
&& let hir::TyKind::Path(qpath) = ty.kind
{
Some(qpath)
} else {
None
}
}
Loading