From 42450d2511f8f174dc7448d0e9839d4b76d64482 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 1 Jun 2023 07:45:55 +0200 Subject: [PATCH 1/3] Add signature help for tuple patterns and expressions --- crates/ide/src/signature_help.rs | 418 ++++++++++++++++++++++++++++--- 1 file changed, 390 insertions(+), 28 deletions(-) diff --git a/crates/ide/src/signature_help.rs b/crates/ide/src/signature_help.rs index 9a0529ec20f..455b519f80b 100644 --- a/crates/ide/src/signature_help.rs +++ b/crates/ide/src/signature_help.rs @@ -15,8 +15,9 @@ use ide_db::{ use stdx::format_to; use syntax::{ algo, - ast::{self, HasArgList}, - match_ast, AstNode, Direction, SyntaxElementChildren, SyntaxToken, TextRange, TextSize, + ast::{self, AstChildren, HasArgList}, + match_ast, AstNode, Direction, NodeOrToken, SyntaxElementChildren, SyntaxNode, SyntaxToken, + TextRange, TextSize, T, }; use crate::RootDatabase; @@ -116,6 +117,20 @@ pub(crate) fn signature_help(db: &RootDatabase, position: FilePosition) -> Optio } return signature_help_for_tuple_struct_pat(&sema, tuple_pat, token); }, + ast::TuplePat(tuple_pat) => { + let cursor_outside = tuple_pat.r_paren_token().as_ref() == Some(&token); + if cursor_outside { + continue; + } + return signature_help_for_tuple_pat(&sema, tuple_pat, token); + }, + ast::TupleExpr(tuple_expr) => { + let cursor_outside = tuple_expr.r_paren_token().as_ref() == Some(&token); + if cursor_outside { + continue; + } + return signature_help_for_tuple_expr(&sema, tuple_expr, token); + }, _ => (), } } @@ -395,19 +410,16 @@ fn signature_help_for_tuple_struct_pat( pat: ast::TupleStructPat, token: SyntaxToken, ) -> Option { - let rest_pat = pat.fields().find(|it| matches!(it, ast::Pat::RestPat(_))); - let is_left_of_rest_pat = - rest_pat.map_or(true, |it| token.text_range().start() < it.syntax().text_range().end()); - + let path = pat.path()?; + let path_res = sema.resolve_path(&path)?; let mut res = SignatureHelp { doc: None, signature: String::new(), parameters: vec![], active_parameter: None, }; - let db = sema.db; - let path_res = sema.resolve_path(&pat.path()?)?; + let fields: Vec<_> = if let PathResolution::Def(ModuleDef::Variant(variant)) = path_res { let en = variant.parent_enum(db); @@ -435,30 +447,72 @@ fn signature_help_for_tuple_struct_pat( _ => return None, } }; - let commas = pat - .syntax() - .children_with_tokens() - .filter_map(syntax::NodeOrToken::into_token) - .filter(|t| t.kind() == syntax::T![,]); - res.active_parameter = Some(if is_left_of_rest_pat { - commas.take_while(|t| t.text_range().start() <= token.text_range().start()).count() - } else { - let n_commas = commas - .collect::>() - .into_iter() - .rev() - .take_while(|t| t.text_range().start() > token.text_range().start()) - .count(); - fields.len().saturating_sub(1).saturating_sub(n_commas) - }); + Some(signature_help_for_tuple_pat_ish( + db, + res, + pat.syntax(), + token, + pat.fields(), + fields.into_iter().map(|it| it.ty(db)), + )) +} +fn signature_help_for_tuple_pat( + sema: &Semantics<'_, RootDatabase>, + pat: ast::TuplePat, + token: SyntaxToken, +) -> Option { + let db = sema.db; + let field_pats = pat.fields(); + let pat = pat.into(); + let ty = sema.type_of_pat(&pat)?; + let fields = ty.original.tuple_fields(db); + + Some(signature_help_for_tuple_pat_ish( + db, + SignatureHelp { + doc: None, + signature: String::from('('), + parameters: vec![], + active_parameter: None, + }, + pat.syntax(), + token, + field_pats, + fields.into_iter(), + )) +} + +fn signature_help_for_tuple_expr( + sema: &Semantics<'_, RootDatabase>, + expr: ast::TupleExpr, + token: SyntaxToken, +) -> Option { + let active_parameter = Some( + expr.syntax() + .children_with_tokens() + .filter_map(NodeOrToken::into_token) + .filter(|t| t.kind() == T![,]) + .take_while(|t| t.text_range().start() <= token.text_range().start()) + .count(), + ); + + let db = sema.db; + let mut res = SignatureHelp { + doc: None, + signature: String::from('('), + parameters: vec![], + active_parameter, + }; + let expr = sema.type_of_expr(&expr.into())?; + let fields = expr.original.tuple_fields(db); let mut buf = String::new(); - for ty in fields.into_iter().map(|it| it.ty(db)) { + for ty in fields { format_to!(buf, "{}", ty.display_truncated(db, Some(20))); res.push_call_param(&buf); buf.clear(); } - res.signature.push_str(")"); + res.signature.push(')'); Some(res) } @@ -470,8 +524,8 @@ fn signature_help_for_record_( token: SyntaxToken, ) -> Option { let active_parameter = field_list_children - .filter_map(syntax::NodeOrToken::into_token) - .filter(|t| t.kind() == syntax::T![,]) + .filter_map(NodeOrToken::into_token) + .filter(|t| t.kind() == T![,]) .take_while(|t| t.text_range().start() <= token.text_range().start()) .count(); @@ -542,6 +596,46 @@ fn signature_help_for_record_( Some(res) } +fn signature_help_for_tuple_pat_ish( + db: &RootDatabase, + mut res: SignatureHelp, + pat: &SyntaxNode, + token: SyntaxToken, + mut field_pats: AstChildren, + fields: impl ExactSizeIterator, +) -> SignatureHelp { + let rest_pat = field_pats.find(|it| matches!(it, ast::Pat::RestPat(_))); + let is_left_of_rest_pat = + rest_pat.map_or(true, |it| token.text_range().start() < it.syntax().text_range().end()); + + let commas = pat + .children_with_tokens() + .filter_map(NodeOrToken::into_token) + .filter(|t| t.kind() == T![,]); + + res.active_parameter = { + Some(if is_left_of_rest_pat { + commas.take_while(|t| t.text_range().start() <= token.text_range().start()).count() + } else { + let n_commas = commas + .collect::>() + .into_iter() + .rev() + .take_while(|t| t.text_range().start() > token.text_range().start()) + .count(); + fields.len().saturating_sub(1).saturating_sub(n_commas) + }) + }; + + let mut buf = String::new(); + for ty in fields { + format_to!(buf, "{}", ty.display_truncated(db, Some(20))); + res.push_call_param(&buf); + buf.clear(); + } + res.signature.push_str(")"); + res +} #[cfg(test)] mod tests { use std::iter; @@ -1851,4 +1945,272 @@ fn main() { "#]], ); } + + #[test] + fn test_tuple_expr_free() { + check( + r#" +fn main() { + (0$0, 1, 3); +} +"#, + expect![[r#" + (i32, i32, i32) + ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + ($0 1, 3); +} +"#, + expect![[r#" + (i32, i32) + ^^^ --- + "#]], + ); + check( + r#" +fn main() { + (1, 3 $0); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + (1, 3 $0,); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + } + + #[test] + fn test_tuple_expr_expected() { + // FIXME: Seems like we discard valuable results in typeck here + check( + r#" +fn main() { + let _: (&str, u32, u32)= ($0, 1, 3); +} +"#, + expect![""], + ); + check( + r#" +fn main() { + let _: (&str, u32, u32, u32)= ($0, 1, 3); +} +"#, + expect![""], + ); + check( + r#" +fn main() { + let _: (&str, u32, u32)= ($0, 1, 3, 5); +} +"#, + expect![""], + ); + } + + #[test] + fn test_tuple_pat_free() { + // FIXME: Seems like we discard valuable results in typeck here + check( + r#" +fn main() { + let (0$0, 1, 3); +} +"#, + expect![[r#" + (i32, i32, i32) + ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + let ($0 1, 3); +} +"#, + expect![[r#" + (i32, i32) + ^^^ --- + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0,); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0, ..); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + let (1, 3, .., $0); +} +"#, + // FIXME: This is wrong, this should not mark the last as active + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + } + + #[test] + fn test_tuple_pat_expected() { + check( + r#" +fn main() { + let (0$0, 1, 3): (i32, i32, i32); +} +"#, + expect![[r#" + (i32, i32, i32) + ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + let ($0 1, 3): (i32, i32, i32); +} +"#, + // FIXME: tuple pat should be of size 3 ideally + expect![[r#" + (i32, i32) + ^^^ --- + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0): (i32,); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0, ..): (i32, i32, i32, i32); +} +"#, + expect![[r#" + (i32, i32, i32, i32) + --- ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + let (1, 3, .., $0): (i32, i32, i32); +} +"#, + expect![[r#" + (i32, i32, i32) + --- --- ^^^ + "#]], + ); + } + #[test] + fn test_tuple_pat_expected_inferred() { + check( + r#" +fn main() { + let (0$0, 1, 3) = (1, 2 ,3); +} +"#, + expect![[r#" + (i32, i32, i32) + ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + let ($0 1, 3) = (1, 2, 3); +} +"#, + // FIXME: tuple pat should be of size 3 ideally + expect![[r#" + (i32, i32) + ^^^ --- + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0) = (1,); +} +"#, + expect![[r#" + (i32, i32) + --- ^^^ + "#]], + ); + check( + r#" +fn main() { + let (1, 3 $0, ..) = (1, 2, 3, 4); +} +"#, + expect![[r#" + (i32, i32, i32, i32) + --- ^^^ --- --- + "#]], + ); + check( + r#" +fn main() { + let (1, 3, .., $0) = (1, 2, 3); +} +"#, + expect![[r#" + (i32, i32, i32) + --- --- ^^^ + "#]], + ); + } } From 7d1bf7023d7c11c20bfc35c9b9f4bfa027858d65 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 1 Jun 2023 08:40:50 +0200 Subject: [PATCH 2/3] Recover from leading comma in tuple pat and expr --- crates/ide/src/signature_help.rs | 17 +++++++++--- crates/parser/src/grammar/expressions/atom.rs | 10 +++++++ crates/parser/src/grammar/patterns.rs | 10 +++++++ .../err/0019_tuple_expr_leading_comma.rast | 24 +++++++++++++++++ .../err/0019_tuple_expr_leading_comma.rs | 3 +++ .../err/0020_tuple_pat_leading_comma.rast | 26 +++++++++++++++++++ .../err/0020_tuple_pat_leading_comma.rs | 3 +++ 7 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rast create mode 100644 crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rs create mode 100644 crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rast create mode 100644 crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rs diff --git a/crates/ide/src/signature_help.rs b/crates/ide/src/signature_help.rs index 455b519f80b..9bd9a948e7f 100644 --- a/crates/ide/src/signature_help.rs +++ b/crates/ide/src/signature_help.rs @@ -2003,7 +2003,10 @@ fn main() { let _: (&str, u32, u32)= ($0, 1, 3); } "#, - expect![""], + expect![[r#" + (&str, u32) + ^^^^ --- + "#]], ); check( r#" @@ -2011,7 +2014,10 @@ fn main() { let _: (&str, u32, u32, u32)= ($0, 1, 3); } "#, - expect![""], + expect![[r#" + (&str, u32) + ^^^^ --- + "#]], ); check( r#" @@ -2019,7 +2025,10 @@ fn main() { let _: (&str, u32, u32)= ($0, 1, 3, 5); } "#, - expect![""], + expect![[r#" + (&str, u32, u32) + ^^^^ --- --- + "#]], ); } @@ -2111,7 +2120,7 @@ fn main() { check( r#" fn main() { - let ($0 1, 3): (i32, i32, i32); + let ($0, 1, 3): (i32, i32, i32); } "#, // FIXME: tuple pat should be of size 3 ideally diff --git a/crates/parser/src/grammar/expressions/atom.rs b/crates/parser/src/grammar/expressions/atom.rs index 3cf9c4dd4b0..d8553d3f953 100644 --- a/crates/parser/src/grammar/expressions/atom.rs +++ b/crates/parser/src/grammar/expressions/atom.rs @@ -184,6 +184,16 @@ fn tuple_expr(p: &mut Parser<'_>) -> CompletedMarker { let mut saw_comma = false; let mut saw_expr = false; + + // test_err tuple_expr_leading_comma + // fn foo() { + // (,); + // } + if p.eat(T![,]) { + p.error("expected expression"); + saw_comma = true; + } + while !p.at(EOF) && !p.at(T![')']) { saw_expr = true; diff --git a/crates/parser/src/grammar/patterns.rs b/crates/parser/src/grammar/patterns.rs index 4801732101f..39ded41bb24 100644 --- a/crates/parser/src/grammar/patterns.rs +++ b/crates/parser/src/grammar/patterns.rs @@ -413,6 +413,16 @@ fn tuple_pat(p: &mut Parser<'_>) -> CompletedMarker { let mut has_comma = false; let mut has_pat = false; let mut has_rest = false; + + // test_err tuple_pat_leading_comma + // fn foo() { + // let (,); + // } + if p.eat(T![,]) { + p.error("expected pattern"); + has_comma = true; + } + while !p.at(EOF) && !p.at(T![')']) { has_pat = true; if !p.at_ts(PAT_TOP_FIRST) { diff --git a/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rast b/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rast new file mode 100644 index 00000000000..3fbc0da4002 --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rast @@ -0,0 +1,24 @@ +SOURCE_FILE + FN + FN_KW "fn" + WHITESPACE " " + NAME + IDENT "foo" + PARAM_LIST + L_PAREN "(" + R_PAREN ")" + WHITESPACE " " + BLOCK_EXPR + STMT_LIST + L_CURLY "{" + WHITESPACE "\n " + EXPR_STMT + TUPLE_EXPR + L_PAREN "(" + COMMA "," + R_PAREN ")" + SEMICOLON ";" + WHITESPACE "\n" + R_CURLY "}" + WHITESPACE "\n" +error 17: expected expression diff --git a/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rs b/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rs new file mode 100644 index 00000000000..12fab59a776 --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rs @@ -0,0 +1,3 @@ +fn foo() { + (,); +} diff --git a/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rast b/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rast new file mode 100644 index 00000000000..9c8837292d2 --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rast @@ -0,0 +1,26 @@ +SOURCE_FILE + FN + FN_KW "fn" + WHITESPACE " " + NAME + IDENT "foo" + PARAM_LIST + L_PAREN "(" + R_PAREN ")" + WHITESPACE " " + BLOCK_EXPR + STMT_LIST + L_CURLY "{" + WHITESPACE "\n " + LET_STMT + LET_KW "let" + WHITESPACE " " + TUPLE_PAT + L_PAREN "(" + COMMA "," + R_PAREN ")" + SEMICOLON ";" + WHITESPACE "\n" + R_CURLY "}" + WHITESPACE "\n" +error 21: expected pattern diff --git a/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rs b/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rs new file mode 100644 index 00000000000..de168521e1d --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rs @@ -0,0 +1,3 @@ +fn foo() { + let (,); +} From 0e2820283261183972b016d86a92144a3f8472e5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 1 Jun 2023 08:56:40 +0200 Subject: [PATCH 3/3] Insert missing expr/pat for leading comma tuples --- crates/hir-def/src/body/lower.rs | 40 +++++++++++++++++++++++++++----- crates/ide/src/signature_help.rs | 35 +++++++++++++++++----------- 2 files changed, 56 insertions(+), 19 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index acc9943481a..ebe05afca6a 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -542,9 +542,18 @@ impl ExprCollector<'_> { self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr) } ast::Expr::TupleExpr(e) => { - let exprs = e.fields().map(|expr| self.collect_expr(expr)).collect(); + let mut exprs: Vec<_> = e.fields().map(|expr| self.collect_expr(expr)).collect(); + // if there is a leading comma, the user is most likely to type out a leading expression + // so we insert a missing expression at the beginning for IDE features + if comma_follows_token(e.l_paren_token()) { + exprs.insert(0, self.missing_expr()); + } + self.alloc_expr( - Expr::Tuple { exprs, is_assignee_expr: self.is_lowering_assignee_expr }, + Expr::Tuple { + exprs: exprs.into_boxed_slice(), + is_assignee_expr: self.is_lowering_assignee_expr, + }, syntax_ptr, ) } @@ -1180,7 +1189,11 @@ impl ExprCollector<'_> { ast::Pat::TupleStructPat(p) => { let path = p.path().and_then(|path| self.expander.parse_path(self.db, path)).map(Box::new); - let (args, ellipsis) = self.collect_tuple_pat(p.fields(), binding_list); + let (args, ellipsis) = self.collect_tuple_pat( + p.fields(), + comma_follows_token(p.l_paren_token()), + binding_list, + ); Pat::TupleStruct { path, args, ellipsis } } ast::Pat::RefPat(p) => { @@ -1199,7 +1212,11 @@ impl ExprCollector<'_> { } ast::Pat::ParenPat(p) => return self.collect_pat_opt(p.pat(), binding_list), ast::Pat::TuplePat(p) => { - let (args, ellipsis) = self.collect_tuple_pat(p.fields(), binding_list); + let (args, ellipsis) = self.collect_tuple_pat( + p.fields(), + comma_follows_token(p.l_paren_token()), + binding_list, + ); Pat::Tuple { args, ellipsis } } ast::Pat::WildcardPat(_) => Pat::Wild, @@ -1323,18 +1340,24 @@ impl ExprCollector<'_> { fn collect_tuple_pat( &mut self, args: AstChildren, + has_leading_comma: bool, binding_list: &mut BindingList, ) -> (Box<[PatId]>, Option) { // Find the location of the `..`, if there is one. Note that we do not // consider the possibility of there being multiple `..` here. let ellipsis = args.clone().position(|p| matches!(p, ast::Pat::RestPat(_))); // We want to skip the `..` pattern here, since we account for it above. - let args = args + let mut args: Vec<_> = args .filter(|p| !matches!(p, ast::Pat::RestPat(_))) .map(|p| self.collect_pat(p, binding_list)) .collect(); + // if there is a leading comma, the user is most likely to type out a leading pattern + // so we insert a missing pattern at the beginning for IDE features + if has_leading_comma { + args.insert(0, self.missing_pat()); + } - (args, ellipsis) + (args.into_boxed_slice(), ellipsis) } // endregion: patterns @@ -1493,3 +1516,8 @@ impl ExprCollector<'_> { self.body.labels.alloc(label) } } + +fn comma_follows_token(t: Option) -> bool { + (|| syntax::algo::skip_trivia_token(t?.next_token()?, syntax::Direction::Next))() + .map_or(false, |it| it.kind() == syntax::T![,]) +} diff --git a/crates/ide/src/signature_help.rs b/crates/ide/src/signature_help.rs index 9bd9a948e7f..7795be54e26 100644 --- a/crates/ide/src/signature_help.rs +++ b/crates/ide/src/signature_help.rs @@ -1996,7 +1996,6 @@ fn main() { #[test] fn test_tuple_expr_expected() { - // FIXME: Seems like we discard valuable results in typeck here check( r#" fn main() { @@ -2004,19 +2003,20 @@ fn main() { } "#, expect![[r#" - (&str, u32) - ^^^^ --- + (&str, u32, u32) + ^^^^ --- --- "#]], ); + // FIXME: Should typeck report a 4-ary tuple for the expression here? check( r#" fn main() { - let _: (&str, u32, u32, u32)= ($0, 1, 3); + let _: (&str, u32, u32, u32) = ($0, 1, 3); } "#, expect![[r#" - (&str, u32) - ^^^^ --- + (&str, u32, u32) + ^^^^ --- --- "#]], ); check( @@ -2026,15 +2026,25 @@ fn main() { } "#, expect![[r#" - (&str, u32, u32) - ^^^^ --- --- + (&str, u32, u32, i32) + ^^^^ --- --- --- "#]], ); } #[test] fn test_tuple_pat_free() { - // FIXME: Seems like we discard valuable results in typeck here + check( + r#" +fn main() { + let ($0, 1, 3); +} +"#, + expect![[r#" + ({unknown}, i32, i32) + ^^^^^^^^^ --- --- + "#]], + ); check( r#" fn main() { @@ -2123,10 +2133,9 @@ fn main() { let ($0, 1, 3): (i32, i32, i32); } "#, - // FIXME: tuple pat should be of size 3 ideally expect![[r#" - (i32, i32) - ^^^ --- + (i32, i32, i32) + ^^^ --- --- "#]], ); check( @@ -2182,7 +2191,7 @@ fn main() { let ($0 1, 3) = (1, 2, 3); } "#, - // FIXME: tuple pat should be of size 3 ideally + // FIXME: Should typeck report a 3-ary tuple for the pattern here? expect![[r#" (i32, i32) ^^^ ---