Skip to content

Commit 23252b8

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

File tree

2 files changed

+169
-132
lines changed

2 files changed

+169
-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

+168-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,35 @@ 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 {
197+
of_trait: Some(trait_ref),
198+
..
199+
} = &item.kind
200+
{
201+
let trait_name = trait_ref
202+
.path
203+
.segments
204+
.iter()
205+
.last()
206+
.expect("path has at least one segment")
207+
.ident
208+
.name
209+
.as_str();
210+
if trait_name == "Debug" {
211+
self.in_debug_impl = true;
212+
}
213+
}
214+
}
215+
216+
fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
217+
self.in_debug_impl = false;
218+
}
219+
190220
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
191221
if mac.path == sym!(println) {
192222
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`");
193-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
223+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
194224
if fmt_str.symbol == Symbol::intern("") {
195225
span_lint_and_sugg(
196226
cx,
@@ -205,7 +235,7 @@ impl EarlyLintPass for Write {
205235
}
206236
} else if mac.path == sym!(print) {
207237
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`");
208-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
238+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
209239
if check_newlines(&fmt_str) {
210240
span_lint_and_then(
211241
cx,
@@ -226,7 +256,7 @@ impl EarlyLintPass for Write {
226256
}
227257
}
228258
} else if mac.path == sym!(write) {
229-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), true) {
259+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
230260
if check_newlines(&fmt_str) {
231261
span_lint_and_then(
232262
cx,
@@ -247,7 +277,7 @@ impl EarlyLintPass for Write {
247277
}
248278
}
249279
} else if mac.path == sym!(writeln) {
250-
if let (Some(fmt_str), expr) = check_tts(cx, &mac.args.inner_tokens(), true) {
280+
if let (Some(fmt_str), expr) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
251281
if fmt_str.symbol == Symbol::intern("") {
252282
let mut applicability = Applicability::MachineApplicable;
253283
let suggestion = expr.map_or_else(
@@ -273,6 +303,138 @@ impl EarlyLintPass for Write {
273303
}
274304
}
275305

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

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-
424461
/// Checks if the format string contains a single newline that terminates it.
425462
///
426463
/// Literal and escaped newlines are both checked (only literal for raw strings).

0 commit comments

Comments
 (0)