Skip to content

Commit 3c872e2

Browse files
estebankestebank
committed
review comments: move logic to their own methods
1 parent de3b4d4 commit 3c872e2

File tree

1 file changed

+139
-137
lines changed

1 file changed

+139
-137
lines changed

‎src/librustc_middle/ty/error.rs

+139-137
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_errors::Applicability::{MachineApplicable, MaybeIncorrect};
55
use rustc_errors::{pluralize, DiagnosticBuilder};
66
use rustc_hir as hir;
77
use rustc_hir::def_id::DefId;
8-
use rustc_span::symbol::sym;
8+
use rustc_span::symbol::{sym, Symbol};
99
use rustc_span::{BytePos, MultiSpan, Span};
1010
use rustc_target::spec::abi;
1111

@@ -616,26 +616,11 @@ impl<T> Trait<T> for X {
616616
// We don't want to suggest calling an assoc fn in a scope where that isn't feasible.
617617
let callable_scope = match body_owner {
618618
Some(
619-
hir::Node::Item(hir::Item {
620-
kind:
621-
hir::ItemKind::Trait(..)
622-
| hir::ItemKind::Impl { .. }
623-
| hir::ItemKind::Const(..)
624-
| hir::ItemKind::Enum(..)
625-
| hir::ItemKind::Struct(..)
626-
| hir::ItemKind::Union(..),
627-
..
628-
})
629-
| hir::Node::TraitItem(hir::TraitItem {
630-
kind: hir::TraitItemKind::Const(..) | hir::TraitItemKind::Type(..),
631-
..
632-
})
633-
| hir::Node::ImplItem(hir::ImplItem {
634-
kind: hir::ImplItemKind::Const(..) | hir::ImplItemKind::TyAlias(..),
635-
..
636-
}),
637-
) => false,
638-
_ => true,
619+
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(..), .. })
620+
| hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. })
621+
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }),
622+
) => true,
623+
_ => false,
639624
};
640625
let impl_comparison = matches!(
641626
cause_code,
@@ -649,112 +634,20 @@ impl<T> Trait<T> for X {
649634
// type error is a comparison of an `impl` with its `trait` or when the
650635
// scope is outside of a `Body`.
651636
} else {
652-
let items = self.associated_items(assoc.container.id());
653-
// Find all the methods in the trait that could be called to construct the
654-
// expected associated type.
655-
// FIXME: consider suggesting the use of associated `const`s.
656-
let methods: Vec<(Span, String)> = items
657-
.items
658-
.iter()
659-
.filter(|(name, item)| {
660-
ty::AssocKind::Fn == item.kind && Some(**name) != current_method_ident
661-
})
662-
.filter_map(|(_, item)| {
663-
let method = self.fn_sig(item.def_id);
664-
match method.output().skip_binder().kind {
665-
ty::Projection(ty::ProjectionTy { item_def_id, .. })
666-
if item_def_id == proj_ty.item_def_id =>
667-
{
668-
Some((
669-
self.sess.source_map().guess_head_span(self.def_span(item.def_id)),
670-
format!("consider calling `{}`", self.def_path_str(item.def_id)),
671-
))
672-
}
673-
_ => None,
674-
}
675-
})
676-
.collect();
677-
if !methods.is_empty() {
678-
// Use a single `help:` to show all the methods in the trait that can
679-
// be used to construct the expected associated type.
680-
let mut span: MultiSpan =
681-
methods.iter().map(|(sp, _)| *sp).collect::<Vec<Span>>().into();
682-
let msg = format!(
683-
"{some} method{s} {are} available that return{r} `{ty}`",
684-
some = if methods.len() == 1 { "a" } else { "some" },
685-
s = pluralize!(methods.len()),
686-
are = if methods.len() == 1 { "is" } else { "are" },
687-
r = if methods.len() == 1 { "s" } else { "" },
688-
ty = values.expected
689-
);
690-
for (sp, label) in methods.into_iter() {
691-
span.push_span_label(sp, label);
692-
}
693-
db.span_help(span, &msg);
694-
suggested = true;
695-
}
637+
suggested |= self.point_at_methods_that_satisfy_associated_type(
638+
db,
639+
assoc.container.id(),
640+
current_method_ident,
641+
proj_ty.item_def_id,
642+
values.expected,
643+
);
696644
// Possibly suggest constraining the associated type to conform to the
697645
// found type.
698646
suggested |=
699647
self.suggest_constraint(db, &msg, body_owner_def_id, proj_ty, values.found);
700648
}
701-
if let (Some(hir_id), false) =
702-
(body_owner_def_id.as_local().map(|id| self.hir().as_local_hir_id(id)), suggested)
703-
{
704-
// When `body_owner` is an `impl` or `trait` item, look in its associated types for
705-
// `expected` and point at it.
706-
let parent_id = self.hir().get_parent_item(hir_id);
707-
let item = self.hir().find(parent_id);
708-
debug!("expected_projection parent item {:?}", item);
709-
match item {
710-
Some(hir::Node::Item(hir::Item {
711-
kind: hir::ItemKind::Trait(.., items), ..
712-
})) => {
713-
// FIXME: account for `#![feature(specialization)]`
714-
for item in &items[..] {
715-
match item.kind {
716-
hir::AssocItemKind::Type | hir::AssocItemKind::OpaqueTy => {
717-
if self.type_of(self.hir().local_def_id(item.id.hir_id))
718-
== values.found
719-
{
720-
if let hir::Defaultness::Default { has_value: true } =
721-
item.defaultness
722-
{
723-
db.span_label(
724-
item.span,
725-
"associated type defaults can't be assumed inside the \
726-
trait defining them",
727-
);
728-
} else {
729-
db.span_label(item.span, "expected this associated type");
730-
}
731-
suggested = true;
732-
}
733-
}
734-
_ => {}
735-
}
736-
}
737-
}
738-
Some(hir::Node::Item(hir::Item {
739-
kind: hir::ItemKind::Impl { items, .. },
740-
..
741-
})) => {
742-
for item in &items[..] {
743-
match item.kind {
744-
hir::AssocItemKind::Type | hir::AssocItemKind::OpaqueTy => {
745-
if self.type_of(self.hir().local_def_id(item.id.hir_id))
746-
== values.found
747-
{
748-
db.span_label(item.span, "expected this associated type");
749-
suggested = true;
750-
}
751-
}
752-
_ => {}
753-
}
754-
}
755-
}
756-
_ => {}
757-
}
649+
if !suggested {
650+
suggested = self.point_at_associated_type(db, body_owner_def_id, values.found);
758651
}
759652
if let ty::Opaque(def_id, _) = proj_ty.self_ty().kind {
760653
// When the expected `impl Trait` is not defined in the current item, it will come from
@@ -800,6 +693,121 @@ fn foo(&self) -> Self::T { String::new() }
800693
}
801694
}
802695

696+
fn point_at_methods_that_satisfy_associated_type(
697+
&self,
698+
db: &mut DiagnosticBuilder<'_>,
699+
assoc_container_id: DefId,
700+
current_method_ident: Option<Symbol>,
701+
proj_ty_item_def_id: DefId,
702+
expected: Ty<'tcx>,
703+
) -> bool {
704+
let items = self.associated_items(assoc_container_id);
705+
// Find all the methods in the trait that could be called to construct the
706+
// expected associated type.
707+
// FIXME: consider suggesting the use of associated `const`s.
708+
let methods: Vec<(Span, String)> = items
709+
.items
710+
.iter()
711+
.filter(|(name, item)| {
712+
ty::AssocKind::Fn == item.kind && Some(**name) != current_method_ident
713+
})
714+
.filter_map(|(_, item)| {
715+
let method = self.fn_sig(item.def_id);
716+
match method.output().skip_binder().kind {
717+
ty::Projection(ty::ProjectionTy { item_def_id, .. })
718+
if item_def_id == proj_ty_item_def_id =>
719+
{
720+
Some((
721+
self.sess.source_map().guess_head_span(self.def_span(item.def_id)),
722+
format!("consider calling `{}`", self.def_path_str(item.def_id)),
723+
))
724+
}
725+
_ => None,
726+
}
727+
})
728+
.collect();
729+
if !methods.is_empty() {
730+
// Use a single `help:` to show all the methods in the trait that can
731+
// be used to construct the expected associated type.
732+
let mut span: MultiSpan =
733+
methods.iter().map(|(sp, _)| *sp).collect::<Vec<Span>>().into();
734+
let msg = format!(
735+
"{some} method{s} {are} available that return{r} `{ty}`",
736+
some = if methods.len() == 1 { "a" } else { "some" },
737+
s = pluralize!(methods.len()),
738+
are = if methods.len() == 1 { "is" } else { "are" },
739+
r = if methods.len() == 1 { "s" } else { "" },
740+
ty = expected
741+
);
742+
for (sp, label) in methods.into_iter() {
743+
span.push_span_label(sp, label);
744+
}
745+
db.span_help(span, &msg);
746+
return true;
747+
}
748+
false
749+
}
750+
751+
fn point_at_associated_type(
752+
&self,
753+
db: &mut DiagnosticBuilder<'_>,
754+
body_owner_def_id: DefId,
755+
found: Ty<'tcx>,
756+
) -> bool {
757+
let hir_id = match body_owner_def_id.as_local().map(|id| self.hir().as_local_hir_id(id)) {
758+
Some(hir_id) => hir_id,
759+
None => return false,
760+
};
761+
// When `body_owner` is an `impl` or `trait` item, look in its associated types for
762+
// `expected` and point at it.
763+
let parent_id = self.hir().get_parent_item(hir_id);
764+
let item = self.hir().find(parent_id);
765+
debug!("expected_projection parent item {:?}", item);
766+
match item {
767+
Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Trait(.., items), .. })) => {
768+
// FIXME: account for `#![feature(specialization)]`
769+
for item in &items[..] {
770+
match item.kind {
771+
hir::AssocItemKind::Type | hir::AssocItemKind::OpaqueTy => {
772+
if self.type_of(self.hir().local_def_id(item.id.hir_id)) == found {
773+
if let hir::Defaultness::Default { has_value: true } =
774+
item.defaultness
775+
{
776+
db.span_label(
777+
item.span,
778+
"associated type defaults can't be assumed inside the \
779+
trait defining them",
780+
);
781+
} else {
782+
db.span_label(item.span, "expected this associated type");
783+
}
784+
return true;
785+
}
786+
}
787+
_ => {}
788+
}
789+
}
790+
}
791+
Some(hir::Node::Item(hir::Item {
792+
kind: hir::ItemKind::Impl { items, .. }, ..
793+
})) => {
794+
for item in &items[..] {
795+
match item.kind {
796+
hir::AssocItemKind::Type | hir::AssocItemKind::OpaqueTy => {
797+
if self.type_of(self.hir().local_def_id(item.id.hir_id)) == found {
798+
db.span_label(item.span, "expected this associated type");
799+
return true;
800+
}
801+
}
802+
_ => {}
803+
}
804+
}
805+
}
806+
_ => {}
807+
}
808+
false
809+
}
810+
803811
/// Given a slice of `hir::GenericBound`s, if any of them corresponds to the `trait_ref`
804812
/// requirement, provide a strucuted suggestion to constrain it to a given type `ty`.
805813
fn constrain_generic_bound_associated_type_structured_suggestion(
@@ -812,22 +820,16 @@ fn foo(&self) -> Self::T { String::new() }
812820
msg: &str,
813821
) -> bool {
814822
// FIXME: we would want to call `resolve_vars_if_possible` on `ty` before suggesting.
815-
for bound in bounds {
816-
match bound {
817-
hir::GenericBound::Trait(ptr, hir::TraitBoundModifier::None) => {
818-
// Relate the type param against `T` in `<A as T>::Foo`.
819-
if ptr.trait_ref.trait_def_id() == Some(trait_ref.def_id)
820-
&& self.constrain_associated_type_structured_suggestion(
821-
db, ptr.span, assoc, ty, msg,
822-
)
823-
{
824-
return true;
825-
}
826-
}
827-
_ => {}
823+
bounds.iter().any(|bound| match bound {
824+
hir::GenericBound::Trait(ptr, hir::TraitBoundModifier::None) => {
825+
// Relate the type param against `T` in `<A as T>::Foo`.
826+
ptr.trait_ref.trait_def_id() == Some(trait_ref.def_id)
827+
&& self.constrain_associated_type_structured_suggestion(
828+
db, ptr.span, assoc, ty, msg,
829+
)
828830
}
829-
}
830-
false
831+
_ => false,
832+
})
831833
}
832834

833835
/// Given a span corresponding to a bound, provide a structured suggestion to set an

0 commit comments

Comments
 (0)