Newtype CargoTomlPath

When reading code, it is not always clear, what path is Cargo.toml
path, and what path is directory path.

Author: stepancheg

crate_universe/src/cli/generate.rs

@@ -13,7 +13,7 @@ use clap::Parser;
 use crate::config::Config;
 use crate::context::Context;
 use crate::lockfile::{lock_context, write_lockfile};
-use crate::metadata::{load_metadata, Annotations, Cargo, SourceAnnotation};
+use crate::metadata::{load_metadata, Annotations, Cargo, CargoTomlPath, SourceAnnotation};
 use crate::rendering::{write_outputs, Renderer};
 use crate::splicing::SplicingManifest;
 use crate::utils::normalize_cargo_file_paths;
@@ -236,7 +236,7 @@ fn update_cargo_lockfile(path: &Path, cargo_lockfile: Lockfile) -> Result<()> {
 fn write_paths_to_track<
     'a,
     SourceAnnotations: Iterator<Item = &'a SourceAnnotation>,
-    Paths: Iterator<Item = Utf8PathBuf>,
+    Paths: Iterator<Item = CargoTomlPath>,
     UnusedPatches: Iterator<Item = &'a cargo_lock::Dependency>,
 >(
     output_file: &Path,
@@ -248,7 +248,7 @@ fn write_paths_to_track<
     let source_annotation_manifests: BTreeSet<_> = source_annotations
         .filter_map(|v| {
             if let SourceAnnotation::Path { path } = v {
-                Some(path.join("Cargo.toml"))
+                Some(CargoTomlPath::for_dir(path))
             } else {
                 None
             }

crate_universe/src/cli/splice.rs

@@ -99,7 +99,7 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
 
     let resolver_data = TreeResolver::new(cargo.clone())
         .generate(
-            manifest_path.as_path_buf(),
+            manifest_path.cargo_toml_path(),
             &config.supported_platform_triples,
         )
         .context("Failed to generate features")?;
@@ -108,8 +108,8 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
         &cargo,
         &cargo_lockfile,
         resolver_data,
-        manifest_path.as_path_buf(),
-        manifest_path.as_path_buf(),
+        manifest_path.cargo_toml_path(),
+        manifest_path.cargo_toml_path(),
     )
     .context("Failed to write registry URLs and feature map")?;
 
@@ -119,19 +119,10 @@ pub fn splice(opt: SpliceOptions) -> Result<()> {
     let (cargo_metadata, _) = Generator::new()
         .with_cargo(cargo)
         .with_rustc(opt.rustc)
-        .generate(manifest_path.as_path_buf())
+        .generate(manifest_path.cargo_toml_path())
         .context("Failed to generate cargo metadata")?;
 
-    let cargo_lockfile_path = manifest_path
-        .as_path_buf()
-        .parent()
-        .with_context(|| {
-            format!(
-                "The path {} is expected to have a parent directory",
-                manifest_path.as_path_buf()
-            )
-        })?
-        .join("Cargo.lock");
+    let cargo_lockfile_path = manifest_path.cargo_toml_path().parent().join("Cargo.lock");
 
     // Generate the consumable outputs of the splicing process
     std::fs::create_dir_all(&output_dir)

crate_universe/src/cli/vendor.rs

@@ -227,7 +227,7 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
     let config = Config::try_from_path(&opt.config)?;
 
     let resolver_data = TreeResolver::new(cargo.clone()).generate(
-        manifest_path.as_path_buf(),
+        manifest_path.cargo_toml_path(),
         &config.supported_platform_triples,
     )?;
 
@@ -236,15 +236,15 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
         &cargo,
         &cargo_lockfile,
         resolver_data,
-        manifest_path.as_path_buf(),
-        manifest_path.as_path_buf(),
+        manifest_path.cargo_toml_path(),
+        manifest_path.cargo_toml_path(),
     )?;
 
     // Write metadata to the workspace for future reuse
     let (cargo_metadata, cargo_lockfile) = Generator::new()
         .with_cargo(cargo.clone())
         .with_rustc(opt.rustc.clone())
-        .generate(manifest_path.as_path_buf())?;
+        .generate(manifest_path.cargo_toml_path())?;
 
     // Annotate metadata
     let annotations = Annotations::new(
@@ -281,7 +281,7 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
 
     if matches!(config.rendering.vendor_mode, Some(VendorMode::Local)) {
         VendorGenerator::new(cargo, opt.rustc.clone())
-            .generate(manifest_path.as_path_buf(), &vendor_dir)
+            .generate(manifest_path.cargo_toml_path(), &vendor_dir)
             .context("Failed to vendor dependencies")?;
     }
 

crate_universe/src/metadata.rs

@@ -1,6 +1,7 @@
 //! Tools for gathering various kinds of metadata (Cargo.lock, Cargo metadata, Crate Index info).
 
 mod cargo_bin;
+mod cargo_toml_path;
 mod cargo_tree_resolver;
 mod dependency;
 mod metadata_annotation;
@@ -12,12 +13,12 @@ use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 use anyhow::{bail, Context, Result};
-use camino::Utf8Path;
 use cargo_lock::Lockfile as CargoLockfile;
 use cargo_metadata::Metadata as CargoMetadata;
 use tracing::debug;
 
 pub(crate) use self::cargo_bin::*;
+pub(crate) use self::cargo_toml_path::*;
 pub(crate) use self::cargo_tree_resolver::*;
 pub(crate) use self::dependency::*;
 pub(crate) use self::metadata_annotation::*;
@@ -190,13 +191,13 @@ impl LockGenerator {
     #[tracing::instrument(name = "LockGenerator::generate", skip_all)]
     pub(crate) fn generate(
         &self,
-        manifest_path: &Utf8Path,
+        manifest_path: &CargoTomlPath,
         existing_lock: &Option<PathBuf>,
         update_request: &Option<CargoUpdateRequest>,
     ) -> Result<cargo_lock::Lockfile> {
         debug!("Generating Cargo Lockfile for {}", manifest_path);
 
-        let manifest_dir = manifest_path.parent().unwrap();
+        let manifest_dir = manifest_path.parent();
         let generated_lockfile_path = manifest_dir.join("Cargo.lock");
 
         if let Some(lock) = existing_lock {
@@ -304,9 +305,9 @@ impl VendorGenerator {
         }
     }
     #[tracing::instrument(name = "VendorGenerator::generate", skip_all)]
-    pub(crate) fn generate(&self, manifest_path: &Utf8Path, output_dir: &Path) -> Result<()> {
+    pub(crate) fn generate(&self, manifest_path: &CargoTomlPath, output_dir: &Path) -> Result<()> {
         debug!("Vendoring {} to {}", manifest_path, output_dir.display());
-        let manifest_dir = manifest_path.parent().unwrap();
+        let manifest_dir = manifest_path.parent();
 
         // Simply invoke `cargo generate-lockfile`
         let output = self

crate_universe/src/metadata/cargo_toml_path.rs

@@ -0,0 +1,60 @@
+use anyhow::{anyhow, Result};
+use camino::{Utf8Path, Utf8PathBuf};
+use serde::{Deserialize, Serialize};
+use std::fmt;
+use std::fmt::Display;
+use std::path::Path;
+
+/// Path that is know to end with `/Cargo.toml`.
+#[derive(Eq, PartialEq, Debug, Clone, Hash, Ord, PartialOrd, Serialize, Deserialize)]
+#[serde(transparent)]
+pub(crate) struct CargoTomlPath(Utf8PathBuf);
+
+impl Display for CargoTomlPath {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        Display::fmt(&self.0, f)
+    }
+}
+
+impl CargoTomlPath {
+    pub(crate) fn unchecked_new(path: impl Into<Utf8PathBuf>) -> Self {
+        CargoTomlPath(path.into())
+    }
+
+    pub(crate) fn new(path: impl Into<Utf8PathBuf>) -> Result<Self> {
+        let path = path.into();
+        if !path.ends_with("Cargo.toml") {
+            return Err(anyhow!("Path does not end with Cargo.toml: {path}"));
+        }
+        Ok(Self::unchecked_new(path))
+    }
+
+    pub(crate) fn for_dir(dir: impl Into<Utf8PathBuf>) -> Self {
+        let mut path = dir.into();
+        path.push("Cargo.toml");
+        CargoTomlPath::unchecked_new(path)
+    }
+
+    /// Directory of `Cargo.toml` path.
+    pub(crate) fn parent(&self) -> &Utf8Path {
+        self.0.parent().expect("Validated in constructor")
+    }
+
+    pub(crate) fn ancestors(&self) -> impl Iterator<Item = &Utf8Path> {
+        self.0.ancestors()
+    }
+
+    pub(crate) fn as_path(&self) -> &Utf8Path {
+        self.0.as_path()
+    }
+
+    pub(crate) fn as_std_path(&self) -> &Path {
+        self.0.as_std_path()
+    }
+}
+
+impl AsRef<Path> for CargoTomlPath {
+    fn as_ref(&self) -> &Path {
+        self.0.as_ref()
+    }
+}

crate_universe/src/metadata/cargo_tree_resolver.rs

@@ -6,14 +6,14 @@ use std::path::{Path, PathBuf};
 use std::process::Child;
 
 use anyhow::{anyhow, bail, Context, Result};
-use camino::Utf8Path;
 use semver::Version;
 use serde::{Deserialize, Serialize};
 use tracing::{debug, trace};
 use url::Url;
 
 use crate::config::CrateId;
 use crate::metadata::cargo_bin::Cargo;
+use crate::metadata::CargoTomlPath;
 use crate::select::{Select, SelectableScalar};
 use crate::utils::symlink::symlink;
 use crate::utils::target_triple::TargetTriple;
@@ -307,7 +307,7 @@ impl TreeResolver {
     #[tracing::instrument(name = "TreeResolver::generate", skip_all)]
     pub(crate) fn generate(
         &self,
-        pristine_manifest_path: &Utf8Path,
+        pristine_manifest_path: &CargoTomlPath,
         target_triples: &BTreeSet<TargetTriple>,
     ) -> Result<TreeResolverMetadata> {
         debug!(
@@ -450,14 +450,14 @@ impl TreeResolver {
     // and if we don't have this fake root injection, cross-compiling from Darwin to Linux won't work because features don't get correctly resolved for the exec=darwin case.
     fn copy_project_with_explicit_deps_on_all_transitive_proc_macros(
         &self,
-        pristine_manifest_path: &Utf8Path,
+        pristine_manifest_path: &CargoTomlPath,
         output_dir: &Path,
     ) -> Result<PathBuf> {
         if !output_dir.exists() {
             std::fs::create_dir_all(output_dir)?;
         }
 
-        let pristine_root = pristine_manifest_path.parent().unwrap();
+        let pristine_root = pristine_manifest_path.parent();
         for file in std::fs::read_dir(pristine_root).context("Failed to read dir")? {
             let source_path = file?.path();
             let file_name = source_path.file_name().unwrap();

crate_universe/src/metadata/workspace_discoverer.rs

@@ -1,23 +1,24 @@
 use std::collections::{BTreeMap, BTreeSet};
 
+use crate::metadata::CargoTomlPath;
 use anyhow::{anyhow, bail, Context, Result};
-use camino::{Utf8Path, Utf8PathBuf};
+use camino::Utf8Path;
 use cargo_toml::Manifest;
 
 /// A description of Cargo.toml files and how they are related in workspaces.
 /// All `Utf8PathBuf` values are paths of Cargo.toml files.
 #[derive(Debug, PartialEq)]
 pub(crate) struct DiscoveredWorkspaces {
-    workspaces_to_members: BTreeMap<Utf8PathBuf, BTreeSet<Utf8PathBuf>>,
-    non_workspaces: BTreeSet<Utf8PathBuf>,
+    workspaces_to_members: BTreeMap<CargoTomlPath, BTreeSet<CargoTomlPath>>,
+    non_workspaces: BTreeSet<CargoTomlPath>,
 }
 
 impl DiscoveredWorkspaces {
-    pub(crate) fn workspaces(&self) -> BTreeSet<Utf8PathBuf> {
+    pub(crate) fn workspaces(&self) -> BTreeSet<CargoTomlPath> {
         self.workspaces_to_members.keys().cloned().collect()
     }
 
-    pub(crate) fn all_workspaces_and_members(&self) -> BTreeSet<Utf8PathBuf> {
+    pub(crate) fn all_workspaces_and_members(&self) -> BTreeSet<CargoTomlPath> {
         self.workspaces_to_members
             .keys()
             .chain(self.workspaces_to_members.values().flatten())
@@ -27,8 +28,8 @@ impl DiscoveredWorkspaces {
 }
 
 pub(crate) fn discover_workspaces(
-    cargo_toml_paths: BTreeSet<Utf8PathBuf>,
-    known_manifests: &BTreeMap<Utf8PathBuf, Manifest>,
+    cargo_toml_paths: BTreeSet<CargoTomlPath>,
+    known_manifests: &BTreeMap<CargoTomlPath, Manifest>,
 ) -> Result<DiscoveredWorkspaces> {
     let mut manifest_cache = ManifestCache {
         cache: BTreeMap::new(),
@@ -38,7 +39,7 @@ pub(crate) fn discover_workspaces(
 }
 
 fn discover_workspaces_with_cache(
-    cargo_toml_paths: BTreeSet<Utf8PathBuf>,
+    cargo_toml_paths: BTreeSet<CargoTomlPath>,
     manifest_cache: &mut ManifestCache,
 ) -> Result<DiscoveredWorkspaces> {
     let mut discovered_workspaces = DiscoveredWorkspaces {
@@ -85,7 +86,7 @@ fn discover_workspaces_with_cache(
             })
             .transpose()?;
 
-        'per_child: for entry in walkdir::WalkDir::new(workspace_path.parent().unwrap())
+        'per_child: for entry in walkdir::WalkDir::new(workspace_path.parent())
             .follow_links(false)
             .follow_root_links(false)
             .into_iter()
@@ -117,6 +118,7 @@ fn discover_workspaces_with_cache(
             let child_path = Utf8Path::from_path(entry.path())
                 .ok_or_else(|| anyhow!("Failed to parse {:?} as UTF-8", entry.path()))?
                 .to_path_buf();
+            let child_path = CargoTomlPath::unchecked_new(child_path);
             if child_path == workspace_path {
                 continue;
             }
@@ -136,7 +138,7 @@ fn discover_workspaces_with_cache(
                             )
                         })?;
                     actual_workspace_path =
-                        child_path.parent().unwrap().join(explicit_workspace_path);
+                        CargoTomlPath::new(child_path.parent().join(explicit_workspace_path))?;
                 }
             }
             if !discovered_workspaces
@@ -146,10 +148,8 @@ fn discover_workspaces_with_cache(
                 bail!("Found manifest at {} which is a member of the workspace at {} which isn't included in the crates_universe", child_path, actual_workspace_path);
             }
 
-            let dir_relative_to_workspace_dir = child_path
-                .parent()
-                .unwrap()
-                .strip_prefix(workspace_path.parent().unwrap());
+            let dir_relative_to_workspace_dir =
+                child_path.parent().strip_prefix(workspace_path.parent());
 
             if let Ok(dir_relative_to_workspace_dir) = dir_relative_to_workspace_dir {
                 use itertools::Itertools;
@@ -201,11 +201,11 @@ fn discover_workspaces_with_cache(
 }
 
 fn discover_workspace_parent(
-    cargo_toml_path: &Utf8PathBuf,
+    cargo_toml_path: &CargoTomlPath,
     manifest_cache: &mut ManifestCache,
-) -> Option<Utf8PathBuf> {
+) -> Option<CargoTomlPath> {
     for parent_dir in cargo_toml_path.ancestors().skip(1) {
-        let maybe_cargo_toml_path = parent_dir.join("Cargo.toml");
+        let maybe_cargo_toml_path = CargoTomlPath::for_dir(parent_dir);
         let maybe_manifest = manifest_cache.get(&maybe_cargo_toml_path);
         if let Some(manifest) = maybe_manifest {
             if manifest.workspace.is_some() {
@@ -217,12 +217,12 @@ fn discover_workspace_parent(
 }
 
 struct ManifestCache<'a> {
-    cache: BTreeMap<Utf8PathBuf, Option<Manifest>>,
-    known_manifests: &'a BTreeMap<Utf8PathBuf, Manifest>,
+    cache: BTreeMap<CargoTomlPath, Option<Manifest>>,
+    known_manifests: &'a BTreeMap<CargoTomlPath, Manifest>,
 }
 
 impl ManifestCache<'_> {
-    fn get(&mut self, path: &Utf8PathBuf) -> Option<Manifest> {
+    fn get(&mut self, path: &CargoTomlPath) -> Option<Manifest> {
         if let Some(manifest) = self.known_manifests.get(path) {
             return Some(manifest.clone());
         }
@@ -417,7 +417,7 @@ mod test {
     fn ws1_discovered_workspaces(root_dir: &Utf8Path) -> DiscoveredWorkspaces {
         let mut workspaces_to_members = BTreeMap::new();
         workspaces_to_members.insert(
-            root_dir.join("ws1").join("Cargo.toml"),
+            CargoTomlPath::for_dir(root_dir.join("ws1")),
             BTreeSet::from([
                 root_dir.join("ws1").join("ws1c1").join("Cargo.toml"),
                 root_dir