Skip to content

Commit

Permalink
Auto merge of #120730 - estebank:confusable-api, r=oli-obk
Browse files Browse the repository at this point in the history
Provide suggestions through `rustc_confusables` annotations

Help with common API confusion, like asking for `push` when the data structure really has `append`.

```
error[E0599]: no method named `size` found for struct `Vec<{integer}>` in the current scope
  --> $DIR/rustc_confusables_std_cases.rs:17:7
   |
LL |     x.size();
   |       ^^^^
   |
help: you might have meant to use `len`
   |
LL |     x.len();
   |       ~~~
help: there is a method with a similar name
   |
LL |     x.resize();
   |       ~~~~~~
```

Fix #59450 (we can open subsequent tickets for specific cases).

Fix #108437:

```
error[E0599]: `Option<{integer}>` is not an iterator
   --> f101.rs:3:9
    |
3   |     opt.flat_map(|val| Some(val));
    |         ^^^^^^^^ `Option<{integer}>` is not an iterator
    |
   ::: /home/gh-estebank/rust/library/core/src/option.rs:571:1
    |
571 | pub enum Option<T> {
    | ------------------ doesn't satisfy `Option<{integer}>: Iterator`
    |
    = note: the following trait bounds were not satisfied:
            `Option<{integer}>: Iterator`
            which is required by `&mut Option<{integer}>: Iterator`
help: you might have meant to use `and_then`
    |
3   |     opt.and_then(|val| Some(val));
    |         ~~~~~~~~
```

On type error of method call arguments, look at confusables for suggestion. Fix #87212:

```
error[E0308]: mismatched types
    --> f101.rs:8:18
     |
8    |     stuff.append(Thing);
     |           ------ ^^^^^ expected `&mut Vec<Thing>`, found `Thing`
     |           |
     |           arguments to this method are incorrect
     |
     = note: expected mutable reference `&mut Vec<Thing>`
                           found struct `Thing`
note: method defined here
    --> /home/gh-estebank/rust/library/alloc/src/vec/mod.rs:2025:12
     |
2025 |     pub fn append(&mut self, other: &mut Self) {
     |            ^^^^^^
help: you might have meant to use `push`
     |
8    |     stuff.push(Thing);
     |           ~~~~
```
  • Loading branch information
bors committed Feb 23, 2024
2 parents 397937d + 91d0b37 commit a28d221
Show file tree
Hide file tree
Showing 72 changed files with 833 additions and 196 deletions.
126 changes: 119 additions & 7 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use crate::coercion::CoerceMany;
use crate::errors::SuggestPtrNullMut;
use crate::fn_ctxt::arg_matrix::{ArgMatrix, Compatibility, Error, ExpectedIdx, ProvidedIdx};
use crate::fn_ctxt::infer::FnCall;
use crate::gather_locals::Declaration;
use crate::method::probe::IsSuggestion;
use crate::method::probe::Mode::MethodCall;
use crate::method::probe::ProbeScope::TraitsInScope;
use crate::method::MethodCallee;
use crate::TupleArgumentsFlag::*;
use crate::{errors, Expectation::*};
Expand Down Expand Up @@ -451,7 +455,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
call_expr: &'tcx hir::Expr<'tcx>,
) -> ErrorGuaranteed {
// Next, let's construct the error
let (error_span, full_call_span, call_name, is_method) = match &call_expr.kind {
let (error_span, call_ident, full_call_span, call_name, is_method) = match &call_expr.kind {
hir::ExprKind::Call(
hir::Expr { hir_id, span, kind: hir::ExprKind::Path(qpath), .. },
_,
Expand All @@ -463,20 +467,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
CtorOf::Struct => "struct",
CtorOf::Variant => "enum variant",
};
(call_span, *span, name, false)
(call_span, None, *span, name, false)
} else {
(call_span, *span, "function", false)
(call_span, None, *span, "function", false)
}
}
hir::ExprKind::Call(hir::Expr { span, .. }, _) => (call_span, *span, "function", false),
hir::ExprKind::Call(hir::Expr { span, .. }, _) => {
(call_span, None, *span, "function", false)
}
hir::ExprKind::MethodCall(path_segment, _, _, span) => {
let ident_span = path_segment.ident.span;
let ident_span = if let Some(args) = path_segment.args {
ident_span.with_hi(args.span_ext.hi())
} else {
ident_span
};
(*span, ident_span, "method", true)
(*span, Some(path_segment.ident), ident_span, "method", true)
}
k => span_bug!(call_span, "checking argument types on a non-call: `{:?}`", k),
};
Expand Down Expand Up @@ -530,6 +536,104 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let callee_ty = callee_expr
.and_then(|callee_expr| self.typeck_results.borrow().expr_ty_adjusted_opt(callee_expr));

// Obtain another method on `Self` that have similar name.
let similar_assoc = |call_name: Ident| -> Option<(ty::AssocItem, ty::FnSig<'_>)> {
if let Some(callee_ty) = callee_ty
&& let Ok(Some(assoc)) = self.probe_op(
call_name.span,
MethodCall,
Some(call_name),
None,
IsSuggestion(true),
callee_ty.peel_refs(),
callee_expr.unwrap().hir_id,
TraitsInScope,
|mut ctxt| ctxt.probe_for_similar_candidate(),
)
&& let ty::AssocKind::Fn = assoc.kind
&& assoc.fn_has_self_parameter
{
let args = self.infcx.fresh_args_for_item(call_name.span, assoc.def_id);
let fn_sig = tcx.fn_sig(assoc.def_id).instantiate(tcx, args);
let fn_sig =
self.instantiate_binder_with_fresh_vars(call_name.span, FnCall, fn_sig);
Some((assoc, fn_sig));
}
None
};

let suggest_confusable = |err: &mut DiagnosticBuilder<'_>| {
let Some(call_name) = call_ident else {
return;
};
let Some(callee_ty) = callee_ty else {
return;
};
let input_types: Vec<Ty<'_>> = provided_arg_tys.iter().map(|(ty, _)| *ty).collect();
// Check for other methods in the following order
// - methods marked as `rustc_confusables` with the provided arguments
// - methods with the same argument type/count and short levenshtein distance
// - methods marked as `rustc_confusables` (done)
// - methods with short levenshtein distance

// Look for commonly confusable method names considering arguments.
if let Some(_name) = self.confusable_method_name(
err,
callee_ty.peel_refs(),
call_name,
Some(input_types.clone()),
) {
return;
}
// Look for method names with short levenshtein distance, considering arguments.
if let Some((assoc, fn_sig)) = similar_assoc(call_name)
&& fn_sig.inputs()[1..]
.iter()
.zip(input_types.iter())
.all(|(expected, found)| self.can_coerce(*expected, *found))
&& fn_sig.inputs()[1..].len() == input_types.len()
{
err.span_suggestion_verbose(
call_name.span,
format!("you might have meant to use `{}`", assoc.name),
assoc.name,
Applicability::MaybeIncorrect,
);
return;
}
// Look for commonly confusable method names disregarding arguments.
if let Some(_name) =
self.confusable_method_name(err, callee_ty.peel_refs(), call_name, None)
{
return;
}
// Look for similarly named methods with levenshtein distance with the right
// number of arguments.
if let Some((assoc, fn_sig)) = similar_assoc(call_name)
&& fn_sig.inputs()[1..].len() == input_types.len()
{
err.span_note(
tcx.def_span(assoc.def_id),
format!(
"there's is a method with similar name `{}`, but the arguments don't match",
assoc.name,
),
);
return;
}
// Fallthrough: look for similarly named methods with levenshtein distance.
if let Some((assoc, _)) = similar_assoc(call_name) {
err.span_note(
tcx.def_span(assoc.def_id),
format!(
"there's is a method with similar name `{}`, but their argument count \
doesn't match",
assoc.name,
),
);
return;
}
};
// A "softer" version of the `demand_compatible`, which checks types without persisting them,
// and treats error types differently
// This will allow us to "probe" for other argument orders that would likely have been correct
Expand Down Expand Up @@ -694,6 +798,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(mismatch_idx),
is_method,
);
suggest_confusable(&mut err);
return err.emit();
}
}
Expand All @@ -718,7 +823,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if cfg!(debug_assertions) {
span_bug!(error_span, "expected errors from argument matrix");
} else {
return tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span });
let mut err =
tcx.dcx().create_err(errors::ArgMismatchIndeterminate { span: error_span });
suggest_confusable(&mut err);
return err.emit();
}
}

Expand All @@ -733,7 +841,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let trace =
mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) {
reported = Some(self.err_ctxt().report_and_explain_type_error(trace, *e).emit());
let mut err = self.err_ctxt().report_and_explain_type_error(trace, *e);
suggest_confusable(&mut err);
reported = Some(err.emit());
return false;
}
true
Expand Down Expand Up @@ -802,6 +912,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(expected_idx.as_usize()),
is_method,
);
suggest_confusable(&mut err);
return err.emit();
}

Expand Down Expand Up @@ -829,6 +940,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.with_code(err_code.to_owned())
};

suggest_confusable(&mut err);
// As we encounter issues, keep track of what we want to provide for the suggestion
let mut labels = vec![];
// If there is a single error, we give a specific suggestion; otherwise, we change to
Expand Down
26 changes: 22 additions & 4 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub use self::PickKind::*;
#[derive(Clone, Copy, Debug)]
pub struct IsSuggestion(pub bool);

struct ProbeContext<'a, 'tcx> {
pub(crate) struct ProbeContext<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
span: Span,
mode: Mode,
Expand Down Expand Up @@ -355,7 +355,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.unwrap()
}

fn probe_op<OP, R>(
pub(crate) fn probe_op<OP, R>(
&'a self,
span: Span,
mode: Mode,
Expand Down Expand Up @@ -1750,7 +1750,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
/// Similarly to `probe_for_return_type`, this method attempts to find the best matching
/// candidate method where the method name may have been misspelled. Similarly to other
/// edit distance based suggestions, we provide at most one such suggestion.
fn probe_for_similar_candidate(&mut self) -> Result<Option<ty::AssocItem>, MethodError<'tcx>> {
pub(crate) fn probe_for_similar_candidate(
&mut self,
) -> Result<Option<ty::AssocItem>, MethodError<'tcx>> {
debug!("probing for method names similar to {:?}", self.method_name);

self.probe(|_| {
Expand All @@ -1766,6 +1768,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
);
pcx.allow_similar_names = true;
pcx.assemble_inherent_candidates();
pcx.assemble_extension_candidates_for_all_traits();

let method_names = pcx.candidate_method_names(|_| true);
pcx.allow_similar_names = false;
Expand All @@ -1775,6 +1778,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
pcx.reset();
pcx.method_name = Some(method_name);
pcx.assemble_inherent_candidates();
pcx.assemble_extension_candidates_for_all_traits();
pcx.pick_core().and_then(|pick| pick.ok()).map(|pick| pick.item)
})
.collect();
Expand Down Expand Up @@ -1942,7 +1946,21 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let hir_id = self.fcx.tcx.local_def_id_to_hir_id(local_def_id);
let attrs = self.fcx.tcx.hir().attrs(hir_id);
for attr in attrs {
let sym::doc = attr.name_or_empty() else {
if sym::doc == attr.name_or_empty() {
} else if sym::rustc_confusables == attr.name_or_empty() {
let Some(confusables) = attr.meta_item_list() else {
continue;
};
// #[rustc_confusables("foo", "bar"))]
for n in confusables {
if let Some(lit) = n.lit()
&& name.as_str() == lit.symbol.as_str()
{
return true;
}
}
continue;
} else {
continue;
};
let Some(values) = attr.meta_item_list() else {
Expand Down
Loading

0 comments on commit a28d221

Please sign in to comment.