Skip to content

Commit

Permalink
Suggest restricting type param when it doesn't satisfy projection
Browse files Browse the repository at this point in the history 
When encountering a projection that isn't satisfied by a type parameter,
suggest constraining the type parameter.
  • Loading branch information
@estebank
estebank committed May 4, 2020
1 parent 75f066d commit 1473a66
Show file tree
Hide file tree
Showing 11 changed files with 358 additions and 194 deletions.
184 changes: 183 additions & 1 deletion src/librustc_middle/ty/diagnostics.rs
View file
Expand Up @@ -2,7 +2,12 @@

use crate::ty::sty::InferTy;
use crate::ty::TyKind::*;
use crate::ty::TyS;
use crate::ty::{TyCtxt, TyS};
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::{QPath, TyKind, WhereBoundPredicate, WherePredicate};
use rustc_span::{BytePos, Span};

impl<'tcx> TyS<'tcx> {
/// Similar to `TyS::is_primitive`, but also considers inferred numeric values to be primitive.
Expand Down Expand Up @@ -67,3 +72,180 @@ impl<'tcx> TyS<'tcx> {
}
}
}

/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_param(
tcx: TyCtxt<'_>,
generics: &hir::Generics<'_>,
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
def_id: Option<DefId>,
) -> bool {
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);

let param = if let Some(param) = param {
param
} else {
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
// restricting it:
//
// fn foo(t: impl Foo) { ... }
// --------
// |
// help: consider further restricting this bound with `+ Bar`
//
// Suggestion for tools in this case is:
//
// fn foo(t: impl Foo) { ... }
// --------
// |
// replace with: `impl Foo + Bar`

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

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:
//
// fn foo<T: Foo>(t: T) { ... }
// ---
// |
// help: consider further restricting this bound with `+ Bar`
//
// Suggestion for tools in this case is:
//
// fn foo<T: Foo>(t: T) { ... }
// --
// |
// replace with: `T: Bar +`
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_suggestion_verbose(
param.span.shrink_to_hi(),
&msg_restrict_type,
format!(": {}", constraint),
Applicability::MachineApplicable,
);
}

true
} else {
// This part is a bit tricky, because using the `where` clause user can
// provide zero, one or many bounds for the same type parameter, so we
// have following cases to consider:
//
// 1) When the type parameter has been provided zero bounds
//
// Message:
// fn foo<X, Y>(x: X, y: Y) where Y: Foo { ... }
// - help: consider restricting this type parameter with `where X: Bar`
//
// Suggestion:
// fn foo<X, Y>(x: X, y: Y) where Y: Foo { ... }
// - insert: `, X: Bar`
//
//
// 2) When the type parameter has been provided one bound
//
// Message:
// fn foo<T>(t: T) where T: Foo { ... }
// ^^^^^^
// |
// help: consider further restricting this bound with `+ Bar`
//
// Suggestion:
// fn foo<T>(t: T) where T: Foo { ... }
// ^^
// |
// replace with: `T: Bar +`
//
//
// 3) When the type parameter has been provided many bounds
//
// Message:
// fn foo<T>(t: T) where T: Foo, T: Bar {... }
// - help: consider further restricting this type parameter with `where T: Zar`
//
// Suggestion:
// fn foo<T>(t: T) where T: Foo, T: Bar {... }
// - insert: `, T: Zar`

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);
}
}
}
}
}

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[..] {
&[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
_ => {
err.span_suggestion_verbose(
where_clause_span,
&msg_restrict_type_further,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
}
}

true
}
}
40 changes: 38 additions & 2 deletions src/librustc_middle/ty/error.rs
View file
@@ -1,4 +1,5 @@
use crate::traits::{ObligationCause, ObligationCauseCode};
use crate::ty::diagnostics::suggest_constraining_type_param;
use crate::ty::{self, BoundRegion, Region, Ty, TyCtxt};
use rustc_ast::ast;
use rustc_errors::Applicability::{MachineApplicable, MaybeIncorrect};
Expand Down Expand Up @@ -401,8 +402,43 @@ impl<'tcx> TyCtxt<'tcx> {
(ty::Projection(_), ty::Projection(_)) => {
db.note("an associated type was expected, but a different one was found");
}
(ty::Param(_), ty::Projection(_)) | (ty::Projection(_), ty::Param(_)) => {
db.note("you might be missing a type parameter or trait bound");
(ty::Param(p), ty::Projection(proj)) | (ty::Projection(proj), ty::Param(p)) => {
let generics = self.generics_of(body_owner_def_id);
let p_span = self.def_span(generics.type_param(p, self).def_id);
if !sp.contains(p_span) {
db.span_label(p_span, "this type parameter");
}
let hir = self.hir();
let mut note = true;
if let Some(generics) = hir
.as_local_hir_id(generics.type_param(p, self).def_id)
.and_then(|id| self.hir().find(self.hir().get_parent_node(id)))
.as_ref()
.and_then(|node| node.generics())
{
// Synthesize the associated type restriction `Add<Output = Expected>`.
// FIXME: extract this logic for use in other diagnostics.
let trait_ref = proj.trait_ref(self);
let path =
self.def_path_str_with_substs(trait_ref.def_id, trait_ref.substs);
let item_name = self.item_name(proj.item_def_id);
let path = if path.ends_with('>') {
format!("{}, {} = {}>", &path[..path.len() - 1], item_name, p)
} else {
format!("{}<{} = {}>", path, item_name, p)
};
note = !suggest_constraining_type_param(
self,
generics,
db,
&format!("{}", proj.self_ty()),
&path,
None,
);
}
if note {
db.note("you might be missing a type parameter or trait bound");
}
}
(ty::Param(p), ty::Dynamic(..) | ty::Opaque(..))
| (ty::Dynamic(..) | ty::Opaque(..), ty::Param(p)) => {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
View file
Expand Up @@ -10,10 +10,9 @@ use rustc_middle::mir::{
FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef,
ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm,
};
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty};
use rustc_span::source_map::DesugaringKind;
use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::suggest_constraining_type_param;

use crate::dataflow::drop_flag_effects;
use crate::dataflow::indexes::{MoveOutIndex, MovePathIndex};
Expand Down

0 comments on commit 1473a66

Please sign in to comment.