From 722d136766ef7adeb525956285bf49f3883b9211 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 28 Aug 2022 15:59:06 +0900 Subject: [PATCH] Use hir::Map to prevent false positives --- .../wrong_number_of_generic_args.rs | 80 +++++++++++++------ src/test/ui/suggestions/issue-89064.rs | 2 - src/test/ui/suggestions/issue-89064.stderr | 16 +--- 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs b/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs index cfa8191d953d..ebf5baf5050c 100644 --- a/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_typeck/src/structured_errors/wrong_number_of_generic_args.rs @@ -524,8 +524,8 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { if self.not_enough_args_provided() { self.suggest_adding_args(err); } else if self.too_many_args_provided() { + self.suggest_moving_args_from_assoc_fn_to_trait(err); self.suggest_removing_args_or_generics(err); - self.suggest_moving_args(err); } else { unreachable!(); } @@ -661,34 +661,62 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { /// ```compile_fail /// Into::into::>(42) // suggests considering `Into::>::into(42)` /// ``` - fn suggest_moving_args(&self, err: &mut Diagnostic) { - if let Some(trait_) = self.tcx.trait_of_item(self.def_id) { - // HACK(hkmatsumoto): Ugly way to tell "::()" from "x.()"; - // we don't care the latter (for now). - if self.path_segment.res == Some(hir::def::Res::Err) { - return; + fn suggest_moving_args_from_assoc_fn_to_trait(&self, err: &mut Diagnostic) { + let trait_ = match self.tcx.trait_of_item(self.def_id) { + Some(def_id) => def_id, + None => return, + }; + + // Skip suggestion when the associated function is itself generic, it is unclear + // how to split the provided parameters between those to suggest to the trait and + // those to remain on the associated type. + let num_assoc_fn_expected_args = + self.num_expected_type_or_const_args() + self.num_expected_lifetime_args(); + if num_assoc_fn_expected_args > 0 { + return; + } + + let num_assoc_fn_excess_args = + self.num_excess_type_or_const_args() + self.num_excess_lifetime_args(); + + let trait_generics = self.tcx.generics_of(trait_); + let num_trait_generics_except_self = + trait_generics.count() - if trait_generics.has_self { 1 } else { 0 }; + + if let Some(hir_id) = self.path_segment.hir_id + && let Some(parent_node) = self.tcx.hir().find_parent_node(hir_id) + && let Some(parent_node) = self.tcx.hir().find(parent_node) + && let hir::Node::Expr(expr) = parent_node { + match expr.kind { + hir::ExprKind::Path(ref qpath) => { + self.suggest_moving_args_from_assoc_fn_to_trait_for_qualified_path( + err, + trait_, + qpath, + num_assoc_fn_excess_args, + num_trait_generics_except_self + ) + }, + // TODO(hkmatsumoto): Emit similar suggestion for "x.()" + hir::ExprKind::MethodCall(..) => return, + _ => return, } + } + } - // Say, if the assoc fn takes `A`, `B` and `C` as generic arguments while expecting 1 - // argument, and its trait expects 2 arguments. It is hard to "split" them right as - // there are too many cases to handle: `A` `B` | `C`, `A` `B` | `C`, `A` `C` | `B`, ... - let num_assoc_fn_expected_args = - self.num_expected_type_or_const_args() + self.num_expected_lifetime_args(); - if num_assoc_fn_expected_args > 0 { - return; - } + fn suggest_moving_args_from_assoc_fn_to_trait_for_qualified_path( + &self, + err: &mut Diagnostic, + trait_: DefId, + qpath: &'tcx hir::QPath<'tcx>, + num_assoc_fn_excess_args: usize, + num_trait_generics_except_self: usize, + ) { + if let hir::QPath::Resolved(_, path) = qpath + && let Some(trait_path_segment) = path.segments.get(0) { + let num_generic_args_supplied_to_trait = trait_path_segment.args().num_generic_params(); - let num_assoc_fn_excess_args = - self.num_excess_type_or_const_args() + self.num_excess_lifetime_args(); - - let trait_generics = self.tcx.generics_of(trait_); - let num_trait_generics_except_self = - trait_generics.count() - if trait_generics.has_self { 1 } else { 0 }; - - // FIXME(hkmatsumoto): RHS of this condition ideally should be - // `num_trait_generics_except_self` - "# of generic args already provided to trait" - // but unable to get that information with `self.def_id`. - if num_assoc_fn_excess_args == num_trait_generics_except_self { + if num_assoc_fn_excess_args == num_trait_generics_except_self - num_generic_args_supplied_to_trait { if let Some(span) = self.gen_args.span_ext() && let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { let msg = format!( diff --git a/src/test/ui/suggestions/issue-89064.rs b/src/test/ui/suggestions/issue-89064.rs index c0fcad4236e8..17a3e8b4cf83 100644 --- a/src/test/ui/suggestions/issue-89064.rs +++ b/src/test/ui/suggestions/issue-89064.rs @@ -24,9 +24,7 @@ fn main() { //~| HELP remove these generics //~| HELP consider moving these generic arguments - // bad suggestion let _ = A::::foo::(); //~^ ERROR //~| HELP remove these generics - //~| HELP consider moving this generic argument } diff --git a/src/test/ui/suggestions/issue-89064.stderr b/src/test/ui/suggestions/issue-89064.stderr index 11d652b4c603..846cd817505c 100644 --- a/src/test/ui/suggestions/issue-89064.stderr +++ b/src/test/ui/suggestions/issue-89064.stderr @@ -43,26 +43,18 @@ LL + let _ = B::::bar(); | error[E0107]: this associated function takes 0 generic arguments but 1 generic argument was supplied - --> $DIR/issue-89064.rs:28:21 + --> $DIR/issue-89064.rs:27:21 | LL | let _ = A::::foo::(); - | ^^^ expected 0 generic arguments + | ^^^----- help: remove these generics + | | + | expected 0 generic arguments | note: associated function defined here, with 0 generic parameters --> $DIR/issue-89064.rs:4:8 | LL | fn foo() {} | ^^^ -help: remove these generics - | -LL - let _ = A::::foo::(); -LL + let _ = A::::foo(); - | -help: consider moving this generic argument to the `A` trait, which takes up to 1 argument - | -LL - let _ = A::::foo::(); -LL + let _ = A::::::foo(); - | error: aborting due to 3 previous errors