Rollup merge of #108930 - Ezrashaw:better-error-for-manual-fn-impl, r=petrochenkov

matthiaskrgr · web-flow · commit 76994625f82f · 2023-03-10T19:59:19.000+01:00
feat: implement better error for manual impl of `Fn*` traits Fixes #39259 cc `@estebank` (you gave me some advice in the linked issue, would you like to review?)
diff --git a/compiler/rustc_hir_analysis/locales/en-US.ftl b/compiler/rustc_hir_analysis/locales/en-US.ftl
@@ -42,6 +42,9 @@ hir_analysis_assoc_type_binding_not_allowed =
     associated type bindings are not allowed here
     .label = associated type not allowed here
 
+hir_analysis_parenthesized_fn_trait_expansion =
+    parenthesized trait syntax expands to `{$expanded_type}`
+
 hir_analysis_typeof_reserved_keyword_used =
     `typeof` is a reserved keyword but unimplemented
     .suggestion = consider replacing `typeof(...)` with an actual type
diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs
@@ -1,10 +1,14 @@
 use crate::astconv::AstConv;
-use crate::errors::{ManualImplementation, MissingTypeParams};
+use crate::errors::{
+    AssocTypeBindingNotAllowed, ManualImplementation, MissingTypeParams,
+    ParenthesizedFnTraitExpansion,
+};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_infer::traits::FulfillmentError;
+use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::{self, Ty};
 use rustc_session::parse::feature_err;
 use rustc_span::edit_distance::find_best_match_for_name;
@@ -78,43 +82,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             // Do not suggest the other syntax if we are in trait impl:
             // the desugaring would contain an associated type constraint.
             if !is_impl {
-                let args = trait_segment
-                    .args
-                    .as_ref()
-                    .and_then(|args| args.args.get(0))
-                    .and_then(|arg| match arg {
-                        hir::GenericArg::Type(ty) => match ty.kind {
-                            hir::TyKind::Tup(t) => t
-                                .iter()
-                                .map(|e| sess.source_map().span_to_snippet(e.span))
-                                .collect::<Result<Vec<_>, _>>()
-                                .map(|a| a.join(", ")),
-                            _ => sess.source_map().span_to_snippet(ty.span),
-                        }
-                        .map(|s| format!("({})", s))
-                        .ok(),
-                        _ => None,
-                    })
-                    .unwrap_or_else(|| "()".to_string());
-                let ret = trait_segment
-                    .args()
-                    .bindings
-                    .iter()
-                    .find_map(|b| match (b.ident.name == sym::Output, &b.kind) {
-                        (true, hir::TypeBindingKind::Equality { term }) => {
-                            let span = match term {
-                                hir::Term::Ty(ty) => ty.span,
-                                hir::Term::Const(c) => self.tcx().hir().span(c.hir_id),
-                            };
-                            sess.source_map().span_to_snippet(span).ok()
-                        }
-                        _ => None,
-                    })
-                    .unwrap_or_else(|| "()".to_string());
                 err.span_suggestion(
                     span,
                     "use parenthetical notation instead",
-                    format!("{}{} -> {}", trait_segment.ident, args, ret),
+                    fn_trait_to_string(self.tcx(), trait_segment, true),
                     Applicability::MaybeIncorrect,
                 );
             }
@@ -629,3 +600,69 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         err.emit();
     }
 }
+
+/// Emits an error regarding forbidden type binding associations
+pub fn prohibit_assoc_ty_binding(
+    tcx: TyCtxt<'_>,
+    span: Span,
+    segment: Option<(&hir::PathSegment<'_>, Span)>,
+) {
+    tcx.sess.emit_err(AssocTypeBindingNotAllowed { span, fn_trait_expansion: if let Some((segment, span)) = segment && segment.args().parenthesized {
+        Some(ParenthesizedFnTraitExpansion { span, expanded_type: fn_trait_to_string(tcx, segment, false) })
+    } else {
+        None
+    }});
+}
+
+pub(crate) fn fn_trait_to_string(
+    tcx: TyCtxt<'_>,
+    trait_segment: &hir::PathSegment<'_>,
+    parenthesized: bool,
+) -> String {
+    let args = trait_segment
+        .args
+        .as_ref()
+        .and_then(|args| args.args.get(0))
+        .and_then(|arg| match arg {
+            hir::GenericArg::Type(ty) => match ty.kind {
+                hir::TyKind::Tup(t) => t
+                    .iter()
+                    .map(|e| tcx.sess.source_map().span_to_snippet(e.span))
+                    .collect::<Result<Vec<_>, _>>()
+                    .map(|a| a.join(", ")),
+                _ => tcx.sess.source_map().span_to_snippet(ty.span),
+            }
+            .map(|s| {
+                // `s.empty()` checks to see if the type is the unit tuple, if so we don't want a comma
+                if parenthesized || s.is_empty() { format!("({})", s) } else { format!("({},)", s) }
+            })
+            .ok(),
+            _ => None,
+        })
+        .unwrap_or_else(|| "()".to_string());
+
+    let ret = trait_segment
+        .args()
+        .bindings
+        .iter()
+        .find_map(|b| match (b.ident.name == sym::Output, &b.kind) {
+            (true, hir::TypeBindingKind::Equality { term }) => {
+                let span = match term {
+                    hir::Term::Ty(ty) => ty.span,
+                    hir::Term::Const(c) => tcx.hir().span(c.hir_id),
+                };
+
+                (span != tcx.hir().span(trait_segment.hir_id))
+                    .then_some(tcx.sess.source_map().span_to_snippet(span).ok())
+                    .flatten()
+            }
+            _ => None,
+        })
+        .unwrap_or_else(|| "()".to_string());
+
+    if parenthesized {
+        format!("{}{} -> {}", trait_segment.ident, args, ret)
+    } else {
+        format!("{}<{}, Output={}>", trait_segment.ident, args, ret)
+    }
+}
diff --git a/compiler/rustc_hir_analysis/src/astconv/generics.rs b/compiler/rustc_hir_analysis/src/astconv/generics.rs
@@ -1,9 +1,8 @@
 use super::IsMethodCall;
 use crate::astconv::{
-    CreateSubstsForGenericArgsCtxt, ExplicitLateBound, GenericArgCountMismatch,
-    GenericArgCountResult, GenericArgPosition,
+    errors::prohibit_assoc_ty_binding, CreateSubstsForGenericArgsCtxt, ExplicitLateBound,
+    GenericArgCountMismatch, GenericArgCountResult, GenericArgPosition,
 };
-use crate::errors::AssocTypeBindingNotAllowed;
 use crate::structured_errors::{GenericArgsInfo, StructuredDiagnostic, WrongNumberOfGenericArgs};
 use rustc_ast::ast::ParamKindOrd;
 use rustc_errors::{struct_span_err, Applicability, Diagnostic, ErrorGuaranteed, MultiSpan};
@@ -433,7 +432,7 @@ pub(crate) fn check_generic_arg_count(
         (gen_pos != GenericArgPosition::Type || infer_args) && !gen_args.has_lifetime_params();
 
     if gen_pos != GenericArgPosition::Type && let Some(b) = gen_args.bindings.first() {
-            prohibit_assoc_ty_binding(tcx, b.span);
+             prohibit_assoc_ty_binding(tcx, b.span, None);
         }
 
     let explicit_late_bound =
@@ -589,11 +588,6 @@ pub(crate) fn check_generic_arg_count(
     }
 }
 
-/// Emits an error regarding forbidden type binding associations
-pub fn prohibit_assoc_ty_binding(tcx: TyCtxt<'_>, span: Span) {
-    tcx.sess.emit_err(AssocTypeBindingNotAllowed { span });
-}
-
 /// Prohibits explicit lifetime arguments if late-bound lifetime parameters
 /// are present. This is used both for datatypes and function calls.
 pub(crate) fn prohibit_explicit_late_bound_lifetimes(
diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs
@@ -5,9 +5,8 @@
 mod errors;
 pub mod generics;
 
-use crate::astconv::generics::{
-    check_generic_arg_count, create_substs_for_generic_args, prohibit_assoc_ty_binding,
-};
+use crate::astconv::errors::prohibit_assoc_ty_binding;
+use crate::astconv::generics::{check_generic_arg_count, create_substs_for_generic_args};
 use crate::bounds::Bounds;
 use crate::collect::HirPlaceholderCollector;
 use crate::errors::{
@@ -295,7 +294,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             ty::BoundConstness::NotConst,
         );
         if let Some(b) = item_segment.args().bindings.first() {
-            prohibit_assoc_ty_binding(self.tcx(), b.span);
+            prohibit_assoc_ty_binding(self.tcx(), b.span, Some((item_segment, span)));
         }
 
         substs
@@ -631,7 +630,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         );
 
         if let Some(b) = item_segment.args().bindings.first() {
-            prohibit_assoc_ty_binding(self.tcx(), b.span);
+            prohibit_assoc_ty_binding(self.tcx(), b.span, Some((item_segment, span)));
         }
 
         args
@@ -825,7 +824,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             constness,
         );
         if let Some(b) = trait_segment.args().bindings.first() {
-            prohibit_assoc_ty_binding(self.tcx(), b.span);
+            prohibit_assoc_ty_binding(self.tcx(), b.span, Some((trait_segment, span)));
         }
         self.tcx().mk_trait_ref(trait_def_id, substs)
     }
@@ -2596,7 +2595,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         for segment in segments {
             // Only emit the first error to avoid overloading the user with error messages.
             if let Some(b) = segment.args().bindings.first() {
-                prohibit_assoc_ty_binding(self.tcx(), b.span);
+                prohibit_assoc_ty_binding(self.tcx(), b.span, None);
                 return true;
             }
         }
diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs
@@ -129,6 +129,18 @@ pub struct AssocTypeBindingNotAllowed {
     #[primary_span]
     #[label]
     pub span: Span,
+
+    #[subdiagnostic]
+    pub fn_trait_expansion: Option<ParenthesizedFnTraitExpansion>,
+}
+
+#[derive(Subdiagnostic)]
+#[help(hir_analysis_parenthesized_fn_trait_expansion)]
+pub struct ParenthesizedFnTraitExpansion {
+    #[primary_span]
+    pub span: Span,
+
+    pub expanded_type: String,
 }
 
 #[derive(Diagnostic)]
diff --git a/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr b/tests/ui/feature-gates/feature-gate-unboxed-closures-manual-impls.stderr
@@ -64,6 +64,12 @@ error[E0229]: associated type bindings are not allowed here
    |
 LL | impl FnOnce() for Foo1 {
    |      ^^^^^^^^ associated type not allowed here
+   |
+help: parenthesized trait syntax expands to `FnOnce<(), Output=()>`
+  --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:16:6
+   |
+LL | impl FnOnce() for Foo1 {
+   |      ^^^^^^^^
 
 error[E0658]: the precise format of `Fn`-family traits' type parameters is subject to change
   --> $DIR/feature-gate-unboxed-closures-manual-impls.rs:23:6
diff --git a/tests/ui/fn/issue-39259.rs b/tests/ui/fn/issue-39259.rs
@@ -0,0 +1,13 @@
+#![feature(fn_traits)]
+#![feature(unboxed_closures)]
+
+struct S;
+
+impl Fn(u32) -> u32 for S {
+//~^ ERROR associated type bindings are not allowed here [E0229]
+    fn call(&self) -> u32 {
+        5
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/fn/issue-39259.stderr b/tests/ui/fn/issue-39259.stderr
@@ -0,0 +1,15 @@
+error[E0229]: associated type bindings are not allowed here
+  --> $DIR/issue-39259.rs:6:17
+   |
+LL | impl Fn(u32) -> u32 for S {
+   |                 ^^^ associated type not allowed here
+   |
+help: parenthesized trait syntax expands to `Fn<(u32,), Output=u32>`
+  --> $DIR/issue-39259.rs:6:6
+   |
+LL | impl Fn(u32) -> u32 for S {
+   |      ^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0229`.
diff --git a/tests/ui/lifetimes/issue-95023.stderr b/tests/ui/lifetimes/issue-95023.stderr
@@ -25,6 +25,12 @@ error[E0229]: associated type bindings are not allowed here
    |
 LL | impl Fn(&isize) for Error {
    |      ^^^^^^^^^^ associated type not allowed here
+   |
+help: parenthesized trait syntax expands to `Fn<(&isize,), Output=()>`
+  --> $DIR/issue-95023.rs:3:6
+   |
+LL | impl Fn(&isize) for Error {
+   |      ^^^^^^^^^^
 
 error[E0220]: associated type `B` not found for `Self`
   --> $DIR/issue-95023.rs:6:44
diff --git a/tests/ui/traits/issue-87558.stderr b/tests/ui/traits/issue-87558.stderr
@@ -17,6 +17,12 @@ error[E0229]: associated type bindings are not allowed here
    |
 LL | impl Fn(&isize) for Error {
    |      ^^^^^^^^^^ associated type not allowed here
+   |
+help: parenthesized trait syntax expands to `Fn<(&isize,), Output=()>`
+  --> $DIR/issue-87558.rs:3:6
+   |
+LL | impl Fn(&isize) for Error {
+   |      ^^^^^^^^^^
 
 error: aborting due to 3 previous errors
 
