diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4f003377a5e5..017e6e2a1423 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -962,7 +962,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf))); store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo)); store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions)); - store.register_early_pass(|| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg)); + store.register_late_pass(|_| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg)); store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf))); store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); diff --git a/clippy_lints/src/literal_string_with_formatting_args.rs b/clippy_lints/src/literal_string_with_formatting_args.rs index 6514a72a44da..378be37fbac5 100644 --- a/clippy_lints/src/literal_string_with_formatting_args.rs +++ b/clippy_lints/src/literal_string_with_formatting_args.rs @@ -1,11 +1,13 @@ -use rustc_ast::ast::{Expr, ExprKind}; -use rustc_ast::token::LitKind; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_ast::{LitKind, StrStyle}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lexer::is_ident; +use rustc_lint::{LateContext, LateLintPass}; use rustc_parse_format::{ParseMode, Parser, Piece}; use rustc_session::declare_lint_pass; -use rustc_span::BytePos; +use rustc_span::{BytePos, Span}; use clippy_utils::diagnostics::span_lint; +use clippy_utils::mir::enclosing_mir; declare_clippy_lint! { /// ### What it does @@ -35,18 +37,65 @@ declare_clippy_lint! { declare_lint_pass!(LiteralStringWithFormattingArg => [LITERAL_STRING_WITH_FORMATTING_ARGS]); -impl EarlyLintPass for LiteralStringWithFormattingArg { - fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { +fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, spans: &[(Span, Option)]) { + if !spans.is_empty() + && let Some(mir) = enclosing_mir(cx.tcx, expr.hir_id) + { + let spans = spans + .iter() + .filter_map(|(span, name)| { + if let Some(name) = name { + // We need to check that the name is a local. + if !mir + .var_debug_info + .iter() + .any(|local| !local.source_info.span.from_expansion() && local.name.as_str() == name) + { + return None; + } + } + Some(*span) + }) + .collect::>(); + match spans.len() { + 0 => {}, + 1 => { + span_lint( + cx, + LITERAL_STRING_WITH_FORMATTING_ARGS, + spans, + "this looks like a formatting argument but it is not part of a formatting macro", + ); + }, + _ => { + span_lint( + cx, + LITERAL_STRING_WITH_FORMATTING_ARGS, + spans, + "these look like formatting arguments but are not part of a formatting macro", + ); + }, + } + } +} + +impl LateLintPass<'_> for LiteralStringWithFormattingArg { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { if expr.span.from_expansion() { return; } if let ExprKind::Lit(lit) = expr.kind { - let add = match lit.kind { - LitKind::Str => 1, - LitKind::StrRaw(nb) => nb as usize + 2, + let (add, symbol) = match lit.node { + LitKind::Str(symbol, style) => { + let add = match style { + StrStyle::Cooked => 1, + StrStyle::Raw(nb) => nb as usize + 2, + }; + (add, symbol) + }, _ => return, }; - let fmt_str = lit.symbol.as_str(); + let fmt_str = symbol.as_str(); let lo = expr.span.lo(); let mut current = fmt_str; let mut diff_len = 0; @@ -74,7 +123,7 @@ impl EarlyLintPass for LiteralStringWithFormattingArg { } if fmt_str[start + 1..].trim_start().starts_with('}') { - // For now, we ignore `{}`. + // We ignore `{}`. continue; } @@ -82,32 +131,29 @@ impl EarlyLintPass for LiteralStringWithFormattingArg { .find('}') .map_or(pos.end, |found| start + 1 + found) + 1; - spans.push( + let ident_start = start + 1; + let colon_pos = fmt_str[ident_start..end].find(':'); + let ident_end = colon_pos.unwrap_or(end - 1); + let mut name = None; + if ident_start < ident_end + && let arg = &fmt_str[ident_start..ident_end] + && !arg.is_empty() + && is_ident(arg) + { + name = Some(arg.to_string()); + } else if colon_pos.is_none() { + // Not a `{:?}`. + continue; + } + spans.push(( expr.span .with_hi(lo + BytePos((start + add).try_into().unwrap())) .with_lo(lo + BytePos((end + add).try_into().unwrap())), - ); + name, + )); } } - match spans.len() { - 0 => {}, - 1 => { - span_lint( - cx, - LITERAL_STRING_WITH_FORMATTING_ARGS, - spans, - "this looks like a formatting argument but it is not part of a formatting macro", - ); - }, - _ => { - span_lint( - cx, - LITERAL_STRING_WITH_FORMATTING_ARGS, - spans, - "these look like formatting arguments but are not part of a formatting macro", - ); - }, - } + emit_lint(cx, expr, &spans); } } } diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index db7710a7a381..03e2a24f6f98 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -18,7 +18,7 @@ clippy::collapsible_else_if, clippy::needless_borrows_for_generic_args, clippy::module_name_repetitions, - clippy::literal_string_with_formatting_arg + clippy::literal_string_with_formatting_args )] mod config; diff --git a/tests/ui/literal_string_with_formatting_arg.rs b/tests/ui/literal_string_with_formatting_arg.rs index fdb8ba606210..20bd243aa300 100644 --- a/tests/ui/literal_string_with_formatting_arg.rs +++ b/tests/ui/literal_string_with_formatting_arg.rs @@ -19,10 +19,12 @@ fn main() { // Should not lint! format!("{y:?}"); println!("{y:?}"); - x.expect(" {} "); // For now we ignore `{}` to limit false positives. - x.expect(" { } "); // For now we ignore `{}` to limit false positives. + x.expect(" {} "); // We ignore `{}` to limit false positives. + x.expect(" { } "); // We ignore `{}` to limit false positives. x.expect("{{y} {x"); x.expect("{{y:?}"); + x.expect(" {0}"); // If it only contains an integer, we ignore it. + x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint x.expect("{y:...}"); let _ = "fn main {\n\ }"; diff --git a/tests/ui/literal_string_with_formatting_arg.stderr b/tests/ui/literal_string_with_formatting_arg.stderr index 515ae978f7a8..32a84f600da7 100644 --- a/tests/ui/literal_string_with_formatting_arg.stderr +++ b/tests/ui/literal_string_with_formatting_arg.stderr @@ -1,65 +1,71 @@ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:7:15 + --> tests/ui/literal_string_with_formatting_arg.rs:7:15 | LL | x.expect("{y} {}"); | ^^^ | - = note: `-D clippy::literal-string-with-formatting-arg` implied by `-D warnings` + = note: `-D clippy::literal-string-with-formatting-args` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::literal_string_with_formatting_args)]` error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:8:16 + --> tests/ui/literal_string_with_formatting_arg.rs:8:16 | LL | x.expect(" {y} bla"); | ^^^ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:9:15 + --> tests/ui/literal_string_with_formatting_arg.rs:9:15 | LL | x.expect("{:?}"); | ^^^^ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:10:15 + --> tests/ui/literal_string_with_formatting_arg.rs:10:15 | LL | x.expect("{y:?}"); | ^^^^^ error: these look like formatting arguments but are not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:11:16 + --> tests/ui/literal_string_with_formatting_arg.rs:11:16 | LL | x.expect(" {y:?} {y:?} "); | ^^^^^ ^^^^^ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:12:23 + --> tests/ui/literal_string_with_formatting_arg.rs:12:23 | LL | x.expect(" {y:..} {y:?} "); | ^^^^^ error: these look like formatting arguments but are not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:13:16 + --> tests/ui/literal_string_with_formatting_arg.rs:13:16 | LL | x.expect(r"{y:?} {y:?} "); | ^^^^^ ^^^^^ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:14:16 + --> tests/ui/literal_string_with_formatting_arg.rs:14:16 | LL | x.expect(r"{y:?} y:?}"); | ^^^^^ error: these look like formatting arguments but are not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:15:19 + --> tests/ui/literal_string_with_formatting_arg.rs:15:19 | LL | x.expect(r##" {y:?} {y:?} "##); | ^^^^^ ^^^^^ error: this looks like a formatting argument but it is not part of a formatting macro - --> tests/ui/literal_string_with_formatting_args.rs:17:18 + --> tests/ui/literal_string_with_formatting_arg.rs:17:18 | LL | x.expect("———{:?}"); | ^^^^ -error: aborting due to 10 previous errors +error: this looks like a formatting argument but it is not part of a formatting macro + --> tests/ui/literal_string_with_formatting_arg.rs:27:19 + | +LL | x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint + | ^^^^^ + +error: aborting due to 11 previous errors diff --git a/tests/ui/regex.rs b/tests/ui/regex.rs index adead5ef33ce..f607a2d50c6d 100644 --- a/tests/ui/regex.rs +++ b/tests/ui/regex.rs @@ -3,8 +3,7 @@ clippy::needless_raw_strings, clippy::needless_raw_string_hashes, clippy::needless_borrow, - clippy::needless_borrows_for_generic_args, - clippy::literal_string_with_formatting_args + clippy::needless_borrows_for_generic_args )] #![warn(clippy::invalid_regex, clippy::trivial_regex, clippy::regex_creation_in_loops)] diff --git a/tests/ui/regex.stderr b/tests/ui/regex.stderr index 919bac8f52f0..18dd538c68b4 100644 --- a/tests/ui/regex.stderr +++ b/tests/ui/regex.stderr @@ -1,5 +1,5 @@ error: trivial regex - --> tests/ui/regex.rs:20:45 + --> tests/ui/regex.rs:19:45 | LL | let pipe_in_wrong_position = Regex::new("|"); | ^^^ @@ -9,7 +9,7 @@ LL | let pipe_in_wrong_position = Regex::new("|"); = help: to override `-D warnings` add `#[allow(clippy::trivial_regex)]` error: trivial regex - --> tests/ui/regex.rs:22:60 + --> tests/ui/regex.rs:21:60 | LL | let pipe_in_wrong_position_builder = RegexBuilder::new("|"); | ^^^ @@ -17,7 +17,7 @@ LL | let pipe_in_wrong_position_builder = RegexBuilder::new("|"); = help: the regex is unlikely to be useful as it is error: regex syntax error: invalid character class range, the start must be <= the end - --> tests/ui/regex.rs:24:42 + --> tests/ui/regex.rs:23:42 | LL | let wrong_char_ranice = Regex::new("[z-a]"); | ^^^ @@ -26,7 +26,7 @@ LL | let wrong_char_ranice = Regex::new("[z-a]"); = help: to override `-D warnings` add `#[allow(clippy::invalid_regex)]` error: regex syntax error: invalid character class range, the start must be <= the end - --> tests/ui/regex.rs:27:37 + --> tests/ui/regex.rs:26:37 | LL | let some_unicode = Regex::new("[é-è]"); | ^^^ @@ -35,13 +35,13 @@ error: regex parse error: ( ^ error: unclosed group - --> tests/ui/regex.rs:30:33 + --> tests/ui/regex.rs:29:33 | LL | let some_regex = Regex::new(OPENING_PAREN); | ^^^^^^^^^^^^^ error: trivial regex - --> tests/ui/regex.rs:32:53 + --> tests/ui/regex.rs:31:53 | LL | let binary_pipe_in_wrong_position = BRegex::new("|"); | ^^^ @@ -52,7 +52,7 @@ error: regex parse error: ( ^ error: unclosed group - --> tests/ui/regex.rs:34:41 + --> tests/ui/regex.rs:33:41 | LL | let some_binary_regex = BRegex::new(OPENING_PAREN); | ^^^^^^^^^^^^^ @@ -61,7 +61,7 @@ error: regex parse error: ( ^ error: unclosed group - --> tests/ui/regex.rs:35:56 + --> tests/ui/regex.rs:34:56 | LL | let some_binary_regex_builder = BRegexBuilder::new(OPENING_PAREN); | ^^^^^^^^^^^^^ @@ -70,7 +70,7 @@ error: regex parse error: ( ^ error: unclosed group - --> tests/ui/regex.rs:47:37 + --> tests/ui/regex.rs:46:37 | LL | let set_error = RegexSet::new(&[OPENING_PAREN, r"[a-z]+\.(com|org|net)"]); | ^^^^^^^^^^^^^ @@ -79,7 +79,7 @@ error: regex parse error: ( ^ error: unclosed group - --> tests/ui/regex.rs:48:39 + --> tests/ui/regex.rs:47:39 | LL | let bset_error = BRegexSet::new(&[OPENING_PAREN, r"[a-z]+\.(com|org|net)"]); | ^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ error: regex parse error: \b\c ^^ error: unrecognized escape sequence - --> tests/ui/regex.rs:55:42 + --> tests/ui/regex.rs:54:42 | LL | let escaped_string_span = Regex::new("\\b\\c"); | ^^^^^^^^ @@ -96,19 +96,19 @@ LL | let escaped_string_span = Regex::new("\\b\\c"); = help: consider using a raw string literal: `r".."` error: regex syntax error: duplicate flag - --> tests/ui/regex.rs:57:34 + --> tests/ui/regex.rs:56:34 | LL | let aux_span = Regex::new("(?ixi)"); | ^ ^ error: regex syntax error: pattern can match invalid UTF-8 - --> tests/ui/regex.rs:63:53 + --> tests/ui/regex.rs:62:53 | LL | let invalid_utf8_should_lint = Regex::new("(?-u)."); | ^ error: trivial regex - --> tests/ui/regex.rs:68:33 + --> tests/ui/regex.rs:67:33 | LL | let trivial_eq = Regex::new("^foobar$"); | ^^^^^^^^^^ @@ -116,7 +116,7 @@ LL | let trivial_eq = Regex::new("^foobar$"); = help: consider using `==` on `str`s error: trivial regex - --> tests/ui/regex.rs:71:48 + --> tests/ui/regex.rs:70:48 | LL | let trivial_eq_builder = RegexBuilder::new("^foobar$"); | ^^^^^^^^^^ @@ -124,7 +124,7 @@ LL | let trivial_eq_builder = RegexBuilder::new("^foobar$"); = help: consider using `==` on `str`s error: trivial regex - --> tests/ui/regex.rs:74:42 + --> tests/ui/regex.rs:73:42 | LL | let trivial_starts_with = Regex::new("^foobar"); | ^^^^^^^^^ @@ -132,7 +132,7 @@ LL | let trivial_starts_with = Regex::new("^foobar"); = help: consider using `str::starts_with` error: trivial regex - --> tests/ui/regex.rs:77:40 + --> tests/ui/regex.rs:76:40 | LL | let trivial_ends_with = Regex::new("foobar$"); | ^^^^^^^^^ @@ -140,7 +140,7 @@ LL | let trivial_ends_with = Regex::new("foobar$"); = help: consider using `str::ends_with` error: trivial regex - --> tests/ui/regex.rs:80:39 + --> tests/ui/regex.rs:79:39 | LL | let trivial_contains = Regex::new("foobar"); | ^^^^^^^^ @@ -148,7 +148,7 @@ LL | let trivial_contains = Regex::new("foobar"); = help: consider using `str::contains` error: trivial regex - --> tests/ui/regex.rs:83:39 + --> tests/ui/regex.rs:82:39 | LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX); | ^^^^^^^^^^^^^^^^ @@ -156,7 +156,7 @@ LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX); = help: consider using `str::contains` error: trivial regex - --> tests/ui/regex.rs:86:40 + --> tests/ui/regex.rs:85:40 | LL | let trivial_backslash = Regex::new("a\\.b"); | ^^^^^^^ @@ -164,7 +164,7 @@ LL | let trivial_backslash = Regex::new("a\\.b"); = help: consider using `str::contains` error: trivial regex - --> tests/ui/regex.rs:90:36 + --> tests/ui/regex.rs:89:36 | LL | let trivial_empty = Regex::new(""); | ^^ @@ -172,7 +172,7 @@ LL | let trivial_empty = Regex::new(""); = help: the regex is unlikely to be useful as it is error: trivial regex - --> tests/ui/regex.rs:93:36 + --> tests/ui/regex.rs:92:36 | LL | let trivial_empty = Regex::new("^"); | ^^^ @@ -180,7 +180,7 @@ LL | let trivial_empty = Regex::new("^"); = help: the regex is unlikely to be useful as it is error: trivial regex - --> tests/ui/regex.rs:96:36 + --> tests/ui/regex.rs:95:36 | LL | let trivial_empty = Regex::new("^$"); | ^^^^ @@ -188,7 +188,7 @@ LL | let trivial_empty = Regex::new("^$"); = help: consider using `str::is_empty` error: trivial regex - --> tests/ui/regex.rs:99:44 + --> tests/ui/regex.rs:98:44 | LL | let binary_trivial_empty = BRegex::new("^$"); | ^^^^