Merge #5769

5769: Don't expose hir::Path out of hir
 r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

9 files changed, 51 insertions, 92 deletions

diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index cce789972..b9ec3f10b 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -53,7 +53,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             |builder| {
                 insert_use_statement(
                     &auto_import_assets.syntax_under_caret,
-                    &import,
+                    &import.to_string(),
                     ctx,
                     builder.text_edit_builder(),
                 );
diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs
index f690ec343..81d0af2f3 100644
--- a/crates/assists/src/handlers/expand_glob_import.rs
+++ b/crates/assists/src/handlers/expand_glob_import.rs
@@ -1,3 +1,4 @@
+use either::Either;
 use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope};
 use ide_db::{
     defs::{classify_name_ref, Definition, NameRefClass},
@@ -10,8 +11,6 @@ use crate::{
     AssistId, AssistKind,
 };
 
-use either::Either;
-
 // Assist: expand_glob_import
 //
 // Expands glob imports.
@@ -40,11 +39,15 @@ use either::Either;
 pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let star = ctx.find_token_at_offset(T![*])?;
     let mod_path = find_mod_path(&star)?;
+    let module = match ctx.sema.resolve_path(&mod_path)? {
+        PathResolution::Def(ModuleDef::Module(it)) => it,
+        _ => return None,
+    };
 
     let source_file = ctx.source_file();
     let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset());
 
-    let defs_in_mod = find_defs_in_mod(ctx, scope, &mod_path)?;
+    let defs_in_mod = find_defs_in_mod(ctx, scope, module)?;
     let name_refs_in_source_file =
         source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect();
     let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file);
@@ -82,17 +85,8 @@ impl Def {
 fn find_defs_in_mod(
     ctx: &AssistContext,
     from: SemanticsScope<'_>,
-    path: &ast::Path,
+    module: hir::Module,
 ) -> Option<Vec<Def>> {
-    let hir_path = ctx.sema.lower_path(&path)?;
-    let module = if let Some(PathResolution::Def(ModuleDef::Module(module))) =
-        from.resolve_hir_path_qualifier(&hir_path)
-    {
-        module
-    } else {
-        return None;
-    };
-
     let module_scope = module.scope(ctx.db(), from.module());
 
     let mut defs = vec![];
diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
index 4bcdae7ba..d62e06b4a 100644
--- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -106,7 +106,12 @@ fn insert_import(
     if let Some(mut mod_path) = mod_path {
         mod_path.segments.pop();
         mod_path.segments.push(variant_hir_name.clone());
-        insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder());
+        insert_use_statement(
+            path.syntax(),
+            &mod_path.to_string(),
+            ctx,
+            builder.text_edit_builder(),
+        );
     }
     Some(())
 }
diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
index 011bf1106..470e5f8ff 100644
--- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
@@ -1,5 +1,5 @@
-use hir;
-use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode};
+use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange};
+use test_utils::mark;
 
 use crate::{
     utils::{find_insert_use_container, insert_use_statement},
@@ -28,12 +28,19 @@ pub(crate) fn replace_qualified_name_with_use(
     if path.syntax().ancestors().find_map(ast::Use::cast).is_some() {
         return None;
     }
-
-    let hir_path = ctx.sema.lower_path(&path)?;
-    let segments = collect_hir_path_segments(&hir_path)?;
-    if segments.len() < 2 {
+    if path.qualifier().is_none() {
+        mark::hit!(dont_import_trivial_paths);
         return None;
     }
+    let path_to_import = path.to_string().clone();
+    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();
     acc.add(
@@ -41,12 +48,16 @@ pub(crate) fn replace_qualified_name_with_use(
         "Replace qualified path with use",
         target,
         |builder| {
-            let path_to_import = hir_path.mod_path().clone();
             let container = match find_insert_use_container(path.syntax(), ctx) {
                 Some(c) => c,
                 None => return,
             };
-            insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder());
+            insert_use_statement(
+                path.syntax(),
+                &path_to_import.to_string(),
+                ctx,
+                builder.text_edit_builder(),
+            );
 
             // 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).
@@ -58,26 +69,6 @@ pub(crate) fn replace_qualified_name_with_use(
     )
 }
 
-fn collect_hir_path_segments(path: &hir::Path) -> Option<Vec<SmolStr>> {
-    let mut ps = Vec::<SmolStr>::with_capacity(10);
-    match path.kind() {
-        hir::PathKind::Abs => ps.push("".into()),
-        hir::PathKind::Crate => ps.push("crate".into()),
-        hir::PathKind::Plain => {}
-        hir::PathKind::Super(0) => ps.push("self".into()),
-        hir::PathKind::Super(lvl) => {
-            let mut chain = "super".to_string();
-            for _ in 0..*lvl {
-                chain += "::super";
-            }
-            ps.push(chain.into());
-        }
-        hir::PathKind::DollarCrate(_) => return None,
-    }
-    ps.extend(path.segments().iter().map(|it| it.name.to_string().into()));
-    Some(ps)
-}
-
 /// Adds replacements to `re` that shorten `path` in all descendants of `node`.
 fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) {
     for child in node.children() {
@@ -467,7 +458,8 @@ impl Debug for Foo {
     }
 
     #[test]
-    fn test_replace_not_applicable_one_segment() {
+    fn dont_import_trivial_paths() {
+        mark::check!(dont_import_trivial_paths);
         check_assist_not_applicable(
             replace_qualified_name_with_use,
             r"
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index 50a62ee82..49096a67c 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -5,7 +5,6 @@
 use std::iter::successors;
 
 use either::Either;
-use hir::{self, ModPath};
 use syntax::{
     ast::{self, NameOwner, VisibilityOwner},
     AstNode, AstToken, Direction, SmolStr,
@@ -35,11 +34,11 @@ pub(crate) fn find_insert_use_container(
 pub(crate) fn insert_use_statement(
     // Ideally the position of the cursor, used to
     position: &SyntaxNode,
-    path_to_import: &ModPath,
+    path_to_import: &str,
     ctx: &AssistContext,
     builder: &mut TextEditBuilder,
 ) {
-    let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::<Vec<_>>();
+    let target = path_to_import.split("::").map(SmolStr::new).collect::<Vec<_>>();
     let container = find_insert_use_container(position, ctx);
 
     if let Some(container) = container {
diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs
index 5dc3ae3b1..c442654dd 100644
--- a/crates/hir/src/code_model.rs
+++ b/crates/hir/src/code_model.rs
@@ -12,6 +12,7 @@ use hir_def::{
     docs::Documentation,
     expr::{BindingAnnotation, Pat, PatId},
     import_map,
+    path::ModPath,
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
     src::HasSource as _,
@@ -344,11 +345,7 @@ impl Module {
 
     /// Finds a path that can be used to refer to the given item from within
     /// this module, if possible.
-    pub fn find_use_path(
-        self,
-        db: &dyn DefDatabase,
-        item: impl Into<ItemInNs>,
-    ) -> Option<hir_def::path::ModPath> {
+    pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
         hir_def::find_path::find_path(db, item.into(), self.into())
     }
 }
@@ -1126,7 +1123,7 @@ impl ImplDef {
             .value
             .attrs()
             .filter_map(|it| {
-                let path = hir_def::path::ModPath::from_src(it.path()?, &hygenic)?;
+                let path = ModPath::from_src(it.path()?, &hygenic)?;
                 if path.as_ident()?.to_string() == "derive" {
                     Some(it)
                 } else {
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 4ae2bd085..8961ba8fd 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -48,7 +48,7 @@ pub use hir_def::{
     builtin_type::BuiltinType,
     docs::Documentation,
     nameres::ModuleSource,
-    path::{ModPath, Path, PathKind},
+    path::ModPath,
     type_ref::{Mutability, TypeRef},
 };
 pub use hir_expand::{
@@ -60,4 +60,7 @@ pub use hir_ty::display::HirDisplay;
 // These are negative re-exports: pub using these names is forbidden, they
 // should remain private to hir internals.
 #[allow(unused)]
-use hir_expand::hygiene::Hygiene;
+use {
+    hir_def::path::{Path, PathKind},
+    hir_expand::hygiene::Hygiene,
+};
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 3953017c3..c693176fa 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -6,7 +6,7 @@ use std::{cell::RefCell, fmt, iter::successors};
 
 use base_db::{FileId, FileRange};
 use hir_def::{
-    resolver::{self, HasResolver, Resolver},
+    resolver::{self, HasResolver, Resolver, TypeNs},
     AsMacroCall, FunctionId, TraitId, VariantId,
 };
 use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo};
@@ -22,12 +22,11 @@ use crate::{
     db::HirDatabase,
     diagnostics::Diagnostic,
     semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
-    source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer},
+    source_analyzer::{resolve_hir_path, SourceAnalyzer},
     AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef,
     Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef,
     VariantDef,
 };
-use resolver::TypeNs;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PathResolution {
@@ -228,10 +227,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
         self.imp.resolve_variant(record_lit).map(VariantDef::from)
     }
 
-    pub fn lower_path(&self, path: &ast::Path) -> Option<Path> {
-        self.imp.lower_path(path)
-    }
-
     pub fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
         self.imp.resolve_bind_pat_to_const(pat)
     }
@@ -467,11 +462,6 @@ impl<'db> SemanticsImpl<'db> {
         self.analyze(record_lit.syntax()).resolve_variant(self.db, record_lit)
     }
 
-    fn lower_path(&self, path: &ast::Path) -> Option<Path> {
-        let src = self.find_file(path.syntax().clone());
-        Path::from_src(path.clone(), &Hygiene::new(self.db.upcast(), src.file_id.into()))
-    }
-
     fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
         self.analyze(pat.syntax()).resolve_bind_pat_to_const(self.db, pat)
     }
@@ -758,28 +748,7 @@ impl<'a> SemanticsScope<'a> {
     pub fn speculative_resolve(&self, path: &ast::Path) -> Option<PathResolution> {
         let hygiene = Hygiene::new(self.db.upcast(), self.file_id);
         let path = Path::from_src(path.clone(), &hygiene)?;
-        self.resolve_hir_path(&path)
-    }
-
-    pub fn resolve_hir_path(&self, path: &Path) -> Option<PathResolution> {
-        resolve_hir_path(self.db, &self.resolver, path)
-    }
-
-    /// Resolves a path where we know it is a qualifier of another path.
-    ///
-    /// For example, if we have:
-    /// ```
-    /// mod my {
-    ///     pub mod foo {
-    ///         struct Bar;
-    ///     }
-    ///
-    ///     pub fn foo() {}
-    /// }
-    /// ```
-    /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
-    pub fn resolve_hir_path_qualifier(&self, path: &Path) -> Option<PathResolution> {
-        resolve_hir_path_qualifier(self.db, &self.resolver, path)
+        resolve_hir_path(self.db, &self.resolver, &path)
     }
 }
 
diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs
index 8750584f9..1d13c4f1d 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -13,6 +13,7 @@ use hir_def::{
         Body, BodySourceMap,
     },
     expr::{ExprId, Pat, PatId},
+    path::{ModPath, Path, PathKind},
     resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs},
     AsMacroCall, DefWithBodyId, FieldId, FunctionId, LocalFieldId, VariantId,
 };
@@ -28,8 +29,7 @@ use syntax::{
 
 use crate::{
     db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Field, Function, Local,
-    MacroDef, ModPath, ModuleDef, Path, PathKind, Static, Struct, Trait, Type, TypeAlias,
-    TypeParam,
+    MacroDef, ModuleDef, Static, Struct, Trait, Type, TypeAlias, TypeParam,
 };
 use base_db::CrateId;
 
@@ -508,7 +508,7 @@ pub(crate) fn resolve_hir_path(
 /// }
 /// ```
 /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
-pub(crate) fn resolve_hir_path_qualifier(
+fn resolve_hir_path_qualifier(
     db: &dyn HirDatabase,
     resolver: &Resolver,
     path: &Path,