diff options
author | Aleksey Kladov <[email protected]> | 2020-08-15 17:50:41 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2020-08-15 17:50:41 +0100 |
commit | 0ca1ba29e8e88c060dcf36946e4e02a6f015754b (patch) | |
tree | 7b652995dda3849f9deb13c8eac6ad72a39cb283 /crates/assists | |
parent | 2052d33b9b0e2254f53848501a9113aa12ddf4da (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
Diffstat (limited to 'crates/assists')
5 files changed, 37 insertions, 47 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 @@ | |||
1 | use either::Either; | ||
1 | use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; | 2 | use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope}; |
2 | use ide_db::{ | 3 | use 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 | ||
13 | use 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; | |||
40 | pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { | 39 | pub(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 { | |||
82 | fn find_defs_in_mod( | 85 | fn 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 @@ | |||
1 | use hir; | 1 | use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange}; |
2 | use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode}; | 2 | use test_utils::mark; |
3 | 3 | ||
4 | use crate::{ | 4 | use 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 | ||
61 | fn 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`. |
82 | fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) { | 73 | fn 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 @@ | |||
5 | use std::iter::successors; | 5 | use std::iter::successors; |
6 | 6 | ||
7 | use either::Either; | 7 | use either::Either; |
8 | use hir::{self, ModPath}; | ||
9 | use syntax::{ | 8 | use 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( | |||
35 | pub(crate) fn insert_use_statement( | 34 | pub(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 { |