Repository: rust-analyzer/rust-analyzer
Pull request #1761 (2019-09-04 16:59:42)

"move type bounds to where clause" assist


Patch:
diff --git a/crates/ra_assists/src/ast_editor.rs b/crates/ra_assists/src/ast_editor.rs
index 0484786625..a710edce8e 100644
--- a/crates/ra_assists/src/ast_editor.rs
+++ b/crates/ra_assists/src/ast_editor.rs
@@ -297,6 +297,11 @@ impl AstBuilder<ast::Path> {
         ast_node_from_file_text(text)
     }
 
+    pub fn from_name(name: ast::Name) -> ast::Path {
+        let name = name.syntax().to_string();
+        Self::from_text(name.as_str())
+    }
+
     pub fn from_pieces(enum_name: ast::Name, var_name: ast::Name) -> ast::Path {
         Self::from_text(&format!("{}::{}", enum_name.syntax(), var_name.syntax()))
     }
@@ -380,6 +385,31 @@ impl AstBuilder<ast::MatchArmList> {
     }
 }
 
+impl AstBuilder<ast::WherePred> {
+    fn from_text(text: &str) -> ast::WherePred {
+        ast_node_from_file_text(&format!("fn f() where {} {{ }}", text))
+    }
+
+    pub fn from_pieces(
+        path: ast::Path,
+        bounds: impl Iterator<Item = ast::TypeBound>,
+    ) -> ast::WherePred {
+        let bounds = bounds.map(|b| b.syntax().to_string()).collect::<Vec<_>>().join(" + ");
+        Self::from_text(&format!("{}: {}", path.syntax(), bounds))
+    }
+}
+
+impl AstBuilder<ast::WhereClause> {
+    fn from_text(text: &str) -> ast::WhereClause {
+        ast_node_from_file_text(&format!("fn f() where {} {{ }}", text))
+    }
+
+    pub fn from_predicates(preds: impl Iterator<Item = ast::WherePred>) -> ast::WhereClause {
+        let preds = preds.map(|p| p.syntax().to_string()).collect::<Vec<_>>().join(", ");
+        Self::from_text(preds.as_str())
+    }
+}
+
 fn ast_node_from_file_text<N: AstNode>(text: &str) -> N {
     let parse = SourceFile::parse(text);
     let res = parse.tree().syntax().descendants().find_map(N::cast).unwrap();
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 03eec73ad8..10ccc345c9 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -102,6 +102,7 @@ mod remove_dbg;
 pub mod auto_import;
 mod add_missing_impl_members;
 mod move_guard;
+mod move_bounds;
 
 fn all_assists<DB: HirDatabase>() -> &'static [fn(AssistCtx<DB>) -> Option<Assist>] {
     &[
@@ -123,6 +124,7 @@ fn all_assists<DB: HirDatabase>() -> &'static [fn(AssistCtx<DB>) -> Option<Assis
         inline_local_variable::inline_local_varialbe,
         move_guard::move_guard_to_arm_body,
         move_guard::move_arm_cond_to_match_guard,
+        move_bounds::move_bounds_to_where_clause,
     ]
 }
 
diff --git a/crates/ra_assists/src/move_bounds.rs b/crates/ra_assists/src/move_bounds.rs
new file mode 100644
index 0000000000..526de1d983
--- /dev/null
+++ b/crates/ra_assists/src/move_bounds.rs
@@ -0,0 +1,135 @@
+use hir::db::HirDatabase;
+use ra_syntax::{
+    ast::{self, AstNode, NameOwner, TypeBoundsOwner},
+    SyntaxElement,
+    SyntaxKind::*,
+    TextRange,
+};
+
+use crate::{ast_editor::AstBuilder, Assist, AssistCtx, AssistId};
+
+pub(crate) fn move_bounds_to_where_clause(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+    let type_param_list = ctx.node_at_offset::<ast::TypeParamList>()?;
+
+    let mut type_params = type_param_list.type_params();
+    if type_params.all(|p| p.type_bound_list().is_none()) {
+        return None;
+    }
+
+    let parent = type_param_list.syntax().parent()?;
+    if parent.children_with_tokens().find(|it| it.kind() == WHERE_CLAUSE).is_some() {
+        return None;
+    }
+
+    let anchor: SyntaxElement = match parent.kind() {
+        FN_DEF => ast::FnDef::cast(parent)?.body()?.syntax().clone().into(),
+        TRAIT_DEF => ast::TraitDef::cast(parent)?.item_list()?.syntax().clone().into(),
+        IMPL_BLOCK => ast::ImplBlock::cast(parent)?.item_list()?.syntax().clone().into(),
+        ENUM_DEF => ast::EnumDef::cast(parent)?.variant_list()?.syntax().clone().into(),
+        STRUCT_DEF => parent
+            .children_with_tokens()
+            .find(|it| it.kind() == RECORD_FIELD_DEF_LIST || it.kind() == SEMI)?,
+        _ => return None,
+    };
+
+    ctx.add_action(
+        AssistId("move_bounds_to_where_clause"),
+        "move_bounds_to_where_clause",
+        |edit| {
+            let type_params = type_param_list.type_params().collect::<Vec<_>>();
+
+            for param in &type_params {
+                if let Some(bounds) = param.type_bound_list() {
+                    let colon = param
+                        .syntax()
+                        .children_with_tokens()
+                        .find(|it| it.kind() == COLON)
+                        .unwrap();
+                    let start = colon.text_range().start();
+                    let end = bounds.syntax().text_range().end();
+                    edit.delete(TextRange::from_to(start, end));
+                }
+            }
+
+            let predicates = type_params.iter().filter_map(build_predicate);
+            let where_clause = AstBuilder::<ast::WhereClause>::from_predicates(predicates);
+
+            let to_insert = match anchor.prev_sibling_or_token() {
+                Some(ref elem) if elem.kind() == WHITESPACE => {
+                    format!("{} ", where_clause.syntax())
+                }
+                _ => format!(" {}", where_clause.syntax()),
+            };
+            edit.insert(anchor.text_range().start(), to_insert);
+            edit.target(type_param_list.syntax().text_range());
+        },
+    );
+
+    ctx.build()
+}
+
+fn build_predicate(param: &ast::TypeParam) -> Option<ast::WherePred> {
+    let path = AstBuilder::<ast::Path>::from_name(param.name()?);
+    let predicate =
+        AstBuilder::<ast::WherePred>::from_pieces(path, param.type_bound_list()?.bounds());
+    Some(predicate)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::helpers::check_assist;
+
+    #[test]
+    fn move_bounds_to_where_clause_fn() {
+        check_assist(
+            move_bounds_to_where_clause,
+            r#"
+            fn foo<T: u32, <|>F: FnOnce(T) -> T>() {}
+            "#,
+            r#"
+            fn foo<T, <|>F>() where T: u32, F: FnOnce(T) -> T {}
+            "#,
+        );
+    }
+
+    #[test]
+    fn move_bounds_to_where_clause_impl() {
+        check_assist(
+            move_bounds_to_where_clause,
+            r#"
+            impl<U: u32, <|>T> A<U, T> {}
+            "#,
+            r#"
+            impl<U, <|>T> A<U, T> where U: u32 {}
+            "#,
+        );
+    }
+
+    #[test]
+    fn move_bounds_to_where_clause_struct() {
+        check_assist(
+            move_bounds_to_where_clause,
+            r#"
+            struct A<<|>T: Iterator<Item = u32>> {}
+            "#,
+            r#"
+            struct A<<|>T> where T: Iterator<Item = u32> {}
+            "#,
+        );
+    }
+
+    #[test]
+    fn move_bounds_to_where_clause_tuple_struct() {
+        check_assist(
+            move_bounds_to_where_clause,
+            r#"
+            struct Pair<<|>T: u32>(T, T);
+            "#,
+            r#"
+            struct Pair<<|>T>(T, T) where T: u32;
+            "#,
+        );
+    }
+}
diff --git a/crates/ra_hir/src/marks.rs b/crates/ra_hir/src/marks.rs
index 2e1d35c8cb..5b15eee901 100644
--- a/crates/ra_hir/src/marks.rs
+++ b/crates/ra_hir/src/marks.rs
@@ -11,5 +11,4 @@ test_utils::marks!(
     match_ergonomics_ref
     trait_resolution_on_fn_type
     infer_while_let
-    macro_rules_from_other_crates_are_visible_with_macro_use
 );
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index f69179bf68..bbdc606cd9 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -101,8 +101,6 @@ pub struct CrateDefMap {
     /// However, do we want to put it as a global variable?
     poison_macros: FxHashSet<MacroDefId>,
 
-    exported_macros: FxHashMap<Name, MacroDefId>,
-
     diagnostics: Vec<DefDiagnostic>,
 }
 
@@ -247,7 +245,6 @@ impl CrateDefMap {
                 root,
                 modules,
                 poison_macros: FxHashSet::default(),
-                exported_macros: FxHashMap::default(),
                 diagnostics: Vec::new(),
             }
         };
diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index 5d1c429265..7da2dcdff1 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -157,42 +157,11 @@ where
         // crate root, even if the parent modules is **not** visible.
         if export {
             self.update(self.def_map.root, None, &[(name.clone(), def.clone())]);
-
-            // Exported macros are collected in crate level ready for
-            // glob import with `#[macro_use]`.
-            self.def_map.exported_macros.insert(name.clone(), macro_id);
         }
         self.update(module_id, None, &[(name.clone(), def)]);
         self.global_macro_scope.insert(name, macro_id);
     }
 
-    /// Import macros from `#[macro_use] extern crate`.
-    ///
-    /// They are non-scoped, and will only be inserted into mutable `global_macro_scope`.
-    fn import_macros_from_extern_crate(&mut self, import: &raw::ImportData) {
-        log::debug!(
-            "importing macros from extern crate: {:?} ({:?})",
-            import,
-            self.def_map.edition,
-        );
-
-        let res = self.def_map.resolve_name_in_extern_prelude(
-            &import
-                .path
-                .as_ident()
-                .expect("extern crate should have been desugared to one-element path"),
-        );
-
-        if let Some(ModuleDef::Module(m)) = res.take_types() {
-            tested_by!(macro_rules_from_other_crates_are_visible_with_macro_use);
-
-            let item_map = self.db.crate_def_map(m.krate);
-            for (name, &macro_id) in &item_map.exported_macros {
-                self.global_macro_scope.insert(name.clone(), macro_id);
-            }
-        }
-    }
-
     fn resolve_imports(&mut self) -> ReachedFixedPoint {
         let mut imports = std::mem::replace(&mut self.unresolved_imports, Vec::new());
         let mut resolved = Vec::new();
@@ -525,17 +494,11 @@ where
         for item in items {
             match *item {
                 raw::RawItem::Module(m) => self.collect_module(&self.raw_items[m]),
-                raw::RawItem::Import(import_id) => {
-                    let import = self.raw_items[import_id].clone();
-                    // This should be processed eagerly instead of deferred to resolving.
-                    // Otherwise, since it will only mutate `global_macro_scope`
-                    // without `update` names in `mod`s, unresolved macros cannot be expanded.
-                    if import.is_extern_crate && import.is_macro_use {
-                        self.def_collector.import_macros_from_extern_crate(&import);
-                    }
-
-                    self.def_collector.unresolved_imports.push((self.module_id, import_id, import));
-                }
+                raw::RawItem::Import(import) => self.def_collector.unresolved_imports.push((
+                    self.module_id,
+                    import,
+                    self.raw_items[import].clone(),
+                )),
                 raw::RawItem::Def(def) => self.define_def(&self.raw_items[def]),
                 raw::RawItem::Macro(mac) => self.collect_macro(&self.raw_items[mac]),
             }
@@ -897,7 +860,6 @@ mod tests {
                 root,
                 modules,
                 poison_macros: FxHashSet::default(),
-                exported_macros: FxHashMap::default(),
                 diagnostics: Vec::new(),
             }
         };
diff --git a/crates/ra_hir/src/nameres/raw.rs b/crates/ra_hir/src/nameres/raw.rs
index 129b047eb5..2f973359fe 100644
--- a/crates/ra_hir/src/nameres/raw.rs
+++ b/crates/ra_hir/src/nameres/raw.rs
@@ -154,7 +154,6 @@ pub struct ImportData {
     pub(super) is_glob: bool,
     pub(super) is_prelude: bool,
     pub(super) is_extern_crate: bool,
-    pub(super) is_macro_use: bool,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -294,14 +293,8 @@ impl RawItemsCollector {
         let is_prelude = use_item.has_atom_attr("prelude_import");
 
         Path::expand_use_item(&use_item, |path, use_tree, is_glob, alias| {
-            let import_data = ImportData {
-                path,
-                alias,
-                is_glob,
-                is_prelude,
-                is_extern_crate: false,
-                is_macro_use: false,
-            };
+            let import_data =
+                ImportData { path, alias, is_glob, is_prelude, is_extern_crate: false };
             self.push_import(current_module, import_data, Either::A(AstPtr::new(use_tree)));
         })
     }
@@ -314,14 +307,12 @@ impl RawItemsCollector {
         if let Some(name_ref) = extern_crate.name_ref() {
             let path = Path::from_name_ref(&name_ref);
             let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name());
-            let is_macro_use = extern_crate.has_atom_attr("macro_use");
             let import_data = ImportData {
                 path,
                 alias,
                 is_glob: false,
                 is_prelude: false,
                 is_extern_crate: true,
-                is_macro_use,
             };
             self.push_import(current_module, import_data, Either::B(AstPtr::new(&extern_crate)));
         }
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index bcf753f784..e2a92ae604 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -934,7 +934,6 @@ impl AstNode for ExternCrateItem {
         &self.syntax
     }
 }
-impl ast::AttrsOwner for ExternCrateItem {}
 impl ExternCrateItem {
     pub fn name_ref(&self) -> Option<NameRef> {
         AstChildren::new(&self.syntax).next()
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index 3e6c2d3f3c..c14ee0e856 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -669,7 +669,6 @@ Grammar(
             collections: [("use_trees", "UseTree")]
         ),
         "ExternCrateItem": (
-            traits: ["AttrsOwner"],
             options: ["NameRef", "Alias"],
         ),
         "ArgList": (
diff --git a/docs/user/features.md b/docs/user/features.md
index b6e6008c42..1034a51172 100644
--- a/docs/user/features.md
+++ b/docs/user/features.md
@@ -435,6 +435,16 @@ fn f() {
 }
 ```
 
+- Move type bounds to where clause
+
+```rust
+// before:
+fn foo<T: u32, F: FnOnce(T) -> T>() {}
+
+// after:
+fn foo<T, F>() where T: u32, F: FnOnce(T) -> T {}
+```
+
 ### Magic Completions
 
 In addition to usual reference completion, rust-analyzer provides some ✨magic✨


Test patch:
diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs
index ebfefe273f..631df2ceff 100644
--- a/crates/ra_hir/src/nameres/tests/macros.rs
+++ b/crates/ra_hir/src/nameres/tests/macros.rs
@@ -99,14 +99,14 @@ fn macro_rules_from_other_crates_are_visible() {
 fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
     let map = def_map_with_crate_graph(
         "
-        //- /main.rs
+        //- /main.rs      
         macro_rules! baz {
             () => {
-                use foo::bar;
+                use foo::bar;                
             }
         }
-
-        foo!();
+        
+        foo!();              
         bar!();
         baz!();
 
@@ -114,7 +114,7 @@ fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
         #[macro_export]
         macro_rules! foo {
             () => {
-                struct Foo { field: u32 }
+                struct Foo { field: u32 } 
             }
         }
         #[macro_export]
@@ -137,57 +137,3 @@ fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
    ⋮foo: m
     "###);
 }
-
-#[test]
-fn macro_rules_from_other_crates_are_visible_with_macro_use() {
-    covers!(macro_rules_from_other_crates_are_visible_with_macro_use);
-    let map = def_map_with_crate_graph(
-        "
-        //- /main.rs
-        #[macro_use]
-        extern crate foo;
-
-        structs!(Foo);
-        structs_priv!(Bar);
-        structs_not_exported!(MacroNotResolved1);
-        crate::structs!(MacroNotResolved2);
-
-        mod bar;
-
-        //- /bar.rs
-        structs!(Baz);
-        crate::structs!(MacroNotResolved3);
-
-        //- /lib.rs
-        #[macro_export]
-        macro_rules! structs {
-            ($i:ident) => { struct $i; }
-        }
-
-        macro_rules! structs_not_exported {
-            ($i:ident) => { struct $i; }
-        }
-
-        mod priv_mod {
-            #[macro_export]
-            macro_rules! structs_priv {
-                ($i:ident) => { struct $i; }
-            }
-        }
-        ",
-        crate_graph! {
-            "main": ("/main.rs", ["foo"]),
-            "foo": ("/lib.rs", []),
-        },
-    );
-    assert_snapshot!(map, @r###"
-   ⋮crate
-   ⋮Bar: t v
-   ⋮Foo: t v
-   ⋮bar: t
-   ⋮foo: t
-   ⋮
-   ⋮crate::bar
-   ⋮Baz: t v
-    "###);
-}