]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/write.rs
Auto merge of #3680 - g-bartoszek:needless-bool-else-if-brackets, r=oli-obk
[rust.git] / clippy_lints / src / write.rs
index a019e23a3014f34821d4ac6df525515edd99cae6..c8c291c8cc873a87d5b69d13531e568e8ff3a42d 100644 (file)
@@ -1,9 +1,11 @@
-use rustc::lint::*;
-use rustc::{declare_lint, lint_array};
+use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg};
+use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_errors::Applicability;
+use std::borrow::Cow;
 use syntax::ast::*;
-use syntax::tokenstream::{ThinTokenStream, TokenStream};
-use syntax::parse::{token, parser};
-use crate::utils::{span_lint, span_lint_and_sugg};
+use syntax::parse::{parser, token};
+use syntax::tokenstream::TokenStream;
 
 /// **What it does:** This lint warns when you use `println!("")` to
 /// print a newline.
 /// ```rust
 /// print!("Hello {}!\n", name);
 /// ```
+/// use println!() instead
+/// ```rust
+/// println!("Hello {}!", name);
+/// ```
 declare_clippy_lint! {
     pub PRINT_WITH_NEWLINE,
     style,
-    "using `print!()` with a format string that ends in a newline"
+    "using `print!()` with a format string that ends in a single newline"
 }
 
 /// **What it does:** Checks for printing on *stdout*. The purpose of this lint
 /// ```rust
 /// println!("{}", "foo");
 /// ```
+/// use the literal without formatting:
+/// ```rust
+/// println!("foo");
+/// ```
 declare_clippy_lint! {
     pub PRINT_LITERAL,
     style,
 declare_clippy_lint! {
     pub WRITE_WITH_NEWLINE,
     style,
-    "using `write!()` with a format string that ends in a newline"
+    "using `write!()` with a format string that ends in a single newline"
 }
 
 /// **What it does:** This lint warns about the use of literals as `write!`/`writeln!` args.
@@ -171,7 +181,7 @@ impl EarlyLintPass for Pass {
     fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
         if mac.node.path == "println" {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `println!`");
-            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false) {
+            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 {
                 if fmtstr == "" {
                     span_lint_and_sugg(
                         cx,
@@ -180,36 +190,56 @@ fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
                         "using `println!(\"\")`",
                         "replace it with",
                         "println!()".to_string(),
+                        Applicability::MachineApplicable,
                     );
                 }
             }
         } else if mac.node.path == "print" {
             span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
-            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false) {
-                if fmtstr.ends_with("\\n") {
-                    span_lint(cx, PRINT_WITH_NEWLINE, mac.span,
-                            "using `print!()` with a format string that ends in a \
-                            newline, consider using `println!()` instead");
+            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 {
+                if check_newlines(&fmtstr) {
+                    span_lint(
+                        cx,
+                        PRINT_WITH_NEWLINE,
+                        mac.span,
+                        "using `print!()` with a format string that ends in a \
+                         single newline, consider using `println!()` instead",
+                    );
                 }
             }
         } else if mac.node.path == "write" {
-            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, true) {
-                if fmtstr.ends_with("\\n") {
-                    span_lint(cx, WRITE_WITH_NEWLINE, mac.span,
-                            "using `write!()` with a format string that ends in a \
-                            newline, consider using `writeln!()` instead");
+            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, true).0 {
+                if check_newlines(&fmtstr) {
+                    span_lint(
+                        cx,
+                        WRITE_WITH_NEWLINE,
+                        mac.span,
+                        "using `write!()` with a format string that ends in a \
+                         single newline, consider using `writeln!()` instead",
+                    );
                 }
             }
         } else if mac.node.path == "writeln" {
-            if let Some(fmtstr) = check_tts(cx, &mac.node.tts, true) {
+            let check_tts = check_tts(cx, &mac.node.tts, true);
+            if let Some(fmtstr) = check_tts.0 {
                 if fmtstr == "" {
+                    let mut applicability = Applicability::MachineApplicable;
+                    let suggestion = check_tts.1.map_or_else(
+                        move || {
+                            applicability = Applicability::HasPlaceholders;
+                            Cow::Borrowed("v")
+                        },
+                        move |expr| snippet_with_applicability(cx, expr.span, "v", &mut applicability),
+                    );
+
                     span_lint_and_sugg(
                         cx,
                         WRITELN_EMPTY_STRING,
                         mac.span,
-                        "using `writeln!(v, \"\")`",
+                        format!("using `writeln!({}, \"\")`", suggestion).as_str(),
                         "replace it with",
-                        "writeln!(v)".to_string(),
+                        format!("writeln!({})", suggestion),
+                        applicability,
                     );
                 }
             }
@@ -217,29 +247,46 @@ fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
     }
 }
 
-fn check_tts(cx: &EarlyContext<'a>, tts: &ThinTokenStream, is_write: bool) -> Option<String> {
-    let tts = TokenStream::from(tts.clone());
-    let mut parser = parser::Parser::new(
-        &cx.sess.parse_sess,
-        tts,
-        None,
-        false,
-        false,
-    );
+/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
+/// options. The first part of the tuple is `format_str` of the macros. The second part of the tuple
+/// is in the `write[ln]!` case the expression the `format_str` should be written to.
+///
+/// Example:
+///
+/// Calling this function on
+/// ```rust,ignore
+/// writeln!(buf, "string to write: {}", something)
+/// ```
+/// will return
+/// ```rust,ignore
+/// (Some("string to write: {}"), Some(buf))
+/// ```
+fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<String>, Option<Expr>) {
+    use fmt_macros::*;
+    let tts = tts.clone();
+    let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false);
+    let mut expr: Option<Expr> = None;
     if is_write {
-        // skip the initial write target
-        parser.parse_expr().map_err(|mut err| err.cancel()).ok()?;
+        expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
+            Ok(p) => Some(p.into_inner()),
+            Err(_) => return (None, None),
+        };
         // might be `writeln!(foo)`
-        parser.expect(&token::Comma).map_err(|mut err| err.cancel()).ok()?;
+        if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
+            return (None, expr);
+        }
     }
-    let fmtstr = parser.parse_str().map_err(|mut err| err.cancel()).ok()?.0.to_string();
-    use fmt_macros::*;
+
+    let fmtstr = match parser.parse_str().map_err(|mut err| err.cancel()) {
+        Ok(token) => token.0.to_string(),
+        Err(_) => return (None, expr),
+    };
     let tmp = fmtstr.clone();
     let mut args = vec![];
-    let mut fmt_parser = Parser::new(&tmp, None);
+    let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
     while let Some(piece) = fmt_parser.next() {
         if !fmt_parser.errors.is_empty() {
-            return None;
+            return (None, expr);
         }
         if let Piece::NextArgument(arg) = piece {
             if arg.format.ty == "?" {
@@ -249,18 +296,9 @@ fn check_tts(cx: &EarlyContext<'a>, tts: &ThinTokenStream, is_write: bool) -> Op
             args.push(arg);
         }
     }
-    let lint = if is_write {
-        WRITE_LITERAL
-    } else {
-        PRINT_LITERAL
-    };
+    let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
     let mut idx = 0;
     loop {
-        if !parser.eat(&token::Comma) {
-            assert!(parser.eat(&token::Eof));
-            return Some(fmtstr);
-        }
-        let expr = parser.parse_expr().map_err(|mut err| err.cancel()).ok()?;
         const SIMPLE: FormatSpec<'_> = FormatSpec {
             fill: None,
             align: AlignUnknown,
@@ -269,43 +307,52 @@ fn check_tts(cx: &EarlyContext<'a>, tts: &ThinTokenStream, is_write: bool) -> Op
             width: CountImplied,
             ty: "",
         };
-        match &expr.node {
+        if !parser.eat(&token::Comma) {
+            return (Some(fmtstr), expr);
+        }
+        let token_expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
+            Ok(expr) => expr,
+            Err(_) => return (Some(fmtstr), None),
+        };
+        match &token_expr.node {
             ExprKind::Lit(_) => {
                 let mut all_simple = true;
                 let mut seen = false;
                 for arg in &args {
                     match arg.position {
-                        | ArgumentImplicitlyIs(n)
-                        | ArgumentIs(n)
-                        => if n == idx {
-                            all_simple &= arg.format == SIMPLE;
-                            seen = true;
+                        ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
+                            if n == idx {
+                                all_simple &= arg.format == SIMPLE;
+                                seen = true;
+                            }
                         },
                         ArgumentNamed(_) => {},
                     }
                 }
                 if all_simple && seen {
-                    span_lint(cx, lint, expr.span, "literal with an empty format string");
+                    span_lint(cx, lint, token_expr.span, "literal with an empty format string");
                 }
                 idx += 1;
             },
             ExprKind::Assign(lhs, rhs) => {
-                if let ExprKind::Path(_, p) = &lhs.node {
-                    let mut all_simple = true;
-                    let mut seen = false;
-                    for arg in &args {
-                        match arg.position {
-                            | ArgumentImplicitlyIs(_)
-                            | ArgumentIs(_)
-                            => {},
-                            ArgumentNamed(name) => if *p == name {
-                                seen = true;
-                                all_simple &= arg.format == SIMPLE;
-                            },
+                if let ExprKind::Lit(_) = rhs.node {
+                    if let ExprKind::Path(_, p) = &lhs.node {
+                        let mut all_simple = true;
+                        let mut seen = false;
+                        for arg in &args {
+                            match arg.position {
+                                ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
+                                ArgumentNamed(name) => {
+                                    if *p == name {
+                                        seen = true;
+                                        all_simple &= arg.format == SIMPLE;
+                                    }
+                                },
+                            }
+                        }
+                        if all_simple && seen {
+                            span_lint(cx, lint, rhs.span, "literal with an empty format string");
                         }
-                    }
-                    if all_simple && seen {
-                        span_lint(cx, lint, rhs.span, "literal with an empty format string");
                     }
                 }
             },
@@ -313,3 +360,29 @@ fn check_tts(cx: &EarlyContext<'a>, tts: &ThinTokenStream, is_write: bool) -> Op
         }
     }
 }
+
+// Checks if `s` constains a single newline that terminates it
+fn check_newlines(s: &str) -> bool {
+    if s.len() < 2 {
+        return false;
+    }
+
+    let bytes = s.as_bytes();
+    if bytes[bytes.len() - 2] != b'\\' || bytes[bytes.len() - 1] != b'n' {
+        return false;
+    }
+
+    let mut escaping = false;
+    for (index, &byte) in bytes.iter().enumerate() {
+        if escaping {
+            if byte == b'n' {
+                return index == bytes.len() - 1;
+            }
+            escaping = false;
+        } else if byte == b'\\' {
+            escaping = true;
+        }
+    }
+
+    false
+}