From bca0ab8d7a7b06406966c7a710e9f4ad61d064fb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 8 Mar 2025 19:44:43 +0000 Subject: [PATCH 1/2] Rework maybe_suggest_add_generic_impl_trait --- .../src/hir_ty_lowering/lint.rs | 95 ++++++++++++++----- .../ui/dyn-keyword/dyn-2021-edition-error.rs | 6 ++ .../dyn-keyword/dyn-2021-edition-error.stderr | 13 ++- ...-struct-or-union-add-generic-impl-trait.rs | 6 +- ...uct-or-union-add-generic-impl-trait.stderr | 17 +++- 5 files changed, 108 insertions(+), 29 deletions(-) rename tests/ui/{suggestions => dyn-keyword}/suggest-struct-or-union-add-generic-impl-trait.rs (71%) rename tests/ui/{suggestions => dyn-keyword}/suggest-struct-or-union-add-generic-impl-trait.stderr (69%) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs index 3611db7c68f16..43348791fa527 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs @@ -123,6 +123,8 @@ 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<'_>, @@ -132,21 +134,38 @@ impl<'tcx> dyn HirTyLowerer<'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!( - "", - 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, + }; + + // FIXME: `T` may already be taken. + sugg.push(( + generics.where_clause_span, + format!("", self.tcx().sess.source_map().span_to_snippet(self_ty.span).unwrap()), + )); + sugg.push((self_ty.span, "T".to_string())); diag.multipart_suggestion_verbose(msg, sugg, Applicability::MachineApplicable); true } @@ -198,6 +217,13 @@ 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<'_>, @@ -206,19 +232,40 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { 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. diff --git a/tests/ui/dyn-keyword/dyn-2021-edition-error.rs b/tests/ui/dyn-keyword/dyn-2021-edition-error.rs index 5d607d82ea1ac..cc23c2c5055db 100644 --- a/tests/ui/dyn-keyword/dyn-2021-edition-error.rs +++ b/tests/ui/dyn-keyword/dyn-2021-edition-error.rs @@ -7,6 +7,12 @@ fn function(x: &SomeTrait, y: Box) { //~^ ERROR expected a type, found a trait } +// Regression test for . +extern "C" { + fn foo() -> *const SomeTrait; + //~^ ERROR expected a type, found a trait +} + trait SomeTrait {} fn main() {} diff --git a/tests/ui/dyn-keyword/dyn-2021-edition-error.stderr b/tests/ui/dyn-keyword/dyn-2021-edition-error.stderr index 8c4b809e76b04..b1d6385bde99e 100644 --- a/tests/ui/dyn-keyword/dyn-2021-edition-error.stderr +++ b/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) { | +++ +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`. diff --git a/tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.rs b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs similarity index 71% rename from tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.rs rename to tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs index 9963b5be4f2b8..dcbed027c7483 100644 --- a/tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.rs +++ b/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,10 @@ enum Enum2 { //~^ ERROR expected a type, found a trait } +// Regression test for . +pub struct InWhereClause +where + Trait:, {} +//~^ ERROR expected a type, found a trait fn main() {} diff --git a/tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.stderr b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr similarity index 69% rename from tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.stderr rename to tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr index 433196919caf2..16e48ef3a1293 100644 --- a/tests/ui/suggestions/suggest-struct-or-union-add-generic-impl-trait.stderr +++ b/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,17 @@ 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:31:5 + | +LL | Trait:, {} + | ^^^^^ + | +help: you can add the `dyn` keyword if you want a trait object + | +LL | dyn Trait:, {} + | +++ + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0782`. From ceb040135dedd7de4b3a77d25c5b4f485b6baf9d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 8 Mar 2025 20:20:15 +0000 Subject: [PATCH 2/2] Fix suggestion when there are generics, inline some things --- .../src/hir_ty_lowering/lint.rs | 46 ++++++++++++------- ...-struct-or-union-add-generic-impl-trait.rs | 7 +++ ...uct-or-union-add-generic-impl-trait.stderr | 14 +++++- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs index 43348791fa527..e78801dd60149 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs +++ b/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); } @@ -131,8 +134,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { diag: &mut Diag<'_>, ) -> bool { let tcx = self.tcx(); - let msg = "you might be missing a type parameter"; - let mut sugg = vec![]; 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; @@ -160,13 +161,27 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { _ => return false, }; - // FIXME: `T` may already be taken. - sugg.push(( - generics.where_clause_span, - format!("", self.tcx().sess.source_map().span_to_snippet(self_ty.span).unwrap()), - )); - sugg.push((self_ty.span, "T".to_string())); - diag.multipart_suggestion_verbose(msg, sugg, Applicability::MachineApplicable); + 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( + "you might be missing a type parameter", + sugg, + Applicability::MachineApplicable, + ); true } /// Make sure that we are in the condition to suggest the blanket implementation. @@ -227,7 +242,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { fn maybe_suggest_dyn_trait( &self, self_ty: &hir::Ty<'_>, - label: &str, sugg: Vec<(Span, String)>, diag: &mut Diag<'_>, ) -> bool { @@ -270,7 +284,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // 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, ); diff --git a/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs index dcbed027c7483..4d573b90d606d 100644 --- a/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs +++ b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs @@ -31,4 +31,11 @@ where Trait:, {} //~^ ERROR expected a type, found a trait +struct HasGenerics { + f: Trait, + //~^ ERROR expected a type, found a trait + t: T, +} + + fn main() {} diff --git a/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr index 16e48ef3a1293..9584147bbc77f 100644 --- a/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr +++ b/tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.stderr @@ -46,6 +46,18 @@ LL | A(u32), LL ~ B(T), | +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 { +LL ~ f: U, + | + error[E0782]: expected a type, found a trait --> $DIR/suggest-struct-or-union-add-generic-impl-trait.rs:31:5 | @@ -57,6 +69,6 @@ help: you can add the `dyn` keyword if you want a trait object LL | dyn Trait:, {} | +++ -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0782`.