Skip to content

Commit f0dc603

Browse files
flip1995flip1995
committed
Don't lint debug formatting in debug impl
1 parent e342047 commit f0dc603

File tree

2 files changed

+169
-135
lines changed

2 files changed

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

+168-134
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ 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};
12-
use syntax::ast::{Expr, ExprKind, Mac, StrLit, StrStyle};
12+
use syntax::ast::{Expr, ExprKind, Item, ItemKind, Mac, StrLit, StrStyle};
1313
use syntax::token;
1414
use syntax::tokenstream::TokenStream;
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(
@@ -273,6 +302,138 @@ impl EarlyLintPass for Write {
273302
}
274303
}
275304

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

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),
331-
};
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-
}
337-
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_span, "use of `Debug`-based formatting");
353-
}
354-
args.push(arg);
355-
}
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,
370-
};
371-
if !parser.eat(&token::Comma) {
372-
return (Some(fmtstr), expr);
373-
}
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-
}
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-
},
413-
}
414-
}
415-
if all_simple && seen {
416-
span_lint(cx, lint, rhs.span, "literal with an empty format string");
417-
}
418-
}
419-
}
420-
},
421-
_ => idx += 1,
422-
}
423-
}
424-
}
425-
426460
/// Checks if the format string contains a single newline that terminates it.
427461
///
428462
/// Literal and escaped newlines are both checked (only literal for raw strings).

0 commit comments

Comments
 (0)