Skip to content

Commit

Permalink
Add new literal_string_with_formatting_args lint (#13410)
Browse files Browse the repository at this point in the history
Fixes #10195.

changelog: Added new [`literal_string_with_formatting_args`] `pedantic`
lint
[#13410](#13410)
  • Loading branch information
xFrednet authored Dec 1, 2024
2 parents 650e0c8 + cfc6444 commit 1f966e9
Show file tree
Hide file tree
Showing 20 changed files with 308 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5685,6 +5685,7 @@ Released 2018-09-13
[`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
[`lint_groups_priority`]: https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority
[`literal_string_with_formatting_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#literal_string_with_formatting_args
[`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::literal_representation::MISTYPED_LITERAL_SUFFIXES_INFO,
crate::literal_representation::UNREADABLE_LITERAL_INFO,
crate::literal_representation::UNUSUAL_BYTE_GROUPINGS_INFO,
crate::literal_string_with_formatting_args::LITERAL_STRING_WITH_FORMATTING_ARGS_INFO,
crate::loops::EMPTY_LOOP_INFO,
crate::loops::EXPLICIT_COUNTER_LOOP_INFO,
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
clippy::missing_docs_in_private_items,
clippy::must_use_candidate,
rustc::diagnostic_outside_of_impl,
rustc::untranslatable_diagnostic
rustc::untranslatable_diagnostic,
clippy::literal_string_with_formatting_args
)]
#![warn(
trivial_casts,
Expand Down Expand Up @@ -49,6 +50,7 @@ extern crate rustc_lexer;
extern crate rustc_lint;
extern crate rustc_middle;
extern crate rustc_parse;
extern crate rustc_parse_format;
extern crate rustc_resolve;
extern crate rustc_session;
extern crate rustc_span;
Expand Down Expand Up @@ -196,6 +198,7 @@ mod let_with_type_underscore;
mod lifetimes;
mod lines_filter_map_ok;
mod literal_representation;
mod literal_string_with_formatting_args;
mod loops;
mod macro_metavars_in_unsafe;
mod macro_use;
Expand Down Expand Up @@ -959,6 +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_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
159 changes: 159 additions & 0 deletions clippy_lints/src/literal_string_with_formatting_args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
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, Span};

use clippy_utils::diagnostics::span_lint;
use clippy_utils::mir::enclosing_mir;

declare_clippy_lint! {
/// ### What it does
/// Checks if string literals have formatting arguments outside of macros
/// using them (like `format!`).
///
/// ### Why is this bad?
/// It will likely not generate the expected content.
///
/// ### Example
/// ```no_run
/// let x: Option<usize> = None;
/// let y = "hello";
/// x.expect("{y:?}");
/// ```
/// Use instead:
/// ```no_run
/// let x: Option<usize> = None;
/// let y = "hello";
/// x.expect(&format!("{y:?}"));
/// ```
#[clippy::version = "1.83.0"]
pub LITERAL_STRING_WITH_FORMATTING_ARGS,
suspicious,
"Checks if string literals have formatting arguments"
}

declare_lint_pass!(LiteralStringWithFormattingArg => [LITERAL_STRING_WITH_FORMATTING_ARGS]);

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, 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 = symbol.as_str();
let lo = expr.span.lo();
let mut current = fmt_str;
let mut diff_len = 0;

let mut parser = Parser::new(current, None, None, false, ParseMode::Format);
let mut spans = Vec::new();
while let Some(piece) = parser.next() {
if let Some(error) = parser.errors.last() {
// We simply ignore the errors and move after them.
if error.span.end >= current.len() {
break;
}
current = &current[error.span.end + 1..];
diff_len = fmt_str.len() - current.len();
parser = Parser::new(current, None, None, false, ParseMode::Format);
} else if let Piece::NextArgument(arg) = piece {
let mut pos = arg.position_span;
pos.start += diff_len;
pos.end += diff_len;

let start = fmt_str[..pos.start].rfind('{').unwrap_or(pos.start);
// If this is a unicode character escape, we don't want to lint.
if start > 1 && fmt_str[..start].ends_with("\\u") {
continue;
}

if fmt_str[start + 1..].trim_start().starts_with('}') {
// We ignore `{}`.
continue;
}

let end = fmt_str[start + 1..]
.find('}')
.map_or(pos.end, |found| start + 1 + found)
+ 1;
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,
));
}
}
emit_lint(cx, expr, &spans);
}
}
}
3 changes: 2 additions & 1 deletion lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
#![allow(
clippy::collapsible_else_if,
clippy::needless_borrows_for_generic_args,
clippy::module_name_repetitions
clippy::module_name_repetitions,
clippy::literal_string_with_formatting_args
)]

mod config;
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/large_include_file/large_include_file.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::large_include_file)]
#![allow(clippy::literal_string_with_formatting_args)]

// Good
const GOOD_INCLUDE_BYTES: &[u8; 68] = include_bytes!("../../ui/author.rs");
Expand Down
6 changes: 3 additions & 3 deletions tests/ui-toml/large_include_file/large_include_file.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: attempted to include a large file
--> tests/ui-toml/large_include_file/large_include_file.rs:13:43
--> tests/ui-toml/large_include_file/large_include_file.rs:14:43
|
LL | const TOO_BIG_INCLUDE_BYTES: &[u8; 654] = include_bytes!("too_big.txt");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -9,15 +9,15 @@ LL | const TOO_BIG_INCLUDE_BYTES: &[u8; 654] = include_bytes!("too_big.txt");
= help: to override `-D warnings` add `#[allow(clippy::large_include_file)]`

error: attempted to include a large file
--> tests/ui-toml/large_include_file/large_include_file.rs:15:35
--> tests/ui-toml/large_include_file/large_include_file.rs:16:35
|
LL | const TOO_BIG_INCLUDE_STR: &str = include_str!("too_big.txt");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the configuration allows a maximum size of 600 bytes

error: attempted to include a large file
--> tests/ui-toml/large_include_file/large_include_file.rs:18:1
--> tests/ui-toml/large_include_file/large_include_file.rs:19:1
|
LL | #[doc = include_str!("too_big.txt")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
1 change: 1 addition & 0 deletions tests/ui/auxiliary/proc_macro_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![allow(incomplete_features)]
#![allow(clippy::field_reassign_with_default)]
#![allow(clippy::eq_op)]
#![allow(clippy::literal_string_with_formatting_args)]

extern crate proc_macro;

Expand Down
3 changes: 2 additions & 1 deletion tests/ui/format.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
clippy::needless_borrow,
clippy::uninlined_format_args,
clippy::needless_raw_string_hashes,
clippy::useless_vec
clippy::useless_vec,
clippy::literal_string_with_formatting_args
)]

struct Foo(pub String);
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
clippy::needless_borrow,
clippy::uninlined_format_args,
clippy::needless_raw_string_hashes,
clippy::useless_vec
clippy::useless_vec,
clippy::literal_string_with_formatting_args
)]

struct Foo(pub String);
Expand Down
30 changes: 15 additions & 15 deletions tests/ui/format.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: useless use of `format!`
--> tests/ui/format.rs:19:5
--> tests/ui/format.rs:20:5
|
LL | format!("foo");
| ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()`
Expand All @@ -8,19 +8,19 @@ LL | format!("foo");
= help: to override `-D warnings` add `#[allow(clippy::useless_format)]`

error: useless use of `format!`
--> tests/ui/format.rs:20:5
--> tests/ui/format.rs:21:5
|
LL | format!("{{}}");
| ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{}".to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:21:5
--> tests/ui/format.rs:22:5
|
LL | format!("{{}} abc {{}}");
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{} abc {}".to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:22:5
--> tests/ui/format.rs:23:5
|
LL | / format!(
LL | | r##"foo {{}}
Expand All @@ -35,67 +35,67 @@ LL ~ " bar"##.to_string();
|

error: useless use of `format!`
--> tests/ui/format.rs:27:13
--> tests/ui/format.rs:28:13
|
LL | let _ = format!("");
| ^^^^^^^^^^^ help: consider using `String::new()`: `String::new()`

error: useless use of `format!`
--> tests/ui/format.rs:29:5
--> tests/ui/format.rs:30:5
|
LL | format!("{}", "foo");
| ^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:37:5
--> tests/ui/format.rs:38:5
|
LL | format!("{}", arg);
| ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `arg.to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:67:5
--> tests/ui/format.rs:68:5
|
LL | format!("{}", 42.to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `42.to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:69:5
--> tests/ui/format.rs:70:5
|
LL | format!("{}", x.display().to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.display().to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:73:18
--> tests/ui/format.rs:74:18
|
LL | let _ = Some(format!("{}", a + "bar"));
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `a + "bar"`

error: useless use of `format!`
--> tests/ui/format.rs:77:22
--> tests/ui/format.rs:78:22
|
LL | let _s: String = format!("{}", &*v.join("\n"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `(&*v.join("\n")).to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:83:13
--> tests/ui/format.rs:84:13
|
LL | let _ = format!("{x}");
| ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:85:13
--> tests/ui/format.rs:86:13
|
LL | let _ = format!("{y}", y = x);
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:89:13
--> tests/ui/format.rs:90:13
|
LL | let _ = format!("{abc}");
| ^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `abc.to_string()`

error: useless use of `format!`
--> tests/ui/format.rs:91:13
--> tests/ui/format.rs:92:13
|
LL | let _ = format!("{xx}");
| ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `xx.to_string()`
Expand Down
Loading

0 comments on commit 1f966e9

Please sign in to comment.