Rollup merge of rust-lang#65752 - estebank:sugg, r=Centril

Use structured suggestions for missing associated items

When encountering an `impl` that is missing associated items required by its `trait`, use structured suggestions at an appropriate place in the `impl`.

src/librustc/hir/mod.rs

@@ -1075,6 +1075,13 @@ impl Mutability {
             MutImmutable => MutMutable,
         }
     }
+
+    pub fn prefix_str(&self) -> &'static str {
+        match self {
+            MutMutable => "mut ",
+            MutImmutable => "",
+        }
+    }
 }
 
 #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable)]
@@ -2184,6 +2191,15 @@ pub enum Unsafety {
     Normal,
 }
 
+impl Unsafety {
+    pub fn prefix_str(&self) -> &'static str {
+        match self {
+            Unsafety::Unsafe => "unsafe ",
+            Unsafety::Normal => "",
+        }
+    }
+}
+
 #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub enum Constness {
     Const,

src/librustc/hir/print.rs

@@ -1734,9 +1734,7 @@ impl<'a> State<'a> {
                     _ => false,
                 };
                 self.s.word("&");
-                if mutbl == hir::MutMutable {
-                    self.s.word("mut ");
-                }
+                self.s.word(mutbl.prefix_str());
                 if is_range_inner {
                     self.popen();
                 }

src/librustc/infer/error_reporting/mod.rs

@@ -897,11 +897,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             } else {
                 r.push(' ');
             }
-            s.push_highlighted(format!(
-                "&{}{}",
-                r,
-                if mutbl == hir::MutMutable { "mut " } else { "" }
-            ));
+            s.push_highlighted(format!("&{}{}", r, mutbl.prefix_str()));
             s.push_normal(ty.to_string());
         }
 

src/librustc/ty/print/obsolete.rs

@@ -80,9 +80,7 @@ impl DefPathBasedNames<'tcx> {
             }
             ty::Ref(_, inner_type, mutbl) => {
                 output.push('&');
-                if mutbl == hir::MutMutable {
-                    output.push_str("mut ");
-                }
+                output.push_str(mutbl.prefix_str());
 
                 self.push_type_name(inner_type, output, debug);
             }
@@ -114,9 +112,7 @@ impl DefPathBasedNames<'tcx> {
             ty::Foreign(did) => self.push_def_path(did, output),
             ty::FnDef(..) | ty::FnPtr(_) => {
                 let sig = t.fn_sig(self.tcx);
-                if sig.unsafety() == hir::Unsafety::Unsafe {
-                    output.push_str("unsafe ");
-                }
+                output.push_str(sig.unsafety().prefix_str());
 
                 let abi = sig.abi();
                 if abi != ::rustc_target::spec::abi::Abi::Rust {

src/librustc/ty/print/pretty.rs

@@ -1666,8 +1666,7 @@ define_print_and_forward_display! {
     }
 
     ty::TypeAndMut<'tcx> {
-        p!(write("{}", if self.mutbl == hir::MutMutable { "mut " } else { "" }),
-            print(self.ty))
+        p!(write("{}", self.mutbl.prefix_str()), print(self.ty))
     }
 
     ty::ExistentialTraitRef<'tcx> {
@@ -1693,9 +1692,7 @@ define_print_and_forward_display! {
     }
 
     ty::FnSig<'tcx> {
-        if self.unsafety == hir::Unsafety::Unsafe {
-            p!(write("unsafe "));
-        }
+        p!(write("{}", self.unsafety.prefix_str()));
 
         if self.abi != Abi::Rust {
             p!(write("extern {} ", self.abi));

src/librustc_codegen_ssa/debuginfo/type_names.rs

@@ -76,9 +76,7 @@ pub fn push_debuginfo_type_name<'tcx>(
             if !cpp_like_names {
                 output.push('&');
             }
-            if mutbl == hir::MutMutable {
-                output.push_str("mut ");
-            }
+            output.push_str(mutbl.prefix_str());
 
             push_debuginfo_type_name(tcx, inner_type, true, output, visited);
 
@@ -140,9 +138,7 @@ pub fn push_debuginfo_type_name<'tcx>(
 
 
             let sig = t.fn_sig(tcx);
-            if sig.unsafety() == hir::Unsafety::Unsafe {
-                output.push_str("unsafe ");
-            }
+            output.push_str(sig.unsafety().prefix_str());
 
             let abi = sig.abi();
             if abi != rustc_target::spec::abi::Abi::Rust {

src/librustc_mir/hair/pattern/mod.rs

@@ -293,10 +293,7 @@ impl<'tcx> fmt::Display for Pat<'tcx> {
                 match self.ty.kind {
                     ty::Adt(def, _) if def.is_box() => write!(f, "box ")?,
                     ty::Ref(_, _, mutbl) => {
-                        write!(f, "&")?;
-                        if mutbl == hir::MutMutable {
-                            write!(f, "mut ")?;
-                        }
+                        write!(f, "&{}", mutbl.prefix_str())?;
                     }
                     _ => bug!("{} is a bad Deref pattern type", self.ty)
                 }

src/librustc_typeck/check/cast.rs

@@ -341,10 +341,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
                                          tstr);
         match self.expr_ty.kind {
             ty::Ref(_, _, mt) => {
-                let mtstr = match mt {
-                    hir::MutMutable => "mut ",
-                    hir::MutImmutable => "",
-                };
+                let mtstr = mt.prefix_str();
                 if self.cast_ty.is_trait() {
                     match fcx.tcx.sess.source_map().span_to_snippet(self.cast_span) {
                         Ok(s) => {

src/librustc_typeck/check/expr.rs

@@ -592,20 +592,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             cause.span,
                             target_id,
                         );
-                        let val = match ty.kind {
-                            ty::Bool => "true",
-                            ty::Char => "'a'",
-                            ty::Int(_) | ty::Uint(_) => "42",
-                            ty::Float(_) => "3.14159",
-                            ty::Error | ty::Never => return,
-                            _ => "value",
-                        };
-                        let msg = "give it a value of the expected type";
-                        let label = destination.label
-                            .map(|l| format!(" {}", l.ident))
-                            .unwrap_or_else(String::new);
-                        let sugg = format!("break{} {}", label, val);
-                        err.span_suggestion(expr.span, msg, sugg, Applicability::HasPlaceholders);
+                        if let Some(val) = ty_kind_suggestion(ty) {
+                            let label = destination.label
+                                .map(|l| format!(" {}", l.ident))
+                                .unwrap_or_else(String::new);
+                            err.span_suggestion(
+                                expr.span,
+                                "give it a value of the expected type",
+                                format!("break{} {}", label, val),
+                                Applicability::HasPlaceholders,
+                            );
+                        }
                     }, false);
                 }
             } else {
@@ -1725,3 +1722,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.tcx.mk_unit()
     }
 }
+
+pub(super) fn ty_kind_suggestion(ty: Ty<'_>) -> Option<&'static str> {
+    Some(match ty.kind {
+        ty::Bool => "true",
+        ty::Char => "'a'",
+        ty::Int(_) | ty::Uint(_) => "42",
+        ty::Float(_) => "3.14159",
+        ty::Error | ty::Never => return None,
+        _ => "value",
+    })
+}

src/librustc_typeck/check/mod.rs

@@ -127,7 +127,7 @@ use syntax::ast;
 use syntax::attr;
 use syntax::feature_gate::{GateIssue, emit_feature_err};
 use syntax::source_map::{DUMMY_SP, original_sp};
-use syntax::symbol::{kw, sym};
+use syntax::symbol::{kw, sym, Ident};
 use syntax::util::parser::ExprPrecedence;
 
 use std::cell::{Cell, RefCell, Ref, RefMut};
@@ -1800,12 +1800,12 @@ fn check_specialization_validity<'tcx>(
 
 fn check_impl_items_against_trait<'tcx>(
     tcx: TyCtxt<'tcx>,
-    impl_span: Span,
+    full_impl_span: Span,
     impl_id: DefId,
     impl_trait_ref: ty::TraitRef<'tcx>,
     impl_item_refs: &[hir::ImplItemRef],
 ) {
-    let impl_span = tcx.sess.source_map().def_span(impl_span);
+    let impl_span = tcx.sess.source_map().def_span(full_impl_span);
 
     // If the trait reference itself is erroneous (so the compilation is going
     // to fail), skip checking the items here -- the `impl_item` table in `tcx`
@@ -1925,35 +1925,132 @@ fn check_impl_items_against_trait<'tcx>(
     }
 
     if !missing_items.is_empty() {
-        let mut err = struct_span_err!(tcx.sess, impl_span, E0046,
-            "not all trait items implemented, missing: `{}`",
-            missing_items.iter()
-                .map(|trait_item| trait_item.ident.to_string())
-                .collect::<Vec<_>>().join("`, `"));
-        err.span_label(impl_span, format!("missing `{}` in implementation",
-                missing_items.iter()
-                    .map(|trait_item| trait_item.ident.to_string())
-                    .collect::<Vec<_>>().join("`, `")));
-        for trait_item in missing_items {
-            if let Some(span) = tcx.hir().span_if_local(trait_item.def_id) {
-                err.span_label(span, format!("`{}` from trait", trait_item.ident));
-            } else {
-                err.note_trait_signature(trait_item.ident.to_string(),
-                                         trait_item.signature(tcx));
-            }
-        }
-        err.emit();
+        missing_items_err(tcx, impl_span, &missing_items, full_impl_span);
     }
 
     if !invalidated_items.is_empty() {
         let invalidator = overridden_associated_type.unwrap();
-        span_err!(tcx.sess, invalidator.span, E0399,
-                  "the following trait items need to be reimplemented \
-                   as `{}` was overridden: `{}`",
-                  invalidator.ident,
-                  invalidated_items.iter()
-                                   .map(|name| name.to_string())
-                                   .collect::<Vec<_>>().join("`, `"))
+        span_err!(
+            tcx.sess,
+            invalidator.span,
+            E0399,
+            "the following trait items need to be reimplemented as `{}` was overridden: `{}`",
+            invalidator.ident,
+            invalidated_items.iter()
+                .map(|name| name.to_string())
+                .collect::<Vec<_>>().join("`, `")
+        )
+    }
+}
+
+fn missing_items_err(
+    tcx: TyCtxt<'_>,
+    impl_span: Span,
+    missing_items: &[ty::AssocItem],
+    full_impl_span: Span,
+) {
+    let missing_items_msg = missing_items.iter()
+        .map(|trait_item| trait_item.ident.to_string())
+        .collect::<Vec<_>>().join("`, `");
+
+    let mut err = struct_span_err!(
+        tcx.sess,
+        impl_span,
+        E0046,
+        "not all trait items implemented, missing: `{}`",
+        missing_items_msg
+    );
+    err.span_label(impl_span, format!("missing `{}` in implementation", missing_items_msg));
+
+    // `Span` before impl block closing brace.
+    let hi = full_impl_span.hi() - BytePos(1);
+    // Point at the place right before the closing brace of the relevant `impl` to suggest
+    // adding the associated item at the end of its body.
+    let sugg_sp = full_impl_span.with_lo(hi).with_hi(hi);
+    // Obtain the level of indentation ending in `sugg_sp`.
+    let indentation = tcx.sess.source_map().span_to_margin(sugg_sp).unwrap_or(0);
+    // Make the whitespace that will make the suggestion have the right indentation.
+    let padding: String = (0..indentation).map(|_| " ").collect();
+
+    for trait_item in missing_items {
+        let snippet = suggestion_signature(&trait_item, tcx);
+        let code = format!("{}{}\n{}", padding, snippet, padding);
+        let msg = format!("implement the missing item: `{}`", snippet);
+        let appl = Applicability::HasPlaceholders;
+        if let Some(span) = tcx.hir().span_if_local(trait_item.def_id) {
+            err.span_label(span, format!("`{}` from trait", trait_item.ident));
+            err.tool_only_span_suggestion(sugg_sp, &msg, code, appl);
+        } else {
+            err.span_suggestion_hidden(sugg_sp, &msg, code, appl);
+        }
+    }
+    err.emit();
+}
+
+/// Return placeholder code for the given function.
+fn fn_sig_suggestion(sig: &ty::FnSig<'_>, ident: Ident) -> String {
+    let args = sig.inputs()
+        .iter()
+        .map(|ty| Some(match ty.kind {
+            ty::Param(param) if param.name == kw::SelfUpper => "self".to_string(),
+            ty::Ref(reg, ref_ty, mutability) => {
+                let reg = match &format!("{}", reg)[..] {
+                    "'_" | "" => String::new(),
+                    reg => format!("{} ", reg),
+                };
+                match ref_ty.kind {
+                    ty::Param(param) if param.name == kw::SelfUpper => {
+                        format!("&{}{}self", reg, mutability.prefix_str())
+                    }
+                    _ => format!("_: {:?}", ty),
+                }
+            }
+            _ => format!("_: {:?}", ty),
+        }))
+        .chain(std::iter::once(if sig.c_variadic {
+            Some("...".to_string())
+        } else {
+            None
+        }))
+        .filter_map(|arg| arg)
+        .collect::<Vec<String>>()
+        .join(", ");
+    let output = sig.output();
+    let output = if !output.is_unit() {
+        format!(" -> {:?}", output)
+    } else {
+        String::new()
+    };
+
+    let unsafety = sig.unsafety.prefix_str();
+    // FIXME: this is not entirely correct, as the lifetimes from borrowed params will
+    // not be present in the `fn` definition, not will we account for renamed
+    // lifetimes between the `impl` and the `trait`, but this should be good enough to
+    // fill in a significant portion of the missing code, and other subsequent
+    // suggestions can help the user fix the code.
+    format!("{}fn {}({}){} {{ unimplemented!() }}", unsafety, ident, args, output)
+}
+
+/// Return placeholder code for the given associated item.
+/// Similar to `ty::AssocItem::suggestion`, but appropriate for use as the code snippet of a
+/// structured suggestion.
+fn suggestion_signature(assoc: &ty::AssocItem, tcx: TyCtxt<'_>) -> String {
+    match assoc.kind {
+        ty::AssocKind::Method => {
+            // We skip the binder here because the binder would deanonymize all
+            // late-bound regions, and we don't want method signatures to show up
+            // `as for<'r> fn(&'r MyType)`.  Pretty-printing handles late-bound
+            // regions just fine, showing `fn(&MyType)`.
+            fn_sig_suggestion(tcx.fn_sig(assoc.def_id).skip_binder(), assoc.ident)
+        }
+        ty::AssocKind::Type => format!("type {} = Type;", assoc.ident),
+        // FIXME(type_alias_impl_trait): we should print bounds here too.
+        ty::AssocKind::OpaqueTy => format!("type {} = Type;", assoc.ident),
+        ty::AssocKind::Const => {
+            let ty = tcx.type_of(assoc.def_id);
+            let val = expr::ty_kind_suggestion(ty).unwrap_or("value");
+            format!("const {}: {:?} = {};", assoc.ident, ty, val)
+        }
     }
 }
 

src/test/ui/impl-trait/trait_type.stderr

@@ -29,7 +29,7 @@ error[E0046]: not all trait items implemented, missing: `fmt`
 LL | impl std::fmt::Display for MyType4 {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `fmt` in implementation
    |
-   = note: `fmt` from trait: `fn(&Self, &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>`
+   = help: implement the missing item: `fn fmt(&self, _: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> { unimplemented!() }`
 
 error: aborting due to 4 previous errors
 

src/test/ui/issues/issue-3344.stderr

@@ -4,7 +4,7 @@ error[E0046]: not all trait items implemented, missing: `partial_cmp`
 LL | impl PartialOrd for Thing {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^ missing `partial_cmp` in implementation
    |
-   = note: `partial_cmp` from trait: `fn(&Self, &Rhs) -> std::option::Option<std::cmp::Ordering>`
+   = help: implement the missing item: `fn partial_cmp(&self, _: &Rhs) -> std::option::Option<std::cmp::Ordering> { unimplemented!() }`
 
 error: aborting due to previous error