Skip to content

Commit 7c823ca

Browse files
borsbors
committed
Auto merge of #3450 - phansch:structured_sugg_for_explicit_write, r=flip1995
Add suggestion for explicit_write lint Closes #2083
2 parents 2f467ac + 194acaf commit 7c823ca

File tree

4 files changed

+109
-38
lines changed

4 files changed

+109
-38
lines changed

‎clippy_lints/src/explicit_write.rs

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
use crate::rustc::hir::*;
1111
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
1212
use crate::rustc::{declare_tool_lint, lint_array};
13-
use crate::utils::opt_def_id;
14-
use crate::utils::{is_expn_of, match_def_path, resolve_node, span_lint};
13+
use crate::rustc_errors::Applicability;
14+
use crate::syntax::ast::LitKind;
15+
use crate::utils::{is_expn_of, match_def_path, opt_def_id, resolve_node, span_lint, span_lint_and_sugg};
1516
use if_chain::if_chain;
1617

1718
/// **What it does:** Checks for usage of `write!()` / `writeln()!` which can be
@@ -81,32 +82,85 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
8182
} else {
8283
""
8384
};
84-
if let Some(macro_name) = calling_macro {
85-
span_lint(
86-
cx,
87-
EXPLICIT_WRITE,
88-
expr.span,
89-
&format!(
90-
"use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead",
91-
macro_name,
92-
dest_name,
93-
prefix,
94-
macro_name.replace("write", "print")
95-
)
96-
);
85+
86+
// We need to remove the last trailing newline from the string because the
87+
// underlying `fmt::write` function doesn't know whether `println!` or `print!` was
88+
// used.
89+
if let Some(mut write_output) = write_output_string(write_args) {
90+
if write_output.ends_with('\n') {
91+
write_output.pop();
92+
}
93+
94+
if let Some(macro_name) = calling_macro {
95+
span_lint_and_sugg(
96+
cx,
97+
EXPLICIT_WRITE,
98+
expr.span,
99+
&format!(
100+
"use of `{}!({}(), ...).unwrap()`",
101+
macro_name,
102+
dest_name
103+
),
104+
"try this",
105+
format!("{}{}!(\"{}\")", prefix, macro_name.replace("write", "print"), write_output.escape_default()),
106+
Applicability::MachineApplicable
107+
);
108+
} else {
109+
span_lint_and_sugg(
110+
cx,
111+
EXPLICIT_WRITE,
112+
expr.span,
113+
&format!("use of `{}().write_fmt(...).unwrap()`", dest_name),
114+
"try this",
115+
format!("{}print!(\"{}\")", prefix, write_output.escape_default()),
116+
Applicability::MachineApplicable
117+
);
118+
}
97119
} else {
98-
span_lint(
99-
cx,
100-
EXPLICIT_WRITE,
101-
expr.span,
102-
&format!(
103-
"use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead",
104-
dest_name,
105-
prefix,
106-
)
107-
);
120+
// We don't have a proper suggestion
121+
if let Some(macro_name) = calling_macro {
122+
span_lint(
123+
cx,
124+
EXPLICIT_WRITE,
125+
expr.span,
126+
&format!(
127+
"use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead",
128+
macro_name,
129+
dest_name,
130+
prefix,
131+
macro_name.replace("write", "print")
132+
)
133+
);
134+
} else {
135+
span_lint(
136+
cx,
137+
EXPLICIT_WRITE,
138+
expr.span,
139+
&format!("use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", dest_name, prefix),
140+
);
141+
}
108142
}
143+
109144
}
110145
}
111146
}
112147
}
148+
149+
// Extract the output string from the given `write_args`.
150+
fn write_output_string(write_args: &HirVec<Expr>) -> Option<String> {
151+
if_chain! {
152+
// Obtain the string that should be printed
153+
if write_args.len() > 1;
154+
if let ExprKind::Call(_, ref output_args) = write_args[1].node;
155+
if output_args.len() > 0;
156+
if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node;
157+
if let ExprKind::Array(ref string_exprs) = output_string_expr.node;
158+
if string_exprs.len() > 0;
159+
if let ExprKind::Lit(ref lit) = string_exprs[0].node;
160+
if let LitKind::Str(ref write_output, _) = lit.node;
161+
then {
162+
return Some(write_output.to_string())
163+
}
164+
}
165+
None
166+
}

‎clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#![feature(slice_patterns)]
1515
#![feature(stmt_expr_attributes)]
1616
#![feature(range_contains)]
17+
#![feature(str_escape)]
1718
#![allow(clippy::missing_docs_in_private_items)]
1819
#![recursion_limit = "256"]
1920
#![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)]

‎tests/ui/explicit_write.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ fn main() {
2727
writeln!(std::io::stderr(), "test").unwrap();
2828
std::io::stdout().write_fmt(format_args!("test")).unwrap();
2929
std::io::stderr().write_fmt(format_args!("test")).unwrap();
30+
31+
// including newlines
32+
writeln!(std::io::stdout(), "test\ntest").unwrap();
33+
writeln!(std::io::stderr(), "test\ntest").unwrap();
3034
}
3135
// these should not warn, different destination
3236
{

‎tests/ui/explicit_write.stderr

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,52 @@
1-
error: use of `write!(stdout(), ...).unwrap()`. Consider using `print!` instead
1+
error: use of `write!(stdout(), ...).unwrap()`
22
--> $DIR/explicit_write.rs:24:9
33
|
44
24 | write!(std::io::stdout(), "test").unwrap();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
66
|
77
= note: `-D clippy::explicit-write` implied by `-D warnings`
88

9-
error: use of `write!(stderr(), ...).unwrap()`. Consider using `eprint!` instead
9+
error: use of `write!(stderr(), ...).unwrap()`
1010
--> $DIR/explicit_write.rs:25:9
1111
|
1212
25 | write!(std::io::stderr(), "test").unwrap();
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
1414

15-
error: use of `writeln!(stdout(), ...).unwrap()`. Consider using `println!` instead
15+
error: use of `writeln!(stdout(), ...).unwrap()`
1616
--> $DIR/explicit_write.rs:26:9
1717
|
1818
26 | writeln!(std::io::stdout(), "test").unwrap();
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test")`
2020

21-
error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead
21+
error: use of `writeln!(stderr(), ...).unwrap()`
2222
--> $DIR/explicit_write.rs:27:9
2323
|
2424
27 | writeln!(std::io::stderr(), "test").unwrap();
25-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test")`
2626

27-
error: use of `stdout().write_fmt(...).unwrap()`. Consider using `print!` instead
27+
error: use of `stdout().write_fmt(...).unwrap()`
2828
--> $DIR/explicit_write.rs:28:9
2929
|
3030
28 | std::io::stdout().write_fmt(format_args!("test")).unwrap();
31-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `print!("test")`
3232

33-
error: use of `stderr().write_fmt(...).unwrap()`. Consider using `eprint!` instead
33+
error: use of `stderr().write_fmt(...).unwrap()`
3434
--> $DIR/explicit_write.rs:29:9
3535
|
3636
29 | std::io::stderr().write_fmt(format_args!("test")).unwrap();
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprint!("test")`
3838

39-
error: aborting due to 6 previous errors
39+
error: use of `writeln!(stdout(), ...).unwrap()`
40+
--> $DIR/explicit_write.rs:32:9
41+
|
42+
32 | writeln!(std::io::stdout(), "test/ntest").unwrap();
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `println!("test/ntest")`
44+
45+
error: use of `writeln!(stderr(), ...).unwrap()`
46+
--> $DIR/explicit_write.rs:33:9
47+
|
48+
33 | writeln!(std::io::stderr(), "test/ntest").unwrap();
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `eprintln!("test/ntest")`
50+
51+
error: aborting due to 8 previous errors
4052

0 commit comments

Comments
 (0)