Auto merge of #106916 - lukas-code:overlapping-substs, r=estebank

bors · bors · commit d8da51366817 · 2023-01-28T10:00:56.000Z
Remove overlapping parts of multipart suggestions This PR adds a debug assertion that the parts of a single substitution cannot overlap, fixes a overlapping substitution from the testsuite, and fixes #106870. Note that a single suggestion can still have multiple overlapping substitutions / possible edits, we just don't suggest overlapping replacements in a single edit anymore. I've also included a fix for an unrelated bug where rustfix for `explicit_outlives_requirements` would produce multiple trailing commas for a where clause.
diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
@@ -629,19 +629,27 @@ impl Diagnostic {
         applicability: Applicability,
         style: SuggestionStyle,
     ) -> &mut Self {
-        assert!(!suggestion.is_empty());
-        debug_assert!(
-            !(suggestion.iter().any(|(sp, text)| sp.is_empty() && text.is_empty())),
-            "Span must not be empty and have no suggestion"
+        let mut parts = suggestion
+            .into_iter()
+            .map(|(span, snippet)| SubstitutionPart { snippet, span })
+            .collect::<Vec<_>>();
+
+        parts.sort_unstable_by_key(|part| part.span);
+
+        assert!(!parts.is_empty());
+        debug_assert_eq!(
+            parts.iter().find(|part| part.span.is_empty() && part.snippet.is_empty()),
+            None,
+            "Span must not be empty and have no suggestion",
+        );
+        debug_assert_eq!(
+            parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
+            None,
+            "suggestion must not have overlapping parts",
         );
 
         self.push_suggestion(CodeSuggestion {
-            substitutions: vec![Substitution {
-                parts: suggestion
-                    .into_iter()
-                    .map(|(span, snippet)| SubstitutionPart { snippet, span })
-                    .collect(),
-            }],
+            substitutions: vec![Substitution { parts }],
             msg: self.subdiagnostic_message_to_diagnostic_message(msg),
             style,
             applicability,
@@ -802,25 +810,34 @@ impl Diagnostic {
         suggestions: impl IntoIterator<Item = Vec<(Span, String)>>,
         applicability: Applicability,
     ) -> &mut Self {
-        let suggestions: Vec<_> = suggestions.into_iter().collect();
-        debug_assert!(
-            !(suggestions
-                .iter()
-                .flatten()
-                .any(|(sp, suggestion)| sp.is_empty() && suggestion.is_empty())),
-            "Span must not be empty and have no suggestion"
-        );
+        let substitutions = suggestions
+            .into_iter()
+            .map(|sugg| {
+                let mut parts = sugg
+                    .into_iter()
+                    .map(|(span, snippet)| SubstitutionPart { snippet, span })
+                    .collect::<Vec<_>>();
+
+                parts.sort_unstable_by_key(|part| part.span);
+
+                assert!(!parts.is_empty());
+                debug_assert_eq!(
+                    parts.iter().find(|part| part.span.is_empty() && part.snippet.is_empty()),
+                    None,
+                    "Span must not be empty and have no suggestion",
+                );
+                debug_assert_eq!(
+                    parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
+                    None,
+                    "suggestion must not have overlapping parts",
+                );
+
+                Substitution { parts }
+            })
+            .collect();
 
         self.push_suggestion(CodeSuggestion {
-            substitutions: suggestions
-                .into_iter()
-                .map(|sugg| Substitution {
-                    parts: sugg
-                        .into_iter()
-                        .map(|(span, snippet)| SubstitutionPart { snippet, span })
-                        .collect(),
-                })
-                .collect(),
+            substitutions,
             msg: self.subdiagnostic_message_to_diagnostic_message(msg),
             style: SuggestionStyle::ShowCode,
             applicability,
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
@@ -3,6 +3,7 @@
 //! This module contains the code for creating and emitting diagnostics.
 
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
+#![feature(array_windows)]
 #![feature(drain_filter)]
 #![feature(if_let_guard)]
 #![feature(is_terminal)]
diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs
@@ -10,7 +10,7 @@ use crate::mbe::transcribe::transcribe;
 
 use rustc_ast as ast;
 use rustc_ast::token::{self, Delimiter, NonterminalKind, Token, TokenKind, TokenKind::*};
-use rustc_ast::tokenstream::{DelimSpan, TokenStream};
+use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree};
 use rustc_ast::{NodeId, DUMMY_NODE_ID};
 use rustc_ast_pretty::pprust;
 use rustc_attr::{self as attr, TransparencyError};
@@ -212,7 +212,6 @@ fn expand_macro<'cx>(
             };
             let arm_span = rhses[i].span();
 
-            let rhs_spans = rhs.tts.iter().map(|t| t.span()).collect::<Vec<_>>();
             // rhs has holes ( `$id` and `$(...)` that need filled)
             let mut tts = match transcribe(cx, &named_matches, &rhs, rhs_span, transparency) {
                 Ok(tts) => tts,
@@ -224,12 +223,25 @@ fn expand_macro<'cx>(
 
             // Replace all the tokens for the corresponding positions in the macro, to maintain
             // proper positions in error reporting, while maintaining the macro_backtrace.
-            if rhs_spans.len() == tts.len() {
+            if tts.len() == rhs.tts.len() {
                 tts = tts.map_enumerated(|i, tt| {
                     let mut tt = tt.clone();
-                    let mut sp = rhs_spans[i];
-                    sp = sp.with_ctxt(tt.span().ctxt());
-                    tt.set_span(sp);
+                    let rhs_tt = &rhs.tts[i];
+                    let ctxt = tt.span().ctxt();
+                    match (&mut tt, rhs_tt) {
+                        // preserve the delim spans if able
+                        (
+                            TokenTree::Delimited(target_sp, ..),
+                            mbe::TokenTree::Delimited(source_sp, ..),
+                        ) => {
+                            target_sp.open = source_sp.open.with_ctxt(ctxt);
+                            target_sp.close = source_sp.close.with_ctxt(ctxt);
+                        }
+                        _ => {
+                            let sp = rhs_tt.span().with_ctxt(ctxt);
+                            tt.set_span(sp);
+                        }
+                    }
                     tt
                 });
             }
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -2175,13 +2175,31 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
                     dropped_predicate_count += 1;
                 }
 
-                if drop_predicate && !in_where_clause {
-                    lint_spans.push(predicate_span);
-                } else if drop_predicate && i + 1 < num_predicates {
-                    // If all the bounds on a predicate were inferable and there are
-                    // further predicates, we want to eat the trailing comma.
-                    let next_predicate_span = hir_generics.predicates[i + 1].span();
-                    where_lint_spans.push(predicate_span.to(next_predicate_span.shrink_to_lo()));
+                if drop_predicate {
+                    if !in_where_clause {
+                        lint_spans.push(predicate_span);
+                    } else if predicate_span.from_expansion() {
+                        // Don't try to extend the span if it comes from a macro expansion.
+                        where_lint_spans.push(predicate_span);
+                    } else if i + 1 < num_predicates {
+                        // If all the bounds on a predicate were inferable and there are
+                        // further predicates, we want to eat the trailing comma.
+                        let next_predicate_span = hir_generics.predicates[i + 1].span();
+                        if next_predicate_span.from_expansion() {
+                            where_lint_spans.push(predicate_span);
+                        } else {
+                            where_lint_spans
+                                .push(predicate_span.to(next_predicate_span.shrink_to_lo()));
+                        }
+                    } else {
+                        // Eat the optional trailing comma after the last predicate.
+                        let where_span = hir_generics.where_clause_span;
+                        if where_span.from_expansion() {
+                            where_lint_spans.push(predicate_span);
+                        } else {
+                            where_lint_spans.push(predicate_span.to(where_span.shrink_to_hi()));
+                        }
+                    }
                 } else {
                     where_lint_spans.extend(self.consolidate_outlives_bound_spans(
                         predicate_span.shrink_to_lo(),
@@ -2225,6 +2243,11 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
                     Applicability::MaybeIncorrect
                 };
 
+                // Due to macros, there might be several predicates with the same span
+                // and we only want to suggest removing them once.
+                lint_spans.sort_unstable();
+                lint_spans.dedup();
+
                 cx.emit_spanned_lint(
                     EXPLICIT_OUTLIVES_REQUIREMENTS,
                     lint_spans.clone(),
diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs
@@ -311,6 +311,10 @@ fn check_uninlined_args(
     // in those cases, make the code suggestion hidden
     let multiline_fix = fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span));
 
+    // Suggest removing each argument only once, for example in `format!("{0} {0}", arg)`.
+    fixes.sort_unstable_by_key(|(span, _)| *span);
+    fixes.dedup_by_key(|(span, _)| *span);
+
     span_lint_and_then(
         cx,
         UNINLINED_FORMAT_ARGS,
diff --git a/tests/ui/const-generics/min_const_generics/macro-fail.stderr b/tests/ui/const-generics/min_const_generics/macro-fail.stderr
@@ -8,7 +8,7 @@ LL | fn make_marker() -> impl Marker<gimme_a_const!(marker)> {
    |                                 in this macro invocation
 ...
 LL |       ($rusty: ident) => {{ let $rusty = 3; *&$rusty }}
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type
+   |                           ^ expected type
    |
    = note: this error originates in the macro `gimme_a_const` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -22,26 +22,21 @@ LL |   Example::<gimme_a_const!(marker)>
    |             in this macro invocation
 ...
 LL |       ($rusty: ident) => {{ let $rusty = 3; *&$rusty }}
-   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type
+   |                           ^ expected type
    |
    = note: this error originates in the macro `gimme_a_const` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: expected type, found `{`
   --> $DIR/macro-fail.rs:4:10
    |
-LL |     () => {{
-   |  __________^
-LL | |
-LL | |     const X: usize = 1337;
-LL | |     X
-LL | |   }}
-   | |___^ expected type
+LL |   () => {{
+   |          ^ expected type
 ...
-LL |     let _fail = Example::<external_macro!()>;
-   |                           -----------------
-   |                           |
-   |                           this macro call doesn't expand to a type
-   |                           in this macro invocation
+LL |   let _fail = Example::<external_macro!()>;
+   |                         -----------------
+   |                         |
+   |                         this macro call doesn't expand to a type
+   |                         in this macro invocation
    |
    = note: this error originates in the macro `external_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
 
diff --git a/tests/ui/imports/import-prefix-macro-1.stderr b/tests/ui/imports/import-prefix-macro-1.stderr
@@ -2,7 +2,7 @@ error: expected one of `::`, `;`, or `as`, found `{`
   --> $DIR/import-prefix-macro-1.rs:11:27
    |
 LL |     ($p: path) => (use $p {S, Z});
-   |                           ^^^^^^ expected one of `::`, `;`, or `as`
+   |                           ^ expected one of `::`, `;`, or `as`
 ...
 LL | import! { a::b::c }
    | ------------------- in this macro invocation
diff --git a/tests/ui/parser/issues/issue-44406.stderr b/tests/ui/parser/issues/issue-44406.stderr
@@ -21,8 +21,8 @@ LL |     foo!(true);
    = note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
 help: if `bar` is a struct, use braces as delimiters
    |
-LL |         bar {  }
-   |             ~
+LL |         bar { baz: $rest }
+   |             ~            ~
 help: if `bar` is a function, use the arguments directly
    |
 LL -         bar(baz: $rest)
diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.rs b/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.rs
@@ -365,4 +365,24 @@ mod unions {
     }
 }
 
+// https://github.com/rust-lang/rust/issues/106870
+mod multiple_predicates_with_same_span {
+    macro_rules! m {
+        ($($name:ident)+) => {
+            struct Inline<'a, $($name: 'a,)+>(&'a ($($name,)+));
+            //~^ ERROR: outlives requirements can be inferred
+            struct FullWhere<'a, $($name,)+>(&'a ($($name,)+)) where $($name: 'a,)+;
+            //~^ ERROR: outlives requirements can be inferred
+            struct PartialWhere<'a, $($name,)+>(&'a ($($name,)+)) where (): Sized, $($name: 'a,)+;
+            //~^ ERROR: outlives requirements can be inferred
+            struct Interleaved<'a, $($name,)+>(&'a ($($name,)+))
+            where
+                (): Sized,
+                $($name: 'a, $name: 'a, )+ //~ ERROR: outlives requirements can be inferred
+                $($name: 'a, $name: 'a, )+;
+        }
+    }
+    m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
+}
+
 fn main() {}
diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.stderr b/tests/ui/rust-2018/edition-lint-infer-outlives-multispan.stderr
@@ -819,5 +819,61 @@ LL -     union BeeWhereAyTeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U:
 LL +     union BeeWhereAyTeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U: Debug,  {
    |
 
-error: aborting due to 68 previous errors
+error: outlives requirements can be inferred
+  --> $DIR/edition-lint-infer-outlives-multispan.rs:372:38
+   |
+LL |             struct Inline<'a, $($name: 'a,)+>(&'a ($($name,)+));
+   |                                      ^^^^ help: remove these bounds
+...
+LL |     m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
+   |     --------------------------------------------------------- in this macro invocation
+   |
+   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: outlives requirements can be inferred
+  --> $DIR/edition-lint-infer-outlives-multispan.rs:374:64
+   |
+LL |             struct FullWhere<'a, $($name,)+>(&'a ($($name,)+)) where $($name: 'a,)+;
+   |                                                                ^^^^^^^^^^^^^^^^^^ help: remove these bounds
+...
+LL |     m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
+   |     --------------------------------------------------------- in this macro invocation
+   |
+   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: outlives requirements can be inferred
+  --> $DIR/edition-lint-infer-outlives-multispan.rs:376:86
+   |
+LL |             struct PartialWhere<'a, $($name,)+>(&'a ($($name,)+)) where (): Sized, $($name: 'a,)+;
+   |                                                                                      ^^^^^^^^^ help: remove these bounds
+...
+LL |     m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
+   |     --------------------------------------------------------- in this macro invocation
+   |
+   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: outlives requirements can be inferred
+  --> $DIR/edition-lint-infer-outlives-multispan.rs:381:19
+   |
+LL |                 $($name: 'a, $name: 'a, )+
+   |                   ^^^^^^^^^  ^^^^^^^^^
+LL |                 $($name: 'a, $name: 'a, )+;
+   |                   ^^^^^^^^^  ^^^^^^^^^
+...
+LL |     m!(T0 T1 T2 T3 T4 T5 T6 T7 T8 T9 T10 T11 T12 T13 T14 T15);
+   |     ---------------------------------------------------------
+   |     |
+   |     in this macro invocation
+   |     in this macro invocation
+   |     in this macro invocation
+   |     in this macro invocation
+   |
+   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: remove these bounds
+   |
+LL ~                 $(, , )+
+LL ~                 $(, , )+;
+   |
+
+error: aborting due to 72 previous errors
 
diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.fixed b/tests/ui/rust-2018/edition-lint-infer-outlives.fixed
@@ -791,5 +791,14 @@ struct StaticRef<T: 'static> {
     field: &'static T
 }
 
+struct TrailingCommaInWhereClause<'a, T, U>
+where
+    T: 'a,
+    
+    //~^ ERROR outlives requirements can be inferred
+{
+    tee: T,
+    yoo: &'a U
+}
 
 fn main() {}
diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.rs b/tests/ui/rust-2018/edition-lint-infer-outlives.rs
@@ -791,5 +791,14 @@ struct StaticRef<T: 'static> {
     field: &'static T
 }
 
+struct TrailingCommaInWhereClause<'a, T, U>
+where
+    T: 'a,
+    U: 'a,
+    //~^ ERROR outlives requirements can be inferred
+{
+    tee: T,
+    yoo: &'a U
+}
 
 fn main() {}
diff --git a/tests/ui/rust-2018/edition-lint-infer-outlives.stderr b/tests/ui/rust-2018/edition-lint-infer-outlives.stderr

Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: expected one of `::`, `;`, or `as`, found `{`
`2`	`2`	`--> $DIR/import-prefix-macro-1.rs:11:27`
`3`	`3`	`\|`
`4`	`4`	`LL \| ($p: path) => (use $p {S, Z});`
`5`		- \| ^^^^^^ expected one of `::`, `;`, or `as`
	`5`	+ \| ^ expected one of `::`, `;`, or `as`
`6`	`6`	`...`
`7`	`7`	`LL \| import! { a::b::c }`
`8`	`8`	`\| ------------------- in this macro invocation`
Original file line number	Diff line number	Diff line change
`@@ -21,8 +21,8 @@ LL \| foo!(true);`
`21`	`21`	= note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
`22`	`22`	help: if `bar` is a struct, use braces as delimiters
`23`	`23`	`\|`
`24`		`-LL \| bar { }`
`25`		`- \| ~`
	`24`	`+LL \| bar { baz: $rest }`
	`25`	`+ \| ~ ~`
`26`	`26`	help: if `bar` is a function, use the arguments directly
`27`	`27`	`\|`
`28`	`28`	`LL - bar(baz: $rest)`