Fix pattern type mismatch in tuples

This commit is contained in:
hkalbasi 2023-05-04 16:03:36 +03:30
parent 9811a3af5f
commit 36c9d5ce17
13 changed files with 139 additions and 45 deletions

View file

@ -148,6 +148,7 @@ impl<'a> PatCtxt<'a> {
hir_def::hir::Pat::Bind { id, subpat, .. } => { hir_def::hir::Pat::Bind { id, subpat, .. } => {
let bm = self.infer.pat_binding_modes[&pat]; let bm = self.infer.pat_binding_modes[&pat];
ty = &self.infer[id];
let name = &self.body.bindings[id].name; let name = &self.body.bindings[id].name;
match (bm, ty.kind(Interner)) { match (bm, ty.kind(Interner)) {
(BindingMode::Ref(_), TyKind::Ref(.., rty)) => ty = rty, (BindingMode::Ref(_), TyKind::Ref(.., rty)) => ty = rty,

View file

@ -263,7 +263,7 @@ impl<'a> InferenceContext<'a> {
// Don't emit type mismatches again, the expression lowering already did that. // Don't emit type mismatches again, the expression lowering already did that.
let ty = self.infer_lit_pat(expr, &expected); let ty = self.infer_lit_pat(expr, &expected);
self.write_pat_ty(pat, ty.clone()); self.write_pat_ty(pat, ty.clone());
return ty; return self.pat_ty_after_adjustment(pat);
} }
Pat::Box { inner } => match self.resolve_boxed_box() { Pat::Box { inner } => match self.resolve_boxed_box() {
Some(box_adt) => { Some(box_adt) => {
@ -298,8 +298,17 @@ impl<'a> InferenceContext<'a> {
.type_mismatches .type_mismatches
.insert(pat.into(), TypeMismatch { expected, actual: ty.clone() }); .insert(pat.into(), TypeMismatch { expected, actual: ty.clone() });
} }
self.write_pat_ty(pat, ty.clone()); self.write_pat_ty(pat, ty);
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( fn infer_ref_pat(
@ -345,7 +354,7 @@ impl<'a> InferenceContext<'a> {
} }
BindingMode::Move => inner_ty.clone(), 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); self.write_binding_ty(binding, bound_ty);
return inner_ty; return inner_ty;
} }
@ -422,14 +431,6 @@ fn is_non_ref_pat(body: &hir_def::body::Body, pat: PatId) -> bool {
Pat::Lit(expr) => { Pat::Lit(expr) => {
!matches!(body[*expr], Expr::Literal(Literal::String(..) | Literal::ByteString(..))) !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, Pat::Wild | Pat::Bind { .. } | Pat::Ref { .. } | Pat::Box { .. } | Pat::Missing => false,
} }
} }

View file

@ -17,7 +17,7 @@ use expect_test::Expect;
use hir_def::{ use hir_def::{
body::{Body, BodySourceMap, SyntheticSyntax}, body::{Body, BodySourceMap, SyntheticSyntax},
db::{DefDatabase, InternDatabase}, db::{DefDatabase, InternDatabase},
hir::{ExprId, PatId}, hir::{ExprId, Pat, PatId},
item_scope::ItemScope, item_scope::ItemScope,
nameres::DefMap, nameres::DefMap,
src::HasSource, 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(); let mut unexpected_type_mismatches = String::new();
for def in defs { 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); 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) { let node = match pat_node(&body_source_map, pat, &db) {
Some(value) => value, Some(value) => value,
None => continue, None => continue,
@ -284,11 +287,15 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String {
let mut buf = String::new(); let mut buf = String::new();
let mut infer_def = |inference_result: Arc<InferenceResult>, let mut infer_def = |inference_result: Arc<InferenceResult>,
body: Arc<Body>,
body_source_map: Arc<BodySourceMap>| { body_source_map: Arc<BodySourceMap>| {
let mut types: Vec<(InFile<SyntaxNode>, &Ty)> = Vec::new(); let mut types: Vec<(InFile<SyntaxNode>, &Ty)> = Vec::new();
let mut mismatches: Vec<(InFile<SyntaxNode>, &TypeMismatch)> = Vec::new(); let mut mismatches: Vec<(InFile<SyntaxNode>, &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) { let syntax_ptr = match body_source_map.pat_syntax(pat) {
Ok(sp) => { Ok(sp) => {
let root = db.parse_or_expand(sp.file_id); 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 { 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); let infer = db.infer(def);
infer_def(infer, source_map); infer_def(infer, body, source_map);
} }
buf.truncate(buf.trim_end().len()); buf.truncate(buf.trim_end().len());

View file

@ -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] #[test]
fn fn_pointer_return() { fn fn_pointer_return() {
check_infer( check_infer(

View file

@ -1535,9 +1535,6 @@ impl DefWithBody {
for (pat_or_expr, mismatch) in infer.type_mismatches() { for (pat_or_expr, mismatch) in infer.type_mismatches() {
let expr_or_pat = match pat_or_expr { let expr_or_pat = match pat_or_expr {
ExprOrPatId::ExprId(expr) => source_map.expr_syntax(expr).map(Either::Left), 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), ExprOrPatId::PatId(pat) => source_map.pat_syntax(pat).map(Either::Right),
}; };
let expr_or_pat = match expr_or_pat { let expr_or_pat = match expr_or_pat {

View file

@ -350,6 +350,13 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
self.imp.type_of_pat(pat) 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<T>` but the result
/// of this function is `&mut Option<T>`
pub fn type_of_binding_in_pat(&self, pat: &ast::IdentPat) -> Option<Type> {
self.imp.type_of_binding_in_pat(pat)
}
pub fn type_of_self(&self, param: &ast::SelfParam) -> Option<Type> { pub fn type_of_self(&self, param: &ast::SelfParam) -> Option<Type> {
self.imp.type_of_self(param) self.imp.type_of_self(param)
} }
@ -1138,6 +1145,10 @@ impl<'db> SemanticsImpl<'db> {
.map(|(ty, coerced)| TypeInfo { original: ty, adjusted: coerced }) .map(|(ty, coerced)| TypeInfo { original: ty, adjusted: coerced })
} }
fn type_of_binding_in_pat(&self, pat: &ast::IdentPat) -> Option<Type> {
self.analyze(pat.syntax())?.type_of_binding_in_pat(self.db, pat)
}
fn type_of_self(&self, param: &ast::SelfParam) -> Option<Type> { fn type_of_self(&self, param: &ast::SelfParam) -> Option<Type> {
self.analyze(param.syntax())?.type_of_self(self.db, param) self.analyze(param.syntax())?.type_of_self(self.db, param)
} }

View file

@ -13,7 +13,7 @@ use hir_def::{
scope::{ExprScopes, ScopeId}, scope::{ExprScopes, ScopeId},
Body, BodySourceMap, Body, BodySourceMap,
}, },
hir::{ExprId, Pat, PatId}, hir::{BindingId, ExprId, Pat, PatId},
lang_item::LangItem, lang_item::LangItem,
lower::LowerCtx, lower::LowerCtx,
macro_id_to_def_id, macro_id_to_def_id,
@ -133,6 +133,15 @@ impl SourceAnalyzer {
self.body_source_map()?.node_pat(src) self.body_source_map()?.node_pat(src)
} }
fn binding_id_of_pat(&self, pat: &ast::IdentPat) -> Option<BindingId> {
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( fn expand_expr(
&self, &self,
db: &dyn HirDatabase, db: &dyn HirDatabase,
@ -198,6 +207,18 @@ impl SourceAnalyzer {
Some((mk_ty(ty), coerced.map(mk_ty))) Some((mk_ty(ty), coerced.map(mk_ty)))
} }
pub(crate) fn type_of_binding_in_pat(
&self,
db: &dyn HirDatabase,
pat: &ast::IdentPat,
) -> Option<Type> {
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( pub(crate) fn type_of_self(
&self, &self,
db: &dyn HirDatabase, db: &dyn HirDatabase,

View file

@ -91,7 +91,7 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext<'_>) -> Option<TupleDat
return None; return None;
} }
let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?.adjusted(); let ty = ctx.sema.type_of_binding_in_pat(&ident_pat)?;
let ref_type = if ty.is_mutable_reference() { let ref_type = if ty.is_mutable_reference() {
Some(RefType::Mutable) Some(RefType::Mutable)
} else if ty.is_reference() { } else if ty.is_reference() {

View file

@ -1,4 +1,4 @@
use hir::TypeInfo; use hir::Type;
use std::{collections::HashMap, iter::successors}; use std::{collections::HashMap, iter::successors};
use syntax::{ use syntax::{
algo::neighbor, algo::neighbor,
@ -95,7 +95,7 @@ fn contains_placeholder(a: &ast::MatchArm) -> bool {
} }
fn are_same_types( fn are_same_types(
current_arm_types: &HashMap<String, Option<TypeInfo>>, current_arm_types: &HashMap<String, Option<Type>>,
arm: &ast::MatchArm, arm: &ast::MatchArm,
ctx: &AssistContext<'_>, ctx: &AssistContext<'_>,
) -> bool { ) -> bool {
@ -103,7 +103,7 @@ fn are_same_types(
for (other_arm_type_name, other_arm_type) in arm_types { for (other_arm_type_name, other_arm_type) in arm_types {
match (current_arm_types.get(&other_arm_type_name), other_arm_type) { match (current_arm_types.get(&other_arm_type_name), other_arm_type) {
(Some(Some(current_arm_type)), Some(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, _ => return false,
} }
} }
@ -114,44 +114,44 @@ fn are_same_types(
fn get_arm_types( fn get_arm_types(
context: &AssistContext<'_>, context: &AssistContext<'_>,
arm: &ast::MatchArm, arm: &ast::MatchArm,
) -> HashMap<String, Option<TypeInfo>> { ) -> HashMap<String, Option<Type>> {
let mut mapping: HashMap<String, Option<TypeInfo>> = HashMap::new(); let mut mapping: HashMap<String, Option<Type>> = HashMap::new();
fn recurse( fn recurse(
map: &mut HashMap<String, Option<TypeInfo>>, map: &mut HashMap<String, Option<Type>>,
ctx: &AssistContext<'_>, ctx: &AssistContext<'_>,
pat: &Option<ast::Pat>, pat: &Option<ast::Pat>,
) { ) {
if let Some(local_pat) = pat { if let Some(local_pat) = pat {
match pat { match local_pat {
Some(ast::Pat::TupleStructPat(tuple)) => { ast::Pat::TupleStructPat(tuple) => {
for field in tuple.fields() { for field in tuple.fields() {
recurse(map, ctx, &Some(field)); recurse(map, ctx, &Some(field));
} }
} }
Some(ast::Pat::TuplePat(tuple)) => { ast::Pat::TuplePat(tuple) => {
for field in tuple.fields() { for field in tuple.fields() {
recurse(map, ctx, &Some(field)); recurse(map, ctx, &Some(field));
} }
} }
Some(ast::Pat::RecordPat(record)) => { ast::Pat::RecordPat(record) => {
if let Some(field_list) = record.record_pat_field_list() { if let Some(field_list) = record.record_pat_field_list() {
for field in field_list.fields() { for field in field_list.fields() {
recurse(map, ctx, &field.pat()); recurse(map, ctx, &field.pat());
} }
} }
} }
Some(ast::Pat::ParenPat(parentheses)) => { ast::Pat::ParenPat(parentheses) => {
recurse(map, ctx, &parentheses.pat()); recurse(map, ctx, &parentheses.pat());
} }
Some(ast::Pat::SlicePat(slice)) => { ast::Pat::SlicePat(slice) => {
for slice_pat in slice.pats() { for slice_pat in slice.pats() {
recurse(map, ctx, &Some(slice_pat)); recurse(map, ctx, &Some(slice_pat));
} }
} }
Some(ast::Pat::IdentPat(ident_pat)) => { ast::Pat::IdentPat(ident_pat) => {
if let Some(name) = ident_pat.name() { 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); map.insert(name.text().to_string(), pat_type);
} }
} }

View file

@ -271,15 +271,20 @@ enum Either2 { C, D }
fn main() { fn main() {
match Either::A { match Either::A {
Either2::C => (), Either2::C => (),
//^^^^^^^^^^ error: expected Either, found Either2
Either2::D => (), Either2::D => (),
//^^^^^^^^^^ error: expected Either, found Either2
} }
match (true, false) { match (true, false) {
(true, false, true) => (), (true, false, true) => (),
//^^^^^^^^^^^^^^^^^^^ error: expected (bool, bool), found (bool, bool, bool)
(true) => (), (true) => (),
// ^^^^ error: expected (bool, bool), found bool // ^^^^ error: expected (bool, bool), found bool
} }
match (true, false) { (true,) => {} } match (true, false) { (true,) => {} }
//^^^^^^^ error: expected (bool, bool), found (bool,)
match (0) { () => () } match (0) { () => () }
//^^ error: expected i32, found ()
match Unresolved::Bar { Unresolved::Baz => () } match Unresolved::Bar { Unresolved::Baz => () }
} }
"#, "#,
@ -293,7 +298,9 @@ fn main() {
r#" r#"
fn main() { fn main() {
match false { true | () => {} } match false { true | () => {} }
//^^ error: expected bool, found ()
match (false,) { (true | (),) => {} } match (false,) { (true | (),) => {} }
//^^ error: expected bool, found ()
} }
"#, "#,
); );
@ -738,17 +745,13 @@ fn main() {
#[test] #[test]
fn binding_ref_has_correct_type() { fn binding_ref_has_correct_type() {
cov_mark::check_count!(validate_match_bailed_out, 1);
// Asserts `PatKind::Binding(ref _x): bool`, not &bool. // Asserts `PatKind::Binding(ref _x): bool`, not &bool.
// If that's not true match checking will panic with "incompatible constructors" // If that's not true match checking will panic with "incompatible constructors"
// FIXME: make facilities to test this directly like `tests::check_infer(..)` // FIXME: make facilities to test this directly like `tests::check_infer(..)`
check_diagnostics( check_diagnostics_no_bails(
r#" r#"
enum Foo { A } enum Foo { A }
fn main() { 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 { match Foo::A {
ref _x => {} ref _x => {}
Foo::A => {} Foo::A => {}
@ -1035,11 +1038,12 @@ fn main() {
#[test] #[test]
fn reference_patterns_in_fields() { 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( check_diagnostics(
r#" r#"
fn main() { fn main() {
match (&false,) { match (&false,) {
//^^^^^^^^^ error: missing match arm: `(&false,)` not covered
(true,) => {} (true,) => {}
} }
match (&false,) { match (&false,) {

View file

@ -650,8 +650,11 @@ fn h() {
r#" r#"
fn f() { fn f() {
let &() = &mut (); let &() = &mut ();
//^^^ error: expected &mut (), found &()
match &() { match &() {
// FIXME: we should only show the deep one.
&9 => () &9 => ()
//^^ error: expected &(), found &i32
//^ error: expected (), found i32 //^ error: expected (), found i32
} }
} }

View file

@ -30,7 +30,7 @@ pub(super) fn hints(
let descended = sema.descend_node_into_attributes(pat.clone()).pop(); let descended = sema.descend_node_into_attributes(pat.clone()).pop();
let desc_pat = descended.as_ref().unwrap_or(pat); 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) { if should_not_display_type_hint(sema, config, pat, &ty) {
return None; return None;

View file

@ -148,7 +148,6 @@ struct Struct {
field: &'static str, field: &'static str,
} }
fn foo(s @ Struct { field, .. }: &Struct) {} fn foo(s @ Struct { field, .. }: &Struct) {}
//^^^^^^^^^^^^^^^^^^^^^^^^ref
//^^^^^^^^^^^^^^^^^^^^& //^^^^^^^^^^^^^^^^^^^^&
//^^^^^ref //^^^^^ref
"#, "#,