about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-12-03 13:16:22 +0000
committerbors <bors@rust-lang.org>2022-12-03 13:16:22 +0000
commit04a2ac2de20565e87c561bdb3c3814dfeec1513c (patch)
treeead6c8f8a5ba4436e81e5728080c546613239ab2
parentd7be2fa1a6d8375465c026efd079f7c7f0678775 (diff)
parent6d4538734e48d74be5f64c87dbac188cfdb1e1fd (diff)
downloadrust-04a2ac2de20565e87c561bdb3c3814dfeec1513c.tar.gz
rust-04a2ac2de20565e87c561bdb3c3814dfeec1513c.zip
Auto merge of #13707 - lowr:feat/move-const-to-impl, r=Veykril
Add `move_const_to_impl` assist

Closes #13277

For the initial implementation, this assist:
- only applies to inherent impl. Much as we can *technically* provide this assist for default impl in trait definitions, it'd be complicated to get it right.
- may break code when the const's name collides with an item of a trait the self type implements.

Comments in the code explain those caveats in a bit more detail.
-rw-r--r--crates/ide-assists/src/handlers/move_const_to_impl.rs481
-rw-r--r--crates/ide-assists/src/lib.rs2
-rw-r--r--crates/ide-assists/src/tests/generated.rs29
-rw-r--r--crates/ide-assists/src/utils.rs17
4 files changed, 529 insertions, 0 deletions
diff --git a/crates/ide-assists/src/handlers/move_const_to_impl.rs b/crates/ide-assists/src/handlers/move_const_to_impl.rs
new file mode 100644
index 00000000000..0e3a1e652b0
--- /dev/null
+++ b/crates/ide-assists/src/handlers/move_const_to_impl.rs
@@ -0,0 +1,481 @@
+use hir::{AsAssocItem, AssocItemContainer, HasCrate, HasSource};
+use ide_db::{assists::AssistId, base_db::FileRange, defs::Definition, search::SearchScope};
+use syntax::{
+    ast::{self, edit::IndentLevel, edit_in_place::Indent, AstNode},
+    SyntaxKind,
+};
+
+use crate::{
+    assist_context::{AssistContext, Assists},
+    utils,
+};
+
+// NOTE: Code may break if the self type implements a trait that has associated const with the same
+// name, but it's pretty expensive to check that (`hir::Impl::all_for_type()`) and we assume that's
+// pretty rare case.
+
+// Assist: move_const_to_impl
+//
+// Move a local constant item in a method to impl's associated constant. All the references will be
+// qualified with `Self::`.
+//
+// ```
+// struct S;
+// impl S {
+//     fn foo() -> usize {
+//         /// The answer.
+//         const C$0: usize = 42;
+//
+//         C * C
+//     }
+// }
+// ```
+// ->
+// ```
+// struct S;
+// impl S {
+//     /// The answer.
+//     const C: usize = 42;
+//
+//     fn foo() -> usize {
+//         Self::C * Self::C
+//     }
+// }
+// ```
+pub(crate) fn move_const_to_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
+    let db = ctx.db();
+    let const_: ast::Const = ctx.find_node_at_offset()?;
+    // Don't show the assist when the cursor is at the const's body.
+    if let Some(body) = const_.body() {
+        if body.syntax().text_range().contains(ctx.offset()) {
+            return None;
+        }
+    }
+
+    let parent_fn = const_.syntax().ancestors().find_map(ast::Fn::cast)?;
+
+    // NOTE: We can technically provide this assist for default methods in trait definitions, but
+    // it's somewhat complex to handle it correctly when the const's name conflicts with
+    // supertrait's item. We may want to consider implementing it in the future.
+    let AssocItemContainer::Impl(impl_) = ctx.sema.to_def(&parent_fn)?.as_assoc_item(db)?.container(db) else { return None; };
+    if impl_.trait_(db).is_some() {
+        return None;
+    }
+
+    let def = ctx.sema.to_def(&const_)?;
+    let name = def.name(db)?;
+    let items = impl_.source(db)?.value.assoc_item_list()?;
+
+    let ty = impl_.self_ty(db);
+    // If there exists another associated item with the same name, skip the assist.
+    if ty
+        .iterate_assoc_items(db, ty.krate(db), |assoc| {
+            // Type aliases wouldn't conflict due to different namespaces, but we're only checking
+            // the items in inherent impls, so we assume `assoc` is never type alias for the sake
+            // of brevity (inherent associated types exist in nightly Rust, but it's *very*
+            // unstable and we don't support them either).
+            assoc.name(db).filter(|it| it == &name)
+        })
+        .is_some()
+    {
+        return None;
+    }
+
+    let usages =
+        Definition::Const(def).usages(&ctx.sema).in_scope(SearchScope::file_range(FileRange {
+            file_id: ctx.file_id(),
+            range: parent_fn.syntax().text_range(),
+        }));
+
+    acc.add(
+        AssistId("move_const_to_impl", crate::AssistKind::RefactorRewrite),
+        "Move const to impl block",
+        const_.syntax().text_range(),
+        |builder| {
+            let range_to_delete = match const_.syntax().next_sibling_or_token() {
+                Some(s) if matches!(s.kind(), SyntaxKind::WHITESPACE) => {
+                    // Remove following whitespaces too.
+                    const_.syntax().text_range().cover(s.text_range())
+                }
+                _ => const_.syntax().text_range(),
+            };
+            builder.delete(range_to_delete);
+
+            let const_ref = format!("Self::{name}");
+            for range in usages.all().file_ranges().map(|it| it.range) {
+                builder.replace(range, const_ref.clone());
+            }
+
+            // Heuristically inserting the extracted const after the consecutive existing consts
+            // from the beginning of assoc items. We assume there are no inherent assoc type as
+            // above.
+            let last_const =
+                items.assoc_items().take_while(|it| matches!(it, ast::AssocItem::Const(_))).last();
+            let insert_offset = match &last_const {
+                Some(it) => it.syntax().text_range().end(),
+                None => match items.l_curly_token() {
+                    Some(l_curly) => l_curly.text_range().end(),
+                    // Not sure if this branch is ever reachable, but it wouldn't hurt to have a
+                    // fallback.
+                    None => items.syntax().text_range().start(),
+                },
+            };
+
+            // If the moved const will be the first item of the impl, add a new line after that.
+            //
+            // We're assuming the code is formatted according to Rust's standard style guidelines
+            // (i.e. no empty lines between impl's `{` token and its first assoc item).
+            let fixup = if last_const.is_none() { "\n" } else { "" };
+            let indent = IndentLevel::from_node(parent_fn.syntax());
+
+            let const_ = const_.clone_for_update();
+            const_.reindent_to(indent);
+            let mut const_text = format!("\n{indent}{const_}{fixup}");
+            utils::escape_non_snippet(&mut const_text);
+            builder.insert(insert_offset, const_text);
+        },
+    )
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    use super::*;
+
+    #[test]
+    fn not_applicable_to_top_level_const() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+const C$0: () = ();
+"#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_to_free_fn() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+fn f() {
+    const C$0: () = ();
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_when_at_const_body() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() {
+        const C: () = ($0);
+    }
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_when_inside_const_body_block() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() {
+        const C: () = {
+            ($0)
+        };
+    }
+}
+            "#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_to_trait_impl_fn() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+trait Trait {
+    fn f();
+}
+impl Trait for () {
+    fn f() {
+        const C$0: () = ();
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_to_non_assoc_fn_inside_impl() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() {
+        fn g() {
+            const C$0: () = ();
+        }
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn not_applicable_when_const_with_same_name_exists() {
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    const C: usize = 42;
+    fn f() {
+        const C$0: () = ();
+    }
+"#,
+        );
+
+        check_assist_not_applicable(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    const C: usize = 42;
+}
+impl S {
+    fn f() {
+        const C$0: () = ();
+    }
+"#,
+        );
+    }
+
+    #[test]
+    fn move_const_simple_body() {
+        check_assist(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() -> usize {
+        /// doc comment
+        const C$0: usize = 42;
+
+        C * C
+    }
+}
+"#,
+            r#"
+struct S;
+impl S {
+    /// doc comment
+    const C: usize = 42;
+
+    fn f() -> usize {
+        Self::C * Self::C
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn move_const_simple_body_existing_const() {
+        check_assist(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    const X: () = ();
+    const Y: () = ();
+
+    fn f() -> usize {
+        /// doc comment
+        const C$0: usize = 42;
+
+        C * C
+    }
+}
+"#,
+            r#"
+struct S;
+impl S {
+    const X: () = ();
+    const Y: () = ();
+    /// doc comment
+    const C: usize = 42;
+
+    fn f() -> usize {
+        Self::C * Self::C
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn move_const_block_body() {
+        check_assist(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() -> usize {
+        /// doc comment
+        const C$0: usize = {
+            let a = 3;
+            let b = 4;
+            a * b
+        };
+
+        C * C
+    }
+}
+"#,
+            r#"
+struct S;
+impl S {
+    /// doc comment
+    const C: usize = {
+        let a = 3;
+        let b = 4;
+        a * b
+    };
+
+    fn f() -> usize {
+        Self::C * Self::C
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn correct_indent_when_nested() {
+        check_assist(
+            move_const_to_impl,
+            r#"
+fn main() {
+    struct S;
+    impl S {
+        fn f() -> usize {
+            /// doc comment
+            const C$0: usize = 42;
+
+            C * C
+        }
+    }
+}
+"#,
+            r#"
+fn main() {
+    struct S;
+    impl S {
+        /// doc comment
+        const C: usize = 42;
+
+        fn f() -> usize {
+            Self::C * Self::C
+        }
+    }
+}
+"#,
+        )
+    }
+
+    #[test]
+    fn move_const_in_nested_scope_with_same_name_in_other_scope() {
+        check_assist(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() -> usize {
+        const C: &str = "outer";
+
+        let n = {
+            /// doc comment
+            const C$0: usize = 42;
+
+            let m = {
+                const C: &str = "inner";
+                C.len()
+            };
+
+            C * m
+        };
+
+        n + C.len()
+    }
+}
+"#,
+            r#"
+struct S;
+impl S {
+    /// doc comment
+    const C: usize = 42;
+
+    fn f() -> usize {
+        const C: &str = "outer";
+
+        let n = {
+            let m = {
+                const C: &str = "inner";
+                C.len()
+            };
+
+            Self::C * m
+        };
+
+        n + C.len()
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn moved_const_body_is_escaped() {
+        // Note that the last argument is what *lsp clients would see* rather than
+        // what users would see. Unescaping happens thereafter.
+        check_assist(
+            move_const_to_impl,
+            r#"
+struct S;
+impl S {
+    fn f() -> usize {
+        /// doc comment
+        /// \\
+        /// ${snippet}
+        const C$0: &str = "\ and $1";
+
+        C.len()
+    }
+}
+"#,
+            r#"
+struct S;
+impl S {
+    /// doc comment
+    /// \\\\
+    /// \${snippet}
+    const C: &str = "\\ and \$1";
+
+    fn f() -> usize {
+        Self::C.len()
+    }
+}
+"#,
+        )
+    }
+}
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index b12f99cc532..a55de800b39 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -165,6 +165,7 @@ mod handlers {
     mod merge_imports;
     mod merge_match_arms;
     mod move_bounds;
+    mod move_const_to_impl;
     mod move_guard;
     mod move_module_to_file;
     mod move_to_mod_rs;
@@ -261,6 +262,7 @@ mod handlers {
             merge_imports::merge_imports,
             merge_match_arms::merge_match_arms,
             move_bounds::move_bounds_to_where_clause,
+            move_const_to_impl::move_const_to_impl,
             move_format_string_arg::move_format_string_arg,
             move_guard::move_arm_cond_to_match_guard,
             move_guard::move_guard_to_arm_body,
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index d797f077672..ccd38119c4a 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -1675,6 +1675,35 @@ fn apply<T, U, F>(f: F, x: T) -> U where F: FnOnce(T) -> U {
 }
 
 #[test]
+fn doctest_move_const_to_impl() {
+    check_doc_test(
+        "move_const_to_impl",
+        r#####"
+struct S;
+impl S {
+    fn foo() -> usize {
+        /// The answer.
+        const C$0: usize = 42;
+
+        C * C
+    }
+}
+"#####,
+        r#####"
+struct S;
+impl S {
+    /// The answer.
+    const C: usize = 42;
+
+    fn foo() -> usize {
+        Self::C * Self::C
+    }
+}
+"#####,
+    )
+}
+
+#[test]
 fn doctest_move_format_string_arg() {
     check_doc_test(
         "move_format_string_arg",
diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs
index 68c31b4f8e9..f38a2d04ff6 100644
--- a/crates/ide-assists/src/utils.rs
+++ b/crates/ide-assists/src/utils.rs
@@ -208,6 +208,23 @@ pub(crate) fn render_snippet(_cap: SnippetCap, node: &SyntaxNode, cursor: Cursor
     }
 }
 
+/// Escapes text that should be rendered as-is, typically those that we're copy-pasting what the
+/// users wrote.
+///
+/// This function should only be used when the text doesn't contain snippet **AND** the text
+/// wouldn't be included in a snippet.
+pub(crate) fn escape_non_snippet(text: &mut String) {
+    // While we *can* escape `}`, we don't really have to in this specific case. We only need to
+    // escape it inside `${}` to disambiguate it from the ending token of the syntax, but after we
+    // escape every occurrence of `$`, we wouldn't have `${}` in the first place.
+    //
+    // This will break if the text contains snippet or it will be included in a snippet (hence doc
+    // comment). Compare `fn escape(buf)` in `render_snippet()` above, where the escaped text is
+    // included in a snippet.
+    stdx::replace(text, '\\', r"\\");
+    stdx::replace(text, '$', r"\$");
+}
+
 pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize {
     node.children_with_tokens()
         .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))