Skip to content

Commit 660a9d1

Browse files
flip1995flip1995
committed
Don't lint debug formatting in debug impl
1 parent a6f310e commit 660a9d1

File tree

2 files changed

+165
-132
lines changed

2 files changed

+165
-132
lines changed

‎clippy_lints/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ mod reexport {
327327
///
328328
/// Used in `./src/driver.rs`.
329329
pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, conf: &Conf) {
330-
store.register_pre_expansion_pass(|| box write::Write);
330+
store.register_pre_expansion_pass(|| box write::Write::default());
331331
store.register_pre_expansion_pass(|| box redundant_field_names::RedundantFieldNames);
332332
let single_char_binding_names_threshold = conf.single_char_binding_names_threshold;
333333
store.register_pre_expansion_pass(move || box non_expressive_names::NonExpressiveNames {

‎clippy_lints/src/write.rs

+164-131
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_errors::Applicability;
66
use rustc_lexer::unescape::{self, EscapeError};
77
use rustc_lint::{EarlyContext, EarlyLintPass};
88
use rustc_parse::parser;
9-
use rustc_session::{declare_lint_pass, declare_tool_lint};
9+
use rustc_session::{declare_tool_lint, impl_lint_pass};
1010
use rustc_span::symbol::Symbol;
1111
use rustc_span::{BytePos, Span};
1212
use syntax::ast::*;
@@ -175,7 +175,12 @@ declare_clippy_lint! {
175175
"writing a literal with a format string"
176176
}
177177

178-
declare_lint_pass!(Write => [
178+
#[derive(Default)]
179+
pub struct Write {
180+
in_debug_impl: bool,
181+
}
182+
183+
impl_lint_pass!(Write => [
179184
PRINT_WITH_NEWLINE,
180185
PRINTLN_EMPTY_STRING,
181186
PRINT_STDOUT,
@@ -187,10 +192,31 @@ declare_lint_pass!(Write => [
187192
]);
188193

189194
impl EarlyLintPass for Write {
195+
fn check_item(&mut self, _: &EarlyContext<'_>, item: &Item) {
196+
if let ItemKind::Impl(_, _, _, _, Some(trait_ref), _, _) = &item.kind {
197+
let trait_name = trait_ref
198+
.path
199+
.segments
200+
.iter()
201+
.last()
202+
.expect("path has at least one segment")
203+
.ident
204+
.name
205+
.as_str();
206+
if trait_name == "Debug" {
207+
self.in_debug_impl = true;
208+
}
209+
}
210+
}
211+
212+
fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
213+
self.in_debug_impl = false;
214+
}
215+
190216
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
191217
if mac.path == sym!(println) {
192218
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`");
193-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
219+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
194220
if fmt_str.symbol == Symbol::intern("") {
195221
span_lint_and_sugg(
196222
cx,
@@ -205,7 +231,7 @@ impl EarlyLintPass for Write {
205231
}
206232
} else if mac.path == sym!(print) {
207233
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`");
208-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
234+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
209235
if check_newlines(&fmt_str) {
210236
span_lint_and_then(
211237
cx,
@@ -226,7 +252,7 @@ impl EarlyLintPass for Write {
226252
}
227253
}
228254
} else if mac.path == sym!(write) {
229-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), true) {
255+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
230256
if check_newlines(&fmt_str) {
231257
span_lint_and_then(
232258
cx,
@@ -247,7 +273,7 @@ impl EarlyLintPass for Write {
247273
}
248274
}
249275
} else if mac.path == sym!(writeln) {
250-
if let (Some(fmt_str), expr) = check_tts(cx, &mac.args.inner_tokens(), true) {
276+
if let (Some(fmt_str), expr) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
251277
if fmt_str.symbol == Symbol::intern("") {
252278
let mut applicability = Applicability::MachineApplicable;
253279
let suggestion = expr.map_or_else(
@@ -273,6 +299,138 @@ impl EarlyLintPass for Write {
273299
}
274300
}
275301

302+
impl Write {
303+
/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
304+
/// `Option`s. The first `Option` of the tuple is the macro's format string. It includes
305+
/// the contents of the string, whether it's a raw string, and the span of the literal in the
306+
/// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the
307+
/// `format_str` should be written to.
308+
///
309+
/// Example:
310+
///
311+
/// Calling this function on
312+
/// ```rust
313+
/// # use std::fmt::Write;
314+
/// # let mut buf = String::new();
315+
/// # let something = "something";
316+
/// writeln!(buf, "string to write: {}", something);
317+
/// ```
318+
/// will return
319+
/// ```rust,ignore
320+
/// (Some("string to write: {}"), Some(buf))
321+
/// ```
322+
#[allow(clippy::too_many_lines)]
323+
fn check_tts<'a>(
324+
&self,
325+
cx: &EarlyContext<'a>,
326+
tts: &TokenStream,
327+
is_write: bool,
328+
) -> (Option<StrLit>, Option<Expr>) {
329+
use fmt_macros::*;
330+
let tts = tts.clone();
331+
332+
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
333+
let mut expr: Option<Expr> = None;
334+
if is_write {
335+
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
336+
Ok(p) => Some(p.into_inner()),
337+
Err(_) => return (None, None),
338+
};
339+
// might be `writeln!(foo)`
340+
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
341+
return (None, expr);
342+
}
343+
}
344+
345+
let fmtstr = match parser.parse_str_lit() {
346+
Ok(fmtstr) => fmtstr,
347+
Err(_) => return (None, expr),
348+
};
349+
let tmp = fmtstr.symbol.as_str();
350+
let mut args = vec![];
351+
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
352+
while let Some(piece) = fmt_parser.next() {
353+
if !fmt_parser.errors.is_empty() {
354+
return (None, expr);
355+
}
356+
if let Piece::NextArgument(arg) = piece {
357+
if !self.in_debug_impl && arg.format.ty == "?" {
358+
// FIXME: modify rustc's fmt string parser to give us the current span
359+
span_lint(cx, USE_DEBUG, parser.prev_span, "use of `Debug`-based formatting");
360+
}
361+
args.push(arg);
362+
}
363+
}
364+
let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
365+
let mut idx = 0;
366+
loop {
367+
const SIMPLE: FormatSpec<'_> = FormatSpec {
368+
fill: None,
369+
align: AlignUnknown,
370+
flags: 0,
371+
precision: CountImplied,
372+
precision_span: None,
373+
width: CountImplied,
374+
width_span: None,
375+
ty: "",
376+
ty_span: None,
377+
};
378+
if !parser.eat(&token::Comma) {
379+
return (Some(fmtstr), expr);
380+
}
381+
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
382+
expr
383+
} else {
384+
return (Some(fmtstr), None);
385+
};
386+
match &token_expr.kind {
387+
ExprKind::Lit(_) => {
388+
let mut all_simple = true;
389+
let mut seen = false;
390+
for arg in &args {
391+
match arg.position {
392+
ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
393+
if n == idx {
394+
all_simple &= arg.format == SIMPLE;
395+
seen = true;
396+
}
397+
},
398+
ArgumentNamed(_) => {},
399+
}
400+
}
401+
if all_simple && seen {
402+
span_lint(cx, lint, token_expr.span, "literal with an empty format string");
403+
}
404+
idx += 1;
405+
},
406+
ExprKind::Assign(lhs, rhs, _) => {
407+
if let ExprKind::Lit(_) = rhs.kind {
408+
if let ExprKind::Path(_, p) = &lhs.kind {
409+
let mut all_simple = true;
410+
let mut seen = false;
411+
for arg in &args {
412+
match arg.position {
413+
ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
414+
ArgumentNamed(name) => {
415+
if *p == name {
416+
seen = true;
417+
all_simple &= arg.format == SIMPLE;
418+
}
419+
},
420+
}
421+
}
422+
if all_simple && seen {
423+
span_lint(cx, lint, rhs.span, "literal with an empty format string");
424+
}
425+
}
426+
}
427+
},
428+
_ => idx += 1,
429+
}
430+
}
431+
}
432+
}
433+
276434
/// Given a format string that ends in a newline and its span, calculates the span of the
277435
/// newline.
278436
fn newline_span(fmtstr: &StrLit) -> Span {
@@ -296,131 +454,6 @@ fn newline_span(fmtstr: &StrLit) -> Span {
296454
sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
297455
}
298456

299-
/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
300-
/// `Option`s. The first `Option` of the tuple is the macro's format string. It includes
301-
/// the contents of the string, whether it's a raw string, and the span of the literal in the
302-
/// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the
303-
/// `format_str` should be written to.
304-
///
305-
/// Example:
306-
///
307-
/// Calling this function on
308-
/// ```rust
309-
/// # use std::fmt::Write;
310-
/// # let mut buf = String::new();
311-
/// # let something = "something";
312-
/// writeln!(buf, "string to write: {}", something);
313-
/// ```
314-
/// will return
315-
/// ```rust,ignore
316-
/// (Some("string to write: {}"), Some(buf))
317-
/// ```
318-
#[allow(clippy::too_many_lines)]
319-
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<StrLit>, Option<Expr>) {
320-
use fmt_macros::*;
321-
let tts = tts.clone();
322-
323-
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
324-
let mut expr: Option<Expr> = None;
325-
if is_write {
326-
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
327-
Ok(p) => Some(p.into_inner()),
328-
Err(_) => return (None, None),
329-
};
330-
// might be `writeln!(foo)`
331-
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
332-
return (None, expr);
333-
}
334-
}
335-
336-
let fmtstr = match parser.parse_str_lit() {
337-
Ok(fmtstr) => fmtstr,
338-
Err(_) => return (None, expr),
339-
};
340-
let tmp = fmtstr.symbol.as_str();
341-
let mut args = vec![];
342-
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
343-
while let Some(piece) = fmt_parser.next() {
344-
if !fmt_parser.errors.is_empty() {
345-
return (None, expr);
346-
}
347-
if let Piece::NextArgument(arg) = piece {
348-
if arg.format.ty == "?" {
349-
// FIXME: modify rustc's fmt string parser to give us the current span
350-
span_lint(cx, USE_DEBUG, parser.prev_span, "use of `Debug`-based formatting");
351-
}
352-
args.push(arg);
353-
}
354-
}
355-
let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
356-
let mut idx = 0;
357-
loop {
358-
const SIMPLE: FormatSpec<'_> = FormatSpec {
359-
fill: None,
360-
align: AlignUnknown,
361-
flags: 0,
362-
precision: CountImplied,
363-
precision_span: None,
364-
width: CountImplied,
365-
width_span: None,
366-
ty: "",
367-
ty_span: None,
368-
};
369-
if !parser.eat(&token::Comma) {
370-
return (Some(fmtstr), expr);
371-
}
372-
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
373-
expr
374-
} else {
375-
return (Some(fmtstr), None);
376-
};
377-
match &token_expr.kind {
378-
ExprKind::Lit(_) => {
379-
let mut all_simple = true;
380-
let mut seen = false;
381-
for arg in &args {
382-
match arg.position {
383-
ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
384-
if n == idx {
385-
all_simple &= arg.format == SIMPLE;
386-
seen = true;
387-
}
388-
},
389-
ArgumentNamed(_) => {},
390-
}
391-
}
392-
if all_simple && seen {
393-
span_lint(cx, lint, token_expr.span, "literal with an empty format string");
394-
}
395-
idx += 1;
396-
},
397-
ExprKind::Assign(lhs, rhs, _) => {
398-
if let ExprKind::Lit(_) = rhs.kind {
399-
if let ExprKind::Path(_, p) = &lhs.kind {
400-
let mut all_simple = true;
401-
let mut seen = false;
402-
for arg in &args {
403-
match arg.position {
404-
ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
405-
ArgumentNamed(name) => {
406-
if *p == name {
407-
seen = true;
408-
all_simple &= arg.format == SIMPLE;
409-
}
410-
},
411-
}
412-
}
413-
if all_simple && seen {
414-
span_lint(cx, lint, rhs.span, "literal with an empty format string");
415-
}
416-
}
417-
}
418-
},
419-
_ => idx += 1,
420-
}
421-
}
422-
}
423-
424457
/// Checks if the format string contains a single newline that terminates it.
425458
///
426459
/// Literal and escaped newlines are both checked (only literal for raw strings).

0 commit comments

Comments
 (0)