new lint: manual_c_str_literals

CHANGELOG.md

@@ -5340,6 +5340,7 @@ Released 2018-09-13
 [`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
+[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
 [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
 [`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
 [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map

book/src/lint_configuration.md

@@ -151,6 +151,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold)
 * [`manual_hash_one`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one)
 * [`iter_kv_map`](https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map)
+* [`manual_c_str_literals`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals)
 
 
 ## `cognitive-complexity-threshold`

clippy_config/src/conf.rs

@@ -260,7 +260,7 @@ define_Conf! {
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),
-    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP.
+    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS.
     ///
     /// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
     #[default_text = ""]

clippy_config/src/msrvs.rs

@@ -17,6 +17,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
+    1,77,0 { C_STR_LITERALS }
     1,76,0 { PTR_FROM_REF }
     1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
     1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }

clippy_lints/src/declared_lints.rs

@@ -387,6 +387,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::ITER_SKIP_ZERO_INFO,
     crate::methods::ITER_WITH_DRAIN_INFO,
     crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
+    crate::methods::MANUAL_C_STR_LITERALS_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
     crate::methods::MANUAL_IS_VARIANT_AND_INFO,

clippy_lints/src/methods/manual_c_str_literals.rs

@@ -0,0 +1,197 @@
+use clippy_config::msrvs::{self, Msrv};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::get_parent_expr;
+use clippy_utils::source::snippet;
+use rustc_ast::{LitKind, StrStyle};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, Node, QPath, TyKind};
+use rustc_lint::LateContext;
+use rustc_span::{sym, Span, Symbol};
+
+use super::MANUAL_C_STR_LITERALS;
+
+/// Checks:
+/// - `b"...".as_ptr()`
+/// - `b"...".as_ptr().cast()`
+/// - `"...".as_ptr()`
+/// - `"...".as_ptr().cast()`
+///
+/// Iff the parent call of `.cast()` isn't `CStr::from_ptr`, to avoid linting twice.
+pub(super) fn check_as_ptr<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    receiver: &'tcx Expr<'tcx>,
+    msrv: &Msrv,
+) {
+    if let ExprKind::Lit(lit) = receiver.kind
+        && let LitKind::ByteStr(_, StrStyle::Cooked) | LitKind::Str(_, StrStyle::Cooked) = lit.node
+        && let casts_removed = peel_ptr_cast_ancestors(cx, expr)
+        && !get_parent_expr(cx, casts_removed).is_some_and(
+            |parent| matches!(parent.kind, ExprKind::Call(func, _) if is_c_str_function(cx, func).is_some()),
+        )
+        && let Some(sugg) = rewrite_as_cstr(cx, lit.span)
+        && msrv.meets(msrvs::C_STR_LITERALS)
+    {
+        span_lint_and_sugg(
+            cx,
+            MANUAL_C_STR_LITERALS,
+            receiver.span,
+            "manually constructing a nul-terminated string",
+            r#"use a `c""` literal"#,
+            sugg,
+            // an additional cast may be needed, since the type of `CStr::as_ptr` and
+            // `"".as_ptr()` can differ and is platform dependent
+            Applicability::HasPlaceholders,
+        );
+    }
+}
+
+/// Checks if the callee is a "relevant" `CStr` function considered by this lint.
+/// Returns the function name.
+fn is_c_str_function(cx: &LateContext<'_>, func: &Expr<'_>) -> Option<Symbol> {
+    if let ExprKind::Path(QPath::TypeRelative(cstr, fn_name)) = &func.kind
+        && let TyKind::Path(QPath::Resolved(_, ty_path)) = &cstr.kind
+        && cx.tcx.lang_items().c_str() == ty_path.res.opt_def_id()
+    {
+        Some(fn_name.ident.name)
+    } else {
+        None
+    }
+}
+
+/// Checks calls to the `CStr` constructor functions:
+/// - `CStr::from_bytes_with_nul(..)`
+/// - `CStr::from_bytes_with_nul_unchecked(..)`
+/// - `CStr::from_ptr(..)`
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args: &[Expr<'_>], msrv: &Msrv) {
+    if let Some(fn_name) = is_c_str_function(cx, func)
+        && let [arg] = args
+        && msrv.meets(msrvs::C_STR_LITERALS)
+    {
+        match fn_name.as_str() {
+            name @ ("from_bytes_with_nul" | "from_bytes_with_nul_unchecked")
+                if !arg.span.from_expansion()
+                    && let ExprKind::Lit(lit) = arg.kind
+                    && let LitKind::ByteStr(_, StrStyle::Cooked) | LitKind::Str(_, StrStyle::Cooked) = lit.node =>
+            {
+                check_from_bytes(cx, expr, arg, name);
+            },
+            "from_ptr" => check_from_ptr(cx, expr, arg),
+            _ => {},
+        }
+    }
+}
+
+/// Checks `CStr::from_ptr(b"foo\0".as_ptr().cast())`
+fn check_from_ptr(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>) {
+    if let ExprKind::MethodCall(method, lit, ..) = peel_ptr_cast(arg).kind
+        && method.ident.name == sym::as_ptr
+        && !lit.span.from_expansion()
+        && let ExprKind::Lit(lit) = lit.kind
+        && let LitKind::ByteStr(_, StrStyle::Cooked) = lit.node
+        && let Some(sugg) = rewrite_as_cstr(cx, lit.span)
+    {
+        span_lint_and_sugg(
+            cx,
+            MANUAL_C_STR_LITERALS,
+            expr.span,
+            "calling `CStr::from_ptr` with a byte string literal",
+            r#"use a `c""` literal"#,
+            sugg,
+            Applicability::MachineApplicable,
+        );
+    }
+}
+/// Checks `CStr::from_bytes_with_nul(b"foo\0")`
+fn check_from_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, method: &str) {
+    let (span, applicability) = if let Some(parent) = get_parent_expr(cx, expr)
+        && let ExprKind::MethodCall(method, ..) = parent.kind
+        && [sym::unwrap, sym::expect].contains(&method.ident.name)
+    {
+        (parent.span, Applicability::MachineApplicable)
+    } else if method == "from_bytes_with_nul_unchecked" {
+        // `*_unchecked` returns `&CStr` directly, nothing needs to be changed
+        (expr.span, Applicability::MachineApplicable)
+    } else {
+        // User needs to remove error handling, can't be machine applicable
+        (expr.span, Applicability::HasPlaceholders)
+    };
+
+    let Some(sugg) = rewrite_as_cstr(cx, arg.span) else {
+        return;
+    };
+
+    span_lint_and_sugg(
+        cx,
+        MANUAL_C_STR_LITERALS,
+        span,
+        "calling `CStr::new` with a byte string literal",
+        r#"use a `c""` literal"#,
+        sugg,
+        applicability,
+    );
+}
+
+/// Rewrites a byte string literal to a c-str literal.
+/// `b"foo\0"` -> `c"foo"`
+///
+/// Returns `None` if it doesn't end in a NUL byte.
+fn rewrite_as_cstr(cx: &LateContext<'_>, span: Span) -> Option<String> {
+    let mut sugg = String::from("c") + snippet(cx, span.source_callsite(), "..").trim_start_matches('b');
+
+    // NUL byte should always be right before the closing quote.
+    if let Some(quote_pos) = sugg.rfind('"') {
+        // Possible values right before the quote:
+        // - literal NUL value
+        if sugg.as_bytes()[quote_pos - 1] == b'\0' {
+            sugg.remove(quote_pos - 1);
+        }
+        // - \x00
+        else if sugg[..quote_pos].ends_with("\\x00") {
+            sugg.replace_range(quote_pos - 4..quote_pos, "");
+        }
+        // - \0
+        else if sugg[..quote_pos].ends_with("\\0") {
+            sugg.replace_range(quote_pos - 2..quote_pos, "");
+        }
+        // No known suffix, so assume it's not a C-string.
+        else {
+            return None;
+        }
+    }
+
+    Some(sugg)
+}
+
+fn get_cast_target<'tcx>(e: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    match &e.kind {
+        ExprKind::MethodCall(method, receiver, [], _) if method.ident.as_str() == "cast" => Some(receiver),
+        ExprKind::Cast(expr, _) => Some(expr),
+        _ => None,
+    }
+}
+
+/// `x.cast()` -> `x`
+/// `x as *const _` -> `x`
+/// `x` -> `x` (returns the same expression for non-cast exprs)
+fn peel_ptr_cast<'tcx>(e: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
+    get_cast_target(e).map_or(e, peel_ptr_cast)
+}
+
+/// Same as `peel_ptr_cast`, but the other way around, by walking up the ancestor cast expressions:
+///
+/// `foo(x.cast() as *const _)`
+///      ^ given this `x` expression, returns the `foo(...)` expression
+fn peel_ptr_cast_ancestors<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
+    let mut prev = e;
+    for (_, node) in cx.tcx.hir().parent_iter(e.hir_id) {
+        if let Node::Expr(e) = node
+            && get_cast_target(e).is_some()
+        {
+            prev = e;
+        } else {
+            break;
+        }
+    }
+    prev
+}

clippy_lints/src/methods/mod.rs

@@ -51,6 +51,7 @@ mod iter_skip_zero;
 mod iter_with_drain;
 mod iterator_step_by_zero;
 mod join_absolute_paths;
+mod manual_c_str_literals;
 mod manual_is_variant_and;
 mod manual_next_back;
 mod manual_ok_or;
@@ -3977,6 +3978,39 @@ declare_clippy_lint! {
     "making no use of the \"map closure\" when calling `.map_or_else(|err| handle_error(err), |n| n)`"
 }
 
+declare_clippy_lint! {
+    /// Checks for the manual creation of C strings (a string with a `NUL` byte at the end), either
+    /// through one of the `CStr` constructor functions, or more plainly by calling `.as_ptr()`
+    /// on a (byte) string literal with a hardcoded `\0` byte at the end.
+    ///
+    /// ### Why is this bad?
+    /// This can be written more concisely using `c"str"` literals and is also less error-prone,
+    /// because the compiler checks for interior `NUL` bytes and the terminating `NUL` byte is inserted automatically.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # use std::ffi::CStr;
+    /// # mod libc { pub unsafe fn puts(_: *const i8) {} }
+    /// fn needs_cstr(_: &CStr) {}
+    ///
+    /// needs_cstr(CStr::from_bytes_with_nul(b"Hello\0").unwrap());
+    /// unsafe { libc::puts("World\0".as_ptr().cast()) }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # use std::ffi::CStr;
+    /// # mod libc { pub unsafe fn puts(_: *const i8) {} }
+    /// fn needs_cstr(_: &CStr) {}
+    ///
+    /// needs_cstr(c"Hello");
+    /// unsafe { libc::puts(c"World".as_ptr()) }
+    /// ```
+    #[clippy::version = "1.76.0"]
+    pub MANUAL_C_STR_LITERALS,
+    pedantic,
+    r#"creating a `CStr` through functions when `c""` literals can be used"#
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4136,6 +4170,7 @@ impl_lint_pass!(Methods => [
     STR_SPLIT_AT_NEWLINE,
     OPTION_AS_REF_CLONED,
     UNNECESSARY_RESULT_MAP_OR_ELSE,
+    MANUAL_C_STR_LITERALS,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4163,6 +4198,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             hir::ExprKind::Call(func, args) => {
                 from_iter_instead_of_collect::check(cx, expr, args, func);
                 unnecessary_fallible_conversions::check_function(cx, expr, func);
+                manual_c_str_literals::check(cx, expr, func, args, &self.msrv);
             },
             hir::ExprKind::MethodCall(method_call, receiver, args, _) => {
                 let method_span = method_call.ident.span;
@@ -4381,6 +4417,7 @@ impl Methods {
                     }
                 },
                 ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv),
+                ("as_ptr", []) => manual_c_str_literals::check_as_ptr(cx, expr, recv, &self.msrv),
                 ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
                 ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
                 ("cloned", []) => {

tests/ui/manual_c_str_literals.fixed

@@ -0,0 +1,60 @@
+#![warn(clippy::manual_c_str_literals)]
+#![allow(clippy::no_effect)]
+
+use std::ffi::CStr;
+
+macro_rules! cstr {
+    ($s:literal) => {
+        CStr::from_bytes_with_nul(concat!($s, "\0").as_bytes()).unwrap()
+    };
+}
+
+macro_rules! macro_returns_c_str {
+    () => {
+        CStr::from_bytes_with_nul(b"foo\0").unwrap();
+    };
+}
+
+macro_rules! macro_returns_byte_string {
+    () => {
+        b"foo\0"
+    };
+}
+
+#[clippy::msrv = "1.76.0"]
+fn pre_stabilization() {
+    CStr::from_bytes_with_nul(b"foo\0");
+}
+
+#[clippy::msrv = "1.77.0"]
+fn post_stabilization() {
+    c"foo";
+}
+
+fn main() {
+    c"foo";
+    c"foo";
+    c"foo";
+    c"foo\\0sdsd";
+    CStr::from_bytes_with_nul(br"foo\\0sdsd\0").unwrap();
+    CStr::from_bytes_with_nul(br"foo\x00").unwrap();
+    CStr::from_bytes_with_nul(br##"foo#a\0"##).unwrap();
+
+    unsafe { c"foo" };
+    unsafe { c"foo" };
+    let _: *const _ = c"foo".as_ptr();
+    let _: *const _ = c"foo".as_ptr();
+    let _: *const _ = "foo".as_ptr(); // not a C-string
+    let _: *const _ = "".as_ptr();
+    let _: *const _ = c"foo".as_ptr().cast::<i8>();
+    let _ = "电脑".as_ptr();
+    let _ = "电脑\\".as_ptr();
+    let _ = c"电脑\\".as_ptr();
+    let _ = c"电脑".as_ptr();
+    let _ = c"电脑".as_ptr();
+
+    // Macro cases, don't lint:
+    cstr!("foo");
+    macro_returns_c_str!();
+    CStr::from_bytes_with_nul(macro_returns_byte_string!()).unwrap();
+}