1
Fork 0

review comments: make suggestion more accurate

This commit is contained in:
Esteban Küber 2022-12-27 09:25:00 -08:00
parent c9381fc334
commit 0c0685bb68
5 changed files with 73 additions and 31 deletions

View file

@ -1604,7 +1604,6 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
None, None,
Some(coercion_error), Some(coercion_error),
); );
fcx.check_for_range_as_method_call(&mut err, expr, found, expected);
} }
if visitor.ret_exprs.len() > 0 && let Some(expr) = expression { if visitor.ret_exprs.len() > 0 && let Some(expr) = expression {

View file

@ -71,6 +71,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.note_type_is_not_clone(err, expected, expr_ty, expr); self.note_type_is_not_clone(err, expected, expr_ty, expr);
self.note_need_for_fn_pointer(err, expected, expr_ty); self.note_need_for_fn_pointer(err, expected, expr_ty);
self.note_internal_mutation_in_method(err, expr, expected, expr_ty); self.note_internal_mutation_in_method(err, expr, expected, expr_ty);
self.check_for_range_as_method_call(err, expr, expr_ty, expected);
} }
/// Requires that the two types unify, and prints an error message if /// Requires that the two types unify, and prints an error message if
@ -1449,14 +1450,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} }
} }
/// Identify when the user has written `foo..bar()` instead of `foo.bar()`.
pub fn check_for_range_as_method_call( pub fn check_for_range_as_method_call(
&self, &self,
err: &mut Diagnostic, err: &mut Diagnostic,
expr: &hir::Expr<'_>, expr: &hir::Expr<'_>,
checked_ty: Ty<'tcx>, checked_ty: Ty<'tcx>,
// FIXME: We should do analysis to see if we can synthesize an expresion that produces
// this type for always accurate suggestions, or at least marking the suggestion as
// machine applicable.
expected_ty: Ty<'tcx>, expected_ty: Ty<'tcx>,
) { ) {
if !hir::is_range_literal(expr) { if !hir::is_range_literal(expr) {
@ -1467,13 +1466,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
[start, end], [start, end],
_, _,
) = expr.kind else { return; }; ) = expr.kind else { return; };
let parent = self.tcx.hir().get_parent_node(expr.hir_id);
if let Some(hir::Node::ExprField(_)) = self.tcx.hir().find(parent) {
// Ignore `Foo { field: a..Default::default() }`
return;
}
let mut expr = end.expr; let mut expr = end.expr;
while let hir::ExprKind::MethodCall(_, rcvr, ..) = expr.kind { while let hir::ExprKind::MethodCall(_, rcvr, ..) = expr.kind {
// Getting to the root receiver and asserting it is a fn call let's us ignore cases in // Getting to the root receiver and asserting it is a fn call let's us ignore cases in
// `src/test/ui/methods/issues/issue-90315.stderr`. // `src/test/ui/methods/issues/issue-90315.stderr`.
expr = rcvr; expr = rcvr;
} }
let hir::ExprKind::Call(..) = expr.kind else { return; }; let hir::ExprKind::Call(method_name, _) = expr.kind else { return; };
let ty::Adt(adt, _) = checked_ty.kind() else { return; }; let ty::Adt(adt, _) = checked_ty.kind() else { return; };
if self.tcx.lang_items().range_struct() != Some(adt.did()) { if self.tcx.lang_items().range_struct() != Some(adt.did()) {
return; return;
@ -1483,11 +1487,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{ {
return; return;
} }
// Check if start has method named end.
let hir::ExprKind::Path(hir::QPath::Resolved(None, p)) = method_name.kind else { return; };
let [hir::PathSegment { ident, .. }] = p.segments else { return; };
let self_ty = self.typeck_results.borrow().expr_ty(start.expr);
let Ok(_pick) = self.probe_for_name(
probe::Mode::MethodCall,
*ident,
probe::IsSuggestion(true),
self_ty,
expr.hir_id,
probe::ProbeScope::AllTraits,
) else { return; };
let mut sugg = ".";
let mut span = start.expr.span.between(end.expr.span);
if span.lo() + BytePos(2) == span.hi() {
// There's no space between the start, the range op and the end, suggest removal which
// will be more noticeable than the replacement of `..` with `.`.
span = span.with_lo(span.lo() + BytePos(1));
sugg = "";
}
err.span_suggestion_verbose( err.span_suggestion_verbose(
start.expr.span.between(end.expr.span), span,
"you might have meant to write a method call instead of a range", "you likely meant to write a method call instead of a range",
".".to_string(), sugg,
Applicability::MaybeIncorrect, Applicability::MachineApplicable,
); );
} }
} }

View file

@ -3344,10 +3344,18 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
let suggestion = if let Some((start, end)) = this.diagnostic_metadata.in_range let suggestion = if let Some((start, end)) = this.diagnostic_metadata.in_range
&& path[0].ident.span.lo() == end.span.lo() && path[0].ident.span.lo() == end.span.lo()
{ {
let mut sugg = ".";
let mut span = start.span.between(end.span);
if span.lo() + BytePos(2) == span.hi() {
// There's no space between the start, the range op and the end, suggest
// removal which will look better.
span = span.with_lo(span.lo() + BytePos(1));
sugg = "";
}
Some(( Some((
start.span.between(end.span), span,
"you might have meant to write a method call instead of a range", "you might have meant to write a method call instead of a range",
".".to_string(), sugg.to_string(),
Applicability::MaybeIncorrect, Applicability::MaybeIncorrect,
)) ))
} else if res.is_none() { } else if res.is_none() {

View file

@ -4,14 +4,22 @@ fn as_ref() -> Option<Vec<u8>> {
struct Type { struct Type {
option: Option<Vec<u8>> option: Option<Vec<u8>>
} }
trait Trait {
fn foo(&self) -> Vec<u8>;
}
impl Trait for Option<Vec<u8>> {
fn foo(&self) -> Vec<u8> {
vec![1, 2, 3]
}
}
impl Type { impl Type {
fn method(&self) -> Option<Vec<u8>> { fn method(&self) -> Option<Vec<u8>> {
self.option..as_ref().map(|x| x) self.option..as_ref().map(|x| x)
//~^ ERROR E0308 //~^ ERROR E0308
} }
fn method2(&self) -> Option<Vec<u8>> { fn method2(&self) -> &u8 {
self.option..foo().map(|x| x) self.option..foo().get(0)
//~^ ERROR E0425 //~^ ERROR E0425
//~| ERROR E0308 //~| ERROR E0308
} }

View file

@ -1,16 +1,17 @@
error[E0425]: cannot find function `foo` in this scope error[E0425]: cannot find function `foo` in this scope
--> $DIR/method-access-to-range-literal-typo.rs:14:22 --> $DIR/method-access-to-range-literal-typo.rs:22:22
| |
LL | self.option..foo().map(|x| x) LL | self.option..foo().get(0)
| ^^^ not found in this scope | ^^^ not found in this scope
| |
help: you might have meant to write a method call instead of a range help: you might have meant to write a method call instead of a range
| |
LL | self.option.foo().map(|x| x) LL - self.option..foo().get(0)
| ~ LL + self.option.foo().get(0)
|
error[E0308]: mismatched types error[E0308]: mismatched types
--> $DIR/method-access-to-range-literal-typo.rs:10:9 --> $DIR/method-access-to-range-literal-typo.rs:18:9
| |
LL | fn method(&self) -> Option<Vec<u8>> { LL | fn method(&self) -> Option<Vec<u8>> {
| --------------- expected `Option<Vec<u8>>` because of return type | --------------- expected `Option<Vec<u8>>` because of return type
@ -19,25 +20,27 @@ LL | self.option..as_ref().map(|x| x)
| |
= note: expected enum `Option<_>` = note: expected enum `Option<_>`
found struct `std::ops::Range<Option<_>>` found struct `std::ops::Range<Option<_>>`
help: you might have meant to write a method call instead of a range help: you likely meant to write a method call instead of a range
|
LL - self.option..as_ref().map(|x| x)
LL + self.option.as_ref().map(|x| x)
| |
LL | self.option.as_ref().map(|x| x)
| ~
error[E0308]: mismatched types error[E0308]: mismatched types
--> $DIR/method-access-to-range-literal-typo.rs:14:9 --> $DIR/method-access-to-range-literal-typo.rs:22:9
| |
LL | fn method2(&self) -> Option<Vec<u8>> { LL | fn method2(&self) -> &u8 {
| --------------- expected `Option<Vec<u8>>` because of return type | --- expected `&u8` because of return type
LL | self.option..foo().map(|x| x) LL | self.option..foo().get(0)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `Option`, found struct `Range` | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&u8`, found struct `Range`
| |
= note: expected enum `Option<_>` = note: expected reference `&u8`
found struct `std::ops::Range<Option<_>>` found struct `std::ops::Range<Option<Vec<u8>>>`
help: you might have meant to write a method call instead of a range help: you likely meant to write a method call instead of a range
|
LL - self.option..foo().get(0)
LL + self.option.foo().get(0)
| |
LL | self.option.foo().map(|x| x)
| ~
error: aborting due to 3 previous errors error: aborting due to 3 previous errors