diff options
| author | Samuel Tardieu <sam@rfc1149.net> | 2025-05-19 15:11:37 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-05-19 15:11:37 +0000 |
| commit | ebc2a68fe8fc8b3fd51dbe56799fc8c6d45c2402 (patch) | |
| tree | 8d01a48c6af1e01c907d0b1c4ea1ccdbc86cf4bc | |
| parent | df33aaf540b24e00b6ad34b11d84c58317660ced (diff) | |
| parent | 0d25090d262cd506d4c4809f3a534d4d8b318870 (diff) | |
| download | rust-ebc2a68fe8fc8b3fd51dbe56799fc8c6d45c2402.tar.gz rust-ebc2a68fe8fc8b3fd51dbe56799fc8c6d45c2402.zip | |
Add new `useless_concat` lint (#13829)
Fixes #13793. Interestingly enough, to actually check that the macro call has at least two arguments, we need to use the rust lexer after getting the original source code snippet. changelog: Add new `useless_concat` lint
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/useless_concat.rs | 104 | ||||
| -rw-r--r-- | clippy_utils/src/paths.rs | 1 | ||||
| -rw-r--r-- | tests/ui/useless_concat.fixed | 41 | ||||
| -rw-r--r-- | tests/ui/useless_concat.rs | 41 | ||||
| -rw-r--r-- | tests/ui/useless_concat.stderr | 89 |
8 files changed, 280 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 28147dfbea3..3a98217f625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6440,6 +6440,7 @@ Released 2018-09-13 [`used_underscore_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_items [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute +[`useless_concat`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_concat [`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion [`useless_format`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_format [`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bb825c7655f..5fcb851dfeb 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -764,6 +764,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::unwrap_in_result::UNWRAP_IN_RESULT_INFO, crate::upper_case_acronyms::UPPER_CASE_ACRONYMS_INFO, crate::use_self::USE_SELF_INFO, + crate::useless_concat::USELESS_CONCAT_INFO, crate::useless_conversion::USELESS_CONVERSION_INFO, crate::vec::USELESS_VEC_INFO, crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 006145cc623..ca1a0b35710 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -393,6 +393,7 @@ mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; mod use_self; +mod useless_concat; mod useless_conversion; mod vec; mod vec_init_then_push; @@ -937,6 +938,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); store.register_early_pass(|| Box::new(empty_line_after::EmptyLineAfter::new())); store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); + store.register_late_pass(|_| Box::new(useless_concat::UselessConcat)); store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern)); store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default()); store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf))); diff --git a/clippy_lints/src/useless_concat.rs b/clippy_lints/src/useless_concat.rs new file mode 100644 index 00000000000..1ed1fbb3b9c --- /dev/null +++ b/clippy_lints/src/useless_concat.rs @@ -0,0 +1,104 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::macros::macro_backtrace; +use clippy_utils::paths::CONCAT; +use clippy_utils::source::snippet_opt; +use clippy_utils::tokenize_with_text; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lexer::TokenKind; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// Checks that the `concat!` macro has at least two arguments. + /// + /// ### Why is this bad? + /// If there are less than 2 arguments, then calling the macro is doing nothing. + /// + /// ### Example + /// ```no_run + /// let x = concat!("a"); + /// ``` + /// Use instead: + /// ```no_run + /// let x = "a"; + /// ``` + #[clippy::version = "1.89.0"] + pub USELESS_CONCAT, + complexity, + "checks that the `concat` macro has at least two arguments" +} + +declare_lint_pass!(UselessConcat => [USELESS_CONCAT]); + +impl LateLintPass<'_> for UselessConcat { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + // Check that the expression is generated by a macro. + if expr.span.from_expansion() + // Check that it's a string literal. + && let ExprKind::Lit(lit) = expr.kind + && let LitKind::Str(lit_s, _) = lit.node + // Get the direct parent of the expression. + && let Some(macro_call) = macro_backtrace(expr.span).next() + // Check if the `concat` macro from the `core` library. + && CONCAT.matches(cx, macro_call.def_id) + // We get the original code to parse it. + && let Some(original_code) = snippet_opt(cx, macro_call.span) + // This check allows us to ensure that the code snippet: + // 1. Doesn't come from proc-macro expansion. + // 2. Doesn't come from foreign macro expansion. + // + // It works as follows: if the snippet we get doesn't contain `concat!(`, then it + // means it's not code written in the current crate so we shouldn't lint. + && let mut parts = original_code.split('!') + && parts.next().is_some_and(|p| p.trim() == "concat") + && parts.next().is_some_and(|p| p.trim().starts_with('(')) + { + let mut literal = None; + let mut nb_commas = 0; + let mut nb_idents = 0; + for (token_kind, token_s, _) in tokenize_with_text(&original_code) { + match token_kind { + TokenKind::Eof => break, + TokenKind::Literal { .. } => { + if literal.is_some() { + return; + } + literal = Some(token_s); + }, + TokenKind::Ident => { + if token_s == "true" || token_s == "false" { + literal = Some(token_s); + } else { + nb_idents += 1; + } + }, + TokenKind::Comma => { + nb_commas += 1; + if nb_commas > 1 { + return; + } + }, + // We're inside a macro definition and we are manipulating something we likely + // shouldn't, so aborting. + TokenKind::Dollar => return, + _ => {}, + } + } + // There should always be the ident of the `concat` macro. + if nb_idents == 1 { + span_lint_and_sugg( + cx, + USELESS_CONCAT, + macro_call.span, + "unneeded use of `concat!` macro", + "replace with", + format!("{lit_s:?}"), + Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index e5179e479cc..9d7f3086b05 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -129,6 +129,7 @@ path_macros! { // Paths in `core`/`alloc`/`std`. This should be avoided and cleaned up by adding diagnostic items. pub static ALIGN_OF: PathLookup = value_path!(core::mem::align_of); pub static CHAR_TO_DIGIT: PathLookup = value_path!(char::to_digit); +pub static CONCAT: PathLookup = macro_path!(core::concat); pub static IO_ERROR_NEW: PathLookup = value_path!(std::io::Error::new); pub static IO_ERRORKIND_OTHER_CTOR: PathLookup = value_path!(std::io::ErrorKind::Other); pub static ITER_STEP: PathLookup = type_path!(core::iter::Step); diff --git a/tests/ui/useless_concat.fixed b/tests/ui/useless_concat.fixed new file mode 100644 index 00000000000..360b6f6ce82 --- /dev/null +++ b/tests/ui/useless_concat.fixed @@ -0,0 +1,41 @@ +//@aux-build:proc_macros.rs + +#![warn(clippy::useless_concat)] +#![allow(clippy::print_literal)] + +extern crate proc_macros; +use proc_macros::{external, with_span}; + +macro_rules! my_concat { + ($fmt:literal $(, $e:expr)*) => { + println!(concat!("ERROR: ", $fmt), $($e,)*); + } +} + +fn main() { + let x = ""; //~ useless_concat + let x = "c"; //~ useless_concat + let x = "\""; //~ useless_concat + let x = "true"; //~ useless_concat + let x = "1"; //~ useless_concat + let x = "1.0000"; //~ useless_concat + let x = "1"; //~ useless_concat + let x = "1"; //~ useless_concat + let x = "1.0000"; //~ useless_concat + let x = "1.0000"; //~ useless_concat + let x = "a😀\n"; //~ useless_concat + let x = "a"; //~ useless_concat + let x = "1"; //~ useless_concat + println!("b: {}", "a"); //~ useless_concat + // Should not lint. + let x = concat!("a", "b"); + let local_i32 = 1; + my_concat!("{}", local_i32); + let x = concat!(file!(), "#L", line!()); + + external! { concat!(); } + with_span! { + span + concat!(); + } +} diff --git a/tests/ui/useless_concat.rs b/tests/ui/useless_concat.rs new file mode 100644 index 00000000000..338d20a48ae --- /dev/null +++ b/tests/ui/useless_concat.rs @@ -0,0 +1,41 @@ +//@aux-build:proc_macros.rs + +#![warn(clippy::useless_concat)] +#![allow(clippy::print_literal)] + +extern crate proc_macros; +use proc_macros::{external, with_span}; + +macro_rules! my_concat { + ($fmt:literal $(, $e:expr)*) => { + println!(concat!("ERROR: ", $fmt), $($e,)*); + } +} + +fn main() { + let x = concat!(); //~ useless_concat + let x = concat!('c'); //~ useless_concat + let x = concat!('"'); //~ useless_concat + let x = concat!(true); //~ useless_concat + let x = concat!(1f32); //~ useless_concat + let x = concat!(1.0000f32); //~ useless_concat + let x = concat!(1_f32); //~ useless_concat + let x = concat!(1_); //~ useless_concat + let x = concat!(1.0000_f32); //~ useless_concat + let x = concat!(1.0000_); //~ useless_concat + let x = concat!("a\u{1f600}\n"); //~ useless_concat + let x = concat!(r##"a"##); //~ useless_concat + let x = concat!(1); //~ useless_concat + println!("b: {}", concat!("a")); //~ useless_concat + // Should not lint. + let x = concat!("a", "b"); + let local_i32 = 1; + my_concat!("{}", local_i32); + let x = concat!(file!(), "#L", line!()); + + external! { concat!(); } + with_span! { + span + concat!(); + } +} diff --git a/tests/ui/useless_concat.stderr b/tests/ui/useless_concat.stderr new file mode 100644 index 00000000000..43d6d9ff579 --- /dev/null +++ b/tests/ui/useless_concat.stderr @@ -0,0 +1,89 @@ +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:16:13 + | +LL | let x = concat!(); + | ^^^^^^^^^ help: replace with: `""` + | + = note: `-D clippy::useless-concat` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::useless_concat)]` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:17:13 + | +LL | let x = concat!('c'); + | ^^^^^^^^^^^^ help: replace with: `"c"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:18:13 + | +LL | let x = concat!('"'); + | ^^^^^^^^^^^^ help: replace with: `"\""` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:19:13 + | +LL | let x = concat!(true); + | ^^^^^^^^^^^^^ help: replace with: `"true"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:20:13 + | +LL | let x = concat!(1f32); + | ^^^^^^^^^^^^^ help: replace with: `"1"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:21:13 + | +LL | let x = concat!(1.0000f32); + | ^^^^^^^^^^^^^^^^^^ help: replace with: `"1.0000"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:22:13 + | +LL | let x = concat!(1_f32); + | ^^^^^^^^^^^^^^ help: replace with: `"1"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:23:13 + | +LL | let x = concat!(1_); + | ^^^^^^^^^^^ help: replace with: `"1"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:24:13 + | +LL | let x = concat!(1.0000_f32); + | ^^^^^^^^^^^^^^^^^^^ help: replace with: `"1.0000"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:25:13 + | +LL | let x = concat!(1.0000_); + | ^^^^^^^^^^^^^^^^ help: replace with: `"1.0000"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:26:13 + | +LL | let x = concat!("a\u{1f600}\n"); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `"a😀\n"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:27:13 + | +LL | let x = concat!(r##"a"##); + | ^^^^^^^^^^^^^^^^^ help: replace with: `"a"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:28:13 + | +LL | let x = concat!(1); + | ^^^^^^^^^^ help: replace with: `"1"` + +error: unneeded use of `concat!` macro + --> tests/ui/useless_concat.rs:29:23 + | +LL | println!("b: {}", concat!("a")); + | ^^^^^^^^^^^^ help: replace with: `"a"` + +error: aborting due to 14 previous errors + |
