Skip to content

Provide structured suggestions when finding structs when expecting a trait #77087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1884,6 +1884,16 @@ impl Clone for Ty {
}
}

impl Ty {
pub fn peel_refs(&self) -> &Self {
let mut final_ty = self;
while let TyKind::Rptr(_, MutTy { ty, .. }) = &final_ty.kind {
final_ty = &ty;
}
final_ty
}
}

#[derive(Clone, Encodable, Decodable, Debug)]
pub struct BareFnTy {
pub unsafety: Unsafe,
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,13 @@ struct DiagnosticMetadata<'ast> {

/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
in_if_condition: Option<&'ast Expr>,

/// If we are currently in a trait object definition. Used to point at the bounds when
/// encountering a struct or enum.
current_trait_object: Option<&'ast [ast::GenericBound]>,

/// Given `where <T as Bar>::Baz: String`, suggest `where T: Bar<Baz = String>`.
current_where_predicate: Option<&'ast WherePredicate>,
}

struct LateResolutionVisitor<'a, 'b, 'ast> {
Expand Down Expand Up @@ -453,6 +460,7 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
self.diagnostic_metadata.current_let_binding = original;
}
fn visit_ty(&mut self, ty: &'ast Ty) {
let prev = self.diagnostic_metadata.current_trait_object;
match ty.kind {
TyKind::Path(ref qself, ref path) => {
self.smart_resolve_path(ty.id, qself.as_ref(), path, PathSource::Type);
Expand All @@ -464,9 +472,13 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
.map_or(Res::Err, |d| d.res());
self.r.record_partial_res(ty.id, PartialRes::new(res));
}
TyKind::TraitObject(ref bounds, ..) => {
self.diagnostic_metadata.current_trait_object = Some(&bounds[..]);
}
_ => (),
}
visit::walk_ty(self, ty);
self.diagnostic_metadata.current_trait_object = prev;
}
fn visit_poly_trait_ref(&mut self, tref: &'ast PolyTraitRef, m: &'ast TraitBoundModifier) {
self.smart_resolve_path(
Expand Down Expand Up @@ -660,6 +672,14 @@ impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
}
self.diagnostic_metadata.currently_processing_generics = prev;
}

fn visit_where_predicate(&mut self, p: &'ast WherePredicate) {
debug!("visit_where_predicate {:?}", p);
let previous_value =
replace(&mut self.diagnostic_metadata.current_where_predicate, Some(p));
visit::walk_where_predicate(self, p);
self.diagnostic_metadata.current_where_predicate = previous_value;
}
}

impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
Expand Down
219 changes: 203 additions & 16 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use crate::diagnostics::{ImportSuggestion, LabelSuggestion, TypoSuggestion};
use crate::late::lifetimes::{ElisionFailureInfo, LifetimeContext};
use crate::late::{LateResolutionVisitor, RibKind};
use crate::late::{AliasPossibility, LateResolutionVisitor, RibKind};
use crate::path_names_to_string;
use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{PathResult, PathSource, Segment};

use rustc_ast::util::lev_distance::find_best_match_for_name;
use rustc_ast::visit::FnKind;
use rustc_ast::{self as ast, Expr, ExprKind, Item, ItemKind, NodeId, Path, Ty, TyKind};
use rustc_ast_pretty::pprust::path_segment_to_string;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
Expand All @@ -19,7 +20,7 @@ use rustc_session::config::nightly_options;
use rustc_session::parse::feature_err;
use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, Span, DUMMY_SP};
use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};

use tracing::debug;

Expand Down Expand Up @@ -439,27 +440,213 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
}
}

if !self.type_ascription_suggestion(&mut err, base_span)
&& !self.r.add_typo_suggestion(&mut err, typo_sugg, ident_span)
{
// Fallback label.
err.span_label(base_span, fallback_label);

match self.diagnostic_metadata.current_let_binding {
Some((pat_sp, Some(ty_sp), None)) if ty_sp.contains(base_span) && could_be_expr => {
err.span_suggestion_short(
pat_sp.between(ty_sp),
"use `=` if you meant to assign",
" = ".to_string(),
Applicability::MaybeIncorrect,
if !self.type_ascription_suggestion(&mut err, base_span) {
let mut fallback = false;
if let (
PathSource::Trait(AliasPossibility::Maybe),
Some(Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, _)),
) = (source, res)
{
if let Some(bounds @ [_, .., _]) = self.diagnostic_metadata.current_trait_object {
fallback = true;
let spans: Vec<Span> = bounds
.iter()
.map(|bound| bound.span())
.filter(|&sp| sp != base_span)
.collect();

let start_span = bounds.iter().map(|bound| bound.span()).next().unwrap();
// `end_span` is the end of the poly trait ref (Foo + 'baz + Bar><)
let end_span = bounds.iter().map(|bound| bound.span()).last().unwrap();
// `last_bound_span` is the last bound of the poly trait ref (Foo + >'baz< + Bar)
let last_bound_span = spans.last().cloned().unwrap();
let mut multi_span: MultiSpan = spans.clone().into();
for sp in spans {
let msg = if sp == last_bound_span {
format!(
"...because of {} bound{}",
if bounds.len() <= 2 { "this" } else { "these" },
if bounds.len() <= 2 { "" } else { "s" },
)
} else {
String::new()
};
multi_span.push_span_label(sp, msg);
}
multi_span.push_span_label(
base_span,
"expected this type to be a trait...".to_string(),
);
err.span_help(
multi_span,
"`+` is used to constrain a \"trait object\" type with lifetimes or \
auto-traits; structs and enums can't be bound in that way",
);
if bounds.iter().all(|bound| match bound {
ast::GenericBound::Outlives(_) => true,
ast::GenericBound::Trait(tr, _) => tr.span == base_span,
}) {
let mut sugg = vec![];
if base_span != start_span {
sugg.push((start_span.until(base_span), String::new()));
}
if base_span != end_span {
sugg.push((base_span.shrink_to_hi().to(end_span), String::new()));
}

err.multipart_suggestion(
"if you meant to use a type and not a trait here, remove the bounds",
sugg,
Applicability::MaybeIncorrect,
);
}
}
_ => {}
}

fallback |= self.restrict_assoc_type_in_where_clause(span, &mut err);

if !self.r.add_typo_suggestion(&mut err, typo_sugg, ident_span) {
fallback = true;
match self.diagnostic_metadata.current_let_binding {
Some((pat_sp, Some(ty_sp), None))
if ty_sp.contains(base_span) && could_be_expr =>
{
err.span_suggestion_short(
pat_sp.between(ty_sp),
"use `=` if you meant to assign",
" = ".to_string(),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
if fallback {
// Fallback label.
err.span_label(base_span, fallback_label);
}
}
(err, candidates)
}

/// Given `where <T as Bar>::Baz: String`, suggest `where T: Bar<Baz = String>`.
fn restrict_assoc_type_in_where_clause(
&mut self,
span: Span,
err: &mut DiagnosticBuilder<'_>,
) -> bool {
// Detect that we are actually in a `where` predicate.
let (bounded_ty, bounds, where_span) =
if let Some(ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate {
bounded_ty,
bound_generic_params,
bounds,
span,
})) = self.diagnostic_metadata.current_where_predicate
{
if !bound_generic_params.is_empty() {
return false;
}
(bounded_ty, bounds, span)
} else {
return false;
};

// Confirm that the target is an associated type.
let (ty, position, path) = if let ast::TyKind::Path(
Some(ast::QSelf { ty, position, .. }),
path,
) = &bounded_ty.kind
{
// use this to verify that ident is a type param.
let partial_res = if let Ok(Some(partial_res)) = self.resolve_qpath_anywhere(
bounded_ty.id,
None,
&Segment::from_path(path),
Namespace::TypeNS,
span,
true,
CrateLint::No,
) {
partial_res
} else {
return false;
};
if !(matches!(
partial_res.base_res(),
hir::def::Res::Def(hir::def::DefKind::AssocTy, _)
Copy link
Contributor

@matthewjasper matthewjasper Sep 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? This suggestion would also make sense for <&T as Trait>::Assoc: String

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might have meant to point at the next matches check, as this one just verifies that we are trying to constrain an associated type. The last commit now accounts for references. I have these checks because I want to avoid incorrect suggestions, as that is more harmful than not providing a suggestion we could have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, this will not help for a case like <(T, T) as Trait>::Assoc: String.

) && partial_res.unresolved_segments() == 0)
{
return false;
}
(ty, position, path)
} else {
return false;
};

if let ast::TyKind::Path(None, type_param_path) = &ty.peel_refs().kind {
// Confirm that the `SelfTy` is a type parameter.
let partial_res = if let Ok(Some(partial_res)) = self.resolve_qpath_anywhere(
bounded_ty.id,
None,
&Segment::from_path(type_param_path),
Namespace::TypeNS,
span,
true,
CrateLint::No,
) {
partial_res
} else {
return false;
};
if !(matches!(
partial_res.base_res(),
hir::def::Res::Def(hir::def::DefKind::TyParam, _)
) && partial_res.unresolved_segments() == 0)
{
return false;
}
if let (
[ast::PathSegment { ident: constrain_ident, args: None, .. }],
[ast::GenericBound::Trait(poly_trait_ref, ast::TraitBoundModifier::None)],
) = (&type_param_path.segments[..], &bounds[..])
{
if let [ast::PathSegment { ident, args: None, .. }] =
&poly_trait_ref.trait_ref.path.segments[..]
{
if ident.span == span {
err.span_suggestion_verbose(
*where_span,
&format!("constrain the associated type to `{}`", ident),
format!(
"{}: {}<{} = {}>",
self.r
.session
.source_map()
.span_to_snippet(ty.span) // Account for `<&'a T as Foo>::Bar`.
.unwrap_or_else(|_| constrain_ident.to_string()),
path.segments[..*position]
.iter()
.map(|segment| path_segment_to_string(segment))
.collect::<Vec<_>>()
.join("::"),
path.segments[*position..]
.iter()
.map(|segment| path_segment_to_string(segment))
.collect::<Vec<_>>()
.join("::"),
ident,
),
Applicability::MaybeIncorrect,
);
}
return true;
}
}
}
false
}

/// Check if the source is call expression and the first argument is `self`. If true,
/// return the span of whole call and the span for all arguments expect the first one (`self`).
fn call_has_self_arg(&self, source: PathSource<'_>) -> Option<(Span, Option<Span>)> {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/traits/assoc_type_bound_with_struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
trait Bar {
type Baz;
}

struct Foo<T> where T: Bar, <T as Bar>::Baz: String { //~ ERROR expected trait, found struct
t: T,
}

struct Qux<'a, T> where T: Bar, <&'a T as Bar>::Baz: String { //~ ERROR expected trait, found struct
t: &'a T,
}

fn foo<T: Bar>(_: T) where <T as Bar>::Baz: String { //~ ERROR expected trait, found struct
}

fn qux<'a, T: Bar>(_: &'a T) where <&'a T as Bar>::Baz: String { //~ ERROR expected trait, found
}

fn main() {}
Loading