Skip to content

Commit 167fef7

Browse files
committed
Fix string_lit_as_bytes lint for macros
Prior to this change, string_lit_as_bytes would trigger for constructs like `include_str!("filename").as_bytes()` and would recommend fixing it by rewriting as `binclude_str!("filename")`. This change updates the lint to act as an EarlyLintPass lint. It then differentiates between string literals and macros that have bytes yielding alternatives. Closes #3205
1 parent 417cf20 commit 167fef7

File tree

7 files changed

+105
-62
lines changed

7 files changed

+105
-62
lines changed

clippy_lints/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ pub mod returns;
174174
pub mod serde_api;
175175
pub mod shadow;
176176
pub mod strings;
177+
pub mod strings_early;
177178
pub mod suspicious_trait_impl;
178179
pub mod swap;
179180
pub mod temporary_assignment;
@@ -200,6 +201,7 @@ mod reexport {
200201
pub fn register_pre_expansion_lints(session: &rustc::session::Session, store: &mut rustc::lint::LintStore, conf: &Conf) {
201202
store.register_pre_expansion_pass(Some(session), box write::Pass);
202203
store.register_pre_expansion_pass(Some(session), box redundant_field_names::RedundantFieldNames);
204+
store.register_pre_expansion_pass(Some(session), box strings_early::StringLitAsBytes);
203205
store.register_pre_expansion_pass(Some(session), box non_expressive_names::NonExpressiveNames {
204206
single_char_binding_names_threshold: conf.single_char_binding_names_threshold,
205207
});
@@ -355,7 +357,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
355357
reg.register_late_lint_pass(box escape::Pass{too_large_for_stack: conf.too_large_for_stack});
356358
reg.register_early_lint_pass(box misc_early::MiscEarly);
357359
reg.register_late_lint_pass(box panic_unimplemented::Pass);
358-
reg.register_late_lint_pass(box strings::StringLitAsBytes);
359360
reg.register_late_lint_pass(box derive::Derive);
360361
reg.register_late_lint_pass(box types::CharLitAsU8);
361362
reg.register_late_lint_pass(box vec::Pass);

clippy_lints/src/strings.rs

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
33
use crate::rustc::{declare_tool_lint, lint_array};
44
use crate::syntax::source_map::Spanned;
55
use crate::utils::SpanlessEq;
6-
use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty};
6+
use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, walk_ptrs_ty};
77

88
/// **What it does:** Checks for string appends of the form `x = x + y` (without
99
/// `let`!).
@@ -133,38 +133,3 @@ fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
133133
_ => false,
134134
}
135135
}
136-
137-
#[derive(Copy, Clone)]
138-
pub struct StringLitAsBytes;
139-
140-
impl LintPass for StringLitAsBytes {
141-
fn get_lints(&self) -> LintArray {
142-
lint_array!(STRING_LIT_AS_BYTES)
143-
}
144-
}
145-
146-
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes {
147-
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
148-
use crate::syntax::ast::LitKind;
149-
use crate::utils::{in_macro, snippet};
150-
151-
if let ExprKind::MethodCall(ref path, _, ref args) = e.node {
152-
if path.ident.name == "as_bytes" {
153-
if let ExprKind::Lit(ref lit) = args[0].node {
154-
if let LitKind::Str(ref lit_content, _) = lit.node {
155-
if lit_content.as_str().chars().all(|c| c.is_ascii()) && !in_macro(args[0].span) {
156-
span_lint_and_sugg(
157-
cx,
158-
STRING_LIT_AS_BYTES,
159-
e.span,
160-
"calling `as_bytes()` on a string literal",
161-
"consider using a byte string literal instead",
162-
format!("b{}", snippet(cx, args[0].span, r#""foo""#)),
163-
);
164-
}
165-
}
166-
}
167-
}
168-
}
169-
}
170-
}

clippy_lints/src/strings_early.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
2+
use crate::rustc::{declare_tool_lint, lint_array};
3+
use crate::utils::span_lint_and_sugg;
4+
5+
/// **What it does:** Checks for the `as_bytes` method called on string literals
6+
/// that contain only ASCII characters.
7+
///
8+
/// **Why is this bad?** Byte string literals (e.g. `b"foo"`) can be used
9+
/// instead. They are shorter but less discoverable than `as_bytes()`.
10+
///
11+
/// **Known Problems:** None.
12+
///
13+
/// **Example:**
14+
/// ```rust
15+
/// let bs = "a byte string".as_bytes();
16+
/// ```
17+
declare_clippy_lint! {
18+
pub STRING_LIT_AS_BYTES,
19+
style,
20+
"calling `as_bytes` on a string literal instead of using a byte string literal"
21+
}
22+
23+
#[derive(Copy, Clone)]
24+
pub struct StringLitAsBytes;
25+
26+
impl LintPass for StringLitAsBytes {
27+
fn get_lints(&self) -> LintArray {
28+
lint_array!(STRING_LIT_AS_BYTES)
29+
}
30+
}
31+
32+
impl EarlyLintPass for StringLitAsBytes {
33+
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &crate::syntax::ast::Expr) {
34+
use crate::syntax::ast::{ExprKind, LitKind};
35+
use crate::utils::{in_macro, snippet};
36+
37+
if let ExprKind::MethodCall(ref path, ref args) = e.node {
38+
if path.ident.name == "as_bytes" {
39+
if let ExprKind::Lit(ref lit) = args[0].node {
40+
if let LitKind::Str(ref lit_content, _) = lit.node {
41+
if lit_content.as_str().chars().all(|c| c.is_ascii()) && !in_macro(args[0].span) {
42+
span_lint_and_sugg(
43+
cx,
44+
STRING_LIT_AS_BYTES,
45+
e.span,
46+
"calling `as_bytes()` on a string literal",
47+
"consider using a byte string literal instead",
48+
format!("b{}", snippet(cx, args[0].span, r#""foo""#)),
49+
);
50+
}
51+
}
52+
} else if let ExprKind::Mac(ref mac) = args[0].node {
53+
if mac.node.path == "include_str" {
54+
cx.sess.err(&format!("{:?}", mac));
55+
span_lint_and_sugg(
56+
cx,
57+
STRING_LIT_AS_BYTES,
58+
e.span,
59+
"calling `as_bytes()` on a include_str macro",
60+
"consider using include_bytes instead",
61+
format!("include_str{}", snippet(cx, mac.span, "include_bytes")),
62+
);
63+
}
64+
}
65+
}
66+
}
67+
}
68+
}

tests/ui/strings.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,6 @@ fn both() {
4444
assert_eq!(&x, &z);
4545
}
4646

47-
#[allow(dead_code, unused_variables)]
48-
#[warn(clippy::string_lit_as_bytes)]
49-
fn str_lit_as_bytes() {
50-
let bs = "hello there".as_bytes();
51-
52-
// no warning, because this cannot be written as a byte string literal:
53-
let ubs = "☃".as_bytes();
54-
55-
let strify = stringify!(foobar).as_bytes();
56-
}
57-
5847
fn main() {
5948
add_only();
6049
add_assign_only();

tests/ui/strings.stderr

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,6 @@ error: you added something to a string. Consider using `String::push_str()` inst
5252
42 | let z = y + "...";
5353
| ^^^^^^^^^
5454

55-
error: calling `as_bytes()` on a string literal
56-
--> $DIR/strings.rs:50:14
57-
|
58-
50 | let bs = "hello there".as_bytes();
59-
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"hello there"`
60-
|
61-
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`
62-
63-
error: calling `as_bytes()` on a string literal
64-
--> $DIR/strings.rs:55:18
65-
|
66-
55 | let strify = stringify!(foobar).as_bytes();
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `bstringify!(foobar)`
68-
6955
error: manual implementation of an assign operation
7056
--> $DIR/strings.rs:65:7
7157
|

tests/ui/strings_early.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![feature(tool_lints)]
2+
3+
#[allow(dead_code, unused_variables)]
4+
#[warn(clippy::string_lit_as_bytes)]
5+
fn str_lit_as_bytes() {
6+
let bs = "hello there".as_bytes();
7+
8+
// no warning, because this cannot be written as a byte string literal:
9+
let ubs = "☃".as_bytes();
10+
11+
let strify = stringify!(foobar).as_bytes();
12+
13+
let includestr = include_str!("entry.rs").as_bytes();
14+
}
15+
16+
fn main() {
17+
assert_eq!(2, 2);
18+
}

tests/ui/strings_early.stderr

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: calling `as_bytes()` on a string literal
2+
--> $DIR/strings_early.rs:6:14
3+
|
4+
6 | let bs = "hello there".as_bytes();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"hello there"`
6+
|
7+
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`
8+
9+
error: calling `as_bytes()` on a string literal
10+
--> $DIR/strings_early.rs:11:18
11+
|
12+
11 | let strify = stringify!(foobar).as_bytes();
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `bstringify!(foobar)`
14+
15+
error: aborting due to 2 previous errors
16+

0 commit comments

Comments
 (0)