parsed_string_literals: new lint

This lint detects parsing of string literals into primitive types or
IP addresses when they are known correct.

Apply review comments

Author: samueltardieu

CHANGELOG.md

@@ -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

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/lib.rs

@@ -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)]

clippy_lints/src/methods/mod.rs

@@ -89,6 +89,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;
@@ -4637,6 +4638,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,
@@ -4818,6 +4849,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.
@@ -5542,6 +5574,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);

clippy_lints/src/methods/parsed_string_literals/ip_addresses.rs

@@ -0,0 +1,184 @@
+use std::net::{Ipv4Addr, Ipv6Addr};
+
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{std_or_core, sym};
+use rustc_errors::Applicability;
+use rustc_hir::{self as hir, AmbigArg, Expr};
+use rustc_lint::LateContext;
+use rustc_span::Symbol;
+
+use super::{maybe_emit_lint, warn_about};
+
+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<hir::Ty<'_, AmbigArg>>,
+    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, app)) = ipv4_subst(cx, lit, ipaddr_consts_available, explicit_type) {
+                maybe_emit_lint(cx, expr, typed_const, IPV4_ENTITY, sugg, app);
+            }
+        },
+        sym::Ipv6Addr => {
+            // Only use constants such as `Ipv4Addr::LOCALHOST` when the type has been explicitly given
+            if let Some((sugg, typed_const, app)) = ipv6_subst(cx, lit, ipaddr_consts_available, explicit_type) {
+                maybe_emit_lint(cx, expr, typed_const, IPV6_ENTITY, sugg, app);
+            }
+        },
+        sym::IpAddr => {
+            if let Some(explicit_type) = explicit_type
+                && let Some((sugg, typed_const, entity, app)) = ip_subst(cx, lit, explicit_type)
+            {
+                maybe_emit_lint(cx, expr, typed_const, entity, sugg, app);
+            } else {
+                warn_about(
+                    cx,
+                    expr,
+                    "an IP address",
+                    format!(
+                        "use a variant of the `{}::net::IpAddr` enum instead",
+                        std_or_core(cx).unwrap_or("std")
+                    ),
+                );
+            }
+        },
+        _ => 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 applicability
+///
+/// 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<hir::Ty<'_, AmbigArg>>,
+) -> Option<(String, bool, Applicability)> {
+    addr.as_str().parse().ok().map(|ipv4: Ipv4Addr| {
+        let bytes = ipv4.as_octets();
+        if let Some(ty) = explicit_type {
+            let mut app = Applicability::MachineApplicable;
+            let ty = snippet_with_applicability(cx, ty.span, "_", &mut app);
+            if with_consts && bytes == &[127, 0, 0, 1] {
+                (format!("{ty}::LOCALHOST"), true, app)
+            } else if with_consts && ipv4.is_broadcast() {
+                (format!("{ty}::BROADCAST"), true, app)
+            } else if with_consts && ipv4.is_unspecified() {
+                (format!("{ty}::UNSPECIFIED"), true, app)
+            } else {
+                (
+                    format!("{ty}::new({}, {}, {}, {})", bytes[0], bytes[1], bytes[2], bytes[3]),
+                    false,
+                    app,
+                )
+            }
+        } else {
+            (
+                format!("[{}, {}, {}, {}].into()", bytes[0], bytes[1], bytes[2], bytes[3]),
+                false,
+                Applicability::MachineApplicable,
+            )
+        }
+    })
+}
+
+/// Suggests a replacement if `addr` is a correct IPv6 address, with:
+/// - the replacement string
+/// - a boolean that indicates if a typed constant is used
+/// - the applicability
+///
+/// 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<hir::Ty<'_, AmbigArg>>,
+) -> Option<(String, bool, Applicability)> {
+    addr.as_str().parse().ok().map(|ipv6: Ipv6Addr| {
+        let addr = || {
+            ipv6.segments()
+                .map(|n| if n < 2 { n.to_string() } else { format!("{n:#x}") })
+                .join(", ")
+        };
+        if let Some(ty) = explicit_type {
+            let mut app = Applicability::MachineApplicable;
+            let ty = snippet_with_applicability(cx, ty.span, "_", &mut app);
+            if with_consts && ipv6.is_loopback() {
+                (format!("{ty}::LOCALHOST"), true, app)
+            } else if with_consts && explicit_type.is_some() && ipv6.is_unspecified() {
+                (format!("{ty}::UNSPECIFIED"), true, app)
+            } else {
+                (format!("{ty}::new({})", addr()), false, app)
+            }
+        } else {
+            (format!("[{}].into()", addr()), false, Applicability::MachineApplicable)
+        }
+    })
+}
+
+/// Suggests a replacement if `addr` is a correct IPv6 address, with:
+/// - the replacement string
+/// - a boolean that indicates if a typed constant is used
+/// - the entity that was detected
+/// - the applicability
+///
+/// `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: hir::Ty<'_, AmbigArg>,
+) -> Option<(String, bool, &'static str, Applicability)> {
+    let (sugg, typed_const, entity, variant) = if let Some((sugg, typed_const, _)) = ipv4_subst(cx, addr, false, None) {
+        (sugg, typed_const, IPV4_ENTITY, "V4")
+    } else if let Some((sugg, typed_const, _)) = ipv6_subst(cx, addr, false, None) {
+        (sugg, typed_const, IPV6_ENTITY, "V6")
+    } else {
+        return None;
+    };
+    // If a typed constant has been used, we cannot know for sure that the `Ipv4Addr`/`Ipv6Addr` is
+    // present in the current scope because the explicit type if present would be `IpAddr`.
+    let mut app = if typed_const {
+        Applicability::MaybeIncorrect
+    } else {
+        Applicability::MachineApplicable
+    };
+    Some((
+        format!(
+            "{}::{variant}({sugg})",
+            snippet_with_applicability(cx, explicit_type.span, "_", &mut app)
+        ),
+        typed_const,
+        entity,
+        app,
+    ))
+}

clippy_lints/src/methods/parsed_string_literals/mod.rs

@@ -0,0 +1,93 @@
+use clippy_utils::diagnostics::{span_lint_and_help, 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, SubdiagMessage};
+use rustc_hir::{self as hir, AmbigArg, Expr, ExprKind, GenericArg, Node};
+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),
+            Some(method @ (sym::IpAddr | sym::Ipv4Addr | sym::Ipv6Addr)) => {
+                ip_addresses::check(cx, expr, lit, method, get_explicit_type(cx, expr, parse_call), 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,
+    applicability: Applicability,
+) {
+    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,
+        );
+    }
+}
+
+/// Warn and help about an expression.
+fn warn_about(cx: &LateContext<'_>, expr: &Expr<'_>, entity: &str, help: impl Into<SubdiagMessage>) {
+    span_lint_and_help(
+        cx,
+        PARSED_STRING_LITERALS,
+        expr.span,
+        format!("unnecessary runtime parsing of {entity}"),
+        None,
+        help,
+    );
+}
+
+/// Returns `T` from the `parse::<T>(…)` call if present, or from a `let _: T =
+/// "…".parse().unwrap();` otherwise if it can be found. The `let` statement has been chosen because
+/// it is quite common to assign the result of parsing to a variable.
+fn get_explicit_type<'hir>(
+    cx: &'hir LateContext<'_>,
+    expr: &'hir Expr<'_>,
+    parse_call: &'hir Expr<'_>,
+) -> Option<hir::Ty<'hir, AmbigArg>> {
+    if let ExprKind::MethodCall(parse, _, _, _) = parse_call.kind
+        && let [GenericArg::Type(ty)] = parse.args().args
+        && matches!(ty.kind, hir::TyKind::Path(_))
+    {
+        Some(**ty)
+    } else 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()
+        && matches!(ty.kind, hir::TyKind::Path(_))
+    {
+        Some(*ty)
+    } else {
+        None
+    }
+}