Skip to content

Commit

Permalink
Limit literal_string_with_formatting_args to known variables
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Nov 19, 2024
1 parent 544f30e commit 076d9ca
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 53 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
108 changes: 77 additions & 31 deletions clippy_lints/src/literal_string_with_formatting_args.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -35,18 +37,61 @@ 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.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;
Expand Down Expand Up @@ -82,32 +127,33 @@ 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()
{
if is_ident(arg) {
name = Some(arg.to_string());
} else if colon_pos.is_none() && !arg.chars().all(|c| c.is_ascii_digit()) {
// Not a `{0}` or a `{0:?}`.
continue;
}
} 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 changes: 1 addition & 1 deletion lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/literal_string_with_formatting_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ fn main() {
let y = "hello";
x.expect("{y} {}"); //~ literal_string_with_formatting_args
x.expect(" {y} bla"); //~ literal_string_with_formatting_args
x.expect(" {0}"); //~ literal_string_with_formatting_args
x.expect("{:?}"); //~ literal_string_with_formatting_args
x.expect("{y:?}"); //~ literal_string_with_formatting_args
x.expect(" {y:?} {y:?} "); //~ literal_string_with_formatting_args
Expand All @@ -23,6 +24,7 @@ fn main() {
x.expect(" { } "); // For now we ignore `{}` to limit false positives.
x.expect("{{y} {x");
x.expect("{{y:?}");
x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint
x.expect("{y:...}");
let _ = "fn main {\n\
}";
Expand Down
36 changes: 24 additions & 12 deletions tests/ui/literal_string_with_formatting_arg.stderr
Original file line number Diff line number Diff line change
@@ -1,65 +1,77 @@
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:16
|
LL | x.expect(" {0}");
| ^^^

error: this looks like a formatting argument but it is not part of a formatting macro
--> tests/ui/literal_string_with_formatting_arg.rs:10: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:11: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:12: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:13: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:14: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:15: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:16: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:18: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 12 previous errors

16 changes: 8 additions & 8 deletions tests/ui/regex.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -196,51 +196,51 @@ LL | let binary_trivial_empty = BRegex::new("^$");
= help: consider using `str::is_empty`

error: compiling a regex in a loop
--> tests/ui/regex.rs:125:21
--> tests/ui/regex.rs:126:21
|
LL | let regex = Regex::new("a.b");
| ^^^^^^^^^^
|
help: move the regex construction outside this loop
--> tests/ui/regex.rs:122:5
--> tests/ui/regex.rs:123:5
|
LL | loop {
| ^^^^
= note: `-D clippy::regex-creation-in-loops` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::regex_creation_in_loops)]`

error: compiling a regex in a loop
--> tests/ui/regex.rs:127:21
--> tests/ui/regex.rs:128:21
|
LL | let regex = BRegex::new("a.b");
| ^^^^^^^^^^^
|
help: move the regex construction outside this loop
--> tests/ui/regex.rs:122:5
--> tests/ui/regex.rs:123:5
|
LL | loop {
| ^^^^

error: compiling a regex in a loop
--> tests/ui/regex.rs:133:25
--> tests/ui/regex.rs:134:25
|
LL | let regex = Regex::new("a.b");
| ^^^^^^^^^^
|
help: move the regex construction outside this loop
--> tests/ui/regex.rs:122:5
--> tests/ui/regex.rs:123:5
|
LL | loop {
| ^^^^

error: compiling a regex in a loop
--> tests/ui/regex.rs:138:32
--> tests/ui/regex.rs:139:32
|
LL | let nested_regex = Regex::new("a.b");
| ^^^^^^^^^^
|
help: move the regex construction outside this loop
--> tests/ui/regex.rs:137:9
--> tests/ui/regex.rs:138:9
|
LL | for _ in 0..10 {
| ^^^^^^^^^^^^^^
Expand Down

0 comments on commit 076d9ca

Please sign in to comment.