diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index 94bd0ab209cb..a0db3ae8df35 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -10,8 +10,9 @@ use crate::rustc::hir::*; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; -use crate::utils::opt_def_id; -use crate::utils::{is_expn_of, match_def_path, resolve_node, span_lint}; +use crate::rustc_errors::Applicability; +use crate::syntax::ast::LitKind; +use crate::utils::{is_expn_of, match_def_path, opt_def_id, resolve_node, span_lint, span_lint_and_sugg}; use if_chain::if_chain; /// **What it does:** Checks for usage of `write!()` / `writeln()!` which can be @@ -81,32 +82,85 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } else { "" }; - if let Some(macro_name) = calling_macro { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", - macro_name, - dest_name, - prefix, - macro_name.replace("write", "print") - ) - ); + + // We need to remove the last trailing newline from the string because the + // underlying `fmt::write` function doesn't know whether `println!` or `print!` was + // used. + if let Some(mut write_output) = write_output_string(write_args) { + if write_output.ends_with('\n') { + write_output.pop(); + } + + if let Some(macro_name) = calling_macro { + span_lint_and_sugg( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}!({}(), ...).unwrap()`", + macro_name, + dest_name + ), + "try this", + format!("{}{}!(\"{}\")", prefix, macro_name.replace("write", "print"), write_output.escape_default()), + Applicability::MachineApplicable + ); + } else { + span_lint_and_sugg( + cx, + EXPLICIT_WRITE, + expr.span, + &format!("use of `{}().write_fmt(...).unwrap()`", dest_name), + "try this", + format!("{}print!(\"{}\")", prefix, write_output.escape_default()), + Applicability::MachineApplicable + ); + } } else { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", - dest_name, - prefix, - ) - ); + // We don't have a proper suggestion + if let Some(macro_name) = calling_macro { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!( + "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", + macro_name, + dest_name, + prefix, + macro_name.replace("write", "print") + ) + ); + } else { + span_lint( + cx, + EXPLICIT_WRITE, + expr.span, + &format!("use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", dest_name, prefix), + ); + } } + } } } } + +// Extract the output string from the given `write_args`. +fn write_output_string(write_args: &HirVec) -> Option { + if_chain! { + // Obtain the string that should be printed + if write_args.len() > 1; + if let ExprKind::Call(_, ref output_args) = write_args[1].node; + if output_args.len() > 0; + if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node; + if let ExprKind::Array(ref string_exprs) = output_string_expr.node; + if string_exprs.len() > 0; + if let ExprKind::Lit(ref lit) = string_exprs[0].node; + if let LitKind::Str(ref write_output, _) = lit.node; + then { + return Some(write_output.to_string()) + } + } + None +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ee41c632077c..a862d774174b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -14,6 +14,7 @@ #![feature(slice_patterns)] #![feature(stmt_expr_attributes)] #![feature(range_contains)] +#![feature(str_escape)] #![allow(clippy::missing_docs_in_private_items)] #![recursion_limit = "256"] #![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)] diff --git a/tests/ui/explicit_write.rs b/tests/ui/explicit_write.rs index 10a4bca9f492..01a63b3a95f2 100644 --- a/tests/ui/explicit_write.rs +++ b/tests/ui/explicit_write.rs @@ -27,6 +27,10 @@ fn main() { writeln!(std::io::stderr(), "test").unwrap(); std::io::stdout().write_fmt(format_args!("test")).unwrap(); std::io::stderr().write_fmt(format_args!("test")).unwrap(); + + // including newlines + writeln!(std::io::stdout(), "test\ntest").unwrap(); + writeln!(std::io::stderr(), "test\ntest").unwrap(); } // these should not warn, different destination { diff --git a/tests/ui/explicit_write.stderr b/tests/ui/explicit_write.stderr index 171bf312a9bd..1a11dbc169bd 100644 --- a/tests/ui/explicit_write.stderr +++ b/tests/ui/explicit_write.stderr @@ -1,40 +1,52 @@ -error: use of `write!(stdout(), ...).unwrap()`. Consider using `print!` instead +error: use of `write!(stdout(), ...).unwrap()` --> $DIR/explicit_write.rs:24:9 | 24 | write!(std::io::stdout(), "test").unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")` | = note: `-D clippy::explicit-write` implied by `-D warnings` -error: use of `write!(stderr(), ...).unwrap()`. Consider using `eprint!` instead +error: use of `write!(stderr(), ...).unwrap()` --> $DIR/explicit_write.rs:25:9 | 25 | write!(std::io::stderr(), "test").unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")` -error: use of `writeln!(stdout(), ...).unwrap()`. Consider using `println!` instead +error: use of `writeln!(stdout(), ...).unwrap()` --> $DIR/explicit_write.rs:26:9 | 26 | writeln!(std::io::stdout(), "test").unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test")` -error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead +error: use of `writeln!(stderr(), ...).unwrap()` --> $DIR/explicit_write.rs:27:9 | 27 | writeln!(std::io::stderr(), "test").unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test")` -error: use of `stdout().write_fmt(...).unwrap()`. Consider using `print!` instead +error: use of `stdout().write_fmt(...).unwrap()` --> $DIR/explicit_write.rs:28:9 | 28 | std::io::stdout().write_fmt(format_args!("test")).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")` -error: use of `stderr().write_fmt(...).unwrap()`. Consider using `eprint!` instead +error: use of `stderr().write_fmt(...).unwrap()` --> $DIR/explicit_write.rs:29:9 | 29 | std::io::stderr().write_fmt(format_args!("test")).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")` -error: aborting due to 6 previous errors +error: use of `writeln!(stdout(), ...).unwrap()` + --> $DIR/explicit_write.rs:32:9 + | +32 | writeln!(std::io::stdout(), "test/ntest").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test/ntest")` + +error: use of `writeln!(stderr(), ...).unwrap()` + --> $DIR/explicit_write.rs:33:9 + | +33 | writeln!(std::io::stderr(), "test/ntest").unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test/ntest")` + +error: aborting due to 8 previous errors