aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukas Wirth <[email protected]>2020-09-25 14:19:22 +0100
committerLukas Wirth <[email protected]>2020-09-25 14:19:22 +0100
commite1d6981f90ba7852e949141dad0add855b57184a (patch)
treeb999ae8577aee299cb6d3592fcf8532f1b6abf75
parent9d3483a74dba6b0a338230fd003d91a0447c5398 (diff)
Don't unnecessarily unnest imports for import insertion
-rw-r--r--crates/assists/src/handlers/add_missing_impl_members.rs35
-rw-r--r--crates/assists/src/handlers/auto_import.rs33
-rw-r--r--crates/hir/src/code_model.rs10
-rw-r--r--crates/hir_def/src/find_path.rs170
4 files changed, 203 insertions, 45 deletions
diff --git a/crates/assists/src/handlers/add_missing_impl_members.rs b/crates/assists/src/handlers/add_missing_impl_members.rs
index 8df1d786b..1ac5fefd6 100644
--- a/crates/assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/assists/src/handlers/add_missing_impl_members.rs
@@ -415,6 +415,41 @@ impl foo::Foo for S {
415 } 415 }
416 416
417 #[test] 417 #[test]
418 fn test_qualify_path_2() {
419 check_assist(
420 add_missing_impl_members,
421 r#"
422mod foo {
423 pub mod bar {
424 pub struct Bar;
425 pub trait Foo { fn foo(&self, bar: Bar); }
426 }
427}
428
429use foo::bar;
430
431struct S;
432impl bar::Foo for S { <|> }"#,
433 r#"
434mod foo {
435 pub mod bar {
436 pub struct Bar;
437 pub trait Foo { fn foo(&self, bar: Bar); }
438 }
439}
440
441use foo::bar;
442
443struct S;
444impl bar::Foo for S {
445 fn foo(&self, bar: bar::Bar) {
446 ${0:todo!()}
447 }
448}"#,
449 );
450 }
451
452 #[test]
418 fn test_qualify_path_generic() { 453 fn test_qualify_path_generic() {
419 check_assist( 454 check_assist(
420 add_missing_impl_members, 455 add_missing_impl_members,
diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index b5eb2c722..ee7277c04 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -196,10 +196,10 @@ impl AutoImportAssets {
196 }) 196 })
197 .filter_map(|candidate| match candidate { 197 .filter_map(|candidate| match candidate {
198 Either::Left(module_def) => { 198 Either::Left(module_def) => {
199 self.module_with_name_to_import.find_use_path(db, module_def) 199 self.module_with_name_to_import.find_use_path_prefixed(db, module_def)
200 } 200 }
201 Either::Right(macro_def) => { 201 Either::Right(macro_def) => {
202 self.module_with_name_to_import.find_use_path(db, macro_def) 202 self.module_with_name_to_import.find_use_path_prefixed(db, macro_def)
203 } 203 }
204 }) 204 })
205 .filter(|use_path| !use_path.segments.is_empty()) 205 .filter(|use_path| !use_path.segments.is_empty())
@@ -291,6 +291,35 @@ mod tests {
291 use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; 291 use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
292 292
293 #[test] 293 #[test]
294 fn applicable_when_found_an_import_partial() {
295 check_assist(
296 auto_import,
297 r"
298 mod std {
299 pub mod fmt {
300 pub struct Formatter;
301 }
302 }
303
304 use std::fmt;
305
306 <|>Formatter
307 ",
308 r"
309 mod std {
310 pub mod fmt {
311 pub struct Formatter;
312 }
313 }
314
315 use std::fmt::{self, Formatter};
316
317 Formatter
318 ",
319 );
320 }
321
322 #[test]
294 fn applicable_when_found_an_import() { 323 fn applicable_when_found_an_import() {
295 check_assist( 324 check_assist(
296 auto_import, 325 auto_import,
diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs
index a2a166e0a..567fd91af 100644
--- a/crates/hir/src/code_model.rs
+++ b/crates/hir/src/code_model.rs
@@ -383,6 +383,16 @@ impl Module {
383 pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> { 383 pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
384 hir_def::find_path::find_path(db, item.into(), self.into()) 384 hir_def::find_path::find_path(db, item.into(), self.into())
385 } 385 }
386
387 /// Finds a path that can be used to refer to the given item from within
388 /// this module, if possible. This is used for returning import paths for use-statements.
389 pub fn find_use_path_prefixed(
390 self,
391 db: &dyn DefDatabase,
392 item: impl Into<ItemInNs>,
393 ) -> Option<ModPath> {
394 hir_def::find_path::find_path_prefixed(db, item.into(), self.into())
395 }
386} 396}
387 397
388#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] 398#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
diff --git a/crates/hir_def/src/find_path.rs b/crates/hir_def/src/find_path.rs
index ac2c54ac5..baf374144 100644
--- a/crates/hir_def/src/find_path.rs
+++ b/crates/hir_def/src/find_path.rs
@@ -4,6 +4,7 @@ use hir_expand::name::{known, AsName, Name};
4use rustc_hash::FxHashSet; 4use rustc_hash::FxHashSet;
5use test_utils::mark; 5use test_utils::mark;
6 6
7use crate::nameres::CrateDefMap;
7use crate::{ 8use crate::{
8 db::DefDatabase, 9 db::DefDatabase,
9 item_scope::ItemInNs, 10 item_scope::ItemInNs,
@@ -18,7 +19,12 @@ use crate::{
18/// *from where* you're referring to the item, hence the `from` parameter. 19/// *from where* you're referring to the item, hence the `from` parameter.
19pub fn find_path(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> { 20pub fn find_path(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
20 let _p = profile::span("find_path"); 21 let _p = profile::span("find_path");
21 find_path_inner(db, item, from, MAX_PATH_LEN) 22 find_path_inner(db, item, from, MAX_PATH_LEN, Prefixed::Not)
23}
24
25pub fn find_path_prefixed(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
26 let _p = profile::span("find_path_absolute");
27 find_path_inner(db, item, from, MAX_PATH_LEN, Prefixed::Plain)
22} 28}
23 29
24const MAX_PATH_LEN: usize = 15; 30const MAX_PATH_LEN: usize = 15;
@@ -36,11 +42,67 @@ impl ModPath {
36 } 42 }
37} 43}
38 44
45fn check_crate_self_super(
46 def_map: &CrateDefMap,
47 item: ItemInNs,
48 from: ModuleId,
49) -> Option<ModPath> {
50 // - if the item is the crate root, return `crate`
51 if item
52 == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
53 krate: from.krate,
54 local_id: def_map.root,
55 }))
56 {
57 Some(ModPath::from_segments(PathKind::Crate, Vec::new()))
58 } else if item == ItemInNs::Types(from.into()) {
59 // - if the item is the module we're in, use `self`
60 Some(ModPath::from_segments(PathKind::Super(0), Vec::new()))
61 } else {
62 if let Some(parent_id) = def_map.modules[from.local_id].parent {
63 // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
64 if item
65 == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
66 krate: from.krate,
67 local_id: parent_id,
68 }))
69 {
70 return Some(ModPath::from_segments(PathKind::Super(1), Vec::new()));
71 }
72 }
73 None
74 }
75}
76
77#[derive(Copy, Clone, PartialEq, Eq)]
78pub enum Prefixed {
79 Not,
80 BySelf,
81 Plain,
82}
83
84impl Prefixed {
85 #[inline]
86 fn prefix(self) -> Option<PathKind> {
87 match self {
88 Prefixed::Not => None,
89 Prefixed::BySelf => Some(PathKind::Super(0)),
90 Prefixed::Plain => Some(PathKind::Plain),
91 }
92 }
93
94 #[inline]
95 fn prefixed(self) -> bool {
96 self != Prefixed::Not
97 }
98}
99
39fn find_path_inner( 100fn find_path_inner(
40 db: &dyn DefDatabase, 101 db: &dyn DefDatabase,
41 item: ItemInNs, 102 item: ItemInNs,
42 from: ModuleId, 103 from: ModuleId,
43 max_len: usize, 104 max_len: usize,
105 prefixed: Prefixed,
44) -> Option<ModPath> { 106) -> Option<ModPath> {
45 if max_len == 0 { 107 if max_len == 0 {
46 return None; 108 return None;
@@ -51,41 +113,22 @@ fn find_path_inner(
51 // - if the item is already in scope, return the name under which it is 113 // - if the item is already in scope, return the name under which it is
52 let def_map = db.crate_def_map(from.krate); 114 let def_map = db.crate_def_map(from.krate);
53 let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope; 115 let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
54 if let Some((name, _)) = from_scope.name_of(item) { 116 let scope_name =
55 return Some(ModPath::from_segments(PathKind::Plain, vec![name.clone()])); 117 if let Some((name, _)) = from_scope.name_of(item) { Some(name.clone()) } else { None };
118 if !prefixed.prefixed() && scope_name.is_some() {
119 return scope_name
120 .map(|scope_name| ModPath::from_segments(PathKind::Plain, vec![scope_name]));
56 } 121 }
57 122
58 // - if the item is the crate root, return `crate` 123 if let modpath @ Some(_) = check_crate_self_super(&def_map, item, from) {
59 if item 124 return modpath;
60 == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
61 krate: from.krate,
62 local_id: def_map.root,
63 }))
64 {
65 return Some(ModPath::from_segments(PathKind::Crate, Vec::new()));
66 }
67
68 // - if the item is the module we're in, use `self`
69 if item == ItemInNs::Types(from.into()) {
70 return Some(ModPath::from_segments(PathKind::Super(0), Vec::new()));
71 }
72
73 // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
74 if let Some(parent_id) = def_map.modules[from.local_id].parent {
75 if item
76 == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
77 krate: from.krate,
78 local_id: parent_id,
79 }))
80 {
81 return Some(ModPath::from_segments(PathKind::Super(1), Vec::new()));
82 }
83 } 125 }
84 126
85 // - if the item is the crate root of a dependency crate, return the name from the extern prelude 127 // - if the item is the crate root of a dependency crate, return the name from the extern prelude
86 for (name, def_id) in &def_map.extern_prelude { 128 for (name, def_id) in &def_map.extern_prelude {
87 if item == ItemInNs::Types(*def_id) { 129 if item == ItemInNs::Types(*def_id) {
88 return Some(ModPath::from_segments(PathKind::Plain, vec![name.clone()])); 130 let name = scope_name.unwrap_or_else(|| name.clone());
131 return Some(ModPath::from_segments(PathKind::Plain, vec![name]));
89 } 132 }
90 } 133 }
91 134
@@ -138,6 +181,7 @@ fn find_path_inner(
138 ItemInNs::Types(ModuleDefId::ModuleId(module_id)), 181 ItemInNs::Types(ModuleDefId::ModuleId(module_id)),
139 from, 182 from,
140 best_path_len - 1, 183 best_path_len - 1,
184 prefixed,
141 ) { 185 ) {
142 path.segments.push(name); 186 path.segments.push(name);
143 187
@@ -165,6 +209,7 @@ fn find_path_inner(
165 ItemInNs::Types(ModuleDefId::ModuleId(info.container)), 209 ItemInNs::Types(ModuleDefId::ModuleId(info.container)),
166 from, 210 from,
167 best_path_len - 1, 211 best_path_len - 1,
212 prefixed,
168 )?; 213 )?;
169 path.segments.push(info.path.segments.last().unwrap().clone()); 214 path.segments.push(info.path.segments.last().unwrap().clone());
170 Some(path) 215 Some(path)
@@ -181,7 +226,13 @@ fn find_path_inner(
181 } 226 }
182 } 227 }
183 228
184 best_path 229 if let Some(prefix) = prefixed.prefix() {
230 best_path.or_else(|| {
231 scope_name.map(|scope_name| ModPath::from_segments(prefix, vec![scope_name]))
232 })
233 } else {
234 best_path
235 }
185} 236}
186 237
187fn select_best_path(old_path: ModPath, new_path: ModPath, prefer_no_std: bool) -> ModPath { 238fn select_best_path(old_path: ModPath, new_path: ModPath, prefer_no_std: bool) -> ModPath {
@@ -304,7 +355,7 @@ mod tests {
304 /// `code` needs to contain a cursor marker; checks that `find_path` for the 355 /// `code` needs to contain a cursor marker; checks that `find_path` for the
305 /// item the `path` refers to returns that same path when called from the 356 /// item the `path` refers to returns that same path when called from the
306 /// module the cursor is in. 357 /// module the cursor is in.
307 fn check_found_path(ra_fixture: &str, path: &str) { 358 fn check_found_path_(ra_fixture: &str, path: &str, absolute: bool) {
308 let (db, pos) = TestDB::with_position(ra_fixture); 359 let (db, pos) = TestDB::with_position(ra_fixture);
309 let module = db.module_for_file(pos.file_id); 360 let module = db.module_for_file(pos.file_id);
310 let parsed_path_file = syntax::SourceFile::parse(&format!("use {};", path)); 361 let parsed_path_file = syntax::SourceFile::parse(&format!("use {};", path));
@@ -324,9 +375,20 @@ mod tests {
324 .take_types() 375 .take_types()
325 .unwrap(); 376 .unwrap();
326 377
327 let found_path = find_path(&db, ItemInNs::Types(resolved), module); 378 let found_path = if absolute { find_path_prefixed } else { find_path }(
379 &db,
380 ItemInNs::Types(resolved),
381 module,
382 );
383 assert_eq!(found_path, Some(mod_path), "absolute {}", absolute);
384 }
385
386 fn check_found_path(ra_fixture: &str, path: &str) {
387 check_found_path_(ra_fixture, path, false);
388 }
328 389
329 assert_eq!(found_path, Some(mod_path)); 390 fn check_found_path_abs(ra_fixture: &str, path: &str) {
391 check_found_path_(ra_fixture, path, true);
330 } 392 }
331 393
332 #[test] 394 #[test]
@@ -337,6 +399,7 @@ mod tests {
337 <|> 399 <|>
338 "#; 400 "#;
339 check_found_path(code, "S"); 401 check_found_path(code, "S");
402 check_found_path_abs(code, "S");
340 } 403 }
341 404
342 #[test] 405 #[test]
@@ -347,6 +410,7 @@ mod tests {
347 <|> 410 <|>
348 "#; 411 "#;
349 check_found_path(code, "E::A"); 412 check_found_path(code, "E::A");
413 check_found_path_abs(code, "E::A");
350 } 414 }
351 415
352 #[test] 416 #[test]
@@ -359,6 +423,7 @@ mod tests {
359 <|> 423 <|>
360 "#; 424 "#;
361 check_found_path(code, "foo::S"); 425 check_found_path(code, "foo::S");
426 check_found_path_abs(code, "foo::S");
362 } 427 }
363 428
364 #[test] 429 #[test]
@@ -373,6 +438,7 @@ mod tests {
373 <|> 438 <|>
374 "#; 439 "#;
375 check_found_path(code, "super::S"); 440 check_found_path(code, "super::S");
441 check_found_path_abs(code, "super::S");
376 } 442 }
377 443
378 #[test] 444 #[test]
@@ -384,6 +450,7 @@ mod tests {
384 <|> 450 <|>
385 "#; 451 "#;
386 check_found_path(code, "self"); 452 check_found_path(code, "self");
453 check_found_path_abs(code, "self");
387 } 454 }
388 455
389 #[test] 456 #[test]
@@ -395,6 +462,7 @@ mod tests {
395 <|> 462 <|>
396 "#; 463 "#;
397 check_found_path(code, "crate"); 464 check_found_path(code, "crate");
465 check_found_path_abs(code, "crate");
398 } 466 }
399 467
400 #[test] 468 #[test]
@@ -407,6 +475,7 @@ mod tests {
407 <|> 475 <|>
408 "#; 476 "#;
409 check_found_path(code, "crate::S"); 477 check_found_path(code, "crate::S");
478 check_found_path_abs(code, "crate::S");
410 } 479 }
411 480
412 #[test] 481 #[test]
@@ -418,6 +487,7 @@ mod tests {
418 pub struct S; 487 pub struct S;
419 "#; 488 "#;
420 check_found_path(code, "std::S"); 489 check_found_path(code, "std::S");
490 check_found_path_abs(code, "std::S");
421 } 491 }
422 492
423 #[test] 493 #[test]
@@ -430,14 +500,14 @@ mod tests {
430 pub struct S; 500 pub struct S;
431 "#; 501 "#;
432 check_found_path(code, "std_renamed::S"); 502 check_found_path(code, "std_renamed::S");
503 check_found_path_abs(code, "std_renamed::S");
433 } 504 }
434 505
435 #[test] 506 #[test]
436 fn partially_imported() { 507 fn partially_imported() {
437 // Tests that short paths are used even for external items, when parts of the path are 508 // Tests that short paths are used even for external items, when parts of the path are
438 // already in scope. 509 // already in scope.
439 check_found_path( 510 let code = r#"
440 r#"
441 //- /main.rs crate:main deps:syntax 511 //- /main.rs crate:main deps:syntax
442 512
443 use syntax::ast; 513 use syntax::ast;
@@ -449,12 +519,11 @@ mod tests {
449 A, B, C, 519 A, B, C,
450 } 520 }
451 } 521 }
452 "#, 522 "#;
453 "ast::ModuleItem", 523 check_found_path(code, "ast::ModuleItem");
454 ); 524 check_found_path_abs(code, "syntax::ast::ModuleItem");
455 525
456 check_found_path( 526 let code = r#"
457 r#"
458 //- /main.rs crate:main deps:syntax 527 //- /main.rs crate:main deps:syntax
459 528
460 <|> 529 <|>
@@ -465,9 +534,9 @@ mod tests {
465 A, B, C, 534 A, B, C,
466 } 535 }
467 } 536 }
468 "#, 537 "#;
469 "syntax::ast::ModuleItem", 538 check_found_path(code, "syntax::ast::ModuleItem");
470 ); 539 check_found_path_abs(code, "syntax::ast::ModuleItem");
471 } 540 }
472 541
473 #[test] 542 #[test]
@@ -481,6 +550,7 @@ mod tests {
481 <|> 550 <|>
482 "#; 551 "#;
483 check_found_path(code, "bar::S"); 552 check_found_path(code, "bar::S");
553 check_found_path_abs(code, "bar::S");
484 } 554 }
485 555
486 #[test] 556 #[test]
@@ -494,6 +564,7 @@ mod tests {
494 <|> 564 <|>
495 "#; 565 "#;
496 check_found_path(code, "bar::U"); 566 check_found_path(code, "bar::U");
567 check_found_path_abs(code, "bar::U");
497 } 568 }
498 569
499 #[test] 570 #[test]
@@ -507,6 +578,7 @@ mod tests {
507 pub struct S; 578 pub struct S;
508 "#; 579 "#;
509 check_found_path(code, "std::S"); 580 check_found_path(code, "std::S");
581 check_found_path_abs(code, "std::S");
510 } 582 }
511 583
512 #[test] 584 #[test]
@@ -520,6 +592,7 @@ mod tests {
520 pub use prelude::*; 592 pub use prelude::*;
521 "#; 593 "#;
522 check_found_path(code, "S"); 594 check_found_path(code, "S");
595 check_found_path_abs(code, "S");
523 } 596 }
524 597
525 #[test] 598 #[test]
@@ -537,6 +610,8 @@ mod tests {
537 "#; 610 "#;
538 check_found_path(code, "None"); 611 check_found_path(code, "None");
539 check_found_path(code, "Some"); 612 check_found_path(code, "Some");
613 check_found_path_abs(code, "None");
614 check_found_path_abs(code, "Some");
540 } 615 }
541 616
542 #[test] 617 #[test]
@@ -553,6 +628,7 @@ mod tests {
553 pub use crate::foo::bar::S; 628 pub use crate::foo::bar::S;
554 "#; 629 "#;
555 check_found_path(code, "baz::S"); 630 check_found_path(code, "baz::S");
631 check_found_path_abs(code, "baz::S");
556 } 632 }
557 633
558 #[test] 634 #[test]
@@ -567,6 +643,7 @@ mod tests {
567 "#; 643 "#;
568 // crate::S would be shorter, but using private imports seems wrong 644 // crate::S would be shorter, but using private imports seems wrong
569 check_found_path(code, "crate::bar::S"); 645 check_found_path(code, "crate::bar::S");
646 check_found_path_abs(code, "crate::bar::S");
570 } 647 }
571 648
572 #[test] 649 #[test]
@@ -585,6 +662,7 @@ mod tests {
585 pub use super::foo; 662 pub use super::foo;
586 "#; 663 "#;
587 check_found_path(code, "crate::foo::S"); 664 check_found_path(code, "crate::foo::S");
665 check_found_path_abs(code, "crate::foo::S");
588 } 666 }
589 667
590 #[test] 668 #[test]
@@ -605,6 +683,7 @@ mod tests {
605 } 683 }
606 "#; 684 "#;
607 check_found_path(code, "std::sync::Arc"); 685 check_found_path(code, "std::sync::Arc");
686 check_found_path_abs(code, "std::sync::Arc");
608 } 687 }
609 688
610 #[test] 689 #[test]
@@ -629,6 +708,7 @@ mod tests {
629 } 708 }
630 "#; 709 "#;
631 check_found_path(code, "core::fmt::Error"); 710 check_found_path(code, "core::fmt::Error");
711 check_found_path_abs(code, "core::fmt::Error");
632 } 712 }
633 713
634 #[test] 714 #[test]
@@ -652,6 +732,7 @@ mod tests {
652 } 732 }
653 "#; 733 "#;
654 check_found_path(code, "alloc::sync::Arc"); 734 check_found_path(code, "alloc::sync::Arc");
735 check_found_path_abs(code, "alloc::sync::Arc");
655 } 736 }
656 737
657 #[test] 738 #[test]
@@ -669,6 +750,7 @@ mod tests {
669 pub struct Arc; 750 pub struct Arc;
670 "#; 751 "#;
671 check_found_path(code, "megaalloc::Arc"); 752 check_found_path(code, "megaalloc::Arc");
753 check_found_path_abs(code, "megaalloc::Arc");
672 } 754 }
673 755
674 #[test] 756 #[test]
@@ -683,5 +765,7 @@ mod tests {
683 "#; 765 "#;
684 check_found_path(code, "u8"); 766 check_found_path(code, "u8");
685 check_found_path(code, "u16"); 767 check_found_path(code, "u16");
768 check_found_path_abs(code, "u8");
769 check_found_path_abs(code, "u16");
686 } 770 }
687} 771}