Skip to content

Commit f211037

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 5f9af5f commit f211037

File tree

3 files changed

+44
-19
lines changed

3 files changed

+44
-19
lines changed

clippy_lints/src/strings.rs

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::rustc::hir::*;
2-
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
2+
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
33
use crate::rustc::{declare_tool_lint, lint_array};
44
use crate::syntax::source_map::Spanned;
55
use crate::utils::SpanlessEq;
@@ -82,7 +82,14 @@ impl LintPass for StringAdd {
8282

8383
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
8484
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
85-
if let ExprKind::Binary(Spanned { node: BinOpKind::Add, .. }, ref left, _) = e.node {
85+
if let ExprKind::Binary(
86+
Spanned {
87+
node: BinOpKind::Add, ..
88+
},
89+
ref left,
90+
_,
91+
) = e.node
92+
{
8693
if is_string(cx, left) {
8794
if !is_allowed(cx, STRING_ADD_ASSIGN, e.id) {
8895
let parent = get_parent_expr(cx, e);
@@ -122,13 +129,15 @@ fn is_string(cx: &LateContext<'_, '_>, e: &Expr) -> bool {
122129

123130
fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
124131
match src.node {
125-
ExprKind::Binary(Spanned { node: BinOpKind::Add, .. }, ref left, _) => SpanlessEq::new(cx).eq_expr(target, left),
132+
ExprKind::Binary(
133+
Spanned {
134+
node: BinOpKind::Add, ..
135+
},
136+
ref left,
137+
_,
138+
) => SpanlessEq::new(cx).eq_expr(target, left),
126139
ExprKind::Block(ref block, _) => {
127-
block.stmts.is_empty()
128-
&& block
129-
.expr
130-
.as_ref()
131-
.map_or(false, |expr| is_add(cx, expr, target))
140+
block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target))
132141
},
133142
_ => false,
134143
}
@@ -152,7 +161,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes {
152161
if path.ident.name == "as_bytes" {
153162
if let ExprKind::Lit(ref lit) = args[0].node {
154163
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) {
164+
let callsite = snippet(cx, args[0].span.source_callsite(), "");
165+
let expanded = format!("\"{}\"", lit_content.as_str());
166+
if callsite.starts_with("include_str!") {
167+
span_lint_and_sugg(
168+
cx,
169+
STRING_LIT_AS_BYTES,
170+
e.span,
171+
"calling `as_bytes()` on `include_str!(..)`",
172+
"consider using `include_bytes!(..)` instead",
173+
snippet(cx, args[0].span, r#""foo""#).replacen("include_str", "include_bytes", 1),
174+
);
175+
} else if callsite == expanded
176+
&& lit_content.as_str().chars().all(|c| c.is_ascii())
177+
&& !in_macro(args[0].span)
178+
{
156179
span_lint_and_sugg(
157180
cx,
158181
STRING_LIT_AS_BYTES,

tests/ui/strings.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#![feature(tool_lints)]
22

3-
43
#[warn(clippy::string_add)]
54
#[allow(clippy::string_add_assign)]
6-
fn add_only() { // ignores assignment distinction
5+
fn add_only() {
6+
// ignores assignment distinction
77
let mut x = "".to_owned();
88

99
for _ in 1..3 {
@@ -53,6 +53,8 @@ fn str_lit_as_bytes() {
5353
let ubs = "☃".as_bytes();
5454

5555
let strify = stringify!(foobar).as_bytes();
56+
57+
let includestr = include_str!("entry.rs").as_bytes();
5658
}
5759

5860
fn main() {
@@ -62,6 +64,6 @@ fn main() {
6264

6365
// the add is only caught for `String`
6466
let mut x = 1;
65-
; x = x + 1;
67+
; x = x + 1;
6668
assert_eq!(2, x);
6769
}

tests/ui/strings.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,17 @@ error: calling `as_bytes()` on a string literal
6060
|
6161
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`
6262

63-
error: calling `as_bytes()` on a string literal
64-
--> $DIR/strings.rs:55:18
63+
error: calling `as_bytes()` on `include_str!(..)`
64+
--> $DIR/strings.rs:57:22
6565
|
66-
55 | let strify = stringify!(foobar).as_bytes();
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `bstringify!(foobar)`
66+
57 | let includestr = include_str!("entry.rs").as_bytes();
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry.rs")`
6868

6969
error: manual implementation of an assign operation
70-
--> $DIR/strings.rs:65:7
70+
--> $DIR/strings.rs:67:6
7171
|
72-
65 | ; x = x + 1;
73-
| ^^^^^^^^^ help: replace it with: `x += 1`
72+
67 | ; x = x + 1;
73+
| ^^^^^^^^^ help: replace it with: `x += 1`
7474

7575
error: aborting due to 11 previous errors
7676

0 commit comments

Comments
 (0)