Auto merge of #13091 - ice1k:hey, r=Veykril

Remove type alias definition on inline

Fix #13079

Author: bors

crates/ide-assists/src/handlers/add_missing_match_arms.rs

@@ -5,6 +5,7 @@ use hir::{Adt, Crate, HasAttrs, HasSource, ModuleDef, Semantics};
 use ide_db::RootDatabase;
 use ide_db::{famous_defs::FamousDefs, helpers::mod_path_to_ast};
 use itertools::Itertools;
+use syntax::ast::edit_in_place::Removable;
 use syntax::ast::{self, make, AstNode, HasName, MatchArmList, MatchExpr, Pat};
 
 use crate::{

crates/ide-assists/src/handlers/inline_call.rs

@@ -7,6 +7,7 @@ use ide_db::{
     imports::insert_use::remove_path_if_in_use_stmt,
     path_transform::PathTransform,
     search::{FileReference, SearchScope},
+    source_change::SourceChangeBuilder,
     syntax_helpers::{insert_whitespace_into_node::insert_ws_into, node_ext::expr_as_name_ref},
     RootDatabase,
 };
@@ -100,18 +101,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
                 builder.edit_file(file_id);
                 let count = refs.len();
                 // The collects are required as we are otherwise iterating while mutating 🙅‍♀️🙅‍♂️
-                let (name_refs, name_refs_use): (Vec<_>, Vec<_>) = refs
-                    .into_iter()
-                    .filter_map(|file_ref| match file_ref.name {
-                        ast::NameLike::NameRef(name_ref) => Some(name_ref),
-                        _ => None,
-                    })
-                    .partition_map(|name_ref| {
-                        match name_ref.syntax().ancestors().find_map(ast::UseTree::cast) {
-                            Some(use_tree) => Either::Right(builder.make_mut(use_tree)),
-                            None => Either::Left(name_ref),
-                        }
-                    });
+                let (name_refs, name_refs_use) = split_refs_and_uses(builder, refs, Some);
                 let call_infos: Vec<_> = name_refs
                     .into_iter()
                     .filter_map(CallInfo::from_name_ref)
@@ -130,11 +120,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
                     .count();
                 if replaced + name_refs_use.len() == count {
                     // we replaced all usages in this file, so we can remove the imports
-                    name_refs_use.into_iter().for_each(|use_tree| {
-                        if let Some(path) = use_tree.path() {
-                            remove_path_if_in_use_stmt(&path);
-                        }
-                    })
+                    name_refs_use.iter().for_each(remove_path_if_in_use_stmt);
                 } else {
                     remove_def = false;
                 }
@@ -153,6 +139,23 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
     )
 }
 
+pub(super) fn split_refs_and_uses<T: ast::AstNode>(
+    builder: &mut SourceChangeBuilder,
+    iter: impl IntoIterator<Item = FileReference>,
+    mut map_ref: impl FnMut(ast::NameRef) -> Option<T>,
+) -> (Vec<T>, Vec<ast::Path>) {
+    iter.into_iter()
+        .filter_map(|file_ref| match file_ref.name {
+            ast::NameLike::NameRef(name_ref) => Some(name_ref),
+            _ => None,
+        })
+        .filter_map(|name_ref| match name_ref.syntax().ancestors().find_map(ast::UseTree::cast) {
+            Some(use_tree) => builder.make_mut(use_tree).path().map(Either::Right),
+            None => map_ref(name_ref).map(Either::Left),
+        })
+        .partition_map(|either| either)
+}
+
 // Assist: inline_call
 //
 // Inlines a function or method body creating a `let` statement per parameter unless the parameter

crates/ide-assists/src/handlers/inline_type_alias.rs

@@ -3,7 +3,10 @@
 // - Remove unused aliases if there are no longer any users, see inline_call.rs.
 
 use hir::{HasSource, PathResolution};
-use ide_db::{defs::Definition, search::FileReference};
+use ide_db::{
+    defs::Definition, imports::insert_use::ast_to_remove_for_path_in_use_stmt,
+    search::FileReference,
+};
 use itertools::Itertools;
 use std::collections::HashMap;
 use syntax::{
@@ -16,6 +19,8 @@ use crate::{
     AssistId, AssistKind,
 };
 
+use super::inline_call::split_refs_and_uses;
+
 // Assist: inline_type_alias_uses
 //
 // Inline a type alias into all of its uses where possible.
@@ -31,7 +36,7 @@ use crate::{
 // ```
 // ->
 // ```
-// type A = i32;
+//
 // fn id(x: i32) -> i32 {
 //     x
 // };
@@ -58,32 +63,41 @@ pub(crate) fn inline_type_alias_uses(acc: &mut Assists, ctx: &AssistContext<'_>)
         name.syntax().text_range(),
         |builder| {
             let usages = usages.all();
+            let mut definition_deleted = false;
 
             let mut inline_refs_for_file = |file_id, refs: Vec<FileReference>| {
                 builder.edit_file(file_id);
 
-                let path_types: Vec<ast::PathType> = refs
-                    .into_iter()
-                    .filter_map(|file_ref| match file_ref.name {
-                        ast::NameLike::NameRef(path_type) => {
-                            path_type.syntax().ancestors().nth(3).and_then(ast::PathType::cast)
-                        }
-                        _ => None,
-                    })
-                    .collect();
+                let (path_types, path_type_uses) =
+                    split_refs_and_uses(builder, refs, |path_type| {
+                        path_type.syntax().ancestors().nth(3).and_then(ast::PathType::cast)
+                    });
 
+                path_type_uses
+                    .iter()
+                    .flat_map(ast_to_remove_for_path_in_use_stmt)
+                    .for_each(|x| builder.delete(x.syntax().text_range()));
                 for (target, replacement) in path_types.into_iter().filter_map(|path_type| {
                     let replacement = inline(&ast_alias, &path_type)?.to_text(&concrete_type);
                     let target = path_type.syntax().text_range();
                     Some((target, replacement))
                 }) {
                     builder.replace(target, replacement);
                 }
+
+                if file_id == ctx.file_id() {
+                    builder.delete(ast_alias.syntax().text_range());
+                    definition_deleted = true;
+                }
             };
 
             for (file_id, refs) in usages.into_iter() {
                 inline_refs_for_file(file_id, refs);
             }
+            if !definition_deleted {
+                builder.edit_file(ctx.file_id());
+                builder.delete(ast_alias.syntax().text_range());
+            }
         },
     )
 }
@@ -929,7 +943,7 @@ fn foo() {
 }
 "#,
                 r#"
-type A = u32;
+
 
 fn foo() {
     let _: u32 = 3;
@@ -960,13 +974,13 @@ fn foo() {
                 r#"
 //- /lib.rs
 mod foo;
-type T<E> = Vec<E>;
+
 fn f() -> Vec<&str> {
     vec!["hello"]
 }
 
 //- /foo.rs
-use super::T;
+
 fn foo() {
     let _: Vec<i8> = Vec::new();
 }
@@ -990,7 +1004,12 @@ fn foo() {
 }
 "#,
                 r#"
-use super::I;
+//- /lib.rs
+mod foo;
+
+
+//- /foo.rs
+
 fn foo() {
     let _: i32 = 0;
 }

crates/ide-assists/src/handlers/merge_imports.rs

@@ -1,6 +1,10 @@
 use either::Either;
 use ide_db::imports::merge_imports::{try_merge_imports, try_merge_trees, MergeBehavior};
-use syntax::{algo::neighbor, ast, match_ast, ted, AstNode, SyntaxElement, SyntaxNode};
+use syntax::{
+    algo::neighbor,
+    ast::{self, edit_in_place::Removable},
+    match_ast, ted, AstNode, SyntaxElement, SyntaxNode,
+};
 
 use crate::{
     assist_context::{AssistContext, Assists},
@@ -76,7 +80,7 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
                 .collect();
             for edit in edits_mut {
                 match edit {
-                    Remove(it) => it.as_ref().either(ast::Use::remove, ast::UseTree::remove),
+                    Remove(it) => it.as_ref().either(Removable::remove, Removable::remove),
                     Replace(old, new) => ted::replace(old, new),
                 }
             }

crates/ide-assists/src/handlers/move_bounds.rs

@@ -1,5 +1,9 @@
 use syntax::{
-    ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasName, HasTypeBounds},
+    ast::{
+        self,
+        edit_in_place::{GenericParamsOwnerEdit, Removable},
+        make, AstNode, HasName, HasTypeBounds,
+    },
     match_ast,
 };
 

crates/ide-assists/src/handlers/unmerge_use.rs

@@ -1,5 +1,5 @@
 use syntax::{
-    ast::{self, make, HasVisibility},
+    ast::{self, edit_in_place::Removable, make, HasVisibility},
     ted::{self, Position},
     AstNode, SyntaxKind,
 };

crates/ide-assists/src/tests/generated.rs

@@ -1390,7 +1390,7 @@ fn foo() {
 }
 "#####,
         r#####"
-type A = i32;
+
 fn id(x: i32) -> i32 {
     x
 };

crates/ide-assists/src/utils.rs

@@ -12,7 +12,7 @@ use syntax::{
     ast::{
         self,
         edit::{self, AstNodeEdit},
-        edit_in_place::AttrsOwnerEdit,
+        edit_in_place::{AttrsOwnerEdit, Removable},
         make, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace,
     },
     ted, AstNode, AstToken, Direction, SmolStr, SourceFile,

crates/ide-db/src/imports/insert_use.rs

@@ -7,7 +7,10 @@ use std::cmp::Ordering;
 use hir::Semantics;
 use syntax::{
     algo,
-    ast::{self, make, AstNode, HasAttrs, HasModuleItem, HasVisibility, PathSegmentKind},
+    ast::{
+        self, edit_in_place::Removable, make, AstNode, HasAttrs, HasModuleItem, HasVisibility,
+        PathSegmentKind,
+    },
     ted, Direction, NodeOrToken, SyntaxKind, SyntaxNode,
 };
 
@@ -192,20 +195,24 @@ pub fn insert_use(scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig) {
     insert_use_(scope, &path, cfg.group, use_item);
 }
 
-pub fn remove_path_if_in_use_stmt(path: &ast::Path) {
+pub fn ast_to_remove_for_path_in_use_stmt(path: &ast::Path) -> Option<Box<dyn Removable>> {
     // FIXME: improve this
     if path.parent_path().is_some() {
-        return;
+        return None;
     }
-    if let Some(use_tree) = path.syntax().parent().and_then(ast::UseTree::cast) {
-        if use_tree.use_tree_list().is_some() || use_tree.star_token().is_some() {
-            return;
-        }
-        if let Some(use_) = use_tree.syntax().parent().and_then(ast::Use::cast) {
-            use_.remove();
-            return;
-        }
-        use_tree.remove();
+    let use_tree = path.syntax().parent().and_then(ast::UseTree::cast)?;
+    if use_tree.use_tree_list().is_some() || use_tree.star_token().is_some() {
+        return None;
+    }
+    if let Some(use_) = use_tree.syntax().parent().and_then(ast::Use::cast) {
+        return Some(Box::new(use_));
+    }
+    Some(Box::new(use_tree))
+}
+
+pub fn remove_path_if_in_use_stmt(path: &ast::Path) {
+    if let Some(node) = ast_to_remove_for_path_in_use_stmt(path) {
+        node.remove();
     }
 }
 

crates/syntax/src/ast/edit_in_place.rs

@@ -248,8 +248,12 @@ impl ast::WhereClause {
     }
 }
 
-impl ast::TypeBoundList {
-    pub fn remove(&self) {
+pub trait Removable: AstNode {
+    fn remove(&self);
+}
+
+impl Removable for ast::TypeBoundList {
+    fn remove(&self) {
         match self.syntax().siblings_with_tokens(Direction::Prev).find(|it| it.kind() == T![:]) {
             Some(colon) => ted::remove_all(colon..=self.syntax().clone().into()),
             None => ted::remove(self.syntax()),
@@ -267,8 +271,8 @@ impl ast::PathSegment {
     }
 }
 
-impl ast::UseTree {
-    pub fn remove(&self) {
+impl Removable for ast::UseTree {
+    fn remove(&self) {
         for dir in [Direction::Next, Direction::Prev] {
             if let Some(next_use_tree) = neighbor(self, dir) {
                 let separators = self
@@ -282,7 +286,9 @@ impl ast::UseTree {
         }
         ted::remove(self.syntax());
     }
+}
 
+impl ast::UseTree {
     pub fn get_or_create_use_tree_list(&self) -> ast::UseTreeList {
         match self.use_tree_list() {
             Some(it) => it,
@@ -373,8 +379,8 @@ impl ast::UseTreeList {
     }
 }
 
-impl ast::Use {
-    pub fn remove(&self) {
+impl Removable for ast::Use {
+    fn remove(&self) {
         let next_ws = self
             .syntax()
             .next_sibling_or_token()
@@ -444,8 +450,8 @@ impl ast::Fn {
     }
 }
 
-impl ast::MatchArm {
-    pub fn remove(&self) {
+impl Removable for ast::MatchArm {
+    fn remove(&self) {
         if let Some(sibling) = self.syntax().prev_sibling_or_token() {
             if sibling.kind() == SyntaxKind::WHITESPACE {
                 ted::remove(sibling);