Auto merge of #117200 - rmehri01:repeated_help, r=WaffleLapkin

Don't add redundant help for object safety violations

Fixes #117186

r? WaffleLapkin

compiler/rustc_infer/src/traits/error_reporting/mod.rs

@@ -101,12 +101,19 @@ pub fn report_object_safety_error<'tcx>(
          to be resolvable dynamically; for more information visit \
          <https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
     );
+
+    // Only provide the help if its a local trait, otherwise it's not actionable.
     if trait_span.is_some() {
         let mut reported_violations: Vec<_> = reported_violations.into_iter().collect();
         reported_violations.sort();
-        for violation in reported_violations {
-            // Only provide the help if its a local trait, otherwise it's not actionable.
-            violation.solution(&mut err);
+
+        let mut potential_solutions: Vec<_> =
+            reported_violations.into_iter().map(|violation| violation.solution()).collect();
+        potential_solutions.sort();
+        // Allows us to skip suggesting that the same item should be moved to another trait multiple times.
+        potential_solutions.dedup();
+        for solution in potential_solutions {
+            solution.add_to(&mut err);
         }
     }
 

compiler/rustc_middle/src/traits/mod.rs

@@ -832,50 +832,31 @@ impl ObjectSafetyViolation {
         }
     }
 
-    pub fn solution(&self, err: &mut Diagnostic) {
+    pub fn solution(&self) -> ObjectSafetyViolationSolution {
         match self {
             ObjectSafetyViolation::SizedSelf(_)
             | ObjectSafetyViolation::SupertraitSelf(_)
-            | ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {}
+            | ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {
+                ObjectSafetyViolationSolution::None
+            }
             ObjectSafetyViolation::Method(
                 name,
                 MethodViolationCode::StaticMethod(Some((add_self_sugg, make_sized_sugg))),
                 _,
-            ) => {
-                err.span_suggestion(
-                    add_self_sugg.1,
-                    format!(
-                        "consider turning `{name}` into a method by giving it a `&self` argument"
-                    ),
-                    add_self_sugg.0.to_string(),
-                    Applicability::MaybeIncorrect,
-                );
-                err.span_suggestion(
-                    make_sized_sugg.1,
-                    format!(
-                        "alternatively, consider constraining `{name}` so it does not apply to \
-                             trait objects"
-                    ),
-                    make_sized_sugg.0.to_string(),
-                    Applicability::MaybeIncorrect,
-                );
-            }
+            ) => ObjectSafetyViolationSolution::AddSelfOrMakeSized {
+                name: *name,
+                add_self_sugg: add_self_sugg.clone(),
+                make_sized_sugg: make_sized_sugg.clone(),
+            },
             ObjectSafetyViolation::Method(
                 name,
                 MethodViolationCode::UndispatchableReceiver(Some(span)),
                 _,
-            ) => {
-                err.span_suggestion(
-                    *span,
-                    format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
-                    "&Self",
-                    Applicability::MachineApplicable,
-                );
-            }
+            ) => ObjectSafetyViolationSolution::ChangeToRefSelf(*name, *span),
             ObjectSafetyViolation::AssocConst(name, _)
             | ObjectSafetyViolation::GAT(name, _)
             | ObjectSafetyViolation::Method(name, ..) => {
-                err.help(format!("consider moving `{name}` to another trait"));
+                ObjectSafetyViolationSolution::MoveToAnotherTrait(*name)
             }
         }
     }
@@ -899,6 +880,60 @@ impl ObjectSafetyViolation {
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
+pub enum ObjectSafetyViolationSolution {
+    None,
+    AddSelfOrMakeSized {
+        name: Symbol,
+        add_self_sugg: (String, Span),
+        make_sized_sugg: (String, Span),
+    },
+    ChangeToRefSelf(Symbol, Span),
+    MoveToAnotherTrait(Symbol),
+}
+
+impl ObjectSafetyViolationSolution {
+    pub fn add_to(self, err: &mut Diagnostic) {
+        match self {
+            ObjectSafetyViolationSolution::None => {}
+            ObjectSafetyViolationSolution::AddSelfOrMakeSized {
+                name,
+                add_self_sugg,
+                make_sized_sugg,
+            } => {
+                err.span_suggestion(
+                    add_self_sugg.1,
+                    format!(
+                        "consider turning `{name}` into a method by giving it a `&self` argument"
+                    ),
+                    add_self_sugg.0,
+                    Applicability::MaybeIncorrect,
+                );
+                err.span_suggestion(
+                    make_sized_sugg.1,
+                    format!(
+                        "alternatively, consider constraining `{name}` so it does not apply to \
+                             trait objects"
+                    ),
+                    make_sized_sugg.0,
+                    Applicability::MaybeIncorrect,
+                );
+            }
+            ObjectSafetyViolationSolution::ChangeToRefSelf(name, span) => {
+                err.span_suggestion(
+                    span,
+                    format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
+                    "&Self",
+                    Applicability::MachineApplicable,
+                );
+            }
+            ObjectSafetyViolationSolution::MoveToAnotherTrait(name) => {
+                err.help(format!("consider moving `{name}` to another trait"));
+            }
+        }
+    }
+}
+
 /// Reasons a method might not be object-safe.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
 pub enum MethodViolationCode {

compiler/rustc_trait_selection/src/traits/object_safety.rs

@@ -192,7 +192,7 @@ fn lint_object_unsafe_trait(
             );
             if node.is_some() {
                 // Only provide the help if its a local trait, otherwise it's not
-                violation.solution(err);
+                violation.solution().add_to(err);
             }
             err
         },

tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr

@@ -14,7 +14,6 @@ LL |     fn test(&self) -> [u8; bar::<Self>()];
    |        |
    |        ...because method `test` references the `Self` type in its `where` clause
    = help: consider moving `test` to another trait
-   = help: consider moving `test` to another trait
    = help: only type `()` implements the trait, consider using it directly instead
 
 error: aborting due to 1 previous error