Skip to content

Commit a628733

Browse files
flip1995flip1995
committed
Don't lint debug formatting in debug impl
1 parent a94b2c1 commit a628733

File tree

2 files changed

+157
-121
lines changed

2 files changed

+157
-121
lines changed

‎clippy_lints/src/lib.rs

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

‎clippy_lints/src/write.rs

+156-120
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ use std::borrow::Cow;
22
use std::ops::Range;
33

44
use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
5-
use rustc_ast::ast::{Expr, ExprKind, Mac, StrLit, StrStyle};
5+
use rustc_ast::ast::{Expr, ExprKind, Item, ItemKind, Mac, StrLit, StrStyle};
66
use rustc_ast::token;
77
use rustc_ast::tokenstream::TokenStream;
88
use rustc_errors::Applicability;
99
use rustc_lexer::unescape::{self, EscapeError};
1010
use rustc_lint::{EarlyContext, EarlyLintPass};
1111
use rustc_parse::parser;
12-
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
use rustc_session::{declare_tool_lint, impl_lint_pass};
1313
use rustc_span::symbol::Symbol;
1414
use rustc_span::{BytePos, Span};
1515

@@ -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,34 @@ 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+
if trait_name == sym!(Debug) {
210+
self.in_debug_impl = true;
211+
}
212+
}
213+
}
214+
215+
fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
216+
self.in_debug_impl = false;
217+
}
218+
190219
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
191220
if mac.path == sym!(println) {
192221
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`");
193-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
222+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
194223
if fmt_str.symbol == Symbol::intern("") {
195224
span_lint_and_sugg(
196225
cx,
@@ -205,7 +234,7 @@ impl EarlyLintPass for Write {
205234
}
206235
} else if mac.path == sym!(print) {
207236
span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`");
208-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), false) {
237+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), false) {
209238
if check_newlines(&fmt_str) {
210239
span_lint_and_then(
211240
cx,
@@ -226,7 +255,7 @@ impl EarlyLintPass for Write {
226255
}
227256
}
228257
} else if mac.path == sym!(write) {
229-
if let (Some(fmt_str), _) = check_tts(cx, &mac.args.inner_tokens(), true) {
258+
if let (Some(fmt_str), _) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
230259
if check_newlines(&fmt_str) {
231260
span_lint_and_then(
232261
cx,
@@ -247,7 +276,7 @@ impl EarlyLintPass for Write {
247276
}
248277
}
249278
} else if mac.path == sym!(writeln) {
250-
if let (Some(fmt_str), expr) = check_tts(cx, &mac.args.inner_tokens(), true) {
279+
if let (Some(fmt_str), expr) = self.check_tts(cx, &mac.args.inner_tokens(), true) {
251280
if fmt_str.symbol == Symbol::intern("") {
252281
let mut applicability = Applicability::MachineApplicable;
253282
let suggestion = expr.map_or_else(
@@ -296,129 +325,136 @@ fn newline_span(fmtstr: &StrLit) -> Span {
296325
sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
297326
}
298327

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-
AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, Parser, Piece,
322-
};
323-
let tts = tts.clone();
324-
325-
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
326-
let mut expr: Option<Expr> = None;
327-
if is_write {
328-
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
329-
Ok(p) => Some(p.into_inner()),
330-
Err(_) => return (None, None),
328+
impl Write {
329+
/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
330+
/// `Option`s. The first `Option` of the tuple is the macro's format string. It includes
331+
/// the contents of the string, whether it's a raw string, and the span of the literal in the
332+
/// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the
333+
/// `format_str` should be written to.
334+
///
335+
/// Example:
336+
///
337+
/// Calling this function on
338+
/// ```rust
339+
/// # use std::fmt::Write;
340+
/// # let mut buf = String::new();
341+
/// # let something = "something";
342+
/// writeln!(buf, "string to write: {}", something);
343+
/// ```
344+
/// will return
345+
/// ```rust,ignore
346+
/// (Some("string to write: {}"), Some(buf))
347+
/// ```
348+
#[allow(clippy::too_many_lines)]
349+
fn check_tts<'a>(
350+
&self,
351+
cx: &EarlyContext<'a>,
352+
tts: &TokenStream,
353+
is_write: bool,
354+
) -> (Option<StrLit>, Option<Expr>) {
355+
use fmt_macros::{
356+
AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, Parser, Piece,
331357
};
332-
// might be `writeln!(foo)`
333-
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
334-
return (None, expr);
335-
}
336-
}
358+
let tts = tts.clone();
337359

338-
let fmtstr = match parser.parse_str_lit() {
339-
Ok(fmtstr) => fmtstr,
340-
Err(_) => return (None, expr),
341-
};
342-
let tmp = fmtstr.symbol.as_str();
343-
let mut args = vec![];
344-
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
345-
while let Some(piece) = fmt_parser.next() {
346-
if !fmt_parser.errors.is_empty() {
347-
return (None, expr);
348-
}
349-
if let Piece::NextArgument(arg) = piece {
350-
if arg.format.ty == "?" {
351-
// FIXME: modify rustc's fmt string parser to give us the current span
352-
span_lint(cx, USE_DEBUG, parser.prev_token.span, "use of `Debug`-based formatting");
360+
let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
361+
let mut expr: Option<Expr> = None;
362+
if is_write {
363+
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
364+
Ok(p) => Some(p.into_inner()),
365+
Err(_) => return (None, None),
366+
};
367+
// might be `writeln!(foo)`
368+
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
369+
return (None, expr);
353370
}
354-
args.push(arg);
355371
}
356-
}
357-
let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
358-
let mut idx = 0;
359-
loop {
360-
const SIMPLE: FormatSpec<'_> = FormatSpec {
361-
fill: None,
362-
align: AlignUnknown,
363-
flags: 0,
364-
precision: CountImplied,
365-
precision_span: None,
366-
width: CountImplied,
367-
width_span: None,
368-
ty: "",
369-
ty_span: None,
372+
373+
let fmtstr = match parser.parse_str_lit() {
374+
Ok(fmtstr) => fmtstr,
375+
Err(_) => return (None, expr),
370376
};
371-
if !parser.eat(&token::Comma) {
372-
return (Some(fmtstr), expr);
377+
let tmp = fmtstr.symbol.as_str();
378+
let mut args = vec![];
379+
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
380+
while let Some(piece) = fmt_parser.next() {
381+
if !fmt_parser.errors.is_empty() {
382+
return (None, expr);
383+
}
384+
if let Piece::NextArgument(arg) = piece {
385+
if !self.in_debug_impl && arg.format.ty == "?" {
386+
// FIXME: modify rustc's fmt string parser to give us the current span
387+
span_lint(cx, USE_DEBUG, parser.prev_token.span, "use of `Debug`-based formatting");
388+
}
389+
args.push(arg);
390+
}
373391
}
374-
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
375-
expr
376-
} else {
377-
return (Some(fmtstr), None);
378-
};
379-
match &token_expr.kind {
380-
ExprKind::Lit(_) => {
381-
let mut all_simple = true;
382-
let mut seen = false;
383-
for arg in &args {
384-
match arg.position {
385-
ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
386-
if n == idx {
387-
all_simple &= arg.format == SIMPLE;
388-
seen = true;
389-
}
390-
},
391-
ArgumentNamed(_) => {},
392+
let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
393+
let mut idx = 0;
394+
loop {
395+
const SIMPLE: FormatSpec<'_> = FormatSpec {
396+
fill: None,
397+
align: AlignUnknown,
398+
flags: 0,
399+
precision: CountImplied,
400+
precision_span: None,
401+
width: CountImplied,
402+
width_span: None,
403+
ty: "",
404+
ty_span: None,
405+
};
406+
if !parser.eat(&token::Comma) {
407+
return (Some(fmtstr), expr);
408+
}
409+
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
410+
expr
411+
} else {
412+
return (Some(fmtstr), None);
413+
};
414+
match &token_expr.kind {
415+
ExprKind::Lit(_) => {
416+
let mut all_simple = true;
417+
let mut seen = false;
418+
for arg in &args {
419+
match arg.position {
420+
ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
421+
if n == idx {
422+
all_simple &= arg.format == SIMPLE;
423+
seen = true;
424+
}
425+
},
426+
ArgumentNamed(_) => {},
427+
}
392428
}
393-
}
394-
if all_simple && seen {
395-
span_lint(cx, lint, token_expr.span, "literal with an empty format string");
396-
}
397-
idx += 1;
398-
},
399-
ExprKind::Assign(lhs, rhs, _) => {
400-
if let ExprKind::Lit(_) = rhs.kind {
401-
if let ExprKind::Path(_, p) = &lhs.kind {
402-
let mut all_simple = true;
403-
let mut seen = false;
404-
for arg in &args {
405-
match arg.position {
406-
ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
407-
ArgumentNamed(name) => {
408-
if *p == name {
409-
seen = true;
410-
all_simple &= arg.format == SIMPLE;
411-
}
412-
},
429+
if all_simple && seen {
430+
span_lint(cx, lint, token_expr.span, "literal with an empty format string");
431+
}
432+
idx += 1;
433+
},
434+
ExprKind::Assign(lhs, rhs, _) => {
435+
if let ExprKind::Lit(_) = rhs.kind {
436+
if let ExprKind::Path(_, p) = &lhs.kind {
437+
let mut all_simple = true;
438+
let mut seen = false;
439+
for arg in &args {
440+
match arg.position {
441+
ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
442+
ArgumentNamed(name) => {
443+
if *p == name {
444+
seen = true;
445+
all_simple &= arg.format == SIMPLE;
446+
}
447+
},
448+
}
449+
}
450+
if all_simple && seen {
451+
span_lint(cx, lint, rhs.span, "literal with an empty format string");
413452
}
414-
}
415-
if all_simple && seen {
416-
span_lint(cx, lint, rhs.span, "literal with an empty format string");
417453
}
418454
}
419-
}
420-
},
421-
_ => idx += 1,
455+
},
456+
_ => idx += 1,
457+
}
422458
}
423459
}
424460
}

0 commit comments

Comments
 (0)