Add new useless_concat lint (rust-lang#13829)

Fixes rust-lang#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

Author: samueltardieu

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

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,

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

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,
+                );
+            }
+        }
+    }
+}

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

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!();
+    }
+}

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!();
+    }
+}

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
+