Skip to content

Commit

Permalink
Rollup merge of #76695 - iximeow:trait-generic-bound-suggestion, r=es…
Browse files Browse the repository at this point in the history
…tebank

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<T=()> where T: Copy
```
than
```
trait A<T: Copy=()>
```
for legibility reasons.

the test case i added here is derived from [this reproduction](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=0bf3ace9f2a183d0bdbd748c6b8e3971):
```
struct B<T: Copy> {
    t: T
}

trait A<T = ()> {
    fn returns_constrained_type(&self, t: T) -> B<T> {
        B { t }
    }
}
```
where the suggested fix,
```
trait A<T = ()>, T: Copy { ... }
```
is in fact invalid syntax!

i also found an error in the existing suggestion for `trait Base<T = String>: Super<T>` where rustc would suggest `trait Base<T = String>: Super<T>, T: Copy`, but `T: Copy` is the first of the trait's `where` clauses and should be `where T: Copy` as well. the test for that suggestion expects invalid syntax, and has been revised to a compiler-pleasing `trait Base<T = String>: Super<T> where T: Copy`.

judging by #70009 i'll.. cc @estebank ?
  • Loading branch information
Dylan-DPC committed Sep 16, 2020
2 parents 2c2f1c2 + e1607c8 commit 54d7728
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 22 deletions.
66 changes: 46 additions & 20 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,33 +202,59 @@ pub fn suggest_constraining_type_param(
// Suggestion:
// fn foo<T>(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 because the generic parameter has a default:
//
// Message:
// trait Foo<T=()> {... }
// - help: consider further restricting this type parameter with `where T: Zar`
//
// Suggestion:
// trait Foo<T=()> 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 is no existing `where` clause *and* the type param has a
// default (`<T=Foo>`), so we suggest adding `where T: Bar`.
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 &param_spans[..] {
&[&param_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 &param_spans[..] {
&[&param_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,
);
}
}
}

Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/trait-impl-bound-suggestions.fixed
Original file line number Diff line number Diff line change
@@ -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: Copy> {
x: X
}

#[allow(dead_code)]
trait InsufficientlyConstrainedGeneric<X=()> where X: Copy {
fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct<X> {
//~^ ERROR the trait bound `X: Copy` is not satisfied
ConstrainedStruct { x }
}
}

pub fn main() { }
20 changes: 20 additions & 0 deletions src/test/ui/trait-impl-bound-suggestions.rs
Original file line number Diff line number Diff line change
@@ -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: Copy> {
x: X
}

#[allow(dead_code)]
trait InsufficientlyConstrainedGeneric<X=()> {
fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct<X> {
//~^ ERROR the trait bound `X: Copy` is not satisfied
ConstrainedStruct { x }
}
}

pub fn main() { }
17 changes: 17 additions & 0 deletions src/test/ui/trait-impl-bound-suggestions.stderr
Original file line number Diff line number Diff line change
@@ -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<X: Copy> {
| ---- required by this bound in `ConstrainedStruct`
...
LL | fn return_the_constrained_type(&self, x: X) -> ConstrainedStruct<X> {
| ^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `X`
|
help: consider further restricting type parameter `X`
|
LL | trait InsufficientlyConstrainedGeneric<X=()> where X: Copy {
| ^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
4 changes: 2 additions & 2 deletions src/test/ui/type/type-check-defaults.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ LL | trait Base<T = String>: Super<T> { }
|
help: consider further restricting type parameter `T`