Merge pull request rust-marker#233 from nitkach/refactoring

Type constraint for paths and IntoUtf8 trait

Cargo.toml

@@ -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-marker/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/src/backend.rs

@@ -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/driver.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/lints.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/build.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/fetch.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()
 }