Suggest missing item from trait in impl

Author: estebank

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()
     }
 }
+
+crate 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

@@ -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`
@@ -1934,26 +1934,115 @@ fn check_impl_items_against_trait<'tcx>(
                 missing_items.iter()
                     .map(|trait_item| trait_item.ident.to_string())
                     .collect::<Vec<_>>().join("`, `")));
+
+        // `Span` before impl block closing brace.
+        let hi = full_impl_span.hi() - BytePos(1);
+        let sugg_sp = full_impl_span.with_lo(hi).with_hi(hi);
+        let indentation = tcx.sess.source_map().span_to_margin(sugg_sp).unwrap_or(0);
+        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.note_trait_signature(trait_item.ident.to_string(),
-                                         trait_item.signature(tcx));
+                err.span_suggestion_hidden(sugg_sp, &msg, code, appl);
             }
         }
         err.emit();
     }
 
     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("`, `"))
+    }
+}
+
+/// Given a `ty::AssocItem` and a `TyCtxt`, return placeholder code for that 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)`.
+            let sig = tcx.fn_sig(assoc.def_id);
+            let unsafety = match sig.unsafety() {
+                hir::Unsafety::Unsafe => "unsafe ",
+                _ => "",
+            };
+            let args = sig.inputs()
+                .skip_binder()
+                .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 mutability = match mutability {
+                            hir::Mutability::MutMutable => "mut ",
+                            _ => "",
+                        };
+                        let mut reg = format!("{}", reg);
+                        if &reg[..] == "'_" {
+                            reg = "".to_string();
+                        }
+                        if &reg[..] != "" {
+                            reg = format!("{} ", reg);
+                        }
+                        match ref_ty.kind {
+                            ty::Param(param)
+                            if param.name == kw::SelfUpper => {
+                                format!("&{}{}self", reg, mutability)
+                            }
+                            _ => 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.skip_binder().is_unit() {
+                format!(" -> {:?}", output.skip_binder())
+            } else {
+                String::new()
+            };
+            // 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, assoc.ident, args, output)
+        }
+        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
 

src/test/ui/missing/missing-items/m2.stderr

@@ -4,9 +4,9 @@ error[E0046]: not all trait items implemented, missing: `CONSTANT`, `Type`, `met
 LL | impl m1::X for X {
    | ^^^^^^^^^^^^^^^^ missing `CONSTANT`, `Type`, `method` in implementation
    |
-   = note: `CONSTANT` from trait: `const CONSTANT: u32;`
-   = note: `Type` from trait: `type Type;`
-   = note: `method` from trait: `fn(&Self, std::string::String) -> <Self as m1::X>::Type`
+   = help: implement the missing item: `const CONSTANT: u32 = 42;`
+   = help: implement the missing item: `type Type = Type;`
+   = help: implement the missing item: `fn method(&self, _: std::string::String) -> <Self as m1::X>::Type { unimplemented!() }`
 
 error: aborting due to previous error
 

src/test/ui/span/impl-wrong-item-for-trait.stderr

@@ -64,7 +64,7 @@ error[E0046]: not all trait items implemented, missing: `fmt`
 LL | impl Debug for FooTypeForMethod {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 8 previous errors
 

src/test/ui/span/issue-23729.stderr

@@ -4,7 +4,7 @@ error[E0046]: not all trait items implemented, missing: `Item`
 LL |         impl Iterator for Recurrence {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Item` in implementation
    |
-   = note: `Item` from trait: `type Item;`
+   = help: implement the missing item: `type Item = Type;`
 
 error: aborting due to previous error
 

src/test/ui/span/issue-23827.stderr

@@ -4,7 +4,7 @@ error[E0046]: not all trait items implemented, missing: `Output`
 LL | impl<C: Component> FnOnce<(C,)> for Prototype {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Output` in implementation
    |
-   = note: `Output` from trait: `type Output;`
+   = help: implement the missing item: `type Output = Type;`
 
 error: aborting due to previous error
 

src/test/ui/span/issue-24356.stderr

@@ -4,7 +4,7 @@ error[E0046]: not all trait items implemented, missing: `Target`
 LL |         impl Deref for Thing {
    |         ^^^^^^^^^^^^^^^^^^^^ missing `Target` in implementation
    |
-   = note: `Target` from trait: `type Target;`
+   = help: implement the missing item: `type Target = Type;`
 
 error: aborting due to previous error
 

src/test/ui/suggestions/missing-trait-item.fixed

@@ -0,0 +1,20 @@
+// run-rustfix
+
+trait T {
+    unsafe fn foo(a: &usize, b: &usize) -> usize;
+    fn bar(&self, a: &usize, b: &usize) -> usize;
+}
+
+mod foo {
+    use super::T;
+    impl T for () {    fn bar(&self, _: &usize, _: &usize) -> usize { unimplemented!() }
+        unsafe fn foo(_: &usize, _: &usize) -> usize { unimplemented!() }
+    } //~ ERROR not all trait items
+
+    impl T for usize { //~ ERROR not all trait items
+        fn bar(&self, _: &usize, _: &usize) -> usize { unimplemented!() }
+        unsafe fn foo(_: &usize, _: &usize) -> usize { unimplemented!() }
+    }
+}
+
+fn main() {}

src/test/ui/suggestions/missing-trait-item.rs

@@ -0,0 +1,16 @@
+// run-rustfix
+
+trait T {
+    unsafe fn foo(a: &usize, b: &usize) -> usize;
+    fn bar(&self, a: &usize, b: &usize) -> usize;
+}
+
+mod foo {
+    use super::T;
+    impl T for () {} //~ ERROR not all trait items
+
+    impl T for usize { //~ ERROR not all trait items
+    }
+}
+
+fn main() {}

src/test/ui/suggestions/missing-trait-item.stderr

@@ -0,0 +1,25 @@
+error[E0046]: not all trait items implemented, missing: `foo`, `bar`
+  --> $DIR/missing-trait-item.rs:10:5
+   |
+LL |     unsafe fn foo(a: &usize, b: &usize) -> usize;
+   |     --------------------------------------------- `foo` from trait
+LL |     fn bar(&self, a: &usize, b: &usize) -> usize;
+   |     --------------------------------------------- `bar` from trait
+...
+LL |     impl T for () {}
+   |     ^^^^^^^^^^^^^ missing `foo`, `bar` in implementation
+
+error[E0046]: not all trait items implemented, missing: `foo`, `bar`
+  --> $DIR/missing-trait-item.rs:12:5
+   |
+LL |     unsafe fn foo(a: &usize, b: &usize) -> usize;
+   |     --------------------------------------------- `foo` from trait
+LL |     fn bar(&self, a: &usize, b: &usize) -> usize;
+   |     --------------------------------------------- `bar` from trait
+...
+LL |     impl T for usize {
+   |     ^^^^^^^^^^^^^^^^ missing `foo`, `bar` in implementation
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0046`.