Fix literal_string_with_formatting_args lint emitted when it should not (rust-lang#13953)

Fixes rust-lang#13885.
Fixes rust-lang#14007.

Problem was that I forgot to check whether or not the `span` was a
"real" one. Because if not, then it starts pointing to pretty much only
wrong content, hence the problems we saw with clippy linting on
`clippy.toml`.

changelog: Fix `literal_string_with_formatting_args` lint emitted when
it should not

r? @samueltardieu

Author: y21

clippy_lints/src/literal_string_with_formatting_args.rs

@@ -7,6 +7,7 @@ use rustc_session::declare_lint_pass;
 use rustc_span::{BytePos, Span};
 
 use clippy_utils::diagnostics::span_lint;
+use clippy_utils::is_from_proc_macro;
 use clippy_utils::mir::enclosing_mir;
 
 declare_clippy_lint! {
@@ -79,9 +80,9 @@ fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, spans: &[(Span, Option<Strin
     }
 }
 
-impl LateLintPass<'_> for LiteralStringWithFormattingArg {
-    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
-        if expr.span.from_expansion() {
+impl<'tcx> LateLintPass<'tcx> for LiteralStringWithFormattingArg {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if expr.span.from_expansion() || expr.span.is_dummy() {
             return;
         }
         if let ExprKind::Lit(lit) = expr.kind {
@@ -95,6 +96,9 @@ impl LateLintPass<'_> for LiteralStringWithFormattingArg {
                 },
                 _ => return,
             };
+            if is_from_proc_macro(cx, expr) {
+                return;
+            }
             let fmt_str = symbol.as_str();
             let lo = expr.span.lo();
             let mut current = fmt_str;
@@ -124,7 +128,11 @@ impl LateLintPass<'_> for LiteralStringWithFormattingArg {
                     pos.start += diff_len;
                     pos.end += diff_len;
 
-                    let start = fmt_str[..pos.start].rfind('{').unwrap_or(pos.start);
+                    let mut start = pos.start;
+                    while start < fmt_str.len() && !fmt_str.is_char_boundary(start) {
+                        start += 1;
+                    }
+                    let start = fmt_str[..start].rfind('{').unwrap_or(start);
                     // If this is a unicode character escape, we don't want to lint.
                     if start > 1 && fmt_str[..start].ends_with("\\u") {
                         continue;

tests/ui/literal_string_with_formatting_arg.rs

@@ -1,6 +1,31 @@
 #![warn(clippy::literal_string_with_formatting_args)]
 #![allow(clippy::unnecessary_literal_unwrap)]
 
+// Regression test for <https://github.com/rust-lang/rust-clippy/issues/13885>.
+// It's not supposed to emit the lint in this case (in `assert!` expansion).
+fn compiler_macro() {
+    fn parse(_: &str) -> Result<(), i32> {
+        unimplemented!()
+    }
+
+    assert!(
+        parse(
+            #[allow(clippy::literal_string_with_formatting_args)]
+            "foo {:}"
+        )
+        .is_err()
+    );
+    let value = 0;
+    assert!(format!("{value}").is_ascii());
+}
+
+// Regression test for <https://github.com/rust-lang/rust-clippy/issues/14007>.
+fn regression_14007() {
+    let s = "{и}";
+    let ш = 12;
+    let s = "{ш}"; //~ literal_string_with_formatting_args
+}
+
 fn main() {
     let x: Option<usize> = None;
     let y = "hello";
@@ -13,6 +38,7 @@ fn main() {
     x.expect(r"{y:?}  {y:?} "); //~ literal_string_with_formatting_args
     x.expect(r"{y:?} y:?}"); //~ literal_string_with_formatting_args
     x.expect(r##" {y:?} {y:?} "##); //~ literal_string_with_formatting_args
+    assert!("{y}".is_ascii()); //~ literal_string_with_formatting_args
     // Ensure that it doesn't try to go in the middle of a unicode character.
     x.expect("———{:?}"); //~ literal_string_with_formatting_args
 

tests/ui/literal_string_with_formatting_arg.stderr

@@ -1,71 +1,83 @@
 error: this looks like a formatting argument but it is not part of a formatting macro
-  --> tests/ui/literal_string_with_formatting_arg.rs:7:15
+  --> tests/ui/literal_string_with_formatting_arg.rs:26:14
    |
-LL |     x.expect("{y} {}");
-   |               ^^^
+LL |     let s = "{ш}";
+   |              ^^^
    |
    = 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_arg.rs:8:16
+  --> tests/ui/literal_string_with_formatting_arg.rs:32:15
+   |
+LL |     x.expect("{y} {}");
+   |               ^^^
+
+error: this looks like a formatting argument but it is not part of a formatting macro
+  --> tests/ui/literal_string_with_formatting_arg.rs:33: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_arg.rs:9:15
+  --> tests/ui/literal_string_with_formatting_arg.rs:34: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_arg.rs:10:15
+  --> tests/ui/literal_string_with_formatting_arg.rs:35: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_arg.rs:11:16
+  --> tests/ui/literal_string_with_formatting_arg.rs:36: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_arg.rs:12:23
+  --> tests/ui/literal_string_with_formatting_arg.rs:37: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_arg.rs:13:16
+  --> tests/ui/literal_string_with_formatting_arg.rs:38: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_arg.rs:14:16
+  --> tests/ui/literal_string_with_formatting_arg.rs:39: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_arg.rs:15:19
+  --> tests/ui/literal_string_with_formatting_arg.rs:40: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_arg.rs:17:18
+  --> tests/ui/literal_string_with_formatting_arg.rs:41:14
+   |
+LL |     assert!("{y}".is_ascii());
+   |              ^^^
+
+error: this looks like a formatting argument but it is not part of a formatting macro
+  --> tests/ui/literal_string_with_formatting_arg.rs:43:18
    |
 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_arg.rs:27:19
+  --> tests/ui/literal_string_with_formatting_arg.rs:53:19
    |
 LL |     x.expect(r##" {x:?} "##); // `x` doesn't exist so we shoud not lint
    |                   ^^^^^
 
-error: aborting due to 11 previous errors
+error: aborting due to 13 previous errors
 

tests/ui/{literal_string_with_formatting_args}.rs

@@ -0,0 +1,46 @@
+// Regression test for <https://github.com/rust-lang/rust-clippy/issues/13885>.
+// The `dbg` macro generates a literal with the name of the current file, so
+// we need to ensure the lint is not emitted in this case.
+
+// Clippy sets `-Zflatten_format_args=no`, which changes the default behavior of how format args
+// are lowered and only that one has this non-macro span. Adding the flag makes it repro on
+// godbolt and shows a root context span for the file name string.
+//
+// So instead of having:
+//
+// ```
+// Lit(
+//     Spanned {
+//         node: Str(
+//             "[/app/example.rs:2:5] \"something\" = ",
+//             Cooked,
+//         ),
+//         span: /rustc/eb54a50837ad4bcc9842924f27e7287ca66e294c/library/std/src/macros.rs:365:35: 365:58 (#4),
+//     },
+// ),
+// ```
+//
+// We get:
+//
+// ```
+// Lit(
+//     Spanned {
+//         node: Str(
+//             "/app/example.rs",
+//             Cooked,
+//         ),
+//         span: /app/example.rs:2:5: 2:22 (#0),
+//     },
+// )
+// ```
+
+#![crate_name = "foo"]
+#![allow(unused)]
+#![warn(clippy::literal_string_with_formatting_args)]
+
+fn another_bad() {
+    let literal_string_with_formatting_args = 0;
+    dbg!("something");
+}
+
+fn main() {}