Auto merge of #14938 - Veykril:sig-help, r=Veykril

Add signature help for tuple patterns and expressions

~~These are somewhat wonky since their signature changes as you type depending on context but they help out nevertheless.~~ should be less wonky now with added parser and lowering recoveries

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<ast::Pat>,
+        has_leading_comma: bool,
         binding_list: &mut BindingList,
     ) -> (Box<[PatId]>, Option<usize>) {
         // 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<syntax::SyntaxToken>) -> bool {
+    (|| syntax::algo::skip_trivia_token(t?.next_token()?, syntax::Direction::Next))()
+        .map_or(false, |it| it.kind() == syntax::T![,])
+}

crates/ide/src/signature_help.rs

@@ -15,8 +15,9 @@ use ide_db::{
 use stdx::format_to;
 use syntax::{
     algo,
-    ast::{self, HasArgList},
+    ast::{self, AstChildren, HasArgList},
-    match_ast, AstNode, Direction, SyntaxElementChildren, SyntaxToken, TextRange, TextSize,
+    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<SignatureHelp> {
-    let rest_pat = pat.fields().find(|it| matches!(it, ast::Pat::RestPat(_)));
+    let path = pat.path()?;
-    let is_left_of_rest_pat =
+    let path_res = sema.resolve_path(&path)?;
-        rest_pat.map_or(true, |it| token.text_range().start() < it.syntax().text_range().end());
-
     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
+    Some(signature_help_for_tuple_pat_ish(
-        .syntax()
+        db,
-        .children_with_tokens()
+        res,
-        .filter_map(syntax::NodeOrToken::into_token)
+        pat.syntax(),
-        .filter(|t| t.kind() == syntax::T![,]);
+        token,
-    res.active_parameter = Some(if is_left_of_rest_pat {
+        pat.fields(),
-        commas.take_while(|t| t.text_range().start() <= token.text_range().start()).count()
+        fields.into_iter().map(|it| it.ty(db)),
-    } else {
+    ))
-        let n_commas = commas
+}
-            .collect::<Vec<_>>()
-            .into_iter()
-            .rev()
-            .take_while(|t| t.text_range().start() > token.text_range().start())
-            .count();
-        fields.len().saturating_sub(1).saturating_sub(n_commas)
-    });
 
+fn signature_help_for_tuple_pat(
+    sema: &Semantics<'_, RootDatabase>,
+    pat: ast::TuplePat,
+    token: SyntaxToken,
+) -> Option<SignatureHelp> {
+    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<SignatureHelp> {
+    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<SignatureHelp> {
     let active_parameter = field_list_children
-        .filter_map(syntax::NodeOrToken::into_token)
+        .filter_map(NodeOrToken::into_token)
-        .filter(|t| t.kind() == syntax::T![,])
+        .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<ast::Pat>,
+    fields: impl ExactSizeIterator<Item = hir::Type>,
+) -> 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::<Vec<_>>()
+                .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,290 @@ 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() {
+        check(
+            r#"
+fn main() {
+    let _: (&str, u32, u32)= ($0, 1, 3);
+}
+"#,
+            expect![[r#"
+                (&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);
+}
+"#,
+            expect![[r#"
+                (&str, u32, u32)
+                 ^^^^  ---  ---
+            "#]],
+        );
+        check(
+            r#"
+fn main() {
+    let _: (&str, u32, u32)= ($0, 1, 3, 5);
+}
+"#,
+            expect![[r#"
+                (&str, u32, u32, i32)
+                 ^^^^  ---  ---  ---
+            "#]],
+        );
+    }
+
+    #[test]
+    fn test_tuple_pat_free() {
+        check(
+            r#"
+fn main() {
+    let ($0, 1, 3);
+}
+"#,
+            expect![[r#"
+                ({unknown}, i32, i32)
+                 ^^^^^^^^^  ---  ---
+            "#]],
+        );
+        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);
+}
+"#,
+            expect![[r#"
+                (i32, 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: Should typeck report a 3-ary tuple for the pattern here?
+            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)
+                 ---  ---  ^^^
+            "#]],
+        );
+    }
 }

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;
 

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) {

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

crates/parser/test_data/parser/inline/err/0019_tuple_expr_leading_comma.rs

@@ -0,0 +1,3 @@
+fn foo() {
+    (,);
+}

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

crates/parser/test_data/parser/inline/err/0020_tuple_pat_leading_comma.rs

@@ -0,0 +1,3 @@
+fn foo() {
+    let (,);
+}