Merge pull request #562 from fhartwig/str-lit-as-bytes

Add lint for "string literal".as_bytes()

Author: llogiq

README.md

@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 93 lints included in this crate:
+There are 94 lints included in this crate:
 
 name                                                                                                           | default | meaning
 ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -83,6 +83,7 @@ name
 [str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string)                                 | warn    | using `to_string()` on a str, which should be `to_owned()`
 [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add)                                       | allow   | using `x + ..` where x is a `String`; suggests using `push_str()` instead
 [string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign)                         | allow   | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
+[string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes)                     | warn    | calling `as_bytes` on a string literal; suggests using a byte string literal instead
 [string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string)                           | warn    | calling `String.to_string()` which is a no-op
 [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment)                   | warn    | assignments to temporaries
 [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                           | warn    | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`.

src/lib.rs

@@ -135,6 +135,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box misc::UsedUnderscoreBinding);
     reg.register_late_lint_pass(box array_indexing::ArrayIndexing);
     reg.register_late_lint_pass(box panic::PanicPass);
+    reg.register_late_lint_pass(box strings::StringLitAsBytes);
 
 
     reg.register_lint_group("clippy_pedantic", vec![
@@ -225,6 +226,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         ranges::RANGE_ZIP_WITH_LEN,
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,
+        strings::STRING_LIT_AS_BYTES,
         temporary_assignment::TEMPORARY_ASSIGNMENT,
         transmute::USELESS_TRANSMUTE,
         types::BOX_VEC,

src/strings.rs

@@ -48,6 +48,22 @@ declare_lint! {
     "using `x + ..` where x is a `String`; suggests using `push_str()` instead"
 }
 
+/// **What it does:** This lint matches the `as_bytes` method called on string
+/// literals that contain only ascii characters. It is `Warn` by default.
+///
+/// **Why is this bad?** Byte string literals (e.g. `b"foo"`) can be used instead. They are shorter but less discoverable than `as_bytes()`.
+///
+/// **Example:**
+///
+/// ```
+/// let bs = "a byte string".as_bytes();
+/// ```
+declare_lint! {
+    pub STRING_LIT_AS_BYTES,
+    Warn,
+    "calling `as_bytes` on a string literal; suggests using a byte string literal instead"
+}
+
 #[derive(Copy, Clone)]
 pub struct StringAdd;
 
@@ -104,3 +120,36 @@ 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 LateLintPass for StringLitAsBytes {
+    fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+        use std::ascii::AsciiExt;
+        use syntax::ast::Lit_::LitStr;
+        use utils::{snippet, in_macro};
+
+        if let ExprMethodCall(ref name, _, ref args) = e.node {
+            if name.node.as_str() == "as_bytes" {
+                if let ExprLit(ref lit) = args[0].node {
+                    if let LitStr(ref lit_content, _) = lit.node {
+                        if lit_content.chars().all(|c| c.is_ascii()) && !in_macro(cx, e.span) {
+                            let msg = format!("calling `as_bytes()` on a string literal. \
+                                               Consider using a byte string literal instead: \
+                                               `b{}`",
+                                               snippet(cx, args[0].span, r#""foo""#));
+                            span_lint(cx, STRING_LIT_AS_BYTES, e.span, &msg);
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

tests/compile-fail/strings.rs

@@ -44,6 +44,14 @@ fn both() {
     assert_eq!(&x, &z);
 }
 
+#[allow(dead_code, unused_variables)]
+#[deny(string_lit_as_bytes)]
+fn str_lit_as_bytes() {
+    let bs = "hello there".as_bytes(); //~ERROR calling `as_bytes()`
+    // no warning, because this cannot be written as a byte string literal:
+    let ubs = "☃".as_bytes();
+}
+
 fn main() {
     add_only();
     add_assign_only();