From 537b4c63053ade616dcf080830a4ccca97f77e02 Mon Sep 17 00:00:00 2001
From: Luuk Wester <luuk.wester@gmail.com>
Date: Fri, 17 May 2024 21:54:35 +0200
Subject: [PATCH 01/15] implement assist to switch between doc and normal
 comments

---
 .../convert_comment_from_or_to_doc.rs         | 645 ++++++++++++++++++
 crates/ide-assists/src/lib.rs                 |   2 +
 crates/ide-assists/src/tests/generated.rs     |  15 +
 3 files changed, 662 insertions(+)
 create mode 100644 crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs

diff --git a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
new file mode 100644
index 0000000000..1ac30d71f2
--- /dev/null
+++ b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
@@ -0,0 +1,645 @@
+use itertools::Itertools;
+use syntax::{
+    ast::{self, edit::IndentLevel, Comment, CommentPlacement, Whitespace},
+    AstToken, Direction, SyntaxElement, TextRange,
+};
+
+use crate::{AssistContext, AssistId, AssistKind, Assists};
+
+// Assist: comment_to_doc
+//
+// Converts comments to documentation.
+//
+// ```
+// // Wow what $0a nice function
+// // I sure hope this shows up when I hover over it
+// ```
+// ->
+// ```
+// //! Wow what a nice function
+// //! I sure hope this shows up when I hover over it
+// ```
+pub(crate) fn convert_comment_from_or_to_doc(
+    acc: &mut Assists,
+    ctx: &AssistContext<'_>,
+) -> Option<()> {
+    let comment = ctx.find_token_at_offset::<ast::Comment>()?;
+
+    match comment.kind().doc {
+        Some(_) => doc_to_comment(acc, comment),
+        None => match can_be_doc_comment(&comment) {
+            Some(doc_comment_style) => comment_to_doc(acc, comment, doc_comment_style),
+            None => None,
+        },
+    }
+}
+
+fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
+    let target = if comment.kind().shape.is_line() {
+        line_comments_text_range(&comment)?
+    } else {
+        comment.syntax().text_range()
+    };
+
+    acc.add(
+        AssistId("doc_to_comment", AssistKind::RefactorRewrite),
+        "Replace a comment with doc comment",
+        target,
+        |edit| {
+            // We need to either replace the first occurrence of /* with /***, or we need to replace
+            // the occurrences // at the start of each line with ///
+            let output = match comment.kind().shape {
+                ast::CommentShape::Line => {
+                    let indentation = IndentLevel::from_token(comment.syntax());
+                    let line_start = comment.prefix();
+                    relevant_line_comments(&comment)
+                        .iter()
+                        .map(|comment| comment.text())
+                        .flat_map(|text| text.lines())
+                        .map(|line| indentation.to_string() + &line.replacen(line_start, "//", 1))
+                        .join("\n")
+                }
+                ast::CommentShape::Block => {
+                    let block_start = comment.prefix();
+                    comment
+                        .text()
+                        .lines()
+                        .enumerate()
+                        .map(|(idx, line)| {
+                            if idx == 0 {
+                                line.replacen(block_start, "/*", 1)
+                            } else {
+                                line.replacen("*  ", "* ", 1)
+                            }
+                        })
+                        .join("\n")
+                }
+            };
+            edit.replace(target, output)
+        },
+    )
+}
+
+fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacement) -> Option<()> {
+    let target = if comment.kind().shape.is_line() {
+        line_comments_text_range(&comment)?
+    } else {
+        comment.syntax().text_range()
+    };
+
+    acc.add(
+        AssistId("comment_to_doc", AssistKind::RefactorRewrite),
+        "Replace a doc comment with comment",
+        target,
+        |edit| {
+            // We need to either replace the first occurrence of /* with /***, or we need to replace
+            // the occurrences // at the start of each line with ///
+            let output = match comment.kind().shape {
+                ast::CommentShape::Line => {
+                    let line_start = match style {
+                        CommentPlacement::Inner => "//!",
+                        CommentPlacement::Outer => "///",
+                    };
+                    let indentation = IndentLevel::from_token(comment.syntax());
+                    relevant_line_comments(&comment)
+                        .iter()
+                        .map(|comment| comment.text())
+                        .flat_map(|text| text.lines())
+                        .map(|line| indentation.to_string() + &line.replacen("//", line_start, 1))
+                        .join("\n")
+                }
+                ast::CommentShape::Block => {
+                    let block_start = match style {
+                        CommentPlacement::Inner => "/*!",
+                        CommentPlacement::Outer => "/**",
+                    };
+                    comment
+                        .text()
+                        .lines()
+                        .enumerate()
+                        .map(|(idx, line)| {
+                            if idx == 0 {
+                                // On the first line we replace the comment start with a doc comment
+                                // start.
+                                line.replacen("/*", block_start, 1)
+                            } else {
+                                // put one extra space after each * since we moved the first line to
+                                // the right by one column as well.
+                                line.replacen("* ", "*  ", 1)
+                            }
+                        })
+                        .join("\n")
+                }
+            };
+            edit.replace(target, output)
+        },
+    )
+}
+
+/// Not all comments are valid candidates for conversion into doc comments. For example, the
+/// comments in the code:
+/// ```rust
+/// // foos the bar
+/// fn foo_bar(foo: Foo) -> Bar {
+///   // Bar the foo
+///   foo.into_bar()
+/// }
+///
+/// trait A {
+///     // The A trait
+/// }
+/// ```
+/// can be converted to doc comments. However, the comments in this example:
+/// ```rust
+/// fn foo_bar(foo: Foo /* not bar yet */) -> Bar {
+///   foo.into_bar()
+///   // Nicely done
+/// }
+/// // end of function
+///
+/// struct S {
+///     // The S struct
+/// }
+/// ```
+/// are not allowed to become doc comments.
+fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
+    use syntax::SyntaxKind::*;
+
+    // if the comment is not on its own line, then we do not propose anything.
+    match comment.syntax().prev_token() {
+        Some(prev) => {
+            // There was a previous token, now check if it was a newline
+            Whitespace::cast(prev).filter(|w| w.text().contains('\n'))?;
+        }
+        // There is no previous token, this is the start of the file.
+        None => return Some(CommentPlacement::Inner),
+    }
+
+    // check if comment is followed by: `struct`, `trait`, `mod`, `fn`, `type`, `extern crate`, `use`, `const`
+    let parent = comment.syntax().parent();
+    let parent_kind = parent.as_ref().map(|parent| parent.kind());
+    if matches!(
+        parent_kind,
+        Some(STRUCT | TRAIT | MODULE | FN | TYPE_KW | EXTERN_CRATE | USE | CONST)
+    ) {
+        return Some(CommentPlacement::Outer);
+    }
+
+    // check if comment is preceded by: `fn f() {`, `trait T {`, `mod M {`:
+    let third_parent_kind = comment
+        .syntax()
+        .parent()
+        .and_then(|p| p.parent())
+        .and_then(|p| p.parent())
+        .map(|parent| parent.kind());
+    let is_first_item_in_parent = comment
+        .syntax()
+        .siblings_with_tokens(Direction::Prev)
+        .filter_map(|not| not.into_node())
+        .next()
+        .is_none();
+
+    if matches!(parent_kind, Some(STMT_LIST))
+        && is_first_item_in_parent
+        && matches!(third_parent_kind, Some(FN | TRAIT | MODULE))
+    {
+        return Some(CommentPlacement::Inner);
+    }
+
+    None
+}
+
+/// The line -> block assist can  be invoked from anywhere within a sequence of line comments.
+/// relevant_line_comments crawls backwards and forwards finding the complete sequence of comments that will
+/// be joined.
+pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> {
+    // The prefix identifies the kind of comment we're dealing with
+    let prefix = comment.prefix();
+    let same_prefix = |c: &ast::Comment| c.prefix() == prefix;
+
+    // These tokens are allowed to exist between comments
+    let skippable = |not: &SyntaxElement| {
+        not.clone()
+            .into_token()
+            .and_then(Whitespace::cast)
+            .map(|w| !w.spans_multiple_lines())
+            .unwrap_or(false)
+    };
+
+    // Find all preceding comments (in reverse order) that have the same prefix
+    let prev_comments = comment
+        .syntax()
+        .siblings_with_tokens(Direction::Prev)
+        .filter(|s| !skippable(s))
+        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
+        .take_while(|opt_com| opt_com.is_some())
+        .flatten()
+        .skip(1); // skip the first element so we don't duplicate it in next_comments
+
+    let next_comments = comment
+        .syntax()
+        .siblings_with_tokens(Direction::Next)
+        .filter(|s| !skippable(s))
+        .map(|not| not.into_token().and_then(Comment::cast).filter(same_prefix))
+        .take_while(|opt_com| opt_com.is_some())
+        .flatten();
+
+    let mut comments: Vec<_> = prev_comments.collect();
+    comments.reverse();
+    comments.extend(next_comments);
+    comments
+}
+
+fn line_comments_text_range(comment: &ast::Comment) -> Option<TextRange> {
+    let comments = relevant_line_comments(comment);
+    let first = comments.first()?;
+    let indentation = IndentLevel::from_token(first.syntax());
+    let start =
+        first.syntax().text_range().start().checked_sub((indentation.0 as u32 * 4).into())?;
+    let end = comments.last()?.syntax().text_range().end();
+    Some(TextRange::new(start, end))
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    use super::*;
+
+    #[test]
+    fn module_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            // such a nice module$0
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+            //! such a nice module
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_line_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            // unseen$0 docs
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /// unseen docs
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_line_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            // unseen$0 docs
+            // make me seen!
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /// unseen docs
+            /// make me seen!
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_line_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /// visible$0 docs
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            // visible docs
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_line_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /// visible$0 docs
+            /// Hide me!
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            // visible docs
+            // Hide me!
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_line_block_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /* unseen$0 docs */
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /** unseen docs */
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_line_block_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /* unseen$0 docs
+            *  make me seen!
+            */
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /** unseen docs
+            *   make me seen!
+            */
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_line_block_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /** visible$0 docs */
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /* visible docs */
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_line_block_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+
+            /** visible$0 docs
+            *   Hide me!
+            */
+            fn main() {
+                foo();
+            }
+            "#,
+            r#"
+
+            /* visible docs
+            *  Hide me!
+            */
+            fn main() {
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_inner_line_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                // unseen$0 docs
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                //! unseen docs
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_inner_line_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                // unseen$0 docs
+                // make me seen!
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                //! unseen docs
+                //! make me seen!
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_inner_line_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                //! visible$0 docs
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                // visible docs
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_inner_line_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                //! visible$0 docs
+                //! Hide me!
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                // visible docs
+                // Hide me!
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_inner_line_block_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                /* unseen$0 docs */
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                /*! unseen docs */
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_inner_line_block_comment_to_doc() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                /* unseen$0 docs
+                *  make me seen!
+                */
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                /*! unseen docs
+                *   make me seen!
+                */
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn single_inner_line_block_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                /*! visible$0 docs */
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                /* visible docs */
+                foo();
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn multi_inner_line_block_doc_to_comment() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                /*! visible$0 docs
+                *   Hide me!
+                */
+                foo();
+            }
+            "#,
+            r#"
+            fn main() {
+                /* visible docs
+                *  Hide me!
+                */
+                foo();
+            }
+        "#,
+        );
+    }
+
+    #[test]
+    fn not_overeager() {
+        check_assist_not_applicable(
+            convert_comment_from_or_to_doc,
+            r#"
+            fn main() {
+                foo();
+                // $0well that settles main
+            }
+            // $1 nicely done
+            "#,
+        );
+    }
+}
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index 0df5e913a5..cbaf03e4d1 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -116,6 +116,7 @@ mod handlers {
     mod change_visibility;
     mod convert_bool_then;
     mod convert_comment_block;
+    mod convert_comment_from_or_to_doc;
     mod convert_from_to_tryfrom;
     mod convert_integer_literal;
     mod convert_into_to_from;
@@ -239,6 +240,7 @@ mod handlers {
             convert_bool_then::convert_bool_then_to_if,
             convert_bool_then::convert_if_to_bool_then,
             convert_comment_block::convert_comment_block,
+            convert_comment_from_or_to_doc::convert_comment_from_or_to_doc,
             convert_from_to_tryfrom::convert_from_to_tryfrom,
             convert_integer_literal::convert_integer_literal,
             convert_into_to_from::convert_into_to_from,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index 937e78f8d7..bd841d72e6 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -345,6 +345,21 @@ pub(crate) fn frobnicate() {}
     )
 }
 
+#[test]
+fn doctest_comment_to_doc() {
+    check_doc_test(
+        "comment_to_doc",
+        r#####"
+// Wow what $0a nice function
+// I sure hope this shows up when I hover over it
+"#####,
+        r#####"
+//! Wow what a nice function
+//! I sure hope this shows up when I hover over it
+"#####,
+    )
+}
+
 #[test]
 fn doctest_convert_bool_then_to_if() {
     check_doc_test(

From 265707857379e12490a344ad7cb70af5edec59e8 Mon Sep 17 00:00:00 2001
From: maxwase <max.vvase@gmail.com>
Date: Sat, 18 May 2024 19:37:04 +0300
Subject: [PATCH 02/15] Add toggle_async_sugar assist code action

---
 .../src/handlers/toggle_async_sugar.rs        | 460 ++++++++++++++++++
 crates/ide-assists/src/lib.rs                 |   2 +
 crates/ide-assists/src/tests/generated.rs     |  17 +
 crates/ide-db/src/famous_defs.rs              |   4 +
 4 files changed, 483 insertions(+)
 create mode 100644 crates/ide-assists/src/handlers/toggle_async_sugar.rs

diff --git a/crates/ide-assists/src/handlers/toggle_async_sugar.rs b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
new file mode 100644
index 0000000000..ea127d65e5
--- /dev/null
+++ b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
@@ -0,0 +1,460 @@
+use ide_db::{
+    assists::{AssistId, AssistKind},
+    famous_defs::FamousDefs,
+};
+use syntax::{
+    ast::{self, HasVisibility},
+    AstNode, NodeOrToken, SyntaxKind, SyntaxNode, SyntaxToken, TextRange,
+};
+
+use crate::{AssistContext, Assists};
+
+// Assist: toggle_async_sugar
+//
+// Rewrites asynchronous function into `impl Future` and back.
+// This action does not touch the function body and therefore `async { 0 }`
+// block does not transform to just `0`.
+//
+// ```
+// pub async f$0n foo() -> usize {
+//     0
+// }
+// ```
+// ->
+// ```
+// pub fn foo() -> impl Future<Output = usize> {
+//     0
+// }
+// ```
+pub(crate) fn toggle_async_sugar(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
+    let function: ast::Fn = ctx.find_node_at_offset()?;
+    match (function.async_token(), function.ret_type()) {
+        // async function returning futures cannot be flattened
+        // const async is not yet supported
+        (None, Some(ret_type)) if function.const_token().is_none() => {
+            add_async(acc, ctx, function, ret_type)
+        }
+        (Some(async_token), ret_type) => remove_async(function, ret_type, acc, async_token),
+        _ => None,
+    }
+}
+
+fn add_async(
+    acc: &mut Assists,
+    ctx: &AssistContext<'_>,
+    function: ast::Fn,
+    ret_type: ast::RetType,
+) -> Option<()> {
+    let ast::Type::ImplTraitType(return_impl_trait) = ret_type.ty()? else {
+        return None;
+    };
+
+    let main_trait_path = return_impl_trait
+        .type_bound_list()?
+        .bounds()
+        .filter_map(|bound| match bound.ty() {
+            Some(ast::Type::PathType(trait_path)) => trait_path.path(),
+            _ => None,
+        })
+        .next()?;
+
+    let trait_type = ctx.sema.resolve_trait(&main_trait_path)?;
+    let scope = ctx.sema.scope(main_trait_path.syntax())?;
+    if trait_type != FamousDefs(&ctx.sema, scope.krate()).core_future_Future()? {
+        return None;
+    }
+    let future_output = unwrap_future_output(main_trait_path)?;
+
+    acc.add(
+        AssistId("toggle_async_sugar", AssistKind::RefactorRewrite),
+        "Convert `impl Future` into async",
+        function.syntax().text_range(),
+        |builder| {
+            match future_output {
+                ast::Type::TupleType(_) => {
+                    let mut ret_type_range = ret_type.syntax().text_range();
+
+                    // find leftover whitespace
+                    let whitespace_range = function
+                        .param_list()
+                        .as_ref()
+                        .map(|params| NodeOrToken::Node(params.syntax()))
+                        .and_then(following_whitespace);
+
+                    if let Some(whitespace_range) = whitespace_range {
+                        ret_type_range =
+                            TextRange::new(whitespace_range.start(), ret_type_range.end());
+                    }
+
+                    builder.delete(ret_type_range);
+                }
+                _ => {
+                    builder.replace(
+                        return_impl_trait.syntax().text_range(),
+                        future_output.syntax().text(),
+                    );
+                }
+            }
+
+            let (place_for_async, async_kw) = match function.visibility() {
+                Some(vis) => (vis.syntax().text_range().end(), " async"),
+                None => (function.syntax().text_range().start(), "async "),
+            };
+            builder.insert(place_for_async, async_kw);
+        },
+    )
+}
+
+fn remove_async(
+    function: ast::Fn,
+    ret_type: Option<ast::RetType>,
+    acc: &mut Assists,
+    async_token: SyntaxToken,
+) -> Option<()> {
+    let rparen = function.param_list()?.r_paren_token()?;
+    let return_type = match ret_type {
+        // unable to get a `ty` makes the action unapplicable
+        Some(ret_type) => Some(ret_type.ty()?),
+        // No type means `-> ()`
+        None => None,
+    };
+
+    acc.add(
+        AssistId("toggle_async_sugar", AssistKind::RefactorRewrite),
+        "Convert async into `impl Future`",
+        function.syntax().text_range(),
+        |builder| {
+            let mut async_range = async_token.text_range();
+
+            if let Some(whitespace_range) = following_whitespace(NodeOrToken::Token(async_token)) {
+                async_range = TextRange::new(async_range.start(), whitespace_range.end());
+            }
+            builder.delete(async_range);
+
+            match return_type {
+                Some(ret_type) => builder.replace(
+                    ret_type.syntax().text_range(),
+                    format!("impl Future<Output = {ret_type}>"),
+                ),
+                None => builder.insert(rparen.text_range().end(), " -> impl Future<Output = ()>"),
+            }
+        },
+    )
+}
+
+fn unwrap_future_output(path: ast::Path) -> Option<ast::Type> {
+    let future_trait = path.segments().last()?;
+    let assoc_list = future_trait.generic_arg_list()?;
+    let future_assoc = assoc_list.generic_args().next()?;
+    match future_assoc {
+        ast::GenericArg::AssocTypeArg(output_type) => output_type.ty(),
+        _ => None,
+    }
+}
+
+fn following_whitespace(nt: NodeOrToken<&SyntaxNode, SyntaxToken>) -> Option<TextRange> {
+    let next_token = match nt {
+        NodeOrToken::Node(node) => node.next_sibling_or_token(),
+        NodeOrToken::Token(token) => token.next_sibling_or_token(),
+    }?;
+    (next_token.kind() == SyntaxKind::WHITESPACE).then_some(next_token.text_range())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    #[test]
+    fn sugar_with_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    f$0n foo() -> impl Future<Output = ()> {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future::Future;
+    async fn foo() {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    f$0n foo() -> impl Future<Output = usize> {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future::Future;
+    async fn foo() -> usize {
+        todo!()
+    }
+    "#,
+        );
+    }
+
+    #[test]
+    fn desugar_with_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    async f$0n foo() {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future::Future;
+    fn foo() -> impl Future<Output = ()> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    async f$0n foo() -> usize {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future::Future;
+    fn foo() -> impl Future<Output = usize> {
+        todo!()
+    }
+    "#,
+        );
+    }
+
+    #[test]
+    fn sugar_without_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = ()> {
+        todo!()
+    }
+    "#,
+            r#"
+    async fn foo() {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = usize> {
+        todo!()
+    }
+    "#,
+            r#"
+    async fn foo() -> usize {
+        todo!()
+    }
+    "#,
+        );
+    }
+
+    #[test]
+    fn desugar_without_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    async f$0n foo() {
+        todo!()
+    }
+    "#,
+            r#"
+    fn foo() -> impl Future<Output = ()> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    async f$0n foo() -> usize {
+        todo!()
+    }
+    "#,
+            r#"
+    fn foo() -> impl Future<Output = usize> {
+        todo!()
+    }
+    "#,
+        );
+    }
+
+    #[test]
+    fn sugar_not_applicable() {
+        check_assist_not_applicable(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    trait Future {
+        type Output;
+    }
+    f$0n foo() -> impl Future<Output = ()> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist_not_applicable(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    trait Future {
+        type Output;
+    }
+    f$0n foo() -> impl Future<Output = usize> {
+        todo!()
+    }
+    "#,
+        );
+    }
+
+    #[test]
+    fn sugar_definition_with_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    f$0n foo() -> impl Future<Output = ()>;
+    "#,
+            r#"
+    use core::future::Future;
+    async fn foo();
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    f$0n foo() -> impl Future<Output = usize>;
+    "#,
+            r#"
+    use core::future::Future;
+    async fn foo() -> usize;
+    "#,
+        );
+    }
+
+    #[test]
+    fn sugar_definition_without_use() {
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = ()>;
+    "#,
+            r#"
+    async fn foo();
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = usize>;
+    "#,
+            r#"
+    async fn foo() -> usize;
+    "#,
+        );
+    }
+
+    #[test]
+    fn sugar_with_modifiers() {
+        check_assist_not_applicable(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    const f$0n foo() -> impl core::future::Future<Output = ()>;
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+            //- minicore: future
+            pub(crate) unsafe f$0n foo() -> impl core::future::Future<Output = usize>;
+        "#,
+            r#"
+            pub(crate) async unsafe fn foo() -> usize;
+        "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    unsafe f$0n foo() -> impl core::future::Future<Output = ()>;
+    "#,
+            r#"
+    async unsafe fn foo();
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    unsafe extern "C" f$0n foo() -> impl core::future::Future<Output = ()>;
+    "#,
+            r#"
+    async unsafe extern "C" fn foo();
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo<T>() -> impl core::future::Future<Output = T>;
+    "#,
+            r#"
+    async fn foo<T>() -> T;
+    "#,
+        );
+
+        check_assist(
+            toggle_async_sugar,
+            r#"
+    //- minicore: future
+    f$0n foo<T>() -> impl core::future::Future<Output = T>
+    where
+        T: Sized;
+    "#,
+            r#"
+    async fn foo<T>() -> T
+    where
+        T: Sized;
+    "#,
+        );
+    }
+}
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index 0df5e913a5..d26ac23099 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -209,6 +209,7 @@ mod handlers {
     mod sort_items;
     mod split_import;
     mod term_search;
+    mod toggle_async_sugar;
     mod toggle_ignore;
     mod unmerge_match_arm;
     mod unmerge_use;
@@ -238,6 +239,7 @@ mod handlers {
             change_visibility::change_visibility,
             convert_bool_then::convert_bool_then_to_if,
             convert_bool_then::convert_if_to_bool_then,
+            toggle_async_sugar::toggle_async_sugar,
             convert_comment_block::convert_comment_block,
             convert_from_to_tryfrom::convert_from_to_tryfrom,
             convert_integer_literal::convert_integer_literal,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index 937e78f8d7..8e0d1bd667 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -3020,6 +3020,23 @@ use std::{collections::HashMap};
     )
 }
 
+#[test]
+fn doctest_toggle_async_sugar() {
+    check_doc_test(
+        "toggle_async_sugar",
+        r#####"
+pub async f$0n foo() -> usize {
+    0
+}
+"#####,
+        r#####"
+pub fn foo() -> impl Future<Output = usize> {
+    0
+}
+"#####,
+    )
+}
+
 #[test]
 fn doctest_toggle_ignore() {
     check_doc_test(
diff --git a/crates/ide-db/src/famous_defs.rs b/crates/ide-db/src/famous_defs.rs
index 3106772e63..e445e9fb68 100644
--- a/crates/ide-db/src/famous_defs.rs
+++ b/crates/ide-db/src/famous_defs.rs
@@ -106,6 +106,10 @@ impl FamousDefs<'_, '_> {
         self.find_trait("core:marker:Copy")
     }
 
+    pub fn core_future_Future(&self) -> Option<Trait> {
+        self.find_trait("core:future:Future")
+    }
+
     pub fn core_macros_builtin_derive(&self) -> Option<Macro> {
         self.find_macro("core:macros:builtin:derive")
     }

From 624f99b4b9f66cd056fc07a83fbb287fd4960aa5 Mon Sep 17 00:00:00 2001
From: maxwase <max.vvase@gmail.com>
Date: Fri, 24 May 2024 01:10:18 +0300
Subject: [PATCH 03/15] Review fixes: Split into 2, check tuple fields

---
 .../src/handlers/toggle_async_sugar.rs        | 161 ++++++++++++------
 crates/ide-assists/src/lib.rs                 |   3 +-
 crates/ide-assists/src/tests/generated.rs     |  30 +++-
 3 files changed, 137 insertions(+), 57 deletions(-)

diff --git a/crates/ide-assists/src/handlers/toggle_async_sugar.rs b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
index ea127d65e5..356e1d50ae 100644
--- a/crates/ide-assists/src/handlers/toggle_async_sugar.rs
+++ b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
@@ -9,42 +9,38 @@ use syntax::{
 
 use crate::{AssistContext, Assists};
 
-// Assist: toggle_async_sugar
+// Assist: sugar_impl_future_into_async
 //
-// Rewrites asynchronous function into `impl Future` and back.
+// Rewrites asynchronous function from `impl Future` to `async fn`.
 // This action does not touch the function body and therefore `async { 0 }`
 // block does not transform to just `0`.
 //
 // ```
-// pub async f$0n foo() -> usize {
-//     0
+// # //- minicore: future
+// pub f$0n foo() -> impl core::future::Future<Output = usize> {
+//     async { 0 }
 // }
 // ```
 // ->
 // ```
-// pub fn foo() -> impl Future<Output = usize> {
-//     0
+// pub async fn foo() -> usize {
+//     async { 0 }
 // }
 // ```
-pub(crate) fn toggle_async_sugar(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
-    let function: ast::Fn = ctx.find_node_at_offset()?;
-    match (function.async_token(), function.ret_type()) {
-        // async function returning futures cannot be flattened
-        // const async is not yet supported
-        (None, Some(ret_type)) if function.const_token().is_none() => {
-            add_async(acc, ctx, function, ret_type)
-        }
-        (Some(async_token), ret_type) => remove_async(function, ret_type, acc, async_token),
-        _ => None,
-    }
-}
-
-fn add_async(
+pub(crate) fn sugar_impl_future_into_async(
     acc: &mut Assists,
     ctx: &AssistContext<'_>,
-    function: ast::Fn,
-    ret_type: ast::RetType,
 ) -> Option<()> {
+    let function: ast::Fn = ctx.find_node_at_offset()?;
+    if function.async_token().is_some() {
+        return None;
+    }
+
+    let ret_type = function.ret_type()?;
+    if function.const_token().is_some() {
+        return None;
+    }
+
     let ast::Type::ImplTraitType(return_impl_trait) = ret_type.ty()? else {
         return None;
     };
@@ -66,12 +62,12 @@ fn add_async(
     let future_output = unwrap_future_output(main_trait_path)?;
 
     acc.add(
-        AssistId("toggle_async_sugar", AssistKind::RefactorRewrite),
+        AssistId("sugar_impl_future_into_async", AssistKind::RefactorRewrite),
         "Convert `impl Future` into async",
         function.syntax().text_range(),
         |builder| {
             match future_output {
-                ast::Type::TupleType(_) => {
+                ast::Type::TupleType(t) if t.fields().next().is_none() => {
                     let mut ret_type_range = ret_type.syntax().text_range();
 
                     // find leftover whitespace
@@ -105,14 +101,32 @@ fn add_async(
     )
 }
 
-fn remove_async(
-    function: ast::Fn,
-    ret_type: Option<ast::RetType>,
+// Assist: desugar_async_into_impl_future
+//
+// Rewrites asynchronous function from `async fn` to `impl Future`.
+// This action does not touch the function body and therefore `0`
+// block does not transform to `async { 0 }`.
+//
+// ```
+// pub async f$0n foo() -> usize {
+//     0
+// }
+// ```
+// ->
+// ```
+// pub fn foo() -> impl Future<Output = usize> {
+//     0
+// }
+// ```
+pub(crate) fn desugar_async_into_impl_future(
     acc: &mut Assists,
-    async_token: SyntaxToken,
+    ctx: &AssistContext<'_>,
 ) -> Option<()> {
+    let function: ast::Fn = ctx.find_node_at_offset()?;
+    let async_token = function.async_token()?;
+
     let rparen = function.param_list()?.r_paren_token()?;
-    let return_type = match ret_type {
+    let return_type = match function.ret_type() {
         // unable to get a `ty` makes the action unapplicable
         Some(ret_type) => Some(ret_type.ty()?),
         // No type means `-> ()`
@@ -120,7 +134,7 @@ fn remove_async(
     };
 
     acc.add(
-        AssistId("toggle_async_sugar", AssistKind::RefactorRewrite),
+        AssistId("desugar_async_into_impl_future", AssistKind::RefactorRewrite),
         "Convert async into `impl Future`",
         function.syntax().text_range(),
         |builder| {
@@ -168,7 +182,7 @@ mod tests {
     #[test]
     fn sugar_with_use() {
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -185,7 +199,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -205,7 +219,7 @@ mod tests {
     #[test]
     fn desugar_with_use() {
         check_assist(
-            toggle_async_sugar,
+            desugar_async_into_impl_future,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -222,7 +236,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            desugar_async_into_impl_future,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -242,7 +256,7 @@ mod tests {
     #[test]
     fn sugar_without_use() {
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo() -> impl core::future::Future<Output = ()> {
@@ -257,7 +271,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo() -> impl core::future::Future<Output = usize> {
@@ -275,7 +289,7 @@ mod tests {
     #[test]
     fn desugar_without_use() {
         check_assist(
-            toggle_async_sugar,
+            desugar_async_into_impl_future,
             r#"
     //- minicore: future
     async f$0n foo() {
@@ -290,7 +304,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            desugar_async_into_impl_future,
             r#"
     //- minicore: future
     async f$0n foo() -> usize {
@@ -308,7 +322,7 @@ mod tests {
     #[test]
     fn sugar_not_applicable() {
         check_assist_not_applicable(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     trait Future {
@@ -321,7 +335,7 @@ mod tests {
         );
 
         check_assist_not_applicable(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     trait Future {
@@ -337,7 +351,7 @@ mod tests {
     #[test]
     fn sugar_definition_with_use() {
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -350,7 +364,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     use core::future::Future;
@@ -366,7 +380,7 @@ mod tests {
     #[test]
     fn sugar_definition_without_use() {
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo() -> impl core::future::Future<Output = ()>;
@@ -377,7 +391,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo() -> impl core::future::Future<Output = usize>;
@@ -388,10 +402,57 @@ mod tests {
         );
     }
 
+    #[test]
+    fn sugar_more_types() {
+        check_assist(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = ()> + Send + Sync;
+    "#,
+            r#"
+    async fn foo();
+    "#,
+        );
+
+        check_assist(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = usize> + Debug;
+    "#,
+            r#"
+    async fn foo() -> usize;
+    "#,
+        );
+
+        check_assist(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = (usize)> + Debug;
+    "#,
+            r#"
+    async fn foo() -> (usize);
+    "#,
+        );
+
+        check_assist(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = (usize, usize)> + Debug;
+    "#,
+            r#"
+    async fn foo() -> (usize, usize);
+    "#,
+        );
+    }
+
     #[test]
     fn sugar_with_modifiers() {
         check_assist_not_applicable(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     const f$0n foo() -> impl core::future::Future<Output = ()>;
@@ -399,7 +460,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
             //- minicore: future
             pub(crate) unsafe f$0n foo() -> impl core::future::Future<Output = usize>;
@@ -410,7 +471,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     unsafe f$0n foo() -> impl core::future::Future<Output = ()>;
@@ -421,7 +482,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     unsafe extern "C" f$0n foo() -> impl core::future::Future<Output = ()>;
@@ -432,7 +493,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo<T>() -> impl core::future::Future<Output = T>;
@@ -443,7 +504,7 @@ mod tests {
         );
 
         check_assist(
-            toggle_async_sugar,
+            sugar_impl_future_into_async,
             r#"
     //- minicore: future
     f$0n foo<T>() -> impl core::future::Future<Output = T>
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index d26ac23099..34ef341c44 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -239,7 +239,8 @@ mod handlers {
             change_visibility::change_visibility,
             convert_bool_then::convert_bool_then_to_if,
             convert_bool_then::convert_if_to_bool_then,
-            toggle_async_sugar::toggle_async_sugar,
+            toggle_async_sugar::desugar_async_into_impl_future,
+            toggle_async_sugar::sugar_impl_future_into_async,
             convert_comment_block::convert_comment_block,
             convert_from_to_tryfrom::convert_from_to_tryfrom,
             convert_integer_literal::convert_integer_literal,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index 8e0d1bd667..5f187880b0 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -800,6 +800,23 @@ fn main() {
     )
 }
 
+#[test]
+fn doctest_desugar_async_into_impl_future() {
+    check_doc_test(
+        "desugar_async_into_impl_future",
+        r#####"
+pub async f$0n foo() -> usize {
+    0
+}
+"#####,
+        r#####"
+pub fn foo() -> impl Future<Output = usize> {
+    0
+}
+"#####,
+    )
+}
+
 #[test]
 fn doctest_desugar_doc_comment() {
     check_doc_test(
@@ -3021,17 +3038,18 @@ use std::{collections::HashMap};
 }
 
 #[test]
-fn doctest_toggle_async_sugar() {
+fn doctest_sugar_impl_future_into_async() {
     check_doc_test(
-        "toggle_async_sugar",
+        "sugar_impl_future_into_async",
         r#####"
-pub async f$0n foo() -> usize {
-    0
+//- minicore: future
+pub f$0n foo() -> impl core::future::Future<Output = usize> {
+    async { 0 }
 }
 "#####,
         r#####"
-pub fn foo() -> impl Future<Output = usize> {
-    0
+pub async fn foo() -> usize {
+    async { 0 }
 }
 "#####,
     )

From 61f8ef5d5747858c99db8338408036a6939083d7 Mon Sep 17 00:00:00 2001
From: maxwase <max.vvase@gmail.com>
Date: Fri, 24 May 2024 01:39:46 +0300
Subject: [PATCH 04/15] Review fixes: Assist scope, trait qualify

---
 .../src/handlers/toggle_async_sugar.rs        | 164 +++++++++++++-----
 crates/ide-assists/src/tests/generated.rs     |   7 +-
 2 files changed, 126 insertions(+), 45 deletions(-)

diff --git a/crates/ide-assists/src/handlers/toggle_async_sugar.rs b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
index 356e1d50ae..30e09648ea 100644
--- a/crates/ide-assists/src/handlers/toggle_async_sugar.rs
+++ b/crates/ide-assists/src/handlers/toggle_async_sugar.rs
@@ -1,3 +1,4 @@
+use hir::{ImportPathConfig, ModuleDef};
 use ide_db::{
     assists::{AssistId, AssistKind},
     famous_defs::FamousDefs,
@@ -11,13 +12,13 @@ use crate::{AssistContext, Assists};
 
 // Assist: sugar_impl_future_into_async
 //
-// Rewrites asynchronous function from `impl Future` to `async fn`.
+// Rewrites asynchronous function from `-> impl Future` into `async fn`.
 // This action does not touch the function body and therefore `async { 0 }`
 // block does not transform to just `0`.
 //
 // ```
 // # //- minicore: future
-// pub f$0n foo() -> impl core::future::Future<Output = usize> {
+// pub fn foo() -> impl core::future::F$0uture<Output = usize> {
 //     async { 0 }
 // }
 // ```
@@ -31,13 +32,10 @@ pub(crate) fn sugar_impl_future_into_async(
     acc: &mut Assists,
     ctx: &AssistContext<'_>,
 ) -> Option<()> {
-    let function: ast::Fn = ctx.find_node_at_offset()?;
-    if function.async_token().is_some() {
-        return None;
-    }
+    let ret_type: ast::RetType = ctx.find_node_at_offset()?;
+    let function = ret_type.syntax().parent().and_then(ast::Fn::cast)?;
 
-    let ret_type = function.ret_type()?;
-    if function.const_token().is_some() {
+    if function.async_token().is_some() || function.const_token().is_some() {
         return None;
     }
 
@@ -67,6 +65,7 @@ pub(crate) fn sugar_impl_future_into_async(
         function.syntax().text_range(),
         |builder| {
             match future_output {
+                // Empty tuple
                 ast::Type::TupleType(t) if t.fields().next().is_none() => {
                     let mut ret_type_range = ret_type.syntax().text_range();
 
@@ -103,18 +102,19 @@ pub(crate) fn sugar_impl_future_into_async(
 
 // Assist: desugar_async_into_impl_future
 //
-// Rewrites asynchronous function from `async fn` to `impl Future`.
+// Rewrites asynchronous function from `async fn` into `-> impl Future`.
 // This action does not touch the function body and therefore `0`
 // block does not transform to `async { 0 }`.
 //
 // ```
-// pub async f$0n foo() -> usize {
+// # //- minicore: future
+// pub as$0ync fn foo() -> usize {
 //     0
 // }
 // ```
 // ->
 // ```
-// pub fn foo() -> impl Future<Output = usize> {
+// pub fn foo() -> impl core::future::Future<Output = usize> {
 //     0
 // }
 // ```
@@ -122,8 +122,8 @@ pub(crate) fn desugar_async_into_impl_future(
     acc: &mut Assists,
     ctx: &AssistContext<'_>,
 ) -> Option<()> {
-    let function: ast::Fn = ctx.find_node_at_offset()?;
-    let async_token = function.async_token()?;
+    let async_token = ctx.find_token_syntax_at_offset(SyntaxKind::ASYNC_KW)?;
+    let function = async_token.parent().and_then(ast::Fn::cast)?;
 
     let rparen = function.param_list()?.r_paren_token()?;
     let return_type = match function.ret_type() {
@@ -133,6 +133,19 @@ pub(crate) fn desugar_async_into_impl_future(
         None => None,
     };
 
+    let scope = ctx.sema.scope(function.syntax())?;
+    let module = scope.module();
+    let future_trait = FamousDefs(&ctx.sema, scope.krate()).core_future_Future()?;
+    let trait_path = module.find_path(
+        ctx.db(),
+        ModuleDef::Trait(future_trait),
+        ImportPathConfig {
+            prefer_no_std: ctx.config.prefer_no_std,
+            prefer_prelude: ctx.config.prefer_prelude,
+        },
+    )?;
+    let trait_path = trait_path.display(ctx.db());
+
     acc.add(
         AssistId("desugar_async_into_impl_future", AssistKind::RefactorRewrite),
         "Convert async into `impl Future`",
@@ -148,9 +161,12 @@ pub(crate) fn desugar_async_into_impl_future(
             match return_type {
                 Some(ret_type) => builder.replace(
                     ret_type.syntax().text_range(),
-                    format!("impl Future<Output = {ret_type}>"),
+                    format!("impl {trait_path}<Output = {ret_type}>"),
+                ),
+                None => builder.insert(
+                    rparen.text_range().end(),
+                    format!(" -> impl {trait_path}<Output = ()>"),
                 ),
-                None => builder.insert(rparen.text_range().end(), " -> impl Future<Output = ()>"),
             }
         },
     )
@@ -186,7 +202,7 @@ mod tests {
             r#"
     //- minicore: future
     use core::future::Future;
-    f$0n foo() -> impl Future<Output = ()> {
+    fn foo() -> impl F$0uture<Output = ()> {
         todo!()
     }
     "#,
@@ -203,7 +219,7 @@ mod tests {
             r#"
     //- minicore: future
     use core::future::Future;
-    f$0n foo() -> impl Future<Output = usize> {
+    fn foo() -> impl F$0uture<Output = usize> {
         todo!()
     }
     "#,
@@ -223,7 +239,7 @@ mod tests {
             r#"
     //- minicore: future
     use core::future::Future;
-    async f$0n foo() {
+    as$0ync fn foo() {
         todo!()
     }
     "#,
@@ -239,8 +255,25 @@ mod tests {
             desugar_async_into_impl_future,
             r#"
     //- minicore: future
+    use core::future;
+    as$0ync fn foo() {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future;
+    fn foo() -> impl future::Future<Output = ()> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist(
+            desugar_async_into_impl_future,
+            r#"
+    //- minicore: future
     use core::future::Future;
-    async f$0n foo() -> usize {
+    as$0ync fn foo() -> usize {
         todo!()
     }
     "#,
@@ -249,6 +282,23 @@ mod tests {
     fn foo() -> impl Future<Output = usize> {
         todo!()
     }
+    "#,
+        );
+
+        check_assist(
+            desugar_async_into_impl_future,
+            r#"
+    //- minicore: future
+    use core::future::Future;
+    as$0ync fn foo() -> impl Future<Output = usize> {
+        todo!()
+    }
+    "#,
+            r#"
+    use core::future::Future;
+    fn foo() -> impl Future<Output = impl Future<Output = usize>> {
+        todo!()
+    }
     "#,
         );
     }
@@ -259,7 +309,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = ()> {
+    fn foo() -> impl core::future::F$0uture<Output = ()> {
         todo!()
     }
     "#,
@@ -274,7 +324,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = usize> {
+    fn foo() -> impl core::future::F$0uture<Output = usize> {
         todo!()
     }
     "#,
@@ -292,12 +342,12 @@ mod tests {
             desugar_async_into_impl_future,
             r#"
     //- minicore: future
-    async f$0n foo() {
+    as$0ync fn foo() {
         todo!()
     }
     "#,
             r#"
-    fn foo() -> impl Future<Output = ()> {
+    fn foo() -> impl core::future::Future<Output = ()> {
         todo!()
     }
     "#,
@@ -307,12 +357,12 @@ mod tests {
             desugar_async_into_impl_future,
             r#"
     //- minicore: future
-    async f$0n foo() -> usize {
+    as$0ync fn foo() -> usize {
         todo!()
     }
     "#,
             r#"
-    fn foo() -> impl Future<Output = usize> {
+    fn foo() -> impl core::future::Future<Output = usize> {
         todo!()
     }
     "#,
@@ -320,7 +370,7 @@ mod tests {
     }
 
     #[test]
-    fn sugar_not_applicable() {
+    fn not_applicable() {
         check_assist_not_applicable(
             sugar_impl_future_into_async,
             r#"
@@ -328,7 +378,7 @@ mod tests {
     trait Future {
         type Output;
     }
-    f$0n foo() -> impl Future<Output = ()> {
+    fn foo() -> impl F$0uture<Output = ()> {
         todo!()
     }
     "#,
@@ -341,7 +391,26 @@ mod tests {
     trait Future {
         type Output;
     }
-    f$0n foo() -> impl Future<Output = usize> {
+    fn foo() -> impl F$0uture<Output = usize> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist_not_applicable(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    f$0n foo() -> impl core::future::Future<Output = usize> {
+        todo!()
+    }
+    "#,
+        );
+
+        check_assist_not_applicable(
+            desugar_async_into_impl_future,
+            r#"
+    async f$0n foo() {
         todo!()
     }
     "#,
@@ -355,7 +424,7 @@ mod tests {
             r#"
     //- minicore: future
     use core::future::Future;
-    f$0n foo() -> impl Future<Output = ()>;
+    fn foo() -> impl F$0uture<Output = ()>;
     "#,
             r#"
     use core::future::Future;
@@ -368,7 +437,7 @@ mod tests {
             r#"
     //- minicore: future
     use core::future::Future;
-    f$0n foo() -> impl Future<Output = usize>;
+    fn foo() -> impl F$0uture<Output = usize>;
     "#,
             r#"
     use core::future::Future;
@@ -383,7 +452,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = ()>;
+    fn foo() -> impl core::future::F$0uture<Output = ()>;
     "#,
             r#"
     async fn foo();
@@ -394,7 +463,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = usize>;
+    fn foo() -> impl core::future::F$0uture<Output = usize>;
     "#,
             r#"
     async fn foo() -> usize;
@@ -408,7 +477,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = ()> + Send + Sync;
+    fn foo() -> impl core::future::F$0uture<Output = ()> + Send + Sync;
     "#,
             r#"
     async fn foo();
@@ -419,7 +488,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = usize> + Debug;
+    fn foo() -> impl core::future::F$0uture<Output = usize> + Debug;
     "#,
             r#"
     async fn foo() -> usize;
@@ -430,7 +499,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = (usize)> + Debug;
+    fn foo() -> impl core::future::F$0uture<Output = (usize)> + Debug;
     "#,
             r#"
     async fn foo() -> (usize);
@@ -441,10 +510,21 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo() -> impl core::future::Future<Output = (usize, usize)> + Debug;
+    fn foo() -> impl core::future::F$0uture<Output = (usize, usize)> + Debug;
     "#,
             r#"
     async fn foo() -> (usize, usize);
+    "#,
+        );
+
+        check_assist(
+            sugar_impl_future_into_async,
+            r#"
+    //- minicore: future
+    fn foo() -> impl core::future::Future<Output = impl core::future::F$0uture<Output = ()> + Send>;
+    "#,
+            r#"
+    async fn foo() -> impl core::future::Future<Output = ()> + Send;
     "#,
         );
     }
@@ -455,7 +535,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    const f$0n foo() -> impl core::future::Future<Output = ()>;
+    const fn foo() -> impl core::future::F$0uture<Output = ()>;
     "#,
         );
 
@@ -463,7 +543,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
             //- minicore: future
-            pub(crate) unsafe f$0n foo() -> impl core::future::Future<Output = usize>;
+            pub(crate) unsafe fn foo() -> impl core::future::F$0uture<Output = usize>;
         "#,
             r#"
             pub(crate) async unsafe fn foo() -> usize;
@@ -474,7 +554,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    unsafe f$0n foo() -> impl core::future::Future<Output = ()>;
+    unsafe fn foo() -> impl core::future::F$0uture<Output = ()>;
     "#,
             r#"
     async unsafe fn foo();
@@ -485,7 +565,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    unsafe extern "C" f$0n foo() -> impl core::future::Future<Output = ()>;
+    unsafe extern "C" fn foo() -> impl core::future::F$0uture<Output = ()>;
     "#,
             r#"
     async unsafe extern "C" fn foo();
@@ -496,7 +576,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo<T>() -> impl core::future::Future<Output = T>;
+    fn foo<T>() -> impl core::future::F$0uture<Output = T>;
     "#,
             r#"
     async fn foo<T>() -> T;
@@ -507,7 +587,7 @@ mod tests {
             sugar_impl_future_into_async,
             r#"
     //- minicore: future
-    f$0n foo<T>() -> impl core::future::Future<Output = T>
+    fn foo<T>() -> impl core::future::F$0uture<Output = T>
     where
         T: Sized;
     "#,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index 5f187880b0..94d9e5edef 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -805,12 +805,13 @@ fn doctest_desugar_async_into_impl_future() {
     check_doc_test(
         "desugar_async_into_impl_future",
         r#####"
-pub async f$0n foo() -> usize {
+//- minicore: future
+pub as$0ync fn foo() -> usize {
     0
 }
 "#####,
         r#####"
-pub fn foo() -> impl Future<Output = usize> {
+pub fn foo() -> impl core::future::Future<Output = usize> {
     0
 }
 "#####,
@@ -3043,7 +3044,7 @@ fn doctest_sugar_impl_future_into_async() {
         "sugar_impl_future_into_async",
         r#####"
 //- minicore: future
-pub f$0n foo() -> impl core::future::Future<Output = usize> {
+pub fn foo() -> impl core::future::F$0uture<Output = usize> {
     async { 0 }
 }
 "#####,

From 2400673ca6aa3d4066df416bc4831705068c6aa2 Mon Sep 17 00:00:00 2001
From: Luuk Wester <luuk.wester@gmail.com>
Date: Fri, 24 May 2024 21:51:05 +0200
Subject: [PATCH 05/15] cosmetic and performance fixes, and drop support for
 adding //! comments anywhere, except for at the top of files.

---
 .../convert_comment_from_or_to_doc.rs         | 189 +++++++-----------
 crates/ide-assists/src/tests/generated.rs     |   4 +-
 2 files changed, 79 insertions(+), 114 deletions(-)

diff --git a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
index 1ac30d71f2..2e73883447 100644
--- a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
+++ b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
@@ -11,12 +11,12 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 // Converts comments to documentation.
 //
 // ```
-// // Wow what $0a nice function
+// // Wow what $0a nice module
 // // I sure hope this shows up when I hover over it
 // ```
 // ->
 // ```
-// //! Wow what a nice function
+// //! Wow what a nice module
 // //! I sure hope this shows up when I hover over it
 // ```
 pub(crate) fn convert_comment_from_or_to_doc(
@@ -43,7 +43,7 @@ fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
 
     acc.add(
         AssistId("doc_to_comment", AssistKind::RefactorRewrite),
-        "Replace a comment with doc comment",
+        "Replace comment with doc comment",
         target,
         |edit| {
             // We need to either replace the first occurrence of /* with /***, or we need to replace
@@ -52,11 +52,12 @@ fn doc_to_comment(acc: &mut Assists, comment: ast::Comment) -> Option<()> {
                 ast::CommentShape::Line => {
                     let indentation = IndentLevel::from_token(comment.syntax());
                     let line_start = comment.prefix();
+                    let prefix = format!("{indentation}//");
                     relevant_line_comments(&comment)
                         .iter()
                         .map(|comment| comment.text())
                         .flat_map(|text| text.lines())
-                        .map(|line| indentation.to_string() + &line.replacen(line_start, "//", 1))
+                        .map(|line| line.replacen(line_start, &prefix, 1))
                         .join("\n")
                 }
                 ast::CommentShape::Block => {
@@ -89,23 +90,23 @@ fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacem
 
     acc.add(
         AssistId("comment_to_doc", AssistKind::RefactorRewrite),
-        "Replace a doc comment with comment",
+        "Replace doc comment with comment",
         target,
         |edit| {
             // We need to either replace the first occurrence of /* with /***, or we need to replace
             // the occurrences // at the start of each line with ///
             let output = match comment.kind().shape {
                 ast::CommentShape::Line => {
-                    let line_start = match style {
-                        CommentPlacement::Inner => "//!",
-                        CommentPlacement::Outer => "///",
-                    };
                     let indentation = IndentLevel::from_token(comment.syntax());
+                    let line_start = match style {
+                        CommentPlacement::Inner => format!("{indentation}//!"),
+                        CommentPlacement::Outer => format!("{indentation}///"),
+                    };
                     relevant_line_comments(&comment)
                         .iter()
                         .map(|comment| comment.text())
                         .flat_map(|text| text.lines())
-                        .map(|line| indentation.to_string() + &line.replacen("//", line_start, 1))
+                        .map(|line| line.replacen("//", &line_start, 1))
                         .join("\n")
                 }
                 ast::CommentShape::Block => {
@@ -139,21 +140,21 @@ fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacem
 /// Not all comments are valid candidates for conversion into doc comments. For example, the
 /// comments in the code:
 /// ```rust
-/// // foos the bar
-/// fn foo_bar(foo: Foo) -> Bar {
-///   // Bar the foo
-///   foo.into_bar()
+/// // Brilliant module right here
+///
+/// // Really good right
+/// fn good_function(foo: Foo) -> Bar {
+///     foo.into_bar()
 /// }
 ///
-/// trait A {
-///     // The A trait
-/// }
+/// // So nice
+/// mod nice_module {}
 /// ```
 /// can be converted to doc comments. However, the comments in this example:
 /// ```rust
 /// fn foo_bar(foo: Foo /* not bar yet */) -> Bar {
-///   foo.into_bar()
-///   // Nicely done
+///     foo.into_bar()
+///     // Nicely done
 /// }
 /// // end of function
 ///
@@ -161,7 +162,23 @@ fn comment_to_doc(acc: &mut Assists, comment: ast::Comment, style: CommentPlacem
 ///     // The S struct
 /// }
 /// ```
-/// are not allowed to become doc comments.
+/// are not allowed to become doc comments. Moreover, some comments _are_ allowed, but aren't common
+/// style in Rust. For example, the following comments are allowed to be doc comments, but it is not
+/// common style for them to be:
+/// ```rust
+/// fn foo_bar(foo: Foo) -> Bar {
+///     // this could be an inner comment with //!
+///     foo.into_bar()
+/// }
+///
+/// trait T {
+///     // The T struct could also be documented from within
+/// }
+///
+/// mod mymod {
+///     // Modules only normally get inner documentation when they are defined as a separate file.
+/// }
+/// ```
 fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
     use syntax::SyntaxKind::*;
 
@@ -175,38 +192,12 @@ fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
         None => return Some(CommentPlacement::Inner),
     }
 
-    // check if comment is followed by: `struct`, `trait`, `mod`, `fn`, `type`, `extern crate`, `use`, `const`
+    // check if comment is followed by: `struct`, `trait`, `mod`, `fn`, `type`, `extern crate`,
+    // `use` or `const`.
     let parent = comment.syntax().parent();
     let parent_kind = parent.as_ref().map(|parent| parent.kind());
-    if matches!(
-        parent_kind,
-        Some(STRUCT | TRAIT | MODULE | FN | TYPE_KW | EXTERN_CRATE | USE | CONST)
-    ) {
-        return Some(CommentPlacement::Outer);
-    }
-
-    // check if comment is preceded by: `fn f() {`, `trait T {`, `mod M {`:
-    let third_parent_kind = comment
-        .syntax()
-        .parent()
-        .and_then(|p| p.parent())
-        .and_then(|p| p.parent())
-        .map(|parent| parent.kind());
-    let is_first_item_in_parent = comment
-        .syntax()
-        .siblings_with_tokens(Direction::Prev)
-        .filter_map(|not| not.into_node())
-        .next()
-        .is_none();
-
-    if matches!(parent_kind, Some(STMT_LIST))
-        && is_first_item_in_parent
-        && matches!(third_parent_kind, Some(FN | TRAIT | MODULE))
-    {
-        return Some(CommentPlacement::Inner);
-    }
-
-    None
+    matches!(parent_kind, Some(STRUCT | TRAIT | MODULE | FN | TYPE_KW | EXTERN_CRATE | USE | CONST))
+        .then_some(CommentPlacement::Outer)
 }
 
 /// The line -> block assist can  be invoked from anywhere within a sequence of line comments.
@@ -467,39 +458,26 @@ mod tests {
 
     #[test]
     fn single_inner_line_comment_to_doc() {
-        check_assist(
+        check_assist_not_applicable(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 // unseen$0 docs
                 foo();
             }
             "#,
-            r#"
-            fn main() {
-                //! unseen docs
-                foo();
-            }
-            "#,
         );
     }
 
     #[test]
     fn multi_inner_line_comment_to_doc() {
-        check_assist(
+        check_assist_not_applicable(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 // unseen$0 docs
                 // make me seen!
-                foo();
-            }
-            "#,
-            r#"
-            fn main() {
-                //! unseen docs
-                //! make me seen!
-                foo();
+                type Int = i32;
             }
             "#,
         );
@@ -510,13 +488,13 @@ mod tests {
         check_assist(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 //! visible$0 docs
                 foo();
             }
             "#,
             r#"
-            fn main() {
+            mod mymod {
                 // visible docs
                 foo();
             }
@@ -529,58 +507,33 @@ mod tests {
         check_assist(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 //! visible$0 docs
                 //! Hide me!
                 foo();
             }
             "#,
             r#"
-            fn main() {
+            mod mymod {
                 // visible docs
                 // Hide me!
                 foo();
             }
             "#,
         );
-    }
-
-    #[test]
-    fn single_inner_line_block_comment_to_doc() {
         check_assist(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
-                /* unseen$0 docs */
+            mod mymod {
+                /// visible$0 docs
+                /// Hide me!
                 foo();
             }
             "#,
             r#"
-            fn main() {
-                /*! unseen docs */
-                foo();
-            }
-            "#,
-        );
-    }
-
-    #[test]
-    fn multi_inner_line_block_comment_to_doc() {
-        check_assist(
-            convert_comment_from_or_to_doc,
-            r#"
-            fn main() {
-                /* unseen$0 docs
-                *  make me seen!
-                */
-                foo();
-            }
-            "#,
-            r#"
-            fn main() {
-                /*! unseen docs
-                *   make me seen!
-                */
+            mod mymod {
+                // visible docs
+                // Hide me!
                 foo();
             }
             "#,
@@ -592,15 +545,15 @@ mod tests {
         check_assist(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 /*! visible$0 docs */
-                foo();
+                type Int = i32;
             }
             "#,
             r#"
-            fn main() {
+            mod mymod {
                 /* visible docs */
-                foo();
+                type Int = i32;
             }
             "#,
         );
@@ -611,21 +564,21 @@ mod tests {
         check_assist(
             convert_comment_from_or_to_doc,
             r#"
-            fn main() {
+            mod mymod {
                 /*! visible$0 docs
                 *   Hide me!
                 */
-                foo();
+                type Int = i32;
             }
             "#,
             r#"
-            fn main() {
+            mod mymod {
                 /* visible docs
                 *  Hide me!
                 */
-                foo();
+                type Int = i32;
             }
-        "#,
+            "#,
         );
     }
 
@@ -642,4 +595,16 @@ mod tests {
             "#,
         );
     }
+
+    #[test]
+    fn no_inner_comments() {
+        check_assist_not_applicable(
+            convert_comment_from_or_to_doc,
+            r#"
+            mod mymod {
+                // aaa$0aa
+            }
+            "#,
+        );
+    }
 }
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index bd841d72e6..5ecce3cbb6 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -350,11 +350,11 @@ fn doctest_comment_to_doc() {
     check_doc_test(
         "comment_to_doc",
         r#####"
-// Wow what $0a nice function
+// Wow what $0a nice module
 // I sure hope this shows up when I hover over it
 "#####,
         r#####"
-//! Wow what a nice function
+//! Wow what a nice module
 //! I sure hope this shows up when I hover over it
 "#####,
     )

From f5d740aa3d56e1d36c6806f2cc1bf996eef05591 Mon Sep 17 00:00:00 2001
From: Luuk Wester <luuk.wester@gmail.com>
Date: Fri, 24 May 2024 22:24:35 +0200
Subject: [PATCH 06/15] add test for every keyword, fix bug

---
 .../convert_comment_from_or_to_doc.rs         | 110 +++++++++++++++---
 1 file changed, 94 insertions(+), 16 deletions(-)

diff --git a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
index 2e73883447..d714f63f11 100644
--- a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
+++ b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
@@ -195,8 +195,8 @@ fn can_be_doc_comment(comment: &ast::Comment) -> Option<CommentPlacement> {
     // check if comment is followed by: `struct`, `trait`, `mod`, `fn`, `type`, `extern crate`,
     // `use` or `const`.
     let parent = comment.syntax().parent();
-    let parent_kind = parent.as_ref().map(|parent| parent.kind());
-    matches!(parent_kind, Some(STRUCT | TRAIT | MODULE | FN | TYPE_KW | EXTERN_CRATE | USE | CONST))
+    let par_kind = parent.as_ref().map(|parent| parent.kind());
+    matches!(par_kind, Some(STRUCT | TRAIT | MODULE | FN | TYPE_ALIAS | EXTERN_CRATE | USE | CONST))
         .then_some(CommentPlacement::Outer)
 }
 
@@ -469,20 +469,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn multi_inner_line_comment_to_doc() {
-        check_assist_not_applicable(
-            convert_comment_from_or_to_doc,
-            r#"
-            mod mymod {
-                // unseen$0 docs
-                // make me seen!
-                type Int = i32;
-            }
-            "#,
-        );
-    }
-
     #[test]
     fn single_inner_line_doc_to_comment() {
         check_assist(
@@ -596,6 +582,98 @@ mod tests {
         );
     }
 
+    #[test]
+    fn all_possible_items() {
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice struct$0 */
+                struct S {}
+            }"#,
+            r#"mod m {
+                /** Nice struct */
+                struct S {}
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice trait$0 */
+                trait T {}
+            }"#,
+            r#"mod m {
+                /** Nice trait */
+                trait T {}
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice module$0 */
+                mod module {}
+            }"#,
+            r#"mod m {
+                /** Nice module */
+                mod module {}
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice function$0 */
+                fn function() {}
+            }"#,
+            r#"mod m {
+                /** Nice function */
+                fn function() {}
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice type$0 */
+                type Type Int = i32;
+            }"#,
+            r#"mod m {
+                /** Nice type */
+                type Type Int = i32;
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice crate$0 */
+                extern crate rust_analyzer;
+            }"#,
+            r#"mod m {
+                /** Nice crate */
+                extern crate rust_analyzer;
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice import$0 */
+                use ide_assists::convert_comment_from_or_to_doc::tests
+            }"#,
+            r#"mod m {
+                /** Nice import */
+                use ide_assists::convert_comment_from_or_to_doc::tests
+            }"#,
+        );
+        check_assist(
+            convert_comment_from_or_to_doc,
+            r#"mod m {
+                /* Nice constant$0 */
+                const CONST: &str = "very const";
+            }"#,
+            r#"mod m {
+                /** Nice constant */
+                const CONST: &str = "very const";
+            }"#,
+        );
+    }
+
     #[test]
     fn no_inner_comments() {
         check_assist_not_applicable(

From 9e5ff0dce7ce55adcf478b6c091900bbf5380018 Mon Sep 17 00:00:00 2001
From: Luuk Wester <luuk.wester@gmail.com>
Date: Fri, 24 May 2024 22:57:35 +0200
Subject: [PATCH 07/15] remove nested match with and_then

---
 .../src/handlers/convert_comment_from_or_to_doc.rs           | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
index d714f63f11..953119fd1f 100644
--- a/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
+++ b/crates/ide-assists/src/handlers/convert_comment_from_or_to_doc.rs
@@ -27,10 +27,7 @@ pub(crate) fn convert_comment_from_or_to_doc(
 
     match comment.kind().doc {
         Some(_) => doc_to_comment(acc, comment),
-        None => match can_be_doc_comment(&comment) {
-            Some(doc_comment_style) => comment_to_doc(acc, comment, doc_comment_style),
-            None => None,
-        },
+        None => can_be_doc_comment(&comment).and_then(|style| comment_to_doc(acc, comment, style)),
     }
 }
 

From 0f6842700ff5f7f8e0c1734809a82ef01c9591dc Mon Sep 17 00:00:00 2001
From: Tavo Annus <tavo.annus@gmail.com>
Date: Sat, 25 May 2024 13:09:26 +0300
Subject: [PATCH 08/15] Fix `data_constructor` ignoring generics for struct

---
 crates/hir/src/term_search.rs                  |  2 +-
 crates/hir/src/term_search/tactics.rs          | 10 ++++++----
 crates/ide-assists/src/handlers/term_search.rs | 12 ++++++++++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/crates/hir/src/term_search.rs b/crates/hir/src/term_search.rs
index 5c5ddae19e..7b70cdf459 100644
--- a/crates/hir/src/term_search.rs
+++ b/crates/hir/src/term_search.rs
@@ -329,7 +329,7 @@ pub fn term_search<DB: HirDatabase>(ctx: &TermSearchCtx<'_, DB>) -> Vec<Expr> {
     while should_continue() {
         lookup.new_round();
 
-        solutions.extend(tactics::type_constructor(ctx, &defs, &mut lookup, should_continue));
+        solutions.extend(tactics::data_constructor(ctx, &defs, &mut lookup, should_continue));
         solutions.extend(tactics::free_function(ctx, &defs, &mut lookup, should_continue));
         solutions.extend(tactics::impl_method(ctx, &defs, &mut lookup, should_continue));
         solutions.extend(tactics::struct_projection(ctx, &defs, &mut lookup, should_continue));
diff --git a/crates/hir/src/term_search/tactics.rs b/crates/hir/src/term_search/tactics.rs
index a26728272d..f95ff1dc0f 100644
--- a/crates/hir/src/term_search/tactics.rs
+++ b/crates/hir/src/term_search/tactics.rs
@@ -87,9 +87,9 @@ pub(super) fn trivial<'a, DB: HirDatabase>(
     })
 }
 
-/// # Type constructor tactic
+/// # Data constructor tactic
 ///
-/// Attempts different type constructors for enums and structs in scope
+/// Attempts different data constructors for enums and structs in scope
 ///
 /// Updates lookup by new types reached and returns iterator that yields
 /// elements that unify with `goal`.
@@ -99,7 +99,7 @@ pub(super) fn trivial<'a, DB: HirDatabase>(
 /// * `defs` - Set of items in scope at term search target location
 /// * `lookup` - Lookup table for types
 /// * `should_continue` - Function that indicates when to stop iterating
-pub(super) fn type_constructor<'a, DB: HirDatabase>(
+pub(super) fn data_constructor<'a, DB: HirDatabase>(
     ctx: &'a TermSearchCtx<'a, DB>,
     defs: &'a FxHashSet<ScopeDef>,
     lookup: &'a mut LookupTable,
@@ -308,7 +308,9 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
                         // Early exit if some param cannot be filled from lookup
                         let param_exprs: Vec<Vec<Expr>> = fields
                             .into_iter()
-                            .map(|field| lookup.find(db, &field.ty(db)))
+                            .map(|field| {
+                                lookup.find(db, &field.ty_with_args(db, generics.iter().cloned()))
+                            })
                             .collect::<Option<_>>()?;
 
                         // Note that we need special case for 0 param constructors because of multi cartesian
diff --git a/crates/ide-assists/src/handlers/term_search.rs b/crates/ide-assists/src/handlers/term_search.rs
index ffd1508ccb..94e0519cba 100644
--- a/crates/ide-assists/src/handlers/term_search.rs
+++ b/crates/ide-assists/src/handlers/term_search.rs
@@ -278,4 +278,16 @@ fn f() { let a = 1; let b = 0.0; let c: (i32, (i32, f64)) = todo$0!(); }"#,
             r#"fn f() { let a = 1; let b = 0.0; let c: (i32, (i32, f64)) = (a, (a, b)); }"#,
         )
     }
+
+    #[test]
+    fn test_tuple_struct_with_generics() {
+        check_assist(
+            term_search,
+            r#"//- minicore: todo, unimplemented
+struct Foo<T>(T);
+fn f() { let a = 1; let b: Foo<i32> = todo$0!(); }"#,
+            r#"struct Foo<T>(T);
+fn f() { let a = 1; let b: Foo<i32> = Foo(a); }"#,
+        )
+    }
 }

From 80b4368ded3b86627d474facba2ca8b9a604e4fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Ml=C3=A1dek?= <david.mladek.cz@gmail.com>
Date: Tue, 28 May 2024 11:07:57 +0200
Subject: [PATCH 09/15] fix diagnostics clearing when flychecks run
 per-workspace

---
 crates/rust-analyzer/src/global_state.rs | 4 ++--
 crates/rust-analyzer/src/main_loop.rs    | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 79b87ecd58..0c2e60fcc9 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -87,7 +87,7 @@ pub(crate) struct GlobalState {
     pub(crate) flycheck_sender: Sender<flycheck::Message>,
     pub(crate) flycheck_receiver: Receiver<flycheck::Message>,
     pub(crate) last_flycheck_error: Option<String>,
-    pub(crate) diagnostics_received: bool,
+    pub(crate) diagnostics_received: FxHashMap<usize, bool>,
 
     // Test explorer
     pub(crate) test_run_session: Option<Vec<flycheck::CargoTestHandle>>,
@@ -225,7 +225,7 @@ impl GlobalState {
             flycheck_sender,
             flycheck_receiver,
             last_flycheck_error: None,
-            diagnostics_received: false,
+            diagnostics_received: FxHashMap::default(),
 
             test_run_session: None,
             test_run_sender,
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 7acd302867..a94ca871bd 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -804,9 +804,9 @@ impl GlobalState {
     fn handle_flycheck_msg(&mut self, message: flycheck::Message) {
         match message {
             flycheck::Message::AddDiagnostic { id, workspace_root, diagnostic } => {
-                if !self.diagnostics_received {
+                if !self.diagnostics_received.get(&id).copied().unwrap_or_default() {
                     self.diagnostics.clear_check(id);
-                    self.diagnostics_received = true;
+                    self.diagnostics_received.insert(id, true);
                 }
                 let snap = self.snapshot();
                 let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
@@ -836,7 +836,7 @@ impl GlobalState {
             flycheck::Message::Progress { id, progress } => {
                 let (state, message) = match progress {
                     flycheck::Progress::DidStart => {
-                        self.diagnostics_received = false;
+                        self.diagnostics_received.insert(id, false);
                         (Progress::Begin, None)
                     }
                     flycheck::Progress::DidCheckCrate(target) => (Progress::Report, Some(target)),
@@ -852,7 +852,7 @@ impl GlobalState {
                     flycheck::Progress::DidFinish(result) => {
                         self.last_flycheck_error =
                             result.err().map(|err| format!("cargo check failed to start: {err}"));
-                        if !self.diagnostics_received {
+                        if !self.diagnostics_received.get(&id).copied().unwrap_or_default() {
                             self.diagnostics.clear_check(id);
                         }
                         (Progress::End, None)

From 8e2f379a5df2ba612bb764fa20a0c1c910a24987 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Ml=C3=A1dek?= <david.mladek.cz@gmail.com>
Date: Tue, 28 May 2024 20:34:51 +0200
Subject: [PATCH 10/15] add `FlycheckStatus` to global state

---
 crates/rust-analyzer/src/global_state.rs | 21 +++++++++++++++++++--
 crates/rust-analyzer/src/main_loop.rs    | 20 +++++++++++++++-----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 0c2e60fcc9..1216de2de4 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -87,7 +87,7 @@ pub(crate) struct GlobalState {
     pub(crate) flycheck_sender: Sender<flycheck::Message>,
     pub(crate) flycheck_receiver: Receiver<flycheck::Message>,
     pub(crate) last_flycheck_error: Option<String>,
-    pub(crate) diagnostics_received: FxHashMap<usize, bool>,
+    pub(crate) flycheck_status: FxHashMap<usize, FlycheckStatus>,
 
     // Test explorer
     pub(crate) test_run_session: Option<Vec<flycheck::CargoTestHandle>>,
@@ -166,6 +166,14 @@ pub(crate) struct GlobalStateSnapshot {
     pub(crate) flycheck: Arc<[FlycheckHandle]>,
 }
 
+#[derive(Debug, Clone)]
+pub(crate) enum FlycheckStatus {
+    Unknown,
+    Started,
+    DiagnosticReceived,
+    Finished,
+}
+
 impl std::panic::UnwindSafe for GlobalStateSnapshot {}
 
 impl GlobalState {
@@ -225,7 +233,7 @@ impl GlobalState {
             flycheck_sender,
             flycheck_receiver,
             last_flycheck_error: None,
-            diagnostics_received: FxHashMap::default(),
+            flycheck_status: FxHashMap::default(),
 
             test_run_session: None,
             test_run_sender,
@@ -513,3 +521,12 @@ pub(crate) fn url_to_file_id(vfs: &vfs::Vfs, url: &Url) -> anyhow::Result<FileId
     let res = vfs.file_id(&path).ok_or_else(|| anyhow::format_err!("file not found: {path}"))?;
     Ok(res)
 }
+
+impl FlycheckStatus {
+    pub(crate) fn should_clear_old_diagnostics(&self) -> bool {
+        match self {
+            FlycheckStatus::Unknown | FlycheckStatus::Started => false,
+            FlycheckStatus::DiagnosticReceived | FlycheckStatus::Finished => true,
+        }
+    }
+}
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index a94ca871bd..efedea77fe 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -19,7 +19,7 @@ use crate::{
     config::Config,
     diagnostics::fetch_native_diagnostics,
     dispatch::{NotificationDispatcher, RequestDispatcher},
-    global_state::{file_id_to_url, url_to_file_id, GlobalState},
+    global_state::{file_id_to_url, url_to_file_id, FlycheckStatus, GlobalState},
     hack_recover_crate_name,
     lsp::{
         from_proto, to_proto,
@@ -804,10 +804,13 @@ impl GlobalState {
     fn handle_flycheck_msg(&mut self, message: flycheck::Message) {
         match message {
             flycheck::Message::AddDiagnostic { id, workspace_root, diagnostic } => {
-                if !self.diagnostics_received.get(&id).copied().unwrap_or_default() {
+                let flycheck_status =
+                    self.flycheck_status.entry(id).or_insert(FlycheckStatus::Unknown);
+
+                if !flycheck_status.should_clear_old_diagnostics() {
                     self.diagnostics.clear_check(id);
-                    self.diagnostics_received.insert(id, true);
                 }
+                *flycheck_status = FlycheckStatus::DiagnosticReceived;
                 let snap = self.snapshot();
                 let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
                     &self.config.diagnostics_map(),
@@ -836,25 +839,32 @@ impl GlobalState {
             flycheck::Message::Progress { id, progress } => {
                 let (state, message) = match progress {
                     flycheck::Progress::DidStart => {
-                        self.diagnostics_received.insert(id, false);
+                        self.flycheck_status.insert(id, FlycheckStatus::Started);
                         (Progress::Begin, None)
                     }
                     flycheck::Progress::DidCheckCrate(target) => (Progress::Report, Some(target)),
                     flycheck::Progress::DidCancel => {
                         self.last_flycheck_error = None;
+                        // We do not clear
+                        self.flycheck_status.insert(id, FlycheckStatus::Finished);
                         (Progress::End, None)
                     }
                     flycheck::Progress::DidFailToRestart(err) => {
                         self.last_flycheck_error =
                             Some(format!("cargo check failed to start: {err}"));
+                        self.flycheck_status.insert(id, FlycheckStatus::Finished);
                         return;
                     }
                     flycheck::Progress::DidFinish(result) => {
                         self.last_flycheck_error =
                             result.err().map(|err| format!("cargo check failed to start: {err}"));
-                        if !self.diagnostics_received.get(&id).copied().unwrap_or_default() {
+                        let flycheck_status =
+                            self.flycheck_status.entry(id).or_insert(FlycheckStatus::Unknown);
+
+                        if !flycheck_status.should_clear_old_diagnostics() {
                             self.diagnostics.clear_check(id);
                         }
+                        *flycheck_status = FlycheckStatus::Finished;
                         (Progress::End, None)
                     }
                 };

From 3c6c5cd0bfc460f52d60775edae9afea01d0b6c3 Mon Sep 17 00:00:00 2001
From: Henry Chen <chenx97@aosc.io>
Date: Wed, 22 May 2024 19:44:59 +0800
Subject: [PATCH 11/15] minor: replace command-group with process-wrap

Because command-group no longer receives updates and depends on an
older version of nix.
---
 Cargo.lock                     | 106 ++++++++++++++++++++-------------
 Cargo.toml                     |   2 +-
 crates/flycheck/Cargo.toml     |   2 +-
 crates/flycheck/src/command.rs |  16 +++--
 4 files changed, 77 insertions(+), 49 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 8eb872514a..3558c39bb3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -216,16 +216,6 @@ dependencies = [
  "tracing",
 ]
 
-[[package]]
-name = "command-group"
-version = "2.1.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5080df6b0f0ecb76cab30808f00d937ba725cebe266a3da8cd89dff92f2a9916"
-dependencies = [
- "nix 0.26.4",
- "winapi",
-]
-
 [[package]]
 name = "countme"
 version = "3.0.1"
@@ -292,7 +282,7 @@ version = "3.4.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "672465ae37dc1bc6380a6547a8883d5dd397b0f1faaad4f265726cc7042a5345"
 dependencies = [
- "nix 0.28.0",
+ "nix",
  "windows-sys 0.52.0",
 ]
 
@@ -432,9 +422,9 @@ name = "flycheck"
 version = "0.0.0"
 dependencies = [
  "cargo_metadata",
- "command-group",
  "crossbeam-channel",
  "paths",
+ "process-wrap",
  "rustc-hash",
  "serde",
  "serde_json",
@@ -1121,17 +1111,6 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
-[[package]]
-name = "nix"
-version = "0.26.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "598beaf3cc6fdd9a5dfb1630c2800c7acd31df7aaf0f565796fba2b53ca1af1b"
-dependencies = [
- "bitflags 1.3.2",
- "cfg-if",
- "libc",
-]
-
 [[package]]
 name = "nix"
 version = "0.28.0"
@@ -1397,6 +1376,18 @@ dependencies = [
  "unicode-ident",
 ]
 
+[[package]]
+name = "process-wrap"
+version = "8.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "38ee68ae331824036479c84060534b18254c864fa73366c58d86db3b7b811619"
+dependencies = [
+ "indexmap",
+ "nix",
+ "tracing",
+ "windows",
+]
+
 [[package]]
 name = "profile"
 version = "0.0.0"
@@ -2374,22 +2365,6 @@ version = "0.11.0+wasi-snapshot-preview1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423"
 
-[[package]]
-name = "winapi"
-version = "0.3.9"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419"
-dependencies = [
- "winapi-i686-pc-windows-gnu",
- "winapi-x86_64-pc-windows-gnu",
-]
-
-[[package]]
-name = "winapi-i686-pc-windows-gnu"
-version = "0.4.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
-
 [[package]]
 name = "winapi-util"
 version = "0.1.8"
@@ -2400,10 +2375,57 @@ dependencies = [
 ]
 
 [[package]]
-name = "winapi-x86_64-pc-windows-gnu"
-version = "0.4.0"
+name = "windows"
+version = "0.56.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
+checksum = "1de69df01bdf1ead2f4ac895dc77c9351aefff65b2f3db429a343f9cbf05e132"
+dependencies = [
+ "windows-core",
+ "windows-targets 0.52.5",
+]
+
+[[package]]
+name = "windows-core"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4698e52ed2d08f8658ab0c39512a7c00ee5fe2688c65f8c0a4f06750d729f2a6"
+dependencies = [
+ "windows-implement",
+ "windows-interface",
+ "windows-result",
+ "windows-targets 0.52.5",
+]
+
+[[package]]
+name = "windows-implement"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f6fc35f58ecd95a9b71c4f2329b911016e6bec66b3f2e6a4aad86bd2e99e2f9b"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "windows-interface"
+version = "0.56.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "08990546bf4edef8f431fa6326e032865f27138718c587dc21bc0265bbcb57cc"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "windows-result"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "749f0da9cc72d82e600d8d2e44cadd0b9eedb9038f71a1c58556ac1c5791813b"
+dependencies = [
+ "windows-targets 0.52.5",
+]
 
 [[package]]
 name = "windows-sys"
diff --git a/Cargo.toml b/Cargo.toml
index 3108c1b3df..ccc27e2133 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -111,7 +111,6 @@ chalk-solve = { version = "0.97.0", default-features = false }
 chalk-ir = "0.97.0"
 chalk-recursive = { version = "0.97.0", default-features = false }
 chalk-derive = "0.97.0"
-command-group = "2.0.1"
 crossbeam-channel = "0.5.8"
 dissimilar = "1.0.7"
 dot = "0.1.4"
@@ -132,6 +131,7 @@ object = { version = "0.33.0", default-features = false, features = [
     "macho",
     "pe",
 ] }
+process-wrap = { version = "8.0.2", features = ["std"] }
 pulldown-cmark-to-cmark = "10.0.4"
 pulldown-cmark = { version = "0.9.0", default-features = false }
 rayon = "1.8.0"
diff --git a/crates/flycheck/Cargo.toml b/crates/flycheck/Cargo.toml
index b8c10da1b6..d81a5fe340 100644
--- a/crates/flycheck/Cargo.toml
+++ b/crates/flycheck/Cargo.toml
@@ -18,7 +18,7 @@ tracing.workspace = true
 rustc-hash.workspace = true
 serde_json.workspace = true
 serde.workspace = true
-command-group.workspace = true
+process-wrap.workspace = true
 
 # local deps
 paths.workspace = true
diff --git a/crates/flycheck/src/command.rs b/crates/flycheck/src/command.rs
index 8ba7018316..38c7c81f57 100644
--- a/crates/flycheck/src/command.rs
+++ b/crates/flycheck/src/command.rs
@@ -9,8 +9,8 @@ use std::{
     process::{ChildStderr, ChildStdout, Command, Stdio},
 };
 
-use command_group::{CommandGroup, GroupChild};
 use crossbeam_channel::Sender;
+use process_wrap::std::{StdChildWrapper, StdCommandWrap};
 use stdx::process::streaming_output;
 
 /// Cargo output is structured as a one JSON per line. This trait abstracts parsing one line of
@@ -85,7 +85,7 @@ impl<T: ParseFromLine> CargoActor<T> {
     }
 }
 
-struct JodGroupChild(GroupChild);
+struct JodGroupChild(Box<dyn StdChildWrapper>);
 
 impl Drop for JodGroupChild {
     fn drop(&mut self) {
@@ -119,14 +119,20 @@ impl<T> fmt::Debug for CommandHandle<T> {
 impl<T: ParseFromLine> CommandHandle<T> {
     pub(crate) fn spawn(mut command: Command, sender: Sender<T>) -> std::io::Result<Self> {
         command.stdout(Stdio::piped()).stderr(Stdio::piped()).stdin(Stdio::null());
-        let mut child = command.group_spawn().map(JodGroupChild)?;
 
         let program = command.get_program().into();
         let arguments = command.get_args().map(|arg| arg.into()).collect::<Vec<OsString>>();
         let current_dir = command.get_current_dir().map(|arg| arg.to_path_buf());
 
-        let stdout = child.0.inner().stdout.take().unwrap();
-        let stderr = child.0.inner().stderr.take().unwrap();
+        let mut child = StdCommandWrap::from(command);
+        #[cfg(unix)]
+        child.wrap(process_wrap::std::ProcessSession);
+        #[cfg(windows)]
+        child.wrap(process_wrap::std::JobObject);
+        let mut child = child.spawn().map(JodGroupChild)?;
+
+        let stdout = child.0.stdout().take().unwrap();
+        let stderr = child.0.stderr().take().unwrap();
 
         let actor = CargoActor::<T>::new(sender, stdout, stderr);
         let thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)

From 8c5ef9c15f22d4fa52ece9889a4e4b64a8ccfc4d Mon Sep 17 00:00:00 2001
From: Luke Franceschini <luke.a.franceschini@gmail.com>
Date: Fri, 31 May 2024 11:24:26 -0400
Subject: [PATCH 12/15] docs: Missing word typo

---
 crates/ide/src/goto_declaration.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/ide/src/goto_declaration.rs b/crates/ide/src/goto_declaration.rs
index fab62e95d1..c19f19803f 100644
--- a/crates/ide/src/goto_declaration.rs
+++ b/crates/ide/src/goto_declaration.rs
@@ -16,7 +16,7 @@ use crate::{
 //
 // This is the same as `Go to Definition` with the following exceptions:
 // - outline modules will navigate to the `mod name;` item declaration
-// - trait assoc items will navigate to the assoc item of the trait declaration opposed to the trait impl
+// - trait assoc items will navigate to the assoc item of the trait declaration as opposed to the trait impl
 // - fields in patterns will navigate to the field declaration of the struct, union or variant
 pub(crate) fn goto_declaration(
     db: &RootDatabase,

From a0b2f3927322f49d95de070b83444e69fc328e2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Ml=C3=A1dek?= <david.mladek.cz@gmail.com>
Date: Sat, 1 Jun 2024 15:43:49 +0200
Subject: [PATCH 13/15] Move state trackig of diagnostic clearing inside
 `FlycheckActor`

---
 crates/flycheck/src/lib.rs               | 27 ++++++++++++++++++++++++
 crates/rust-analyzer/src/global_state.rs | 19 -----------------
 crates/rust-analyzer/src/main_loop.rs    | 26 ++++-------------------
 3 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs
index 6d5ca8321e..afdc3e389b 100644
--- a/crates/flycheck/src/lib.rs
+++ b/crates/flycheck/src/lib.rs
@@ -163,6 +163,9 @@ pub enum Message {
     /// Request adding a diagnostic with fixes included to a file
     AddDiagnostic { id: usize, workspace_root: AbsPathBuf, diagnostic: Diagnostic },
 
+    /// Request clearing all previous diagnostics
+    ClearDiagnostics { id: usize },
+
     /// Request check progress notification to client
     Progress {
         /// Flycheck instance ID
@@ -180,6 +183,9 @@ impl fmt::Debug for Message {
                 .field("workspace_root", workspace_root)
                 .field("diagnostic_code", &diagnostic.code.as_ref().map(|it| &it.code))
                 .finish(),
+            Message::ClearDiagnostics { id } => {
+                f.debug_struct("ClearDiagnostics").field("id", id).finish()
+            }
             Message::Progress { id, progress } => {
                 f.debug_struct("Progress").field("id", id).field("progress", progress).finish()
             }
@@ -220,6 +226,8 @@ struct FlycheckActor {
     command_handle: Option<CommandHandle<CargoCheckMessage>>,
     /// The receiver side of the channel mentioned above.
     command_receiver: Option<Receiver<CargoCheckMessage>>,
+
+    status: FlycheckStatus,
 }
 
 enum Event {
@@ -227,6 +235,13 @@ enum Event {
     CheckEvent(Option<CargoCheckMessage>),
 }
 
+#[derive(PartialEq)]
+enum FlycheckStatus {
+    Started,
+    DiagnosticSent,
+    Finished,
+}
+
 const SAVED_FILE_PLACEHOLDER: &str = "$saved_file";
 
 impl FlycheckActor {
@@ -248,6 +263,7 @@ impl FlycheckActor {
             manifest_path,
             command_handle: None,
             command_receiver: None,
+            status: FlycheckStatus::Finished,
         }
     }
 
@@ -298,12 +314,14 @@ impl FlycheckActor {
                             self.command_handle = Some(command_handle);
                             self.command_receiver = Some(receiver);
                             self.report_progress(Progress::DidStart);
+                            self.status = FlycheckStatus::Started;
                         }
                         Err(error) => {
                             self.report_progress(Progress::DidFailToRestart(format!(
                                 "Failed to run the following command: {} error={}",
                                 formatted_command, error
                             )));
+                            self.status = FlycheckStatus::Finished;
                         }
                     }
                 }
@@ -323,7 +341,11 @@ impl FlycheckActor {
                             error
                         );
                     }
+                    if self.status == FlycheckStatus::Started {
+                        self.send(Message::ClearDiagnostics { id: self.id });
+                    }
                     self.report_progress(Progress::DidFinish(res));
+                    self.status = FlycheckStatus::Finished;
                 }
                 Event::CheckEvent(Some(message)) => match message {
                     CargoCheckMessage::CompilerArtifact(msg) => {
@@ -341,11 +363,15 @@ impl FlycheckActor {
                             message = msg.message,
                             "diagnostic received"
                         );
+                        if self.status == FlycheckStatus::Started {
+                            self.send(Message::ClearDiagnostics { id: self.id });
+                        }
                         self.send(Message::AddDiagnostic {
                             id: self.id,
                             workspace_root: self.root.clone(),
                             diagnostic: msg,
                         });
+                        self.status = FlycheckStatus::DiagnosticSent;
                     }
                 },
             }
@@ -362,6 +388,7 @@ impl FlycheckActor {
             );
             command_handle.cancel();
             self.report_progress(Progress::DidCancel);
+            self.status = FlycheckStatus::Finished;
         }
     }
 
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 1216de2de4..f64e66183d 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -87,7 +87,6 @@ pub(crate) struct GlobalState {
     pub(crate) flycheck_sender: Sender<flycheck::Message>,
     pub(crate) flycheck_receiver: Receiver<flycheck::Message>,
     pub(crate) last_flycheck_error: Option<String>,
-    pub(crate) flycheck_status: FxHashMap<usize, FlycheckStatus>,
 
     // Test explorer
     pub(crate) test_run_session: Option<Vec<flycheck::CargoTestHandle>>,
@@ -166,14 +165,6 @@ pub(crate) struct GlobalStateSnapshot {
     pub(crate) flycheck: Arc<[FlycheckHandle]>,
 }
 
-#[derive(Debug, Clone)]
-pub(crate) enum FlycheckStatus {
-    Unknown,
-    Started,
-    DiagnosticReceived,
-    Finished,
-}
-
 impl std::panic::UnwindSafe for GlobalStateSnapshot {}
 
 impl GlobalState {
@@ -233,7 +224,6 @@ impl GlobalState {
             flycheck_sender,
             flycheck_receiver,
             last_flycheck_error: None,
-            flycheck_status: FxHashMap::default(),
 
             test_run_session: None,
             test_run_sender,
@@ -521,12 +511,3 @@ pub(crate) fn url_to_file_id(vfs: &vfs::Vfs, url: &Url) -> anyhow::Result<FileId
     let res = vfs.file_id(&path).ok_or_else(|| anyhow::format_err!("file not found: {path}"))?;
     Ok(res)
 }
-
-impl FlycheckStatus {
-    pub(crate) fn should_clear_old_diagnostics(&self) -> bool {
-        match self {
-            FlycheckStatus::Unknown | FlycheckStatus::Started => false,
-            FlycheckStatus::DiagnosticReceived | FlycheckStatus::Finished => true,
-        }
-    }
-}
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index efedea77fe..193b3fdd4a 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -19,7 +19,7 @@ use crate::{
     config::Config,
     diagnostics::fetch_native_diagnostics,
     dispatch::{NotificationDispatcher, RequestDispatcher},
-    global_state::{file_id_to_url, url_to_file_id, FlycheckStatus, GlobalState},
+    global_state::{file_id_to_url, url_to_file_id, GlobalState},
     hack_recover_crate_name,
     lsp::{
         from_proto, to_proto,
@@ -804,13 +804,6 @@ impl GlobalState {
     fn handle_flycheck_msg(&mut self, message: flycheck::Message) {
         match message {
             flycheck::Message::AddDiagnostic { id, workspace_root, diagnostic } => {
-                let flycheck_status =
-                    self.flycheck_status.entry(id).or_insert(FlycheckStatus::Unknown);
-
-                if !flycheck_status.should_clear_old_diagnostics() {
-                    self.diagnostics.clear_check(id);
-                }
-                *flycheck_status = FlycheckStatus::DiagnosticReceived;
                 let snap = self.snapshot();
                 let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
                     &self.config.diagnostics_map(),
@@ -836,35 +829,24 @@ impl GlobalState {
                 }
             }
 
+            flycheck::Message::ClearDiagnostics { id } => self.diagnostics.clear_check(id),
+
             flycheck::Message::Progress { id, progress } => {
                 let (state, message) = match progress {
-                    flycheck::Progress::DidStart => {
-                        self.flycheck_status.insert(id, FlycheckStatus::Started);
-                        (Progress::Begin, None)
-                    }
+                    flycheck::Progress::DidStart => (Progress::Begin, None),
                     flycheck::Progress::DidCheckCrate(target) => (Progress::Report, Some(target)),
                     flycheck::Progress::DidCancel => {
                         self.last_flycheck_error = None;
-                        // We do not clear
-                        self.flycheck_status.insert(id, FlycheckStatus::Finished);
                         (Progress::End, None)
                     }
                     flycheck::Progress::DidFailToRestart(err) => {
                         self.last_flycheck_error =
                             Some(format!("cargo check failed to start: {err}"));
-                        self.flycheck_status.insert(id, FlycheckStatus::Finished);
                         return;
                     }
                     flycheck::Progress::DidFinish(result) => {
                         self.last_flycheck_error =
                             result.err().map(|err| format!("cargo check failed to start: {err}"));
-                        let flycheck_status =
-                            self.flycheck_status.entry(id).or_insert(FlycheckStatus::Unknown);
-
-                        if !flycheck_status.should_clear_old_diagnostics() {
-                            self.diagnostics.clear_check(id);
-                        }
-                        *flycheck_status = FlycheckStatus::Finished;
                         (Progress::End, None)
                     }
                 };

From c0171bdd325985f04637d79ff84dfe63089d9d92 Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Sat, 1 Jun 2024 20:57:29 +0200
Subject: [PATCH 14/15] fix: Fix container search failing for tokens
 originating within derive attributes

---
 crates/hir-expand/src/db.rs               |  2 +-
 crates/hir-expand/src/files.rs            | 22 ++++++++----------
 crates/hir-expand/src/lib.rs              | 16 +++++++++++--
 crates/hir/src/semantics.rs               | 28 ++++++++---------------
 crates/hir/src/semantics/source_to_def.rs | 20 +++++++++++-----
 5 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index 2e5fa6131a..12421bbe70 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs
@@ -298,7 +298,7 @@ pub fn expand_speculative(
             // prefer tokens of the same kind and text
             // Note the inversion of the score here, as we want to prefer the first token in case
             // of all tokens having the same score
-            (t.kind() != token_to_map.kind()) as u8 + (t.text() != token_to_map.text()) as u8
+            (t.kind() != token_to_map.kind()) as u8 + 2 * ((t.text() != token_to_map.text()) as u8)
         })?;
     Some((node.syntax_node(), token))
 }
diff --git a/crates/hir-expand/src/files.rs b/crates/hir-expand/src/files.rs
index 04a4851ddb..1ba85c5c7e 100644
--- a/crates/hir-expand/src/files.rs
+++ b/crates/hir-expand/src/files.rs
@@ -153,24 +153,20 @@ impl<FileId: Copy, N: AstNode> InFileWrapper<FileId, N> {
 // region:specific impls
 
 impl InFile<&SyntaxNode> {
-    /// Skips the attributed item that caused the macro invocation we are climbing up
-    pub fn ancestors_with_macros_skip_attr_item(
+    /// Traverse up macro calls and skips the macro invocation node
+    pub fn ancestors_with_macros(
         self,
         db: &dyn db::ExpandDatabase,
     ) -> impl Iterator<Item = InFile<SyntaxNode>> + '_ {
         let succ = move |node: &InFile<SyntaxNode>| match node.value.parent() {
             Some(parent) => Some(node.with_value(parent)),
-            None => {
-                let macro_file_id = node.file_id.macro_file()?;
-                let parent_node = macro_file_id.call_node(db);
-                if macro_file_id.is_attr_macro(db) {
-                    // macro call was an attributed item, skip it
-                    // FIXME: does this fail if this is a direct expansion of another macro?
-                    parent_node.map(|node| node.parent()).transpose()
-                } else {
-                    Some(parent_node)
-                }
-            }
+            None => db
+                .lookup_intern_macro_call(node.file_id.macro_file()?.macro_call_id)
+                .to_node_item(db)
+                .syntax()
+                .cloned()
+                .map(|node| node.parent())
+                .transpose(),
         };
         iter::successors(succ(&self.cloned()), succ)
     }
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index 4ab989bec2..83e92565f4 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -33,8 +33,8 @@ use std::{fmt, hash::Hash};
 use base_db::{salsa::impl_intern_value_trivial, CrateId, FileId};
 use either::Either;
 use span::{
-    Edition, ErasedFileAstId, FileRange, HirFileIdRepr, Span, SpanAnchor, SyntaxContextData,
-    SyntaxContextId,
+    Edition, ErasedFileAstId, FileAstId, FileRange, HirFileIdRepr, Span, SpanAnchor,
+    SyntaxContextData, SyntaxContextId,
 };
 use syntax::{
     ast::{self, AstNode},
@@ -546,6 +546,18 @@ impl MacroCallLoc {
         }
     }
 
+    pub fn to_node_item(&self, db: &dyn ExpandDatabase) -> InFile<ast::Item> {
+        match self.kind {
+            MacroCallKind::FnLike { ast_id, .. } => {
+                InFile::new(ast_id.file_id, ast_id.map(FileAstId::upcast).to_node(db))
+            }
+            MacroCallKind::Derive { ast_id, .. } => {
+                InFile::new(ast_id.file_id, ast_id.map(FileAstId::upcast).to_node(db))
+            }
+            MacroCallKind::Attr { ast_id, .. } => InFile::new(ast_id.file_id, ast_id.to_node(db)),
+        }
+    }
+
     fn expand_to(&self) -> ExpandTo {
         match self.kind {
             MacroCallKind::FnLike { expand_to, .. } => expand_to,
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 6c70cc4baf..53242611f8 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -862,10 +862,9 @@ impl<'db> SemanticsImpl<'db> {
                         // attribute we failed expansion for earlier, this might be a derive invocation
                         // or derive helper attribute
                         let attr = meta.parent_attr()?;
-
                         let adt = if let Some(adt) = attr.syntax().parent().and_then(ast::Adt::cast)
                         {
-                            // this might be a derive, or a derive helper on an ADT
+                            // this might be a derive on an ADT
                             let derive_call = self.with_ctx(|ctx| {
                                 // so try downmapping the token into the pseudo derive expansion
                                 // see [hir_expand::builtin_attr_macro] for how the pseudo derive expansion works
@@ -882,7 +881,7 @@ impl<'db> SemanticsImpl<'db> {
                                     let file_id = call_id.as_macro_file();
                                     let text_range = attr.syntax().text_range();
                                     // remove any other token in this macro input, all their mappings are the
-                                    // same as this one
+                                    // same as this
                                     tokens.retain(|t| !text_range.contains_range(t.text_range()));
                                     return process_expansion_for_token(&mut stack, file_id);
                                 }
@@ -890,21 +889,14 @@ impl<'db> SemanticsImpl<'db> {
                             }
                         } else {
                             // Otherwise this could be a derive helper on a variant or field
-                            if let Some(field) =
-                                attr.syntax().parent().and_then(ast::RecordField::cast)
-                            {
-                                field.syntax().ancestors().take(4).find_map(ast::Adt::cast)
-                            } else if let Some(field) =
-                                attr.syntax().parent().and_then(ast::TupleField::cast)
-                            {
-                                field.syntax().ancestors().take(4).find_map(ast::Adt::cast)
-                            } else if let Some(variant) =
-                                attr.syntax().parent().and_then(ast::Variant::cast)
-                            {
-                                variant.syntax().ancestors().nth(2).and_then(ast::Adt::cast)
-                            } else {
-                                None
-                            }
+                            attr.syntax().ancestors().find_map(ast::Item::cast).and_then(|it| {
+                                match it {
+                                    ast::Item::Struct(it) => Some(ast::Adt::Struct(it)),
+                                    ast::Item::Enum(it) => Some(ast::Adt::Enum(it)),
+                                    ast::Item::Union(it) => Some(ast::Adt::Union(it)),
+                                    _ => None,
+                                }
+                            })
                         }?;
                         if !self.with_ctx(|ctx| ctx.has_derives(InFile::new(file_id, &adt))) {
                             return None;
diff --git a/crates/hir/src/semantics/source_to_def.rs b/crates/hir/src/semantics/source_to_def.rs
index d2bd8b0e79..77e7cdb58a 100644
--- a/crates/hir/src/semantics/source_to_def.rs
+++ b/crates/hir/src/semantics/source_to_def.rs
@@ -139,7 +139,7 @@ impl SourceToDefCtx<'_, '_> {
         let _p = tracing::span!(tracing::Level::INFO, "module_to_def").entered();
         let parent_declaration = src
             .syntax()
-            .ancestors_with_macros_skip_attr_item(self.db.upcast())
+            .ancestors_with_macros(self.db.upcast())
             .find_map(|it| it.map(Either::<ast::Module, ast::BlockExpr>::cast).transpose())
             .map(|it| it.transpose());
 
@@ -366,7 +366,7 @@ impl SourceToDefCtx<'_, '_> {
     }
 
     pub(super) fn find_container(&mut self, src: InFile<&SyntaxNode>) -> Option<ChildContainer> {
-        for container in src.ancestors_with_macros_skip_attr_item(self.db.upcast()) {
+        for container in src.ancestors_with_macros(self.db.upcast()) {
             if let Some(res) = self.container_to_def(container) {
                 return Some(res);
             }
@@ -420,7 +420,7 @@ impl SourceToDefCtx<'_, '_> {
     }
 
     fn find_generic_param_container(&mut self, src: InFile<&SyntaxNode>) -> Option<GenericDefId> {
-        let ancestors = src.ancestors_with_macros_skip_attr_item(self.db.upcast());
+        let ancestors = src.ancestors_with_macros(self.db.upcast());
         for InFile { file_id, value } in ancestors {
             let item = match ast::Item::cast(value) {
                 Some(it) => it,
@@ -429,6 +429,7 @@ impl SourceToDefCtx<'_, '_> {
             let res: GenericDefId = match item {
                 ast::Item::Fn(it) => self.fn_to_def(InFile::new(file_id, it))?.into(),
                 ast::Item::Struct(it) => self.struct_to_def(InFile::new(file_id, it))?.into(),
+                ast::Item::Union(it) => self.union_to_def(InFile::new(file_id, it))?.into(),
                 ast::Item::Enum(it) => self.enum_to_def(InFile::new(file_id, it))?.into(),
                 ast::Item::Trait(it) => self.trait_to_def(InFile::new(file_id, it))?.into(),
                 ast::Item::TraitAlias(it) => {
@@ -446,11 +447,18 @@ impl SourceToDefCtx<'_, '_> {
     }
 
     fn find_pat_or_label_container(&mut self, src: InFile<&SyntaxNode>) -> Option<DefWithBodyId> {
-        let ancestors = src.ancestors_with_macros_skip_attr_item(self.db.upcast());
+        let ancestors = src.ancestors_with_macros(self.db.upcast());
         for InFile { file_id, value } in ancestors {
-            let item = match ast::Item::cast(value) {
+            let item = match ast::Item::cast(value.clone()) {
                 Some(it) => it,
-                None => continue,
+                None => {
+                    if let Some(variant) = ast::Variant::cast(value.clone()) {
+                        return self
+                            .enum_variant_to_def(InFile::new(file_id, variant))
+                            .map(Into::into);
+                    }
+                    continue;
+                }
             };
             let res: DefWithBodyId = match item {
                 ast::Item::Const(it) => self.const_to_def(InFile::new(file_id, it))?.into(),

From 3116f76fbab16c2e6020c41f01af34c330a99018 Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Sun, 2 Jun 2024 09:28:34 +0200
Subject: [PATCH 15/15] feat: Enable completions within derive helper
 attributes

---
 crates/hir/src/semantics.rs                   | 221 +++++++++++-------
 crates/ide-completion/src/context/analysis.rs |  86 ++++++-
 2 files changed, 209 insertions(+), 98 deletions(-)

diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 53242611f8..43de2a6ee7 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -380,6 +380,27 @@ impl<'db> SemanticsImpl<'db> {
         self.with_ctx(|ctx| ctx.has_derives(adt))
     }
 
+    pub fn derive_helper(&self, attr: &ast::Attr) -> Option<Vec<(Macro, MacroFileId)>> {
+        let adt = attr.syntax().ancestors().find_map(ast::Item::cast).and_then(|it| match it {
+            ast::Item::Struct(it) => Some(ast::Adt::Struct(it)),
+            ast::Item::Enum(it) => Some(ast::Adt::Enum(it)),
+            ast::Item::Union(it) => Some(ast::Adt::Union(it)),
+            _ => None,
+        })?;
+        let attr_name = attr.path().and_then(|it| it.as_single_name_ref())?.as_name();
+        let sa = self.analyze_no_infer(adt.syntax())?;
+        let id = self.db.ast_id_map(sa.file_id).ast_id(&adt);
+        let res: Vec<_> = sa
+            .resolver
+            .def_map()
+            .derive_helpers_in_scope(InFile::new(sa.file_id, id))?
+            .iter()
+            .filter(|&(name, _, _)| *name == attr_name)
+            .map(|&(_, macro_, call)| (macro_.into(), call.as_macro_file()))
+            .collect();
+        res.is_empty().not().then_some(res)
+    }
+
     pub fn is_attr_macro_call(&self, item: &ast::Item) -> bool {
         let file_id = self.find_file(item.syntax()).file_id;
         let src = InFile::new(file_id, item.clone());
@@ -409,6 +430,20 @@ impl<'db> SemanticsImpl<'db> {
         )
     }
 
+    pub fn speculative_expand_raw(
+        &self,
+        macro_file: MacroFileId,
+        speculative_args: &SyntaxNode,
+        token_to_map: SyntaxToken,
+    ) -> Option<(SyntaxNode, SyntaxToken)> {
+        hir_expand::db::expand_speculative(
+            self.db.upcast(),
+            macro_file.macro_call_id,
+            speculative_args,
+            token_to_map,
+        )
+    }
+
     /// Expand the macro call with a different item as the input, mapping the `token_to_map` down into the
     /// expansion. `token_to_map` should be a token from the `speculative args` node.
     pub fn speculative_expand_attr_macro(
@@ -826,99 +861,109 @@ impl<'db> SemanticsImpl<'db> {
 
                     // Then check for token trees, that means we are either in a function-like macro or
                     // secondary attribute inputs
-                    let tt = token.parent_ancestors().map_while(ast::TokenTree::cast).last()?;
-                    let parent = tt.syntax().parent()?;
-
-                    if tt.left_delimiter_token().map_or(false, |it| it == token) {
-                        return None;
-                    }
-                    if tt.right_delimiter_token().map_or(false, |it| it == token) {
-                        return None;
-                    }
-
-                    if let Some(macro_call) = ast::MacroCall::cast(parent.clone()) {
-                        let mcall: hir_expand::files::InFileWrapper<HirFileId, ast::MacroCall> =
-                            InFile::new(file_id, macro_call);
-                        let file_id = match mcache.get(&mcall) {
-                            Some(&it) => it,
-                            None => {
-                                let it = sa.expand(self.db, mcall.as_ref())?;
-                                mcache.insert(mcall, it);
-                                it
+                    let tt = token
+                        .parent_ancestors()
+                        .map_while(Either::<ast::TokenTree, ast::Meta>::cast)
+                        .last()?;
+                    match tt {
+                        Either::Left(tt) => {
+                            if tt.left_delimiter_token().map_or(false, |it| it == token) {
+                                return None;
                             }
-                        };
-                        let text_range = tt.syntax().text_range();
-                        // remove any other token in this macro input, all their mappings are the
-                        // same as this one
-                        tokens.retain(|t| !text_range.contains_range(t.text_range()));
-
-                        process_expansion_for_token(&mut stack, file_id).or(file_id
-                            .eager_arg(self.db.upcast())
-                            .and_then(|arg| {
-                                // also descend into eager expansions
-                                process_expansion_for_token(&mut stack, arg.as_macro_file())
-                            }))
-                    } else if let Some(meta) = ast::Meta::cast(parent) {
-                        // attribute we failed expansion for earlier, this might be a derive invocation
-                        // or derive helper attribute
-                        let attr = meta.parent_attr()?;
-                        let adt = if let Some(adt) = attr.syntax().parent().and_then(ast::Adt::cast)
-                        {
-                            // this might be a derive on an ADT
-                            let derive_call = self.with_ctx(|ctx| {
-                                // so try downmapping the token into the pseudo derive expansion
-                                // see [hir_expand::builtin_attr_macro] for how the pseudo derive expansion works
-                                ctx.attr_to_derive_macro_call(
-                                    InFile::new(file_id, &adt),
-                                    InFile::new(file_id, attr.clone()),
-                                )
-                                .map(|(_, call_id, _)| call_id)
-                            });
-
-                            match derive_call {
-                                Some(call_id) => {
-                                    // resolved to a derive
-                                    let file_id = call_id.as_macro_file();
-                                    let text_range = attr.syntax().text_range();
-                                    // remove any other token in this macro input, all their mappings are the
-                                    // same as this
-                                    tokens.retain(|t| !text_range.contains_range(t.text_range()));
-                                    return process_expansion_for_token(&mut stack, file_id);
-                                }
-                                None => Some(adt),
+                            if tt.right_delimiter_token().map_or(false, |it| it == token) {
+                                return None;
                             }
-                        } else {
-                            // Otherwise this could be a derive helper on a variant or field
-                            attr.syntax().ancestors().find_map(ast::Item::cast).and_then(|it| {
-                                match it {
-                                    ast::Item::Struct(it) => Some(ast::Adt::Struct(it)),
-                                    ast::Item::Enum(it) => Some(ast::Adt::Enum(it)),
-                                    ast::Item::Union(it) => Some(ast::Adt::Union(it)),
-                                    _ => None,
+                            let macro_call = tt.syntax().parent().and_then(ast::MacroCall::cast)?;
+                            let mcall: hir_expand::files::InFileWrapper<HirFileId, ast::MacroCall> =
+                                InFile::new(file_id, macro_call);
+                            let file_id = match mcache.get(&mcall) {
+                                Some(&it) => it,
+                                None => {
+                                    let it = sa.expand(self.db, mcall.as_ref())?;
+                                    mcache.insert(mcall, it);
+                                    it
                                 }
-                            })
-                        }?;
-                        if !self.with_ctx(|ctx| ctx.has_derives(InFile::new(file_id, &adt))) {
-                            return None;
+                            };
+                            let text_range = tt.syntax().text_range();
+                            // remove any other token in this macro input, all their mappings are the
+                            // same as this one
+                            tokens.retain(|t| !text_range.contains_range(t.text_range()));
+
+                            process_expansion_for_token(&mut stack, file_id).or(file_id
+                                .eager_arg(self.db.upcast())
+                                .and_then(|arg| {
+                                    // also descend into eager expansions
+                                    process_expansion_for_token(&mut stack, arg.as_macro_file())
+                                }))
                         }
-                        // Not an attribute, nor a derive, so it's either a builtin or a derive helper
-                        // Try to resolve to a derive helper and downmap
-                        let attr_name =
-                            attr.path().and_then(|it| it.as_single_name_ref())?.as_name();
-                        let id = self.db.ast_id_map(file_id).ast_id(&adt);
-                        let helpers = def_map.derive_helpers_in_scope(InFile::new(file_id, id))?;
-                        let mut res = None;
-                        for (.., derive) in
-                            helpers.iter().filter(|(helper, ..)| *helper == attr_name)
-                        {
-                            res = res.or(process_expansion_for_token(
-                                &mut stack,
-                                derive.as_macro_file(),
-                            ));
+                        Either::Right(meta) => {
+                            // attribute we failed expansion for earlier, this might be a derive invocation
+                            // or derive helper attribute
+                            let attr = meta.parent_attr()?;
+                            let adt = match attr.syntax().parent().and_then(ast::Adt::cast) {
+                                Some(adt) => {
+                                    // this might be a derive on an ADT
+                                    let derive_call = self.with_ctx(|ctx| {
+                                        // so try downmapping the token into the pseudo derive expansion
+                                        // see [hir_expand::builtin_attr_macro] for how the pseudo derive expansion works
+                                        ctx.attr_to_derive_macro_call(
+                                            InFile::new(file_id, &adt),
+                                            InFile::new(file_id, attr.clone()),
+                                        )
+                                        .map(|(_, call_id, _)| call_id)
+                                    });
+
+                                    match derive_call {
+                                        Some(call_id) => {
+                                            // resolved to a derive
+                                            let file_id = call_id.as_macro_file();
+                                            let text_range = attr.syntax().text_range();
+                                            // remove any other token in this macro input, all their mappings are the
+                                            // same as this
+                                            tokens.retain(|t| {
+                                                !text_range.contains_range(t.text_range())
+                                            });
+                                            return process_expansion_for_token(
+                                                &mut stack, file_id,
+                                            );
+                                        }
+                                        None => Some(adt),
+                                    }
+                                }
+                                None => {
+                                    // Otherwise this could be a derive helper on a variant or field
+                                    attr.syntax().ancestors().find_map(ast::Item::cast).and_then(
+                                        |it| match it {
+                                            ast::Item::Struct(it) => Some(ast::Adt::Struct(it)),
+                                            ast::Item::Enum(it) => Some(ast::Adt::Enum(it)),
+                                            ast::Item::Union(it) => Some(ast::Adt::Union(it)),
+                                            _ => None,
+                                        },
+                                    )
+                                }
+                            }?;
+                            if !self.with_ctx(|ctx| ctx.has_derives(InFile::new(file_id, &adt))) {
+                                return None;
+                            }
+                            let attr_name =
+                                attr.path().and_then(|it| it.as_single_name_ref())?.as_name();
+                            // Not an attribute, nor a derive, so it's either a builtin or a derive helper
+                            // Try to resolve to a derive helper and downmap
+                            let id = self.db.ast_id_map(file_id).ast_id(&adt);
+                            let helpers =
+                                def_map.derive_helpers_in_scope(InFile::new(file_id, id))?;
+
+                            let mut res = None;
+                            for (.., derive) in
+                                helpers.iter().filter(|(helper, ..)| *helper == attr_name)
+                            {
+                                res = res.or(process_expansion_for_token(
+                                    &mut stack,
+                                    derive.as_macro_file(),
+                                ));
+                            }
+                            res
                         }
-                        res
-                    } else {
-                        None
                     }
                 })()
                 .is_none();
diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs
index 79c503e0a1..f0c6e7a63b 100644
--- a/crates/ide-completion/src/context/analysis.rs
+++ b/crates/ide-completion/src/context/analysis.rs
@@ -3,8 +3,9 @@ use std::iter;
 
 use hir::{Semantics, Type, TypeInfo, Variant};
 use ide_db::{active_parameter::ActiveParameter, RootDatabase};
+use itertools::Either;
 use syntax::{
-    algo::{find_node_at_offset, non_trivia_sibling},
+    algo::{ancestors_at_offset, find_node_at_offset, non_trivia_sibling},
     ast::{self, AttrKind, HasArgList, HasGenericParams, HasLoopBody, HasName, NameOrNameRef},
     match_ast, AstNode, AstToken, Direction, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode,
     SyntaxToken, TextRange, TextSize, T,
@@ -119,20 +120,45 @@ fn expand(
         }
 
         // No attributes have been expanded, so look for macro_call! token trees or derive token trees
-        let orig_tt = match find_node_at_offset::<ast::TokenTree>(&original_file, offset) {
+        let orig_tt = match ancestors_at_offset(&original_file, offset)
+            .map_while(Either::<ast::TokenTree, ast::Meta>::cast)
+            .last()
+        {
             Some(it) => it,
             None => break 'expansion,
         };
-        let spec_tt = match find_node_at_offset::<ast::TokenTree>(&speculative_file, offset) {
+        let spec_tt = match ancestors_at_offset(&speculative_file, offset)
+            .map_while(Either::<ast::TokenTree, ast::Meta>::cast)
+            .last()
+        {
             Some(it) => it,
             None => break 'expansion,
         };
 
-        // Expand pseudo-derive expansion
-        if let (Some(orig_attr), Some(spec_attr)) = (
-            orig_tt.syntax().parent().and_then(ast::Meta::cast).and_then(|it| it.parent_attr()),
-            spec_tt.syntax().parent().and_then(ast::Meta::cast).and_then(|it| it.parent_attr()),
-        ) {
+        let (tts, attrs) = match (orig_tt, spec_tt) {
+            (Either::Left(orig_tt), Either::Left(spec_tt)) => {
+                let attrs = orig_tt
+                    .syntax()
+                    .parent()
+                    .and_then(ast::Meta::cast)
+                    .and_then(|it| it.parent_attr())
+                    .zip(
+                        spec_tt
+                            .syntax()
+                            .parent()
+                            .and_then(ast::Meta::cast)
+                            .and_then(|it| it.parent_attr()),
+                    );
+                (Some((orig_tt, spec_tt)), attrs)
+            }
+            (Either::Right(orig_path), Either::Right(spec_path)) => {
+                (None, orig_path.parent_attr().zip(spec_path.parent_attr()))
+            }
+            _ => break 'expansion,
+        };
+
+        // Expand pseudo-derive expansion aka `derive(Debug$0)`
+        if let Some((orig_attr, spec_attr)) = attrs {
             if let (Some(actual_expansion), Some((fake_expansion, fake_mapped_token))) = (
                 sema.expand_derive_as_pseudo_attr_macro(&orig_attr),
                 sema.speculative_expand_derive_as_pseudo_attr_macro(
@@ -147,15 +173,54 @@ fn expand(
                     fake_mapped_token.text_range().start(),
                     orig_attr,
                 ));
+                break 'expansion;
+            }
+
+            if let Some(spec_adt) =
+                spec_attr.syntax().ancestors().find_map(ast::Item::cast).and_then(|it| match it {
+                    ast::Item::Struct(it) => Some(ast::Adt::Struct(it)),
+                    ast::Item::Enum(it) => Some(ast::Adt::Enum(it)),
+                    ast::Item::Union(it) => Some(ast::Adt::Union(it)),
+                    _ => None,
+                })
+            {
+                // might be the path of derive helper or a token tree inside of one
+                if let Some(helpers) = sema.derive_helper(&orig_attr) {
+                    for (_mac, file) in helpers {
+                        if let Some((fake_expansion, fake_mapped_token)) = sema
+                            .speculative_expand_raw(
+                                file,
+                                spec_adt.syntax(),
+                                fake_ident_token.clone(),
+                            )
+                        {
+                            // we are inside a derive helper token tree, treat this as being inside
+                            // the derive expansion
+                            let actual_expansion = sema.parse_or_expand(file.into());
+                            let new_offset = fake_mapped_token.text_range().start();
+                            if new_offset + relative_offset > actual_expansion.text_range().end() {
+                                // offset outside of bounds from the original expansion,
+                                // stop here to prevent problems from happening
+                                break 'expansion;
+                            }
+                            original_file = actual_expansion;
+                            speculative_file = fake_expansion;
+                            fake_ident_token = fake_mapped_token;
+                            offset = new_offset;
+                            continue 'expansion;
+                        }
+                    }
+                }
             }
             // at this point we won't have any more successful expansions, so stop
             break 'expansion;
         }
 
         // Expand fn-like macro calls
+        let Some((orig_tt, spec_tt)) = tts else { break 'expansion };
         if let (Some(actual_macro_call), Some(macro_call_with_fake_ident)) = (
-            orig_tt.syntax().ancestors().find_map(ast::MacroCall::cast),
-            spec_tt.syntax().ancestors().find_map(ast::MacroCall::cast),
+            orig_tt.syntax().parent().and_then(ast::MacroCall::cast),
+            spec_tt.syntax().parent().and_then(ast::MacroCall::cast),
         ) {
             let mac_call_path0 = actual_macro_call.path().as_ref().map(|s| s.syntax().text());
             let mac_call_path1 =
@@ -201,6 +266,7 @@ fn expand(
         // none of our states have changed so stop the loop
         break 'expansion;
     }
+
     ExpansionResult { original_file, speculative_file, offset, fake_ident_token, derive_ctx }
 }