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
This commit is contained in:
parent
457e7f12e9
commit
c209fc9349
@ -92,7 +92,14 @@ impl LintPass for StringAdd {
|
|||||||
|
|
||||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
|
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
|
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
|
||||||
if let ExprKind::Binary(Spanned { node: BinOpKind::Add, .. }, ref left, _) = e.node {
|
if let ExprKind::Binary(
|
||||||
|
Spanned {
|
||||||
|
node: BinOpKind::Add, ..
|
||||||
|
},
|
||||||
|
ref left,
|
||||||
|
_,
|
||||||
|
) = e.node
|
||||||
|
{
|
||||||
if is_string(cx, left) {
|
if is_string(cx, left) {
|
||||||
if !is_allowed(cx, STRING_ADD_ASSIGN, e.id) {
|
if !is_allowed(cx, STRING_ADD_ASSIGN, e.id) {
|
||||||
let parent = get_parent_expr(cx, e);
|
let parent = get_parent_expr(cx, e);
|
||||||
@ -132,13 +139,15 @@ fn is_string(cx: &LateContext<'_, '_>, e: &Expr) -> bool {
|
|||||||
|
|
||||||
fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
|
fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
|
||||||
match src.node {
|
match src.node {
|
||||||
ExprKind::Binary(Spanned { node: BinOpKind::Add, .. }, ref left, _) => SpanlessEq::new(cx).eq_expr(target, left),
|
ExprKind::Binary(
|
||||||
|
Spanned {
|
||||||
|
node: BinOpKind::Add, ..
|
||||||
|
},
|
||||||
|
ref left,
|
||||||
|
_,
|
||||||
|
) => SpanlessEq::new(cx).eq_expr(target, left),
|
||||||
ExprKind::Block(ref block, _) => {
|
ExprKind::Block(ref block, _) => {
|
||||||
block.stmts.is_empty()
|
block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target))
|
||||||
&& block
|
|
||||||
.expr
|
|
||||||
.as_ref()
|
|
||||||
.map_or(false, |expr| is_add(cx, expr, target))
|
|
||||||
},
|
},
|
||||||
_ => false,
|
_ => false,
|
||||||
}
|
}
|
||||||
@ -162,7 +171,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes {
|
|||||||
if path.ident.name == "as_bytes" {
|
if path.ident.name == "as_bytes" {
|
||||||
if let ExprKind::Lit(ref lit) = args[0].node {
|
if let ExprKind::Lit(ref lit) = args[0].node {
|
||||||
if let LitKind::Str(ref lit_content, _) = lit.node {
|
if let LitKind::Str(ref lit_content, _) = lit.node {
|
||||||
if lit_content.as_str().chars().all(|c| c.is_ascii()) && !in_macro(args[0].span) {
|
let callsite = snippet(cx, args[0].span.source_callsite(), "");
|
||||||
|
let expanded = format!("\"{}\"", lit_content.as_str());
|
||||||
|
if callsite.starts_with("include_str!") {
|
||||||
|
span_lint_and_sugg(
|
||||||
|
cx,
|
||||||
|
STRING_LIT_AS_BYTES,
|
||||||
|
e.span,
|
||||||
|
"calling `as_bytes()` on `include_str!(..)`",
|
||||||
|
"consider using `include_bytes!(..)` instead",
|
||||||
|
snippet(cx, args[0].span, r#""foo""#).replacen("include_str", "include_bytes", 1),
|
||||||
|
);
|
||||||
|
} else if callsite == expanded
|
||||||
|
&& lit_content.as_str().chars().all(|c| c.is_ascii())
|
||||||
|
&& !in_macro(args[0].span)
|
||||||
|
{
|
||||||
span_lint_and_sugg(
|
span_lint_and_sugg(
|
||||||
cx,
|
cx,
|
||||||
STRING_LIT_AS_BYTES,
|
STRING_LIT_AS_BYTES,
|
||||||
|
@ -10,10 +10,10 @@
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
#[warn(clippy::string_add)]
|
#[warn(clippy::string_add)]
|
||||||
#[allow(clippy::string_add_assign)]
|
#[allow(clippy::string_add_assign)]
|
||||||
fn add_only() { // ignores assignment distinction
|
fn add_only() {
|
||||||
|
// ignores assignment distinction
|
||||||
let mut x = "".to_owned();
|
let mut x = "".to_owned();
|
||||||
|
|
||||||
for _ in 1..3 {
|
for _ in 1..3 {
|
||||||
@ -63,6 +63,8 @@ fn str_lit_as_bytes() {
|
|||||||
let ubs = "☃".as_bytes();
|
let ubs = "☃".as_bytes();
|
||||||
|
|
||||||
let strify = stringify!(foobar).as_bytes();
|
let strify = stringify!(foobar).as_bytes();
|
||||||
|
|
||||||
|
let includestr = include_str!("entry.rs").as_bytes();
|
||||||
}
|
}
|
||||||
|
|
||||||
fn main() {
|
fn main() {
|
||||||
@ -72,6 +74,6 @@ fn main() {
|
|||||||
|
|
||||||
// the add is only caught for `String`
|
// the add is only caught for `String`
|
||||||
let mut x = 1;
|
let mut x = 1;
|
||||||
; x = x + 1;
|
; x = x + 1;
|
||||||
assert_eq!(2, x);
|
assert_eq!(2, x);
|
||||||
}
|
}
|
||||||
|
@ -60,17 +60,5 @@ error: calling `as_bytes()` on a string literal
|
|||||||
|
|
|
|
||||||
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`
|
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`
|
||||||
|
|
||||||
error: calling `as_bytes()` on a string literal
|
|
||||||
--> $DIR/strings.rs:65:18
|
|
||||||
|
|
|
||||||
65 | let strify = stringify!(foobar).as_bytes();
|
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `bstringify!(foobar)`
|
|
||||||
|
|
||||||
error: manual implementation of an assign operation
|
|
||||||
--> $DIR/strings.rs:75:7
|
|
||||||
|
|
|
||||||
75 | ; x = x + 1;
|
|
||||||
| ^^^^^^^^^ help: replace it with: `x += 1`
|
|
||||||
|
|
||||||
error: aborting due to 11 previous errors
|
error: aborting due to 11 previous errors
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user