Skip to content

Commit

Permalink
Tweak suggest_constraining_type_param
Browse files Browse the repository at this point in the history 
Some of the bound restriction structured suggestions were incorrect
while others had subpar output.
  • Loading branch information
@estebank
estebank committed Mar 29, 2020
1 parent 285519d commit 2c71894
Show file tree
Hide file tree
Showing 70 changed files with 439 additions and 640 deletions.
2 changes: 0 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&mut err,
&param.name.as_str(),
"Copy",
tcx.sess.source_map(),
span,
None,
);
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_trait_selection/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#![feature(drain_filter)]
#![feature(in_band_lifetimes)]
#![feature(crate_visibility_modifier)]
#![feature(or_patterns)]
#![recursion_limit = "512"] // For rustdoc

#[macro_use]
Expand Down
130 changes: 40 additions & 90 deletions src/librustc_trait_selection/traits/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate};
use rustc_session::DiagnosticMessageId;
use rustc_span::source_map::SourceMap;
use rustc_span::{ExpnKind, Span, DUMMY_SP};
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
use std::fmt;

use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
Expand Down Expand Up @@ -1679,14 +1678,8 @@ pub fn suggest_constraining_type_param(
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
source_map: &SourceMap,
span: Span,
def_id: Option<DefId>,
) -> bool {
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound with";
const MSG_RESTRICT_TYPE: &str = "consider restricting this type parameter with";
const MSG_RESTRICT_TYPE_FURTHER: &str = "consider further restricting this type parameter with";

let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);

let param = if let Some(param) = param {
Expand All @@ -1695,11 +1688,24 @@ pub fn suggest_constraining_type_param(
return false;
};

const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
let msg_restrict_type_further =
format!("consider further restricting type parameter `{}`", param_name);

if def_id == tcx.lang_items().sized_trait() {
// Type parameters are already `Sized` by default.
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
return true;
}
let mut suggest_restrict = |span| {
err.span_suggestion_verbose(
span,
MSG_RESTRICT_BOUND_FURTHER,
format!(" + {}", constraint),
Applicability::MachineApplicable,
);
};

if param_name.starts_with("impl ") {
// If there's an `impl Trait` used in argument position, suggest
Expand All @@ -1717,19 +1723,15 @@ pub fn suggest_constraining_type_param(
// |
// replace with: `impl Foo + Bar`

err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));

err.tool_only_span_suggestion(
param.span,
MSG_RESTRICT_BOUND_FURTHER,
format!("{} + {}", param_name, constraint),
Applicability::MachineApplicable,
);

suggest_restrict(param.span.shrink_to_hi());
return true;
}

if generics.where_clause.predicates.is_empty() {
if generics.where_clause.predicates.is_empty()
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`.
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
{
if let Some(bounds_span) = param.bounds_span() {
// If user has provided some bounds, suggest restricting them:
//
Expand All @@ -1744,38 +1746,16 @@ pub fn suggest_constraining_type_param(
// --
// |
// replace with: `T: Bar +`

err.span_help(
bounds_span,
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
);

let span_hi = param.span.with_hi(span.hi());
let span_with_colon = source_map.span_through_char(span_hi, ':');

if span_hi != param.span && span_with_colon != span_hi {
err.tool_only_span_suggestion(
span_with_colon,
MSG_RESTRICT_BOUND_FURTHER,
format!("{}: {} + ", param_name, constraint),
Applicability::MachineApplicable,
);
}
suggest_restrict(bounds_span.shrink_to_hi());
} else {
// If user hasn't provided any bounds, suggest adding a new one:
//
// fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo`

err.span_help(
param.span,
&format!("{} `{}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
);

err.tool_only_span_suggestion(
param.span,
MSG_RESTRICT_TYPE,
format!("{}: {}", param_name, constraint),
err.span_suggestion_verbose(
param.span.shrink_to_hi(),
&msg_restrict_type,
format!(": {}", constraint),
Applicability::MachineApplicable,
);
}
Expand Down Expand Up @@ -1839,55 +1819,25 @@ pub fn suggest_constraining_type_param(
}
}

let where_clause_span =
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place();
// Account for `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
let mut trailing_comma = false;
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) {
trailing_comma = snippet.ends_with(",");
}
let where_clause_span = if trailing_comma {
let hi = where_clause_span.hi();
Span::new(hi - BytePos(1), hi, where_clause_span.ctxt())
} else {
where_clause_span.shrink_to_hi()
};

match &param_spans[..] {
&[] => {
err.span_help(
param.span,
&format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
);

err.tool_only_span_suggestion(
where_clause_span,
MSG_RESTRICT_TYPE,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
}

&[&param_span] => {
err.span_help(
param_span,
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
);

let span_hi = param_span.with_hi(span.hi());
let span_with_colon = source_map.span_through_char(span_hi, ':');

if span_hi != param_span && span_with_colon != span_hi {
err.tool_only_span_suggestion(
span_with_colon,
MSG_RESTRICT_BOUND_FURTHER,
format!("{}: {} +", param_name, constraint),
Applicability::MachineApplicable,
);
}
}

&[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
_ => {
err.span_help(
param.span,
&format!(
"{} `where {}: {}`",
MSG_RESTRICT_TYPE_FURTHER, param_name, constraint,
),
);

err.tool_only_span_suggestion(
err.span_suggestion_verbose(
where_clause_span,
MSG_RESTRICT_BOUND_FURTHER,
&msg_restrict_type_further,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
Expand Down
77 changes: 21 additions & 56 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return;
}

hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. })
| hir::Node::TraitItem(hir::TraitItem {
hir::Node::TraitItem(hir::TraitItem {
generics,
kind: hir::TraitItemKind::Fn(..),
..
Expand All @@ -206,63 +205,31 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
kind: hir::ImplItemKind::Fn(..),
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, _, _),
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl { generics, .. }, ..
}) if projection.is_some() => {
| hir::Node::Item(
hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. }
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. },
) if projection.is_some() => {
// Missing associated type bound.
suggest_restriction(&generics, "the associated type", err);
return;
}

hir::Node::Item(hir::Item {
kind: hir::ItemKind::Struct(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Enum(_, generics), span, ..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Union(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, ..),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl { generics, .. },
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, generics, _),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::TyAlias(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::TraitAlias(generics, _),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }),
span,
..
})
| hir::Node::TraitItem(hir::TraitItem { generics, span, .. })
| hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
hir::Node::Item(
hir::Item { kind: hir::ItemKind::Struct(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Enum(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Union(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, ..), .. }
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. }
| hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
| hir::Item { kind: hir::ItemKind::TyAlias(_, generics), .. }
| hir::Item { kind: hir::ItemKind::TraitAlias(generics, _), .. }
| hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }), ..
},
)
| hir::Node::TraitItem(hir::TraitItem { generics, .. })
| hir::Node::ImplItem(hir::ImplItem { generics, .. })
if param_ty =>
{
// Missing generic type parameter bound.
Expand All @@ -274,8 +241,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&mut err,
&param_name,
&constraint,
self.tcx.sess.source_map(),
*span,
Some(trait_ref.def_id()),
) {
return;
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-const/associated-const-type-parameter-arrays-2.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ LL | const Y: usize;
LL | let _array = [4; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
help: consider further restricting this bound with `+ Foo`
--> $DIR/associated-const-type-parameter-arrays-2.rs:15:16
help: consider further restricting this bound
|
LL | pub fn test<A: Foo, B: Foo>() {
| ^^^
LL | pub fn test<A: Foo + Foo, B: Foo>() {
| ^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-const/associated-const-type-parameter-arrays.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ LL | const Y: usize;
LL | let _array: [u32; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
help: consider further restricting this bound with `+ Foo`
--> $DIR/associated-const-type-parameter-arrays.rs:15:16
help: consider further restricting this bound
|
LL | pub fn test<A: Foo, B: Foo>() {
| ^^^
LL | pub fn test<A: Foo + Foo, B: Foo>() {
| ^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-types/associated-types-invalid-trait-ref-issue-18865.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Foo<usize>` is not satisfied
LL | let u: <T as Foo<usize>>::Bar = t.get_bar();
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo<usize>` is not implemented for `T`
|
help: consider further restricting this bound with `+ Foo<usize>`
--> $DIR/associated-types-invalid-trait-ref-issue-18865.rs:9:8
help: consider further restricting this bound
|
LL | fn f<T:Foo<isize>>(t: &T) {
| ^^^^^^^^^^
LL | fn f<T:Foo<isize> + Foo<usize>>(t: &T) {
| ^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-types/associated-types-no-suitable-bound.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Get` is not satisfied
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
|
help: consider restricting this type parameter with `T: Get`
--> $DIR/associated-types-no-suitable-bound.rs:11:13
help: consider restricting type parameter `T`
|
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}
| ^^^^^

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 2c71894

Please sign in to comment.