Skip to content

Commit c0adb72

Browse files
Extend byte_char_slices to cover arrays (#16770)
Closes rust-lang/rust-clippy#16759 changelog: [`byte_char_slices`] enhance to cover arrays
2 parents effe235 + d0c0f3a commit c0adb72

8 files changed

Lines changed: 171 additions & 50 deletions

File tree

Lines changed: 62 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use rustc_ast::ast::{BorrowKind, Expr, ExprKind, Mutability};
3-
use rustc_ast::token::{Lit, LitKind};
1+
use std::borrow::Cow;
2+
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::source::snippet_with_applicability;
5+
use clippy_utils::sugg::Sugg;
6+
use clippy_utils::{get_parent_expr, span_contains_cfg, span_contains_comment};
7+
use rustc_ast::LitKind;
48
use rustc_errors::Applicability;
5-
use rustc_lint::{EarlyContext, EarlyLintPass};
9+
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability};
10+
use rustc_lint::{LateContext, LateLintPass};
611
use rustc_session::declare_lint_pass;
12+
use rustc_span::Span;
713

814
declare_clippy_lint! {
915
/// ### What it does
@@ -30,47 +36,73 @@ declare_clippy_lint! {
3036

3137
declare_lint_pass!(ByteCharSlice => [BYTE_CHAR_SLICES]);
3238

33-
impl EarlyLintPass for ByteCharSlice {
34-
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
39+
impl<'tcx> LateLintPass<'tcx> for ByteCharSlice {
40+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
3541
if !expr.span.from_expansion()
36-
&& let Some(slice) = is_byte_char_slices(expr)
42+
&& let Some((has_ref, slice)) = is_byte_char_slices(cx, expr)
3743
{
38-
span_lint_and_sugg(
44+
span_lint_and_then(
3945
cx,
4046
BYTE_CHAR_SLICES,
4147
expr.span,
4248
"can be more succinctly written as a byte str",
43-
"try",
44-
format!("b\"{slice}\""),
45-
Applicability::MachineApplicable,
49+
|diag| {
50+
let mut app = Applicability::MachineApplicable;
51+
let mut sugg = Sugg::hir_from_snippet(cx, expr, |_| {
52+
let mut slice = slice.iter().fold("b\"".to_owned(), |mut acc, span| {
53+
let snippet = snippet_with_applicability(cx, *span, "b'?'", &mut app);
54+
acc.push_str(match &snippet[2..snippet.len() - 1] {
55+
"\"" => "\\\"",
56+
"\\'" => "'",
57+
other => other,
58+
});
59+
acc
60+
});
61+
slice.push('"');
62+
Cow::Owned(slice)
63+
});
64+
if !has_ref && !cx.typeck_results().expr_ty_adjusted(expr).is_array_slice() {
65+
sugg = sugg.deref();
66+
}
67+
68+
diag.span_suggestion(expr.span, "try", sugg, app);
69+
},
4670
);
4771
}
4872
}
4973
}
5074

5175
/// Checks whether the slice is that of byte chars, and if so, builds a byte-string out of it
52-
fn is_byte_char_slices(expr: &Expr) -> Option<String> {
53-
if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, expr) = &expr.kind
54-
&& let ExprKind::Array(members) = &expr.kind
76+
fn is_byte_char_slices<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(bool, Vec<Span>)> {
77+
let (has_ref, expr) = if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = expr.kind {
78+
(true, inner)
79+
} else if let Some(parent) = get_parent_expr(cx, expr) // Already checked by the parent expr.
80+
&& let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) = parent.kind
81+
{
82+
return None;
83+
} else {
84+
(false, expr)
85+
};
86+
87+
if let ExprKind::Array(members) = expr.kind
5588
&& !members.is_empty()
89+
&& !span_contains_comment(cx, expr.span)
90+
&& !span_contains_cfg(cx, expr.span)
5691
{
57-
members
92+
return members
5893
.iter()
59-
.map(|member| match &member.kind {
60-
ExprKind::Lit(Lit {
61-
kind: LitKind::Byte,
62-
symbol,
63-
..
64-
}) => Some(symbol.as_str()),
65-
_ => None,
66-
})
67-
.map(|maybe_quote| match maybe_quote {
68-
Some("\"") => Some("\\\""),
69-
Some("\\'") => Some("'"),
70-
other => other,
94+
.try_fold(Vec::new(), |mut acc, member| {
95+
if let ExprKind::Lit(lit) = member.kind
96+
&& let LitKind::Byte(_) = lit.node
97+
&& expr.span.eq_ctxt(member.span)
98+
{
99+
acc.push(lit.span);
100+
return Some(acc);
101+
}
102+
None
71103
})
72-
.collect::<Option<String>>()
73-
} else {
74-
None
104+
.map(|s| (has_ref, s));
75105
}
106+
107+
None
76108
}

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
515515
Box::new(|| Box::new(visibility::Visibility)),
516516
Box::new(|| Box::new(multiple_bound_locations::MultipleBoundLocations)),
517517
Box::new(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers)),
518-
Box::new(|| Box::new(byte_char_slices::ByteCharSlice)),
519518
Box::new(|| Box::new(cfg_not_test::CfgNotTest)),
520519
Box::new(|| Box::new(empty_line_after::EmptyLineAfter::new())),
521520
// add early passes here, used by `cargo dev new_lint`
@@ -867,6 +866,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
867866
Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)),
868867
Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))),
869868
Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))),
869+
Box::new(|_| Box::new(byte_char_slices::ByteCharSlice)),
870870
// add late passes here, used by `cargo dev new_lint`
871871
];
872872
store.late_passes.extend(late_lints);

clippy_lints/src/returns/needless_return.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ fn emit_return_lint(
259259
// Go backwards while encountering whitespace and extend the given Span to that point.
260260
fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span {
261261
if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) {
262-
let ws = [b' ', b'\t', b'\n'];
262+
let ws = *b" \t\n";
263263
if let Some(non_ws_pos) = prev_source.bytes().rposition(|c| !ws.contains(&c)) {
264264
let len = prev_source.len() - non_ws_pos - 1;
265265
return sp.with_lo(sp.lo() - BytePos::from_usize(len));

tests/ui/byte_char_slices.fixed

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::byte_char_slices)]
2+
#![allow(clippy::useless_vec)]
23

34
fn main() {
45
let bad = b"abc";
@@ -11,7 +12,40 @@ fn main() {
1112
//~^ byte_char_slices
1213

1314
let good = &[b'a', 0x42];
14-
let good = [b'a', b'a'];
15-
//~^ useless_vec
16-
let good: u8 = [b'a', b'c'].into_iter().sum();
15+
let good = vec![b'a', b'a'];
16+
}
17+
18+
fn takes_array_ref(_: &[u8; 2]) {}
19+
20+
fn takes_array_ref_ref(_: &&[u8; 2]) {}
21+
22+
fn issue16759(bytes: [u32; 3]) {
23+
const START: u32 = u32::from_le_bytes(*b"WORK");
24+
//~^ byte_char_slices
25+
26+
let auto_deref_to_slice: u8 = b"ac".iter().copied().sum();
27+
//~^ byte_char_slices
28+
29+
let with_comment = [
30+
// 1 2 3
31+
b'a', b'b', b'c', // x
32+
b'd', b'e', b'f', // 2x
33+
b'g', b'h', b'i', // 3x
34+
];
35+
let with_cfg = [
36+
b'a',
37+
b'b',
38+
b'c',
39+
#[cfg(feature = "foo")]
40+
b'd',
41+
];
42+
43+
let with_escape: u8 = b"'\"\x00\n\\".iter().copied().sum();
44+
//~^ byte_char_slices
45+
46+
takes_array_ref(b"ab");
47+
//~^ byte_char_slices
48+
49+
takes_array_ref_ref(&b"ab");
50+
//~^ byte_char_slices
1751
}

tests/ui/byte_char_slices.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::byte_char_slices)]
2+
#![allow(clippy::useless_vec)]
23

34
fn main() {
45
let bad = &[b'a', b'b', b'c'];
@@ -12,6 +13,39 @@ fn main() {
1213

1314
let good = &[b'a', 0x42];
1415
let good = vec![b'a', b'a'];
15-
//~^ useless_vec
16-
let good: u8 = [b'a', b'c'].into_iter().sum();
16+
}
17+
18+
fn takes_array_ref(_: &[u8; 2]) {}
19+
20+
fn takes_array_ref_ref(_: &&[u8; 2]) {}
21+
22+
fn issue16759(bytes: [u32; 3]) {
23+
const START: u32 = u32::from_le_bytes([b'W', b'O', b'R', b'K']);
24+
//~^ byte_char_slices
25+
26+
let auto_deref_to_slice: u8 = [b'a', b'c'].iter().copied().sum();
27+
//~^ byte_char_slices
28+
29+
let with_comment = [
30+
// 1 2 3
31+
b'a', b'b', b'c', // x
32+
b'd', b'e', b'f', // 2x
33+
b'g', b'h', b'i', // 3x
34+
];
35+
let with_cfg = [
36+
b'a',
37+
b'b',
38+
b'c',
39+
#[cfg(feature = "foo")]
40+
b'd',
41+
];
42+
43+
let with_escape: u8 = [b'\'', b'"', b'\x00', b'\n', b'\\'].iter().copied().sum();
44+
//~^ byte_char_slices
45+
46+
takes_array_ref(&[b'a', b'b']);
47+
//~^ byte_char_slices
48+
49+
takes_array_ref_ref(&&[b'a', b'b']);
50+
//~^ byte_char_slices
1751
}

tests/ui/byte_char_slices.stderr

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: can be more succinctly written as a byte str
2-
--> tests/ui/byte_char_slices.rs:4:15
2+
--> tests/ui/byte_char_slices.rs:5:15
33
|
44
LL | let bad = &[b'a', b'b', b'c'];
55
| ^^^^^^^^^^^^^^^^^^^ help: try: `b"abc"`
@@ -8,31 +8,52 @@ LL | let bad = &[b'a', b'b', b'c'];
88
= help: to override `-D warnings` add `#[allow(clippy::byte_char_slices)]`
99

1010
error: can be more succinctly written as a byte str
11-
--> tests/ui/byte_char_slices.rs:6:18
11+
--> tests/ui/byte_char_slices.rs:7:18
1212
|
1313
LL | let quotes = &[b'"', b'H', b'i'];
1414
| ^^^^^^^^^^^^^^^^^^^ help: try: `b"\"Hi"`
1515

1616
error: can be more succinctly written as a byte str
17-
--> tests/ui/byte_char_slices.rs:8:18
17+
--> tests/ui/byte_char_slices.rs:9:18
1818
|
1919
LL | let quotes = &[b'\'', b'S', b'u', b'p'];
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b"'Sup"`
2121

2222
error: can be more succinctly written as a byte str
23-
--> tests/ui/byte_char_slices.rs:10:19
23+
--> tests/ui/byte_char_slices.rs:11:19
2424
|
2525
LL | let escapes = &[b'\x42', b'E', b's', b'c'];
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b"\x42Esc"`
2727

28-
error: useless use of `vec!`
29-
--> tests/ui/byte_char_slices.rs:14:16
28+
error: can be more succinctly written as a byte str
29+
--> tests/ui/byte_char_slices.rs:23:43
30+
|
31+
LL | const START: u32 = u32::from_le_bytes([b'W', b'O', b'R', b'K']);
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `*b"WORK"`
33+
34+
error: can be more succinctly written as a byte str
35+
--> tests/ui/byte_char_slices.rs:26:35
36+
|
37+
LL | let auto_deref_to_slice: u8 = [b'a', b'c'].iter().copied().sum();
38+
| ^^^^^^^^^^^^ help: try: `b"ac"`
39+
40+
error: can be more succinctly written as a byte str
41+
--> tests/ui/byte_char_slices.rs:43:27
42+
|
43+
LL | let with_escape: u8 = [b'\'', b'"', b'\x00', b'\n', b'\\'].iter().copied().sum();
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b"'\"\x00\n\\"`
45+
46+
error: can be more succinctly written as a byte str
47+
--> tests/ui/byte_char_slices.rs:46:21
3048
|
31-
LL | let good = vec![b'a', b'a'];
32-
| ^^^^^^^^^^^^^^^^ help: you can use an array directly: `[b'a', b'a']`
49+
LL | takes_array_ref(&[b'a', b'b']);
50+
| ^^^^^^^^^^^^^ help: try: `b"ab"`
51+
52+
error: can be more succinctly written as a byte str
53+
--> tests/ui/byte_char_slices.rs:49:26
3354
|
34-
= note: `-D clippy::useless-vec` implied by `-D warnings`
35-
= help: to override `-D warnings` add `#[allow(clippy::useless_vec)]`
55+
LL | takes_array_ref_ref(&&[b'a', b'b']);
56+
| ^^^^^^^^^^^^^ help: try: `b"ab"`
3657

37-
error: aborting due to 5 previous errors
58+
error: aborting due to 9 previous errors
3859

tests/ui/ptr_offset_by_literal.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::ptr_offset_by_literal)]
2-
#![allow(clippy::inconsistent_digit_grouping)]
2+
#![allow(clippy::inconsistent_digit_grouping, clippy::byte_char_slices)]
33

44
fn main() {
55
let arr = [b'a', b'b', b'c'];

tests/ui/ptr_offset_by_literal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::ptr_offset_by_literal)]
2-
#![allow(clippy::inconsistent_digit_grouping)]
2+
#![allow(clippy::inconsistent_digit_grouping, clippy::byte_char_slices)]
33

44
fn main() {
55
let arr = [b'a', b'b', b'c'];

0 commit comments

Comments
 (0)