Skip to content

Commit

Permalink
Fix string_lit_as_bytes lint for macros
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yaahc committed Sep 25, 2018
1 parent 417cf20 commit 40b03fb
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 62 deletions.
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ pub mod returns;
pub mod serde_api;
pub mod shadow;
pub mod strings;
pub mod strings_early;
pub mod suspicious_trait_impl;
pub mod swap;
pub mod temporary_assignment;
Expand Down Expand Up @@ -355,7 +356,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box escape::Pass{too_large_for_stack: conf.too_large_for_stack});
reg.register_early_lint_pass(box misc_early::MiscEarly);
reg.register_late_lint_pass(box panic_unimplemented::Pass);
reg.register_late_lint_pass(box strings::StringLitAsBytes);
reg.register_early_lint_pass(box strings_early::StringLitAsBytes);
reg.register_late_lint_pass(box derive::Derive);
reg.register_late_lint_pass(box types::CharLitAsU8);
reg.register_late_lint_pass(box vec::Pass);
Expand Down
37 changes: 1 addition & 36 deletions clippy_lints/src/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::syntax::source_map::Spanned;
use crate::utils::SpanlessEq;
use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty};
use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, walk_ptrs_ty};

/// **What it does:** Checks for string appends of the form `x = x + y` (without
/// `let`!).
Expand Down Expand Up @@ -133,38 +133,3 @@ fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
_ => false,
}
}

#[derive(Copy, Clone)]
pub struct StringLitAsBytes;

impl LintPass for StringLitAsBytes {
fn get_lints(&self) -> LintArray {
lint_array!(STRING_LIT_AS_BYTES)
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
use crate::syntax::ast::LitKind;
use crate::utils::{in_macro, snippet};

if let ExprKind::MethodCall(ref path, _, ref args) = e.node {
if path.ident.name == "as_bytes" {
if let ExprKind::Lit(ref lit) = args[0].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) {
span_lint_and_sugg(
cx,
STRING_LIT_AS_BYTES,
e.span,
"calling `as_bytes()` on a string literal",
"consider using a byte string literal instead",
format!("b{}", snippet(cx, args[0].span, r#""foo""#)),
);
}
}
}
}
}
}
}
60 changes: 60 additions & 0 deletions clippy_lints/src/strings_early.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::utils::span_lint_and_sugg;

/// **What it does:** Checks for the `as_bytes` method called on string literals
/// that contain only ASCII characters.
///
/// **Why is this bad?** Byte string literals (e.g. `b"foo"`) can be used
/// instead. They are shorter but less discoverable than `as_bytes()`.
///
/// **Known Problems:** None.
///
/// **Example:**
/// ```rust
/// let bs = "a byte string".as_bytes();
/// ```
declare_clippy_lint! {
pub STRING_LIT_AS_BYTES,
style,
"calling `as_bytes` on a string literal instead of using a byte string literal"
}

#[derive(Copy, Clone)]
pub struct StringLitAsBytes;

impl LintPass for StringLitAsBytes {
fn get_lints(&self) -> LintArray {
lint_array!(STRING_LIT_AS_BYTES)
}
}

impl EarlyLintPass for StringLitAsBytes {
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &crate::syntax::ast::Expr) {
use crate::syntax::ast::ExprKind;
use crate::syntax::ast::LitKind;
use crate::utils::{in_macro, snippet};

if let ExprKind::MethodCall(ref path, ref args) = e.node {
if path.ident.name == "as_bytes" {
cx.sess.err(&format!("{:?}", e.node));
if let ExprKind::Lit(ref lit) = args[0].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) {
span_lint_and_sugg(
cx,
STRING_LIT_AS_BYTES,
e.span,
"calling `as_bytes()` on a string literal",
"consider using a byte string literal instead",
format!("b{}", snippet(cx, args[0].span, r#""foo""#)),
);
}
}
} else if let ExprKind::Mac(ref mac) = args[0].node {
cx.sess.err(&format!("{:?}", mac))
}
}
}
}
}
11 changes: 0 additions & 11 deletions tests/ui/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,6 @@ fn both() {
assert_eq!(&x, &z);
}

#[allow(dead_code, unused_variables)]
#[warn(clippy::string_lit_as_bytes)]
fn str_lit_as_bytes() {
let bs = "hello there".as_bytes();

// no warning, because this cannot be written as a byte string literal:
let ubs = "☃".as_bytes();

let strify = stringify!(foobar).as_bytes();
}

fn main() {
add_only();
add_assign_only();
Expand Down
14 changes: 0 additions & 14 deletions tests/ui/strings.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,6 @@ error: you added something to a string. Consider using `String::push_str()` inst
42 | let z = y + "...";
| ^^^^^^^^^

error: calling `as_bytes()` on a string literal
--> $DIR/strings.rs:50:14
|
50 | let bs = "hello there".as_bytes();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"hello there"`
|
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`

error: calling `as_bytes()` on a string literal
--> $DIR/strings.rs:55:18
|
55 | 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:65:7
|
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/strings_early.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![feature(tool_lints)]

#[allow(dead_code, unused_variables)]
#[warn(clippy::string_lit_as_bytes)]
fn str_lit_as_bytes() {
let bs = "hello there".as_bytes();

// no warning, because this cannot be written as a byte string literal:
let ubs = "☃".as_bytes();

let strify = stringify!(foobar).as_bytes();

let includestr = include_str!("entry.rs").as_bytes();
}

fn main() {
assert_eq!(2, 2);
}
16 changes: 16 additions & 0 deletions tests/ui/strings_early.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: calling `as_bytes()` on a string literal
--> $DIR/strings_early.rs:6:14
|
6 | let bs = "hello there".as_bytes();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"hello there"`
|
= note: `-D clippy::string-lit-as-bytes` implied by `-D warnings`

error: calling `as_bytes()` on a string literal
--> $DIR/strings_early.rs:11:18
|
11 | let strify = stringify!(foobar).as_bytes();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using a byte string literal instead: `bstringify!(foobar)`

error: aborting due to 2 previous errors

0 comments on commit 40b03fb

Please sign in to comment.