Skip to content

Commit

Permalink
Deduplicate type param constraint suggestion code
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Nov 28, 2019
1 parent 02bc412 commit 9fb446d
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 154 deletions.
77 changes: 75 additions & 2 deletions src/librustc/hir/mod.rs
Expand Up @@ -16,9 +16,9 @@ use crate::ty::AdtKind;
use crate::ty::query::Providers;
use crate::util::nodemap::{NodeMap, FxHashSet};

use errors::FatalError;
use errors::{Applicability, DiagnosticBuilder, FatalError};
use syntax_pos::{Span, DUMMY_SP, MultiSpan};
use syntax::source_map::Spanned;
use syntax::source_map::{Spanned, SourceMap};
use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, AsmDialect};
use syntax::ast::{Attribute, Label, LitKind, StrStyle, FloatTy, IntTy, UintTy};
pub use syntax::ast::{Mutability, Constness, Unsafety, Movability, CaptureBy};
Expand Down Expand Up @@ -644,6 +644,79 @@ impl Generics {
self.params.iter().map(|p| p.span).collect::<Vec<Span>>().into()
}
}

/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_param(
&self,
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
source_map: &SourceMap,
span: Span,
) -> bool {
let restrict_msg = "consider further restricting this bound";
if let Some(param) = self.params.iter().filter(|p| {
p.name.ident().as_str() == param_name
}).next() {
if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
param.span,
restrict_msg,
// `impl CurrentTrait + MissingTrait`
format!("{} + {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else if self.where_clause.predicates.is_empty() &&
param.bounds.is_empty()
{
// If there are no bounds whatsoever, suggest adding a constraint
// to the type parameter:
// `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
err.span_suggestion(
param.span,
"consider restricting this bound",
format!("{}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else if !self.where_clause.predicates.is_empty() {
// There is a `where` clause, so suggest expanding it:
// `fn foo<T>(t: T) where T: Debug {}` →
// `fn foo<T>(t: T) where T: Debug, T: Trait {}`
err.span_suggestion(
self.where_clause.span().unwrap().shrink_to_hi(),
&format!("consider further restricting type parameter `{}`", param_name),
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else {
// If there is no `where` clause lean towards constraining to the
// type parameter:
// `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
// `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
let sp = param.span.with_hi(span.hi());
let span = source_map.span_through_char(sp, ':');
if sp != param.span && sp != span {
// Only suggest if we have high certainty that the span
// covers the colon in `foo<T: Trait>`.
err.span_suggestion(
span,
restrict_msg,
format!("{}: {} + ", param_name, constraint),
Applicability::MachineApplicable,
);
} else {
err.span_label(
param.span,
&format!("consider adding a `where {}: {}` bound", param_name, constraint),
);
}
}
return true;
}
false
}
}

/// Synthetic type parameters are converted to another form during lowering; this allows
Expand Down
81 changes: 16 additions & 65 deletions src/librustc/traits/error_reporting.rs
Expand Up @@ -1091,7 +1091,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
fn suggest_restricting_param_bound(
&self,
err: &mut DiagnosticBuilder<'_>,
mut err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::PolyTraitRef<'_>,
body_id: hir::HirId,
) {
Expand All @@ -1102,7 +1102,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
_ => return,
};

let mut suggest_restriction = |generics: &hir::Generics, msg| {
let suggest_restriction = |
generics: &hir::Generics,
msg,
err: &mut DiagnosticBuilder<'_>,
| {
let span = generics.where_clause.span_for_predicates_or_empty_place();
if !span.from_expansion() && span.desugaring_kind().is_none() {
err.span_suggestion(
Expand Down Expand Up @@ -1132,7 +1136,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
kind: hir::TraitItemKind::Method(..), ..
}) if param_ty && self_ty == self.tcx.types.self_param => {
// Restricting `Self` for a single method.
suggest_restriction(&generics, "`Self`");
suggest_restriction(&generics, "`Self`", err);
return;
}

Expand All @@ -1154,7 +1158,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
kind: hir::ItemKind::Impl(_, _, _, generics, ..), ..
}) if projection.is_some() => {
// Missing associated type bound.
suggest_restriction(&generics, "the associated type");
suggest_restriction(&generics, "the associated type", err);
return;
}

Expand Down Expand Up @@ -1183,68 +1187,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
if param_ty => {
// Missing generic type parameter bound.
let restrict_msg = "consider further restricting this bound";
let param_name = self_ty.to_string();
for param in generics.params.iter().filter(|p| {
p.name.ident().as_str() == param_name
}) {
if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
param.span,
restrict_msg,
// `impl CurrentTrait + MissingTrait`
format!("{} + {}", param.name.ident(), trait_ref),
Applicability::MachineApplicable,
);
} else if generics.where_clause.predicates.is_empty() &&
param.bounds.is_empty()
{
// If there are no bounds whatsoever, suggest adding a constraint
// to the type parameter:
// `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
err.span_suggestion(
param.span,
"consider restricting this bound",
format!("{}", trait_ref.to_predicate()),
Applicability::MachineApplicable,
);
} else if !generics.where_clause.predicates.is_empty() {
// There is a `where` clause, so suggest expanding it:
// `fn foo<T>(t: T) where T: Debug {}` →
// `fn foo<T>(t: T) where T: Debug, T: Trait {}`
err.span_suggestion(
generics.where_clause.span().unwrap().shrink_to_hi(),
&format!(
"consider further restricting type parameter `{}`",
param_name,
),
format!(", {}", trait_ref.to_predicate()),
Applicability::MachineApplicable,
);
} else {
// If there is no `where` clause lean towards constraining to the
// type parameter:
// `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
// `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
let sp = param.span.with_hi(span.hi());
let span = self.tcx.sess.source_map()
.span_through_char(sp, ':');
if sp != param.span && sp != span {
// Only suggest if we have high certainty that the span
// covers the colon in `foo<T: Trait>`.
err.span_suggestion(span, restrict_msg, format!(
"{} + ",
trait_ref.to_predicate(),
), Applicability::MachineApplicable);
} else {
err.span_label(param.span, &format!(
"consider adding a `where {}` bound",
trait_ref.to_predicate(),
));
}
}
let constraint = trait_ref.to_string();
if generics.suggest_constraining_type_param(
&mut err,
&param_name,
&constraint,
self.tcx.sess.source_map(),
*span,
) {
return;
}
}
Expand Down
64 changes: 7 additions & 57 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Expand Up @@ -233,63 +233,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let generics = tcx.generics_of(self.mir_def_id);
let param = generics.type_param(&param_ty, tcx);
let generics = tcx.hir().get_generics(self.mir_def_id).unwrap();
let msg = "consider adding a `Copy` constraint to this type argument";
for param in generics.params.iter().filter(|p| {
p.name.ident().as_str() == param.name.as_str()
}) {
let param_name = param.name.ident().as_str();
if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
param.span,
msg,
// `impl CurrentTrait + MissingTrait`
format!("{} + Copy", param_name),
Applicability::MachineApplicable,
);
} else if generics.where_clause.predicates.is_empty() &&
param.bounds.is_empty()
{
// If there are no bounds whatsoever, suggest adding a constraint
// to the type parameter:
// `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
err.span_suggestion(
param.span,
msg,
format!("{}: Copy", param_name),
Applicability::MachineApplicable,
);
} else if !generics.where_clause.predicates.is_empty() {
// There is a `where` clause, so suggest expanding it:
// `fn foo<T>(t: T) where T: Debug {}` →
// `fn foo<T>(t: T) where T: Debug, T: Trait {}`
err.span_suggestion(
generics.where_clause.span().unwrap().shrink_to_hi(),
msg,
format!(", {}: Copy", param_name),
Applicability::MachineApplicable,
);
} else {
// If there is no `where` clause lean towards constraining to the
// type parameter:
// `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
// `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
let sp = param.span.with_hi(span.hi());
let span = tcx.sess.source_map()
.span_through_char(sp, ':');
if sp != param.span && sp != span {
// Only suggest if we have high certainty that the span
// covers the colon in `foo<T: Trait>`.
err.span_suggestion(span, msg, format!(
"{}: Copy +",
param_name,
), Applicability::MachineApplicable);
} else {
err.span_label(param.span, msg);
}
}
}
generics.suggest_constraining_type_param(
&mut err,
&param.name.as_str(),
"Copy",
tcx.sess.source_map(),
span,
);
}
let span = if let Some(local) = place.as_local() {
let decl = &self.body.local_decls[local];
Expand Down

0 comments on commit 9fb446d

Please sign in to comment.