From bafa89bc467a2b5862c409fc6cae9c01fdf25c6e Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 21 Aug 2022 16:59:09 +0900 Subject: [PATCH 1/4] Suggest moving redundant generic args of an assoc fn to its trait --- .../wrong_number_of_generic_args.rs | 59 ++++++++++++++++ src/test/ui/suggestions/issue-89064.rs | 32 +++++++++ src/test/ui/suggestions/issue-89064.stderr | 69 +++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 src/test/ui/suggestions/issue-89064.rs create mode 100644 src/test/ui/suggestions/issue-89064.stderr 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 99729391e02b0..cfa8191d953d5 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 @@ -525,6 +525,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { self.suggest_adding_args(err); } else if self.too_many_args_provided() { self.suggest_removing_args_or_generics(err); + self.suggest_moving_args(err); } else { unreachable!(); } @@ -654,6 +655,64 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { } } + /// Suggests moving redundant argument(s) of an associate function to the + /// trait it belongs to. + /// + /// ```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; + } + + // 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; + } + + 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 let Some(span) = self.gen_args.span_ext() + && let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { + let msg = format!( + "consider moving {these} generic argument{s} to the `{name}` trait, which takes up to {num} argument{s}", + these = pluralize!("this", num_assoc_fn_excess_args), + s = pluralize!(num_assoc_fn_excess_args), + name = self.tcx.item_name(trait_), + num = num_trait_generics_except_self, + ); + let sugg = vec![ + (self.path_segment.ident.span, format!("{}::{}", snippet, self.path_segment.ident)), + (span.with_lo(self.path_segment.ident.span.hi()), "".to_owned()) + ]; + + err.multipart_suggestion( + msg, + sugg, + Applicability::MaybeIncorrect + ); + } + } + } + } + /// Suggests to remove redundant argument(s): /// /// ```text diff --git a/src/test/ui/suggestions/issue-89064.rs b/src/test/ui/suggestions/issue-89064.rs new file mode 100644 index 0000000000000..c0fcad4236e84 --- /dev/null +++ b/src/test/ui/suggestions/issue-89064.rs @@ -0,0 +1,32 @@ +use std::convert::TryInto; + +trait A { + fn foo() {} +} + +trait B { + fn bar() {} +} + +struct S; + +impl A for S {} +impl B for S {} + +fn main() { + let _ = A::foo::(); + //~^ ERROR + //~| HELP remove these generics + //~| HELP consider moving this generic argument + + let _ = B::bar::(); + //~^ ERROR + //~| 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 new file mode 100644 index 0000000000000..11d652b4c6035 --- /dev/null +++ b/src/test/ui/suggestions/issue-89064.stderr @@ -0,0 +1,69 @@ +error[E0107]: this associated function takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/issue-89064.rs:17:16 + | +LL | let _ = A::foo::(); + | ^^^ 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[E0107]: this associated function takes 0 generic arguments but 2 generic arguments were supplied + --> $DIR/issue-89064.rs:22:16 + | +LL | let _ = B::bar::(); + | ^^^ expected 0 generic arguments + | +note: associated function defined here, with 0 generic parameters + --> $DIR/issue-89064.rs:8:8 + | +LL | fn bar() {} + | ^^^ +help: remove these generics + | +LL - let _ = B::bar::(); +LL + let _ = B::bar(); + | +help: consider moving these generic arguments to the `B` trait, which takes up to 2 arguments + | +LL - let _ = B::bar::(); +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 + | +LL | let _ = A::::foo::(); + | ^^^ 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 + +For more information about this error, try `rustc --explain E0107`. From 722d136766ef7adeb525956285bf49f3883b9211 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 28 Aug 2022 15:59:06 +0900 Subject: [PATCH 2/4] 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 cfa8191d953d5..ebf5baf5050c6 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; - } - - // 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(&self, err: &mut Diagnostic) { + let trait_ = match self.tcx.trait_of_item(self.def_id) { + Some(def_id) => def_id, + None => return, + }; - let num_assoc_fn_excess_args = - self.num_excess_type_or_const_args() + self.num_excess_lifetime_args(); + // 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 trait_generics = self.tcx.generics_of(trait_); - let num_trait_generics_except_self = - trait_generics.count() - if trait_generics.has_self { 1 } else { 0 }; + 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, + } + } + } - // 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 { + 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(); + + 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 c0fcad4236e84..17a3e8b4cf831 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 11d652b4c6035..846cd817505c3 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 From 75ed56f0cba4ab95b5d4a67c1f9235a5dbe9df8a Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 28 Aug 2022 16:18:44 +0900 Subject: [PATCH 3/4] Make CI pass --- .../src/structured_errors/wrong_number_of_generic_args.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ebf5baf5050c6..e4a90329108e0 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 @@ -697,7 +697,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { num_trait_generics_except_self ) }, - // TODO(hkmatsumoto): Emit similar suggestion for "x.()" + // FIXME(hkmatsumoto): Emit similar suggestion for "x.()" hir::ExprKind::MethodCall(..) => return, _ => return, } From 152913767a365d963f2486fb88eff976501d7f72 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 28 Aug 2022 17:51:28 +0900 Subject: [PATCH 4/4] Support method calls --- .../wrong_number_of_generic_args.rs | 53 +++++++++++++++---- .../invalid-const-arg-for-type-param.stderr | 13 +++-- src/test/ui/suggestions/issue-89064.rs | 5 ++ src/test/ui/suggestions/issue-89064.stderr | 39 ++++++++++---- 4 files changed, 87 insertions(+), 23 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 e4a90329108e0..36e94c3dce5ae 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 @@ -683,6 +683,14 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { let num_trait_generics_except_self = trait_generics.count() - if trait_generics.has_self { 1 } else { 0 }; + let msg = format!( + "consider moving {these} generic argument{s} to the `{name}` trait, which takes up to {num} argument{s}", + these = pluralize!("this", num_assoc_fn_excess_args), + s = pluralize!(num_assoc_fn_excess_args), + name = self.tcx.item_name(trait_), + num = num_trait_generics_except_self, + ); + 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) @@ -691,14 +699,22 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { hir::ExprKind::Path(ref qpath) => { self.suggest_moving_args_from_assoc_fn_to_trait_for_qualified_path( err, - trait_, qpath, + msg, + num_assoc_fn_excess_args, + num_trait_generics_except_self + ) + }, + hir::ExprKind::MethodCall(..) => { + self.suggest_moving_args_from_assoc_fn_to_trait_for_method_call( + err, + trait_, + expr, + msg, num_assoc_fn_excess_args, num_trait_generics_except_self ) }, - // FIXME(hkmatsumoto): Emit similar suggestion for "x.()" - hir::ExprKind::MethodCall(..) => return, _ => return, } } @@ -707,8 +723,8 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { fn suggest_moving_args_from_assoc_fn_to_trait_for_qualified_path( &self, err: &mut Diagnostic, - trait_: DefId, qpath: &'tcx hir::QPath<'tcx>, + msg: String, num_assoc_fn_excess_args: usize, num_trait_generics_except_self: usize, ) { @@ -719,13 +735,6 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { 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!( - "consider moving {these} generic argument{s} to the `{name}` trait, which takes up to {num} argument{s}", - these = pluralize!("this", num_assoc_fn_excess_args), - s = pluralize!(num_assoc_fn_excess_args), - name = self.tcx.item_name(trait_), - num = num_trait_generics_except_self, - ); let sugg = vec![ (self.path_segment.ident.span, format!("{}::{}", snippet, self.path_segment.ident)), (span.with_lo(self.path_segment.ident.span.hi()), "".to_owned()) @@ -741,6 +750,28 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { } } + fn suggest_moving_args_from_assoc_fn_to_trait_for_method_call( + &self, + err: &mut Diagnostic, + trait_: DefId, + expr: &'tcx hir::Expr<'tcx>, + msg: String, + num_assoc_fn_excess_args: usize, + num_trait_generics_except_self: usize, + ) { + if let hir::ExprKind::MethodCall(_, args, _) = expr.kind { + assert_eq!(args.len(), 1); + if num_assoc_fn_excess_args == num_trait_generics_except_self { + if let Some(gen_args) = self.gen_args.span_ext() + && let Ok(gen_args) = self.tcx.sess.source_map().span_to_snippet(gen_args) + && let Ok(args) = self.tcx.sess.source_map().span_to_snippet(args[0].span) { + let sugg = format!("{}::{}::{}({})", self.tcx.item_name(trait_), gen_args, self.tcx.item_name(self.def_id), args); + err.span_suggestion(expr.span, msg, sugg, Applicability::MaybeIncorrect); + } + } + } + } + /// Suggests to remove redundant argument(s): /// /// ```text diff --git a/src/test/ui/const-generics/invalid-const-arg-for-type-param.stderr b/src/test/ui/const-generics/invalid-const-arg-for-type-param.stderr index b1b359619dcfa..d955b4f9651da 100644 --- a/src/test/ui/const-generics/invalid-const-arg-for-type-param.stderr +++ b/src/test/ui/const-generics/invalid-const-arg-for-type-param.stderr @@ -2,15 +2,22 @@ error[E0107]: this associated function takes 0 generic arguments but 1 generic a --> $DIR/invalid-const-arg-for-type-param.rs:6:23 | LL | let _: u32 = 5i32.try_into::<32>().unwrap(); - | ^^^^^^^^------ help: remove these generics - | | - | expected 0 generic arguments + | ^^^^^^^^ expected 0 generic arguments | note: associated function defined here, with 0 generic parameters --> $SRC_DIR/core/src/convert/mod.rs:LL:COL | LL | fn try_into(self) -> Result; | ^^^^^^^^ +help: consider moving this generic argument to the `TryInto` trait, which takes up to 1 argument + | +LL | let _: u32 = TryInto::<32>::try_into(5i32).unwrap(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: remove these generics + | +LL - let _: u32 = 5i32.try_into::<32>().unwrap(); +LL + let _: u32 = 5i32.try_into().unwrap(); + | error[E0599]: no method named `f` found for struct `S` in the current scope --> $DIR/invalid-const-arg-for-type-param.rs:9:7 diff --git a/src/test/ui/suggestions/issue-89064.rs b/src/test/ui/suggestions/issue-89064.rs index 17a3e8b4cf831..fa5fc899dc083 100644 --- a/src/test/ui/suggestions/issue-89064.rs +++ b/src/test/ui/suggestions/issue-89064.rs @@ -27,4 +27,9 @@ fn main() { let _ = A::::foo::(); //~^ ERROR //~| HELP remove these generics + + let _ = 42.into::>(); + //~^ 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 846cd817505c3..8b2a388162806 100644 --- a/src/test/ui/suggestions/issue-89064.stderr +++ b/src/test/ui/suggestions/issue-89064.stderr @@ -9,15 +9,15 @@ note: associated function defined here, with 0 generic parameters | LL | fn foo() {} | ^^^ -help: remove these generics +help: consider moving this generic argument to the `A` trait, which takes up to 1 argument | LL - let _ = A::foo::(); -LL + let _ = A::foo(); +LL + let _ = A::::foo(); | -help: consider moving this generic argument to the `A` trait, which takes up to 1 argument +help: remove these generics | LL - let _ = A::foo::(); -LL + let _ = A::::foo(); +LL + let _ = A::foo(); | error[E0107]: this associated function takes 0 generic arguments but 2 generic arguments were supplied @@ -31,15 +31,15 @@ note: associated function defined here, with 0 generic parameters | LL | fn bar() {} | ^^^ -help: remove these generics +help: consider moving these generic arguments to the `B` trait, which takes up to 2 arguments | LL - let _ = B::bar::(); -LL + let _ = B::bar(); +LL + let _ = B::::bar(); | -help: consider moving these generic arguments to the `B` trait, which takes up to 2 arguments +help: remove these generics | LL - let _ = B::bar::(); -LL + let _ = B::::bar(); +LL + let _ = B::bar(); | error[E0107]: this associated function takes 0 generic arguments but 1 generic argument was supplied @@ -56,6 +56,27 @@ note: associated function defined here, with 0 generic parameters LL | fn foo() {} | ^^^ -error: aborting due to 3 previous errors +error[E0107]: this associated function takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/issue-89064.rs:31:16 + | +LL | let _ = 42.into::>(); + | ^^^^ expected 0 generic arguments + | +note: associated function defined here, with 0 generic parameters + --> $SRC_DIR/core/src/convert/mod.rs:LL:COL + | +LL | fn into(self) -> T; + | ^^^^ +help: consider moving this generic argument to the `Into` trait, which takes up to 1 argument + | +LL | let _ = Into::>::into(42); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ +help: remove these generics + | +LL - let _ = 42.into::>(); +LL + let _ = 42.into(); + | + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0107`.