Skip to content

Commit 88d9ec9

Browse files
committed
[WIP] Add PRINTLN_EMPTY_STRING lint.
I have no idea of how I could properly check that there is really an empty string argument in a `println` invocation. See `HACK` in print.rs for details.
1 parent 4d9ed8b commit 88d9ec9

File tree

4 files changed

+94
-30
lines changed

4 files changed

+94
-30
lines changed

clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
530530
partialeq_ne_impl::PARTIALEQ_NE_IMPL,
531531
precedence::PRECEDENCE,
532532
print::PRINT_WITH_NEWLINE,
533+
print::PRINTLN_EMPTY_STRING,
533534
ptr::CMP_NULL,
534535
ptr::MUT_FROM_REF,
535536
ptr::PTR_ARG,

clippy_lints/src/print.rs

Lines changed: 80 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,30 @@
1+
use std::ops::Deref;
12
use rustc::hir::*;
23
use rustc::hir::map::Node::{NodeImplItem, NodeItem};
34
use rustc::lint::*;
45
use syntax::ast::LitKind;
56
use syntax::symbol::InternedString;
7+
use syntax_pos::Span;
68
use utils::{is_expn_of, match_def_path, match_path, resolve_node, span_lint};
79
use utils::{paths, opt_def_id};
810

11+
/// **What it does:** This lint warns when you using `println!("")` to
12+
/// print a newline.
13+
///
14+
/// **Why is this bad?** You should use `println!()`, which is simpler.
15+
///
16+
/// **Known problems:** None.
17+
///
18+
/// **Example:**
19+
/// ```rust
20+
/// println!("");
21+
/// ```
22+
declare_lint! {
23+
pub PRINTLN_EMPTY_STRING,
24+
Warn,
25+
"using `print!()` with a format string that ends in a newline"
26+
}
27+
928
/// **What it does:** This lint warns when you using `print!()` with a format
1029
/// string that
1130
/// ends in a newline.
@@ -64,7 +83,7 @@ pub struct Pass;
6483

6584
impl LintPass for Pass {
6685
fn get_lints(&self) -> LintArray {
67-
lint_array!(PRINT_WITH_NEWLINE, PRINT_STDOUT, USE_DEBUG)
86+
lint_array!(PRINT_WITH_NEWLINE, PRINTLN_EMPTY_STRING, PRINT_STDOUT, USE_DEBUG)
6887
}
6988
}
7089

@@ -88,35 +107,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
88107

89108
span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name));
90109

91-
// Check print! with format string ending in "\n".
92-
if_let_chain!{[
93-
name == "print",
94-
95-
// ensure we're calling Arguments::new_v1
96-
args.len() == 1,
97-
let ExprCall(ref args_fun, ref args_args) = args[0].node,
98-
let ExprPath(ref qpath) = args_fun.node,
99-
let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id)),
100-
match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1),
101-
args_args.len() == 2,
102-
let ExprAddrOf(_, ref match_expr) = args_args[1].node,
103-
let ExprMatch(ref args, _, _) = match_expr.node,
104-
let ExprTup(ref args) = args.node,
105-
106-
// collect the format string parts and check the last one
107-
let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]),
108-
let Some('\n') = fmtstr.chars().last(),
109-
110-
// "foo{}bar" is made into two strings + one argument,
111-
// if the format string starts with `{}` (eg. "{}foo"),
112-
// the string array is prepended an empty string "".
113-
// We only want to check the last string after any `{}`:
114-
args.len() < fmtlen,
115-
], {
116-
span_lint(cx, PRINT_WITH_NEWLINE, span,
117-
"using `print!()` with a format string that ends in a \
118-
newline, consider using `println!()` instead");
119-
}}
110+
match name {
111+
"print" => check_print(cx, span, args),
112+
"println" => check_println(cx, span, args),
113+
_ => (),
114+
}
120115
}
121116
}
122117
// Search for something like
@@ -135,6 +130,61 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
135130
}
136131
}
137132

133+
// Check for print!("... \n", ...).
134+
fn check_print<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, args: &HirVec<Expr>) {
135+
if_let_chain!{[
136+
// ensure we're calling Arguments::new_v1
137+
args.len() == 1,
138+
let ExprCall(ref args_fun, ref args_args) = args[0].node,
139+
let ExprPath(ref qpath) = args_fun.node,
140+
let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id)),
141+
match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1),
142+
args_args.len() == 2,
143+
let ExprAddrOf(_, ref match_expr) = args_args[1].node,
144+
let ExprMatch(ref args, _, _) = match_expr.node,
145+
let ExprTup(ref args) = args.node,
146+
147+
// collect the format string parts and check the last one
148+
let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]),
149+
let Some('\n') = fmtstr.chars().last(),
150+
151+
// "foo{}bar" is made into two strings + one argument,
152+
// if the format string starts with `{}` (eg. "{}foo"),
153+
// the string array is prepended an empty string "".
154+
// We only want to check the last string after any `{}`:
155+
args.len() < fmtlen,
156+
], {
157+
span_lint(cx, PRINT_WITH_NEWLINE, span,
158+
"using `print!()` with a format string that ends in a \
159+
newline, consider using `println!()` instead");
160+
}}
161+
}
162+
163+
/// Check for println!("")
164+
fn check_println<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, args: &HirVec<Expr>) {
165+
if_let_chain!{[
166+
// ensure we're calling Arguments::new_v1
167+
args.len() == 1,
168+
let ExprCall(ref args_fun, ref args_args) = args[0].node,
169+
let ExprPath(ref qpath) = args_fun.node,
170+
let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id)),
171+
match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1),
172+
args_args.len() == 2,
173+
174+
// check that the string is empty
175+
let Some((fmtstr, 1)) = get_argument_fmtstr_parts(&args_args[0]),
176+
fmtstr.deref() == "\n",
177+
178+
// HACK: check size of the macro invocation:
179+
// println!(); // 11 chars: don't lint
180+
// println!("") // 12 chars: lint
181+
(span.hi() - span.lo()).0 > 11,
182+
], {
183+
span_lint(cx, PRINT_WITH_NEWLINE, span,
184+
"using `println!(\"\")`, consider using `println!()` instead");
185+
}}
186+
}
187+
138188
fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool {
139189
let map = &cx.tcx.hir;
140190

tests/ui/println_empty_string.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fn main() {
2+
3+
println!();
4+
println!("") // no semicolon intended
5+
}

tests/ui/println_empty_string.stderr

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: using `println!("")`, consider using `println!()` instead
2+
--> $DIR/println_empty_string.rs:4:5
3+
|
4+
4 | println!("") // no semicolon intended
5+
| ^^^^^^^^^^^^
6+
|
7+
= note: `-D print-with-newline` implied by `-D warnings`
8+

0 commit comments

Comments
 (0)