diff --git a/tools/Cargo.lock b/tools/Cargo.lock index 90aa3be7..274a23ad 100644 --- a/tools/Cargo.lock +++ b/tools/Cargo.lock @@ -624,8 +624,17 @@ dependencies = [ "sha2", "syn 2.0.119", "tempfile", - "toml", "twox-hash", + "xray-common", +] + +[[package]] +name = "xray-common" +version = "0.1.0" +dependencies = [ + "serde", + "tempfile", + "toml", "walkdir", ] diff --git a/tools/Cargo.toml b/tools/Cargo.toml index e520828c..7f5779c2 100644 --- a/tools/Cargo.toml +++ b/tools/Cargo.toml @@ -9,7 +9,7 @@ # cargo clippy --manifest-path tools/Cargo.toml --all-targets -- -D warnings [workspace] resolver = "3" -members = ["xray", "xray-clones", "xray-clusters"] +members = ["xray", "xray-clones", "xray-clusters", "xray-common"] [workspace.package] version = "0.1.0" diff --git a/tools/xray-clones/Cargo.toml b/tools/xray-clones/Cargo.toml index 1ad5200c..2b8c4d3b 100644 --- a/tools/xray-clones/Cargo.toml +++ b/tools/xray-clones/Cargo.toml @@ -17,8 +17,7 @@ proc-macro2 = { version = "1", features = ["span-locations"] } clap = { version = "4", features = ["derive"] } serde = { version = "1", features = ["derive"] } serde_json = "1" -toml = "0.9" -walkdir = "2" +xray-common = { path = "../xray-common" } rayon = "1" # A fingerprint is written into allow.toml by hand, so it has to stay the same # across compiler and crate versions. sha2 and xxhash are both fixed by their diff --git a/tools/xray-clones/src/collect.rs b/tools/xray-clones/src/collect.rs index 6c3b18a1..1f63aa3c 100644 --- a/tools/xray-clones/src/collect.rs +++ b/tools/xray-clones/src/collect.rs @@ -71,7 +71,7 @@ impl Skipped { /// is machine-written or does not parse. pub fn collect_file(path: &Path, krate: &str, opts: &Options) -> Option { let text = std::fs::read_to_string(path).ok()?; - if is_generated(&text) { + if xray_common::is_generated(&text) { return None; } let file = syn::parse_file(&text).ok()?; @@ -90,12 +90,6 @@ pub fn collect_file(path: &Path, krate: &str, opts: &Options) -> Option bool { - text.lines() - .take(5) - .any(|l| l.contains("@generated") || l.contains("DO NOT EDIT")) -} - struct Walker<'a> { krate: String, file: PathBuf, diff --git a/tools/xray-clones/src/main.rs b/tools/xray-clones/src/main.rs index 931eb6e8..b5df449b 100644 --- a/tools/xray-clones/src/main.rs +++ b/tools/xray-clones/src/main.rs @@ -2,7 +2,6 @@ //! //! See `README.md` for what the measure catches and what it does not. -mod allow; mod collect; mod normalize; mod similar; @@ -12,6 +11,7 @@ use collect::{Item, Kind, Options, Skipped}; use rayon::prelude::*; use std::collections::BTreeSet; use std::path::{Path, PathBuf}; +use xray_common::{discover, Allowlist}; #[derive(Parser)] #[command( @@ -111,10 +111,10 @@ fn main() -> std::process::ExitCode { fn run(args: &Args) -> Result { let allowlist = if args.no_allow { - allow::Allowlist::default() + Allowlist::default() } else { let path = args.allow.clone().unwrap_or_else(default_allow_path); - allow::Allowlist::load(&path)? + Allowlist::load(&path)? }; let opts = Options { @@ -230,47 +230,6 @@ fn default_allow_path() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")).join("allow.toml") } -/// Every `.rs` file under `crates/*/src` and `crates/*/tests`, plus any -/// directory named with `--scan`, paired with the crate it belongs to. -fn discover(root: &Path, extra: &[PathBuf]) -> Vec<(PathBuf, String)> { - let mut out = Vec::new(); - let crates = root.join("crates"); - if let Ok(entries) = std::fs::read_dir(&crates) { - let mut dirs: Vec<_> = entries.flatten().map(|e| e.path()).collect(); - dirs.sort(); - for dir in dirs { - let Some(name) = dir.file_name().and_then(|n| n.to_str()) else { - continue; - }; - let name = name.to_string(); - for sub in ["src", "tests"] { - walk_rs(&dir.join(sub), &name, &mut out); - } - } - } - for dir in extra { - let name = dir - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("scan") - .to_string(); - walk_rs(dir, &name, &mut out); - } - out -} - -fn walk_rs(dir: &Path, krate: &str, out: &mut Vec<(PathBuf, String)>) { - if !dir.is_dir() { - return; - } - for entry in walkdir::WalkDir::new(dir).into_iter().flatten() { - let p = entry.path(); - if p.extension().is_some_and(|e| e == "rs") && p.is_file() { - out.push((p.to_path_buf(), krate.to_string())); - } - } -} - #[derive(serde::Serialize)] struct Summary { files: usize, @@ -315,7 +274,7 @@ impl Report { g: similar::Group, items: &[Item], args: &Args, - allowlist: &allow::Allowlist, + allowlist: &Allowlist, ) -> Option { let mut members: Vec<&Item> = g.members.iter().map(|&i| &items[i]).collect(); // An `impl` block and a method inside it overlap in the source, so diff --git a/tools/xray-common/Cargo.toml b/tools/xray-common/Cargo.toml new file mode 100644 index 00000000..9e96961c --- /dev/null +++ b/tools/xray-common/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "xray-common" +description = "What the xray tools share: which files to read, and which findings are already reviewed." +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +publish.workspace = true + +[dependencies] +serde = { version = "1", features = ["derive"] } +toml = "0.9" +walkdir = "2" + +[dev-dependencies] +tempfile = "3" diff --git a/tools/xray-clones/src/allow.rs b/tools/xray-common/src/allow.rs similarity index 83% rename from tools/xray-clones/src/allow.rs rename to tools/xray-common/src/allow.rs index 01a25973..60919182 100644 --- a/tools/xray-clones/src/allow.rs +++ b/tools/xray-common/src/allow.rs @@ -1,8 +1,9 @@ -//! The reviewed-duplicate list. +//! The reviewed-finding list. //! -//! An entry silences one shape by its fingerprint and must say why. Silencing -//! a reviewed duplicate one fingerprint at a time keeps the threshold honest; -//! lowering the threshold to hide one pair hides every pair like it. +//! An entry silences one finding by the fingerprint the tool prints beside it, +//! and must say why. Silencing one fingerprint at a time keeps a tool's +//! thresholds honest; loosening a threshold to hide one finding hides every +//! finding like it. use std::collections::HashMap; use std::path::Path; @@ -16,7 +17,7 @@ struct File { #[derive(serde::Deserialize)] struct Entry { fingerprint: String, - /// Why this duplicate is acceptable. Required: an entry without one is a + /// Why this finding is acceptable. Required: an entry without one is a /// parse error, not a silent allow. reason: String, } @@ -41,7 +42,7 @@ impl Allowlist { )) } - /// The reason a group is allowed, if any of its fingerprints is listed. + /// The reason a finding is allowed, if any of its fingerprints is listed. pub fn reason<'a>(&'a self, fingerprints: impl Iterator) -> Option<&'a str> { fingerprints .filter_map(|f| self.0.get(f)) @@ -52,6 +53,10 @@ impl Allowlist { pub fn len(&self) -> usize { self.0.len() } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } } #[cfg(test)] diff --git a/tools/xray-common/src/discover.rs b/tools/xray-common/src/discover.rs new file mode 100644 index 00000000..c363b6b6 --- /dev/null +++ b/tools/xray-common/src/discover.rs @@ -0,0 +1,51 @@ +//! Which files an xray tool reads. + +use std::path::{Path, PathBuf}; + +/// Every `.rs` file under `crates/*/src` and `crates/*/tests`, plus any +/// directory named in `extra`, paired with the crate it belongs to. +pub fn discover(root: &Path, extra: &[PathBuf]) -> Vec<(PathBuf, String)> { + let mut out = Vec::new(); + let crates = root.join("crates"); + if let Ok(entries) = std::fs::read_dir(&crates) { + let mut dirs: Vec<_> = entries.flatten().map(|e| e.path()).collect(); + dirs.sort(); + for dir in dirs { + let Some(name) = dir.file_name().and_then(|n| n.to_str()) else { + continue; + }; + let name = name.to_string(); + for sub in ["src", "tests"] { + walk_rs(&dir.join(sub), &name, &mut out); + } + } + } + for dir in extra { + let name = dir + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("scan") + .to_string(); + walk_rs(dir, &name, &mut out); + } + out +} + +fn walk_rs(dir: &Path, krate: &str, out: &mut Vec<(PathBuf, String)>) { + if !dir.is_dir() { + return; + } + for entry in walkdir::WalkDir::new(dir).into_iter().flatten() { + let p = entry.path(); + if p.extension().is_some_and(|e| e == "rs") && p.is_file() { + out.push((p.to_path_buf(), krate.to_string())); + } + } +} + +/// True for a file whose opening lines say a machine wrote it. +pub fn is_generated(text: &str) -> bool { + text.lines() + .take(5) + .any(|l| l.contains("@generated") || l.contains("DO NOT EDIT")) +} diff --git a/tools/xray-common/src/lib.rs b/tools/xray-common/src/lib.rs new file mode 100644 index 00000000..a8a7b3f6 --- /dev/null +++ b/tools/xray-common/src/lib.rs @@ -0,0 +1,10 @@ +//! What the xray tools share. +//! +//! Both tools read the same files and both answer to a reviewed-finding list. +//! A tool that reports duplication should not be built out of it. + +pub mod allow; +pub mod discover; + +pub use allow::Allowlist; +pub use discover::{discover, is_generated};