Skip to content

Commit

Permalink
Prefer implemented traits in suggestions.
Browse files Browse the repository at this point in the history
If `a.method();` can't be resolved, we first look for implemented traits
globally and suggest those. If there are no such traits found, we only
then fall back to suggesting from the unfiltered list of traits.
  • Loading branch information
huonw committed Jan 16, 2015
1 parent 06ad8bb commit 0a55aac
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 31 deletions.
7 changes: 4 additions & 3 deletions src/librustc_typeck/check/method/mod.rs
Expand Up @@ -38,8 +38,9 @@ mod suggest;

pub enum MethodError {
// Did not find an applicable method, but we did find various
// static methods that may apply.
NoMatch(Vec<CandidateSource>),
// static methods that may apply, as well as a list of
// not-in-scope traits which may work.
NoMatch(Vec<CandidateSource>, Vec<ast::DefId>),

// Multiple methods might apply.
Ambiguity(Vec<CandidateSource>),
Expand All @@ -65,7 +66,7 @@ pub fn exists<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
{
match probe::probe(fcx, span, method_name, self_ty, call_expr_id) {
Ok(_) => true,
Err(NoMatch(_)) => false,
Err(NoMatch(_, _)) => false,
Err(Ambiguity(_)) => true,
}
}
Expand Down
64 changes: 62 additions & 2 deletions src/librustc_typeck/check/method/probe.rs
Expand Up @@ -11,6 +11,7 @@
use super::{MethodError,Ambiguity,NoMatch};
use super::MethodIndex;
use super::{CandidateSource,ImplSource,TraitSource};
use super::suggest;

use check;
use check::{FnCtxt, NoPreference};
Expand All @@ -25,6 +26,7 @@ use middle::infer::InferCtxt;
use syntax::ast;
use syntax::codemap::{Span, DUMMY_SP};
use std::collections::HashSet;
use std::mem;
use std::rc::Rc;
use util::ppaux::Repr;

Expand All @@ -42,6 +44,7 @@ struct ProbeContext<'a, 'tcx:'a> {
extension_candidates: Vec<Candidate<'tcx>>,
impl_dups: HashSet<ast::DefId>,
static_candidates: Vec<CandidateSource>,
all_traits_search: bool,
}

struct CandidateStep<'tcx> {
Expand Down Expand Up @@ -127,7 +130,7 @@ pub fn probe<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
// take place in the `fcx.infcx().probe` below.
let steps = match create_steps(fcx, span, self_ty) {
Some(steps) => steps,
None => return Err(NoMatch(Vec::new())),
None => return Err(NoMatch(Vec::new(), Vec::new())),
};

// Create a list of simplified self types, if we can.
Expand Down Expand Up @@ -208,9 +211,17 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
steps: Rc::new(steps),
opt_simplified_steps: opt_simplified_steps,
static_candidates: Vec::new(),
all_traits_search: false,
}
}

fn reset(&mut self) {
self.inherent_candidates.clear();
self.extension_candidates.clear();
self.impl_dups.clear();
self.static_candidates.clear();
}

fn tcx(&self) -> &'a ty::ctxt<'tcx> {
self.fcx.tcx()
}
Expand Down Expand Up @@ -446,6 +457,15 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
}
}

fn assemble_extension_candidates_for_all_traits(&mut self) {
let mut duplicates = HashSet::new();
for trait_info in suggest::all_traits(self.fcx.ccx) {
if duplicates.insert(trait_info.def_id) {
self.assemble_extension_candidates_for_trait(trait_info.def_id)
}
}
}

fn assemble_extension_candidates_for_trait(&mut self,
trait_def_id: ast::DefId) {
debug!("assemble_extension_candidates_for_trait(trait_def_id={})",
Expand Down Expand Up @@ -715,7 +735,47 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
}
}

Err(NoMatch(self.static_candidates))
let static_candidates = mem::replace(&mut self.static_candidates, vec![]);

let out_of_scope_traits = if !self.all_traits_search {
// things failed, and we haven't yet looked through all
// traits, so lets do that now:
self.reset();
self.all_traits_search = true;

let span = self.span;
let tcx = self.tcx();

self.assemble_extension_candidates_for_all_traits();

match self.pick() {
Ok(p) => vec![p.method_ty.container.id()],
Err(Ambiguity(v)) => v.into_iter().map(|source| {
match source {
TraitSource(id) => id,
ImplSource(impl_id) => {
match ty::trait_id_of_impl(tcx, impl_id) {
Some(id) => id,
None => tcx.sess.span_bug(span,
"found inherent method when looking \
at traits")
}
}
}
}).collect(),
// it'd be really weird for this assertion to trigger,
// given the `vec![]` in the else branch below
Err(NoMatch(_, others)) => {
assert!(others.is_empty());
vec![]
}
}
} else {
// we've just looked through all traits and didn't find
// anything at all.
vec![]
};
Err(NoMatch(static_candidates, out_of_scope_traits))
}

fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option<PickResult<'tcx>> {
Expand Down
57 changes: 38 additions & 19 deletions src/librustc_typeck/check/method/suggest.rs
Expand Up @@ -35,7 +35,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
error: MethodError)
{
match error {
MethodError::NoMatch(static_sources) => {
MethodError::NoMatch(static_sources, out_of_scope_traits) => {
let cx = fcx.tcx();
let method_ustring = method_name.user_string(cx);

Expand Down Expand Up @@ -75,7 +75,7 @@ pub fn report_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
report_candidates(fcx, span, method_name, static_sources);
}

suggest_traits_to_import(fcx, span, rcvr_ty, method_name)
suggest_traits_to_import(fcx, span, rcvr_ty, method_name, out_of_scope_traits)
}

MethodError::Ambiguity(sources) => {
Expand Down Expand Up @@ -136,10 +136,35 @@ pub type AllTraitsVec = Vec<TraitInfo>;
fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
span: Span,
_rcvr_ty: Ty<'tcx>,
method_name: ast::Name)
method_name: ast::Name,
valid_out_of_scope_traits: Vec<ast::DefId>)
{
let tcx = fcx.tcx();
let method_ustring = method_name.user_string(tcx);

if !valid_out_of_scope_traits.is_empty() {
let mut candidates = valid_out_of_scope_traits;
candidates.sort();
let msg = format!(
"methods from traits can only be called if the trait is in scope; \
the following {traits_are} implemented and {define} a method `{name}`:",
traits_are = if candidates.len() == 1 {"trait is"} else {"traits are"},
define = if candidates.len() == 1 {"defines"} else {"define"},
name = method_ustring);

fcx.sess().fileline_help(span, &msg[]);

for (i, trait_did) in candidates.iter().enumerate() {
fcx.sess().fileline_help(span,
&*format!("candidate #{}: `{}`",
i + 1,
ty::item_path_str(fcx.tcx(), *trait_did)))

}
return
}

// there's no implemented traits, so lets suggest some traits to implement
let mut candidates = all_traits(fcx.ccx)
.filter(|info| trait_method(tcx, info.def_id, method_name).is_some())
.collect::<Vec<_>>();
Expand All @@ -148,22 +173,16 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
// sort from most relevant to least relevant
candidates.sort_by(|a, b| a.cmp(b).reverse());

let method_ustring = method_name.user_string(tcx);
let msg = format!(
"methods from traits can only be called if the trait is implemented and \
in scope; no such traits are but the following {traits_define} a method `{name}`:",
traits_define = if candidates.len() == 1 {"trait defines"} else {"traits define"},
name = method_ustring);

span_help!(fcx.sess(), span,
"methods from traits can only be called if the trait is implemented \
and in scope; the following trait{s} define{inv_s} a method `{name}`:",
s = if candidates.len() == 1 {""} else {"s"},
inv_s = if candidates.len() == 1 {"s"} else {""},
name = method_ustring);
fcx.sess().fileline_help(span, &msg[]);

for (i, trait_info) in candidates.iter().enumerate() {
// provide a good-as-possible span; the span of
// the trait if it is local, or the span of the
// method call itself if not
let trait_span = fcx.tcx().map.def_id_span(trait_info.def_id, span);

fcx.sess().fileline_help(trait_span,
fcx.sess().fileline_help(span,
&*format!("candidate #{}: `{}`",
i + 1,
ty::item_path_str(fcx.tcx(), trait_info.def_id)))
Expand All @@ -173,7 +192,7 @@ fn suggest_traits_to_import<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,

#[derive(Copy)]
pub struct TraitInfo {
def_id: ast::DefId,
pub def_id: ast::DefId,
}

impl TraitInfo {
Expand Down Expand Up @@ -206,7 +225,7 @@ impl Ord for TraitInfo {
}

/// Retrieve all traits in this crate and any dependent crates.
fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
pub fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
if ccx.all_traits.borrow().is_none() {
use syntax::visit;

Expand Down Expand Up @@ -268,7 +287,7 @@ fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> {
}
}

struct AllTraits<'a> {
pub struct AllTraits<'a> {
borrow: cell::Ref<'a Option<AllTraitsVec>>,
idx: usize
}
Expand Down
7 changes: 6 additions & 1 deletion src/test/auxiliary/no_method_suggested_traits.rs
Expand Up @@ -12,8 +12,13 @@ pub use reexport::Reexported;

pub mod foo {
pub trait PubPub {
fn method(&self);
fn method(&self) {}

fn method3(&self) {}
}

impl PubPub for u32 {}
impl PubPub for i32 {}
}
pub mod bar {
trait PubPriv {
Expand Down
44 changes: 38 additions & 6 deletions src/test/compile-fail/no-method-suggested-traits.rs
Expand Up @@ -13,18 +13,50 @@
extern crate no_method_suggested_traits;

mod foo {
trait Bar { //~ HELP `foo::Bar`
fn method(&self);
trait Bar {
fn method(&self) {}

fn method2(&self) {}
}

impl Bar for u32 {}

impl Bar for char {}
}

fn main() {
1u32.method();
//~^ ERROR does not implement
//~^^ HELP the following traits are implemented and define a method `method`
//~^^^ HELP `foo::Bar`
//~^^^^ HELP `no_method_suggested_traits::foo::PubPub`

'a'.method();
//~^ ERROR does not implement
//~^^ HELP the following trait is implemented and defines a method `method`
//~^^^ HELP `foo::Bar`

1i32.method();
//~^ ERROR does not implement
//~^^ HELP the following trait is implemented and defines a method `method`
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`

1u64.method();
//~^ ERROR does not implement
//~^^ HELP the following traits define a method `method`
//~^^^ HELP `foo::Bar`
//~^^^^ HELP `no_method_suggested_traits::foo::PubPub`
//~^^^^^ HELP `no_method_suggested_traits::reexport::Reexported`
//~^^^^^^ HELP `no_method_suggested_traits::bar::PubPriv`
//~^^^^^^^ HELP `no_method_suggested_traits::qux::PrivPub`
//~^^^^^^^^ HELP `no_method_suggested_traits::quz::PrivPriv`

1u64.method2();
//~^ ERROR does not implement
//~^^ HELP the following trait defines a method `method2`
//~^^^ HELP `foo::Bar`
1u64.method3();
//~^ ERROR does not implement
//~^^ HELP the following trait defines a method `method3`
//~^^^ HELP `no_method_suggested_traits::foo::PubPub`
//~^^^^ HELP `no_method_suggested_traits::reexport::Reexported`
//~^^^^^ HELP `no_method_suggested_traits::bar::PubPriv`
//~^^^^^^ HELP `no_method_suggested_traits::qux::PrivPub`
//~^^^^^^^ HELP `no_method_suggested_traits::quz::PrivPriv`
}

7 comments on commit 0a55aac

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 16, 2015

Choose a reason for hiding this comment

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

saw approval from nikomatsakis
at huonw@0a55aac

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 16, 2015

Choose a reason for hiding this comment

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

merging huonw/rust/trait-suggestions = 0a55aac into auto

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 16, 2015

Choose a reason for hiding this comment

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

status: {"merge_sha": "ed530d7a3b67989047e6fe61fe101b9d8158585f"}

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 16, 2015

Choose a reason for hiding this comment

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

huonw/rust/trait-suggestions = 0a55aac merged ok, testing candidate = ed530d7

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 17, 2015

Choose a reason for hiding this comment

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

fast-forwarding master to auto = ed530d7

@bors
Copy link
Contributor

@bors bors commented on 0a55aac Jan 17, 2015

Choose a reason for hiding this comment

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

fast-forwarding master to auto = ed530d7

Please sign in to comment.