Rollup merge of #98506 - compiler-errors:object-safety-suggestions, r=oli-obk

Fix span issues in object safety suggestions

Fixes #98500

2 files changed, 74 insertions, 71 deletions

diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs
index 94ec89776a9..ed8de24a65e 100644
--- a/compiler/rustc_middle/src/traits/mod.rs
+++ b/compiler/rustc_middle/src/traits/mod.rs
@@ -863,7 +863,7 @@ pub enum ObjectSafetyViolation {
 
 impl ObjectSafetyViolation {
     pub fn error_msg(&self) -> Cow<'static, str> {
-        match *self {
+        match self {
             ObjectSafetyViolation::SizedSelf(_) => "it requires `Self: Sized`".into(),
             ObjectSafetyViolation::SupertraitSelf(ref spans) => {
                 if spans.iter().any(|sp| *sp != DUMMY_SP) {
@@ -873,7 +873,7 @@ impl ObjectSafetyViolation {
                         .into()
                 }
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_, _, _), _) => {
+            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_), _) => {
                 format!("associated function `{}` has no `self` parameter", name).into()
             }
             ObjectSafetyViolation::Method(
@@ -897,9 +897,11 @@ impl ObjectSafetyViolation {
             ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, _) => {
                 format!("method `{}` has generic type parameters", name).into()
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) => {
-                format!("method `{}`'s `self` parameter cannot be dispatched on", name).into()
-            }
+            ObjectSafetyViolation::Method(
+                name,
+                MethodViolationCode::UndispatchableReceiver(_),
+                _,
+            ) => format!("method `{}`'s `self` parameter cannot be dispatched on", name).into(),
             ObjectSafetyViolation::AssocConst(name, DUMMY_SP) => {
                 format!("it contains associated `const` `{}`", name).into()
             }
@@ -911,51 +913,40 @@ impl ObjectSafetyViolation {
     }
 
     pub fn solution(&self, err: &mut Diagnostic) {
-        match *self {
+        match self {
             ObjectSafetyViolation::SizedSelf(_) | ObjectSafetyViolation::SupertraitSelf(_) => {}
             ObjectSafetyViolation::Method(
                 name,
-                MethodViolationCode::StaticMethod(sugg, self_span, has_args),
+                MethodViolationCode::StaticMethod(Some((add_self_sugg, make_sized_sugg))),
                 _,
             ) => {
                 err.span_suggestion(
-                    self_span,
-                    &format!(
+                    add_self_sugg.1,
+                    format!(
                         "consider turning `{}` into a method by giving it a `&self` argument",
                         name
                     ),
-                    format!("&self{}", if has_args { ", " } else { "" }),
+                    add_self_sugg.0.to_string(),
+                    Applicability::MaybeIncorrect,
+                );
+                err.span_suggestion(
+                    make_sized_sugg.1,
+                    format!(
+                        "alternatively, consider constraining `{}` so it does not apply to \
+                             trait objects",
+                        name
+                    ),
+                    make_sized_sugg.0.to_string(),
                     Applicability::MaybeIncorrect,
                 );
-                match sugg {
-                    Some((sugg, span)) => {
-                        err.span_suggestion(
-                            span,
-                            &format!(
-                                "alternatively, consider constraining `{}` so it does not apply to \
-                                 trait objects",
-                                name
-                            ),
-                            sugg,
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
-                    None => {
-                        err.help(&format!(
-                            "consider turning `{}` into a method by giving it a `&self` \
-                             argument or constraining it so it does not apply to trait objects",
-                            name
-                        ));
-                    }
-                }
             }
             ObjectSafetyViolation::Method(
                 name,
-                MethodViolationCode::UndispatchableReceiver,
-                span,
+                MethodViolationCode::UndispatchableReceiver(Some(span)),
+                _,
             ) => {
                 err.span_suggestion(
-                    span,
+                    *span,
                     &format!(
                         "consider changing method `{}`'s `self` parameter to be `&self`",
                         name
@@ -991,13 +982,13 @@ impl ObjectSafetyViolation {
 }
 
 /// Reasons a method might not be object-safe.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
 pub enum MethodViolationCode {
     /// e.g., `fn foo()`
-    StaticMethod(Option<(&'static str, Span)>, Span, bool /* has args */),
+    StaticMethod(Option<(/* add &self */ (String, Span), /* add Self: Sized */ (String, Span))>),
 
     /// e.g., `fn foo(&self, x: Self)`
-    ReferencesSelfInput(usize),
+    ReferencesSelfInput(Option<Span>),
 
     /// e.g., `fn foo(&self) -> Self`
     ReferencesSelfOutput,
@@ -1009,7 +1000,7 @@ pub enum MethodViolationCode {
     Generic,
 
     /// the method's receiver (`self` argument) can't be dispatched on
-    UndispatchableReceiver,
+    UndispatchableReceiver(Option<Span>),
 }
 
 /// These are the error cases for `codegen_fulfill_obligation`.
diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs
index 132c335a7e6..8d344591915 100644
--- a/compiler/rustc_trait_selection/src/traits/object_safety.rs
+++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs
@@ -366,15 +366,9 @@ fn object_safety_violation_for_method(
     // Get an accurate span depending on the violation.
     violation.map(|v| {
         let node = tcx.hir().get_if_local(method.def_id);
-        let span = match (v, node) {
-            (MethodViolationCode::ReferencesSelfInput(arg), Some(node)) => node
-                .fn_decl()
-                .and_then(|decl| decl.inputs.get(arg + 1))
-                .map_or(method.ident(tcx).span, |arg| arg.span),
-            (MethodViolationCode::UndispatchableReceiver, Some(node)) => node
-                .fn_decl()
-                .and_then(|decl| decl.inputs.get(0))
-                .map_or(method.ident(tcx).span, |arg| arg.span),
+        let span = match (&v, node) {
+            (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
+            (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
             (MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
                 node.fn_decl().map_or(method.ident(tcx).span, |decl| decl.output.span())
             }
@@ -397,32 +391,41 @@ fn virtual_call_violation_for_method<'tcx>(
 
     // The method's first parameter must be named `self`
     if !method.fn_has_self_parameter {
-        // We'll attempt to provide a structured suggestion for `Self: Sized`.
-        let sugg =
-            tcx.hir().get_if_local(method.def_id).as_ref().and_then(|node| node.generics()).map(
-                |generics| match generics.predicates {
-                    [] => (" where Self: Sized", generics.where_clause_span),
-                    [.., pred] => (", Self: Sized", pred.span().shrink_to_hi()),
-                },
-            );
-        // Get the span pointing at where the `self` receiver should be.
-        let sm = tcx.sess.source_map();
-        let self_span = method.ident(tcx).span.to(tcx
-            .hir()
-            .span_if_local(method.def_id)
-            .unwrap_or_else(|| sm.next_point(method.ident(tcx).span))
-            .shrink_to_hi());
-        let self_span = sm.span_through_char(self_span, '(').shrink_to_hi();
-        return Some(MethodViolationCode::StaticMethod(
-            sugg,
-            self_span,
-            !sig.inputs().skip_binder().is_empty(),
-        ));
+        let sugg = if let Some(hir::Node::TraitItem(hir::TraitItem {
+            generics,
+            kind: hir::TraitItemKind::Fn(sig, _),
+            ..
+        })) = tcx.hir().get_if_local(method.def_id).as_ref()
+        {
+            let sm = tcx.sess.source_map();
+            Some((
+                (
+                    format!("&self{}", if sig.decl.inputs.is_empty() { "" } else { ", " }),
+                    sm.span_through_char(sig.span, '(').shrink_to_hi(),
+                ),
+                (
+                    format!("{} Self: Sized", generics.add_where_or_trailing_comma()),
+                    generics.tail_span_for_predicate_suggestion(),
+                ),
+            ))
+        } else {
+            None
+        };
+        return Some(MethodViolationCode::StaticMethod(sugg));
     }
 
-    for (i, &input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() {
+    for (i, &input_ty) in sig.skip_binder().inputs().iter().enumerate().skip(1) {
         if contains_illegal_self_type_reference(tcx, trait_def_id, sig.rebind(input_ty)) {
-            return Some(MethodViolationCode::ReferencesSelfInput(i));
+            let span = if let Some(hir::Node::TraitItem(hir::TraitItem {
+                kind: hir::TraitItemKind::Fn(sig, _),
+                ..
+            })) = tcx.hir().get_if_local(method.def_id).as_ref()
+            {
+                Some(sig.decl.inputs[i].span)
+            } else {
+                None
+            };
+            return Some(MethodViolationCode::ReferencesSelfInput(span));
         }
     }
     if contains_illegal_self_type_reference(tcx, trait_def_id, sig.output()) {
@@ -456,7 +459,16 @@ fn virtual_call_violation_for_method<'tcx>(
     // `Receiver: Unsize<Receiver[Self => dyn Trait]>`.
     if receiver_ty != tcx.types.self_param {
         if !receiver_is_dispatchable(tcx, method, receiver_ty) {
-            return Some(MethodViolationCode::UndispatchableReceiver);
+            let span = if let Some(hir::Node::TraitItem(hir::TraitItem {
+                kind: hir::TraitItemKind::Fn(sig, _),
+                ..
+            })) = tcx.hir().get_if_local(method.def_id).as_ref()
+            {
+                Some(sig.decl.inputs[0].span)
+            } else {
+                None
+            };
+            return Some(MethodViolationCode::UndispatchableReceiver(span));
         } else {
             // Do sanity check to make sure the receiver actually has the layout of a pointer.