Auto merge of #14735 - Veykril:forbid-relative-json, r=Veykril

internal: Forbid canonicalization of paths and normalize all rust-project.json paths

Closes https://github.com/rust-lang/rust-analyzer/issues/14728
cc https://github.com/rust-lang/rust-analyzer/pull/14430

- Removes canonicalization (and forbids it from being used in a sense, clippy could help here again with its lint in the future)
- Normalizes all paths in rust-project.json which we weren't doing in some cases

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<Path>) -> AbsPathBuf {
+        self.join(path).normalize()
+    }
+
     /// Equivalent of [`Path::join`] for `AbsPath`.
     pub fn join(&self, path: impl AsRef<Path>) -> AbsPathBuf {
         self.as_ref().join(path).try_into().unwrap()
@@ -166,9 +171,8 @@ impl AbsPath {
         AbsPathBuf::try_from(self.0.to_path_buf()).unwrap()
     }
 
-    /// Equivalent of [`Path::canonicalize`] for `AbsPath`.
-    pub fn canonicalize(&self) -> Result<AbsPathBuf, std::io::Error> {
-        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`.

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<ManifestPath, std::io::Error> {
-        Ok((&**self).canonicalize()?.try_into().unwrap())
+    pub fn canonicalize(&self) -> ! {
+        (&**self).canonicalize()
     }
 }
 

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,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_on_base = |p| base.absolutize(p);
         ProjectJson {
-            sysroot: data.sysroot.map(|it| base.join(it)),
-            sysroot_src: data.sysroot_src.map(|it| base.join(it)),
+            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 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_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<PathBuf>| {
-                                dirs.into_iter()
-                                    .map(|it| base.join(it).normalize())
-                                    .collect::<Vec<_>>()
+                                dirs.into_iter().map(absolutize_on_base).collect::<Vec<_>>()
                             };
                             (absolutize(src.include_dirs), absolutize(src.exclude_dirs))
                         }
@@ -147,7 +143,7 @@ impl ProjectJson {
                         env: crate_data.env,
                         proc_macro_dylib_path: crate_data
                             .proc_macro_dylib_path
-                            .map(|it| base.join(it)),
+                            .map(absolutize_on_base),
                         is_workspace_member,
                         include,
                         exclude,
@@ -155,7 +151,7 @@ impl ProjectJson {
                         repository: crate_data.repository,
                     }
                 })
-                .collect::<Vec<_>>(),
+                .collect(),
         }
     }
 
@@ -243,7 +239,7 @@ struct CrateSource {
     exclude_dirs: Vec<PathBuf>,
 }
 
-fn deserialize_crate_name<'de, D>(de: D) -> Result<CrateName, D::Error>
+fn deserialize_crate_name<'de, D>(de: D) -> std::result::Result<CrateName, D::Error>
 where
     D: de::Deserializer<'de>,
 {

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())
                 })?;