fix: Dedup duplicate crates with differing origins in CrateGraph construction

crates/base-db/src/fixture.rs

@@ -13,9 +13,9 @@ use vfs::{file_set::FileSet, VfsPath};
 
 use crate::{
     input::{CrateName, CrateOrigin, LangCrateOrigin},
-    Change, CrateDisplayName, CrateGraph, CrateId, Dependency, Edition, Env, FileId, FilePosition,
-    FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros, ReleaseChannel,
-    SourceDatabaseExt, SourceRoot, SourceRootId,
+    Change, CrateDisplayName, CrateGraph, CrateId, Dependency, DependencyKind, Edition, Env,
+    FileId, FilePosition, FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError,
+    ProcMacros, ReleaseChannel, SourceDatabaseExt, SourceRoot, SourceRootId,
 };
 
 pub const WORKSPACE: SourceRootId = SourceRootId(0);
@@ -237,7 +237,12 @@ impl ChangeFixture {
                 crate_graph
                     .add_dep(
                         from_id,
-                        Dependency::with_prelude(CrateName::new(&to).unwrap(), to_id, prelude),
+                        Dependency::with_prelude(
+                            CrateName::new(&to).unwrap(),
+                            to_id,
+                            prelude,
+                            DependencyKind::Normal,
+                        ),
                     )
                     .unwrap();
             }
@@ -275,7 +280,14 @@ impl ChangeFixture {
 
             for krate in all_crates {
                 crate_graph
-                    .add_dep(krate, Dependency::new(CrateName::new("core").unwrap(), core_crate))
+                    .add_dep(
+                        krate,
+                        Dependency::new(
+                            CrateName::new("core").unwrap(),
+                            core_crate,
+                            DependencyKind::Normal,
+                        ),
+                    )
                     .unwrap();
             }
         }
@@ -317,7 +329,11 @@ impl ChangeFixture {
                 crate_graph
                     .add_dep(
                         krate,
-                        Dependency::new(CrateName::new("proc_macros").unwrap(), proc_macros_crate),
+                        Dependency::new(
+                            CrateName::new("proc_macros").unwrap(),
+                            proc_macros_crate,
+                            DependencyKind::Normal,
+                        ),
                     )
                     .unwrap();
             }

crates/base-db/src/input.rs

@@ -155,6 +155,10 @@ impl CrateOrigin {
     pub fn is_local(&self) -> bool {
         matches!(self, CrateOrigin::Local { .. })
     }
+
+    pub fn is_lib(&self) -> bool {
+        matches!(self, CrateOrigin::Library { .. })
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -324,6 +328,62 @@ pub struct CrateData {
     pub channel: Option<ReleaseChannel>,
 }
 
+impl CrateData {
+    /// Check if [`other`] is almost equal to [`self`] ignoring `CrateOrigin` value.
+    pub fn eq_ignoring_origin_and_deps(&self, other: &CrateData, ignore_dev_deps: bool) -> bool {
+        // This method has some obscure bits. These are mostly there to be compliant with
+        // some patches. References to the patches are given.
+        if self.root_file_id != other.root_file_id {
+            return false;
+        }
+
+        if self.display_name != other.display_name {
+            return false;
+        }
+
+        if self.is_proc_macro != other.is_proc_macro {
+            return false;
+        }
+
+        if self.edition != other.edition {
+            return false;
+        }
+
+        if self.version != other.version {
+            return false;
+        }
+
+        let mut opts = self.cfg_options.difference(&other.cfg_options);
+        if let Some(it) = opts.next() {
+            // Don't care if rust_analyzer CfgAtom is the only cfg in the difference set of self's and other's cfgs.
+            // https://github.com/rust-lang/rust-analyzer/blob/0840038f02daec6ba3238f05d8caa037d28701a0/crates/project-model/src/workspace.rs#L894
+            if it.to_string() != "rust_analyzer" {
+                return false;
+            }
+
+            if let Some(_) = opts.next() {
+                return false;
+            }
+        }
+
+        if self.env != other.env {
+            return false;
+        }
+
+        let slf_deps = self.dependencies.iter();
+        let other_deps = other.dependencies.iter();
+
+        if ignore_dev_deps {
+            return slf_deps
+                .clone()
+                .filter(|it| it.kind != DependencyKind::Dev)
+                .eq(other_deps.clone().filter(|it| it.kind != DependencyKind::Dev));
+        }
+
+        slf_deps.eq(other_deps)
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum Edition {
     Edition2015,
@@ -351,26 +411,43 @@ impl Env {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub enum DependencyKind {
+    Normal,
+    Dev,
+    Build,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Dependency {
     pub crate_id: CrateId,
     pub name: CrateName,
+    kind: DependencyKind,
     prelude: bool,
 }
 
 impl Dependency {
-    pub fn new(name: CrateName, crate_id: CrateId) -> Self {
-        Self { name, crate_id, prelude: true }
+    pub fn new(name: CrateName, crate_id: CrateId, kind: DependencyKind) -> Self {
+        Self { name, crate_id, prelude: true, kind }
     }
 
-    pub fn with_prelude(name: CrateName, crate_id: CrateId, prelude: bool) -> Self {
-        Self { name, crate_id, prelude }
+    pub fn with_prelude(
+        name: CrateName,
+        crate_id: CrateId,
+        prelude: bool,
+        kind: DependencyKind,
+    ) -> Self {
+        Self { name, crate_id, prelude, kind }
     }
 
     /// Whether this dependency is to be added to the depending crate's extern prelude.
     pub fn is_prelude(&self) -> bool {
         self.prelude
     }
+
+    pub fn kind(&self) -> DependencyKind {
+        self.kind
+    }
 }
 
 impl CrateGraph {
@@ -574,23 +651,46 @@ impl CrateGraph {
     pub fn extend(&mut self, mut other: CrateGraph, proc_macros: &mut ProcMacroPaths) {
         let topo = other.crates_in_topological_order();
         let mut id_map: FxHashMap<CrateId, CrateId> = FxHashMap::default();
-
         for topo in topo {
             let crate_data = &mut other.arena[topo];
+
             crate_data.dependencies.iter_mut().for_each(|dep| dep.crate_id = id_map[&dep.crate_id]);
             crate_data.dependencies.sort_by_key(|dep| dep.crate_id);
-
-            let res = self.arena.iter().find_map(
-                |(id, data)| {
-                    if data == crate_data {
-                        Some(id)
-                    } else {
-                        None
+            let res = self.arena.iter().find_map(|(id, data)| {
+                match (&data.origin, &crate_data.origin) {
+                    (a, b) if a == b => {
+                        if data.eq_ignoring_origin_and_deps(&crate_data, false) {
+                            return Some((id, false));
+                        }
+                    }
+                    (a @ CrateOrigin::Local { .. }, CrateOrigin::Library { .. })
+                    | (a @ CrateOrigin::Library { .. }, CrateOrigin::Local { .. }) => {
+                        // If the origins differ, check if the two crates are equal without
+                        // considering the dev dependencies, if they are, they most likely are in
+                        // different loaded workspaces which may cause issues. We keep the local
+                        // version and discard the library one as the local version may have
+                        // dev-dependencies that we want to keep resolving. See #15656 for more
+                        // information.
+                        if data.eq_ignoring_origin_and_deps(&crate_data, true) {
+                            return Some((id, if a.is_local() { false } else { true }));
+                        }
                     }
-                },
-            );
-            if let Some(res) = res {
+                    (_, _) => return None,
+                }
+
+                None
+            });
+
+            if let Some((res, should_update_lib_to_local)) = res {
                 id_map.insert(topo, res);
+                if should_update_lib_to_local {
+                    assert!(self.arena[res].origin.is_lib());
+                    assert!(crate_data.origin.is_local());
+                    self.arena[res].origin = crate_data.origin.clone();
+
+                    // Move local's dev dependencies into the newly-local-formerly-lib crate.
+                    self.arena[res].dependencies = crate_data.dependencies.clone();
+                }
             } else {
                 let id = self.arena.alloc(crate_data.clone());
                 id_map.insert(topo, id);
@@ -636,9 +736,11 @@ impl CrateGraph {
         match (cfg_if, std) {
             (Some(cfg_if), Some(std)) => {
                 self.arena[cfg_if].dependencies.clear();
-                self.arena[std]
-                    .dependencies
-                    .push(Dependency::new(CrateName::new("cfg_if").unwrap(), cfg_if));
+                self.arena[std].dependencies.push(Dependency::new(
+                    CrateName::new("cfg_if").unwrap(),
+                    cfg_if,
+                    DependencyKind::Normal,
+                ));
                 true
             }
             _ => false,
@@ -658,6 +760,8 @@ impl ops::Index<CrateId> for CrateGraph {
 }
 
 impl CrateData {
+    /// Add a dependency to `self` without checking if the dependency
+    // is existent among `self.dependencies`.
     fn add_dep(&mut self, dep: Dependency) {
         self.dependencies.push(dep)
     }
@@ -759,7 +863,7 @@ impl fmt::Display for CyclicDependenciesError {
 
 #[cfg(test)]
 mod tests {
-    use crate::CrateOrigin;
+    use crate::{CrateOrigin, DependencyKind};
 
     use super::{CrateGraph, CrateName, Dependency, Edition::Edition2018, Env, FileId};
 
@@ -806,13 +910,22 @@ mod tests {
             None,
         );
         assert!(graph
-            .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
+            .add_dep(
+                crate1,
+                Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
+            )
             .is_ok());
         assert!(graph
-            .add_dep(crate2, Dependency::new(CrateName::new("crate3").unwrap(), crate3))
+            .add_dep(
+                crate2,
+                Dependency::new(CrateName::new("crate3").unwrap(), crate3, DependencyKind::Normal)
+            )
             .is_ok());
         assert!(graph
-            .add_dep(crate3, Dependency::new(CrateName::new("crate1").unwrap(), crate1))
+            .add_dep(
+                crate3,
+                Dependency::new(CrateName::new("crate1").unwrap(), crate1, DependencyKind::Normal)
+            )
             .is_err());
     }
 
@@ -846,10 +959,16 @@ mod tests {
             None,
         );
         assert!(graph
-            .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
+            .add_dep(
+                crate1,
+                Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
+            )
             .is_ok());
         assert!(graph
-            .add_dep(crate2, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
+            .add_dep(
+                crate2,
+                Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
+            )
             .is_err());
     }
 
@@ -896,10 +1015,16 @@ mod tests {
             None,
         );
         assert!(graph
-            .add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
+            .add_dep(
+                crate1,
+                Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
+            )
             .is_ok());
         assert!(graph
-            .add_dep(crate2, Dependency::new(CrateName::new("crate3").unwrap(), crate3))
+            .add_dep(
+                crate2,
+                Dependency::new(CrateName::new("crate3").unwrap(), crate3, DependencyKind::Normal)
+            )
             .is_ok());
     }
 
@@ -935,12 +1060,20 @@ mod tests {
         assert!(graph
             .add_dep(
                 crate1,
-                Dependency::new(CrateName::normalize_dashes("crate-name-with-dashes"), crate2)
+                Dependency::new(
+                    CrateName::normalize_dashes("crate-name-with-dashes"),
+                    crate2,
+                    DependencyKind::Normal
+                )
             )
             .is_ok());
         assert_eq!(
             graph[crate1].dependencies,
-            vec![Dependency::new(CrateName::new("crate_name_with_dashes").unwrap(), crate2)]
+            vec![Dependency::new(
+                CrateName::new("crate_name_with_dashes").unwrap(),
+                crate2,
+                DependencyKind::Normal
+            )]
         );
     }
 }

crates/base-db/src/lib.rs

@@ -12,6 +12,7 @@ use rustc_hash::FxHashSet;
 use syntax::{ast, Parse, SourceFile, TextRange, TextSize};
 use triomphe::Arc;
 
+pub use crate::input::DependencyKind;
 pub use crate::{
     change::Change,
     input::{

crates/cfg/src/lib.rs

@@ -58,6 +58,13 @@ impl CfgOptions {
         self.enabled.insert(CfgAtom::KeyValue { key, value });
     }
 
+    pub fn difference<'a>(
+        &'a self,
+        other: &'a CfgOptions,
+    ) -> impl Iterator<Item = &'a CfgAtom> + 'a {
+        self.enabled.difference(&other.enabled)
+    }
+
     pub fn apply_diff(&mut self, diff: CfgDiff) {
         for atom in diff.enable {
             self.enabled.insert(atom);

crates/project-model/src/project_json.rs

@@ -49,7 +49,7 @@
 //! user explores them belongs to that extension (it's totally valid to change
 //! rust-project.json over time via configuration request!)
 
-use base_db::{CrateDisplayName, CrateId, CrateName, Dependency, Edition};
+use base_db::{CrateDisplayName, CrateId, CrateName, Dependency, DependencyKind, Edition};
 use la_arena::RawIdx;
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
@@ -135,6 +135,7 @@ impl ProjectJson {
                                 Dependency::new(
                                     dep_data.name,
                                     CrateId::from_raw(RawIdx::from(dep_data.krate as u32)),
+                                    DependencyKind::Normal,
                                 )
                             })
                             .collect::<Vec<_>>(),