From 36c9d5ce17f352b8228f4f973118a3c6b543d4eb Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Thu, 4 May 2023 16:03:36 +0330 Subject: [PATCH] Fix pattern type mismatch in tuples --- crates/hir-ty/src/diagnostics/match_check.rs | 1 + crates/hir-ty/src/infer/pat.rs | 25 +++++----- crates/hir-ty/src/tests.rs | 19 ++++--- crates/hir-ty/src/tests/simple.rs | 50 +++++++++++++++++++ crates/hir/src/lib.rs | 3 -- crates/hir/src/semantics.rs | 11 ++++ crates/hir/src/source_analyzer.rs | 23 ++++++++- .../src/handlers/destructure_tuple_binding.rs | 2 +- .../src/handlers/merge_match_arms.rs | 28 +++++------ .../src/handlers/missing_match_arms.rs | 16 +++--- .../src/handlers/type_mismatch.rs | 3 ++ crates/ide/src/inlay_hints/bind_pat.rs | 2 +- crates/ide/src/inlay_hints/binding_mode.rs | 1 - 13 files changed, 139 insertions(+), 45 deletions(-) diff --git a/crates/hir-ty/src/diagnostics/match_check.rs b/crates/hir-ty/src/diagnostics/match_check.rs index 202f4aa66bc..125df2ba761 100644 --- a/crates/hir-ty/src/diagnostics/match_check.rs +++ b/crates/hir-ty/src/diagnostics/match_check.rs @@ -148,6 +148,7 @@ fn lower_pattern_unadjusted(&mut self, pat: PatId) -> Pat { hir_def::hir::Pat::Bind { id, subpat, .. } => { let bm = self.infer.pat_binding_modes[&pat]; + ty = &self.infer[id]; let name = &self.body.bindings[id].name; match (bm, ty.kind(Interner)) { (BindingMode::Ref(_), TyKind::Ref(.., rty)) => ty = rty, diff --git a/crates/hir-ty/src/infer/pat.rs b/crates/hir-ty/src/infer/pat.rs index ce179210d34..57480395110 100644 --- a/crates/hir-ty/src/infer/pat.rs +++ b/crates/hir-ty/src/infer/pat.rs @@ -263,7 +263,7 @@ fn infer_pat(&mut self, pat: PatId, expected: &Ty, mut default_bm: BindingMode) // Don't emit type mismatches again, the expression lowering already did that. let ty = self.infer_lit_pat(expr, &expected); self.write_pat_ty(pat, ty.clone()); - return ty; + return self.pat_ty_after_adjustment(pat); } Pat::Box { inner } => match self.resolve_boxed_box() { Some(box_adt) => { @@ -298,8 +298,17 @@ fn infer_pat(&mut self, pat: PatId, expected: &Ty, mut default_bm: BindingMode) .type_mismatches .insert(pat.into(), TypeMismatch { expected, actual: ty.clone() }); } - self.write_pat_ty(pat, ty.clone()); - ty + self.write_pat_ty(pat, ty); + self.pat_ty_after_adjustment(pat) + } + + fn pat_ty_after_adjustment(&self, pat: PatId) -> Ty { + self.result + .pat_adjustments + .get(&pat) + .and_then(|x| x.first()) + .unwrap_or(&self.result.type_of_pat[pat]) + .clone() } fn infer_ref_pat( @@ -345,7 +354,7 @@ fn infer_bind_pat( } BindingMode::Move => inner_ty.clone(), }; - self.write_pat_ty(pat, bound_ty.clone()); + self.write_pat_ty(pat, inner_ty.clone()); self.write_binding_ty(binding, bound_ty); return inner_ty; } @@ -422,14 +431,6 @@ fn is_non_ref_pat(body: &hir_def::body::Body, pat: PatId) -> bool { Pat::Lit(expr) => { !matches!(body[*expr], Expr::Literal(Literal::String(..) | Literal::ByteString(..))) } - Pat::Bind { id, subpat: Some(subpat), .. } - if matches!( - body.bindings[*id].mode, - BindingAnnotation::Mutable | BindingAnnotation::Unannotated - ) => - { - is_non_ref_pat(body, *subpat) - } Pat::Wild | Pat::Bind { .. } | Pat::Ref { .. } | Pat::Box { .. } | Pat::Missing => false, } } diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs index a2adad31696..2db04024b7b 100644 --- a/crates/hir-ty/src/tests.rs +++ b/crates/hir-ty/src/tests.rs @@ -17,7 +17,7 @@ use hir_def::{ body::{Body, BodySourceMap, SyntheticSyntax}, db::{DefDatabase, InternDatabase}, - hir::{ExprId, PatId}, + hir::{ExprId, Pat, PatId}, item_scope::ItemScope, nameres::DefMap, src::HasSource, @@ -149,10 +149,13 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour }); let mut unexpected_type_mismatches = String::new(); for def in defs { - let (_body, body_source_map) = db.body_with_source_map(def); + let (body, body_source_map) = db.body_with_source_map(def); let inference_result = db.infer(def); - for (pat, ty) in inference_result.type_of_pat.iter() { + for (pat, mut ty) in inference_result.type_of_pat.iter() { + if let Pat::Bind { id, .. } = body.pats[pat] { + ty = &inference_result.type_of_binding[id]; + } let node = match pat_node(&body_source_map, pat, &db) { Some(value) => value, None => continue, @@ -284,11 +287,15 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String { let mut buf = String::new(); let mut infer_def = |inference_result: Arc, + body: Arc, body_source_map: Arc| { let mut types: Vec<(InFile, &Ty)> = Vec::new(); let mut mismatches: Vec<(InFile, &TypeMismatch)> = Vec::new(); - for (pat, ty) in inference_result.type_of_pat.iter() { + for (pat, mut ty) in inference_result.type_of_pat.iter() { + if let Pat::Bind { id, .. } = body.pats[pat] { + ty = &inference_result.type_of_binding[id]; + } let syntax_ptr = match body_source_map.pat_syntax(pat) { Ok(sp) => { let root = db.parse_or_expand(sp.file_id); @@ -386,9 +393,9 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String { } }); for def in defs { - let (_body, source_map) = db.body_with_source_map(def); + let (body, source_map) = db.body_with_source_map(def); let infer = db.infer(def); - infer_def(infer, source_map); + infer_def(infer, body, source_map); } buf.truncate(buf.trim_end().len()); diff --git a/crates/hir-ty/src/tests/simple.rs b/crates/hir-ty/src/tests/simple.rs index 2b14a28a672..72e98138a34 100644 --- a/crates/hir-ty/src/tests/simple.rs +++ b/crates/hir-ty/src/tests/simple.rs @@ -2033,6 +2033,56 @@ fn test() { ); } +#[test] +fn tuple_pattern_nested_match_ergonomics() { + check_no_mismatches( + r#" +fn f(x: (&i32, &i32)) -> i32 { + match x { + (3, 4) => 5, + _ => 12, + } +} + "#, + ); + check_types( + r#" +fn f(x: (&&&&i32, &&&i32)) { + let f = match x { + t @ (3, 4) => t, + _ => loop {}, + }; + f; + //^ (&&&&i32, &&&i32) +} + "#, + ); + check_types( + r#" +fn f() { + let x = &&&(&&&2, &&&&&3); + let (y, z) = x; + //^ &&&&i32 + let t @ (y, z) = x; + t; + //^ &&&(&&&i32, &&&&&i32) +} + "#, + ); + check_types( + r#" +fn f() { + let x = &&&(&&&2, &&&&&3); + let (y, z) = x; + //^ &&&&i32 + let t @ (y, z) = x; + t; + //^ &&&(&&&i32, &&&&&i32) +} + "#, + ); +} + #[test] fn fn_pointer_return() { check_infer( diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d9cb2a7b6da..b3a8a33cac9 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1535,9 +1535,6 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { for (pat_or_expr, mismatch) in infer.type_mismatches() { let expr_or_pat = match pat_or_expr { ExprOrPatId::ExprId(expr) => source_map.expr_syntax(expr).map(Either::Left), - // FIXME: Re-enable these once we have less false positives - ExprOrPatId::PatId(_pat) => continue, - #[allow(unreachable_patterns)] ExprOrPatId::PatId(pat) => source_map.pat_syntax(pat).map(Either::Right), }; let expr_or_pat = match expr_or_pat { diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index a9f78131501..81ea99522f7 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -350,6 +350,13 @@ pub fn type_of_pat(&self, pat: &ast::Pat) -> Option { self.imp.type_of_pat(pat) } + /// It also includes the changes that binding mode makes in the type. For example in + /// `let ref x @ Some(_) = None` the result of `type_of_pat` is `Option` but the result + /// of this function is `&mut Option` + pub fn type_of_binding_in_pat(&self, pat: &ast::IdentPat) -> Option { + self.imp.type_of_binding_in_pat(pat) + } + pub fn type_of_self(&self, param: &ast::SelfParam) -> Option { self.imp.type_of_self(param) } @@ -1138,6 +1145,10 @@ fn type_of_pat(&self, pat: &ast::Pat) -> Option { .map(|(ty, coerced)| TypeInfo { original: ty, adjusted: coerced }) } + fn type_of_binding_in_pat(&self, pat: &ast::IdentPat) -> Option { + self.analyze(pat.syntax())?.type_of_binding_in_pat(self.db, pat) + } + fn type_of_self(&self, param: &ast::SelfParam) -> Option { self.analyze(param.syntax())?.type_of_self(self.db, param) } diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 19179d11ef8..159601955f8 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -13,7 +13,7 @@ scope::{ExprScopes, ScopeId}, Body, BodySourceMap, }, - hir::{ExprId, Pat, PatId}, + hir::{BindingId, ExprId, Pat, PatId}, lang_item::LangItem, lower::LowerCtx, macro_id_to_def_id, @@ -133,6 +133,15 @@ fn pat_id(&self, pat: &ast::Pat) -> Option { self.body_source_map()?.node_pat(src) } + fn binding_id_of_pat(&self, pat: &ast::IdentPat) -> Option { + let pat_id = self.pat_id(&pat.clone().into())?; + if let Pat::Bind { id, .. } = self.body()?.pats[pat_id] { + Some(id) + } else { + None + } + } + fn expand_expr( &self, db: &dyn HirDatabase, @@ -198,6 +207,18 @@ pub(crate) fn type_of_pat( Some((mk_ty(ty), coerced.map(mk_ty))) } + pub(crate) fn type_of_binding_in_pat( + &self, + db: &dyn HirDatabase, + pat: &ast::IdentPat, + ) -> Option { + let binding_id = self.binding_id_of_pat(pat)?; + let infer = self.infer.as_ref()?; + let ty = infer[binding_id].clone(); + let mk_ty = |ty| Type::new_with_resolver(db, &self.resolver, ty); + Some(mk_ty(ty)) + } + pub(crate) fn type_of_self( &self, db: &dyn HirDatabase, diff --git a/crates/ide-assists/src/handlers/destructure_tuple_binding.rs b/crates/ide-assists/src/handlers/destructure_tuple_binding.rs index 31c2ce7c1b5..ea71d165e6a 100644 --- a/crates/ide-assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide-assists/src/handlers/destructure_tuple_binding.rs @@ -91,7 +91,7 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext<'_>) -> Option bool { } fn are_same_types( - current_arm_types: &HashMap>, + current_arm_types: &HashMap>, arm: &ast::MatchArm, ctx: &AssistContext<'_>, ) -> bool { @@ -103,7 +103,7 @@ fn are_same_types( for (other_arm_type_name, other_arm_type) in arm_types { match (current_arm_types.get(&other_arm_type_name), other_arm_type) { (Some(Some(current_arm_type)), Some(other_arm_type)) - if other_arm_type.original == current_arm_type.original => {} + if other_arm_type == *current_arm_type => {} _ => return false, } } @@ -114,44 +114,44 @@ fn are_same_types( fn get_arm_types( context: &AssistContext<'_>, arm: &ast::MatchArm, -) -> HashMap> { - let mut mapping: HashMap> = HashMap::new(); +) -> HashMap> { + let mut mapping: HashMap> = HashMap::new(); fn recurse( - map: &mut HashMap>, + map: &mut HashMap>, ctx: &AssistContext<'_>, pat: &Option, ) { if let Some(local_pat) = pat { - match pat { - Some(ast::Pat::TupleStructPat(tuple)) => { + match local_pat { + ast::Pat::TupleStructPat(tuple) => { for field in tuple.fields() { recurse(map, ctx, &Some(field)); } } - Some(ast::Pat::TuplePat(tuple)) => { + ast::Pat::TuplePat(tuple) => { for field in tuple.fields() { recurse(map, ctx, &Some(field)); } } - Some(ast::Pat::RecordPat(record)) => { + ast::Pat::RecordPat(record) => { if let Some(field_list) = record.record_pat_field_list() { for field in field_list.fields() { recurse(map, ctx, &field.pat()); } } } - Some(ast::Pat::ParenPat(parentheses)) => { + ast::Pat::ParenPat(parentheses) => { recurse(map, ctx, &parentheses.pat()); } - Some(ast::Pat::SlicePat(slice)) => { + ast::Pat::SlicePat(slice) => { for slice_pat in slice.pats() { recurse(map, ctx, &Some(slice_pat)); } } - Some(ast::Pat::IdentPat(ident_pat)) => { + ast::Pat::IdentPat(ident_pat) => { if let Some(name) = ident_pat.name() { - let pat_type = ctx.sema.type_of_pat(local_pat); + let pat_type = ctx.sema.type_of_binding_in_pat(ident_pat); map.insert(name.text().to_string(), pat_type); } } diff --git a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs index ac4463331f2..b5e619e2a03 100644 --- a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs +++ b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs @@ -271,15 +271,20 @@ enum Either2 { C, D } fn main() { match Either::A { Either2::C => (), + //^^^^^^^^^^ error: expected Either, found Either2 Either2::D => (), + //^^^^^^^^^^ error: expected Either, found Either2 } match (true, false) { (true, false, true) => (), + //^^^^^^^^^^^^^^^^^^^ error: expected (bool, bool), found (bool, bool, bool) (true) => (), // ^^^^ error: expected (bool, bool), found bool } match (true, false) { (true,) => {} } + //^^^^^^^ error: expected (bool, bool), found (bool,) match (0) { () => () } + //^^ error: expected i32, found () match Unresolved::Bar { Unresolved::Baz => () } } "#, @@ -293,7 +298,9 @@ fn mismatched_types_in_or_patterns() { r#" fn main() { match false { true | () => {} } + //^^ error: expected bool, found () match (false,) { (true | (),) => {} } + //^^ error: expected bool, found () } "#, ); @@ -738,17 +745,13 @@ fn main() { #[test] fn binding_ref_has_correct_type() { - cov_mark::check_count!(validate_match_bailed_out, 1); - // Asserts `PatKind::Binding(ref _x): bool`, not &bool. // If that's not true match checking will panic with "incompatible constructors" // FIXME: make facilities to test this directly like `tests::check_infer(..)` - check_diagnostics( + check_diagnostics_no_bails( r#" enum Foo { A } fn main() { - // FIXME: this should not bail out but current behavior is such as the old algorithm. - // ExprValidator::validate_match(..) checks types of top level patterns incorrectly. match Foo::A { ref _x => {} Foo::A => {} @@ -1035,11 +1038,12 @@ fn main() { #[test] fn reference_patterns_in_fields() { - cov_mark::check_count!(validate_match_bailed_out, 2); + cov_mark::check_count!(validate_match_bailed_out, 1); check_diagnostics( r#" fn main() { match (&false,) { + //^^^^^^^^^ error: missing match arm: `(&false,)` not covered (true,) => {} } match (&false,) { diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index b49141b55b7..fee160c3e7f 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -650,8 +650,11 @@ fn type_mismatch_pat_smoke_test() { r#" fn f() { let &() = &mut (); + //^^^ error: expected &mut (), found &() match &() { + // FIXME: we should only show the deep one. &9 => () + //^^ error: expected &(), found &i32 //^ error: expected (), found i32 } } diff --git a/crates/ide/src/inlay_hints/bind_pat.rs b/crates/ide/src/inlay_hints/bind_pat.rs index a131427f5fd..6991a66c7c2 100644 --- a/crates/ide/src/inlay_hints/bind_pat.rs +++ b/crates/ide/src/inlay_hints/bind_pat.rs @@ -30,7 +30,7 @@ pub(super) fn hints( let descended = sema.descend_node_into_attributes(pat.clone()).pop(); let desc_pat = descended.as_ref().unwrap_or(pat); - let ty = sema.type_of_pat(&desc_pat.clone().into())?.original; + let ty = sema.type_of_binding_in_pat(desc_pat)?; if should_not_display_type_hint(sema, config, pat, &ty) { return None; diff --git a/crates/ide/src/inlay_hints/binding_mode.rs b/crates/ide/src/inlay_hints/binding_mode.rs index 3d7f969aaab..dada4dd0485 100644 --- a/crates/ide/src/inlay_hints/binding_mode.rs +++ b/crates/ide/src/inlay_hints/binding_mode.rs @@ -148,7 +148,6 @@ struct Struct { field: &'static str, } fn foo(s @ Struct { field, .. }: &Struct) {} - //^^^^^^^^^^^^^^^^^^^^^^^^ref //^^^^^^^^^^^^^^^^^^^^& //^^^^^ref "#,