From 886eaa0a7d2b1d7098fd49ec56e3b6f65a457449 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sun, 24 Sep 2023 18:27:15 +0200 Subject: [PATCH 1/7] Relaxation for crate graph mergin Partially fixes #15656 . When a crate graph is extended which is the case when new workspaces are added to the project the rules for deduplication were too strict. One problem that arises from this is that in certain conditions when we see the same crate having different `CrateOrigin`s the first form would be maintained. This approach however results in some unwanted results such as making renaming forbidden as this has been recently only made available for local crates. The given example in #15656 can still not be resolved with this PR as that involves taking inconsistencies between dependencies into consideration. This will be addressed in a future PR. --- crates/base-db/src/fixture.rs | 28 ++- crates/base-db/src/input.rs | 221 +++++++++++++++--- crates/base-db/src/lib.rs | 1 + crates/cfg/src/lib.rs | 4 +- crates/project-model/src/project_json.rs | 3 +- crates/project-model/src/workspace.rs | 67 ++++-- .../cargo_hello_world_project_model.txt | 7 + ...project_model_with_selective_overrides.txt | 7 + ..._project_model_with_wildcard_overrides.txt | 7 + ...rust_project_hello_world_project_model.txt | 16 ++ crates/rust-analyzer/tests/slow-tests/main.rs | 208 ++++++++++++++++- 11 files changed, 515 insertions(+), 54 deletions(-) diff --git a/crates/base-db/src/fixture.rs b/crates/base-db/src/fixture.rs index 3f5ccb621c76..3da555a47acb 100644 --- a/crates/base-db/src/fixture.rs +++ b/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(); } diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 65db5c0fc7d3..fb69e667ad95 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -8,7 +8,7 @@ use std::{fmt, mem, ops, panic::RefUnwindSafe, str::FromStr, sync}; -use cfg::CfgOptions; +use cfg::{CfgDiff, CfgOptions}; use la_arena::{Arena, Idx}; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::SmolStr; @@ -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,99 @@ pub struct CrateData { pub channel: Option, } +impl CrateData { + /** + Check if [`other`] is almost equal to [`self`]. + This method has some obscure bits. These are mostly there to be compliant with + some patches. References to the patches are given. + */ + pub fn almost_eq(&self, other: &CrateData) -> bool { + 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.clone(); + opts.apply_diff(CfgDiff { + disable: other.cfg_options.clone().into_iter().collect(), + enable: vec![], + }); + + let mut cfgs = opts.into_iter(); + if let Some(cfg) = cfgs.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 !cfgs.next().is_none() || cfg.to_string() != "rust_analyzer" { + return false; + } + } + + let mut itself = self.dependencies.iter(); + let mut otself = other.dependencies.iter(); + let (mut anx, mut bnx) = (itself.next(), otself.next()); + loop { + match (anx, bnx) { + (None, None) => { + break; + } + (None, Some(b)) => { + if b.kind != DependencyKind::Normal { + bnx = otself.next(); + } else { + break; + } + } + (Some(a), None) => { + if a.kind != DependencyKind::Normal { + anx = itself.next(); + } else { + break; + } + } + (Some(a), Some(b)) => { + if a.kind != DependencyKind::Normal { + anx = itself.next(); + continue; + } + + if b.kind != DependencyKind::Normal { + bnx = otself.next(); + continue; + } + + if a != b { + return false; + } + + anx = itself.next(); + bnx = otself.next(); + } + } + } + + if self.env != other.env { + return false; + } + + true + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Edition { Edition2015, @@ -351,26 +448,43 @@ impl Env { } } +#[derive(Debug, 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 { @@ -572,25 +686,41 @@ impl CrateGraph { /// Note that for deduplication to fully work, `self`'s crate dependencies must be sorted by crate id. /// If the crate dependencies were sorted, the resulting graph from this `extend` call will also have the crate dependencies sorted. pub fn extend(&mut self, mut other: CrateGraph, proc_macros: &mut ProcMacroPaths) { + enum ExtendStrategy { + Dedup(CrateId), + Replace(CrateId), + } + let topo = other.crates_in_topological_order(); let mut id_map: FxHashMap = 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 + 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.almost_eq(crate_data) { + if data.origin.is_lib() && crate_data.origin.is_local() { + // See #15656 for a relevant example. + return Some(ExtendStrategy::Replace(id)); } - }, - ); + + return Some(ExtendStrategy::Dedup(id)); + } + None + }); + if let Some(res) = res { - id_map.insert(topo, res); + match res { + ExtendStrategy::Dedup(res) => id_map.insert(topo, res), + ExtendStrategy::Replace(res) => { + let id = self.arena.alloc(crate_data.clone()); + let _ = self.remove_and_replace(res, id); + id_map.insert(topo, id) + } + }; } else { let id = self.arena.alloc(crate_data.clone()); id_map.insert(topo, id); @@ -636,9 +766,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, @@ -759,7 +891,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 +938,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 +987,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 +1043,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 +1088,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 + )] ); } } diff --git a/crates/base-db/src/lib.rs b/crates/base-db/src/lib.rs index c5c4afa30f79..40cfab88afdb 100644 --- a/crates/base-db/src/lib.rs +++ b/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::{ diff --git a/crates/cfg/src/lib.rs b/crates/cfg/src/lib.rs index 0aeb0b050524..90dba008ad70 100644 --- a/crates/cfg/src/lib.rs +++ b/crates/cfg/src/lib.rs @@ -115,8 +115,8 @@ impl<'a> IntoIterator for &'a CfgOptions { #[derive(Default, Clone, Debug, PartialEq, Eq)] pub struct CfgDiff { // Invariants: No duplicates, no atom that's both in `enable` and `disable`. - enable: Vec, - disable: Vec, + pub enable: Vec, + pub disable: Vec, } impl CfgDiff { diff --git a/crates/project-model/src/project_json.rs b/crates/project-model/src/project_json.rs index 80897f7478cf..931eba115766 100644 --- a/crates/project-model/src/project_json.rs +++ b/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::>(), diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index e0209ca15a53..9333570354a0 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -6,8 +6,8 @@ use std::{collections::VecDeque, fmt, fs, iter, process::Command, str::FromStr, use anyhow::{format_err, Context}; use base_db::{ - CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency, Edition, Env, - FileId, LangCrateOrigin, ProcMacroPaths, ReleaseChannel, TargetLayoutLoadResult, + CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency, DependencyKind, + Edition, Env, FileId, LangCrateOrigin, ProcMacroPaths, ReleaseChannel, TargetLayoutLoadResult, }; use cfg::{CfgDiff, CfgOptions}; use paths::{AbsPath, AbsPathBuf}; @@ -834,7 +834,7 @@ fn project_json_to_crate_graph( for dep in &krate.deps { if let Some(&to) = crates.get(&dep.crate_id) { - add_dep(crate_graph, from, dep.name.clone(), to) + add_dep(crate_graph, from, dep.name.clone(), to, dep.kind().to_owned()) } } } @@ -979,7 +979,7 @@ fn cargo_to_crate_graph( // cargo metadata does not do any normalization, // so we do it ourselves currently let name = CrateName::normalize_dashes(&name); - add_dep(crate_graph, from, name, to); + add_dep(crate_graph, from, name, to, DependencyKind::Normal); } } } @@ -999,7 +999,17 @@ fn cargo_to_crate_graph( continue; } - add_dep(crate_graph, from, name.clone(), to) + add_dep( + crate_graph, + from, + name.clone(), + to, + match dep.kind { + DepKind::Normal => DependencyKind::Normal, + DepKind::Dev => DependencyKind::Dev, + DepKind::Build => DependencyKind::Build, + }, + ) } } } @@ -1187,7 +1197,17 @@ fn handle_rustc_crates( let name = CrateName::new(&dep.name).unwrap(); if let Some(&to) = pkg_to_lib_crate.get(&dep.pkg) { for &from in rustc_pkg_crates.get(&pkg).into_iter().flatten() { - add_dep(crate_graph, from, name.clone(), to); + add_dep( + crate_graph, + from, + name.clone(), + to, + match dep.kind { + DepKind::Normal => DependencyKind::Normal, + DepKind::Dev => DependencyKind::Dev, + DepKind::Build => DependencyKind::Build, + }, + ); } } } @@ -1209,7 +1229,7 @@ fn handle_rustc_crates( // `rust_analyzer` thinks that it should use the one from the `rustc_source` // instead of the one from `crates.io` if !crate_graph[*from].dependencies.iter().any(|d| d.name == name) { - add_dep(crate_graph, *from, name.clone(), to); + add_dep(crate_graph, *from, name.clone(), to, DependencyKind::Normal); } } } @@ -1308,7 +1328,14 @@ impl SysrootPublicDeps { /// Makes `from` depend on the public sysroot crates. fn add_to_crate_graph(&self, crate_graph: &mut CrateGraph, from: CrateId) { for (name, krate, prelude) in &self.deps { - add_dep_with_prelude(crate_graph, from, name.clone(), *krate, *prelude); + add_dep_with_prelude( + crate_graph, + from, + name.clone(), + *krate, + *prelude, + DependencyKind::Normal, + ); } } } @@ -1363,7 +1390,7 @@ fn sysroot_to_crate_graph( for &to in sysroot[from].deps.iter() { let name = CrateName::new(&sysroot[to].name).unwrap(); if let (Some(&from), Some(&to)) = (sysroot_crates.get(&from), sysroot_crates.get(&to)) { - add_dep(crate_graph, from, name, to); + add_dep(crate_graph, from, name, to, DependencyKind::Normal); } } } @@ -1442,8 +1469,14 @@ fn handle_hack_cargo_workspace( .collect() } -fn add_dep(graph: &mut CrateGraph, from: CrateId, name: CrateName, to: CrateId) { - add_dep_inner(graph, from, Dependency::new(name, to)) +fn add_dep( + graph: &mut CrateGraph, + from: CrateId, + name: CrateName, + to: CrateId, + kind: DependencyKind, +) { + add_dep_inner(graph, from, Dependency::new(name, to, kind)) } fn add_dep_with_prelude( @@ -1452,12 +1485,20 @@ fn add_dep_with_prelude( name: CrateName, to: CrateId, prelude: bool, + kind: DependencyKind, ) { - add_dep_inner(graph, from, Dependency::with_prelude(name, to, prelude)) + add_dep_inner(graph, from, Dependency::with_prelude(name, to, prelude, kind)) } fn add_proc_macro_dep(crate_graph: &mut CrateGraph, from: CrateId, to: CrateId, prelude: bool) { - add_dep_with_prelude(crate_graph, from, CrateName::new("proc_macro").unwrap(), to, prelude); + add_dep_with_prelude( + crate_graph, + from, + CrateName::new("proc_macro").unwrap(), + to, + prelude, + DependencyKind::Normal, + ); } fn add_dep_inner(graph: &mut CrateGraph, from: CrateId, dep: Dependency) { diff --git a/crates/project-model/test_data/output/cargo_hello_world_project_model.txt b/crates/project-model/test_data/output/cargo_hello_world_project_model.txt index 727d39a3077c..e98f016ca7df 100644 --- a/crates/project-model/test_data/output/cargo_hello_world_project_model.txt +++ b/crates/project-model/test_data/output/cargo_hello_world_project_model.txt @@ -48,6 +48,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -112,6 +113,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -119,6 +121,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -183,6 +186,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -190,6 +194,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -254,6 +259,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -261,6 +267,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], diff --git a/crates/project-model/test_data/output/cargo_hello_world_project_model_with_selective_overrides.txt b/crates/project-model/test_data/output/cargo_hello_world_project_model_with_selective_overrides.txt index 727d39a3077c..e98f016ca7df 100644 --- a/crates/project-model/test_data/output/cargo_hello_world_project_model_with_selective_overrides.txt +++ b/crates/project-model/test_data/output/cargo_hello_world_project_model_with_selective_overrides.txt @@ -48,6 +48,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -112,6 +113,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -119,6 +121,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -183,6 +186,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -190,6 +194,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -254,6 +259,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -261,6 +267,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], diff --git a/crates/project-model/test_data/output/cargo_hello_world_project_model_with_wildcard_overrides.txt b/crates/project-model/test_data/output/cargo_hello_world_project_model_with_wildcard_overrides.txt index 89728babd82e..7ecd53572e2f 100644 --- a/crates/project-model/test_data/output/cargo_hello_world_project_model_with_wildcard_overrides.txt +++ b/crates/project-model/test_data/output/cargo_hello_world_project_model_with_wildcard_overrides.txt @@ -47,6 +47,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -110,6 +111,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -117,6 +119,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -180,6 +183,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -187,6 +191,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], @@ -250,6 +255,7 @@ name: CrateName( "hello_world", ), + kind: Normal, prelude: true, }, Dependency { @@ -257,6 +263,7 @@ name: CrateName( "libc", ), + kind: Normal, prelude: true, }, ], diff --git a/crates/project-model/test_data/output/rust_project_hello_world_project_model.txt b/crates/project-model/test_data/output/rust_project_hello_world_project_model.txt index b7bf6cb2774e..581a6afc148f 100644 --- a/crates/project-model/test_data/output/rust_project_hello_world_project_model.txt +++ b/crates/project-model/test_data/output/rust_project_hello_world_project_model.txt @@ -28,6 +28,7 @@ name: CrateName( "core", ), + kind: Normal, prelude: true, }, ], @@ -168,6 +169,7 @@ name: CrateName( "std", ), + kind: Normal, prelude: true, }, Dependency { @@ -175,6 +177,7 @@ name: CrateName( "core", ), + kind: Normal, prelude: true, }, ], @@ -249,6 +252,7 @@ name: CrateName( "alloc", ), + kind: Normal, prelude: true, }, Dependency { @@ -256,6 +260,7 @@ name: CrateName( "panic_unwind", ), + kind: Normal, prelude: true, }, Dependency { @@ -263,6 +268,7 @@ name: CrateName( "panic_abort", ), + kind: Normal, prelude: true, }, Dependency { @@ -270,6 +276,7 @@ name: CrateName( "core", ), + kind: Normal, prelude: true, }, Dependency { @@ -277,6 +284,7 @@ name: CrateName( "profiler_builtins", ), + kind: Normal, prelude: true, }, Dependency { @@ -284,6 +292,7 @@ name: CrateName( "unwind", ), + kind: Normal, prelude: true, }, Dependency { @@ -291,6 +300,7 @@ name: CrateName( "std_detect", ), + kind: Normal, prelude: true, }, Dependency { @@ -298,6 +308,7 @@ name: CrateName( "test", ), + kind: Normal, prelude: true, }, ], @@ -438,6 +449,7 @@ name: CrateName( "core", ), + kind: Normal, prelude: true, }, Dependency { @@ -445,6 +457,7 @@ name: CrateName( "alloc", ), + kind: Normal, prelude: true, }, Dependency { @@ -452,6 +465,7 @@ name: CrateName( "std", ), + kind: Normal, prelude: true, }, Dependency { @@ -459,6 +473,7 @@ name: CrateName( "test", ), + kind: Normal, prelude: false, }, Dependency { @@ -466,6 +481,7 @@ name: CrateName( "proc_macro", ), + kind: Normal, prelude: false, }, ], diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index d59914298991..423c07183e25 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -21,12 +21,12 @@ use std::{collections::HashMap, path::PathBuf, time::Instant}; use lsp_types::{ notification::DidOpenTextDocument, request::{ - CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, + CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, Rename, WillRenameFiles, WorkspaceSymbolRequest, }, CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams, - PartialResultParams, Position, Range, RenameFilesParams, TextDocumentItem, + PartialResultParams, Position, Range, RenameFilesParams, RenameParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, }; use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams}; @@ -1131,3 +1131,207 @@ version = "0.0.0" server.request::(Default::default(), json!([])); } + +#[test] +fn test_deduplicate_crate_differing_in_origin() { + let fixture = r#" +//- /projects/p1/Cargo.toml +[package] +name = "p1" +version = "0.0.0" + +//- /projects/p1/src/lib.rs +pub fn add2(left: usize, right: usize) -> usize { + left + right +} + +//- /projects/p2/Cargo.toml +[package] +name = "p2" +version = "0.0.0" + +[dependencies] +p1 = { path = "../p1" } + +//- /projects/p2/src/lib.rs +use p1::add2; + +pub fn bar() {} + "#; + + let server = Project::with_fixture(fixture) + .with_config(serde_json::json!({ + "linkedProjects" : [ + "./projects/p1/Cargo.toml", + "./projects/p2/Cargo.toml" + ], + } + )) + .with_config(serde_json::json!({ + "cargo": { "sysroot": null }, + })) + .server() + .wait_until_workspace_is_loaded(); + + let doc_id = server.doc_id("./projects/p2/src/lib.rs"); + let doc2_id = server.doc_id("./projects/p1/src/lib.rs"); + + server.request::( + RenameParams { + text_document_position: TextDocumentPositionParams { + text_document: doc_id.clone(), + position: Position { line: 0, character: 8 }, + }, + new_name: "ABC".to_owned(), + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + }, + json!({ + "documentChanges": [ + { + "textDocument": { + "uri": doc2_id.uri, + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 0, + "character": 7 + }, + "end": { + "line": 0, + "character": 11 + } + }, + "newText": "ABC" + } + ] + }, + { + "textDocument": { + "uri": doc_id.uri, + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 0, + "character": 8 + }, + "end": { + "line": 0, + "character": 12 + } + }, + "newText": "ABC" + } + ] + }, + ] + }), + ); +} + +#[test] +fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { + let fixture = r#" +//- /projects/p1/Cargo.toml +[package] +name = "p1" +version = "0.0.0" + +//- /projects/p1/src/lib.rs +pub fn add2(left: usize, right: usize) -> usize { + left + right +} + +//- /projects/p2/Cargo.toml +[package] +name = "p2" +version = "0.0.0" + +[dependencies] +p1 = { path = "../p1" } + +//- /projects/p2/src/lib.rs +use p1::add2; + +pub fn bar() {} + "#; + + let server = Project::with_fixture(fixture) + .with_config(serde_json::json!({ + "linkedProjects" : [ + "./projects/p2/Cargo.toml", + "./projects/p1/Cargo.toml", + ], + } + )) + .with_config(serde_json::json!({ + "cargo": { "sysroot": null }, + })) + .server() + .wait_until_workspace_is_loaded(); + + let doc_id = server.doc_id("./projects/p2/src/lib.rs"); + let doc2_id = server.doc_id("./projects/p1/src/lib.rs"); + + server.request::( + RenameParams { + text_document_position: TextDocumentPositionParams { + text_document: doc_id.clone(), + position: Position { line: 0, character: 8 }, + }, + new_name: "ABC".to_owned(), + work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, + }, + json!({ + "documentChanges": [ + { + "textDocument": { + "uri": doc2_id.uri, + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 0, + "character": 7 + }, + "end": { + "line": 0, + "character": 11 + } + }, + "newText": "ABC" + } + ] + }, + { + "textDocument": { + "uri": doc_id.uri, + "version": null + }, + "edits": [ + { + "range": { + "start": { + "line": 0, + "character": 8 + }, + "end": { + "line": 0, + "character": 12 + } + }, + "newText": "ABC" + } + ] + }, + ] + }), + ); +} From 7e4aad5ba570745ef8924f1fa6ad90aedf6aa51e Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sun, 15 Oct 2023 17:32:12 +0200 Subject: [PATCH 2/7] v2 --- crates/base-db/src/input.rs | 52 ++--- crates/cfg/src/lib.rs | 4 +- crates/project-model/src/tests.rs | 50 +++++ .../deduplication_crate_graph_A.json | 66 ++++++ .../deduplication_crate_graph_B.json | 140 ++++++++++++ crates/rust-analyzer/tests/slow-tests/main.rs | 208 +----------------- 6 files changed, 282 insertions(+), 238 deletions(-) create mode 100644 crates/project-model/test_data/deduplication_crate_graph_A.json create mode 100644 crates/project-model/test_data/deduplication_crate_graph_B.json diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index fb69e667ad95..61a17da9aa6e 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -329,12 +329,10 @@ pub struct CrateData { } impl CrateData { - /** - Check if [`other`] is almost equal to [`self`]. - This method has some obscure bits. These are mostly there to be compliant with - some patches. References to the patches are given. - */ - pub fn almost_eq(&self, other: &CrateData) -> bool { + /// Check if [`other`] is almost equal to [`self`] ignoring `CrateOrigin` value. + pub fn eq_ignoring_origin(&self, other: &CrateData) -> 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; } @@ -356,16 +354,16 @@ impl CrateData { } let mut opts = self.cfg_options.clone(); - opts.apply_diff(CfgDiff { - disable: other.cfg_options.clone().into_iter().collect(), - enable: vec![], - }); + opts.apply_diff( + CfgDiff::new(vec![], other.cfg_options.clone().into_iter().collect()) + .expect("CfgOptions were expected to contain no duplicates."), + ); let mut cfgs = opts.into_iter(); if let Some(cfg) = cfgs.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 !cfgs.next().is_none() || cfg.to_string() != "rust_analyzer" { + if cfgs.next().is_some() || cfg.to_string() != "rust_analyzer" { return false; } } @@ -686,41 +684,35 @@ impl CrateGraph { /// Note that for deduplication to fully work, `self`'s crate dependencies must be sorted by crate id. /// If the crate dependencies were sorted, the resulting graph from this `extend` call will also have the crate dependencies sorted. pub fn extend(&mut self, mut other: CrateGraph, proc_macros: &mut ProcMacroPaths) { - enum ExtendStrategy { - Dedup(CrateId), - Replace(CrateId), - } - let topo = other.crates_in_topological_order(); let mut id_map: FxHashMap = 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.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.almost_eq(crate_data) { + if data.eq_ignoring_origin(crate_data) { if data.origin.is_lib() && crate_data.origin.is_local() { // See #15656 for a relevant example. - return Some(ExtendStrategy::Replace(id)); + return Some((id, true)); } - return Some(ExtendStrategy::Dedup(id)); + return Some((id, false)); } None }); - if let Some(res) = res { - match res { - ExtendStrategy::Dedup(res) => id_map.insert(topo, res), - ExtendStrategy::Replace(res) => { - let id = self.arena.alloc(crate_data.clone()); - let _ = self.remove_and_replace(res, id); - id_map.insert(topo, id) + if let Some((res, should_update_lib_to_local)) = res { + id_map.insert(topo, res); + if should_update_lib_to_local { + let origin_old = self.arena[res].origin.clone(); + assert!(origin_old.is_lib()); + + if let CrateOrigin::Library { repo, name } = origin_old { + self.arena[res].origin = CrateOrigin::Local { repo, name: Some(name) }; } - }; + } } else { let id = self.arena.alloc(crate_data.clone()); id_map.insert(topo, id); diff --git a/crates/cfg/src/lib.rs b/crates/cfg/src/lib.rs index 90dba008ad70..0aeb0b050524 100644 --- a/crates/cfg/src/lib.rs +++ b/crates/cfg/src/lib.rs @@ -115,8 +115,8 @@ impl<'a> IntoIterator for &'a CfgOptions { #[derive(Default, Clone, Debug, PartialEq, Eq)] pub struct CfgDiff { // Invariants: No duplicates, no atom that's both in `enable` and `disable`. - pub enable: Vec, - pub disable: Vec, + enable: Vec, + disable: Vec, } impl CfgDiff { diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 7815b9dda77f..65c6f0b25624 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -249,3 +249,53 @@ fn crate_graph_dedup() { crate_graph.extend(regex_crate_graph, &mut regex_proc_macros); assert_eq!(crate_graph.iter().count(), 118); } + +#[test] +fn test_deduplicate_crate_differing_in_origin() { + let path_map = &mut Default::default(); + let (mut crate_graph, _proc_macros) = + load_cargo_with_sysroot(path_map, "deduplication_crate_graph_A.json"); + crate_graph.sort_deps(); + let (crate_graph_1, mut _proc_macros_2) = + load_cargo_with_sysroot(path_map, "deduplication_crate_graph_B.json"); + + crate_graph.extend(crate_graph_1, &mut _proc_macros_2); + + let mut crates_named_p1 = vec![]; + for id in crate_graph.iter() { + let krate = &crate_graph[id]; + if let Some(name) = krate.display_name.as_ref() { + if name.to_string() == "p1" { + crates_named_p1.push(krate); + } + } + } + + assert!(crates_named_p1.len() == 1); + assert!(crates_named_p1[0].origin.is_local()); +} + +#[test] +fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { + let path_map = &mut Default::default(); + let (mut crate_graph, _proc_macros) = + load_cargo_with_sysroot(path_map, "deduplication_crate_graph_B.json"); + crate_graph.sort_deps(); + let (crate_graph_1, mut _proc_macros_2) = + load_cargo_with_sysroot(path_map, "deduplication_crate_graph_A.json"); + + crate_graph.extend(crate_graph_1, &mut _proc_macros_2); + + let mut crates_named_p1 = vec![]; + for id in crate_graph.iter() { + let krate = &crate_graph[id]; + if let Some(name) = krate.display_name.as_ref() { + if name.to_string() == "p1" { + crates_named_p1.push(krate); + } + } + } + + assert!(crates_named_p1.len() == 1); + assert!(crates_named_p1[0].origin.is_local()); +} diff --git a/crates/project-model/test_data/deduplication_crate_graph_A.json b/crates/project-model/test_data/deduplication_crate_graph_A.json new file mode 100644 index 000000000000..3f627082f9f9 --- /dev/null +++ b/crates/project-model/test_data/deduplication_crate_graph_A.json @@ -0,0 +1,66 @@ +{ + "packages": [ + { + "name": "p1", + "version": "0.1.0", + "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "p1", + "src_path": "/path/to/project/projects/p1/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "/path/to/project/projects/p1/Cargo.toml", + "metadata": null, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2021", + "links": null, + "default_run": null, + "rust_version": null + } + ], + "workspace_members": [ + "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + ], + "workspace_default_members": [ + "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + ], + "resolve": { + "nodes": [ + { + "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "dependencies": [], + "deps": [], + "features": [] + } + ], + "root": "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + }, + "target_directory": "/path/to/project/projects/p1/target", + "version": 1, + "workspace_root": "/path/to/project/projects/p1", + "metadata": null +} \ No newline at end of file diff --git a/crates/project-model/test_data/deduplication_crate_graph_B.json b/crates/project-model/test_data/deduplication_crate_graph_B.json new file mode 100644 index 000000000000..a2bf1af04407 --- /dev/null +++ b/crates/project-model/test_data/deduplication_crate_graph_B.json @@ -0,0 +1,140 @@ +{ + "packages": [ + { + "name": "p1", + "version": "0.1.0", + "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "p1", + "src_path": "/path/to/project/projects/p1/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "/path/to/project/projects/p1/Cargo.toml", + "metadata": null, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2021", + "links": null, + "default_run": null, + "rust_version": null + }, + { + "name": "p2", + "version": "0.1.0", + "id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [ + { + "name": "p1", + "source": null, + "req": "*", + "kind": null, + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null, + "path": "/path/to/project/projects/p1" + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "p2", + "src_path": "/path/to/project/projects/p2/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "/path/to/project/projects/p2/Cargo.toml", + "metadata": null, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2021", + "links": null, + "default_run": null, + "rust_version": null + } + ], + "workspace_members": [ + "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + ], + "workspace_default_members": [ + "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + ], + "resolve": { + "nodes": [ + { + "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "dependencies": [], + "deps": [], + "features": [] + }, + { + "id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)", + "dependencies": [ + "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + ], + "deps": [ + { + "name": "p1", + "pkg": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + } + ], + "features": [] + } + ], + "root": "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + }, + "target_directory": "/path/to/project/projects/p2/target", + "version": 1, + "workspace_root": "/path/to/project/projects/p2", + "metadata": null +} \ No newline at end of file diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index 423c07183e25..d59914298991 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -21,12 +21,12 @@ use std::{collections::HashMap, path::PathBuf, time::Instant}; use lsp_types::{ notification::DidOpenTextDocument, request::{ - CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, Rename, + CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, WillRenameFiles, WorkspaceSymbolRequest, }, CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams, - PartialResultParams, Position, Range, RenameFilesParams, RenameParams, TextDocumentItem, + PartialResultParams, Position, Range, RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams, }; use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams}; @@ -1131,207 +1131,3 @@ version = "0.0.0" server.request::(Default::default(), json!([])); } - -#[test] -fn test_deduplicate_crate_differing_in_origin() { - let fixture = r#" -//- /projects/p1/Cargo.toml -[package] -name = "p1" -version = "0.0.0" - -//- /projects/p1/src/lib.rs -pub fn add2(left: usize, right: usize) -> usize { - left + right -} - -//- /projects/p2/Cargo.toml -[package] -name = "p2" -version = "0.0.0" - -[dependencies] -p1 = { path = "../p1" } - -//- /projects/p2/src/lib.rs -use p1::add2; - -pub fn bar() {} - "#; - - let server = Project::with_fixture(fixture) - .with_config(serde_json::json!({ - "linkedProjects" : [ - "./projects/p1/Cargo.toml", - "./projects/p2/Cargo.toml" - ], - } - )) - .with_config(serde_json::json!({ - "cargo": { "sysroot": null }, - })) - .server() - .wait_until_workspace_is_loaded(); - - let doc_id = server.doc_id("./projects/p2/src/lib.rs"); - let doc2_id = server.doc_id("./projects/p1/src/lib.rs"); - - server.request::( - RenameParams { - text_document_position: TextDocumentPositionParams { - text_document: doc_id.clone(), - position: Position { line: 0, character: 8 }, - }, - new_name: "ABC".to_owned(), - work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, - }, - json!({ - "documentChanges": [ - { - "textDocument": { - "uri": doc2_id.uri, - "version": null - }, - "edits": [ - { - "range": { - "start": { - "line": 0, - "character": 7 - }, - "end": { - "line": 0, - "character": 11 - } - }, - "newText": "ABC" - } - ] - }, - { - "textDocument": { - "uri": doc_id.uri, - "version": null - }, - "edits": [ - { - "range": { - "start": { - "line": 0, - "character": 8 - }, - "end": { - "line": 0, - "character": 12 - } - }, - "newText": "ABC" - } - ] - }, - ] - }), - ); -} - -#[test] -fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { - let fixture = r#" -//- /projects/p1/Cargo.toml -[package] -name = "p1" -version = "0.0.0" - -//- /projects/p1/src/lib.rs -pub fn add2(left: usize, right: usize) -> usize { - left + right -} - -//- /projects/p2/Cargo.toml -[package] -name = "p2" -version = "0.0.0" - -[dependencies] -p1 = { path = "../p1" } - -//- /projects/p2/src/lib.rs -use p1::add2; - -pub fn bar() {} - "#; - - let server = Project::with_fixture(fixture) - .with_config(serde_json::json!({ - "linkedProjects" : [ - "./projects/p2/Cargo.toml", - "./projects/p1/Cargo.toml", - ], - } - )) - .with_config(serde_json::json!({ - "cargo": { "sysroot": null }, - })) - .server() - .wait_until_workspace_is_loaded(); - - let doc_id = server.doc_id("./projects/p2/src/lib.rs"); - let doc2_id = server.doc_id("./projects/p1/src/lib.rs"); - - server.request::( - RenameParams { - text_document_position: TextDocumentPositionParams { - text_document: doc_id.clone(), - position: Position { line: 0, character: 8 }, - }, - new_name: "ABC".to_owned(), - work_done_progress_params: WorkDoneProgressParams { work_done_token: None }, - }, - json!({ - "documentChanges": [ - { - "textDocument": { - "uri": doc2_id.uri, - "version": null - }, - "edits": [ - { - "range": { - "start": { - "line": 0, - "character": 7 - }, - "end": { - "line": 0, - "character": 11 - } - }, - "newText": "ABC" - } - ] - }, - { - "textDocument": { - "uri": doc_id.uri, - "version": null - }, - "edits": [ - { - "range": { - "start": { - "line": 0, - "character": 8 - }, - "end": { - "line": 0, - "character": 12 - } - }, - "newText": "ABC" - } - ] - }, - ] - }), - ); -} From 25e990d753982682c71ae09888383f2c862411c3 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Wed, 8 Nov 2023 16:51:20 +0100 Subject: [PATCH 3/7] v3 --- crates/base-db/src/input.rs | 108 +++++++++++++++--------------------- crates/cfg/src/lib.rs | 6 +- 2 files changed, 49 insertions(+), 65 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 61a17da9aa6e..7dec01ff4dee 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -8,7 +8,7 @@ use std::{fmt, mem, ops, panic::RefUnwindSafe, str::FromStr, sync}; -use cfg::{CfgDiff, CfgOptions}; +use cfg::CfgOptions; use la_arena::{Arena, Idx}; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::SmolStr; @@ -330,7 +330,7 @@ pub struct CrateData { impl CrateData { /// Check if [`other`] is almost equal to [`self`] ignoring `CrateOrigin` value. - pub fn eq_ignoring_origin(&self, other: &CrateData) -> bool { + 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 { @@ -353,69 +353,36 @@ impl CrateData { return false; } - let mut opts = self.cfg_options.clone(); - opts.apply_diff( - CfgDiff::new(vec![], other.cfg_options.clone().into_iter().collect()) - .expect("CfgOptions were expected to contain no duplicates."), - ); - - let mut cfgs = opts.into_iter(); - if let Some(cfg) = cfgs.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 cfgs.next().is_some() || cfg.to_string() != "rust_analyzer" { - return false; - } - } - - let mut itself = self.dependencies.iter(); - let mut otself = other.dependencies.iter(); - let (mut anx, mut bnx) = (itself.next(), otself.next()); - loop { - match (anx, bnx) { - (None, None) => { - break; - } - (None, Some(b)) => { - if b.kind != DependencyKind::Normal { - bnx = otself.next(); - } else { - break; - } - } - (Some(a), None) => { - if a.kind != DependencyKind::Normal { - anx = itself.next(); - } else { - break; - } - } - (Some(a), Some(b)) => { - if a.kind != DependencyKind::Normal { - anx = itself.next(); - continue; - } - - if b.kind != DependencyKind::Normal { - bnx = otself.next(); - continue; - } - - if a != b { + let mut opts = self.cfg_options.diff(&other.cfg_options).into_iter(); + match opts.len() { + 0 => (), + 1 => { + // 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 let Some(cfg) = opts.next() { + if cfg.to_string() != "rust_analyzer" { return false; } - - anx = itself.next(); - bnx = otself.next(); } } - } + _ => return false, + }; if self.env != other.env { return false; } - true + let slf_deps = self.dependencies.iter(); + let other_deps = other.dependencies.iter(); + + if ignore_dev_deps { + slf_deps + .clone() + .filter(|it| it.kind == DependencyKind::Normal) + .eq(other_deps.clone().filter(|it| it.kind == DependencyKind::Normal)); + } + + slf_deps.eq(other_deps) } } @@ -446,7 +413,7 @@ impl Env { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum DependencyKind { Normal, Dev, @@ -480,8 +447,8 @@ impl Dependency { self.prelude } - pub fn kind(&self) -> &DependencyKind { - &self.kind + pub fn kind(&self) -> DependencyKind { + self.kind } } @@ -692,14 +659,27 @@ impl CrateGraph { 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.eq_ignoring_origin(crate_data) { - if data.origin.is_lib() && crate_data.origin.is_local() { + 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)); + } + } + (CrateOrigin::Local { .. }, CrateOrigin::Library { .. }) => { // See #15656 for a relevant example. - return Some((id, true)); + if data.eq_ignoring_origin_and_deps(&crate_data, true) { + return Some((id, false)); + } } - - return Some((id, false)); + (CrateOrigin::Library { .. }, CrateOrigin::Local { .. }) => { + // See #15656 for a relevant example. + if data.eq_ignoring_origin_and_deps(&crate_data, true) { + return Some((id, true)); + } + } + (_, _) => return None, } + None }); diff --git a/crates/cfg/src/lib.rs b/crates/cfg/src/lib.rs index 0aeb0b050524..f197dad2b25e 100644 --- a/crates/cfg/src/lib.rs +++ b/crates/cfg/src/lib.rs @@ -7,7 +7,7 @@ mod dnf; #[cfg(test)] mod tests; -use std::fmt; +use std::{collections::HashSet, fmt}; use rustc_hash::FxHashSet; use tt::SmolStr; @@ -58,6 +58,10 @@ impl CfgOptions { self.enabled.insert(CfgAtom::KeyValue { key, value }); } + pub fn diff<'a>(&'a self, other: &'a CfgOptions) -> HashSet<&CfgAtom> { + self.enabled.difference(&other.enabled).collect() + } + pub fn apply_diff(&mut self, diff: CfgDiff) { for atom in diff.enable { self.enabled.insert(atom); From 74d8fdc8fef42fe47b12d533bb0d58ae024c4c66 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 11 Nov 2023 13:45:50 +0100 Subject: [PATCH 4/7] Update test data for crate deduping Make data reflect a case where dev deps are existent. base-db::CrateGraph::extend now adds dev dependencies for a crate in case of its upgrading from a CrateOrigin::Lib kind of a crate to a CrateOrigin::Local one. --- crates/base-db/src/input.rs | 51 +++++++--- crates/project-model/src/tests.rs | 10 +- .../deduplication_crate_graph_A.json | 94 +++++++++++++++++-- .../deduplication_crate_graph_B.json | 48 ++++++---- 4 files changed, 162 insertions(+), 41 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 7dec01ff4dee..cec3e91eee47 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -376,7 +376,7 @@ impl CrateData { let other_deps = other.dependencies.iter(); if ignore_dev_deps { - slf_deps + return slf_deps .clone() .filter(|it| it.kind == DependencyKind::Normal) .eq(other_deps.clone().filter(|it| it.kind == DependencyKind::Normal)); @@ -524,7 +524,7 @@ impl CrateGraph { self.check_cycle_after_dependency(from, dep.crate_id)?; - self.arena[from].add_dep(dep); + self.arena[from].add_dep_unchecked(dep); Ok(()) } @@ -665,16 +665,11 @@ impl CrateGraph { return Some((id, false)); } } - (CrateOrigin::Local { .. }, CrateOrigin::Library { .. }) => { + (a @ CrateOrigin::Local { .. }, CrateOrigin::Library { .. }) + | (a @ CrateOrigin::Library { .. }, CrateOrigin::Local { .. }) => { // See #15656 for a relevant example. if data.eq_ignoring_origin_and_deps(&crate_data, true) { - return Some((id, false)); - } - } - (CrateOrigin::Library { .. }, CrateOrigin::Local { .. }) => { - // See #15656 for a relevant example. - if data.eq_ignoring_origin_and_deps(&crate_data, true) { - return Some((id, true)); + return Some((id, if a.is_local() { false } else { true })); } } (_, _) => return None, @@ -692,6 +687,16 @@ impl CrateGraph { if let CrateOrigin::Library { repo, name } = origin_old { self.arena[res].origin = CrateOrigin::Local { repo, name: Some(name) }; } + + // Move local's dev dependencies into the newly-local-formerly-lib crate. + let dev_deps = crate_data + .dependencies + .clone() + .into_iter() + .filter(|dep| dep.kind() == DependencyKind::Dev) + .collect::>(); + + self.arena[res].add_dep(dev_deps).unwrap_or_default(); } } else { let id = self.arena.alloc(crate_data.clone()); @@ -761,10 +766,34 @@ impl ops::Index for CrateGraph { } } +struct ExistingDepsError(Vec); + impl CrateData { - fn add_dep(&mut self, dep: Dependency) { + /// Add a dependency to `self` without checking if the dependency + // is existent among `self.dependencies`. + fn add_dep_unchecked(&mut self, dep: Dependency) { self.dependencies.push(dep) } + + /// Add `deps` to `self` if the dependency is not already listed. + /// Finally returning an `Err` propagating the dependencies it couldn't add. + fn add_dep(&mut self, deps: Vec) -> Result<(), ExistingDepsError> { + let mut existing_deps: Vec = vec![]; + + deps.into_iter().for_each(|dep| { + if !self.dependencies.contains(&dep) { + self.dependencies.push(dep); + } else { + existing_deps.push(dep); + } + }); + + if !existing_deps.is_empty() { + return Err(ExistingDepsError(existing_deps)); + } + + Ok(()) + } } impl FromStr for Edition { diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 65c6f0b25624..35ac80eee328 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use base_db::{CrateGraph, FileId, ProcMacroPaths}; +use base_db::{CrateGraph, DependencyKind, FileId, ProcMacroPaths}; use cfg::{CfgAtom, CfgDiff}; use expect_test::{expect_file, ExpectFile}; use paths::{AbsPath, AbsPathBuf}; @@ -272,7 +272,9 @@ fn test_deduplicate_crate_differing_in_origin() { } assert!(crates_named_p1.len() == 1); - assert!(crates_named_p1[0].origin.is_local()); + let p1 = crates_named_p1[0]; + assert!(p1.dependencies.iter().filter(|dep| dep.kind() == DependencyKind::Dev).count() == 1); + assert!(p1.origin.is_local()); } #[test] @@ -297,5 +299,7 @@ fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { } assert!(crates_named_p1.len() == 1); - assert!(crates_named_p1[0].origin.is_local()); + let p1 = crates_named_p1[0]; + assert!(p1.dependencies.iter().filter(|dep| dep.kind() == DependencyKind::Dev).count() == 1); + assert!(p1.origin.is_local()); } diff --git a/crates/project-model/test_data/deduplication_crate_graph_A.json b/crates/project-model/test_data/deduplication_crate_graph_A.json index 3f627082f9f9..edaf185fc61a 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_A.json +++ b/crates/project-model/test_data/deduplication_crate_graph_A.json @@ -3,12 +3,26 @@ { "name": "p1", "version": "0.1.0", - "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", "license": null, "license_file": null, "description": null, "source": null, - "dependencies": [], + "dependencies": [ + { + "name": "p3", + "source": null, + "req": "*", + "kind": "dev", + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null, + "path": "/path/to/project/example_project/projects/p3" + } + ], "targets": [ { "kind": [ @@ -18,7 +32,7 @@ "lib" ], "name": "p1", - "src_path": "/path/to/project/projects/p1/src/lib.rs", + "src_path": "/path/to/project/example_project/projects/p1/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -26,7 +40,48 @@ } ], "features": {}, - "manifest_path": "/path/to/project/projects/p1/Cargo.toml", + "manifest_path": "/path/to/project/example_project/projects/p1/Cargo.toml", + "metadata": null, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2021", + "links": null, + "default_run": null, + "rust_version": null + }, + { + "name": "p3", + "version": "0.1.0", + "id": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "p3", + "src_path": "/path/to/project/example_project/projects/p3/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "/path/to/project/example_project/projects/p3/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -43,24 +98,43 @@ } ], "workspace_members": [ - "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" ], "workspace_default_members": [ - "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" ], "resolve": { "nodes": [ { - "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", + "dependencies": [ + "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)" + ], + "deps": [ + { + "name": "p3", + "pkg": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", + "dep_kinds": [ + { + "kind": "dev", + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", "dependencies": [], "deps": [], "features": [] } ], - "root": "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + "root": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" }, - "target_directory": "/path/to/project/projects/p1/target", + "target_directory": "/path/to/project/example_project/projects/p1/target", "version": 1, - "workspace_root": "/path/to/project/projects/p1", + "workspace_root": "/path/to/project/example_project/projects/p1", "metadata": null } \ No newline at end of file diff --git a/crates/project-model/test_data/deduplication_crate_graph_B.json b/crates/project-model/test_data/deduplication_crate_graph_B.json index a2bf1af04407..4f753db71bf0 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_B.json +++ b/crates/project-model/test_data/deduplication_crate_graph_B.json @@ -3,12 +3,26 @@ { "name": "p1", "version": "0.1.0", - "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", "license": null, "license_file": null, "description": null, "source": null, - "dependencies": [], + "dependencies": [ + { + "name": "p3", + "source": null, + "req": "*", + "kind": "dev", + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null, + "path": "/path/to/project/example_project/projects/p3" + } + ], "targets": [ { "kind": [ @@ -18,7 +32,7 @@ "lib" ], "name": "p1", - "src_path": "/path/to/project/projects/p1/src/lib.rs", + "src_path": "/path/to/project/example_project/projects/p1/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -26,7 +40,7 @@ } ], "features": {}, - "manifest_path": "/path/to/project/projects/p1/Cargo.toml", + "manifest_path": "/path/to/project/example_project/projects/p1/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -44,7 +58,7 @@ { "name": "p2", "version": "0.1.0", - "id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)", + "id": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)", "license": null, "license_file": null, "description": null, @@ -61,7 +75,7 @@ "features": [], "target": null, "registry": null, - "path": "/path/to/project/projects/p1" + "path": "/path/to/project/example_project/projects/p1" } ], "targets": [ @@ -73,7 +87,7 @@ "lib" ], "name": "p2", - "src_path": "/path/to/project/projects/p2/src/lib.rs", + "src_path": "/path/to/project/example_project/projects/p2/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -81,7 +95,7 @@ } ], "features": {}, - "manifest_path": "/path/to/project/projects/p2/Cargo.toml", + "manifest_path": "/path/to/project/example_project/projects/p2/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -98,28 +112,28 @@ } ], "workspace_members": [ - "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" ], "workspace_default_members": [ - "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" ], "resolve": { "nodes": [ { - "id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", "dependencies": [], "deps": [], "features": [] }, { - "id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)", + "id": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)", "dependencies": [ - "p1 0.1.0 (path+file:///path/to/project/projects/p1)" + "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" ], "deps": [ { "name": "p1", - "pkg": "p1 0.1.0 (path+file:///path/to/project/projects/p1)", + "pkg": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", "dep_kinds": [ { "kind": null, @@ -131,10 +145,10 @@ "features": [] } ], - "root": "p2 0.1.0 (path+file:///path/to/project/projects/p2)" + "root": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" }, - "target_directory": "/path/to/project/projects/p2/target", + "target_directory": "/path/to/project/example_project/projects/p2/target", "version": 1, - "workspace_root": "/path/to/project/projects/p2", + "workspace_root": "/path/to/project/example_project/projects/p2", "metadata": null } \ No newline at end of file From f79e8182c1e611b385fd91c924e347ea5f6c71d3 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Tue, 21 Nov 2023 14:05:31 +0100 Subject: [PATCH 5/7] v3 --- crates/base-db/src/input.rs | 79 ++++++++++++------------------------- crates/cfg/src/lib.rs | 9 +++-- 2 files changed, 31 insertions(+), 57 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index cec3e91eee47..82149892bd68 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -353,20 +353,18 @@ impl CrateData { return false; } - let mut opts = self.cfg_options.diff(&other.cfg_options).into_iter(); - match opts.len() { - 0 => (), - 1 => { - // 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 let Some(cfg) = opts.next() { - if cfg.to_string() != "rust_analyzer" { - 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; } - _ => return false, - }; + + if let Some(_) = opts.next() { + return false; + } + } if self.env != other.env { return false; @@ -378,8 +376,8 @@ impl CrateData { if ignore_dev_deps { return slf_deps .clone() - .filter(|it| it.kind == DependencyKind::Normal) - .eq(other_deps.clone().filter(|it| it.kind == DependencyKind::Normal)); + .filter(|it| it.kind != DependencyKind::Dev) + .eq(other_deps.clone().filter(|it| it.kind != DependencyKind::Dev)); } slf_deps.eq(other_deps) @@ -524,7 +522,7 @@ impl CrateGraph { self.check_cycle_after_dependency(from, dep.crate_id)?; - self.arena[from].add_dep_unchecked(dep); + self.arena[from].add_dep(dep); Ok(()) } @@ -667,7 +665,12 @@ impl CrateGraph { } (a @ CrateOrigin::Local { .. }, CrateOrigin::Library { .. }) | (a @ CrateOrigin::Library { .. }, CrateOrigin::Local { .. }) => { - // See #15656 for a relevant example. + // 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 })); } @@ -681,22 +684,12 @@ impl CrateGraph { if let Some((res, should_update_lib_to_local)) = res { id_map.insert(topo, res); if should_update_lib_to_local { - let origin_old = self.arena[res].origin.clone(); - assert!(origin_old.is_lib()); - - if let CrateOrigin::Library { repo, name } = origin_old { - self.arena[res].origin = CrateOrigin::Local { repo, name: Some(name) }; - } + 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. - let dev_deps = crate_data - .dependencies - .clone() - .into_iter() - .filter(|dep| dep.kind() == DependencyKind::Dev) - .collect::>(); - - self.arena[res].add_dep(dev_deps).unwrap_or_default(); + self.arena[res].dependencies = crate_data.dependencies.clone(); } } else { let id = self.arena.alloc(crate_data.clone()); @@ -766,34 +759,12 @@ impl ops::Index for CrateGraph { } } -struct ExistingDepsError(Vec); - impl CrateData { /// Add a dependency to `self` without checking if the dependency // is existent among `self.dependencies`. - fn add_dep_unchecked(&mut self, dep: Dependency) { + fn add_dep(&mut self, dep: Dependency) { self.dependencies.push(dep) } - - /// Add `deps` to `self` if the dependency is not already listed. - /// Finally returning an `Err` propagating the dependencies it couldn't add. - fn add_dep(&mut self, deps: Vec) -> Result<(), ExistingDepsError> { - let mut existing_deps: Vec = vec![]; - - deps.into_iter().for_each(|dep| { - if !self.dependencies.contains(&dep) { - self.dependencies.push(dep); - } else { - existing_deps.push(dep); - } - }); - - if !existing_deps.is_empty() { - return Err(ExistingDepsError(existing_deps)); - } - - Ok(()) - } } impl FromStr for Edition { diff --git a/crates/cfg/src/lib.rs b/crates/cfg/src/lib.rs index f197dad2b25e..8bbe5e2a8c29 100644 --- a/crates/cfg/src/lib.rs +++ b/crates/cfg/src/lib.rs @@ -7,7 +7,7 @@ mod dnf; #[cfg(test)] mod tests; -use std::{collections::HashSet, fmt}; +use std::fmt; use rustc_hash::FxHashSet; use tt::SmolStr; @@ -58,8 +58,11 @@ impl CfgOptions { self.enabled.insert(CfgAtom::KeyValue { key, value }); } - pub fn diff<'a>(&'a self, other: &'a CfgOptions) -> HashSet<&CfgAtom> { - self.enabled.difference(&other.enabled).collect() + pub fn difference<'a>( + &'a self, + other: &'a CfgOptions, + ) -> impl Iterator + 'a { + self.enabled.difference(&other.enabled) } pub fn apply_diff(&mut self, diff: CfgDiff) { From 736994f02664aa4cd84f55be481aae33bcf48ca3 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 23 Nov 2023 02:08:30 +0100 Subject: [PATCH 6/7] Make test cases simpler --- crates/base-db/src/input.rs | 2 +- crates/project-model/src/tests.rs | 32 +++--- .../deduplication_crate_graph_A.json | 44 +++---- .../deduplication_crate_graph_B.json | 108 ++---------------- 4 files changed, 48 insertions(+), 138 deletions(-) diff --git a/crates/base-db/src/input.rs b/crates/base-db/src/input.rs index 82149892bd68..e4f78321e215 100644 --- a/crates/base-db/src/input.rs +++ b/crates/base-db/src/input.rs @@ -668,7 +668,7 @@ impl CrateGraph { // 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 + // 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) { diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs index 35ac80eee328..98f3063bb982 100644 --- a/crates/project-model/src/tests.rs +++ b/crates/project-model/src/tests.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use base_db::{CrateGraph, DependencyKind, FileId, ProcMacroPaths}; +use base_db::{CrateGraph, FileId, ProcMacroPaths}; use cfg::{CfgAtom, CfgDiff}; use expect_test::{expect_file, ExpectFile}; use paths::{AbsPath, AbsPathBuf}; @@ -251,7 +251,7 @@ fn crate_graph_dedup() { } #[test] -fn test_deduplicate_crate_differing_in_origin() { +fn test_deduplicate_origin_dev() { let path_map = &mut Default::default(); let (mut crate_graph, _proc_macros) = load_cargo_with_sysroot(path_map, "deduplication_crate_graph_A.json"); @@ -261,24 +261,23 @@ fn test_deduplicate_crate_differing_in_origin() { crate_graph.extend(crate_graph_1, &mut _proc_macros_2); - let mut crates_named_p1 = vec![]; + let mut crates_named_p2 = vec![]; for id in crate_graph.iter() { let krate = &crate_graph[id]; if let Some(name) = krate.display_name.as_ref() { - if name.to_string() == "p1" { - crates_named_p1.push(krate); + if name.to_string() == "p2" { + crates_named_p2.push(krate); } } } - assert!(crates_named_p1.len() == 1); - let p1 = crates_named_p1[0]; - assert!(p1.dependencies.iter().filter(|dep| dep.kind() == DependencyKind::Dev).count() == 1); - assert!(p1.origin.is_local()); + assert!(crates_named_p2.len() == 1); + let p2 = crates_named_p2[0]; + assert!(p2.origin.is_local()); } #[test] -fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { +fn test_deduplicate_origin_dev_rev() { let path_map = &mut Default::default(); let (mut crate_graph, _proc_macros) = load_cargo_with_sysroot(path_map, "deduplication_crate_graph_B.json"); @@ -288,18 +287,17 @@ fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() { crate_graph.extend(crate_graph_1, &mut _proc_macros_2); - let mut crates_named_p1 = vec![]; + let mut crates_named_p2 = vec![]; for id in crate_graph.iter() { let krate = &crate_graph[id]; if let Some(name) = krate.display_name.as_ref() { - if name.to_string() == "p1" { - crates_named_p1.push(krate); + if name.to_string() == "p2" { + crates_named_p2.push(krate); } } } - assert!(crates_named_p1.len() == 1); - let p1 = crates_named_p1[0]; - assert!(p1.dependencies.iter().filter(|dep| dep.kind() == DependencyKind::Dev).count() == 1); - assert!(p1.origin.is_local()); + assert!(crates_named_p2.len() == 1); + let p2 = crates_named_p2[0]; + assert!(p2.origin.is_local()); } diff --git a/crates/project-model/test_data/deduplication_crate_graph_A.json b/crates/project-model/test_data/deduplication_crate_graph_A.json index edaf185fc61a..66bfcb2bf6ef 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_A.json +++ b/crates/project-model/test_data/deduplication_crate_graph_A.json @@ -3,24 +3,24 @@ { "name": "p1", "version": "0.1.0", - "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", + "id": "p1 0.1.0 (path+file:///example_project/p1)", "license": null, "license_file": null, "description": null, "source": null, "dependencies": [ { - "name": "p3", + "name": "p2", "source": null, "req": "*", - "kind": "dev", + "kind": null, "rename": null, "optional": false, "uses_default_features": true, "features": [], "target": null, "registry": null, - "path": "/path/to/project/example_project/projects/p3" + "path": "/example_project/p2" } ], "targets": [ @@ -32,7 +32,7 @@ "lib" ], "name": "p1", - "src_path": "/path/to/project/example_project/projects/p1/src/lib.rs", + "src_path": "/example_project/p1/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -40,7 +40,7 @@ } ], "features": {}, - "manifest_path": "/path/to/project/example_project/projects/p1/Cargo.toml", + "manifest_path": "/example_project/p1/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -56,9 +56,9 @@ "rust_version": null }, { - "name": "p3", + "name": "p2", "version": "0.1.0", - "id": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", + "id": "p2 0.1.0 (path+file:///example_project/p2)", "license": null, "license_file": null, "description": null, @@ -72,8 +72,8 @@ "crate_types": [ "lib" ], - "name": "p3", - "src_path": "/path/to/project/example_project/projects/p3/src/lib.rs", + "name": "p2", + "src_path": "/example_project/p2/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -81,7 +81,7 @@ } ], "features": {}, - "manifest_path": "/path/to/project/example_project/projects/p3/Cargo.toml", + "manifest_path": "/example_project/p2/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -98,25 +98,25 @@ } ], "workspace_members": [ - "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" + "p1 0.1.0 (path+file:///example_project/p1)" ], "workspace_default_members": [ - "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" + "p1 0.1.0 (path+file:///example_project/p1)" ], "resolve": { "nodes": [ { - "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", + "id": "p1 0.1.0 (path+file:///example_project/p1)", "dependencies": [ - "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)" + "p2 0.1.0 (path+file:///example_project/p2)" ], "deps": [ { - "name": "p3", - "pkg": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", + "name": "p2", + "pkg": "p2 0.1.0 (path+file:///example_project/p2)", "dep_kinds": [ { - "kind": "dev", + "kind": null, "target": null } ] @@ -125,16 +125,16 @@ "features": [] }, { - "id": "p3 0.1.0 (path+file:///path/to/project/example_project/projects/p3)", + "id": "p2 0.1.0 (path+file:///example_project/p2)", "dependencies": [], "deps": [], "features": [] } ], - "root": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" + "root": "p1 0.1.0 (path+file:///example_project/p1)" }, - "target_directory": "/path/to/project/example_project/projects/p1/target", + "target_directory": "/example_project/p1/target", "version": 1, - "workspace_root": "/path/to/project/example_project/projects/p1", + "workspace_root": "/example_project/p1", "metadata": null } \ No newline at end of file diff --git a/crates/project-model/test_data/deduplication_crate_graph_B.json b/crates/project-model/test_data/deduplication_crate_graph_B.json index 4f753db71bf0..6e67da7eae13 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_B.json +++ b/crates/project-model/test_data/deduplication_crate_graph_B.json @@ -1,83 +1,14 @@ { "packages": [ - { - "name": "p1", - "version": "0.1.0", - "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", - "license": null, - "license_file": null, - "description": null, - "source": null, - "dependencies": [ - { - "name": "p3", - "source": null, - "req": "*", - "kind": "dev", - "rename": null, - "optional": false, - "uses_default_features": true, - "features": [], - "target": null, - "registry": null, - "path": "/path/to/project/example_project/projects/p3" - } - ], - "targets": [ - { - "kind": [ - "lib" - ], - "crate_types": [ - "lib" - ], - "name": "p1", - "src_path": "/path/to/project/example_project/projects/p1/src/lib.rs", - "edition": "2021", - "doc": true, - "doctest": true, - "test": true - } - ], - "features": {}, - "manifest_path": "/path/to/project/example_project/projects/p1/Cargo.toml", - "metadata": null, - "publish": null, - "authors": [], - "categories": [], - "keywords": [], - "readme": null, - "repository": null, - "homepage": null, - "documentation": null, - "edition": "2021", - "links": null, - "default_run": null, - "rust_version": null - }, { "name": "p2", "version": "0.1.0", - "id": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)", + "id": "p2 0.1.0 (path+file:///example_project/p2)", "license": null, "license_file": null, "description": null, "source": null, - "dependencies": [ - { - "name": "p1", - "source": null, - "req": "*", - "kind": null, - "rename": null, - "optional": false, - "uses_default_features": true, - "features": [], - "target": null, - "registry": null, - "path": "/path/to/project/example_project/projects/p1" - } - ], + "dependencies": [], "targets": [ { "kind": [ @@ -87,7 +18,7 @@ "lib" ], "name": "p2", - "src_path": "/path/to/project/example_project/projects/p2/src/lib.rs", + "src_path": "/example_project/p2/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -95,7 +26,7 @@ } ], "features": {}, - "manifest_path": "/path/to/project/example_project/projects/p2/Cargo.toml", + "manifest_path": "/example_project/p2/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -112,43 +43,24 @@ } ], "workspace_members": [ - "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" + "p2 0.1.0 (path+file:///example_project/p2)" ], "workspace_default_members": [ - "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" + "p2 0.1.0 (path+file:///example_project/p2)" ], "resolve": { "nodes": [ { - "id": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", + "id": "p2 0.1.0 (path+file:///example_project/p2)", "dependencies": [], "deps": [], "features": [] - }, - { - "id": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)", - "dependencies": [ - "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)" - ], - "deps": [ - { - "name": "p1", - "pkg": "p1 0.1.0 (path+file:///path/to/project/example_project/projects/p1)", - "dep_kinds": [ - { - "kind": null, - "target": null - } - ] - } - ], - "features": [] } ], - "root": "p2 0.1.0 (path+file:///path/to/project/example_project/projects/p2)" + "root": "p2 0.1.0 (path+file:///example_project/p2)" }, - "target_directory": "/path/to/project/example_project/projects/p2/target", + "target_directory": "/example_project/p2/target", "version": 1, - "workspace_root": "/path/to/project/example_project/projects/p2", + "workspace_root": "/example_project/p2", "metadata": null } \ No newline at end of file From ba1b08080559fdaa714d9e58ce21f7f2030adc56 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 23 Nov 2023 12:34:38 +0100 Subject: [PATCH 7/7] Precede paths with $ROOT$ --- .../test_data/deduplication_crate_graph_A.json | 14 +++++++------- .../test_data/deduplication_crate_graph_B.json | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/project-model/test_data/deduplication_crate_graph_A.json b/crates/project-model/test_data/deduplication_crate_graph_A.json index 66bfcb2bf6ef..b0fb5845cef7 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_A.json +++ b/crates/project-model/test_data/deduplication_crate_graph_A.json @@ -20,7 +20,7 @@ "features": [], "target": null, "registry": null, - "path": "/example_project/p2" + "path": "$ROOT$example_project/p2" } ], "targets": [ @@ -32,7 +32,7 @@ "lib" ], "name": "p1", - "src_path": "/example_project/p1/src/lib.rs", + "src_path": "$ROOT$example_project/p1/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -40,7 +40,7 @@ } ], "features": {}, - "manifest_path": "/example_project/p1/Cargo.toml", + "manifest_path": "$ROOT$example_project/p1/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -73,7 +73,7 @@ "lib" ], "name": "p2", - "src_path": "/example_project/p2/src/lib.rs", + "src_path": "$ROOT$example_project/p2/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -81,7 +81,7 @@ } ], "features": {}, - "manifest_path": "/example_project/p2/Cargo.toml", + "manifest_path": "$ROOT$example_project/p2/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -133,8 +133,8 @@ ], "root": "p1 0.1.0 (path+file:///example_project/p1)" }, - "target_directory": "/example_project/p1/target", + "target_directory": "$ROOT$example_project/p1/target", "version": 1, - "workspace_root": "/example_project/p1", + "workspace_root": "$ROOT$example_project/p1", "metadata": null } \ No newline at end of file diff --git a/crates/project-model/test_data/deduplication_crate_graph_B.json b/crates/project-model/test_data/deduplication_crate_graph_B.json index 6e67da7eae13..b5d1e16e62e0 100644 --- a/crates/project-model/test_data/deduplication_crate_graph_B.json +++ b/crates/project-model/test_data/deduplication_crate_graph_B.json @@ -18,7 +18,7 @@ "lib" ], "name": "p2", - "src_path": "/example_project/p2/src/lib.rs", + "src_path": "$ROOT$example_project/p2/src/lib.rs", "edition": "2021", "doc": true, "doctest": true, @@ -26,7 +26,7 @@ } ], "features": {}, - "manifest_path": "/example_project/p2/Cargo.toml", + "manifest_path": "$ROOT$example_project/p2/Cargo.toml", "metadata": null, "publish": null, "authors": [], @@ -59,8 +59,8 @@ ], "root": "p2 0.1.0 (path+file:///example_project/p2)" }, - "target_directory": "/example_project/p2/target", + "target_directory": "$ROOT$example_project/p2/target", "version": 1, - "workspace_root": "/example_project/p2", + "workspace_root": "$ROOT$example_project/p2", "metadata": null } \ No newline at end of file