Skip to content

Commit 5ff2b9c

Browse files
mkaputmkaput
authored
Fix all cases when std canonicalization was used instead of dunce (#695)
1 parent e8da195 commit 5ff2b9c

File tree

13 files changed

+58
-95
lines changed

13 files changed

+58
-95
lines changed

‎clippy.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
disallowed-methods = [
2+
{ path = "std::fs::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
3+
{ path = "std::path::Path::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
4+
{ path = "camino::Utf8Path::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
5+
{ path = "camino::Utf8Path::canonicalize_utf8", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
6+
{ path = "camino::Utf8PathBuf::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
7+
{ path = "camino::Utf8PathBuf::canonicalize_utf8", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
8+
]

‎scarb/src/core/manifest/toml_manifest.rs

+8-17
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use crate::core::package::PackageId;
2222
use crate::core::source::{GitReference, SourceId};
2323
use crate::core::{DependencyVersionReq, ManifestBuilder, ManifestCompilerConfig, PackageName};
2424
use crate::internal::fsx;
25-
use crate::internal::fsx::PathBufUtf8Ext;
2625
use crate::internal::serdex::{toml_merge, RelativeUtf8PathBuf};
2726
use crate::internal::to_version::ToVersion;
2827
use crate::{DEFAULT_SOURCE_PATH, MANIFEST_FILE_NAME};
@@ -621,10 +620,8 @@ impl TomlManifest {
621620
let source_path = target
622621
.source_path
623622
.clone()
624-
.map(|p| root.as_std_path().to_path_buf().join(p))
625-
.map(fsx::canonicalize)
626-
.transpose()?
627-
.map(|p| p.try_into_utf8())
623+
.map(|p| root.join_os(p))
624+
.map(fsx::canonicalize_utf8)
628625
.transpose()?
629626
.unwrap_or(default_source_path.to_path_buf());
630627

@@ -767,18 +764,12 @@ pub fn readme_for_package(
767764
fn abs_canonical_path(prefix: &Utf8Path, readme: Option<&Utf8Path>) -> Result<Option<Utf8PathBuf>> {
768765
match readme {
769766
None => Ok(None),
770-
Some(readme) => prefix
771-
.parent()
772-
.unwrap()
773-
.join(readme)
774-
.canonicalize_utf8()
775-
.map(Some)
776-
.with_context(|| {
777-
format!(
778-
"failed to find the readme at {}",
779-
prefix.parent().unwrap().join(readme)
780-
)
781-
}),
767+
Some(readme) => {
768+
let path = prefix.parent().unwrap().join(readme);
769+
let path = fsx::canonicalize_utf8(&path)
770+
.with_context(|| format!("failed to find the readme at {path}"))?;
771+
Ok(Some(path))
772+
}
782773
}
783774
}
784775

‎scarb/src/internal/fsx.rs

+7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ use anyhow::{anyhow, Context, Result};
88
use camino::{Utf8Path, Utf8PathBuf};
99

1010
/// Equivalent to [`fs::canonicalize`] with better error messages.
11+
///
12+
/// Uses [`dunce`] to generate more familiar paths on Windows.
1113
pub fn canonicalize(p: impl AsRef<Path>) -> Result<PathBuf> {
1214
return inner(p.as_ref());
1315

@@ -17,6 +19,11 @@ pub fn canonicalize(p: impl AsRef<Path>) -> Result<PathBuf> {
1719
}
1820
}
1921

22+
/// Equivalent to [`fs::canonicalize`], but for Utf-8 paths, with better error messages.
23+
pub fn canonicalize_utf8(p: impl AsRef<Path>) -> Result<Utf8PathBuf> {
24+
canonicalize(p)?.try_into_utf8()
25+
}
26+
2027
/// Equivalent to [`fs::create_dir_all`] with better error messages.
2128
pub fn create_dir_all(p: impl AsRef<Path>) -> Result<()> {
2229
return inner(p.as_ref());

‎scarb/src/internal/serdex.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use camino::{Utf8Path, Utf8PathBuf};
33
use serde::{Deserialize, Serialize};
44

55
use crate::internal::fsx;
6-
use crate::internal::fsx::PathUtf8Ext;
76

87
/// Merge two `toml::Value` serializable structs.
98
pub fn toml_merge<'de, T, S>(target: &T, source: &S) -> anyhow::Result<T>
@@ -32,7 +31,7 @@ pub struct RelativeUtf8PathBuf(Utf8PathBuf);
3231

3332
impl RelativeUtf8PathBuf {
3433
pub fn relative_to_directory(&self, root: &Utf8Path) -> Result<Utf8PathBuf> {
35-
fsx::canonicalize(root.join(&self.0))?.try_to_utf8()
34+
fsx::canonicalize_utf8(root.join(&self.0))
3635
}
3736

3837
pub fn relative_to_file(&self, file: &Utf8Path) -> Result<Utf8PathBuf> {

‎scarb/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//!
33
//! [cairo]: https://cairo-lang.org/
44
5+
#![deny(clippy::disallowed_methods)]
56
#![deny(rustdoc::broken_intra_doc_links)]
67
#![deny(rustdoc::private_intra_doc_links)]
78
#![warn(rust_2018_idioms)]

‎scarb/src/manifest_editor/add.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use url::Url;
77

88
use crate::core::{GitReference, PackageName};
99
use crate::internal::fsx;
10-
use crate::internal::fsx::PathBufUtf8Ext;
1110
use crate::sources::canonical_url::CanonicalUrl;
1211

1312
use super::tomlx::get_table_mut;
@@ -115,7 +114,7 @@ impl Dep {
115114
});
116115

117116
let source: Box<dyn Source> = if let Some(path) = op.path {
118-
let path = fsx::canonicalize(path)?.try_into_utf8()?;
117+
let path = fsx::canonicalize_utf8(path)?;
119118
let path = path_value(ctx.manifest_path, &path);
120119

121120
Box::new(PathSource { version, path })

‎scarb/src/manifest_editor/mod.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ pub use remove::RemoveDependency;
1212

1313
use crate::core::Config;
1414
use crate::internal::fsx;
15-
use crate::internal::fsx::PathBufUtf8Ext;
1615

1716
mod add;
1817
mod dep_id;
@@ -40,7 +39,7 @@ pub fn edit(
4039
ops: Vec<Box<dyn Op>>,
4140
opts: EditManifestOptions<'_>,
4241
) -> Result<()> {
43-
let manifest_path = fsx::canonicalize(manifest_path)?.try_into_utf8()?;
42+
let manifest_path = fsx::canonicalize_utf8(manifest_path)?;
4443

4544
let original_raw_manifest = fsx::read_to_string(&manifest_path)?;
4645
let mut doc = Document::from_str(&original_raw_manifest)

‎scarb/src/ops/manifest.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use camino::{Utf8Path, Utf8PathBuf};
55

66
use crate::core::manifest::TomlManifest;
77
use crate::internal::fsx;
8-
use crate::internal::fsx::{PathBufUtf8Ext, PathUtf8Ext};
8+
use crate::internal::fsx::PathBufUtf8Ext;
99
use crate::MANIFEST_FILE_NAME;
1010

1111
#[tracing::instrument(level = "debug")]
@@ -15,7 +15,7 @@ pub fn find_manifest_path(user_override: Option<&Utf8Path>) -> Result<Utf8PathBu
1515
.unwrap_or_else(|_| user_override.into())
1616
.try_into_utf8()?),
1717
None => {
18-
let pwd = fsx::canonicalize(env::current_dir()?)?.try_to_utf8()?;
18+
let pwd = fsx::canonicalize_utf8(env::current_dir()?)?;
1919
let accept_all = |_| Ok(true);
2020
let manifest_path = try_find_manifest_of_pwd(pwd.clone(), accept_all)?
2121
.unwrap_or_else(|| pwd.join(MANIFEST_FILE_NAME));

‎scarb/src/ops/new.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use itertools::Itertools;
55

66
use crate::core::{Config, PackageName};
77
use crate::internal::fsx;
8-
use crate::internal::fsx::PathBufUtf8Ext;
98
use crate::internal::restricted_names;
109
use crate::{ops, DEFAULT_SOURCE_PATH, DEFAULT_TARGET_DIR_NAME, MANIFEST_FILE_NAME};
1110

@@ -126,11 +125,7 @@ fn mk(
126125
// Create project directory in case we are called from `new` op.
127126
fsx::create_dir_all(&path)?;
128127

129-
let canonical_path = if let Ok(canonicalize_path) = fsx::canonicalize(&path) {
130-
canonicalize_path.try_into_utf8()?
131-
} else {
132-
path
133-
};
128+
let canonical_path = fsx::canonicalize_utf8(&path).unwrap_or(path);
134129

135130
init_vcs(&canonical_path, version_control)?;
136131
write_vcs_ignore(&canonical_path, config, version_control)?;

‎scarb/src/ops/workspace.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,7 @@ fn find_member_paths(
165165
// Look for manifest file, continuing if it does not exist.
166166
let path = path.join(MANIFEST_FILE_NAME);
167167
if path.is_file() {
168-
let path = fsx::canonicalize(path)?;
169-
let path = path.try_into_utf8()?;
170-
168+
let path = fsx::canonicalize_utf8(path)?;
171169
paths.push(path)
172170
} else {
173171
config.ui().warn(format!(

‎scarb/tests/metadata.rs

+15-53
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ use std::collections::BTreeMap;
22

33
use assert_fs::prelude::*;
44
use indoc::indoc;
5+
use serde_json::json;
6+
57
use scarb_metadata::{Cfg, ManifestMetadataBuilder, Metadata, PackageMetadata};
68
use scarb_test_support::command::{CommandExt, Scarb};
7-
use scarb_test_support::fsx::PathBufUtf8Ext;
9+
use scarb_test_support::fsx;
810
use scarb_test_support::project_builder::ProjectBuilder;
911
use scarb_test_support::workspace_builder::WorkspaceBuilder;
10-
use serde_json::json;
1112

1213
fn packages_by_name(meta: Metadata) -> BTreeMap<String, PackageMetadata> {
1314
meta.packages
@@ -285,10 +286,7 @@ fn manifest_targets_and_metadata() {
285286
.license(Some("MIT License".to_string()))
286287
.license_file(Some("./license.md".to_string()))
287288
.readme(
288-
t.join("README.md")
289-
.canonicalize()
290-
.unwrap()
291-
.try_into_utf8()
289+
fsx::canonicalize_utf8(t.join("README.md"))
292290
.unwrap()
293291
.into_string()
294292
)
@@ -617,10 +615,7 @@ fn infer_readme_simple() {
617615
.manifest_metadata
618616
.readme,
619617
Some(
620-
t.join("README")
621-
.canonicalize()
622-
.unwrap()
623-
.try_into_utf8()
618+
fsx::canonicalize_utf8(t.join("README"))
624619
.unwrap()
625620
.into_string()
626621
)
@@ -643,10 +638,7 @@ fn infer_readme_simple() {
643638
.manifest_metadata
644639
.readme,
645640
Some(
646-
t.join("README.txt")
647-
.canonicalize()
648-
.unwrap()
649-
.try_into_utf8()
641+
fsx::canonicalize_utf8(t.join("README.txt"))
650642
.unwrap()
651643
.into_string()
652644
)
@@ -669,10 +661,7 @@ fn infer_readme_simple() {
669661
.manifest_metadata
670662
.readme,
671663
Some(
672-
t.join("README.md")
673-
.canonicalize()
674-
.unwrap()
675-
.try_into_utf8()
664+
fsx::canonicalize_utf8(t.join("README.md"))
676665
.unwrap()
677666
.into_string()
678667
)
@@ -711,10 +700,7 @@ fn infer_readme_simple() {
711700
.manifest_metadata
712701
.readme,
713702
Some(
714-
t.join("a/b/c/MEREAD.md")
715-
.canonicalize()
716-
.unwrap()
717-
.try_into_utf8()
703+
fsx::canonicalize_utf8(t.join("a/b/c/MEREAD.md"))
718704
.unwrap()
719705
.into_string()
720706
)
@@ -797,10 +783,7 @@ fn infer_readme_simple_bool() {
797783
.manifest_metadata
798784
.readme,
799785
Some(
800-
t.join("README.md")
801-
.canonicalize()
802-
.unwrap()
803-
.try_into_utf8()
786+
fsx::canonicalize_utf8(t.join("README.md"))
804787
.unwrap()
805788
.into_string()
806789
)
@@ -986,68 +969,47 @@ fn infer_readme_workspace() {
986969
assert_eq!(
987970
packages.get("hello").unwrap().manifest_metadata.readme,
988971
Some(
989-
t.join("MEREAD.md")
990-
.canonicalize()
991-
.unwrap()
992-
.try_into_utf8()
972+
fsx::canonicalize_utf8(t.join("MEREAD.md"))
993973
.unwrap()
994974
.into_string()
995975
)
996976
);
997977
assert_eq!(
998978
packages.get("t7").unwrap().manifest_metadata.readme,
999979
Some(
1000-
t.join("MEREAD.md")
1001-
.canonicalize()
1002-
.unwrap()
1003-
.try_into_utf8()
980+
fsx::canonicalize_utf8(t.join("MEREAD.md"))
1004981
.unwrap()
1005982
.into_string()
1006983
)
1007984
);
1008985
assert_eq!(
1009986
packages.get("t1").unwrap().manifest_metadata.readme,
1010987
Some(
1011-
t.join("MEREAD.md")
1012-
.canonicalize()
1013-
.unwrap()
1014-
.try_into_utf8()
988+
fsx::canonicalize_utf8(t.join("MEREAD.md"))
1015989
.unwrap()
1016990
.into_string()
1017991
)
1018992
);
1019993
assert_eq!(
1020994
packages.get("t2").unwrap().manifest_metadata.readme,
1021995
Some(
1022-
t.child("t2")
1023-
.join("README.md")
1024-
.canonicalize()
1025-
.unwrap()
1026-
.try_into_utf8()
996+
fsx::canonicalize_utf8(t.child("t2").join("README.md"))
1027997
.unwrap()
1028998
.into_string()
1029999
)
10301000
);
10311001
assert_eq!(
10321002
packages.get("t3").unwrap().manifest_metadata.readme,
10331003
Some(
1034-
t.child("t3")
1035-
.join("README.txt")
1036-
.canonicalize()
1037-
.unwrap()
1038-
.try_into_utf8()
1004+
fsx::canonicalize_utf8(t.child("t3").join("README.txt"))
10391005
.unwrap()
10401006
.into_string()
10411007
)
10421008
);
10431009
assert_eq!(
10441010
packages.get("t4").unwrap().manifest_metadata.readme,
10451011
Some(
1046-
t.child("t4")
1047-
.join("TEST.txt")
1048-
.canonicalize()
1049-
.unwrap()
1050-
.try_into_utf8()
1012+
fsx::canonicalize_utf8(t.child("t4").join("TEST.txt"))
10511013
.unwrap()
10521014
.into_string()
10531015
)

‎scarb/tests/workspace.rs

+11-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
use assert_fs::fixture::{PathChild, PathCreateDir};
22
use assert_fs::TempDir;
33
use indoc::indoc;
4-
use scarb_metadata::Metadata;
54

5+
use scarb_metadata::Metadata;
66
use scarb_test_support::command::{CommandExt, Scarb};
7+
use scarb_test_support::fsx;
78
use scarb_test_support::project_builder::ProjectBuilder;
89
use scarb_test_support::workspace_builder::WorkspaceBuilder;
910

@@ -79,11 +80,14 @@ fn unify_target_dir() {
7980

8081
assert_eq!(root_metadata.target_dir, pkg_metadata.target_dir);
8182
assert_eq!(
82-
root_metadata
83-
.target_dir
84-
.unwrap()
85-
.to_owned()
86-
.into_std_path_buf(),
87-
t.child("target").canonicalize().unwrap()
83+
fsx::canonicalize(
84+
root_metadata
85+
.target_dir
86+
.unwrap()
87+
.to_owned()
88+
.into_std_path_buf()
89+
)
90+
.unwrap(),
91+
fsx::canonicalize(t.child("target")).unwrap()
8892
);
8993
}

‎utils/scarb-test-support/src/fsx.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use camino::Utf8Path;
77
use itertools::Itertools;
88
use serde::de::DeserializeOwned;
99

10-
pub use internal_fsx::{PathBufUtf8Ext, PathUtf8Ext};
10+
pub use internal_fsx::{canonicalize, canonicalize_utf8, PathBufUtf8Ext, PathUtf8Ext};
1111

1212
#[allow(unused)]
1313
#[path = "../../../scarb/src/internal/fsx.rs"]

0 commit comments

Comments
 (0)