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

Author: yaahc

clippy_lints/src/lib.rs

@@ -200,6 +200,7 @@ mod reexport {
 pub fn register_pre_expansion_lints(session: &rustc::session::Session, store: &mut rustc::lint::LintStore, conf: &Conf) {
     store.register_pre_expansion_pass(Some(session), box write::Pass);
     store.register_pre_expansion_pass(Some(session), box redundant_field_names::RedundantFieldNames);
+    store.register_pre_expansion_pass(Some(session), box strings::StringLitAsBytes);
     store.register_pre_expansion_pass(Some(session), box non_expressive_names::NonExpressiveNames {
         single_char_binding_names_threshold: conf.single_char_binding_names_threshold,
     });
@@ -355,7 +356,6 @@ 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_late_lint_pass(box derive::Derive);
     reg.register_late_lint_pass(box types::CharLitAsU8);
     reg.register_late_lint_pass(box vec::Pass);

clippy_lints/src/strings.rs

@@ -1,9 +1,10 @@
 use crate::rustc::hir::*;
-use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::lint::{EarlyContext, EarlyLintPass, 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, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq,
+};
 
 /// **What it does:** Checks for string appends of the form `x = x + y` (without
 /// `let`!).
@@ -82,7 +83,14 @@ impl LintPass for StringAdd {
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
     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_allowed(cx, STRING_ADD_ASSIGN, e.id) {
                     let parent = get_parent_expr(cx, e);
@@ -122,13 +130,15 @@ fn is_string(cx: &LateContext<'_, '_>, e: &Expr) -> bool {
 
 fn is_add(cx: &LateContext<'_, '_>, src: &Expr, target: &Expr) -> bool {
     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, _) => {
-            block.stmts.is_empty()
-                && block
-                    .expr
-                    .as_ref()
-                    .map_or(false, |expr| is_add(cx, expr, target))
+            block.stmts.is_empty() && block.expr.as_ref().map_or(false, |expr| is_add(cx, expr, target))
         },
         _ => false,
     }
@@ -143,12 +153,12 @@ impl LintPass for StringLitAsBytes {
     }
 }
 
-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;
+impl EarlyLintPass for StringLitAsBytes {
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &crate::syntax::ast::Expr) {
+        use crate::syntax::ast::{ExprKind, LitKind};
         use crate::utils::{in_macro, snippet};
 
-        if let ExprKind::MethodCall(ref path, _, ref args) = e.node {
+        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 {
@@ -163,6 +173,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringLitAsBytes {
                             );
                         }
                     }
+                } else if let ExprKind::Mac(ref mac) = args[0].node {
+                    if mac.node.path == "include_str" {
+                        span_lint_and_sugg(
+                            cx,
+                            STRING_LIT_AS_BYTES,
+                            e.span,
+                            "calling `as_bytes()` on a include_str macro",
+                            "consider using include_bytes instead",
+                            format!("include_bytes!({})", mac.node.stream()),
+                        );
+                    }
                 }
             }
         }

tests/ui/strings.rs

@@ -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();

tests/ui/strings.stderr

@@ -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
    |

tests/ui/strings_early.rs

@@ -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);
+}

tests/ui/strings_early.stderr

@@ -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 include_str macro
+  --> $DIR/strings_early.rs:13:22
+   |
+13 |     let includestr = include_str!("entry.rs").as_bytes();
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using include_bytes instead: `include_bytes!("entry.rs")`
+
+error: aborting due to 2 previous errors
+