Rollup merge of rust-lang#138238 - compiler-errors:dyn-suggestion-in-struct, r=nnethercote

Fix dyn -> param suggestion in struct ICEs

Makes the logic from rust-lang#138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code.

Fixes rust-lang#138229
Fixes rust-lang#138211

r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

Author: matthiaskrgr

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

@@ -78,13 +78,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         }
 
         if self_ty.span.edition().at_least_rust_2021() {
-            let msg = "expected a type, found a trait";
-            let label = "you can add the `dyn` keyword if you want a trait object";
-            let mut diag =
-                rustc_errors::struct_span_code_err!(self.dcx(), self_ty.span, E0782, "{}", msg);
+            let mut diag = rustc_errors::struct_span_code_err!(
+                self.dcx(),
+                self_ty.span,
+                E0782,
+                "{}",
+                "expected a type, found a trait"
+            );
             if self_ty.span.can_be_used_for_suggestions()
                 && !self.maybe_suggest_impl_trait(self_ty, &mut diag)
-                && !self.maybe_suggest_dyn_trait(self_ty, label, sugg, &mut diag)
+                && !self.maybe_suggest_dyn_trait(self_ty, sugg, &mut diag)
             {
                 self.maybe_suggest_add_generic_impl_trait(self_ty, &mut diag);
             }
@@ -123,31 +126,62 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         }
     }
 
+    /// For a struct or enum with an invalid bare trait object field, suggest turning
+    /// it into a generic type bound.
     fn maybe_suggest_add_generic_impl_trait(
         &self,
         self_ty: &hir::Ty<'_>,
         diag: &mut Diag<'_>,
     ) -> bool {
         let tcx = self.tcx();
-        let msg = "you might be missing a type parameter";
-        let mut sugg = vec![];
 
-        let parent_id = tcx.hir_get_parent_item(self_ty.hir_id).def_id;
-        let parent_item = tcx.hir_node_by_def_id(parent_id).expect_item();
-        match parent_item.kind {
-            hir::ItemKind::Struct(_, generics) | hir::ItemKind::Enum(_, generics) => {
-                sugg.push((
-                    generics.where_clause_span,
-                    format!(
-                        "<T: {}>",
-                        self.tcx().sess.source_map().span_to_snippet(self_ty.span).unwrap()
-                    ),
-                ));
-                sugg.push((self_ty.span, "T".to_string()));
+        let parent_hir_id = tcx.parent_hir_id(self_ty.hir_id);
+        let parent_item = tcx.hir_get_parent_item(self_ty.hir_id).def_id;
+
+        let generics = match tcx.hir_node_by_def_id(parent_item) {
+            hir::Node::Item(hir::Item {
+                kind: hir::ItemKind::Struct(variant, generics), ..
+            }) => {
+                if !variant.fields().iter().any(|field| field.hir_id == parent_hir_id) {
+                    return false;
+                }
+                generics
             }
-            _ => {}
+            hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(def, generics), .. }) => {
+                if !def
+                    .variants
+                    .iter()
+                    .flat_map(|variant| variant.data.fields().iter())
+                    .any(|field| field.hir_id == parent_hir_id)
+                {
+                    return false;
+                }
+                generics
+            }
+            _ => return false,
+        };
+
+        let Ok(rendered_ty) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
+            return false;
+        };
+
+        let param = "TUV"
+            .chars()
+            .map(|c| c.to_string())
+            .chain((0..).map(|i| format!("P{i}")))
+            .find(|s| !generics.params.iter().any(|param| param.name.ident().as_str() == s))
+            .expect("we definitely can find at least one param name to generate");
+        let mut sugg = vec![(self_ty.span, param.to_string())];
+        if let Some(insertion_span) = generics.span_for_param_suggestion() {
+            sugg.push((insertion_span, format!(", {param}: {}", rendered_ty)));
+        } else {
+            sugg.push((generics.where_clause_span, format!("<{param}: {}>", rendered_ty)));
         }
-        diag.multipart_suggestion_verbose(msg, sugg, Applicability::MachineApplicable);
+        diag.multipart_suggestion_verbose(
+            "you might be missing a type parameter",
+            sugg,
+            Applicability::MachineApplicable,
+        );
         true
     }
     /// Make sure that we are in the condition to suggest the blanket implementation.
@@ -198,32 +232,59 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         }
     }
 
+    /// Try our best to approximate when adding `dyn` would be helpful for a bare
+    /// trait object.
+    ///
+    /// Right now, this is if the type is either directly nested in another ty,
+    /// or if it's in the tail field within a struct. This approximates what the
+    /// user would've gotten on edition 2015, except for the case where we have
+    /// an *obvious* knock-on `Sized` error.
     fn maybe_suggest_dyn_trait(
         &self,
         self_ty: &hir::Ty<'_>,
-        label: &str,
         sugg: Vec<(Span, String)>,
         diag: &mut Diag<'_>,
     ) -> bool {
         let tcx = self.tcx();
-        let parent_id = tcx.hir_get_parent_item(self_ty.hir_id).def_id;
-        let parent_item = tcx.hir_node_by_def_id(parent_id).expect_item();
 
-        // If the parent item is an enum, don't suggest the dyn trait.
-        if let hir::ItemKind::Enum(..) = parent_item.kind {
-            return false;
-        }
+        // Look at the direct HIR parent, since we care about the relationship between
+        // the type and the thing that directly encloses it.
+        match tcx.parent_hir_node(self_ty.hir_id) {
+            // These are all generally ok. Namely, when a trait object is nested
+            // into another expression or ty, it's either very certain that they
+            // missed the ty (e.g. `&Trait`) or it's not really possible to tell
+            // what their intention is, so let's not give confusing suggestions and
+            // just mention `dyn`. The user can make up their mind what to do here.
+            hir::Node::Ty(_)
+            | hir::Node::Expr(_)
+            | hir::Node::PatExpr(_)
+            | hir::Node::PathSegment(_)
+            | hir::Node::AssocItemConstraint(_)
+            | hir::Node::TraitRef(_)
+            | hir::Node::Item(_)
+            | hir::Node::WherePredicate(_) => {}
 
-        // If the parent item is a struct, check if self_ty is the last field.
-        if let hir::ItemKind::Struct(variant_data, _) = parent_item.kind {
-            if variant_data.fields().last().unwrap().ty.span != self_ty.span {
-                return false;
+            hir::Node::Field(field) => {
+                // Enums can't have unsized fields, fields can only have an unsized tail field.
+                if let hir::Node::Item(hir::Item {
+                    kind: hir::ItemKind::Struct(variant, _), ..
+                }) = tcx.parent_hir_node(field.hir_id)
+                    && variant
+                        .fields()
+                        .last()
+                        .is_some_and(|tail_field| tail_field.hir_id == field.hir_id)
+                {
+                    // Ok
+                } else {
+                    return false;
+                }
             }
+            _ => return false,
         }
 
         // FIXME: Only emit this suggestion if the trait is dyn-compatible.
         diag.multipart_suggestion_verbose(
-            label.to_string(),
+            "you can add the `dyn` keyword if you want a trait object",
             sugg,
             Applicability::MachineApplicable,
         );

tests/ui/dyn-keyword/dyn-2021-edition-error.rs

@@ -7,6 +7,12 @@ fn function(x: &SomeTrait, y: Box<SomeTrait>) {
     //~^ ERROR expected a type, found a trait
 }
 
+// Regression test for <https://github.com/rust-lang/rust/issues/138211>.
+extern "C" {
+    fn foo() -> *const SomeTrait;
+    //~^ ERROR expected a type, found a trait
+}
+
 trait SomeTrait {}
 
 fn main() {}

tests/ui/dyn-keyword/dyn-2021-edition-error.stderr

@@ -29,6 +29,17 @@ help: you can add the `dyn` keyword if you want a trait object
 LL | fn function(x: &SomeTrait, y: Box<dyn SomeTrait>) {
    |                                   +++
 
+error[E0782]: expected a type, found a trait
+  --> $DIR/dyn-2021-edition-error.rs:12:24
+   |
+LL |     fn foo() -> *const SomeTrait;
+   |                        ^^^^^^^^^
+   |
+help: you can add the `dyn` keyword if you want a trait object
+   |
+LL |     fn foo() -> *const dyn SomeTrait;
+   |                        +++
+
 error[E0782]: expected a type, found a trait
   --> $DIR/dyn-2021-edition-error.rs:6:14
    |
@@ -40,6 +51,6 @@ help: you can add the `dyn` keyword if you want a trait object
 LL |     let _x: &dyn SomeTrait = todo!();
    |              +++
 
-error: aborting due to 3 previous errors
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0782`.

tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.rs renamed to tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs

@@ -13,7 +13,6 @@ struct Foo2 {
     //~^ ERROR expected a type, found a trait
 }
 
-
 enum Enum1 {
     A(Trait),
     //~^ ERROR expected a type, found a trait
@@ -26,5 +25,17 @@ enum Enum2 {
     //~^ ERROR expected a type, found a trait
 }
 
+// Regression test for <https://github.com/rust-lang/rust/issues/138229>.
+pub struct InWhereClause
+where
+    Trait:, {}
+//~^ ERROR expected a type, found a trait
+
+struct HasGenerics<T> {
+    f: Trait,
+    //~^ ERROR expected a type, found a trait
+    t: T,
+}
+
 
 fn main() {}

tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.stderr renamed to tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr

@@ -22,7 +22,7 @@ LL |     b: dyn Trait,
    |        +++
 
 error[E0782]: expected a type, found a trait
-  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:18:7
+  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:17:7
    |
 LL |     A(Trait),
    |       ^^^^^
@@ -34,7 +34,7 @@ LL ~     A(T),
    |
 
 error[E0782]: expected a type, found a trait
-  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:25:7
+  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:24:7
    |
 LL |     B(Trait),
    |       ^^^^^
@@ -46,6 +46,29 @@ LL |     A(u32),
 LL ~     B(T),
    |
 
-error: aborting due to 4 previous errors
+error[E0782]: expected a type, found a trait
+  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:35:8
+   |
+LL |     f: Trait,
+   |        ^^^^^
+   |
+help: you might be missing a type parameter
+   |
+LL ~ struct HasGenerics<T, U: Trait> {
+LL ~     f: U,
+   |
+
+error[E0782]: expected a type, found a trait
+  --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:31:5
+   |
+LL |     Trait:, {}
+   |     ^^^^^
+   |
+help: you can add the `dyn` keyword if you want a trait object
+   |
+LL |     dyn Trait:, {}
+   |     +++
+
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0782`.