user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Limit literal_string_with_formatting_args to known variables if no formatting argument is passed

Browse source
This commit is contained in:
Guillaume Gomez 2024-11-19 18:25:12 +01:00
parent 607a3f6c08
commit cfc6444f84
7 changed files with 127 additions and 74 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/lib.rs
View file

@ -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));

110
clippy_lints/src/literal_string_with_formatting_args.rs
View file

@ -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<String>)]) {
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::<Vec<_>>();
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);
}
}
}

2
lintcheck/src/main.rs
View file

@ -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;

6
tests/ui/literal_string_with_formatting_arg.rs
View file

@ -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\
}";

30
tests/ui/literal_string_with_formatting_arg.stderr
View file

@ -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

3
tests/ui/regex.rs
View file

@ -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)]

48
tests/ui/regex.stderr
View file

@ -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("^$");
| ^^^^