From 939ebb44544379d0ea483e7e8d5d061f04e6ed12 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 4 May 2023 14:20:52 +0200 Subject: [PATCH 1/2] Forbid canonicalization of paths and normalize all rust-project.json paths --- crates/paths/src/lib.rs | 5 ++-- crates/project-model/src/manifest_path.rs | 5 ++-- crates/project-model/src/project_json.rs | 31 ++++++++++------------- crates/project-model/src/workspace.rs | 1 - 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 083dfcf43d96..0cfbf1d9a168 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -166,9 +166,8 @@ impl AbsPath { AbsPathBuf::try_from(self.0.to_path_buf()).unwrap() } - /// Equivalent of [`Path::canonicalize`] for `AbsPath`. - pub fn canonicalize(&self) -> Result { - Ok(self.as_ref().canonicalize()?.try_into().unwrap()) + pub fn canonicalize(&self) -> ! { + panic!("We explicitly do not provide canonicalization API, as that is almost always a wrong solution, see #14430") } /// Equivalent of [`Path::strip_prefix`] for `AbsPath`. diff --git a/crates/project-model/src/manifest_path.rs b/crates/project-model/src/manifest_path.rs index d6754116e608..3f60e4dd92f4 100644 --- a/crates/project-model/src/manifest_path.rs +++ b/crates/project-model/src/manifest_path.rs @@ -35,9 +35,8 @@ impl ManifestPath { self.file.parent().unwrap() } - /// Equivalent of [`Path::canonicalize`] for `ManifestPath`. - pub fn canonicalize(&self) -> Result { - Ok((&**self).canonicalize()?.try_into().unwrap()) + pub fn canonicalize(&self) -> ! { + (&**self).canonicalize() } } diff --git a/crates/project-model/src/project_json.rs b/crates/project-model/src/project_json.rs index 4dc24c4d567f..ede2dc769e3c 100644 --- a/crates/project-model/src/project_json.rs +++ b/crates/project-model/src/project_json.rs @@ -49,13 +49,12 @@ //! user explores them belongs to that extension (it's totally valid to change //! rust-project.json over time via configuration request!) -use std::path::PathBuf; - use base_db::{CrateDisplayName, CrateId, CrateName, Dependency, Edition}; use la_arena::RawIdx; use paths::{AbsPath, AbsPathBuf}; use rustc_hash::FxHashMap; use serde::{de, Deserialize}; +use std::path::PathBuf; use crate::cfg_flag::CfgFlag; @@ -99,26 +98,24 @@ impl ProjectJson { /// * `data` - The parsed contents of `rust-project.json`, or project json that's passed via /// configuration. pub fn new(base: &AbsPath, data: ProjectJsonData) -> ProjectJson { + let absolutize = + |p| AbsPathBuf::try_from(p).unwrap_or_else(|path| base.join(&path)).normalize(); ProjectJson { - sysroot: data.sysroot.map(|it| base.join(it)), - sysroot_src: data.sysroot_src.map(|it| base.join(it)), + sysroot: data.sysroot.map(absolutize), + sysroot_src: data.sysroot_src.map(absolutize), project_root: base.to_path_buf(), crates: data .crates .into_iter() .map(|crate_data| { - let is_workspace_member = crate_data.is_workspace_member.unwrap_or_else(|| { - crate_data.root_module.is_relative() - && !crate_data.root_module.starts_with("..") - || crate_data.root_module.starts_with(base) - }); - let root_module = base.join(crate_data.root_module).normalize(); + let root_module = absolutize(crate_data.root_module); + let is_workspace_member = crate_data + .is_workspace_member + .unwrap_or_else(|| root_module.starts_with(base)); let (include, exclude) = match crate_data.source { Some(src) => { let absolutize = |dirs: Vec| { - dirs.into_iter() - .map(|it| base.join(it).normalize()) - .collect::>() + dirs.into_iter().map(absolutize).collect::>() }; (absolutize(src.include_dirs), absolutize(src.exclude_dirs)) } @@ -145,9 +142,7 @@ impl ProjectJson { cfg: crate_data.cfg, target: crate_data.target, env: crate_data.env, - proc_macro_dylib_path: crate_data - .proc_macro_dylib_path - .map(|it| base.join(it)), + proc_macro_dylib_path: crate_data.proc_macro_dylib_path.map(absolutize), is_workspace_member, include, exclude, @@ -155,7 +150,7 @@ impl ProjectJson { repository: crate_data.repository, } }) - .collect::>(), + .collect(), } } @@ -243,7 +238,7 @@ struct CrateSource { exclude_dirs: Vec, } -fn deserialize_crate_name<'de, D>(de: D) -> Result +fn deserialize_crate_name<'de, D>(de: D) -> std::result::Result where D: de::Deserializer<'de>, { diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index bfd2cab31fb8..4adaa6d86ee3 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -179,7 +179,6 @@ impl ProjectWorkspace { }; let res = match manifest { ProjectManifest::ProjectJson(project_json) => { - let project_json = project_json.canonicalize()?; let file = fs::read_to_string(&project_json).with_context(|| { format!("Failed to read json file {}", project_json.display()) })?; From f47caa666e0a2f7dd9ee9ac2535a2d26b301d4fb Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 13 May 2023 11:51:28 +0200 Subject: [PATCH 2/2] Add AbsPath::absolutize --- crates/paths/src/lib.rs | 5 +++++ crates/project-model/src/project_json.rs | 15 ++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 0cfbf1d9a168..e0c20a4143bd 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -140,6 +140,11 @@ impl AbsPath { self.0.parent().map(AbsPath::assert) } + /// Equivalent of [`Path::join`] for `AbsPath` with an additional normalize step afterwards. + pub fn absolutize(&self, path: impl AsRef) -> AbsPathBuf { + self.join(path).normalize() + } + /// Equivalent of [`Path::join`] for `AbsPath`. pub fn join(&self, path: impl AsRef) -> AbsPathBuf { self.as_ref().join(path).try_into().unwrap() diff --git a/crates/project-model/src/project_json.rs b/crates/project-model/src/project_json.rs index ede2dc769e3c..80897f7478cf 100644 --- a/crates/project-model/src/project_json.rs +++ b/crates/project-model/src/project_json.rs @@ -98,24 +98,23 @@ impl ProjectJson { /// * `data` - The parsed contents of `rust-project.json`, or project json that's passed via /// configuration. pub fn new(base: &AbsPath, data: ProjectJsonData) -> ProjectJson { - let absolutize = - |p| AbsPathBuf::try_from(p).unwrap_or_else(|path| base.join(&path)).normalize(); + let absolutize_on_base = |p| base.absolutize(p); ProjectJson { - sysroot: data.sysroot.map(absolutize), - sysroot_src: data.sysroot_src.map(absolutize), + sysroot: data.sysroot.map(absolutize_on_base), + sysroot_src: data.sysroot_src.map(absolutize_on_base), project_root: base.to_path_buf(), crates: data .crates .into_iter() .map(|crate_data| { - let root_module = absolutize(crate_data.root_module); + let root_module = absolutize_on_base(crate_data.root_module); let is_workspace_member = crate_data .is_workspace_member .unwrap_or_else(|| root_module.starts_with(base)); let (include, exclude) = match crate_data.source { Some(src) => { let absolutize = |dirs: Vec| { - dirs.into_iter().map(absolutize).collect::>() + dirs.into_iter().map(absolutize_on_base).collect::>() }; (absolutize(src.include_dirs), absolutize(src.exclude_dirs)) } @@ -142,7 +141,9 @@ impl ProjectJson { cfg: crate_data.cfg, target: crate_data.target, env: crate_data.env, - proc_macro_dylib_path: crate_data.proc_macro_dylib_path.map(absolutize), + proc_macro_dylib_path: crate_data + .proc_macro_dylib_path + .map(absolutize_on_base), is_workspace_member, include, exclude,