Skip to content

Commit

Permalink
Auto merge of rust-lang#14735 - Veykril:forbid-relative-json, r=Veykril
Browse files Browse the repository at this point in the history
internal: Forbid canonicalization of paths and normalize all rust-project.json paths

Closes rust-lang/rust-analyzer#14728
cc rust-lang/rust-analyzer#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
  • Loading branch information
bors committed May 13, 2023
2 parents e9f9bc2 + f47caa6 commit a7944a9
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 23 deletions.
10 changes: 7 additions & 3 deletions crates/paths/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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`.
Expand Down
5 changes: 2 additions & 3 deletions crates/project-model/src/manifest_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
28 changes: 12 additions & 16 deletions crates/project-model/src/project_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -147,15 +143,15 @@ 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,
is_proc_macro: crate_data.is_proc_macro,
repository: crate_data.repository,
}
})
.collect::<Vec<_>>(),
.collect(),
}
}

Expand Down Expand Up @@ -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>,
{
Expand Down
1 change: 0 additions & 1 deletion crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})?;
Expand Down

0 comments on commit a7944a9

Please sign in to comment.