From 429e0284e8e2f9a1c194d9c25e7e600685056322 Mon Sep 17 00:00:00 2001 From: Francis McKenzie Date: Fri, 28 Jun 2024 09:04:09 +0800 Subject: [PATCH] Fix for #17497 - Invalid RA diagnostic error: expected 2 arguments, found 1 Also fixes #17233 The issue occurs because in some configurations of traits where one of them has Deref as a supertrait, RA's type inference algorithm fails to fully resolve a trait's self type, and instead uses a TyKind::BoundVar. This then erroneously matches with types that do not implement the trait. In other words, unconnected types appear to inherit the trait's methods. The fix is to insert a heuristic when invoking lookup_method() that allows the caller to determine if the returned method is a good match. If the returned method would result in diagnostic errors, the caller instructs the algorithm to continue iterating the possible methods to see if a better one exists (i.e. one that won't result in diagnostic errors). Includes a unit test that only passes after applying the fixes in this commit. --- crates/hir-ty/src/infer/expr.rs | 54 ++++++++++++++++++-- crates/hir-ty/src/method_resolution.rs | 36 ++++++++++++- crates/hir-ty/src/tests/method_resolution.rs | 39 ++++++++++++++ 3 files changed, 122 insertions(+), 7 deletions(-) diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 364724353704..96ec6f5da1a8 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -378,6 +378,7 @@ impl InferenceContext<'_> { ¶m_tys, &indices_to_skip, is_varargs, + false, ); self.normalize_associated_types_in(ret_ty) } @@ -1615,14 +1616,22 @@ impl InferenceContext<'_> { ) -> Ty { let receiver_ty = self.infer_expr_inner(receiver, &Expectation::none()); let canonicalized_receiver = self.canonicalize(receiver_ty.clone()); + let traits_in_scope = self.get_traits_in_scope().left_or_else(|it| it.clone()); - let resolved = method_resolution::lookup_method( + let resolved = method_resolution::lookup_method_using_heuristic( self.db, &canonicalized_receiver, self.table.trait_env.clone(), - self.get_traits_in_scope().as_ref().left_or_else(|&it| it), + &traits_in_scope, VisibleFromModule::Filter(self.resolver.module()), method_name, + |adjust, func| { + let (receiver_ty, _) = adjust.apply(&mut self.table, receiver_ty.clone()); + let generics = generics(self.db.upcast(), func.into()); + let substs = self.substs_for_method_call(generics, generic_args); + let method_ty = self.db.value_ty(func.into()).unwrap(); + self.try_method_call(tgt_expr.clone(), args, method_ty, substs, receiver_ty, expected) + } ); let (receiver_ty, method_ty, substs) = match resolved { Some((adjust, func, visible)) => { @@ -1684,6 +1693,21 @@ impl InferenceContext<'_> { self.check_method_call(tgt_expr, args, method_ty, substs, receiver_ty, expected) } + #[inline] + fn try_method_call( + &mut self, + tgt_expr: ExprId, + args: &[ExprId], + method_ty: Binders, + substs: Substitution, + receiver_ty: Ty, + expected: &Expectation, + ) -> bool { + self.check_method_call_inner(tgt_expr, args, method_ty, substs, receiver_ty, expected, true) + .is_ok() + } + + #[inline] fn check_method_call( &mut self, tgt_expr: ExprId, @@ -1693,6 +1717,20 @@ impl InferenceContext<'_> { receiver_ty: Ty, expected: &Expectation, ) -> Ty { + self.check_method_call_inner(tgt_expr, args, method_ty, substs, receiver_ty, expected, false) + .unwrap() + } + + fn check_method_call_inner( + &mut self, + tgt_expr: ExprId, + args: &[ExprId], + method_ty: Binders, + substs: Substitution, + receiver_ty: Ty, + expected: &Expectation, + is_dryrun: bool, + ) -> Result { let method_ty = method_ty.substitute(Interner, &substs); self.register_obligations_for_call(&method_ty); let ((formal_receiver_ty, param_tys), ret_ty, is_varargs) = @@ -1713,8 +1751,10 @@ impl InferenceContext<'_> { let expected_inputs = self.expected_inputs_for_expected_output(expected, ret_ty.clone(), param_tys.clone()); - self.check_call_arguments(tgt_expr, args, &expected_inputs, ¶m_tys, &[], is_varargs); - self.normalize_associated_types_in(ret_ty) + if !self.check_call_arguments(tgt_expr, args, &expected_inputs, ¶m_tys, &[], is_varargs, is_dryrun) { + if is_dryrun { return Err(()); } + } + Ok(self.normalize_associated_types_in(ret_ty)) } fn expected_inputs_for_expected_output( @@ -1752,8 +1792,10 @@ impl InferenceContext<'_> { param_tys: &[Ty], skip_indices: &[u32], is_varargs: bool, - ) { + is_dryrun: bool, + ) -> bool { if args.len() != param_tys.len() + skip_indices.len() && !is_varargs { + if is_dryrun { return false; } self.push_diagnostic(InferenceDiagnostic::MismatchedArgCount { call_expr: expr, expected: param_tys.len() + skip_indices.len(), @@ -1814,6 +1856,7 @@ impl InferenceContext<'_> { // type vars here to avoid type mismatch false positive. let coercion_target = self.insert_type_vars(coercion_target); if self.coerce(Some(arg), &ty, &coercion_target).is_err() { + if is_dryrun { return false; } self.result.type_mismatches.insert( arg.into(), TypeMismatch { expected: coercion_target, actual: ty.clone() }, @@ -1821,6 +1864,7 @@ impl InferenceContext<'_> { } } } + true } fn substs_for_method_call( diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index ac11da789a1f..fdb8cd77bf71 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -437,6 +437,7 @@ pub fn def_crates( } /// Look up the method with the given name. +#[inline] pub(crate) fn lookup_method( db: &dyn HirDatabase, ty: &Canonical, @@ -444,8 +445,30 @@ pub(crate) fn lookup_method( traits_in_scope: &FxHashSet, visible_from_module: VisibleFromModule, name: &Name, +) -> Option<(ReceiverAdjustments, FunctionId, bool)> { + lookup_method_using_heuristic( + db, + ty, + env, + traits_in_scope, + visible_from_module, + name, + |_,_| true, + ) +} + +/// Look up the method with the given name, using a callback to evaluate matches. +pub(crate) fn lookup_method_using_heuristic( + db: &dyn HirDatabase, + ty: &Canonical, + env: Arc, + traits_in_scope: &FxHashSet, + visible_from_module: VisibleFromModule, + name: &Name, + mut is_best_match: impl FnMut(&ReceiverAdjustments, FunctionId) -> bool, ) -> Option<(ReceiverAdjustments, FunctionId, bool)> { let mut not_visible = None; + let mut not_best_match = None; let res = iterate_method_candidates( ty, db, @@ -455,7 +478,16 @@ pub(crate) fn lookup_method( Some(name), LookupMode::MethodCall, |adjustments, f, visible| match f { - AssocItemId::FunctionId(f) if visible => Some((adjustments, f, true)), + AssocItemId::FunctionId(f) if visible => { + if is_best_match(&adjustments, f) { + Some((adjustments, f, true)) + } else { + if not_best_match.is_none() { + not_best_match = Some((adjustments, f, true)); + } + None + } + }, AssocItemId::FunctionId(f) if not_visible.is_none() => { not_visible = Some((adjustments, f, false)); None @@ -463,7 +495,7 @@ pub(crate) fn lookup_method( _ => None, }, ); - res.or(not_visible) + res.or(not_best_match).or(not_visible) } /// Whether we're looking up a dotted method call (like `v.len()`) or a path diff --git a/crates/hir-ty/src/tests/method_resolution.rs b/crates/hir-ty/src/tests/method_resolution.rs index 63a83d403fa9..d34ac167c4d4 100644 --- a/crates/hir-ty/src/tests/method_resolution.rs +++ b/crates/hir-ty/src/tests/method_resolution.rs @@ -2050,3 +2050,42 @@ fn test() { "#, ); } + +#[test] +fn mismatched_args_due_to_subtraits_with_deref() { + check_no_mismatches( + r#" +//- minicore: deref +use core::ops::Deref; + +trait Trait1 { + type Assoc: Deref; +} + +trait Trait2: Trait1 { +} + +trait Trait3 { + type T1: Trait1; + type T2: Trait2; + fn bar(&self, x: bool, y: bool); +} + +struct Foo; + +impl Foo { + fn bar(&mut self, _: &'static str) {} +} + +impl Deref for Foo { + type Target = u32; + fn deref(&self) -> &Self::Target { &0 } +} + +fn problem_method() { + let mut foo = Foo; + foo.bar("hello"); // Rustc ok, RA errors (mismatched args) +} +"#, + ); +}