Merge #6019

6019: Remove make::path_from_text r=matklad a=Veykril

This removes the `make::path_from_text` function, which according to a note should've been private. I removed it since it didn't really serve a purpose as it was simply wrapping `make::ast_from_text`.

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/assists/src/handlers/auto_import.rs

@@ -1,6 +1,5 @@
 use std::collections::BTreeSet;
 
-use ast::make;
 use either::Either;
 use hir::{
     AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait,
@@ -54,11 +53,8 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             format!("Import `{}`", &import),
             range,
             |builder| {
-                let new_syntax = insert_use(
+                let new_syntax =
-                    &scope,
+                    insert_use(&scope, import.to_ast_path(), ctx.config.insert_use.merge);
-                    make::path_from_text(&import.to_string()),
-                    ctx.config.insert_use.merge,
-                );
                 builder.replace(syntax.text_range(), new_syntax.to_string())
             },
         );

crates/assists/src/handlers/expand_glob_import.rs

@@ -264,12 +264,8 @@ fn replace_ast(
     match use_trees.as_slice() {
         [name] => {
             if let Some(end_path) = name.path() {
-                let replacement = make::use_tree(
+                let replacement =
-                    make::path_from_text(&format!("{}::{}", path, end_path)),
+                    make::use_tree(make::path_concat(path, end_path), None, None, false);
-                    None,
-                    None,
-                    false,
-                );
 
                 algo::diff(
                     &parent.either(|n| n.syntax().clone(), |n| n.syntax().clone()),

crates/assists/src/handlers/extract_struct_from_enum_variant.rs

@@ -12,7 +12,6 @@ use syntax::{
 use crate::{
     assist_context::AssistBuilder, utils::insert_use, AssistContext, AssistId, AssistKind, Assists,
 };
-use ast::make;
 use insert_use::ImportScope;
 
 // Assist: extract_struct_from_enum_variant
@@ -112,11 +111,7 @@ fn insert_import(
         let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?;
         let syntax = scope.as_syntax_node();
 
-        let new_syntax = insert_use(
+        let new_syntax = insert_use(&scope, mod_path.to_ast_path(), ctx.config.insert_use.merge);
-            &scope,
-            make::path_from_text(&mod_path.to_string()),
-            ctx.config.insert_use.merge,
-        );
         // FIXME: this will currently panic as multiple imports will have overlapping text ranges
         builder.replace(syntax.text_range(), new_syntax.to_string())
     }

crates/assists/src/handlers/replace_qualified_name_with_use.rs

@@ -1,11 +1,10 @@
-use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange};
+use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode};
 use test_utils::mark;
 
 use crate::{
     utils::{insert_use, ImportScope},
     AssistContext, AssistId, AssistKind, Assists,
 };
-use ast::make;
 
 // Assist: replace_qualified_name_with_use
 //
@@ -33,15 +32,6 @@ pub(crate) fn replace_qualified_name_with_use(
         mark::hit!(dont_import_trivial_paths);
         return None;
     }
-    let path_to_import = path.to_string();
-    let path_to_import = match path.segment()?.generic_arg_list() {
-        Some(generic_args) => {
-            let generic_args_start =
-                generic_args.syntax().text_range().start() - path.syntax().text_range().start();
-            &path_to_import[TextRange::up_to(generic_args_start)]
-        }
-        None => path_to_import.as_str(),
-    };
 
     let target = path.syntax().text_range();
     let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?;
@@ -54,14 +44,10 @@ pub(crate) fn replace_qualified_name_with_use(
             // Now that we've brought the name into scope, re-qualify all paths that could be
             // affected (that is, all paths inside the node we added the `use` to).
             let mut rewriter = SyntaxRewriter::default();
-            shorten_paths(&mut rewriter, syntax.clone(), path);
+            shorten_paths(&mut rewriter, syntax.clone(), &path);
             let rewritten_syntax = rewriter.rewrite(&syntax);
             if let Some(ref import_scope) = ImportScope::from(rewritten_syntax) {
-                let new_syntax = insert_use(
+                let new_syntax = insert_use(import_scope, path, ctx.config.insert_use.merge);
-                    import_scope,
-                    make::path_from_text(path_to_import),
-                    ctx.config.insert_use.merge,
-                );
                 builder.replace(syntax.text_range(), new_syntax.to_string())
             }
         },
@@ -69,7 +55,7 @@ pub(crate) fn replace_qualified_name_with_use(
 }
 
 /// Adds replacements to `re` that shorten `path` in all descendants of `node`.
-fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) {
+fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ast::Path) {
     for child in node.children() {
         match_ast! {
             match child {
@@ -82,10 +68,10 @@ fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path:
                 ast::Path(p) => {
                     match maybe_replace_path(rewriter, p.clone(), path.clone()) {
                         Some(()) => {},
-                        None => shorten_paths(rewriter, p.syntax().clone(), path.clone()),
+                        None => shorten_paths(rewriter, p.syntax().clone(), path),
                     }
                 },
-                _ => shorten_paths(rewriter, child, path.clone()),
+                _ => shorten_paths(rewriter, child, path),
             }
         }
     }

crates/hir_def/src/path.rs

@@ -13,7 +13,7 @@ use hir_expand::{
     hygiene::Hygiene,
     name::{AsName, Name},
 };
-use syntax::ast;
+use syntax::ast::{self, make};
 
 use crate::{
     type_ref::{TypeBound, TypeRef},
@@ -100,6 +100,26 @@ impl ModPath {
         }
         self.segments.first()
     }
+
+    pub fn to_ast_path(&self) -> ast::Path {
+        let mut segments = Vec::new();
+        let mut is_abs = false;
+        match self.kind {
+            PathKind::Plain => {}
+            PathKind::Super(0) => segments.push(make::path_segment_self()),
+            PathKind::Super(n) => segments.extend((0..n).map(|_| make::path_segment_super())),
+            PathKind::Crate => segments.push(make::path_segment_crate()),
+            PathKind::Abs => is_abs = true,
+            PathKind::DollarCrate(_) => (),
+        }
+
+        segments.extend(
+            self.segments
+                .iter()
+                .map(|segment| make::path_segment(make::name_ref(&segment.to_string()))),
+        );
+        make::path_from_segments(segments, is_abs)
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -286,10 +306,8 @@ impl Display for ModPath {
         };
         match self.kind {
             PathKind::Plain => {}
+            PathKind::Super(0) => add_segment("self")?,
             PathKind::Super(n) => {
-                if n == 0 {
-                    add_segment("self")?;
-                }
                 for _ in 0..n {
                     add_segment("super")?;
                 }

crates/syntax/src/ast/make.rs

@@ -28,18 +28,41 @@ pub fn assoc_item_list() -> ast::AssocItemList {
 pub fn path_segment(name_ref: ast::NameRef) -> ast::PathSegment {
     ast_from_text(&format!("use {};", name_ref))
 }
+
 pub fn path_segment_self() -> ast::PathSegment {
     ast_from_text("use self;")
 }
+
+pub fn path_segment_super() -> ast::PathSegment {
+    ast_from_text("use super;")
+}
+
+pub fn path_segment_crate() -> ast::PathSegment {
+    ast_from_text("use crate;")
+}
+
 pub fn path_unqualified(segment: ast::PathSegment) -> ast::Path {
-    path_from_text(&format!("use {}", segment))
+    ast_from_text(&format!("use {}", segment))
 }
+
 pub fn path_qualified(qual: ast::Path, segment: ast::PathSegment) -> ast::Path {
-    path_from_text(&format!("{}::{}", qual, segment))
+    ast_from_text(&format!("{}::{}", qual, segment))
 }
-// FIXME: make this private
+
-pub fn path_from_text(text: &str) -> ast::Path {
+pub fn path_concat(first: ast::Path, second: ast::Path) -> ast::Path {
-    ast_from_text(text)
+    ast_from_text(&format!("{}::{}", first, second))
+}
+
+pub fn path_from_segments(
+    segments: impl IntoIterator<Item = ast::PathSegment>,
+    is_abs: bool,
+) -> ast::Path {
+    let segments = segments.into_iter().map(|it| it.syntax().clone()).join("::");
+    ast_from_text(&if is_abs {
+        format!("use ::{};", segments)
+    } else {
+        format!("use {};", segments)
+    })
 }
 
 pub fn glob_use_tree() -> ast::UseTree {