Merge #9684

9684: Complete editable private items r=jonas-schievink a=jonas-schievink

This checks if a private item's location is editable (local source root), and completes them anyways if that's the case.

In order to test this, the `new_source_root` fixture command has been changed to take a `local` or `library` value, and to apply to all *following* files instead of the preceding ones (which would be hard to understand).

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

6 files changed, 108 insertions, 39 deletions

diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs
index 5bdc427f739..8025d6398c4 100644
--- a/crates/base_db/src/fixture.rs
+++ b/crates/base_db/src/fixture.rs
@@ -92,6 +92,7 @@ impl ChangeFixture {
         let mut default_cfg = CfgOptions::default();
 
         let mut file_set = FileSet::default();
+        let mut current_source_root_kind = SourceRootKind::Local;
         let source_root_prefix = "/".to_string();
         let mut file_id = FileId(0);
         let mut roots = Vec::new();
@@ -118,8 +119,13 @@ impl ChangeFixture {
                 assert!(meta.krate.is_some(), "can't specify deps without naming the crate")
             }
 
-            if meta.introduce_new_source_root {
-                roots.push(SourceRoot::new_local(mem::take(&mut file_set)));
+            if let Some(kind) = &meta.introduce_new_source_root {
+                let root = match current_source_root_kind {
+                    SourceRootKind::Local => SourceRoot::new_local(mem::take(&mut file_set)),
+                    SourceRootKind::Library => SourceRoot::new_library(mem::take(&mut file_set)),
+                };
+                roots.push(root);
+                current_source_root_kind = *kind;
             }
 
             if let Some(krate) = meta.krate {
@@ -197,7 +203,11 @@ impl ChangeFixture {
                 crate_graph.add_dep(krate, CrateName::new("core").unwrap(), core_crate).unwrap();
             }
         }
-        roots.push(SourceRoot::new_local(mem::take(&mut file_set)));
+        let root = match current_source_root_kind {
+            SourceRootKind::Local => SourceRoot::new_local(mem::take(&mut file_set)),
+            SourceRootKind::Library => SourceRoot::new_library(mem::take(&mut file_set)),
+        };
+        roots.push(root);
         change.set_roots(roots);
         change.set_crate_graph(crate_graph);
 
@@ -205,6 +215,12 @@ impl ChangeFixture {
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+enum SourceRootKind {
+    Local,
+    Library,
+}
+
 #[derive(Debug)]
 struct FileMeta {
     path: String,
@@ -213,7 +229,7 @@ struct FileMeta {
     cfg: CfgOptions,
     edition: Edition,
     env: Env,
-    introduce_new_source_root: bool,
+    introduce_new_source_root: Option<SourceRootKind>,
 }
 
 impl From<Fixture> for FileMeta {
@@ -229,7 +245,11 @@ impl From<Fixture> for FileMeta {
             cfg,
             edition: f.edition.as_ref().map_or(Edition::CURRENT, |v| Edition::from_str(v).unwrap()),
             env: f.env.into_iter().collect(),
-            introduce_new_source_root: f.introduce_new_source_root,
+            introduce_new_source_root: f.introduce_new_source_root.map(|kind| match &*kind {
+                "local" => SourceRootKind::Local,
+                "library" => SourceRootKind::Library,
+                invalid => panic!("invalid source root kind '{}'", invalid),
+            }),
         }
     }
 }
diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs
index 305836f79d8..d7a2b454841 100644
--- a/crates/ide/src/references.rs
+++ b/crates/ide/src/references.rs
@@ -1375,13 +1375,13 @@ fn foo(_: bool) -> bo$0ol { true }
     fn test_transitive() {
         check(
             r#"
-//- /level3.rs new_source_root: crate:level3
+//- /level3.rs new_source_root:local crate:level3
 pub struct Fo$0o;
-//- /level2.rs new_source_root: crate:level2 deps:level3
+//- /level2.rs new_source_root:local crate:level2 deps:level3
 pub use level3::Foo;
-//- /level1.rs new_source_root: crate:level1 deps:level2
+//- /level1.rs new_source_root:local crate:level1 deps:level2
 pub use level2::Foo;
-//- /level0.rs new_source_root: crate:level0 deps:level1
+//- /level0.rs new_source_root:local crate:level0 deps:level1
 pub use level1::Foo;
 "#,
             expect![[r#"
@@ -1411,7 +1411,7 @@ macro_rules! foo$0 {
 }
 //- /bar.rs
 foo!();
-//- /other.rs crate:other deps:lib new_source_root:
+//- /other.rs crate:other deps:lib new_source_root:local
 lib::foo!();
 "#,
             expect![[r#"
diff --git a/crates/ide_completion/src/completions/dot.rs b/crates/ide_completion/src/completions/dot.rs
index 97be0f0e4fa..6208b89728d 100644
--- a/crates/ide_completion/src/completions/dot.rs
+++ b/crates/ide_completion/src/completions/dot.rs
@@ -174,36 +174,81 @@ fn foo(a: A) { a.$0() }
     fn test_visibility_filtering() {
         check(
             r#"
-mod inner {
+//- /lib.rs crate:lib new_source_root:local
+pub mod m {
     pub struct A {
         private_field: u32,
         pub pub_field: u32,
         pub(crate) crate_field: u32,
-        pub(crate) super_field: u32,
+        pub(super) super_field: u32,
     }
 }
-fn foo(a: inner::A) { a.$0 }
+//- /main.rs crate:main deps:lib new_source_root:local
+fn foo(a: lib::m::A) { a.$0 }
 "#,
             expect![[r#"
-                fd pub_field   u32
-                fd crate_field u32
-                fd super_field u32
+                fd private_field u32
+                fd pub_field     u32
+                fd crate_field   u32
+                fd super_field   u32
             "#]],
         );
 
         check(
             r#"
-struct A {}
+//- /lib.rs crate:lib new_source_root:library
+pub mod m {
+    pub struct A {
+        private_field: u32,
+        pub pub_field: u32,
+        pub(crate) crate_field: u32,
+        pub(super) super_field: u32,
+    }
+}
+//- /main.rs crate:main deps:lib new_source_root:local
+fn foo(a: lib::m::A) { a.$0 }
+"#,
+            expect![[r#"
+                fd pub_field u32
+            "#]],
+        );
+
+        check(
+            r#"
+//- /lib.rs crate:lib new_source_root:local
+pub struct A {}
 mod m {
     impl super::A {
         fn private_method(&self) {}
-        pub(crate) fn the_method(&self) {}
+        pub(crate) fn crate_method(&self) {}
+        pub fn pub_method(&self) {}
     }
 }
-fn foo(a: A) { a.$0 }
+//- /main.rs crate:main deps:lib new_source_root:local
+fn foo(a: lib::A) { a.$0 }
 "#,
             expect![[r#"
-                me the_method() fn(&self)
+                me private_method() fn(&self)
+                me crate_method()   fn(&self)
+                me pub_method()     fn(&self)
+            "#]],
+        );
+        check(
+            r#"
+//- /lib.rs crate:lib new_source_root:library
+pub struct A {}
+mod m {
+    impl super::A {
+        fn private_method(&self) {}
+        pub(crate) fn crate_method(&self) {}
+        pub fn pub_method(&self) {}
+    }
+}
+//- /main.rs crate:main deps:lib new_source_root:local
+fn foo(a: lib::A) { a.$0 }
+"#,
+            expect![[r#"
+                me pub_method() fn(&self)
             "#]],
         );
     }
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index 6c05812bf85..1647ecb99a8 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -259,25 +259,25 @@ mod tests {
     fn associated_item_visibility() {
         check(
             r#"
-struct S;
-
-mod m {
-    impl super::S {
-        pub(crate) fn public_method() { }
-        fn private_method() { }
-        pub(crate) type PublicType = u32;
-        type PrivateType = u32;
-        pub(crate) const PUBLIC_CONST: u32 = 1;
-        const PRIVATE_CONST: u32 = 1;
-    }
+//- /lib.rs crate:lib new_source_root:library
+pub struct S;
+
+impl S {
+    pub fn public_method() { }
+    fn private_method() { }
+    pub type PublicType = u32;
+    type PrivateType = u32;
+    pub const PUBLIC_CONST: u32 = 1;
+    const PRIVATE_CONST: u32 = 1;
 }
 
-fn foo() { let _ = S::$0 }
+//- /main.rs crate:main deps:lib new_source_root:local
+fn foo() { let _ = lib::S::$0 }
 "#,
             expect![[r#"
                 fn public_method() fn()
-                ct PUBLIC_CONST    pub(crate) const PUBLIC_CONST: u32 = 1;
-                ta PublicType      pub(crate) type PublicType = u32;
+                ct PUBLIC_CONST    pub const PUBLIC_CONST: u32 = 1;
+                ta PublicType      pub type PublicType = u32;
             "#]],
         );
     }
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index a056b5405a5..09694abd79e 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -1,5 +1,6 @@
 //! See `CompletionContext` structure.
 
+use base_db::SourceDatabaseExt;
 use hir::{Local, ScopeDef, Semantics, SemanticsScope, Type};
 use ide_db::{
     base_db::{FilePosition, SourceDatabase},
@@ -380,8 +381,11 @@ impl<'a> CompletionContext<'a> {
             None => return false,
         };
         if !vis.is_visible_from(self.db, module.into()) {
-            // FIXME: if the definition location is editable, also show private items
-            return false;
+            // If the definition location is editable, also show private items
+            let root_file = defining_crate.root_file(self.db);
+            let source_root_id = self.db.file_source_root(root_file);
+            let is_editable = !self.db.source_root(source_root_id).is_library;
+            return is_editable;
         }
 
         if module.krate() != defining_crate && attrs.has_doc_hidden() {
diff --git a/crates/test_utils/src/fixture.rs b/crates/test_utils/src/fixture.rs
index 44656267fed..166df3ea017 100644
--- a/crates/test_utils/src/fixture.rs
+++ b/crates/test_utils/src/fixture.rs
@@ -74,7 +74,7 @@ pub struct Fixture {
     pub cfg_key_values: Vec<(String, String)>,
     pub edition: Option<String>,
     pub env: FxHashMap<String, String>,
-    pub introduce_new_source_root: bool,
+    pub introduce_new_source_root: Option<String>,
 }
 
 pub struct MiniCore {
@@ -162,7 +162,7 @@ impl Fixture {
         let mut cfg_atoms = Vec::new();
         let mut cfg_key_values = Vec::new();
         let mut env = FxHashMap::default();
-        let mut introduce_new_source_root = false;
+        let mut introduce_new_source_root = None;
         for component in components[1..].iter() {
             let (key, value) = component
                 .split_once(':')
@@ -186,7 +186,7 @@ impl Fixture {
                         }
                     }
                 }
-                "new_source_root" => introduce_new_source_root = true,
+                "new_source_root" => introduce_new_source_root = Some(value.to_string()),
                 _ => panic!("bad component: {:?}", component),
             }
         }