Add debug assertions for empty replacements and overlapping spans

clippy_lints/src/from_over_into.rs

@@ -181,9 +181,6 @@ fn convert_to_from(
     let from = self_ty.span.get_source_text(cx)?;
     let into = target_ty.span.get_source_text(cx)?;
 
-    let return_type = matches!(sig.decl.output, FnRetTy::Return(_))
-        .then_some(String::from("Self"))
-        .unwrap_or_default();
     let mut suggestions = vec![
         // impl Into<T> for U  ->  impl From<T> for U
         //      ~~~~                    ~~~~
@@ -200,10 +197,13 @@ fn convert_to_from(
         // fn into([mut] self) -> T  ->  fn into([mut] v: T) -> T
         //               ~~~~                          ~~~~
         (self_ident.span, format!("val: {from}")),
+    ];
+
+    if let FnRetTy::Return(_) = sig.decl.output {
         // fn into(self) -> T  ->  fn into(self) -> Self
         //                  ~                       ~~~~
-        (sig.decl.output.span(), return_type),
-    ];
+        suggestions.push((sig.decl.output.span(), String::from("Self")));
+    }
 
     let mut finder = SelfFinder {
         cx,

clippy_lints/src/semicolon_block.rs

@@ -101,17 +101,21 @@ impl SemicolonBlock {
         );
     }
 
-    fn semicolon_outside_block(
-        &self,
-        cx: &LateContext<'_>,
-        block: &Block<'_>,
-        tail_stmt_expr: &Expr<'_>,
-        semi_span: Span,
-    ) {
+    fn semicolon_outside_block(&self, cx: &LateContext<'_>, block: &Block<'_>, tail_stmt_expr: &Expr<'_>) {
         let insert_span = block.span.with_lo(block.span.hi());
-        // account for macro calls
-        let semi_span = cx.sess().source_map().stmt_span(semi_span, block.span);
-        let remove_span = semi_span.with_lo(tail_stmt_expr.span.source_callsite().hi());
+
+        // For macro call semicolon statements (`mac!();`), the statement's span does not actually
+        // include the semicolon itself, so use `mac_call_stmt_semi_span`, which finds the semicolon
+        // based on a source snippet.
+        // (Does not use `stmt_span` as that requires `.from_expansion()` to return true,
+        // which is not the case for e.g. `line!();` and `asm!();`)
+        let Some(remove_span) = cx
+            .sess()
+            .source_map()
+            .mac_call_stmt_semi_span(tail_stmt_expr.span.source_callsite())
+        else {
+            return;
+        };
 
         if self.semicolon_outside_block_ignore_multiline && get_line(cx, remove_span) != get_line(cx, insert_span) {
             return;
@@ -150,13 +154,12 @@ impl LateLintPass<'_> for SemicolonBlock {
                 };
                 let &Stmt {
                     kind: StmtKind::Semi(expr),
-                    span,
                     ..
                 } = stmt
                 else {
                     return;
                 };
-                self.semicolon_outside_block(cx, block, expr, span);
+                self.semicolon_outside_block(cx, block, expr);
             },
             StmtKind::Semi(Expr {
                 kind: ExprKind::Block(block @ Block { expr: Some(tail), .. }, _),

clippy_utils/src/diagnostics.rs

@@ -8,7 +8,9 @@
 //! Thank you!
 //! ~The `INTERNAL_METADATA_COLLECTOR` lint
 
-use rustc_errors::{Applicability, Diag, DiagMessage, MultiSpan, SubdiagMessage};
+use rustc_errors::{
+    Applicability, Diag, DiagMessage, EmissionGuarantee, MultiSpan, SubdiagMessage, SubstitutionPart, Suggestions,
+};
 use rustc_hir::HirId;
 use rustc_lint::{LateContext, Lint, LintContext};
 use rustc_span::Span;
@@ -28,6 +30,42 @@ fn docs_link(diag: &mut Diag<'_, ()>, lint: &'static Lint) {
     }
 }
 
+/// Makes sure that a diagnostic is well formed.
+///
+/// rustc debug asserts a few properties about spans,
+/// but the clippy repo uses a distributed rustc build with debug assertions disabled,
+/// so this has historically led to problems during subtree syncs where those debug assertions
+/// only started triggered there.
+///
+/// This function makes sure we also validate them in debug clippy builds.
+fn validate_diag(diag: &Diag<'_, impl EmissionGuarantee>) {
+    let suggestions = match &diag.suggestions {
+        Suggestions::Enabled(suggs) => &**suggs,
+        Suggestions::Sealed(suggs) => &**suggs,
+        Suggestions::Disabled => return,
+    };
+
+    for substitution in suggestions.iter().flat_map(|s| &s.substitutions) {
+        assert_eq!(
+            substitution
+                .parts
+                .iter()
+                .find(|SubstitutionPart { snippet, span }| snippet.is_empty() && span.is_empty()),
+            None,
+            "span must not be empty and have no suggestion"
+        );
+
+        assert_eq!(
+            substitution
+                .parts
+                .array_windows()
+                .find(|[a, b]| a.span.overlaps(b.span)),
+            None,
+            "suggestion must not have overlapping parts"
+        );
+    }
+}
+
 /// Emit a basic lint message with a `msg` and a `span`.
 ///
 /// This is the most primitive of our lint emission methods and can
@@ -64,6 +102,9 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
     cx.span_lint(lint, sp, |diag| {
         diag.primary_message(msg);
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -118,6 +159,9 @@ pub fn span_lint_and_help<T: LintContext>(
             diag.help(help.into());
         }
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -175,6 +219,9 @@ pub fn span_lint_and_note<T: LintContext>(
             diag.note(note.into());
         }
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -208,6 +255,9 @@ where
         diag.primary_message(msg);
         f(diag);
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -240,6 +290,9 @@ pub fn span_lint_hir(cx: &LateContext<'_>, lint: &'static Lint, hir_id: HirId, s
     cx.tcx.node_span_lint(lint, hir_id, sp, |diag| {
         diag.primary_message(msg);
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -280,6 +333,9 @@ pub fn span_lint_hir_and_then(
         diag.primary_message(msg);
         f(diag);
         docs_link(diag, lint);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }
 
@@ -316,7 +372,7 @@ pub fn span_lint_hir_and_then(
 ///     |
 ///     = note: `-D fold-any` implied by `-D warnings`
 /// ```
-#[expect(clippy::collapsible_span_lint_calls)]
+#[cfg_attr(not(debug_assertions), expect(clippy::collapsible_span_lint_calls))]
 pub fn span_lint_and_sugg<T: LintContext>(
     cx: &T,
     lint: &'static Lint,
@@ -328,5 +384,8 @@ pub fn span_lint_and_sugg<T: LintContext>(
 ) {
     span_lint_and_then(cx, lint, sp, msg.into(), |diag| {
         diag.span_suggestion(sp, help.into(), sugg, applicability);
+
+        #[cfg(debug_assertions)]
+        validate_diag(diag);
     });
 }

clippy_utils/src/lib.rs

@@ -9,6 +9,7 @@
 #![feature(rustc_private)]
 #![feature(assert_matches)]
 #![feature(unwrap_infallible)]
+#![feature(array_windows)]
 #![recursion_limit = "512"]
 #![allow(
     clippy::missing_errors_doc,

tests/ui/semicolon_outside_block.fixed

@@ -80,5 +80,13 @@ fn main() {
 
     { unit_fn_block(); };
 
+    unsafe {
+        std::arch::asm!("")
+    };
+
+    {
+        line!()
+    };
+
     unit_fn_block()
 }

tests/ui/semicolon_outside_block.rs

@@ -80,5 +80,13 @@ fn main() {
 
     { unit_fn_block(); };
 
+    unsafe {
+        std::arch::asm!("");
+    }
+
+    {
+        line!();
+    }
+
     unit_fn_block()
 }

tests/ui/semicolon_outside_block.stderr

@@ -51,5 +51,33 @@ LL -     { m!(()); }
 LL +     { m!(()) };
    |
 
-error: aborting due to 4 previous errors
+error: consider moving the `;` outside the block for consistent formatting
+  --> tests/ui/semicolon_outside_block.rs:83:5
+   |
+LL | /     unsafe {
+LL | |         std::arch::asm!("");
+LL | |     }
+   | |_____^
+   |
+help: put the `;` here
+   |
+LL ~         std::arch::asm!("")
+LL ~     };
+   |
+
+error: consider moving the `;` outside the block for consistent formatting
+  --> tests/ui/semicolon_outside_block.rs:87:5
+   |
+LL | /     {
+LL | |         line!();
+LL | |     }
+   | |_____^
+   |
+help: put the `;` here
+   |
+LL ~         line!()
+LL ~     };
+   |
+
+error: aborting due to 6 previous errors