From 1e08d19a39a053f7d8ab78c0779a1aa1fae0b85c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Thu, 14 Sep 2023 14:09:30 -0600 Subject: [PATCH 1/2] TreeRelativePathBuf isn't worth it --- src/cargo.rs | 10 +++-- src/mutate.rs | 8 ++-- src/path.rs | 105 +------------------------------------------------- src/source.rs | 10 ++--- src/visit.rs | 17 ++++---- 5 files changed, 22 insertions(+), 128 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 5cd32166..6ded779a 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -12,7 +12,6 @@ use tracing::debug_span; #[allow(unused_imports)] use tracing::{debug, error, info, span, trace, warn, Level}; -use crate::path::TreeRelativePathBuf; use crate::process::get_command_output; use crate::source::Package; use crate::tool::Tool; @@ -221,13 +220,16 @@ fn rustflags() -> String { fn direct_package_sources( workspace_root: &Utf8Path, package_metadata: &cargo_metadata::Package, -) -> Result> { +) -> Result> { let mut found = Vec::new(); let pkg_dir = package_metadata.manifest_path.parent().unwrap(); for target in &package_metadata.targets { if should_mutate_target(target) { - if let Ok(relpath) = target.src_path.strip_prefix(workspace_root) { - let relpath = TreeRelativePathBuf::new(relpath.into()); + if let Ok(relpath) = target + .src_path + .strip_prefix(workspace_root) + .map(ToOwned::to_owned) + { debug!( "found mutation target {} of kind {:?}", relpath, target.kind diff --git a/src/mutate.rs b/src/mutate.rs index 55b0cad7..8cda8d83 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -6,6 +6,7 @@ use std::fmt; use std::fs; use std::sync::Arc; +use anyhow::ensure; use anyhow::Context; use anyhow::Result; use serde::ser::{SerializeStruct, Serializer}; @@ -138,12 +139,9 @@ impl Mutant { } fn write_in_dir(&self, build_dir: &BuildDir, code: &str) -> Result<()> { - let path = self - .source_file - .tree_relative_path() - .within(build_dir.path()); + let path = build_dir.path().join(&self.source_file.tree_relative_path); // for safety, don't follow symlinks - assert!(path.is_file(), "{path:?} is not a file"); + ensure!(path.is_file(), "{path:?} is not a file"); fs::write(&path, code.as_bytes()) .with_context(|| format!("failed to write mutated code to {path:?}")) } diff --git a/src/path.rs b/src/path.rs index 7ae5d4e4..745e7815 100644 --- a/src/path.rs +++ b/src/path.rs @@ -2,13 +2,7 @@ //! Utilities for file paths. -use std::convert::TryInto; -use std::fmt; -use std::path::{Path, PathBuf}; -use std::str::FromStr; - -use camino::{Utf8Path, Utf8PathBuf}; -use serde::Serialize; +use camino::Utf8Path; /// Measures how far above its starting point a path ascends. /// @@ -54,103 +48,6 @@ impl Utf8PathSlashes for Utf8Path { } } -/// A path relative to the top of the source tree. -#[derive(Debug, PartialEq, Eq, Clone, PartialOrd, Ord, Serialize)] -pub struct TreeRelativePathBuf(Utf8PathBuf); - -impl fmt::Display for TreeRelativePathBuf { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = self - .0 - .components() - .map(|c| c.as_str()) - .collect::>() - .join("/"); - f.write_str(&s) - } -} - -impl TreeRelativePathBuf { - pub fn new(path: Utf8PathBuf) -> Self { - assert!(path.is_relative()); - TreeRelativePathBuf(path) - } - - /// Make an empty tree-relative path, identifying the root. - #[allow(dead_code)] - pub fn empty() -> Self { - TreeRelativePathBuf(Utf8PathBuf::new()) - } - - #[allow(dead_code)] - pub fn from_absolute(path: &Utf8Path, tree_root: &Utf8Path) -> Self { - TreeRelativePathBuf( - path.strip_prefix(tree_root) - .expect("path is within tree root") - .to_owned(), - ) - } - - pub fn within(&self, tree_path: &Utf8Path) -> Utf8PathBuf { - tree_path.join(&self.0) - } - - #[allow(dead_code)] - pub fn join(&self, p: impl AsRef) -> Self { - TreeRelativePathBuf(self.0.join(p)) - } - - /// Return the tree-relative path of the containing directory. - /// - /// Panics if there is no parent, i.e. if self is already the tree root. - #[allow(dead_code)] - pub fn parent(&self) -> TreeRelativePathBuf { - self.0 - .parent() - .expect("TreeRelativePath has no parent") - .to_owned() - .into() - } -} - -impl AsRef for TreeRelativePathBuf { - fn as_ref(&self) -> &Utf8Path { - &self.0 - } -} - -impl From<&Utf8Path> for TreeRelativePathBuf { - fn from(path_buf: &Utf8Path) -> Self { - TreeRelativePathBuf::new(path_buf.to_owned()) - } -} - -impl From for TreeRelativePathBuf { - fn from(path_buf: Utf8PathBuf) -> Self { - TreeRelativePathBuf::new(path_buf) - } -} - -impl From for TreeRelativePathBuf { - fn from(path_buf: PathBuf) -> Self { - TreeRelativePathBuf::new(path_buf.try_into().expect("path must be UTF-8")) - } -} - -impl From<&Path> for TreeRelativePathBuf { - fn from(path: &Path) -> Self { - TreeRelativePathBuf::from(path.to_owned()) - } -} - -impl FromStr for TreeRelativePathBuf { - type Err = anyhow::Error; - - fn from_str(s: &str) -> Result { - Ok(TreeRelativePathBuf::new(s.parse()?)) - } -} - #[cfg(test)] mod test { use camino::{Utf8Path, Utf8PathBuf}; diff --git a/src/source.rs b/src/source.rs index 535577c7..e2bf4bac 100644 --- a/src/source.rs +++ b/src/source.rs @@ -9,8 +9,6 @@ use camino::{Utf8Path, Utf8PathBuf}; #[allow(unused_imports)] use tracing::{debug, info, warn}; -use crate::path::TreeRelativePathBuf; - /// A Rust source file within a source tree. /// /// It can be viewed either relative to the source tree (for display) @@ -24,7 +22,7 @@ pub struct SourceFile { pub package: Arc, /// Path relative to the root of the tree. - pub tree_relative_path: TreeRelativePathBuf, + pub tree_relative_path: Utf8PathBuf, /// Full copy of the source. pub code: Arc, @@ -36,10 +34,10 @@ impl SourceFile { /// This eagerly loads the text of the file. pub fn new( tree_path: &Utf8Path, - tree_relative_path: TreeRelativePathBuf, + tree_relative_path: Utf8PathBuf, package: &Arc, ) -> Result { - let full_path = tree_relative_path.within(tree_path); + let full_path = tree_path.join(&tree_relative_path); let code = std::fs::read_to_string(&full_path) .with_context(|| format!("failed to read source of {full_path:?}"))? .replace("\r\n", "\n"); @@ -56,7 +54,7 @@ impl SourceFile { } /// Return the path of this file relative to the base of the source tree. - pub fn tree_relative_path(&self) -> &TreeRelativePathBuf { + pub fn tree_relative_path(&self) -> &Utf8Path { &self.tree_relative_path } } diff --git a/src/visit.rs b/src/visit.rs index 16db26ff..1ed4f3b6 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -23,7 +23,6 @@ use syn::{ }; use tracing::{debug, debug_span, trace, trace_span, warn}; -use crate::path::TreeRelativePathBuf; use crate::source::SourceFile; use crate::*; @@ -61,13 +60,13 @@ pub fn walk_tree(tool: &dyn Tool, root: &Utf8Path, options: &Options) -> Result< } let path = &source_file.tree_relative_path; if let Some(examine_globset) = &options.examine_globset { - if !examine_globset.is_match(path.as_ref()) { + if !examine_globset.is_match(path) { trace!("{path:?} does not match examine globset"); continue; } } if let Some(exclude_globset) = &options.exclude_globset { - if exclude_globset.is_match(path.as_ref()) { + if exclude_globset.is_match(path) { trace!("{path:?} excluded by globset"); continue; } @@ -96,7 +95,7 @@ fn walk_file( source_file: Arc, options: &Options, error_exprs: &[Expr], -) -> Result<(Vec, Vec)> { +) -> Result<(Vec, Vec)> { let _span = debug_span!("source_file", path = source_file.tree_relative_slashes()).entered(); debug!("visit source file"); let syn_file = syn::parse_str::(&source_file.code) @@ -130,7 +129,7 @@ struct DiscoveryVisitor<'o> { namespace_stack: Vec, /// Files discovered by `mod` statements. - more_files: Vec, + more_files: Vec, /// Global options. #[allow(unused)] @@ -269,7 +268,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { // Having determined the directory then we can look for either // `foo.rs` or `foo/mod.rs`. if node.content.is_none() { - let my_path: &Utf8Path = self.source_file.tree_relative_path().as_ref(); + let my_path: &Utf8Path = self.source_file.tree_relative_path(); // Maybe matching on the name here is no the right approach and // we should instead remember how this file was found? let dir = if my_path.ends_with("mod.rs") @@ -282,9 +281,9 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { }; let mut found = false; let mut tried_paths = Vec::new(); - for &ext in &[".rs", "/mod.rs"] { - let relative_path = TreeRelativePathBuf::new(dir.join(format!("{mod_name}{ext}"))); - let full_path = relative_path.within(&self.root); + for &tail in &[".rs", "/mod.rs"] { + let relative_path = dir.join(format!("{mod_name}{tail}")); + let full_path = self.root.join(&relative_path); if full_path.is_file() { trace!("found submodule in {full_path}"); self.more_files.push(relative_path); From a79c18d9a540611edd8478aed30abe6f1d92ae90 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Thu, 14 Sep 2023 14:34:40 -0600 Subject: [PATCH 2/2] Tweak string handling --- src/visit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/visit.rs b/src/visit.rs index 1ed4f3b6..c88e7968 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -282,7 +282,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { let mut found = false; let mut tried_paths = Vec::new(); for &tail in &[".rs", "/mod.rs"] { - let relative_path = dir.join(format!("{mod_name}{tail}")); + let relative_path = dir.join(mod_name.clone() + tail); let full_path = self.root.join(&relative_path); if full_path.is_file() { trace!("found submodule in {full_path}");