Merge pull request #233 from nitkach/refactoring

Type constraint for paths and IntoUtf8 trait

Author: xFrednet

Cargo.lock

@@ -29,6 +29,7 @@ bumpalo            = "3.12"
 camino             = { version = "1.1", features = ["serde1"] }
 cargo_metadata     = "0.17"
 clap               = { version = "4.0", features = ["string", "derive"] }
+expect-test        = "1.4"
 itertools          = "0.11"
 libloading         = "0.8.0"
 miette             = { version = "5.9", features = ["fancy-no-backtrace"] }

Cargo.toml

@@ -34,3 +34,6 @@ tracing            = { workspace = true }
 tracing-error      = { workspace = true }
 tracing-subscriber = { workspace = true, features = ["env-filter"] }
 yansi              = { workspace = true }
+
+[dev-dependencies]
+expect-test = { workspace = true }

cargo-marker/Cargo.toml

@@ -12,9 +12,10 @@ use crate::observability::prelude::*;
 use std::{
     collections::HashMap,
     ffi::{OsStr, OsString},
-    path::PathBuf,
 };
 
+use camino::Utf8PathBuf;
+
 pub mod cargo;
 pub mod driver;
 pub mod lints;
@@ -31,7 +32,7 @@ pub struct Config {
     /// This should generally be used as a base path for everything. Notable
     /// exceptions can be the installation of a driver or the compilation of
     /// a lint for uitests.
-    pub marker_dir: PathBuf,
+    pub marker_dir: Utf8PathBuf,
     /// The list of lints.
     pub lints: HashMap<String, LintDependencyEntry>,
     /// Additional flags, which should be passed to rustc during the compilation
@@ -53,11 +54,11 @@ impl Config {
         })
     }
 
-    fn markers_target_dir(&self) -> PathBuf {
+    fn markers_target_dir(&self) -> Utf8PathBuf {
         self.marker_dir.join("target")
     }
 
-    fn lint_crate_dir(&self) -> PathBuf {
+    fn lint_crate_dir(&self) -> Utf8PathBuf {
         self.marker_dir.join("lints")
     }
 }

cargo-marker/src/backend.rs

@@ -2,8 +2,10 @@ use super::toolchain::{get_toolchain_folder, rustup_which, Toolchain};
 use crate::error::prelude::*;
 use crate::observability::display::print_stage;
 use crate::observability::prelude::*;
+use crate::utils::utf8::IntoUtf8;
 use crate::{utils::is_local_driver, Result};
-use std::{path::Path, process::Command};
+use camino::Utf8Path;
+use std::process::Command;
 use yansi::Paint;
 
 pub fn marker_driver_bin_name() -> String {
@@ -28,7 +30,7 @@ pub struct DriverVersionInfo {
 }
 
 impl DriverVersionInfo {
-    pub fn try_from_toolchain(toolchain: &Toolchain, manifest: &Path) -> Result<DriverVersionInfo> {
+    pub fn try_from_toolchain(toolchain: &Toolchain, manifest: &Utf8Path) -> Result<DriverVersionInfo> {
         // The driver has to be invoked via cargo, to ensure that the libraries
         // are correctly linked. Toolchains are truly fun...
         let output = toolchain
@@ -50,7 +52,7 @@ impl DriverVersionInfo {
             ));
         }
 
-        let info = String::from_utf8(output.stdout).context(|| "Failed to parse the driver metadata as UTF8")?;
+        let info = output.stdout.into_utf8()?;
 
         let fields = ["toolchain", "driver", "marker-api"];
 

cargo-marker/src/backend/driver.rs

@@ -1,6 +1,6 @@
 use super::Config;
 use crate::error::prelude::*;
-use std::path::PathBuf;
+use camino::Utf8PathBuf;
 
 mod build;
 mod fetch;
@@ -14,7 +14,7 @@ pub struct LintCrateSource {
     /// that will be used to construct the dynamic library.
     name: String,
     /// The absolute path to the manifest of this lint crate
-    manifest: PathBuf,
+    manifest: Utf8PathBuf,
 }
 
 /// The information of a compiled lint crate.
@@ -23,7 +23,7 @@ pub struct LintCrate {
     /// The name of the crate
     pub name: String,
     /// The absolute path of the compiled crate, as a dynamic library.
-    pub file: PathBuf,
+    pub file: Utf8PathBuf,
 }
 
 /// This function fetches and builds all lints specified in the given [`Config`]

cargo-marker/src/backend/lints.rs

@@ -2,8 +2,10 @@ use super::{LintCrate, LintCrateSource};
 use crate::backend::Config;
 use crate::error::prelude::*;
 use crate::observability::prelude::*;
+use crate::utils::utf8::IntoUtf8;
+use camino::Utf8Path;
 use itertools::Itertools;
-use std::{collections::HashSet, ffi::OsStr, path::Path};
+use std::{collections::HashSet, ffi::OsStr};
 use yansi::Paint;
 
 #[cfg(target_os = "linux")]
@@ -45,15 +47,15 @@ pub fn build_lints(sources: &[LintCrateSource], config: &Config) -> Result<Vec<L
 
     // Build lint crates and find the output of those builds
     let mut found_paths = HashSet::new();
-    let ending = OsStr::new(DYNAMIC_LIB_FILE_ENDING);
+    let ending = DYNAMIC_LIB_FILE_ENDING;
     let mut lints = Vec::with_capacity(sources.len());
 
     for lint_src in sources {
         build_lint(lint_src, config)?;
         match std::fs::read_dir(&lints_dir) {
             Ok(dir) => {
                 for file in dir {
-                    let file = file.unwrap().path();
+                    let file = file.unwrap().path().into_utf8()?;
                     if file.extension() == Some(ending) && !found_paths.contains(&file) {
                         found_paths.insert(file.clone());
                         lints.push(LintCrate {
@@ -66,10 +68,7 @@ pub fn build_lints(sources: &[LintCrateSource], config: &Config) -> Result<Vec<L
             Err(err) => {
                 // This shouldn't really be a point of failure. In this case, I'm
                 // more interested in the HOW?
-                panic!(
-                    "unable to read lints dir after lint compilation: {} ({err:#?})",
-                    lints_dir.display()
-                );
+                panic!("unable to read lints dir after lint compilation: {lints_dir} ({err:#?})");
             },
         }
     }
@@ -82,7 +81,7 @@ pub fn build_lints(sources: &[LintCrateSource], config: &Config) -> Result<Vec<L
 ///
 /// This is an extra function to not call `delete_dir_all` and just accidentally delete
 /// the entire system.
-fn clear_lints_dir(lints_dir: &Path) -> Result {
+fn clear_lints_dir(lints_dir: &Utf8Path) -> Result {
     // Delete all files
     let dir = match std::fs::read_dir(lints_dir) {
         Ok(dir) => dir,
@@ -115,7 +114,7 @@ fn clear_lints_dir(lints_dir: &Path) -> Result {
     }
 
     // The dir should now be empty
-    std::fs::remove_dir(lints_dir).context(|| format!("Failed to remove lints directory {}", lints_dir.display()))
+    std::fs::remove_dir(lints_dir).context(|| format!("Failed to remove lints directory {lints_dir}"))
 }
 
 fn build_lint(lint_src: &LintCrateSource, config: &Config) -> Result {

cargo-marker/src/backend/lints/build.rs

@@ -14,9 +14,9 @@ use super::LintCrateSource;
 use crate::error::prelude::*;
 use crate::observability::prelude::*;
 use crate::{backend::Config, config::LintDependencyEntry};
+use camino::{Utf8Path, Utf8PathBuf};
 use cargo_metadata::Metadata;
 use std::collections::HashMap;
-use std::path::{Path, PathBuf};
 
 /// This function fetches and locates all lint crates specified in the given
 /// configuration.
@@ -35,7 +35,7 @@ pub fn fetch_crates(config: &Config) -> Result<Vec<LintCrateSource>> {
 
 /// This function sets up the dummy crate with all the lints listed as dependencies.
 /// It returns the path of the manifest, if everything was successful.
-fn setup_dummy_crate(config: &Config) -> Result<PathBuf> {
+fn setup_dummy_crate(config: &Config) -> Result<Utf8PathBuf> {
     /// A small hack, to have the lints namespaced under the `[dependencies]` section
     #[derive(serde::Serialize)]
     struct DepNamespace<'a> {
@@ -58,15 +58,14 @@ fn setup_dummy_crate(config: &Config) -> Result<PathBuf> {
     Ok(manifest_path)
 }
 
-fn write_to_file(path: &PathBuf, content: &str) -> Result {
+fn write_to_file(path: &Utf8Path, content: &str) -> Result {
     let parent = path
         .parent()
-        .unwrap_or_else(|| panic!("The file must have a parent directory. Path: {}", path.display()));
+        .unwrap_or_else(|| panic!("The file must have a parent directory. Path: {path}"));
 
-    std::fs::create_dir_all(parent)
-        .context(|| format!("Failed to create the ditectory structure for {}", parent.display()))?;
+    std::fs::create_dir_all(parent).context(|| format!("Failed to create the directory structure for {parent}"))?;
 
-    std::fs::write(path, content).context(|| format!("Failed to write a file at {}", path.display()))
+    std::fs::write(path, content).context(|| format!("Failed to write a file at {path}"))
 }
 
 const DUMMY_MANIFEST_TEMPLATE: &str = r#"
@@ -94,7 +93,7 @@ const DUMMY_MAIN_CONTENT: &str = r#"
     }
 "#;
 
-fn call_cargo_fetch(manifest: &Path, config: &Config) -> Result {
+fn call_cargo_fetch(manifest: &Utf8Path, config: &Config) -> Result {
     let mut cmd = config.toolchain.cargo.command();
     cmd.arg("fetch");
     cmd.arg("--manifest-path");
@@ -121,19 +120,14 @@ fn call_cargo_fetch(manifest: &Path, config: &Config) -> Result {
     Err(Error::root("cargo fetch failed for lint crates"))
 }
 
-fn call_cargo_metadata(manifest: &PathBuf, config: &Config) -> Result<Metadata> {
+fn call_cargo_metadata(manifest: &Utf8Path, config: &Config) -> Result<Metadata> {
     config
         .toolchain
         .cargo
         .metadata()
         .manifest_path(manifest)
         .exec()
-        .context(|| {
-            format!(
-                "Failed to get cargo metadata for the lint crates at {}",
-                manifest.display()
-            )
-        })
+        .context(|| format!("Failed to get cargo metadata for the lint crates at {manifest}"))
 }
 
 fn extract_lint_crate_sources(metadata: &Metadata, marker_config: &Config) -> Vec<LintCrateSource> {
@@ -143,7 +137,7 @@ fn extract_lint_crate_sources(metadata: &Metadata, marker_config: &Config) -> Ve
         .filter(|pkg| marker_config.lints.contains_key(&pkg.name))
         .map(|pkg| LintCrateSource {
             name: pkg.name.clone(),
-            manifest: pkg.manifest_path.clone().into(),
+            manifest: pkg.manifest_path.clone(),
         })
         .collect()
 }