From 0eac38b7a65c29734f4b2d34f35ee0aa9cb00a74 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sun, 13 Sep 2020 20:55:06 -0700 Subject: [PATCH 1/2] fix syntax error in suggesting generic constraint in trait parameter suggest `where T: Foo` for the first bound on a trait, then suggest `, T: Foo` when the suggested bound would add to an existing set of `where` clauses. `where T: Foo` may be the first bound if `T` has a default, because we'd rather suggest ``` trait A where T: Copy ``` than ``` trait A ``` for legibility reasons. --- compiler/rustc_middle/src/ty/diagnostics.rs | 66 +++++++++++++------ .../ui/trait-impl-bound-suggestions.fixed | 20 ++++++ src/test/ui/trait-impl-bound-suggestions.rs | 20 ++++++ .../ui/trait-impl-bound-suggestions.stderr | 17 +++++ src/test/ui/type/type-check-defaults.stderr | 4 +- 5 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/trait-impl-bound-suggestions.fixed create mode 100644 src/test/ui/trait-impl-bound-suggestions.rs create mode 100644 src/test/ui/trait-impl-bound-suggestions.stderr diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index bc51c8b6cd4..0416ef9e643 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -202,33 +202,59 @@ pub fn suggest_constraining_type_param( // Suggestion: // fn foo(t: T) where T: Foo, T: Bar {... } // - insert: `, T: Zar` + // + // Additionally, there may be no `where` clause whatsoever in the case that this was + // reached becauase the generic parameter has a default: + // + // Message: + // trait Foo {... } + // - help: consider further restricting this type parameter with `where T: Zar` + // + // Suggestion: + // trait Foo where T: Zar {... } + // - insert: `where T: Zar` - let mut param_spans = Vec::new(); + if matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) + && generics.where_clause.predicates.len() == 0 + { + // Suggest a bound, but there are no existing where clauses for this ``, so + // suggest adding one. + err.span_suggestion_verbose( + generics.where_clause.tail_span_for_suggestion(), + &msg_restrict_type_further, + format!(" where {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + let mut param_spans = Vec::new(); - for predicate in generics.where_clause.predicates { - if let WherePredicate::BoundPredicate(WhereBoundPredicate { - span, bounded_ty, .. - }) = predicate - { - if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind { - if let Some(segment) = path.segments.first() { - if segment.ident.to_string() == param_name { - param_spans.push(span); + for predicate in generics.where_clause.predicates { + if let WherePredicate::BoundPredicate(WhereBoundPredicate { + span, + bounded_ty, + .. + }) = predicate + { + if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind { + if let Some(segment) = path.segments.first() { + if segment.ident.to_string() == param_name { + param_spans.push(span); + } } } } } - } - match ¶m_spans[..] { - &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), - _ => { - err.span_suggestion_verbose( - generics.where_clause.tail_span_for_suggestion(), - &msg_restrict_type_further, - format!(", {}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); + match ¶m_spans[..] { + &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), + _ => { + err.span_suggestion_verbose( + generics.where_clause.tail_span_for_suggestion(), + &msg_restrict_type_further, + format!(", {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } } } diff --git a/src/test/ui/trait-impl-bound-suggestions.fixed b/src/test/ui/trait-impl-bound-suggestions.fixed new file mode 100644 index 00000000000..db3a95f5c4f --- /dev/null +++ b/src/test/ui/trait-impl-bound-suggestions.fixed @@ -0,0 +1,20 @@ +// run-rustfix + +#[allow(unused)] +use std::fmt::Debug; +// Rustfix should add this, or use `std::fmt::Debug` instead. + +#[allow(dead_code)] +struct ConstrainedStruct { + x: X +} + +#[allow(dead_code)] +trait InsufficientlyConstrainedGeneric where X: Copy { + fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct { + //~^ ERROR the trait bound `X: Copy` is not satisfied + ConstrainedStruct { x } + } +} + +pub fn main() { } diff --git a/src/test/ui/trait-impl-bound-suggestions.rs b/src/test/ui/trait-impl-bound-suggestions.rs new file mode 100644 index 00000000000..bf75175179e --- /dev/null +++ b/src/test/ui/trait-impl-bound-suggestions.rs @@ -0,0 +1,20 @@ +// run-rustfix + +#[allow(unused)] +use std::fmt::Debug; +// Rustfix should add this, or use `std::fmt::Debug` instead. + +#[allow(dead_code)] +struct ConstrainedStruct { + x: X +} + +#[allow(dead_code)] +trait InsufficientlyConstrainedGeneric { + fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct { + //~^ ERROR the trait bound `X: Copy` is not satisfied + ConstrainedStruct { x } + } +} + +pub fn main() { } diff --git a/src/test/ui/trait-impl-bound-suggestions.stderr b/src/test/ui/trait-impl-bound-suggestions.stderr new file mode 100644 index 00000000000..3a21e9c6b2a --- /dev/null +++ b/src/test/ui/trait-impl-bound-suggestions.stderr @@ -0,0 +1,17 @@ +error[E0277]: the trait bound `X: Copy` is not satisfied + --> $DIR/trait-impl-bound-suggestions.rs:14:52 + | +LL | struct ConstrainedStruct { + | ---- required by this bound in `ConstrainedStruct` +... +LL | fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct { + | ^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `X` + | +help: consider further restricting type parameter `X` + | +LL | trait InsufficientlyConstrainedGeneric where X: Copy { + | ^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/type/type-check-defaults.stderr b/src/test/ui/type/type-check-defaults.stderr index fa6f3422410..d8c7f595e62 100644 --- a/src/test/ui/type/type-check-defaults.stderr +++ b/src/test/ui/type/type-check-defaults.stderr @@ -56,8 +56,8 @@ LL | trait Base: Super { } | help: consider further restricting type parameter `T` | -LL | trait Base: Super, T: Copy { } - | ^^^^^^^^^ +LL | trait Base: Super where T: Copy { } + | ^^^^^^^^^^^^^ error[E0277]: cannot add `u8` to `i32` --> $DIR/type-check-defaults.rs:24:66 From e1607c87f0bb96c1c59d84a2789b7f7d2b69e182 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 14 Sep 2020 13:13:02 -0700 Subject: [PATCH 2/2] clean up comment text a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Esteban Küber --- compiler/rustc_middle/src/ty/diagnostics.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 0416ef9e643..715319747e3 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -204,7 +204,7 @@ pub fn suggest_constraining_type_param( // - insert: `, T: Zar` // // Additionally, there may be no `where` clause whatsoever in the case that this was - // reached becauase the generic parameter has a default: + // reached because the generic parameter has a default: // // Message: // trait Foo {... } @@ -217,8 +217,8 @@ pub fn suggest_constraining_type_param( if matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) && generics.where_clause.predicates.len() == 0 { - // Suggest a bound, but there are no existing where clauses for this ``, so - // suggest adding one. + // Suggest a bound, but there is no existing `where` clause *and* the type param has a + // default (``), so we suggest adding `where T: Bar`. err.span_suggestion_verbose( generics.where_clause.tail_span_for_suggestion(), &msg_restrict_type_further,