Skip to content

Commit

Permalink
Rollup merge of rust-lang#67127 - estebank:disambiguate-suggestion, r…
Browse files Browse the repository at this point in the history 
…=varkor

Use structured suggestion for disambiguating method calls

Fix rust-lang#65635.
  • Loading branch information
@Mark-Simulacrum
Mark-Simulacrum authored Dec 18, 2019
2 parents 624694d + 8c4f1d5 commit 67d560b
Show file tree
Hide file tree
Showing 18 changed files with 274 additions and 82 deletions.
11 changes: 11 additions & 0 deletions src/librustc/ty/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,17 @@ pub enum AssocKind {
Type
}

impl AssocKind {
pub fn suggestion_descr(&self) -> &'static str {
match self {
ty::AssocKind::Method => "method call",
ty::AssocKind::Type |
ty::AssocKind::OpaqueTy => "associated type",
ty::AssocKind::Const => "associated constant",
}
}
}

impl AssocItem {
pub fn def_kind(&self) -> DefKind {
match self.kind {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/check/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1425,8 +1425,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
field: ast::Ident,
) -> Ty<'tcx> {
let expr_t = self.check_expr_with_needs(base, needs);
let expr_t = self.structurally_resolved_type(base.span,
expr_t);
let expr_t = self.structurally_resolved_type(base.span, expr_t);
let mut private_candidate = None;
let mut autoderef = self.autoderef(expr.span, expr_t);
while let Some((base_t, _)) = autoderef.next() {
Expand Down
161 changes: 118 additions & 43 deletions src/librustc_typeck/check/method/suggest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc::traits::Obligation;
use rustc::ty::{self, Ty, TyCtxt, ToPolyTraitRef, ToPredicate, TypeFoldable};
use rustc::ty::print::with_crate_prefix;
use syntax_pos::{Span, FileName};
use syntax::ast;
use syntax::{ast, source_map};
use syntax::util::lev_distance;

use rustc_error_codes::*;
Expand Down Expand Up @@ -79,37 +79,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return None;
}

let print_disambiguation_help = |
err: &mut DiagnosticBuilder<'_>,
trait_name: String,
| {
err.help(&format!(
"to disambiguate the method call, write `{}::{}({}{})` instead",
trait_name,
item_name,
if rcvr_ty.is_region_ptr() && args.is_some() {
if rcvr_ty.is_mutable_ptr() {
"&mut "
} else {
"&"
}
} else {
""
},
args.map(|arg| arg
.iter()
.map(|arg| self.tcx.sess.source_map().span_to_snippet(arg.span)
.unwrap_or_else(|_| "...".to_owned()))
.collect::<Vec<_>>()
.join(", ")
).unwrap_or_else(|| "...".to_owned())
));
};

let report_candidates = |
span: Span,
err: &mut DiagnosticBuilder<'_>,
mut sources: Vec<CandidateSource>,
sugg_span: Span,
| {
sources.sort();
sources.dedup();
Expand Down Expand Up @@ -150,15 +124,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};

let note_str = if sources.len() > 1 {
format!("candidate #{} is defined in an impl{} for the type `{}`",
idx + 1,
insertion,
impl_ty)
let (note_str, idx) = if sources.len() > 1 {
(format!(
"candidate #{} is defined in an impl{} for the type `{}`",
idx + 1,
insertion,
impl_ty,
), Some(idx + 1))
} else {
format!("the candidate is defined in an impl{} for the type `{}`",
insertion,
impl_ty)
(format!(
"the candidate is defined in an impl{} for the type `{}`",
insertion,
impl_ty,
), None)
};
if let Some(note_span) = note_span {
// We have a span pointing to the method. Show note with snippet.
Expand All @@ -168,7 +146,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err.note(&note_str);
}
if let Some(trait_ref) = self.tcx.impl_trait_ref(impl_did) {
print_disambiguation_help(err, self.tcx.def_path_str(trait_ref.def_id));
let path = self.tcx.def_path_str(trait_ref.def_id);

let ty = match item.kind {
ty::AssocKind::Const |
ty::AssocKind::Type |
ty::AssocKind::OpaqueTy => rcvr_ty,
ty::AssocKind::Method => self.tcx.fn_sig(item.def_id)
.inputs()
.skip_binder()
.get(0)
.filter(|ty| ty.is_region_ptr() && !rcvr_ty.is_region_ptr())
.map(|ty| *ty)
.unwrap_or(rcvr_ty),
};
print_disambiguation_help(
item_name,
args,
err,
path,
ty,
item.kind,
sugg_span,
idx,
self.tcx.sess.source_map(),
);
}
}
CandidateSource::TraitSource(trait_did) => {
Expand All @@ -182,19 +184,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
let item_span = self.tcx.sess.source_map()
.def_span(self.tcx.def_span(item.def_id));
if sources.len() > 1 {
let idx = if sources.len() > 1 {
span_note!(err,
item_span,
"candidate #{} is defined in the trait `{}`",
idx + 1,
self.tcx.def_path_str(trait_did));
Some(idx + 1)
} else {
span_note!(err,
item_span,
"the candidate is defined in the trait `{}`",
self.tcx.def_path_str(trait_did));
}
print_disambiguation_help(err, self.tcx.def_path_str(trait_did));
None
};
let path = self.tcx.def_path_str(trait_did);
print_disambiguation_help(
item_name,
args,
err,
path,
rcvr_ty,
item.kind,
sugg_span,
idx,
self.tcx.sess.source_map(),
);
}
}
}
Expand All @@ -203,6 +218,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};

let sugg_span = if let SelfSource::MethodCall(expr) = source {
// Given `foo.bar(baz)`, `expr` is `bar`, but we want to point to the whole thing.
self.tcx.hir().expect_expr(self.tcx.hir().get_parent_node(expr.hir_id)).span
} else {
span
};

match error {
MethodError::NoMatch(NoMatchData {
static_candidates: static_sources,
Expand Down Expand Up @@ -495,9 +517,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
));
}

report_candidates(span, &mut err, static_sources);
report_candidates(span, &mut err, static_sources, sugg_span);
} else if static_sources.len() > 1 {
report_candidates(span, &mut err, static_sources);
report_candidates(span, &mut err, static_sources, sugg_span);
}

if !unsatisfied_predicates.is_empty() {
Expand Down Expand Up @@ -584,7 +606,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"multiple applicable items in scope");
err.span_label(span, format!("multiple `{}` found", item_name));

report_candidates(span, &mut err, sources);
report_candidates(span, &mut err, sources, sugg_span);
err.emit();
}

Expand Down Expand Up @@ -1123,3 +1145,56 @@ impl hir::intravisit::Visitor<'tcx> for UsePlacementFinder<'tcx> {
hir::intravisit::NestedVisitorMap::None
}
}

fn print_disambiguation_help(
item_name: ast::Ident,
args: Option<&'tcx [hir::Expr]>,
err: &mut DiagnosticBuilder<'_>,
trait_name: String,
rcvr_ty: Ty<'_>,
kind: ty::AssocKind,
span: Span,
candidate: Option<usize>,
source_map: &source_map::SourceMap,
) {
let mut applicability = Applicability::MachineApplicable;
let sugg_args = if let (ty::AssocKind::Method, Some(args)) = (kind, args) {
format!(
"({}{})",
if rcvr_ty.is_region_ptr() {
if rcvr_ty.is_mutable_ptr() {
"&mut "
} else {
"&"
}
} else {
""
},
args.iter()
.map(|arg| source_map.span_to_snippet(arg.span)
.unwrap_or_else(|_| {
applicability = Applicability::HasPlaceholders;
"_".to_owned()
}))
.collect::<Vec<_>>()
.join(", "),
)
} else {
String::new()
};
let sugg = format!("{}::{}{}", trait_name, item_name, sugg_args);
err.span_suggestion(
span,
&format!(
"disambiguate the {} for {}",
kind.suggestion_descr(),
if let Some(candidate) = candidate {
format!("candidate #{}", candidate)
} else {
"the candidate".to_string()
},
),
sugg,
applicability,
);
}
10 changes: 8 additions & 2 deletions src/test/ui/associated-const/associated-const-ambiguity-report.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Foo` for the type `i32`
|
LL | const ID: i32 = 1;
| ^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `Foo::ID(...)` instead
note: candidate #2 is defined in an impl of the trait `Bar` for the type `i32`
--> $DIR/associated-const-ambiguity-report.rs:14:5
|
LL | const ID: i32 = 3;
| ^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `Bar::ID(...)` instead
help: disambiguate the associated constant for candidate #1
|
LL | const X: i32 = Foo::ID;
| ^^^^^^^
help: disambiguate the associated constant for candidate #2
|
LL | const X: i32 = Bar::ID;
| ^^^^^^^

error: aborting due to previous error

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/error-codes/E0034.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `Trait1` for the type `Tes
|
LL | fn foo() {}
| ^^^^^^^^
= help: to disambiguate the method call, write `Trait1::foo(...)` instead
note: candidate #2 is defined in an impl of the trait `Trait2` for the type `Test`
--> $DIR/E0034.rs:16:5
|
LL | fn foo() {}
| ^^^^^^^^
= help: to disambiguate the method call, write `Trait2::foo(...)` instead
help: disambiguate the method call for candidate #1
|
LL | Trait1::foo()
| ^^^^^^^^^^^
help: disambiguate the method call for candidate #2
|
LL | Trait2::foo()
| ^^^^^^^^^^^

error: aborting due to previous error

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/inference/inference_unstable_featured.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ LL | assert_eq!('x'.ipu_flatten(), 0);
| ^^^^^^^^^^^ multiple `ipu_flatten` found
|
= note: candidate #1 is defined in an impl of the trait `inference_unstable_iterator::IpuIterator` for the type `char`
= help: to disambiguate the method call, write `inference_unstable_iterator::IpuIterator::ipu_flatten('x')` instead
= note: candidate #2 is defined in an impl of the trait `inference_unstable_itertools::IpuItertools` for the type `char`
= help: to disambiguate the method call, write `inference_unstable_itertools::IpuItertools::ipu_flatten('x')` instead
help: disambiguate the method call for candidate #1
|
LL | assert_eq!(inference_unstable_iterator::IpuIterator::ipu_flatten(&'x'), 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the method call for candidate #2
|
LL | assert_eq!(inference_unstable_itertools::IpuItertools::ipu_flatten(&'x'), 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
6 changes: 4 additions & 2 deletions src/test/ui/issues/issue-18446.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ error[E0034]: multiple applicable items in scope
--> $DIR/issue-18446.rs:18:7
|
LL | x.foo();
| ^^^ multiple `foo` found
| --^^^--
| | |
| | multiple `foo` found
| help: disambiguate the method call for candidate #2: `T::foo(&x)`
|
note: candidate #1 is defined in an impl for the type `dyn T`
--> $DIR/issue-18446.rs:9:5
Expand All @@ -14,7 +17,6 @@ note: candidate #2 is defined in the trait `T`
|
LL | fn foo(&self);
| ^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `T::foo(&x)` instead

error: aborting due to previous error

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-3702-2.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `ToPrimitive` for the type
|
LL | fn to_int(&self) -> isize { 0 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `ToPrimitive::to_int(&self)` instead
note: candidate #2 is defined in an impl of the trait `Add` for the type `isize`
--> $DIR/issue-3702-2.rs:14:5
|
LL | fn to_int(&self) -> isize { *self }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `Add::to_int(&self)` instead
help: disambiguate the method call for candidate #1
|
LL | ToPrimitive::to_int(&self) + other.to_int()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the method call for candidate #2
|
LL | Add::to_int(&self) + other.to_int()
| ^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-65634-raw-ident-suggestion.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@ note: candidate #1 is defined in an impl of the trait `async` for the type `r#fn
|
LL | fn r#struct(&self) {
| ^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `async::r#struct(r#fn {})` instead
note: candidate #2 is defined in an impl of the trait `await` for the type `r#fn`
--> $DIR/issue-65634-raw-ident-suggestion.rs:10:5
|
LL | fn r#struct(&self) {
| ^^^^^^^^^^^^^^^^^^
= help: to disambiguate the method call, write `await::r#struct(r#fn {})` instead
help: disambiguate the method call for candidate #1
|
LL | async::r#struct(&r#fn {});
| ^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the method call for candidate #2
|
LL | await::r#struct(&r#fn {});
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 67d560b

Please sign in to comment.