From d1338354ca7afae7088f84756ed103ea94ce82f9 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Fri, 10 Apr 2020 20:14:53 -0700 Subject: fix match arm false positive --- crates/ra_hir_ty/src/_match.rs | 15 +++++++++------ crates/ra_hir_ty/src/expr.rs | 13 +++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 9e9a9d047..342a78b45 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1315,8 +1315,9 @@ mod tests { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] @@ -1330,8 +1331,9 @@ mod tests { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] @@ -1344,8 +1346,9 @@ mod tests { } "; - // Match arms with the incorrect type are filtered out. - check_diagnostic(content); + // Match statements with arms that don't match the + // expression pattern do not fire this diagnostic. + check_no_diagnostic(content); } #[test] diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 827b687de..03ef488b9 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -163,12 +163,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let mut seen = Matrix::empty(); for pat in pats { - // We skip any patterns whose type we cannot resolve. - // - // This could lead to false positives in this diagnostic, so - // it might be better to skip the entire diagnostic if we either - // cannot resolve a match arm or determine that the match arm has - // the wrong type. if let Some(pat_ty) = infer.type_of_pat.get(pat) { // We only include patterns whose type matches the type // of the match expression. If we had a InvalidMatchArmPattern @@ -191,8 +185,15 @@ impl<'a, 'b> ExprValidator<'a, 'b> { // to the matrix here. let v = PatStack::from_pattern(pat); seen.push(&cx, v); + continue; } } + + // If we can't resolve the type of a pattern, or the pattern type doesn't + // fit the match expression, we skip this diagnostic. Skipping the entire + // diagnostic rather than just not including this match arm is preferred + // to avoid the chance of false positives. + return; } match is_useful(&cx, &seen, &PatStack::from_wild()) { -- cgit v1.2.3 From 26e5bb0a4efbe894d33cde3c1bc3f712fcf33cde Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 11 Apr 2020 07:07:47 -0700 Subject: missing match arms add tests for match expression diverging --- crates/ra_hir_ty/src/_match.rs | 77 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 342a78b45..4be08e9a3 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1386,6 +1386,42 @@ mod tests { // we don't create a diagnostic). check_no_diagnostic(content); } + + #[test] + fn expr_diverges() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop {} { + Either::A => (), + Either::B => (), + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn expr_loop_with_break() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop { break Foo::A } { + Either::A => (), + Either::B => (), + } + } + "; + + check_no_diagnostic(content); + } } #[cfg(test)] @@ -1455,4 +1491,45 @@ mod false_negatives { // We do not currently handle patterns with internal `or`s. check_no_diagnostic(content); } + + #[test] + fn expr_diverges_missing_arm() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop {} { + Either::A => (), + } + } + "; + + // This is a false negative. + // Even though the match expression diverges, rustc fails + // to compile here since `Either::B` is missing. + check_no_diagnostic(content); + } + + #[test] + fn expr_loop_missing_arm() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop { break Foo::A } { + Either::A => (), + } + } + "; + + // This is a false negative. + // We currently infer the type of `loop { break Foo::A }` to `!`, which + // causes us to skip the diagnostic since `Either::A` doesn't type check + // with `!`. + check_no_diagnostic(content); + } } -- cgit v1.2.3 From aec20e50946ea427ceb6a44451459f0cb3a84a4f Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 11 Apr 2020 07:13:47 -0700 Subject: missing match arm add test for partially diverging type --- crates/ra_hir_ty/src/_match.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 4be08e9a3..596194dcf 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1422,6 +1422,27 @@ mod tests { check_no_diagnostic(content); } + + #[test] + fn expr_partially_diverges() { + let content = r" + enum Either { + A(T), + B, + } + fn foo() -> Either { + Either::B + } + fn test_fn() -> u32 { + match foo() { + Either::A(val) => val, + Either::B => 0, + } + } + "; + + check_no_diagnostic(content); + } } #[cfg(test)] -- cgit v1.2.3 From 0f5d6766fd5b18716ede611060a69cbff9c2a35c Mon Sep 17 00:00:00 2001 From: Jeremy Kolb Date: Sat, 11 Apr 2020 15:47:09 -0400 Subject: Remove #[should_panic] from call_info tests --- crates/ra_ide/src/call_info.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/crates/ra_ide/src/call_info.rs b/crates/ra_ide/src/call_info.rs index ca57eceff..f95b6baf3 100644 --- a/crates/ra_ide/src/call_info.rs +++ b/crates/ra_ide/src/call_info.rs @@ -208,9 +208,20 @@ mod tests { } } - fn call_info(text: &str) -> CallInfo { + fn call_info_helper(text: &str) -> Option { let (analysis, position) = single_file_with_position(text); - analysis.call_info(position).unwrap().unwrap() + analysis.call_info(position).unwrap() + } + + fn call_info(text: &str) -> CallInfo { + let info = call_info_helper(text); + assert!(info.is_some()); + info.unwrap() + } + + fn no_call_info(text: &str) { + let info = call_info_helper(text); + assert!(info.is_none()); } #[test] @@ -558,9 +569,8 @@ fn main() { } #[test] - #[should_panic] fn cant_call_named_structs() { - let _ = call_info( + no_call_info( r#" struct TS { x: u32, y: i32 } fn main() { @@ -594,9 +604,8 @@ fn main() { } #[test] - #[should_panic] fn cant_call_enum_records() { - let _ = call_info( + no_call_info( r#" enum E { /// A Variant -- cgit v1.2.3 From 6b49e774e23c04a04ff5f377fc8dae25b5c69bb0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 11 Apr 2020 23:08:05 +0200 Subject: Remove dead code --- crates/ra_parser/src/grammar.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/ra_parser/src/grammar.rs b/crates/ra_parser/src/grammar.rs index d0530955e..c2a6e82e9 100644 --- a/crates/ra_parser/src/grammar.rs +++ b/crates/ra_parser/src/grammar.rs @@ -282,13 +282,10 @@ fn name_ref(p: &mut Parser) { } fn name_ref_or_index(p: &mut Parser) { - if p.at(IDENT) || p.at(INT_NUMBER) { - let m = p.start(); - p.bump_any(); - m.complete(p, NAME_REF); - } else { - p.err_and_bump("expected identifier"); - } + assert!(p.at(IDENT) || p.at(INT_NUMBER)); + let m = p.start(); + p.bump_any(); + m.complete(p, NAME_REF); } fn error_block(p: &mut Parser, message: &str) { -- cgit v1.2.3 From 5e5eb6a108b00c573455d8d088742592012707be Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 11 Apr 2020 23:33:17 +0200 Subject: Align grammar for record patterns and literals The grammar now looks like this [name_ref :] pat --- crates/ra_assists/src/handlers/reorder_fields.rs | 32 ++++++-------- crates/ra_hir_def/src/body/lower.rs | 2 +- crates/ra_hir_expand/src/name.rs | 9 ++++ crates/ra_hir_ty/src/tests/patterns.rs | 3 +- crates/ra_ide/src/completion/complete_pattern.rs | 4 ++ crates/ra_ide/src/completion/complete_record.rs | 2 +- .../src/completion/complete_unqualified_path.rs | 9 +++- crates/ra_ide/src/completion/completion_context.rs | 12 ++++-- crates/ra_parser/src/grammar/patterns.rs | 50 ++++++++++------------ crates/ra_syntax/src/ast.rs | 4 +- crates/ra_syntax/src/ast/extensions.rs | 30 +++++++++++++ crates/ra_syntax/src/ast/generated/nodes.rs | 2 +- .../inline/ok/0102_record_field_pat_list.rast | 26 +++++------ .../test_data/parser/inline/ok/0143_box_pat.rast | 15 ++++--- .../parser/inline/ok/0145_record_field_pat.rast | 5 ++- .../parser/ok/0063_trait_fn_patterns.rast | 14 +++--- .../test_data/parser/ok/0064_impl_fn_params.rast | 14 +++--- xtask/src/ast_src.rs | 2 +- 18 files changed, 145 insertions(+), 90 deletions(-) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index 692dd1315..a43e53a11 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -1,20 +1,20 @@ use std::collections::HashMap; -use itertools::Itertools; - use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct}; +use itertools::Itertools; use ra_ide_db::RootDatabase; use ra_syntax::{ - algo, ast, - ast::{Name, Path, RecordLit, RecordPat}, - AstNode, SyntaxKind, SyntaxNode, + algo, + ast::{self, Path, RecordLit, RecordPat}, + match_ast, AstNode, SyntaxKind, + SyntaxKind::*, + SyntaxNode, }; use crate::{ assist_ctx::{Assist, AssistCtx}, AssistId, }; -use ra_syntax::ast::{Expr, NameRef}; // Assist: reorder_fields // @@ -59,7 +59,6 @@ fn reorder(ctx: AssistCtx) -> Option { } fn get_fields_kind(node: &SyntaxNode) -> Vec { - use SyntaxKind::*; match node.kind() { RECORD_LIT => vec![RECORD_FIELD], RECORD_PAT => vec![RECORD_FIELD_PAT, BIND_PAT], @@ -68,19 +67,14 @@ fn get_fields_kind(node: &SyntaxNode) -> Vec { } fn get_field_name(node: &SyntaxNode) -> String { - use SyntaxKind::*; - match node.kind() { - RECORD_FIELD => { - if let Some(name) = node.children().find_map(NameRef::cast) { - return name.to_string(); - } - node.children().find_map(Expr::cast).map(|expr| expr.to_string()).unwrap_or_default() - } - BIND_PAT | RECORD_FIELD_PAT => { - node.children().find_map(Name::cast).map(|n| n.to_string()).unwrap_or_default() + let res = match_ast! { + match node { + ast::RecordField(field) => { field.field_name().map(|it| it.to_string()) }, + ast::RecordFieldPat(field) => { field.field_name().map(|it| it.to_string()) }, + _ => None, } - _ => String::new(), - } + }; + res.unwrap_or_default() } fn get_fields(record: &SyntaxNode) -> Vec { diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index c057dc8f2..6caa87db4 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -637,7 +637,7 @@ impl ExprCollector<'_> { let iter = record_field_pat_list.record_field_pats().filter_map(|f| { let ast_pat = f.pat()?; let pat = self.collect_pat(ast_pat); - let name = f.name()?.as_name(); + let name = f.field_name()?.as_name(); Some(RecordFieldPat { name, pat }) }); fields.extend(iter); diff --git a/crates/ra_hir_expand/src/name.rs b/crates/ra_hir_expand/src/name.rs index 25cc1e9fc..fecce224e 100644 --- a/crates/ra_hir_expand/src/name.rs +++ b/crates/ra_hir_expand/src/name.rs @@ -83,6 +83,15 @@ impl AsName for ast::Name { } } +impl AsName for ast::NameOrNameRef { + fn as_name(&self) -> Name { + match self { + ast::NameOrNameRef::Name(it) => it.as_name(), + ast::NameOrNameRef::NameRef(it) => it.as_name(), + } + } +} + impl AsName for tt::Ident { fn as_name(&self) -> Name { Name::resolve(&self.text) diff --git a/crates/ra_hir_ty/src/tests/patterns.rs b/crates/ra_hir_ty/src/tests/patterns.rs index 6e5d2247c..07cbc521a 100644 --- a/crates/ra_hir_ty/src/tests/patterns.rs +++ b/crates/ra_hir_ty/src/tests/patterns.rs @@ -1,7 +1,8 @@ -use super::{infer, infer_with_mismatches}; use insta::assert_snapshot; use test_utils::covers; +use super::{infer, infer_with_mismatches}; + #[test] fn infer_pattern() { assert_snapshot!( diff --git a/crates/ra_ide/src/completion/complete_pattern.rs b/crates/ra_ide/src/completion/complete_pattern.rs index 1b7d3122f..a8b4ce114 100644 --- a/crates/ra_ide/src/completion/complete_pattern.rs +++ b/crates/ra_ide/src/completion/complete_pattern.rs @@ -7,6 +7,10 @@ pub(super) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) { if !ctx.is_pat_binding_or_const { return; } + if ctx.record_pat_syntax.is_some() { + return; + } + // FIXME: ideally, we should look at the type we are matching against and // suggest variants + auto-imports ctx.scope().process_all_names(&mut |name, res| { diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs index f46bcee5c..83a553155 100644 --- a/crates/ra_ide/src/completion/complete_record.rs +++ b/crates/ra_ide/src/completion/complete_record.rs @@ -2,7 +2,7 @@ use crate::completion::{CompletionContext, Completions}; pub(super) fn complete_record(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - let missing_fields = match (ctx.record_lit_pat.as_ref(), ctx.record_lit_syntax.as_ref()) { + let missing_fields = match (ctx.record_pat_syntax.as_ref(), ctx.record_lit_syntax.as_ref()) { (None, None) => return None, (Some(_), Some(_)) => unreachable!("A record cannot be both a literal and a pattern"), (Some(record_pat), _) => ctx.sema.record_pattern_missing_fields(record_pat), diff --git a/crates/ra_ide/src/completion/complete_unqualified_path.rs b/crates/ra_ide/src/completion/complete_unqualified_path.rs index 0b0da6ee4..2d8e0776c 100644 --- a/crates/ra_ide/src/completion/complete_unqualified_path.rs +++ b/crates/ra_ide/src/completion/complete_unqualified_path.rs @@ -3,7 +3,14 @@ use crate::completion::{CompletionContext, Completions}; pub(super) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionContext) { - if !(ctx.is_trivial_path && !ctx.is_pat_binding_or_const && !ctx.record_lit_syntax.is_some()) { + if !ctx.is_trivial_path { + return; + } + + if ctx.is_pat_binding_or_const + || ctx.record_lit_syntax.is_some() + || ctx.record_pat_syntax.is_some() + { return; } diff --git a/crates/ra_ide/src/completion/completion_context.rs b/crates/ra_ide/src/completion/completion_context.rs index 14a4a14d7..8b3401595 100644 --- a/crates/ra_ide/src/completion/completion_context.rs +++ b/crates/ra_ide/src/completion/completion_context.rs @@ -30,7 +30,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) function_syntax: Option, pub(super) use_item_syntax: Option, pub(super) record_lit_syntax: Option, - pub(super) record_lit_pat: Option, + pub(super) record_pat_syntax: Option, pub(super) impl_def: Option, pub(super) is_param: bool, /// If a name-binding or reference to a const in a pattern. @@ -93,7 +93,7 @@ impl<'a> CompletionContext<'a> { function_syntax: None, use_item_syntax: None, record_lit_syntax: None, - record_lit_pat: None, + record_pat_syntax: None, impl_def: None, is_param: false, is_pat_binding_or_const: false, @@ -182,6 +182,11 @@ impl<'a> CompletionContext<'a> { self.is_param = true; return; } + // FIXME: remove this (V) duplication and make the check more precise + if name_ref.syntax().ancestors().find_map(ast::RecordFieldPatList::cast).is_some() { + self.record_pat_syntax = + self.sema.find_node_at_offset_with_macros(&original_file, offset); + } self.classify_name_ref(original_file, name_ref, offset); } @@ -211,8 +216,9 @@ impl<'a> CompletionContext<'a> { self.is_param = true; return; } + // FIXME: remove this (^) duplication and make the check more precise if name.syntax().ancestors().find_map(ast::RecordFieldPatList::cast).is_some() { - self.record_lit_pat = + self.record_pat_syntax = self.sema.find_node_at_offset_with_macros(&original_file, offset); } } diff --git a/crates/ra_parser/src/grammar/patterns.rs b/crates/ra_parser/src/grammar/patterns.rs index 936d27575..68fb2fc73 100644 --- a/crates/ra_parser/src/grammar/patterns.rs +++ b/crates/ra_parser/src/grammar/patterns.rs @@ -192,14 +192,30 @@ fn record_field_pat_list(p: &mut Parser) { match p.current() { // A trailing `..` is *not* treated as a DOT_DOT_PAT. T![.] if p.at(T![..]) => p.bump(T![..]), - - IDENT | INT_NUMBER if p.nth(1) == T![:] => record_field_pat(p), T!['{'] => error_block(p, "expected ident"), - T![box] => { - box_pat(p); - } - _ => { - bind_pat(p, false); + + c => { + let m = p.start(); + match c { + // test record_field_pat + // fn foo() { + // let S { 0: 1 } = (); + // let S { x: 1 } = (); + // } + IDENT | INT_NUMBER if p.nth(1) == T![:] => { + name_ref_or_index(p); + p.bump(T![:]); + pattern(p); + } + T![box] => { + // FIXME: not all box patterns should be allowed + box_pat(p); + } + _ => { + bind_pat(p, false); + } + } + m.complete(p, RECORD_FIELD_PAT); } } if !p.at(T!['}']) { @@ -210,26 +226,6 @@ fn record_field_pat_list(p: &mut Parser) { m.complete(p, RECORD_FIELD_PAT_LIST); } -// test record_field_pat -// fn foo() { -// let S { 0: 1 } = (); -// let S { x: 1 } = (); -// } -fn record_field_pat(p: &mut Parser) { - assert!(p.at(IDENT) || p.at(INT_NUMBER)); - assert!(p.nth(1) == T![:]); - - let m = p.start(); - - if !p.eat(INT_NUMBER) { - name(p) - } - - p.bump_any(); - pattern(p); - m.complete(p, RECORD_FIELD_PAT); -} - // test placeholder_pat // fn main() { let _ = (); } fn placeholder_pat(p: &mut Parser) -> CompletedMarker { diff --git a/crates/ra_syntax/src/ast.rs b/crates/ra_syntax/src/ast.rs index 99c6b7219..7fca5661e 100644 --- a/crates/ra_syntax/src/ast.rs +++ b/crates/ra_syntax/src/ast.rs @@ -18,8 +18,8 @@ use crate::{ pub use self::{ expr_extensions::{ArrayExprKind, BinOp, ElseBranch, LiteralKind, PrefixOp, RangeOp}, extensions::{ - AttrKind, FieldKind, PathSegmentKind, SelfParamKind, SlicePatComponents, StructKind, - TypeBoundKind, VisibilityKind, + AttrKind, FieldKind, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, + StructKind, TypeBoundKind, VisibilityKind, }, generated::{nodes::*, tokens::*}, tokens::*, diff --git a/crates/ra_syntax/src/ast/extensions.rs b/crates/ra_syntax/src/ast/extensions.rs index 63e272fbf..f2ea5088e 100644 --- a/crates/ra_syntax/src/ast/extensions.rs +++ b/crates/ra_syntax/src/ast/extensions.rs @@ -1,6 +1,8 @@ //! Various extension methods to ast Nodes, which are hard to code-generate. //! Extensions for various expressions live in a sibling `expr_extensions` module. +use std::fmt; + use itertools::Itertools; use ra_parser::SyntaxKind; @@ -217,6 +219,34 @@ impl ast::RecordField { } } +pub enum NameOrNameRef { + Name(ast::Name), + NameRef(ast::NameRef), +} + +impl fmt::Display for NameOrNameRef { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + NameOrNameRef::Name(it) => fmt::Display::fmt(it, f), + NameOrNameRef::NameRef(it) => fmt::Display::fmt(it, f), + } + } +} + +impl ast::RecordFieldPat { + /// Deals with field init shorthand + pub fn field_name(&self) -> Option { + if let Some(name_ref) = self.name_ref() { + return Some(NameOrNameRef::NameRef(name_ref)); + } + if let Some(ast::Pat::BindPat(pat)) = self.pat() { + let name = pat.name()?; + return Some(NameOrNameRef::Name(name)); + } + None + } +} + impl ast::EnumVariant { pub fn parent_enum(&self) -> ast::EnumDef { self.syntax() diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs index f1098755b..188f0df96 100644 --- a/crates/ra_syntax/src/ast/generated/nodes.rs +++ b/crates/ra_syntax/src/ast/generated/nodes.rs @@ -1806,8 +1806,8 @@ impl AstNode for RecordFieldPat { fn syntax(&self) -> &SyntaxNode { &self.syntax } } impl ast::AttrsOwner for RecordFieldPat {} -impl ast::NameOwner for RecordFieldPat {} impl RecordFieldPat { + pub fn name_ref(&self) -> Option { support::child(&self.syntax) } pub fn colon_token(&self) -> Option { support::token(&self.syntax, T![:]) } pub fn pat(&self) -> Option { support::child(&self.syntax) } } diff --git a/crates/ra_syntax/test_data/parser/inline/ok/0102_record_field_pat_list.rast b/crates/ra_syntax/test_data/parser/inline/ok/0102_record_field_pat_list.rast index c2614543c..fcd099de9 100644 --- a/crates/ra_syntax/test_data/parser/inline/ok/0102_record_field_pat_list.rast +++ b/crates/ra_syntax/test_data/parser/inline/ok/0102_record_field_pat_list.rast @@ -44,18 +44,20 @@ SOURCE_FILE@[0; 119) RECORD_FIELD_PAT_LIST@[40; 56) L_CURLY@[40; 41) "{" WHITESPACE@[41; 42) " " - BIND_PAT@[42; 43) - NAME@[42; 43) - IDENT@[42; 43) "f" + RECORD_FIELD_PAT@[42; 43) + BIND_PAT@[42; 43) + NAME@[42; 43) + IDENT@[42; 43) "f" COMMA@[43; 44) "," WHITESPACE@[44; 45) " " - BIND_PAT@[45; 54) - REF_KW@[45; 48) "ref" - WHITESPACE@[48; 49) " " - MUT_KW@[49; 52) "mut" - WHITESPACE@[52; 53) " " - NAME@[53; 54) - IDENT@[53; 54) "g" + RECORD_FIELD_PAT@[45; 54) + BIND_PAT@[45; 54) + REF_KW@[45; 48) "ref" + WHITESPACE@[48; 49) " " + MUT_KW@[49; 52) "mut" + WHITESPACE@[52; 53) " " + NAME@[53; 54) + IDENT@[53; 54) "g" WHITESPACE@[54; 55) " " R_CURLY@[55; 56) "}" WHITESPACE@[56; 57) " " @@ -79,7 +81,7 @@ SOURCE_FILE@[0; 119) L_CURLY@[73; 74) "{" WHITESPACE@[74; 75) " " RECORD_FIELD_PAT@[75; 79) - NAME@[75; 76) + NAME_REF@[75; 76) IDENT@[75; 76) "h" COLON@[76; 77) ":" WHITESPACE@[77; 78) " " @@ -110,7 +112,7 @@ SOURCE_FILE@[0; 119) L_CURLY@[101; 102) "{" WHITESPACE@[102; 103) " " RECORD_FIELD_PAT@[103; 107) - NAME@[103; 104) + NAME_REF@[103; 104) IDENT@[103; 104) "h" COLON@[104; 105) ":" WHITESPACE@[105; 106) " " diff --git a/crates/ra_syntax/test_data/parser/inline/ok/0143_box_pat.rast b/crates/ra_syntax/test_data/parser/inline/ok/0143_box_pat.rast index f75673070..1d245f8f3 100644 --- a/crates/ra_syntax/test_data/parser/inline/ok/0143_box_pat.rast +++ b/crates/ra_syntax/test_data/parser/inline/ok/0143_box_pat.rast @@ -44,16 +44,17 @@ SOURCE_FILE@[0; 118) RECORD_FIELD_PAT_LIST@[50; 81) L_CURLY@[50; 51) "{" WHITESPACE@[51; 52) " " - BOX_PAT@[52; 57) - BOX_KW@[52; 55) "box" - WHITESPACE@[55; 56) " " - BIND_PAT@[56; 57) - NAME@[56; 57) - IDENT@[56; 57) "i" + RECORD_FIELD_PAT@[52; 57) + BOX_PAT@[52; 57) + BOX_KW@[52; 55) "box" + WHITESPACE@[55; 56) " " + BIND_PAT@[56; 57) + NAME@[56; 57) + IDENT@[56; 57) "i" COMMA@[57; 58) "," WHITESPACE@[58; 59) " " RECORD_FIELD_PAT@[59; 79) - NAME@[59; 60) + NAME_REF@[59; 60) IDENT@[59; 60) "j" COLON@[60; 61) ":" WHITESPACE@[61; 62) " " diff --git a/crates/ra_syntax/test_data/parser/inline/ok/0145_record_field_pat.rast b/crates/ra_syntax/test_data/parser/inline/ok/0145_record_field_pat.rast index 0d786f597..cac2ffdcf 100644 --- a/crates/ra_syntax/test_data/parser/inline/ok/0145_record_field_pat.rast +++ b/crates/ra_syntax/test_data/parser/inline/ok/0145_record_field_pat.rast @@ -25,7 +25,8 @@ SOURCE_FILE@[0; 63) L_CURLY@[21; 22) "{" WHITESPACE@[22; 23) " " RECORD_FIELD_PAT@[23; 27) - INT_NUMBER@[23; 24) "0" + NAME_REF@[23; 24) + INT_NUMBER@[23; 24) "0" COLON@[24; 25) ":" WHITESPACE@[25; 26) " " LITERAL_PAT@[26; 27) @@ -54,7 +55,7 @@ SOURCE_FILE@[0; 63) L_CURLY@[46; 47) "{" WHITESPACE@[47; 48) " " RECORD_FIELD_PAT@[48; 52) - NAME@[48; 49) + NAME_REF@[48; 49) IDENT@[48; 49) "x" COLON@[49; 50) ":" WHITESPACE@[50; 51) " " diff --git a/crates/ra_syntax/test_data/parser/ok/0063_trait_fn_patterns.rast b/crates/ra_syntax/test_data/parser/ok/0063_trait_fn_patterns.rast index 9b5954ebd..d0623ba90 100644 --- a/crates/ra_syntax/test_data/parser/ok/0063_trait_fn_patterns.rast +++ b/crates/ra_syntax/test_data/parser/ok/0063_trait_fn_patterns.rast @@ -68,14 +68,16 @@ SOURCE_FILE@[0; 170) RECORD_FIELD_PAT_LIST@[59; 67) L_CURLY@[59; 60) "{" WHITESPACE@[60; 61) " " - BIND_PAT@[61; 62) - NAME@[61; 62) - IDENT@[61; 62) "a" + RECORD_FIELD_PAT@[61; 62) + BIND_PAT@[61; 62) + NAME@[61; 62) + IDENT@[61; 62) "a" COMMA@[62; 63) "," WHITESPACE@[63; 64) " " - BIND_PAT@[64; 65) - NAME@[64; 65) - IDENT@[64; 65) "b" + RECORD_FIELD_PAT@[64; 65) + BIND_PAT@[64; 65) + NAME@[64; 65) + IDENT@[64; 65) "b" WHITESPACE@[65; 66) " " R_CURLY@[66; 67) "}" COLON@[67; 68) ":" diff --git a/crates/ra_syntax/test_data/parser/ok/0064_impl_fn_params.rast b/crates/ra_syntax/test_data/parser/ok/0064_impl_fn_params.rast index b30030de3..5e96b695b 100644 --- a/crates/ra_syntax/test_data/parser/ok/0064_impl_fn_params.rast +++ b/crates/ra_syntax/test_data/parser/ok/0064_impl_fn_params.rast @@ -71,14 +71,16 @@ SOURCE_FILE@[0; 137) RECORD_FIELD_PAT_LIST@[58; 66) L_CURLY@[58; 59) "{" WHITESPACE@[59; 60) " " - BIND_PAT@[60; 61) - NAME@[60; 61) - IDENT@[60; 61) "a" + RECORD_FIELD_PAT@[60; 61) + BIND_PAT@[60; 61) + NAME@[60; 61) + IDENT@[60; 61) "a" COMMA@[61; 62) "," WHITESPACE@[62; 63) " " - BIND_PAT@[63; 64) - NAME@[63; 64) - IDENT@[63; 64) "b" + RECORD_FIELD_PAT@[63; 64) + BIND_PAT@[63; 64) + NAME@[63; 64) + IDENT@[63; 64) "b" WHITESPACE@[64; 65) " " R_CURLY@[65; 66) "}" COLON@[66; 67) ":" diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs index 69cba9168..9c02f7c6f 100644 --- a/xtask/src/ast_src.rs +++ b/xtask/src/ast_src.rs @@ -511,7 +511,7 @@ pub(crate) const AST_SRC: AstSrc = AstSrc { T![..], T!['}'] } - struct RecordFieldPat: AttrsOwner, NameOwner { T![:], Pat } + struct RecordFieldPat: AttrsOwner { NameRef, T![:], Pat } struct TupleStructPat { Path, T!['('], args: [Pat], T![')'] } struct TuplePat { T!['('], args: [Pat], T![')'] } -- cgit v1.2.3 From a59179ac2d0894dc45d614242816665b9bd6ef8a Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 11 Apr 2020 20:50:54 -0700 Subject: missing match arms add test cases to demonstrate behavior of tuple with pattern --- crates/ra_hir_ty/src/_match.rs | 75 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 596194dcf..c9a6551ab 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -972,6 +972,47 @@ mod tests { check_no_diagnostic(content); } + #[test] + fn tuple_of_bools_with_ellipsis_at_end_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (false, ..) => {}, + (true, ..) => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_at_beginning_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (.., false) => {}, + (.., true) => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_no_diagnostic() { + let content = r" + fn test_fn() { + match (false, true, false) { + (..) => {}, + } + } + "; + + check_no_diagnostic(content); + } + #[test] fn tuple_of_tuple_and_bools_no_arms() { let content = r" @@ -1553,4 +1594,38 @@ mod false_negatives { // with `!`. check_no_diagnostic(content); } + + #[test] + fn tuple_of_bools_with_ellipsis_at_end_missing_arm() { + let content = r" + fn test_fn() { + match (false, true, false) { + (false, ..) => {}, + } + } + "; + + // This is a false negative. + // The `..` pattern is currently lowered to a single `Pat::Wild` + // no matter how many fields the `..` pattern is covering. This + // causes the match arm in this test not to type check against + // the match expression, which causes this diagnostic not to + // fire. + check_no_diagnostic(content); + } + + #[test] + fn tuple_of_bools_with_ellipsis_at_beginning_missing_arm() { + let content = r" + fn test_fn() { + match (false, true, false) { + (.., false) => {}, + } + } + "; + + // This is a false negative. + // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. + check_no_diagnostic(content); + } } -- cgit v1.2.3 From bb2e5308b77e5d115df17411aaa2dd26be171b0a Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sat, 11 Apr 2020 21:20:52 -0700 Subject: missing match arm add test cases to demonstrate enum tuple struct with ellipsis behavior --- crates/ra_hir_ty/src/_match.rs | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index c9a6551ab..c482cf619 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1484,6 +1484,45 @@ mod tests { check_no_diagnostic(content); } + + #[test] + fn enum_tuple_partial_ellipsis_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., true) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_ellipsis_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(..) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } } #[cfg(test)] @@ -1628,4 +1667,29 @@ mod false_negatives { // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. check_no_diagnostic(content); } + + #[test] + fn enum_tuple_partial_ellipsis_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + // This is a false negative. + // The `..` pattern is currently lowered to a single `Pat::Wild` + // no matter how many fields the `..` pattern is covering. This + // causes us to return a `MatchCheckErr::MalformedMatchArm` in + // `Pat::specialize_constructor`. + check_no_diagnostic(content); + } } -- cgit v1.2.3 From e809328c122b20137a60f4533fada9cbe9d31e09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Sun, 12 Apr 2020 18:20:03 +0300 Subject: Remove more unnecessary braces --- crates/ra_assists/src/handlers/reorder_fields.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index a43e53a11..5cbb98d73 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -69,8 +69,8 @@ fn get_fields_kind(node: &SyntaxNode) -> Vec { fn get_field_name(node: &SyntaxNode) -> String { let res = match_ast! { match node { - ast::RecordField(field) => { field.field_name().map(|it| it.to_string()) }, - ast::RecordFieldPat(field) => { field.field_name().map(|it| it.to_string()) }, + ast::RecordField(field) => field.field_name().map(|it| it.to_string()), + ast::RecordFieldPat(field) => field.field_name().map(|it| it.to_string()), _ => None, } }; -- cgit v1.2.3 From abdf725c558f860b47883a6843bfd2a5a7c633bd Mon Sep 17 00:00:00 2001 From: IceSentry Date: Sun, 12 Apr 2020 21:29:14 -0400 Subject: Fix double comma when merge imports on second line This fixes the a bug when merging imports from the second line when it already has a comma it would previously insert a comma. There's probably a better way to check for a COMMA. This also ends up with a weird indentation, but rust-fmt can easily deal with it so I'm not sure how to resolve that. Closes #3832 --- crates/ra_assists/src/handlers/merge_imports.rs | 40 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs index 0958f52f1..ab1fb2a0b 100644 --- a/crates/ra_assists/src/handlers/merge_imports.rs +++ b/crates/ra_assists/src/handlers/merge_imports.rs @@ -3,7 +3,8 @@ use std::iter::successors; use ra_syntax::{ algo::{neighbor, SyntaxRewriter}, ast::{self, edit::AstNodeEdit, make}, - AstNode, Direction, InsertPosition, SyntaxElement, T, + AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxToken, Token, + T, }; use crate::{Assist, AssistCtx, AssistId}; @@ -72,9 +73,24 @@ fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option = Vec::new(); - to_insert.push(make::token(T![,]).into()); - to_insert.push(make::tokens::single_space().into()); + if should_insert_comma { + to_insert.push(make::token(T![,]).into()); + to_insert.push(make::tokens::single_space().into()); + } to_insert.extend( rhs.use_tree_list()? .syntax() @@ -247,4 +263,22 @@ use { ", ); } + + #[test] + fn test_double_comma() { + check_assist( + merge_imports, + r" +use hyper::service::make_service_fn; +use hyper::<|>{ + StatusCode, +}; +", + r" +use hyper::{<|> + StatusCode, +service::make_service_fn}; +", + ) + } } -- cgit v1.2.3 From 2e279ca031a52e90cd19dff513ee9d5b80cee8aa Mon Sep 17 00:00:00 2001 From: IceSentry Date: Sun, 12 Apr 2020 21:34:01 -0400 Subject: Generalize test and clean up imports --- crates/ra_assists/src/handlers/merge_imports.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs index ab1fb2a0b..b3101f3ab 100644 --- a/crates/ra_assists/src/handlers/merge_imports.rs +++ b/crates/ra_assists/src/handlers/merge_imports.rs @@ -3,8 +3,7 @@ use std::iter::successors; use ra_syntax::{ algo::{neighbor, SyntaxRewriter}, ast::{self, edit::AstNodeEdit, make}, - AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxToken, Token, - T, + AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, T, }; use crate::{Assist, AssistCtx, AssistId}; @@ -269,15 +268,15 @@ use { check_assist( merge_imports, r" -use hyper::service::make_service_fn; -use hyper::<|>{ - StatusCode, +use foo::bar::baz; +use foo::<|>{ + FooBar, }; ", r" -use hyper::{<|> - StatusCode, -service::make_service_fn}; +use foo::{<|> + FooBar, +bar::baz}; ", ) } -- cgit v1.2.3 From ee822d19b7662a9055bc6693c4c40d8dcf752ea1 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Sun, 12 Apr 2020 08:40:09 -0700 Subject: handle tuple patterns with ellipsis --- crates/ra_hir_def/src/body/lower.rs | 27 ++++-- crates/ra_hir_def/src/expr.rs | 6 +- crates/ra_hir_ty/src/_match.rs | 160 +++++++++++++++++++++++++----------- crates/ra_hir_ty/src/infer/pat.rs | 6 +- 4 files changed, 141 insertions(+), 58 deletions(-) diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 6caa87db4..79abe55ce 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -33,6 +33,7 @@ use crate::{ }; use super::{ExprSource, PatSource}; +use ast::AstChildren; pub(super) fn lower( db: &dyn DefDatabase, @@ -598,8 +599,8 @@ impl ExprCollector<'_> { } ast::Pat::TupleStructPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(path)); - let args = p.args().map(|p| self.collect_pat(p)).collect(); - Pat::TupleStruct { path, args } + let (args, ellipsis) = self.collect_tuple_pat(p.args()); + Pat::TupleStruct { path, args, ellipsis } } ast::Pat::RefPat(p) => { let pat = self.collect_pat_opt(p.pat()); @@ -616,10 +617,10 @@ impl ExprCollector<'_> { } ast::Pat::ParenPat(p) => return self.collect_pat_opt(p.pat()), ast::Pat::TuplePat(p) => { - let args = p.args().map(|p| self.collect_pat(p)).collect(); - Pat::Tuple(args) + let (args, ellipsis) = self.collect_tuple_pat(p.args()); + Pat::Tuple { args, ellipsis } } - ast::Pat::PlaceholderPat(_) | ast::Pat::DotDotPat(_) => Pat::Wild, + ast::Pat::PlaceholderPat(_) => Pat::Wild, ast::Pat::RecordPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(path)); let record_field_pat_list = @@ -665,6 +666,9 @@ impl ExprCollector<'_> { Pat::Missing } } + ast::Pat::DotDotPat(_) => unreachable!( + "`DotDotPat` requires special handling and should not be mapped to a Pat." + ), // FIXME: implement ast::Pat::BoxPat(_) | ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing, }; @@ -679,6 +683,19 @@ impl ExprCollector<'_> { self.missing_pat() } } + + fn collect_tuple_pat(&mut self, args: AstChildren) -> (Vec, Option) { + // Find the location of the `..`, if there is one. Note that we do not + // consider the possiblity of there being multiple `..` here. + let ellipsis = args.clone().position(|p| matches!(p, ast::Pat::DotDotPat(_))); + // We want to skip the `..` pattern here, since we account for it above. + let args = args + .filter(|p| !matches!(p, ast::Pat::DotDotPat(_))) + .map(|p| self.collect_pat(p)) + .collect(); + + (args, ellipsis) + } } impl From for BinaryOp { diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs index e11bdf3ec..a0cdad529 100644 --- a/crates/ra_hir_def/src/expr.rs +++ b/crates/ra_hir_def/src/expr.rs @@ -374,7 +374,7 @@ pub struct RecordFieldPat { pub enum Pat { Missing, Wild, - Tuple(Vec), + Tuple { args: Vec, ellipsis: Option }, Or(Vec), Record { path: Option, args: Vec, ellipsis: bool }, Range { start: ExprId, end: ExprId }, @@ -382,7 +382,7 @@ pub enum Pat { Path(Path), Lit(ExprId), Bind { mode: BindingAnnotation, name: Name, subpat: Option }, - TupleStruct { path: Option, args: Vec }, + TupleStruct { path: Option, args: Vec, ellipsis: Option }, Ref { pat: PatId, mutability: Mutability }, } @@ -393,7 +393,7 @@ impl Pat { Pat::Bind { subpat, .. } => { subpat.iter().copied().for_each(f); } - Pat::Or(args) | Pat::Tuple(args) | Pat::TupleStruct { args, .. } => { + Pat::Or(args) | Pat::Tuple { args, .. } | Pat::TupleStruct { args, .. } => { args.iter().copied().for_each(f); } Pat::Ref { pat, .. } => f(*pat), diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index c482cf619..a64be9848 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -289,7 +289,7 @@ impl PatStack { Self::from_slice(&self.0[1..]) } - fn replace_head_with(&self, pat_ids: &[PatId]) -> PatStack { + fn replace_head_with + Copy>(&self, pat_ids: &[T]) -> PatStack { let mut patterns: PatStackInner = smallvec![]; for pat in pat_ids { patterns.push((*pat).into()); @@ -320,12 +320,14 @@ impl PatStack { constructor: &Constructor, ) -> MatchCheckResult> { let result = match (self.head().as_pat(cx), constructor) { - (Pat::Tuple(ref pat_ids), Constructor::Tuple { arity }) => { - debug_assert_eq!( - pat_ids.len(), - *arity, - "we type check before calling this code, so we should never hit this case", - ); + (Pat::Tuple { args: ref pat_ids, ellipsis }, Constructor::Tuple { arity: _ }) => { + if ellipsis.is_some() { + // If there are ellipsis here, we should add the correct number of + // Pat::Wild patterns to `pat_ids`. We should be able to use the + // constructors arity for this, but at the time of writing we aren't + // correctly calculating this arity when ellipsis are present. + return Err(MatchCheckErr::NotImplemented); + } Some(self.replace_head_with(pat_ids)) } @@ -351,19 +353,47 @@ impl PatStack { Some(self.to_tail()) } } - (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => { + ( + Pat::TupleStruct { args: ref pat_ids, ellipsis, .. }, + Constructor::Enum(enum_constructor), + ) => { let pat_id = self.head().as_id().expect("we know this isn't a wild"); if !enum_variant_matches(cx, pat_id, *enum_constructor) { None } else { - // If the enum variant matches, then we need to confirm - // that the number of patterns aligns with the expected - // number of patterns for that enum variant. - if pat_ids.len() != constructor.arity(cx)? { - return Err(MatchCheckErr::MalformedMatchArm); + let constructor_arity = constructor.arity(cx)?; + if let Some(ellipsis_position) = ellipsis { + // If there are ellipsis in the pattern, the ellipsis must take the place + // of at least one sub-pattern, so `pat_ids` should be smaller than the + // constructor arity. + if pat_ids.len() < constructor_arity { + let mut new_patterns: Vec = vec![]; + + for pat_id in &pat_ids[0..ellipsis_position] { + new_patterns.push((*pat_id).into()); + } + + for _ in 0..(constructor_arity - pat_ids.len()) { + new_patterns.push(PatIdOrWild::Wild); + } + + for pat_id in &pat_ids[ellipsis_position..pat_ids.len()] { + new_patterns.push((*pat_id).into()); + } + + Some(self.replace_head_with(&new_patterns)) + } else { + return Err(MatchCheckErr::MalformedMatchArm); + } + } else { + // If there is no ellipsis in the tuple pattern, the number + // of patterns must equal the constructor arity. + if pat_ids.len() == constructor_arity { + Some(self.replace_head_with(pat_ids)) + } else { + return Err(MatchCheckErr::MalformedMatchArm); + } } - - Some(self.replace_head_with(pat_ids)) } } (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), @@ -644,7 +674,11 @@ impl Constructor { fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult> { let res = match pat.as_pat(cx) { Pat::Wild => None, - Pat::Tuple(pats) => Some(Constructor::Tuple { arity: pats.len() }), + // FIXME somehow create the Tuple constructor with the proper arity. If there are + // ellipsis, the arity is not equal to the number of patterns. + Pat::Tuple { args: pats, ellipsis } if ellipsis.is_none() => { + Some(Constructor::Tuple { arity: pats.len() }) + } Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), _ => return Err(MatchCheckErr::NotImplemented), @@ -1506,6 +1540,67 @@ mod tests { check_no_diagnostic(content); } + #[test] + fn enum_tuple_partial_ellipsis_2_no_diagnostic() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(.., true) => {}, + Either::A(.., false) => {}, + Either::B => {}, + } + } + "; + + check_no_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(false, .., false) => {}, + Either::B => {}, + } + } + "; + + check_diagnostic(content); + } + + #[test] + fn enum_tuple_partial_ellipsis_2_missing_arm() { + let content = r" + enum Either { + A(bool, bool, bool, bool), + B, + } + fn test_fn() { + match Either::B { + Either::A(true, .., true) => {}, + Either::A(true, .., false) => {}, + Either::A(.., true) => {}, + Either::B => {}, + } + } + "; + + check_diagnostic(content); + } + #[test] fn enum_tuple_ellipsis_no_diagnostic() { let content = r" @@ -1645,11 +1740,7 @@ mod false_negatives { "; // This is a false negative. - // The `..` pattern is currently lowered to a single `Pat::Wild` - // no matter how many fields the `..` pattern is covering. This - // causes the match arm in this test not to type check against - // the match expression, which causes this diagnostic not to - // fire. + // We don't currently handle tuple patterns with ellipsis. check_no_diagnostic(content); } @@ -1664,32 +1755,7 @@ mod false_negatives { "; // This is a false negative. - // See comments on `tuple_of_bools_with_ellipsis_at_end_missing_arm`. - check_no_diagnostic(content); - } - - #[test] - fn enum_tuple_partial_ellipsis_missing_arm() { - let content = r" - enum Either { - A(bool, bool, bool, bool), - B, - } - fn test_fn() { - match Either::B { - Either::A(true, .., true) => {}, - Either::A(true, .., false) => {}, - Either::A(false, .., false) => {}, - Either::B => {}, - } - } - "; - - // This is a false negative. - // The `..` pattern is currently lowered to a single `Pat::Wild` - // no matter how many fields the `..` pattern is covering. This - // causes us to return a `MatchCheckErr::MalformedMatchArm` in - // `Pat::specialize_constructor`. + // We don't currently handle tuple patterns with ellipsis. check_no_diagnostic(content); } } diff --git a/crates/ra_hir_ty/src/infer/pat.rs b/crates/ra_hir_ty/src/infer/pat.rs index 078476f76..8ec4d4ace 100644 --- a/crates/ra_hir_ty/src/infer/pat.rs +++ b/crates/ra_hir_ty/src/infer/pat.rs @@ -85,7 +85,7 @@ impl<'a> InferenceContext<'a> { let body = Arc::clone(&self.body); // avoid borrow checker problem let is_non_ref_pat = match &body[pat] { - Pat::Tuple(..) + Pat::Tuple { .. } | Pat::Or(..) | Pat::TupleStruct { .. } | Pat::Record { .. } @@ -116,7 +116,7 @@ impl<'a> InferenceContext<'a> { let expected = expected; let ty = match &body[pat] { - Pat::Tuple(ref args) => { + Pat::Tuple { ref args, .. } => { let expectations = match expected.as_tuple() { Some(parameters) => &*parameters.0, _ => &[], @@ -155,7 +155,7 @@ impl<'a> InferenceContext<'a> { let subty = self.infer_pat(*pat, expectation, default_bm); Ty::apply_one(TypeCtor::Ref(*mutability), subty) } - Pat::TupleStruct { path: p, args: subpats } => { + Pat::TupleStruct { path: p, args: subpats, .. } => { self.infer_tuple_struct_pat(p.as_ref(), subpats, expected, default_bm, pat) } Pat::Record { path: p, args: fields, ellipsis: _ } => { -- cgit v1.2.3 From ed0eedb1dda9ae2a46735bb469aeaf5cf8a28601 Mon Sep 17 00:00:00 2001 From: IceSentry Date: Mon, 13 Apr 2020 13:59:30 -0400 Subject: Fix PR --- crates/ra_assists/src/handlers/merge_imports.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs index b3101f3ab..ef0ce0586 100644 --- a/crates/ra_assists/src/handlers/merge_imports.rs +++ b/crates/ra_assists/src/handlers/merge_imports.rs @@ -1,9 +1,9 @@ use std::iter::successors; use ra_syntax::{ - algo::{neighbor, SyntaxRewriter}, + algo::{neighbor, skip_trivia_token, SyntaxRewriter}, ast::{self, edit::AstNodeEdit, make}, - AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, T, + AstNode, Direction, InsertPosition, SyntaxElement, T, }; use crate::{Assist, AssistCtx, AssistId}; @@ -72,18 +72,12 @@ fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option = Vec::new(); if should_insert_comma { -- cgit v1.2.3