fix: Updating settings should not clobber discovered projects

`linkedProjects` is owned by the user's configuration, so when users
update this setting, `linkedProjects` is reset. This is problematic when
`linkedProjects` also contains projects discovered with `discoverCommand`.

The buggy behaviour occurred when:

(1) The user configures `discoverCommand` and loads a Rust project.

(2) The user changes any setting in VS Code, so rust-analyzer receives
`workspace/didChangeConfiguration`.

(3) `handle_did_change_configuration` ultimately calls
`Client::apply_change_with_sink()`, which updates `config.user_config`
and discards any items we added in `linkedProjects`.

Instead, separate out `discovered_projects_from_filesystem` and
`discovered_projects_from_command` from user configuration, so user
settings cannot affect any type of discovered project.

This fixes the subtle issue mentioned here:
https://github.com/rust-lang/rust-analyzer/pull/17246#issuecomment-2185259122
This commit is contained in:
Wilfred Hughes 2024-09-05 12:36:38 -07:00
parent 124c748216
commit 3cf28f1fc6
4 changed files with 86 additions and 62 deletions

View file

@ -52,6 +52,15 @@ pub use crate::{
}; };
pub use cargo_metadata::Metadata; pub use cargo_metadata::Metadata;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ProjectJsonFromCommand {
/// The data describing this project, such as its dependencies.
pub data: ProjectJsonData,
/// The build system specific file that describes this project,
/// such as a `my-project/BUCK` file.
pub buildfile: AbsPathBuf,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)] #[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)]
pub enum ProjectManifest { pub enum ProjectManifest {
ProjectJson(ManifestPath), ProjectJson(ManifestPath),

View file

@ -66,6 +66,8 @@ pub struct ProjectJson {
/// e.g. `path/to/sysroot/lib/rustlib/src/rust` /// e.g. `path/to/sysroot/lib/rustlib/src/rust`
pub(crate) sysroot_src: Option<AbsPathBuf>, pub(crate) sysroot_src: Option<AbsPathBuf>,
project_root: AbsPathBuf, project_root: AbsPathBuf,
/// The path to the rust-project.json file. May be None if this
/// data was generated by the discoverConfig command.
manifest: Option<ManifestPath>, manifest: Option<ManifestPath>,
crates: Vec<Crate>, crates: Vec<Crate>,
/// Configuration for CLI commands. /// Configuration for CLI commands.

View file

@ -24,7 +24,8 @@ use ide_db::{
use itertools::Itertools; use itertools::Itertools;
use paths::{Utf8Path, Utf8PathBuf}; use paths::{Utf8Path, Utf8PathBuf};
use project_model::{ use project_model::{
CargoConfig, CargoFeatures, ProjectJson, ProjectJsonData, ProjectManifest, RustLibSource, CargoConfig, CargoFeatures, ProjectJson, ProjectJsonData, ProjectJsonFromCommand,
ProjectManifest, RustLibSource,
}; };
use rustc_hash::{FxHashMap, FxHashSet}; use rustc_hash::{FxHashMap, FxHashSet};
use semver::Version; use semver::Version;
@ -761,7 +762,13 @@ enum RatomlFile {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct Config { pub struct Config {
discovered_projects: Vec<ProjectManifest>, /// Projects that have a Cargo.toml or a rust-project.json in a
/// parent directory, so we can discover them by walking the
/// file system.
discovered_projects_from_filesystem: Vec<ProjectManifest>,
/// Projects whose configuration was generated by a command
/// configured in discoverConfig.
discovered_projects_from_command: Vec<ProjectJsonFromCommand>,
/// The workspace roots as registered by the LSP client /// The workspace roots as registered by the LSP client
workspace_roots: Vec<AbsPathBuf>, workspace_roots: Vec<AbsPathBuf>,
caps: ClientCapabilities, caps: ClientCapabilities,
@ -1054,19 +1061,19 @@ impl Config {
(config, e, should_update) (config, e, should_update)
} }
pub fn add_linked_projects(&mut self, data: ProjectJsonData, buildfile: AbsPathBuf) { pub fn add_discovered_project_from_command(
let linked_projects = &mut self.client_config.0.global.linkedProjects; &mut self,
data: ProjectJsonData,
buildfile: AbsPathBuf,
) {
for proj in self.discovered_projects_from_command.iter_mut() {
if proj.buildfile == buildfile {
proj.data = data;
return;
}
}
let new_project = ManifestOrProjectJson::DiscoveredProjectJson { data, buildfile }; self.discovered_projects_from_command.push(ProjectJsonFromCommand { data, buildfile });
match linked_projects {
Some(projects) => {
match projects.iter_mut().find(|p| p.manifest() == new_project.manifest()) {
Some(p) => *p = new_project,
None => projects.push(new_project),
}
}
None => *linked_projects = Some(vec![new_project]),
}
} }
} }
@ -1344,7 +1351,8 @@ impl Config {
Config { Config {
caps: ClientCapabilities::new(caps), caps: ClientCapabilities::new(caps),
discovered_projects: Vec::new(), discovered_projects_from_filesystem: Vec::new(),
discovered_projects_from_command: Vec::new(),
root_path, root_path,
snippets: Default::default(), snippets: Default::default(),
workspace_roots, workspace_roots,
@ -1365,7 +1373,7 @@ impl Config {
if discovered.is_empty() { if discovered.is_empty() {
tracing::error!("failed to find any projects in {:?}", &self.workspace_roots); tracing::error!("failed to find any projects in {:?}", &self.workspace_roots);
} }
self.discovered_projects = discovered; self.discovered_projects_from_filesystem = discovered;
} }
pub fn remove_workspace(&mut self, path: &AbsPath) { pub fn remove_workspace(&mut self, path: &AbsPath) {
@ -1687,21 +1695,40 @@ impl Config {
self.workspace_discoverConfig().as_ref() self.workspace_discoverConfig().as_ref()
} }
pub fn linked_or_discovered_projects(&self) -> Vec<LinkedProject> { fn discovered_projects(&self) -> Vec<ManifestOrProjectJson> {
match self.linkedProjects().as_slice() {
[] => {
let exclude_dirs: Vec<_> = let exclude_dirs: Vec<_> =
self.files_excludeDirs().iter().map(|p| self.root_path.join(p)).collect(); self.files_excludeDirs().iter().map(|p| self.root_path.join(p)).collect();
self.discovered_projects
.iter() let mut projects = vec![];
.filter(|project| { for fs_proj in &self.discovered_projects_from_filesystem {
!exclude_dirs.iter().any(|p| project.manifest_path().starts_with(p)) let manifest_path = fs_proj.manifest_path();
}) if exclude_dirs.iter().any(|p| manifest_path.starts_with(p)) {
.cloned() continue;
.map(LinkedProject::from)
.collect()
} }
linked_projects => linked_projects
let buf: Utf8PathBuf = manifest_path.to_path_buf().into();
projects.push(ManifestOrProjectJson::Manifest(buf));
}
for dis_proj in &self.discovered_projects_from_command {
projects.push(ManifestOrProjectJson::DiscoveredProjectJson {
data: dis_proj.data.clone(),
buildfile: dis_proj.buildfile.clone(),
});
}
projects
}
pub fn linked_or_discovered_projects(&self) -> Vec<LinkedProject> {
let linked_projects = self.linkedProjects();
let projects = if linked_projects.is_empty() {
self.discovered_projects()
} else {
linked_projects.clone()
};
projects
.iter() .iter()
.filter_map(|linked_project| match linked_project { .filter_map(|linked_project| match linked_project {
ManifestOrProjectJson::Manifest(it) => { ManifestOrProjectJson::Manifest(it) => {
@ -1712,8 +1739,7 @@ impl Config {
.map(Into::into) .map(Into::into)
} }
ManifestOrProjectJson::DiscoveredProjectJson { data, buildfile } => { ManifestOrProjectJson::DiscoveredProjectJson { data, buildfile } => {
let root_path = let root_path = buildfile.parent().expect("Unable to get parent of buildfile");
buildfile.parent().expect("Unable to get parent of buildfile");
Some(ProjectJson::new(None, root_path, data.clone()).into()) Some(ProjectJson::new(None, root_path, data.clone()).into())
} }
@ -1721,8 +1747,7 @@ impl Config {
Some(ProjectJson::new(None, &self.root_path, it.clone()).into()) Some(ProjectJson::new(None, &self.root_path, it.clone()).into())
} }
}) })
.collect(), .collect()
}
} }
pub fn prefill_caches(&self) -> bool { pub fn prefill_caches(&self) -> bool {
@ -2282,18 +2307,6 @@ where
se.serialize_str(path.as_str()) se.serialize_str(path.as_str())
} }
impl ManifestOrProjectJson {
fn manifest(&self) -> Option<&Utf8Path> {
match self {
ManifestOrProjectJson::Manifest(manifest) => Some(manifest),
ManifestOrProjectJson::DiscoveredProjectJson { buildfile, .. } => {
Some(buildfile.as_ref())
}
ManifestOrProjectJson::ProjectJson(_) => None,
}
}
}
#[derive(Serialize, Deserialize, Debug, Clone)] #[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")] #[serde(rename_all = "snake_case")]
enum ExprFillDefaultDef { enum ExprFillDefaultDef {

View file

@ -875,7 +875,7 @@ impl GlobalState {
self.discover_workspace_queue.op_completed(()); self.discover_workspace_queue.op_completed(());
let mut config = Config::clone(&*self.config); let mut config = Config::clone(&*self.config);
config.add_linked_projects(project, buildfile); config.add_discovered_project_from_command(project, buildfile);
self.update_configuration(config); self.update_configuration(config);
} }
DiscoverProjectMessage::Progress { message } => { DiscoverProjectMessage::Progress { message } => {