Skip to content

Commit 4be054f

Browse files
committed
new lint: manual_c_str_literals
1 parent ee83760 commit 4be054f

10 files changed

+299
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5220,6 +5220,7 @@ Released 2018-09-13
52205220
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
52215221
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
52225222
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
5223+
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
52235224
[`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
52245225
[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
52255226
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map

clippy_config/src/msrvs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ macro_rules! msrv_aliases {
1616

1717
// names may refer to stabilized feature flags or library items
1818
msrv_aliases! {
19+
1,76,0 { C_STR_LITERALS }
1920
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
2021
1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN }
2122
1,68,0 { PATH_MAIN_SEPARATOR_STR }

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
379379
crate::methods::ITER_SKIP_ZERO_INFO,
380380
crate::methods::ITER_WITH_DRAIN_INFO,
381381
crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
382+
crate::methods::MANUAL_C_STR_LITERALS_INFO,
382383
crate::methods::MANUAL_FILTER_MAP_INFO,
383384
crate::methods::MANUAL_FIND_MAP_INFO,
384385
crate::methods::MANUAL_NEXT_BACK_INFO,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use clippy_config::msrvs::{self, Msrv};
2+
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::get_parent_expr;
4+
use clippy_utils::source::snippet;
5+
use rustc_ast::{LitKind, StrStyle};
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{Expr, ExprKind, QPath, TyKind};
8+
use rustc_lint::LateContext;
9+
use rustc_span::{sym, Span};
10+
11+
use super::MANUAL_C_STR_LITERALS;
12+
13+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args: &[Expr<'_>], msrv: &Msrv) {
14+
if let ExprKind::Path(QPath::TypeRelative(cstr, fn_name)) = &func.kind
15+
&& let TyKind::Path(QPath::Resolved(_, ty_path)) = &cstr.kind
16+
&& cx.tcx.lang_items().c_str() == ty_path.res.opt_def_id()
17+
&& let [arg] = args
18+
&& msrv.meets(msrvs::C_STR_LITERALS)
19+
{
20+
match fn_name.ident.name.as_str() {
21+
name @ ("from_bytes_with_nul" | "from_bytes_with_nul_unchecked")
22+
if !arg.span.from_expansion()
23+
&& let ExprKind::Lit(lit) = arg.kind
24+
&& let LitKind::ByteStr(_, StrStyle::Cooked) = lit.node =>
25+
{
26+
check_from_bytes(cx, expr, arg, name);
27+
},
28+
"from_ptr" => check_from_ptr(cx, expr, arg),
29+
_ => {},
30+
}
31+
}
32+
}
33+
34+
/// Checks `CStr::from_bytes_with_nul(b"foo\0")`
35+
fn check_from_ptr(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>) {
36+
if let ExprKind::MethodCall(method, lit, [], _) = peel_ptr_cast(arg).kind
37+
&& method.ident.name == sym::as_ptr
38+
&& !lit.span.from_expansion()
39+
&& let ExprKind::Lit(lit) = lit.kind
40+
&& let LitKind::ByteStr(_, StrStyle::Cooked) = lit.node
41+
{
42+
span_lint_and_sugg(
43+
cx,
44+
MANUAL_C_STR_LITERALS,
45+
expr.span,
46+
"calling `CStr::from_ptr` with a byte string literal",
47+
r#"use a `c""` literal"#,
48+
rewrite_as_cstr(cx, lit.span),
49+
Applicability::MachineApplicable,
50+
);
51+
}
52+
}
53+
54+
/// Checks `CStr::from_ptr(b"foo\0".as_ptr().cast())`
55+
fn check_from_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, method: &str) {
56+
let (span, applicability) = if let Some(parent) = get_parent_expr(cx, expr)
57+
&& let ExprKind::MethodCall(method, ..) = parent.kind
58+
&& [sym::unwrap, sym::expect].contains(&method.ident.name)
59+
{
60+
(parent.span, Applicability::MachineApplicable)
61+
} else if method == "from_bytes_with_nul_unchecked" {
62+
// `*_unchecked` returns `&CStr` directly, nothing needs to be changed
63+
(expr.span, Applicability::MachineApplicable)
64+
} else {
65+
// User needs to remove error handling, can't be machine applicable
66+
(expr.span, Applicability::MaybeIncorrect)
67+
};
68+
69+
span_lint_and_sugg(
70+
cx,
71+
MANUAL_C_STR_LITERALS,
72+
span,
73+
"calling `CStr::new` with a byte string literal",
74+
r#"use a `c""` literal"#,
75+
rewrite_as_cstr(cx, arg.span),
76+
applicability,
77+
);
78+
}
79+
80+
/// Rewrites a byte string literal to a c-str literal.
81+
/// `b"foo\0"` -> `c"foo"`
82+
pub fn rewrite_as_cstr(cx: &LateContext<'_>, span: Span) -> String {
83+
let mut sugg = String::from("c") + snippet(cx, span.source_callsite(), "..").trim_start_matches('b');
84+
85+
// NUL byte should always be right before the closing quote.
86+
// (Can rfind ever return `None`?)
87+
if let Some(quote_pos) = sugg.rfind('"') {
88+
// Possible values right before the quote:
89+
// - literal NUL value
90+
if sugg.as_bytes()[quote_pos - 1] == b'\0' {
91+
sugg.remove(quote_pos - 1);
92+
}
93+
// - \x00
94+
else if sugg[..quote_pos].ends_with("\\x00") {
95+
sugg.replace_range(quote_pos - 4..quote_pos, "");
96+
}
97+
// - \0
98+
else if sugg[..quote_pos].ends_with("\\0") {
99+
sugg.replace_range(quote_pos - 2..quote_pos, "");
100+
}
101+
}
102+
103+
sugg
104+
}
105+
106+
/// `x.cast()` -> `x`
107+
/// `x as *const _` -> `x`
108+
fn peel_ptr_cast<'tcx>(e: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
109+
match &e.kind {
110+
ExprKind::MethodCall(method, receiver, [], _) if method.ident.as_str() == "cast" => peel_ptr_cast(receiver),
111+
ExprKind::Cast(expr, _) => peel_ptr_cast(expr),
112+
_ => e,
113+
}
114+
}

clippy_lints/src/methods/mod.rs

+31
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ mod iter_skip_zero;
5050
mod iter_with_drain;
5151
mod iterator_step_by_zero;
5252
mod join_absolute_paths;
53+
mod manual_c_str_literals;
5354
mod manual_next_back;
5455
mod manual_ok_or;
5556
mod manual_saturating_arithmetic;
@@ -3752,6 +3753,33 @@ declare_clippy_lint! {
37523753
"using `Option.map_or(Err(_), Ok)`, which is more succinctly expressed as `Option.ok_or(_)`"
37533754
}
37543755

3756+
declare_clippy_lint! {
3757+
/// ### What it does
3758+
/// Checks for calls to `CString::from_ptr` and `CStr::from_bytes_with_nul` with byte string literals as arguments.
3759+
///
3760+
/// ### Why is this bad?
3761+
/// This can be written more concisely using `c"str"` literals and is also less error-prone,
3762+
/// because the compiler checks for interior nul bytes.
3763+
///
3764+
/// ### Example
3765+
/// ```no_run
3766+
/// # use std::ffi::CStr;
3767+
/// # fn needs_cstr(_: &CStr) {}
3768+
/// needs_cstr(CStr::from_bytes_with_nul(b":)").unwrap());
3769+
/// ```
3770+
/// Use instead:
3771+
/// ```no_run
3772+
/// # #![feature(c_str_literals)]
3773+
/// # use std::ffi::CStr;
3774+
/// # fn needs_cstr(_: &CStr) {}
3775+
/// needs_cstr(c":)");
3776+
/// ```
3777+
#[clippy::version = "1.76.0"]
3778+
pub MANUAL_C_STR_LITERALS,
3779+
pedantic,
3780+
r#"creating a `CStr` through functions when `c""` literals can be used"#
3781+
}
3782+
37553783
pub struct Methods {
37563784
avoid_breaking_exported_api: bool,
37573785
msrv: Msrv,
@@ -3903,6 +3931,7 @@ impl_lint_pass!(Methods => [
39033931
UNNECESSARY_FALLIBLE_CONVERSIONS,
39043932
JOIN_ABSOLUTE_PATHS,
39053933
OPTION_MAP_OR_ERR_OK,
3934+
MANUAL_C_STR_LITERALS,
39063935
]);
39073936

39083937
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3920,6 +3949,7 @@ fn method_call<'tcx>(
39203949

39213950
impl<'tcx> LateLintPass<'tcx> for Methods {
39223951
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
3952+
manual_c_str_literals::rewrite_as_cstr(cx, expr.span);
39233953
if expr.span.from_expansion() {
39243954
return;
39253955
}
@@ -3930,6 +3960,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
39303960
hir::ExprKind::Call(func, args) => {
39313961
from_iter_instead_of_collect::check(cx, expr, args, func);
39323962
unnecessary_fallible_conversions::check_function(cx, expr, func);
3963+
manual_c_str_literals::check(cx, expr, func, args, &self.msrv);
39333964
},
39343965
hir::ExprKind::MethodCall(method_call, receiver, args, _) => {
39353966
let method_span = method_call.ident.span;

tests/ui/manual_c_str_literals.fixed

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
#![feature(c_str_literals)] // TODO: remove in the next sync
2+
#![warn(clippy::manual_c_str_literals)]
3+
#![allow(clippy::no_effect)]
4+
5+
use std::ffi::CStr;
6+
7+
macro_rules! cstr {
8+
($s:literal) => {
9+
CStr::from_bytes_with_nul(concat!($s, "\0").as_bytes()).unwrap()
10+
};
11+
}
12+
13+
macro_rules! macro_returns_c_str {
14+
() => {
15+
CStr::from_bytes_with_nul(b"foo\0").unwrap();
16+
};
17+
}
18+
19+
macro_rules! macro_returns_byte_string {
20+
() => {
21+
b"foo\0"
22+
};
23+
}
24+
25+
#[clippy::msrv = "1.75.0"]
26+
fn pre_stabilization() {
27+
CStr::from_bytes_with_nul(b"foo\0");
28+
}
29+
30+
#[clippy::msrv = "1.76.0"]
31+
fn post_stabilization() {
32+
c"foo";
33+
}
34+
35+
fn main() {
36+
c"foo";
37+
c"foo";
38+
c"foo";
39+
c"foo\\0sdsd";
40+
CStr::from_bytes_with_nul(br"foo\\0sdsd\0").unwrap();
41+
CStr::from_bytes_with_nul(br"foo\x00").unwrap();
42+
CStr::from_bytes_with_nul(br##"foo#a\0"##).unwrap();
43+
44+
unsafe { c"foo" };
45+
unsafe { c"foo" };
46+
47+
// Macro cases, don't lint:
48+
cstr!("foo");
49+
macro_returns_c_str!();
50+
CStr::from_bytes_with_nul(macro_returns_byte_string!()).unwrap();
51+
}

tests/ui/manual_c_str_literals.rs

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
#![feature(c_str_literals)] // TODO: remove in the next sync
2+
#![warn(clippy::manual_c_str_literals)]
3+
#![allow(clippy::no_effect)]
4+
5+
use std::ffi::CStr;
6+
7+
macro_rules! cstr {
8+
($s:literal) => {
9+
CStr::from_bytes_with_nul(concat!($s, "\0").as_bytes()).unwrap()
10+
};
11+
}
12+
13+
macro_rules! macro_returns_c_str {
14+
() => {
15+
CStr::from_bytes_with_nul(b"foo\0").unwrap();
16+
};
17+
}
18+
19+
macro_rules! macro_returns_byte_string {
20+
() => {
21+
b"foo\0"
22+
};
23+
}
24+
25+
#[clippy::msrv = "1.75.0"]
26+
fn pre_stabilization() {
27+
CStr::from_bytes_with_nul(b"foo\0");
28+
}
29+
30+
#[clippy::msrv = "1.76.0"]
31+
fn post_stabilization() {
32+
CStr::from_bytes_with_nul(b"foo\0");
33+
}
34+
35+
fn main() {
36+
CStr::from_bytes_with_nul(b"foo\0");
37+
CStr::from_bytes_with_nul(b"foo\x00");
38+
CStr::from_bytes_with_nul(b"foo\0").unwrap();
39+
CStr::from_bytes_with_nul(b"foo\\0sdsd\0").unwrap();
40+
CStr::from_bytes_with_nul(br"foo\\0sdsd\0").unwrap();
41+
CStr::from_bytes_with_nul(br"foo\x00").unwrap();
42+
CStr::from_bytes_with_nul(br##"foo#a\0"##).unwrap();
43+
44+
unsafe { CStr::from_ptr(b"foo\0".as_ptr().cast()) };
45+
unsafe { CStr::from_ptr(b"foo\0".as_ptr() as *const _) };
46+
47+
// Macro cases, don't lint:
48+
cstr!("foo");
49+
macro_returns_c_str!();
50+
CStr::from_bytes_with_nul(macro_returns_byte_string!()).unwrap();
51+
}

tests/ui/manual_c_str_literals.stderr

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error: calling `CStr::new` with a byte string literal
2+
--> $DIR/manual_c_str_literals.rs:32:5
3+
|
4+
LL | CStr::from_bytes_with_nul(b"foo\0");
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
6+
|
7+
= note: `-D clippy::manual-c-str-literals` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]`
9+
10+
error: calling `CStr::new` with a byte string literal
11+
--> $DIR/manual_c_str_literals.rs:36:5
12+
|
13+
LL | CStr::from_bytes_with_nul(b"foo\0");
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
15+
16+
error: calling `CStr::new` with a byte string literal
17+
--> $DIR/manual_c_str_literals.rs:37:5
18+
|
19+
LL | CStr::from_bytes_with_nul(b"foo\x00");
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
21+
22+
error: calling `CStr::new` with a byte string literal
23+
--> $DIR/manual_c_str_literals.rs:38:5
24+
|
25+
LL | CStr::from_bytes_with_nul(b"foo\0").unwrap();
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
27+
28+
error: calling `CStr::new` with a byte string literal
29+
--> $DIR/manual_c_str_literals.rs:39:5
30+
|
31+
LL | CStr::from_bytes_with_nul(b"foo\\0sdsd\0").unwrap();
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo\\0sdsd"`
33+
34+
error: calling `CStr::from_ptr` with a byte string literal
35+
--> $DIR/manual_c_str_literals.rs:44:14
36+
|
37+
LL | unsafe { CStr::from_ptr(b"foo\0".as_ptr().cast()) };
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
39+
40+
error: calling `CStr::from_ptr` with a byte string literal
41+
--> $DIR/manual_c_str_literals.rs:45:14
42+
|
43+
LL | unsafe { CStr::from_ptr(b"foo\0".as_ptr() as *const _) };
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a `c""` literal: `c"foo"`
45+
46+
error: aborting due to 7 previous errors
47+

tests/ui/strlen_on_c_strings.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::strlen_on_c_strings)]
2-
#![allow(dead_code)]
2+
#![allow(dead_code, clippy::manual_c_str_literals)]
33
#![feature(rustc_private)]
44
extern crate libc;
55

tests/ui/strlen_on_c_strings.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::strlen_on_c_strings)]
2-
#![allow(dead_code)]
2+
#![allow(dead_code, clippy::manual_c_str_literals)]
33
#![feature(rustc_private)]
44
extern crate libc;
55

0 commit comments

Comments
 (0)