aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksey Kladov <[email protected]>2020-08-15 17:50:41 +0100
committerAleksey Kladov <[email protected]>2020-08-15 17:50:41 +0100
commit0ca1ba29e8e88c060dcf36946e4e02a6f015754b (patch)
tree7b652995dda3849f9deb13c8eac6ad72a39cb283
parent2052d33b9b0e2254f53848501a9113aa12ddf4da (diff)
Don't expose hir::Path out of hir
Conjecture: it's impossible to use hir::Path *correctly* from an IDE. I am not entirely sure about this, and we might need to add it back at some point, but I have to arguments that convince me that we probably won't: * `hir::Path` has to know about hygiene, which an IDE can't set up properly. * `hir::Path` lacks identity, but you actually have to know identity to resolve it correctly
-rw-r--r--crates/assists/src/handlers/auto_import.rs2
-rw-r--r--crates/assists/src/handlers/expand_glob_import.rs20
-rw-r--r--crates/assists/src/handlers/extract_struct_from_enum_variant.rs7
-rw-r--r--crates/assists/src/handlers/replace_qualified_name_with_use.rs50
-rw-r--r--crates/assists/src/utils/insert_use.rs5
-rw-r--r--crates/hir/src/code_model.rs9
-rw-r--r--crates/hir/src/lib.rs7
-rw-r--r--crates/hir/src/semantics.rs37
-rw-r--r--crates/hir/src/source_analyzer.rs6
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<()>
53 |builder| { 53 |builder| {
54 insert_use_statement( 54 insert_use_statement(
55 &auto_import_assets.syntax_under_caret, 55 &auto_import_assets.syntax_under_caret,
56 &import, 56 &import.to_string(),
57 ctx, 57 ctx,
58 builder.text_edit_builder(), 58 builder.text_edit_builder(),
59 ); 59 );
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 @@
1use either::Either;
1use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; 2use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope};
2use ide_db::{ 3use ide_db::{
3 defs::{classify_name_ref, Definition, NameRefClass}, 4 defs::{classify_name_ref, Definition, NameRefClass},
@@ -10,8 +11,6 @@ use crate::{
10 AssistId, AssistKind, 11 AssistId, AssistKind,
11}; 12};
12 13
13use either::Either;
14
15// Assist: expand_glob_import 14// Assist: expand_glob_import
16// 15//
17// Expands glob imports. 16// Expands glob imports.
@@ -40,11 +39,15 @@ use either::Either;
40pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { 39pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
41 let star = ctx.find_token_at_offset(T![*])?; 40 let star = ctx.find_token_at_offset(T![*])?;
42 let mod_path = find_mod_path(&star)?; 41 let mod_path = find_mod_path(&star)?;
42 let module = match ctx.sema.resolve_path(&mod_path)? {
43 PathResolution::Def(ModuleDef::Module(it)) => it,
44 _ => return None,
45 };
43 46
44 let source_file = ctx.source_file(); 47 let source_file = ctx.source_file();
45 let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset()); 48 let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset());
46 49
47 let defs_in_mod = find_defs_in_mod(ctx, scope, &mod_path)?; 50 let defs_in_mod = find_defs_in_mod(ctx, scope, module)?;
48 let name_refs_in_source_file = 51 let name_refs_in_source_file =
49 source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect(); 52 source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect();
50 let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file); 53 let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file);
@@ -82,17 +85,8 @@ impl Def {
82fn find_defs_in_mod( 85fn find_defs_in_mod(
83 ctx: &AssistContext, 86 ctx: &AssistContext,
84 from: SemanticsScope<'_>, 87 from: SemanticsScope<'_>,
85 path: &ast::Path, 88 module: hir::Module,
86) -> Option<Vec<Def>> { 89) -> Option<Vec<Def>> {
87 let hir_path = ctx.sema.lower_path(&path)?;
88 let module = if let Some(PathResolution::Def(ModuleDef::Module(module))) =
89 from.resolve_hir_path_qualifier(&hir_path)
90 {
91 module
92 } else {
93 return None;
94 };
95
96 let module_scope = module.scope(ctx.db(), from.module()); 90 let module_scope = module.scope(ctx.db(), from.module());
97 91
98 let mut defs = vec![]; 92 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(
106 if let Some(mut mod_path) = mod_path { 106 if let Some(mut mod_path) = mod_path {
107 mod_path.segments.pop(); 107 mod_path.segments.pop();
108 mod_path.segments.push(variant_hir_name.clone()); 108 mod_path.segments.push(variant_hir_name.clone());
109 insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder()); 109 insert_use_statement(
110 path.syntax(),
111 &mod_path.to_string(),
112 ctx,
113 builder.text_edit_builder(),
114 );
110 } 115 }
111 Some(()) 116 Some(())
112} 117}
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 @@
1use hir; 1use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange};
2use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode}; 2use test_utils::mark;
3 3
4use crate::{ 4use crate::{
5 utils::{find_insert_use_container, insert_use_statement}, 5 utils::{find_insert_use_container, insert_use_statement},
@@ -28,12 +28,19 @@ pub(crate) fn replace_qualified_name_with_use(
28 if path.syntax().ancestors().find_map(ast::Use::cast).is_some() { 28 if path.syntax().ancestors().find_map(ast::Use::cast).is_some() {
29 return None; 29 return None;
30 } 30 }
31 31 if path.qualifier().is_none() {
32 let hir_path = ctx.sema.lower_path(&path)?; 32 mark::hit!(dont_import_trivial_paths);
33 let segments = collect_hir_path_segments(&hir_path)?;
34 if segments.len() < 2 {
35 return None; 33 return None;
36 } 34 }
35 let path_to_import = path.to_string().clone();
36 let path_to_import = match path.segment()?.generic_arg_list() {
37 Some(generic_args) => {
38 let generic_args_start =
39 generic_args.syntax().text_range().start() - path.syntax().text_range().start();
40 &path_to_import[TextRange::up_to(generic_args_start)]
41 }
42 None => path_to_import.as_str(),
43 };
37 44
38 let target = path.syntax().text_range(); 45 let target = path.syntax().text_range();
39 acc.add( 46 acc.add(
@@ -41,12 +48,16 @@ pub(crate) fn replace_qualified_name_with_use(
41 "Replace qualified path with use", 48 "Replace qualified path with use",
42 target, 49 target,
43 |builder| { 50 |builder| {
44 let path_to_import = hir_path.mod_path().clone();
45 let container = match find_insert_use_container(path.syntax(), ctx) { 51 let container = match find_insert_use_container(path.syntax(), ctx) {
46 Some(c) => c, 52 Some(c) => c,
47 None => return, 53 None => return,
48 }; 54 };
49 insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder()); 55 insert_use_statement(
56 path.syntax(),
57 &path_to_import.to_string(),
58 ctx,
59 builder.text_edit_builder(),
60 );
50 61
51 // Now that we've brought the name into scope, re-qualify all paths that could be 62 // Now that we've brought the name into scope, re-qualify all paths that could be
52 // affected (that is, all paths inside the node we added the `use` to). 63 // 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(
58 ) 69 )
59} 70}
60 71
61fn collect_hir_path_segments(path: &hir::Path) -> Option<Vec<SmolStr>> {
62 let mut ps = Vec::<SmolStr>::with_capacity(10);
63 match path.kind() {
64 hir::PathKind::Abs => ps.push("".into()),
65 hir::PathKind::Crate => ps.push("crate".into()),
66 hir::PathKind::Plain => {}
67 hir::PathKind::Super(0) => ps.push("self".into()),
68 hir::PathKind::Super(lvl) => {
69 let mut chain = "super".to_string();
70 for _ in 0..*lvl {
71 chain += "::super";
72 }
73 ps.push(chain.into());
74 }
75 hir::PathKind::DollarCrate(_) => return None,
76 }
77 ps.extend(path.segments().iter().map(|it| it.name.to_string().into()));
78 Some(ps)
79}
80
81/// Adds replacements to `re` that shorten `path` in all descendants of `node`. 72/// Adds replacements to `re` that shorten `path` in all descendants of `node`.
82fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) { 73fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) {
83 for child in node.children() { 74 for child in node.children() {
@@ -467,7 +458,8 @@ impl Debug for Foo {
467 } 458 }
468 459
469 #[test] 460 #[test]
470 fn test_replace_not_applicable_one_segment() { 461 fn dont_import_trivial_paths() {
462 mark::check!(dont_import_trivial_paths);
471 check_assist_not_applicable( 463 check_assist_not_applicable(
472 replace_qualified_name_with_use, 464 replace_qualified_name_with_use,
473 r" 465 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 @@
5use std::iter::successors; 5use std::iter::successors;
6 6
7use either::Either; 7use either::Either;
8use hir::{self, ModPath};
9use syntax::{ 8use syntax::{
10 ast::{self, NameOwner, VisibilityOwner}, 9 ast::{self, NameOwner, VisibilityOwner},
11 AstNode, AstToken, Direction, SmolStr, 10 AstNode, AstToken, Direction, SmolStr,
@@ -35,11 +34,11 @@ pub(crate) fn find_insert_use_container(
35pub(crate) fn insert_use_statement( 34pub(crate) fn insert_use_statement(
36 // Ideally the position of the cursor, used to 35 // Ideally the position of the cursor, used to
37 position: &SyntaxNode, 36 position: &SyntaxNode,
38 path_to_import: &ModPath, 37 path_to_import: &str,
39 ctx: &AssistContext, 38 ctx: &AssistContext,
40 builder: &mut TextEditBuilder, 39 builder: &mut TextEditBuilder,
41) { 40) {
42 let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::<Vec<_>>(); 41 let target = path_to_import.split("::").map(SmolStr::new).collect::<Vec<_>>();
43 let container = find_insert_use_container(position, ctx); 42 let container = find_insert_use_container(position, ctx);
44 43
45 if let Some(container) = container { 44 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::{
12 docs::Documentation, 12 docs::Documentation,
13 expr::{BindingAnnotation, Pat, PatId}, 13 expr::{BindingAnnotation, Pat, PatId},
14 import_map, 14 import_map,
15 path::ModPath,
15 per_ns::PerNs, 16 per_ns::PerNs,
16 resolver::{HasResolver, Resolver}, 17 resolver::{HasResolver, Resolver},
17 src::HasSource as _, 18 src::HasSource as _,
@@ -344,11 +345,7 @@ impl Module {
344 345
345 /// Finds a path that can be used to refer to the given item from within 346 /// Finds a path that can be used to refer to the given item from within
346 /// this module, if possible. 347 /// this module, if possible.
347 pub fn find_use_path( 348 pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
348 self,
349 db: &dyn DefDatabase,
350 item: impl Into<ItemInNs>,
351 ) -> Option<hir_def::path::ModPath> {
352 hir_def::find_path::find_path(db, item.into(), self.into()) 349 hir_def::find_path::find_path(db, item.into(), self.into())
353 } 350 }
354} 351}
@@ -1126,7 +1123,7 @@ impl ImplDef {
1126 .value 1123 .value
1127 .attrs() 1124 .attrs()
1128 .filter_map(|it| { 1125 .filter_map(|it| {
1129 let path = hir_def::path::ModPath::from_src(it.path()?, &hygenic)?; 1126 let path = ModPath::from_src(it.path()?, &hygenic)?;
1130 if path.as_ident()?.to_string() == "derive" { 1127 if path.as_ident()?.to_string() == "derive" {
1131 Some(it) 1128 Some(it)
1132 } else { 1129 } 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::{
48 builtin_type::BuiltinType, 48 builtin_type::BuiltinType,
49 docs::Documentation, 49 docs::Documentation,
50 nameres::ModuleSource, 50 nameres::ModuleSource,
51 path::{ModPath, Path, PathKind}, 51 path::ModPath,
52 type_ref::{Mutability, TypeRef}, 52 type_ref::{Mutability, TypeRef},
53}; 53};
54pub use hir_expand::{ 54pub use hir_expand::{
@@ -60,4 +60,7 @@ pub use hir_ty::display::HirDisplay;
60// These are negative re-exports: pub using these names is forbidden, they 60// These are negative re-exports: pub using these names is forbidden, they
61// should remain private to hir internals. 61// should remain private to hir internals.
62#[allow(unused)] 62#[allow(unused)]
63use hir_expand::hygiene::Hygiene; 63use {
64 hir_def::path::{Path, PathKind},
65 hir_expand::hygiene::Hygiene,
66};
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};
6 6
7use base_db::{FileId, FileRange}; 7use base_db::{FileId, FileRange};
8use hir_def::{ 8use hir_def::{
9 resolver::{self, HasResolver, Resolver}, 9 resolver::{self, HasResolver, Resolver, TypeNs},
10 AsMacroCall, FunctionId, TraitId, VariantId, 10 AsMacroCall, FunctionId, TraitId, VariantId,
11}; 11};
12use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo}; 12use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo};
@@ -22,12 +22,11 @@ use crate::{
22 db::HirDatabase, 22 db::HirDatabase,
23 diagnostics::Diagnostic, 23 diagnostics::Diagnostic,
24 semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, 24 semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
25 source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer}, 25 source_analyzer::{resolve_hir_path, SourceAnalyzer},
26 AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef, 26 AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef,
27 Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef, 27 Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef,
28 VariantDef, 28 VariantDef,
29}; 29};
30use resolver::TypeNs;
31 30
32#[derive(Debug, Clone, PartialEq, Eq)] 31#[derive(Debug, Clone, PartialEq, Eq)]
33pub enum PathResolution { 32pub enum PathResolution {
@@ -228,10 +227,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
228 self.imp.resolve_variant(record_lit).map(VariantDef::from) 227 self.imp.resolve_variant(record_lit).map(VariantDef::from)
229 } 228 }
230 229
231 pub fn lower_path(&self, path: &ast::Path) -> Option<Path> {
232 self.imp.lower_path(path)
233 }
234
235 pub fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> { 230 pub fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
236 self.imp.resolve_bind_pat_to_const(pat) 231 self.imp.resolve_bind_pat_to_const(pat)
237 } 232 }
@@ -467,11 +462,6 @@ impl<'db> SemanticsImpl<'db> {
467 self.analyze(record_lit.syntax()).resolve_variant(self.db, record_lit) 462 self.analyze(record_lit.syntax()).resolve_variant(self.db, record_lit)
468 } 463 }
469 464
470 fn lower_path(&self, path: &ast::Path) -> Option<Path> {
471 let src = self.find_file(path.syntax().clone());
472 Path::from_src(path.clone(), &Hygiene::new(self.db.upcast(), src.file_id.into()))
473 }
474
475 fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> { 465 fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
476 self.analyze(pat.syntax()).resolve_bind_pat_to_const(self.db, pat) 466 self.analyze(pat.syntax()).resolve_bind_pat_to_const(self.db, pat)
477 } 467 }
@@ -758,28 +748,7 @@ impl<'a> SemanticsScope<'a> {
758 pub fn speculative_resolve(&self, path: &ast::Path) -> Option<PathResolution> { 748 pub fn speculative_resolve(&self, path: &ast::Path) -> Option<PathResolution> {
759 let hygiene = Hygiene::new(self.db.upcast(), self.file_id); 749 let hygiene = Hygiene::new(self.db.upcast(), self.file_id);
760 let path = Path::from_src(path.clone(), &hygiene)?; 750 let path = Path::from_src(path.clone(), &hygiene)?;
761 self.resolve_hir_path(&path) 751 resolve_hir_path(self.db, &self.resolver, &path)
762 }
763
764 pub fn resolve_hir_path(&self, path: &Path) -> Option<PathResolution> {
765 resolve_hir_path(self.db, &self.resolver, path)
766 }
767
768 /// Resolves a path where we know it is a qualifier of another path.
769 ///
770 /// For example, if we have:
771 /// ```
772 /// mod my {
773 /// pub mod foo {
774 /// struct Bar;
775 /// }
776 ///
777 /// pub fn foo() {}
778 /// }
779 /// ```
780 /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
781 pub fn resolve_hir_path_qualifier(&self, path: &Path) -> Option<PathResolution> {
782 resolve_hir_path_qualifier(self.db, &self.resolver, path)
783 } 752 }
784} 753}
785 754
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::{
13 Body, BodySourceMap, 13 Body, BodySourceMap,
14 }, 14 },
15 expr::{ExprId, Pat, PatId}, 15 expr::{ExprId, Pat, PatId},
16 path::{ModPath, Path, PathKind},
16 resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs}, 17 resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs},
17 AsMacroCall, DefWithBodyId, FieldId, FunctionId, LocalFieldId, VariantId, 18 AsMacroCall, DefWithBodyId, FieldId, FunctionId, LocalFieldId, VariantId,
18}; 19};
@@ -28,8 +29,7 @@ use syntax::{
28 29
29use crate::{ 30use crate::{
30 db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Field, Function, Local, 31 db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Field, Function, Local,
31 MacroDef, ModPath, ModuleDef, Path, PathKind, Static, Struct, Trait, Type, TypeAlias, 32 MacroDef, ModuleDef, Static, Struct, Trait, Type, TypeAlias, TypeParam,
32 TypeParam,
33}; 33};
34use base_db::CrateId; 34use base_db::CrateId;
35 35
@@ -508,7 +508,7 @@ pub(crate) fn resolve_hir_path(
508/// } 508/// }
509/// ``` 509/// ```
510/// then we know that `foo` in `my::foo::Bar` refers to the module, not the function. 510/// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
511pub(crate) fn resolve_hir_path_qualifier( 511fn resolve_hir_path_qualifier(
512 db: &dyn HirDatabase, 512 db: &dyn HirDatabase,
513 resolver: &Resolver, 513 resolver: &Resolver,
514 path: &Path, 514 path: &Path,