add debug assertions for overlapping spans and empty replacements

Author: y21

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