Repository: rust-lang/rust-analyzer (rust)

Issue:
Import suggestions do not take into account visibility
I never had problems with this in the past, but suddenly rust-analyzer's import suggestions no longer take into account `pub` visibility. Unfortunately I don't have a minimal reproduction, but I do have a reproduction.

1. Clone `https://github.com/pola-rs/polars` (suggest `--depth 1`).
2. Edit file `crates/polars-expr/src/lib.rs`, add the following function:
  ```rust
fn foo() -> DataType {
    todo!()
}
```
3. Ask rust-analyzer to import `DataType` for you: ![Image](https://github.com/user-attachments/assets/b1e9fd53-a882-427b-be65-4d0f4b150dbf)
4. Suggestion is wrong, it's not visibile to us here, it suggests something else:
  ![Image](https://github.com/user-attachments/assets/f2c21473-8ae0-4008-ad6d-f30258cc2a63)
5. Suggestion is *still* wrong, it finally gets a public symbol:
  ![Image](https://github.com/user-attachments/assets/5fdfc58a-b24d-478a-8534-eeeacf90400b)



**rust-analyzer version**: rust-analyzer version: 0.4.2148-standalone (418c1365e 2024-10-15) 

**rustc version**: rustc 1.83.0-nightly (ed04567ba 2024-09-28)

**editor or extension**: VS Code

**repository link (if public, optional)**: https://github.com/polar-rs/polars


Pull request:
fix: Prevent public re-export of private item
Fixes #18308

The original issue is quite complicated.
`polar-rs` monorepo has the following structure;

```rust
// crate polars-core
pub mod prelude {
    pub use crate::datatype::DataType;
}

// crate polars-ops
pub mod frame {
    pub use join::*;

    pub mod join {
        pub use hash_join::*;
        use polars_core::prelude::*;

        mod hash_join {
            pub use super::*;
        }
    }
}
```

1. The glob import `pub use super::*` inside `mod hash_join` reexports the visibility of the glob import `use polars_core::prelude::*` inside the `mod join` as "public", which was previously "module(join)"
2. The glob import `pub use hash_join::*` inside `mod join` overrides visibility of the previous visibility of the glob import `use polars_core::prelude::*` with "public" in 1., which was previously "module(join)"

The problematic step is 1., as rustc doesn't allow such re-export with E0603.
This PR prevent such "more visible" re-export by taking minimum of resolved item visibility and the re-export visibility

Patch:
diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs
index f5e03e5281..a615abd1bb 100644
--- a/crates/hir-def/src/find_path.rs
+++ b/crates/hir-def/src/find_path.rs
@@ -1025,7 +1025,7 @@ pub mod ast {
         check_found_path(
             r#"
 mod bar {
-    mod foo { pub(super) struct S; }
+    mod foo { pub(crate) struct S; }
     pub(crate) use foo::*;
 }
 $0
@@ -1047,7 +1047,7 @@ $0
         check_found_path(
             r#"
 mod bar {
-    mod foo { pub(super) struct S; }
+    mod foo { pub(crate) struct S; }
     pub(crate) use foo::S as U;
 }
 $0
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 22b9c2b4e3..b9aec93690 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -985,12 +985,8 @@ impl DefCollector<'_> {
         for (name, res) in resolutions {
             match name {
                 Some(name) => {
-                    changed |= self.push_res_and_update_glob_vis(
-                        module_id,
-                        name,
-                        res.with_visibility(vis),
-                        import,
-                    );
+                    changed |=
+                        self.push_res_and_update_glob_vis(module_id, name, *res, vis, import);
                 }
                 None => {
                     let tr = match res.take_types() {
@@ -1043,10 +1039,11 @@ impl DefCollector<'_> {
             .collect::<Vec<_>>();
 
         for (glob_importing_module, glob_import_vis, use_) in glob_imports {
+            let vis = glob_import_vis.min(vis, &self.def_map).unwrap_or(glob_import_vis);
             self.update_recursive(
                 glob_importing_module,
                 resolutions,
-                glob_import_vis,
+                vis,
                 Some(ImportType::Glob(use_)),
                 depth + 1,
             );
@@ -1058,8 +1055,19 @@ impl DefCollector<'_> {
         module_id: LocalModuleId,
         name: &Name,
         mut defs: PerNs,
+        vis: Visibility,
         def_import_type: Option<ImportType>,
     ) -> bool {
+        if let Some((_, v, _)) = defs.types.as_mut() {
+            *v = v.min(vis, &self.def_map).unwrap_or(vis);
+        }
+        if let Some((_, v, _)) = defs.values.as_mut() {
+            *v = v.min(vis, &self.def_map).unwrap_or(vis);
+        }
+        if let Some((_, v, _)) = defs.macros.as_mut() {
+            *v = v.min(vis, &self.def_map).unwrap_or(vis);
+        }
+
         let mut changed = false;
 
         if let Some(ImportType::Glob(_)) = def_import_type {
diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs
index 3aeb88047a..4edb683592 100644
--- a/crates/hir-def/src/visibility.rs
+++ b/crates/hir-def/src/visibility.rs
@@ -191,6 +191,11 @@ impl Visibility {
                     return None;
                 }
 
+                let def_block = def_map.block_id();
+                if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
+                    return None;
+                }
+
                 let mut a_ancestors =
                     iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
                 let mut b_ancestors =
@@ -210,6 +215,43 @@ impl Visibility {
             }
         }
     }
+
+    /// Returns the least permissive visibility of `self` and `other`.
+    ///
+    /// If there is no subset relation between `self` and `other`, returns `None` (ie. they're only
+    /// visible in unrelated modules).
+    pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
+        match (self, other) {
+            (vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis),
+            (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
+                if mod_a.krate != mod_b.krate {
+                    return None;
+                }
+
+                let def_block = def_map.block_id();
+                if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
+                    return None;
+                }
+
+                let mut a_ancestors =
+                    iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
+                let mut b_ancestors =
+                    iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
+
+                if a_ancestors.any(|m| m == mod_b.local_id) {
+                    // B is above A
+                    return Some(Visibility::Module(mod_a, expl_b));
+                }
+
+                if b_ancestors.any(|m| m == mod_a.local_id) {
+                    // A is above B
+                    return Some(Visibility::Module(mod_b, expl_a));
+                }
+
+                None
+            }
+        }
+    }
 }
 
 /// Whether the item was imported through an explicit `pub(crate) use` or just a `use` without
diff --git a/crates/rust-analyzer/src/lsp/capabilities.rs b/crates/rust-analyzer/src/lsp/capabilities.rs
index 939593dd45..3b19284f24 100644
--- a/crates/rust-analyzer/src/lsp/capabilities.rs
+++ b/crates/rust-analyzer/src/lsp/capabilities.rs
@@ -210,7 +210,9 @@ impl ClientCapabilities {
                 .completion_item
                 .as_ref()?
                 .label_details_support
-        })() == Some(true)
+                .as_ref()
+        })()
+        .is_some()
     }
 
     fn completion_item(&self) -> Option<CompletionOptionsCompletionItem> {
diff --git a/crates/rust-analyzer/src/tracing/config.rs b/crates/rust-analyzer/src/tracing/config.rs
index 02ae4186ab..5ab2dc2b67 100644
--- a/crates/rust-analyzer/src/tracing/config.rs
+++ b/crates/rust-analyzer/src/tracing/config.rs
@@ -58,22 +58,14 @@ where
         let writer = self.writer;
 
         let ra_fmt_layer = tracing_subscriber::fmt::layer()
+            .with_timer(
+                time::OffsetTime::local_rfc_3339()
+                    .expect("Could not get local offset, make sure you're on the main thread"),
+            )
             .with_target(false)
             .with_ansi(false)
-            .with_writer(writer);
-
-        let ra_fmt_layer = match time::OffsetTime::local_rfc_3339() {
-            Ok(timer) => {
-                // If we can get the time offset, format logs with the timezone.
-                ra_fmt_layer.with_timer(timer).boxed()
-            }
-            Err(_) => {
-                // Use system time if we can't get the time offset. This should
-                // never happen on Linux, but can happen on e.g. OpenBSD.
-                ra_fmt_layer.boxed()
-            }
-        }
-        .with_filter(targets_filter);
+            .with_writer(writer)
+            .with_filter(targets_filter);
 
         let chalk_layer = match self.chalk_filter {
             Some(chalk_filter) => {


Test patch:
diff --git a/crates/hir-def/src/nameres/tests/globs.rs b/crates/hir-def/src/nameres/tests/globs.rs
index a2696055ca..543ab41cd5 100644
--- a/crates/hir-def/src/nameres/tests/globs.rs
+++ b/crates/hir-def/src/nameres/tests/globs.rs
@@ -412,3 +412,42 @@ use reexport::*;
         "#]],
     );
 }
+
+#[test]
+fn regression_18308() {
+    check(
+        r#"
+use outer::*;
+
+mod outer {
+    mod inner_superglob {
+        pub use super::*;
+    }
+
+    // The importing order matters!
+    pub use inner_superglob::*;
+    use super::glob_target::*;
+}
+
+mod glob_target {
+    pub struct ShouldBePrivate;
+}
+"#,
+        expect![[r#"
+            crate
+            glob_target: t
+            outer: t
+
+            crate::glob_target
+            ShouldBePrivate: t v
+
+            crate::outer
+            ShouldBePrivate: t v
+            inner_superglob: t
+
+            crate::outer::inner_superglob
+            ShouldBePrivate: t v
+            inner_superglob: t
+        "#]],
+    );
+}