diff --git a/.gitignore b/.gitignore index 30e82aa1..ded89aa1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ # Rust / Cargo /target /target-verify-* +# tools/ is its own workspace, so its build output is its own directory. +/tools/target /vendor/*/target **/*.rs.bk *.pdb diff --git a/tools/Cargo.lock b/tools/Cargo.lock index a7de6cff..fcc58884 100644 --- a/tools/Cargo.lock +++ b/tools/Cargo.lock @@ -58,22 +58,6 @@ version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" -[[package]] -name = "bitflags" -version = "2.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" - -[[package]] -name = "cc" -version = "1.4.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3eb0f42d6c360dc3f8a821f6bf2fdea7f72bfd36b3076eb0e6d1e9e0752fff4" -dependencies = [ - "find-msvc-tools", - "shlex", -] - [[package]] name = "clap" version = "4.6.7" @@ -120,48 +104,6 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" -[[package]] -name = "fallible-iterator" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" - -[[package]] -name = "fallible-streaming-iterator" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" - -[[package]] -name = "find-msvc-tools" -version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e0f1c7c3a72c66fd80abe965175f7523475c0489a87d3ff9d6e8c87d87a9d2d" - -[[package]] -name = "foldhash" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" - -[[package]] -name = "hashbrown" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" -dependencies = [ - "foldhash", -] - -[[package]] -name = "hashlink" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7382cf6263419f2d8df38c55d7da83da5c18aef87fc7a7fc1fb1e344edfe14c1" -dependencies = [ - "hashbrown", -] - [[package]] name = "heck" version = "0.5.0" @@ -174,47 +116,12 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" -[[package]] -name = "itoa" -version = "1.0.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" - -[[package]] -name = "libsqlite3-sys" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "133c182a6a2c87864fe97778797e46c7e999672690dc9fa3ee8e241aa4a9c13f" -dependencies = [ - "cc", - "pkg-config", - "vcpkg", -] - -[[package]] -name = "memchr" -version = "2.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" - -[[package]] -name = "once_cell" -version = "1.21.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" - [[package]] name = "once_cell_polyfill" version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" -[[package]] -name = "pkg-config" -version = "0.3.34" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6b464fbc74e149a392436b17d523f769e057cb6877f6a5c4618bc6f11800548" - [[package]] name = "proc-macro2" version = "1.0.107" @@ -224,26 +131,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "protobuf" -version = "3.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d65a1d4ddae7d8b5de68153b48f6aa3bba8cb002b243dbdbc55a5afbc98f99f4" -dependencies = [ - "once_cell", - "protobuf-support", - "thiserror", -] - -[[package]] -name = "protobuf-support" -version = "3.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e36c2f31e0a47f9280fb347ef5e461ffcd2c52dd520d8e216b52f93b0b0d7d6" -dependencies = [ - "thiserror", -] - [[package]] name = "quote" version = "1.0.47" @@ -254,82 +141,14 @@ dependencies = [ ] [[package]] -name = "rusqlite" -version = "0.37.0" +name = "same-file" +version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "165ca6e57b20e1351573e3729b958bc62f0e48025386970b6e4d29e7a7e71f3f" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" dependencies = [ - "bitflags", - "fallible-iterator", - "fallible-streaming-iterator", - "hashlink", - "libsqlite3-sys", - "smallvec", + "winapi-util", ] -[[package]] -name = "scip" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7f651fbd0f98742a47b58623a7bcfd8fad14455912689e4867a1f43ee7e02c6" -dependencies = [ - "protobuf", -] - -[[package]] -name = "serde" -version = "1.0.229" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" -dependencies = [ - "serde_core", -] - -[[package]] -name = "serde_core" -version = "1.0.229" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" -dependencies = [ - "serde_derive", -] - -[[package]] -name = "serde_derive" -version = "1.0.229" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" -dependencies = [ - "proc-macro2", - "quote", - "syn 3.0.6", -] - -[[package]] -name = "serde_json" -version = "1.0.151" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" -dependencies = [ - "itoa", - "memchr", - "serde", - "serde_core", - "zmij", -] - -[[package]] -name = "shlex" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" - -[[package]] -name = "smallvec" -version = "1.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba467056f1b547ed52077911161fc86985becbc60e8e1857c8a144dab0def891" - [[package]] name = "strsim" version = "0.11.1" @@ -358,26 +177,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "thiserror" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" -dependencies = [ - "thiserror-impl", -] - -[[package]] -name = "thiserror-impl" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "unicode-ident" version = "1.0.26" @@ -391,10 +190,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] -name = "vcpkg" -version = "0.2.15" +name = "walkdir" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" +dependencies = [ + "same-file", + "winapi-util", +] + +[[package]] +name = "winapi-util" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" +dependencies = [ + "windows-sys", +] [[package]] name = "windows-link" @@ -412,19 +224,12 @@ dependencies = [ ] [[package]] -name = "xray" +name = "xray-clusters" version = "0.1.0" dependencies = [ "anyhow", "clap", - "protobuf", - "rusqlite", - "scip", - "serde_json", + "proc-macro2", + "syn 2.0.119", + "walkdir", ] - -[[package]] -name = "zmij" -version = "1.0.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" diff --git a/tools/Cargo.toml b/tools/Cargo.toml index 14ce1031..ca4592ff 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"] +members = ["xray", "xray-clusters"] [workspace.package] version = "0.1.0" diff --git a/tools/xray-clusters/Cargo.toml b/tools/xray-clusters/Cargo.toml new file mode 100644 index 00000000..22edca2d --- /dev/null +++ b/tools/xray-clusters/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "xray-clusters" +description = "Finds the independent clusters inside a Rust source file." +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +publish.workspace = true + +[dependencies] +anyhow = "1" +clap = { version = "4", features = ["derive"] } +# `span-locations` is what turns a parsed item back into line numbers. Without +# it every span reports line 0 and the whole tool reports nothing. +proc-macro2 = { version = "1", features = ["span-locations"] } +syn = { version = "2", features = ["full", "visit", "extra-traits", "parsing", "printing"] } +walkdir = "2" + +[lints.clippy] +all = "warn" diff --git a/tools/xray-clusters/src/cluster.rs b/tools/xray-clusters/src/cluster.rs new file mode 100644 index 00000000..2298ecef --- /dev/null +++ b/tools/xray-clusters/src/cluster.rs @@ -0,0 +1,770 @@ +//! Builds the reference graph and cuts it into connected components. + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::item::{Import, Item, ParsedFile}; + +/// Method names that nearly every type has. A call to one of these says +/// nothing about which item in the file it landed on, so it draws no edge. +const UNIVERSAL_METHODS: &[&str] = &[ + "as_bytes", + "as_mut", + "as_ref", + "as_str", + "borrow", + "clone", + "cloned", + "cmp", + "collect", + "default", + "deref", + "drop", + "eq", + "fmt", + "from", + "get", + "hash", + "insert", + "into", + "into_iter", + "is_empty", + "iter", + "len", + "map", + "ne", + "next", + "partial_cmp", + "poll", + "push", + "remove", + "serialize", + "to_owned", + "to_string", + "try_from", + "try_into", + "unwrap", +]; + +/// Path heads that are never an item in this file. +const PATH_ROOTS: &[&str] = &["crate", "self", "super", "Self"]; + +/// One connected component: a set of items that cannot be separated without +/// one importing from the other. +pub struct Cluster { + /// Indices into [`Analysis::items`]. + pub items: Vec, + pub lines: usize, + /// Merged, sorted line ranges the cluster occupies. + pub spans: Vec<(usize, usize)>, + /// Names this cluster would have to import, as `use` lines. + pub imports: BTreeSet, + /// Modules outside the file it depends on, and how often. + pub external: BTreeMap, + /// True when every item in it is test code. + pub is_test: bool, + /// True when it is a single `impl` of a foreign trait on a foreign type. + pub is_foreign_impl: bool, +} + +impl Cluster { + /// The cluster's title in reports: its largest item's name. + pub fn title(&self, items: &[Item]) -> String { + let head = self + .items + .iter() + .max_by_key(|&&i| items[i].lines()) + .copied() + .unwrap_or(0); + items[head].name.clone() + } +} + +/// How woven together the clusters are, plus the shape numbers around it. +pub struct Scores { + pub lines: usize, + /// Lines held by non-test items. + pub code_lines: usize, + /// Lines held by test items. + pub test_lines: usize, + pub cluster_count: usize, + /// Cohesion groups: the partition to read when a file is one component. + pub group_count: usize, + /// Clusters of at least 25 lines: the ones worth a file of their own. + pub substantial: usize, + pub largest_cluster_share: f64, + /// 0.0 when each cluster sits in one contiguous block, 1.0 when they + /// alternate as much as they possibly could. + pub interleaving: f64, + /// Maximal runs of same-cluster items in line order. Equals the cluster + /// count when the file is already sorted by cluster. + pub blocks: usize, +} + +impl Scores { + /// The `--all` ranking: how much code there is, times how many pieces it + /// falls into. Cohesion groups do the counting, because a file held + /// together by one shared type is one connected component however many + /// separate things it holds. + pub fn rank(&self) -> usize { + self.code_lines * self.group_count + } +} + +/// A set of items that names nothing outside itself. It can move to a new +/// file, and what stays behind only has to `use` it -- no import comes back. +pub struct Sink { + /// Indices into [`Analysis::items`], in line order. + pub items: Vec, + pub lines: usize, + pub spans: Vec<(usize, usize)>, + /// Names the new file would have to import. + pub imports: BTreeSet, + /// Items left behind that reference it. + pub used_by: usize, + /// Members the rest of the file reaches into: the new module's public + /// interface. One is the cleanest a split gets. + pub boundary: Vec, +} + +/// An item whose removal would break the file up further. +pub struct Hub { + /// Index into [`Analysis::items`]. + pub item: usize, + /// Clusters the rest would fall into without it. + pub clusters: usize, + /// Lines the biggest of those clusters would still hold. + pub largest: usize, +} + +/// The result of analysing one file. +pub struct Analysis { + pub path: String, + pub items: Vec, + pub clusters: Vec, + pub scores: Scores, + /// Items holding the rest together, worst first. + pub hubs: Vec, + /// Items pulled out as a shared spine by `--peel`. + pub peeled: Vec, + /// Non-overlapping one-way splits, biggest first. + pub sinks: Vec, + /// The modularity partition: every non-test item, grouped by where the + /// references are densest. Biggest first. + pub groups: Vec, + /// `use foo::*` prefixes. Any of these makes name resolution incomplete. + pub globs: Vec, +} + +struct Resolver<'a> { + /// Name defined in this file -> item index. + owner: BTreeMap, + /// Method name -> the one item defining it, when only one does. + method_owner: BTreeMap>, + imports: &'a BTreeMap, +} + +impl<'a> Resolver<'a> { + fn new(file: &'a ParsedFile) -> Self { + let mut owner = BTreeMap::new(); + let mut method_owner: BTreeMap> = BTreeMap::new(); + for (i, item) in file.items.iter().enumerate() { + for name in &item.defines { + owner.entry(name.clone()).or_insert(i); + } + for m in &item.methods { + method_owner + .entry(m.clone()) + .and_modify(|slot| { + if *slot != Some(i) { + *slot = None; + } + }) + .or_insert(Some(i)); + } + } + Resolver { + owner, + method_owner, + imports: &file.imports, + } + } + + /// Where a path leads: an item in this file, or a module outside it. + fn resolve(&self, segs: &[String], generics: &BTreeSet) -> Target { + let head = &segs[0]; + if !generics.contains(head) && !PATH_ROOTS.contains(&head.as_str()) { + if let Some(&i) = self.owner.get(head) { + return Target::Local(i); + } + } + let mut full: Vec<&str>; + let mut import = None; + match self.imports.get(head) { + Some(imp) => { + full = imp.path.iter().map(String::as_str).collect(); + full.extend(segs[1..].iter().map(String::as_str)); + import = Some(imp); + } + None if segs.len() >= 2 => full = segs.iter().map(String::as_str).collect(), + // A bare name that is neither defined here nor imported is a + // local binding, a prelude name or a glob import. Nothing to say. + None => return Target::Unknown, + } + let module = full[..full.len() - 1].join("::"); + Target::External { + module, + import: import.map(|i| i.use_line()), + } + } +} + +enum Target { + Local(usize), + External { + module: String, + import: Option, + }, + Unknown, +} + +struct Dsu { + parent: Vec, +} + +impl Dsu { + fn new(n: usize) -> Self { + Dsu { + parent: (0..n).collect(), + } + } + fn find(&mut self, mut x: usize) -> usize { + while self.parent[x] != x { + self.parent[x] = self.parent[self.parent[x]]; + x = self.parent[x]; + } + x + } + fn union(&mut self, a: usize, b: usize) { + let (a, b) = (self.find(a), self.find(b)); + if a != b { + self.parent[b] = a; + } + } +} + +/// Every edge the graph has, as unordered item pairs. +fn edges(file: &ParsedFile, resolver: &Resolver<'_>) -> Vec<(usize, usize)> { + let mut out = Vec::new(); + for (i, item) in file.items.iter().enumerate() { + if item.is_test { + continue; + } + let push = |j: usize, out: &mut Vec<(usize, usize)>| { + if j != i && !file.items[j].is_test { + out.push((i, j)); + } + }; + for segs in &item.refs.paths { + if let Target::Local(j) = resolver.resolve(segs, &item.refs.generics) { + push(j, &mut out); + // `Type::helper` and `Self::helper` name a method, not just + // the type it hangs off. + if let Some(Some(k)) = segs.get(1).and_then(|m| resolver.method_owner.get(m)) { + push(*k, &mut out); + } + } + } + for m in &item.refs.methods { + if UNIVERSAL_METHODS.contains(&m.as_str()) { + continue; + } + if let Some(Some(j)) = resolver.method_owner.get(m) { + push(*j, &mut out); + } + } + // An `impl` belongs with its type; failing that, with its trait. + if let Some(ty) = &item.impl_type { + if let Some(&j) = resolver.owner.get(ty) { + push(j, &mut out); + continue; + } + } + if let Some(tr) = &item.impl_trait { + if let Some(&j) = resolver.owner.get(tr) { + push(j, &mut out); + } + } + } + out +} + +/// How the live items fall apart when `skip` is set aside: how many pieces, +/// and how many lines the biggest one still holds. +fn shape( + items: &[Item], + edges: &[(usize, usize)], + live: &[bool], + skip: Option, +) -> (usize, usize) { + let n = items.len(); + let mut dsu = Dsu::new(n); + for &(a, b) in edges { + if Some(a) == skip || Some(b) == skip || !live[a] || !live[b] { + continue; + } + dsu.union(a, b); + } + let mut sizes: BTreeMap = BTreeMap::new(); + for i in 0..n { + if live[i] && Some(i) != skip { + *sizes.entry(dsu.find(i)).or_default() += items[i].lines(); + } + } + (sizes.len(), sizes.values().copied().max().unwrap_or(0)) +} + +/// Analyse one already-parsed file. `peel` sets aside that many hub items -- +/// the shared spine -- before partitioning what is left. +pub fn analyse(path: &str, file: ParsedFile, peel: usize, limits: SplitLimits) -> Analysis { + let resolver = Resolver::new(&file); + let all_edges = edges(&file, &resolver); + let n = file.items.len(); + let live: Vec = file.items.iter().map(|i| !i.is_test).collect(); + + // A hub is judged by how much of the file it is holding together, not by + // how many pieces fall off. Shaking one constant loose is not a seam. + let (_, base_largest) = shape(&file.items, &all_edges, &live, None); + let mut hubs: Vec = (0..n) + .filter(|&i| live[i]) + .map(|i| { + let (count, largest) = shape(&file.items, &all_edges, &live, Some(i)); + Hub { + item: i, + clusters: count, + largest, + } + }) + .filter(|h| h.largest < base_largest) + .collect(); + hubs.sort_by(|a, b| a.largest.cmp(&b.largest).then(a.item.cmp(&b.item))); + + // Peeling is greedy: set aside whichever item leaves the smallest + // remaining blob, then look again. + let mut peeled = Vec::new(); + let mut remaining = live.clone(); + for _ in 0..peel { + let (_, here) = shape(&file.items, &all_edges, &remaining, None); + let best = (0..n) + .filter(|&i| remaining[i]) + .map(|i| (i, shape(&file.items, &all_edges, &remaining, Some(i)).1)) + .filter(|&(_, largest)| largest < here) + .min_by_key(|&(i, largest)| (largest, file.items[i].lines(), i)); + match best { + Some((i, _)) => { + remaining[i] = false; + peeled.push(i); + } + None => break, + } + } + + let mut dsu = Dsu::new(n); + for &(a, b) in &all_edges { + if remaining[a] && remaining[b] { + dsu.union(a, b); + } + } + + // Test items never joined the graph, so each stands alone under its own + // index. Peeled items are set aside entirely. + let mut groups: BTreeMap> = BTreeMap::new(); + for (i, &kept) in remaining.iter().enumerate() { + if peeled.contains(&i) { + continue; + } + let key = if kept { dsu.find(i) } else { i }; + groups.entry(key).or_default().push(i); + } + + let mut clusters: Vec = groups + .into_values() + .map(|members| build_cluster(members, &file, &resolver)) + .collect(); + clusters.sort_by(|a, b| { + a.is_test + .cmp(&b.is_test) + .then(b.lines.cmp(&a.lines)) + .then(a.spans[0].0.cmp(&b.spans[0].0)) + }); + + let mut scores = score(&file, &clusters); + let sinks = sinks( + &file, + &resolver, + &all_edges, + &remaining, + scores.code_lines, + limits, + ); + let glued = glue_impls(&file, &resolver); + let mut groups: Vec = communities(&file, &all_edges, &glued, &remaining) + .into_iter() + .map(|members| build_cluster(members, &file, &resolver)) + .collect(); + groups.sort_by(|a, b| b.lines.cmp(&a.lines).then(a.spans[0].0.cmp(&b.spans[0].0))); + scores.group_count = groups.len(); + Analysis { + path: path.to_string(), + items: file.items, + clusters, + scores, + hubs, + peeled, + sinks, + groups, + globs: file.globs, + } +} + +/// Group each `impl` with the type it implements. Splitting those apart is +/// legal Rust and never what anyone means. +fn glue_impls(file: &ParsedFile, resolver: &Resolver<'_>) -> Vec { + let n = file.items.len(); + let mut dsu = Dsu::new(n); + for (i, item) in file.items.iter().enumerate() { + if item.kind != crate::item::Kind::Impl { + continue; + } + if let Some(ty) = &item.impl_type { + if let Some(&j) = resolver.owner.get(ty) { + dsu.union(j, i); + } + } + } + (0..n).map(|i| dsu.find(i)).collect() +} + +/// Every distinct set an item can reach, expressed over glued supernodes. +fn closures(n: usize, edges: &[(usize, usize)], group: &[usize], live: &[bool]) -> Vec> { + let mut succ: BTreeMap> = BTreeMap::new(); + for &(a, b) in edges { + if !live[a] || !live[b] { + continue; + } + let (ga, gb) = (group[a], group[b]); + if ga != gb { + succ.entry(ga).or_default().insert(gb); + } + } + let roots: BTreeSet = (0..n).filter(|&i| live[i]).map(|i| group[i]).collect(); + let mut out = Vec::new(); + for &root in &roots { + let mut seen = vec![false; n]; + let mut stack = vec![root]; + seen[root] = true; + while let Some(g) = stack.pop() { + for &next in succ.get(&g).into_iter().flatten() { + if !seen[next] { + seen[next] = true; + stack.push(next); + } + } + } + out.push(seen); + } + out +} + +/// How big a one-way split has to be to be worth proposing, and how big it is +/// allowed to get before it is just "the file minus its entry points". +#[derive(Clone, Copy)] +pub struct SplitLimits { + /// Smallest split worth a file of its own. + pub min_lines: usize, + /// Largest share of the file's code a split may hold, 0.0 to 1.0. + pub max_share: f64, + /// An `impl` longer than this becomes one item per method. + pub explode_impls_over: usize, +} + +impl Default for SplitLimits { + fn default() -> Self { + // 40 lines is about where a module stops being a stray helper. Half + // the file is where a "split" stops telling you anything: every file + // is its own entry points plus everything underneath them. + // 60 lines is about where an `impl` stops being one idea. Below it a + // whole block moves as a unit; above it the methods are the seams. + SplitLimits { + min_lines: 40, + max_share: 0.5, + explode_impls_over: 60, + } + } +} + +/// The one-way splits worth proposing: biggest first, none overlapping. +fn sinks( + file: &ParsedFile, + resolver: &Resolver<'_>, + edges: &[(usize, usize)], + live: &[bool], + code_lines: usize, + limits: SplitLimits, +) -> Vec { + let n = file.items.len(); + let group = glue_impls(file, resolver); + let mut candidates: Vec<(usize, Vec)> = Vec::new(); + let mut seen: BTreeSet> = BTreeSet::new(); + for closed in closures(n, edges, &group, live) { + let members: Vec = (0..n).filter(|&i| live[i] && closed[group[i]]).collect(); + if members.is_empty() || !seen.insert(members.clone()) { + continue; + } + let lines: usize = members.iter().map(|&i| file.items[i].lines()).sum(); + // Too small to be a file, or so big it is the file itself. + if lines < limits.min_lines || lines as f64 > code_lines as f64 * limits.max_share { + continue; + } + candidates.push((lines, members)); + } + // A split is judged by its interface, not its size: the fewer of its + // names the rest of the file reaches for, the more it is a module and the + // less it is an arbitrary cut through the middle of one. + let mut scored: Vec<(usize, usize, Vec)> = candidates + .into_iter() + .map(|(lines, members)| { + let inside: BTreeSet = members.iter().copied().collect(); + let boundary = members + .iter() + .filter(|&&b| { + edges + .iter() + .any(|&(a, t)| t == b && live[a] && !inside.contains(&a)) + }) + .count(); + (boundary, lines, members) + }) + .collect(); + // Lines per name in the interface: how much code one `use` buys you. + let cohesion = |boundary: usize, lines: usize| lines as f64 / boundary.max(1) as f64; + scored.sort_by(|a, b| { + cohesion(b.0, b.1) + .partial_cmp(&cohesion(a.0, a.1)) + .unwrap_or(std::cmp::Ordering::Equal) + .then(b.1.cmp(&a.1)) + }); + + let mut taken = vec![false; n]; + let mut out = Vec::new(); + for (_, lines, members) in scored { + if members.iter().any(|&i| taken[i]) { + continue; + } + for &i in &members { + taken[i] = true; + } + let inside: BTreeSet = members.iter().copied().collect(); + let used_by = (0..n) + .filter(|&i| live[i] && !inside.contains(&i)) + .filter(|&i| edges.iter().any(|&(a, b)| a == i && inside.contains(&b))) + .count(); + let boundary: Vec = members + .iter() + .copied() + .filter(|&b| { + edges + .iter() + .any(|&(a, t)| t == b && live[a] && !inside.contains(&a)) + }) + .collect(); + let cluster = build_cluster(members, file, resolver); + out.push(Sink { + items: cluster.items, + lines, + spans: cluster.spans, + imports: cluster.imports, + used_by, + boundary, + }); + } + out.sort_by_key(|sink| std::cmp::Reverse(sink.lines)); + out +} + +/// Cuts the graph where the references are thinnest, by greedy modularity: +/// start every item alone, repeatedly merge the two groups whose merge most +/// improves modularity, and stop when no merge improves it. +/// +/// Unlike a connected component this tolerates references in both directions, +/// which is what a module boundary inside one crate actually allows. It is +/// the partition to read when a file is one connected blob. +fn communities( + file: &ParsedFile, + edges: &[(usize, usize)], + group: &[usize], + live: &[bool], +) -> Vec> { + let n = file.items.len(); + let nodes: Vec = { + let set: BTreeSet = (0..n).filter(|&i| live[i]).map(|i| group[i]).collect(); + set.into_iter().collect() + }; + let index: BTreeMap = nodes.iter().enumerate().map(|(k, &g)| (g, k)).collect(); + let k = nodes.len(); + let mut adj: Vec> = vec![BTreeMap::new(); k]; + let mut total = 0.0f64; + for &(a, b) in edges { + if !live[a] || !live[b] || group[a] == group[b] { + continue; + } + let (x, y) = (index[&group[a]], index[&group[b]]); + *adj[x].entry(y).or_insert(0.0) += 1.0; + *adj[y].entry(x).or_insert(0.0) += 1.0; + total += 1.0; + } + + let mut dsu = Dsu::new(k); + if total > 0.0 { + let two_m = 2.0 * total; + let mut degree: Vec = adj.iter().map(|a| a.values().sum()).collect(); + let mut alive = vec![true; k]; + loop { + let mut best: Option<(f64, usize, usize)> = None; + for x in 0..k { + if !alive[x] { + continue; + } + for (&y, &w) in &adj[x] { + if y <= x || !alive[y] { + continue; + } + let gain = w / two_m - (degree[x] * degree[y]) / (two_m * two_m); + if best.is_none_or(|(b, _, _)| gain > b) { + best = Some((gain, x, y)); + } + } + } + let Some((gain, x, y)) = best else { break }; + if gain <= 0.0 { + break; + } + let moved: Vec<(usize, f64)> = adj[y].iter().map(|(&z, &w)| (z, w)).collect(); + for (z, w) in moved { + adj[z].remove(&y); + if z == x { + continue; + } + *adj[x].entry(z).or_insert(0.0) += w; + *adj[z].entry(x).or_insert(0.0) += w; + } + adj[x].remove(&y); + adj[y].clear(); + degree[x] += degree[y]; + alive[y] = false; + dsu.union(x, y); + } + } + + let mut out: BTreeMap> = BTreeMap::new(); + for i in 0..n { + if live[i] { + out.entry(dsu.find(index[&group[i]])).or_default().push(i); + } + } + out.into_values().collect() +} + +fn build_cluster(mut members: Vec, file: &ParsedFile, resolver: &Resolver<'_>) -> Cluster { + members.sort_by_key(|&i| file.items[i].start); + let lines = members.iter().map(|&i| file.items[i].lines()).sum(); + let mut spans: Vec<(usize, usize)> = Vec::new(); + for &i in &members { + let it = &file.items[i]; + match spans.last_mut() { + Some(last) if it.start <= last.1 + 1 => last.1 = last.1.max(it.end), + _ => spans.push((it.start, it.end)), + } + } + let mut imports = BTreeSet::new(); + let mut external: BTreeMap = BTreeMap::new(); + for &i in &members { + let it = &file.items[i]; + for segs in &it.refs.paths { + if let Target::External { module, import } = resolver.resolve(segs, &it.refs.generics) { + if !module.is_empty() { + *external.entry(module).or_default() += 1; + } + if let Some(line) = import { + imports.insert(line); + } + } + } + } + let is_test = members.iter().all(|&i| file.items[i].is_test); + let is_foreign_impl = members.len() == 1 && { + let it = &file.items[members[0]]; + it.impl_type.is_some() && !resolver.owner.contains_key(it.impl_type.as_ref().unwrap()) + }; + Cluster { + items: members, + lines, + spans, + imports, + external, + is_test, + is_foreign_impl, + } +} + +fn score(file: &ParsedFile, clusters: &[Cluster]) -> Scores { + let code: Vec<&Cluster> = clusters.iter().filter(|c| !c.is_test).collect(); + let code_lines: usize = code.iter().map(|c| c.lines).sum(); + let test_lines: usize = clusters.iter().filter(|c| c.is_test).map(|c| c.lines).sum(); + let largest = code.iter().map(|c| c.lines).max().unwrap_or(0); + + // Line order over non-test items, labelled with the cluster they landed + // in. Runs of one label are blocks; extra transitions are interleaving. + let mut labelled: Vec<(usize, usize)> = Vec::new(); + for (cid, c) in clusters.iter().enumerate() { + if c.is_test { + continue; + } + for &i in &c.items { + labelled.push((file.items[i].start, cid)); + } + } + labelled.sort(); + let mut blocks = 0usize; + let mut prev = usize::MAX; + for &(_, cid) in &labelled { + if cid != prev { + blocks += 1; + prev = cid; + } + } + let k = code.len(); + let transitions = blocks.saturating_sub(1); + let floor = k.saturating_sub(1); + let ceiling = labelled.len().saturating_sub(1); + let interleaving = if ceiling > floor { + (transitions.saturating_sub(floor)) as f64 / (ceiling - floor) as f64 + } else { + 0.0 + }; + + Scores { + lines: file.lines, + code_lines, + test_lines, + cluster_count: k, + group_count: 0, + substantial: code.iter().filter(|c| c.lines >= 25).count(), + largest_cluster_share: if code_lines == 0 { + 0.0 + } else { + largest as f64 / code_lines as f64 + }, + interleaving, + blocks, + } +} diff --git a/tools/xray-clusters/src/item.rs b/tools/xray-clusters/src/item.rs new file mode 100644 index 00000000..90bf7e36 --- /dev/null +++ b/tools/xray-clusters/src/item.rs @@ -0,0 +1,505 @@ +//! Turns a Rust source file into the items and names the rest of the tool +//! reasons about. + +use std::collections::{BTreeMap, BTreeSet}; + +use syn::spanned::Spanned; +use syn::visit::Visit; + +/// What sort of top-level item this is. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Kind { + Fn, + Struct, + Enum, + Union, + Trait, + Impl, + Mod, + Const, + Static, + Type, + Macro, + Other, +} + +impl Kind { + /// The word printed in reports. + pub fn label(self) -> &'static str { + match self { + Kind::Fn => "fn", + Kind::Struct => "struct", + Kind::Enum => "enum", + Kind::Union => "union", + Kind::Trait => "trait", + Kind::Impl => "impl", + Kind::Mod => "mod", + Kind::Const => "const", + Kind::Static => "static", + Kind::Type => "type", + Kind::Macro => "macro", + Kind::Other => "item", + } + } +} + +/// Every name an item mentions, split by how it was mentioned. +#[derive(Default, Debug)] +pub struct Refs { + /// Every path, as its segment idents. + pub paths: Vec>, + /// Receiver-style calls: the `bar` in `foo.bar()`. + pub methods: BTreeSet, + /// Type and const generic parameters, which look like type names but + /// never refer to another item. + pub generics: BTreeSet, +} + +/// One top-level item, with its span and everything it names. +#[derive(Debug)] +pub struct Item { + pub kind: Kind, + /// How the item is printed, e.g. `impl Display for Zone`. + pub name: String, + /// The names this item puts into file scope. + pub defines: Vec, + pub start: usize, + pub end: usize, + /// `#[cfg(test)]`, `#[test]`, or an inline `mod tests`. + pub is_test: bool, + /// For an `impl`, the type being implemented. + pub impl_type: Option, + /// For a trait impl, the trait. + pub impl_trait: Option, + /// Method names this item introduces (inherent, trait, or trait impl). + pub methods: Vec, + pub refs: Refs, +} + +impl Item { + /// Lines the item occupies, doc comments and attributes included. + pub fn lines(&self) -> usize { + self.end.saturating_sub(self.start) + 1 + } +} + +/// A `use` leaf, or a `mod foo;` that stands for another file. +#[derive(Clone, Debug)] +pub struct Import { + /// The name the file refers to it by, after any `as`. + pub name: String, + /// Full path segments, leaf included. + pub path: Vec, +} + +impl Import { + /// The `use` line that would bring this name into a new file. + pub fn use_line(&self) -> String { + let full = self.path.join("::"); + if self.path.last().map(String::as_str) == Some(self.name.as_str()) { + format!("use {full};") + } else { + format!("use {full} as {};", self.name) + } + } + + /// The module the name lives in: everything but the leaf. + pub fn module(&self) -> String { + let n = self.path.len().saturating_sub(1); + self.path[..n].join("::") + } +} + +/// A parsed file. +pub struct ParsedFile { + pub lines: usize, + pub items: Vec, + pub imports: BTreeMap, + /// `use foo::*` prefixes, which make single-segment names unresolvable. + pub globs: Vec, +} + +/// Parse `source` into items, imports and references. An `impl` block longer +/// than `explode_over` lines becomes one item per method, because that is +/// where the seams in a very long one are. +pub fn parse(source: &str, explode_over: usize) -> syn::Result { + let file = syn::parse_file(source)?; + let mut out = ParsedFile { + lines: source.lines().count(), + items: Vec::new(), + imports: BTreeMap::new(), + globs: Vec::new(), + }; + for item in &file.items { + match item { + syn::Item::Use(u) => { + collect_use(&u.tree, &mut Vec::new(), &mut out); + continue; + } + // `mod foo;` is a pointer at another file, so it behaves like an + // import rather than like code living here. + syn::Item::Mod(m) if m.content.is_none() => { + let name = m.ident.to_string(); + out.imports.insert( + name.clone(), + Import { + path: vec!["self".to_string(), name.clone()], + name, + }, + ); + continue; + } + syn::Item::ExternCrate(_) => continue, + _ => {} + } + describe_into(item, explode_over, &mut out.items); + } + Ok(out) +} + +fn collect_use(tree: &syn::UseTree, prefix: &mut Vec, out: &mut ParsedFile) { + match tree { + syn::UseTree::Path(p) => { + prefix.push(p.ident.to_string()); + collect_use(&p.tree, prefix, out); + prefix.pop(); + } + syn::UseTree::Name(n) => { + let name = n.ident.to_string(); + let mut path = prefix.clone(); + path.push(name.clone()); + out.imports.insert(name.clone(), Import { name, path }); + } + syn::UseTree::Rename(r) => { + let name = r.rename.to_string(); + let mut path = prefix.clone(); + path.push(r.ident.to_string()); + out.imports.insert(name.clone(), Import { name, path }); + } + syn::UseTree::Glob(_) => out.globs.push(prefix.join("::")), + syn::UseTree::Group(g) => { + for t in &g.items { + collect_use(t, prefix, out); + } + } + } +} + +fn attrs_of(item: &syn::Item) -> &[syn::Attribute] { + macro_rules! arms { + ($($v:ident),*) => { + match item { $(syn::Item::$v(x) => &x.attrs,)* _ => &[] } + }; + } + arms!( + Const, + Enum, + ExternCrate, + Fn, + ForeignMod, + Impl, + Macro, + Mod, + Static, + Struct, + Trait, + TraitAlias, + Type, + Union, + Use + ) +} + +/// True when an attribute gates the item to test builds, or marks a test. +fn is_test_attr(attr: &syn::Attribute) -> bool { + if attr + .path() + .segments + .last() + .is_some_and(|s| s.ident == "test") + { + return true; + } + if attr.path().is_ident("cfg") { + if let syn::Meta::List(list) = &attr.meta { + return list.tokens.to_string().contains("test"); + } + } + false +} + +fn describe_into(item: &syn::Item, explode_over: usize, out: &mut Vec) { + let span = item.span(); + let start = span.start().line; + let end = span.end().line; + let attrs = attrs_of(item); + let mut is_test = attrs.iter().any(is_test_attr); + + let (kind, name, defines, methods) = match item { + syn::Item::Fn(f) => { + let n = f.sig.ident.to_string(); + (Kind::Fn, n.clone(), vec![n], vec![]) + } + syn::Item::Struct(s) => { + let n = s.ident.to_string(); + (Kind::Struct, n.clone(), vec![n], vec![]) + } + syn::Item::Enum(e) => { + let n = e.ident.to_string(); + (Kind::Enum, n.clone(), vec![n], vec![]) + } + syn::Item::Union(u) => { + let n = u.ident.to_string(); + (Kind::Union, n.clone(), vec![n], vec![]) + } + syn::Item::Trait(t) => { + let n = t.ident.to_string(); + let methods = t + .items + .iter() + .filter_map(|i| match i { + syn::TraitItem::Fn(f) => Some(f.sig.ident.to_string()), + _ => None, + }) + .collect(); + (Kind::Trait, n.clone(), vec![n], methods) + } + syn::Item::TraitAlias(t) => { + let n = t.ident.to_string(); + (Kind::Trait, n.clone(), vec![n], vec![]) + } + syn::Item::Const(c) => { + let n = c.ident.to_string(); + (Kind::Const, n.clone(), vec![n], vec![]) + } + syn::Item::Static(s) => { + let n = s.ident.to_string(); + (Kind::Static, n.clone(), vec![n], vec![]) + } + syn::Item::Type(t) => { + let n = t.ident.to_string(); + (Kind::Type, n.clone(), vec![n], vec![]) + } + syn::Item::Macro(m) => { + let n = m + .ident + .as_ref() + .map(ToString::to_string) + .or_else(|| path_head(&m.mac.path)) + .unwrap_or_else(|| "macro".to_string()); + let defines = if m.ident.is_some() { + vec![n.clone()] + } else { + vec![] + }; + (Kind::Macro, n, defines, vec![]) + } + syn::Item::Mod(m) => { + let n = m.ident.to_string(); + if n == "tests" || n == "test" { + is_test = true; + } + (Kind::Mod, n.clone(), vec![n], vec![]) + } + syn::Item::Impl(i) => { + explode_impl(i, span, is_test, explode_over, out); + return; + } + syn::Item::ForeignMod(_) | syn::Item::Verbatim(_) => { + (Kind::Other, "extern".to_string(), vec![], vec![]) + } + _ => (Kind::Other, "item".to_string(), vec![], vec![]), + }; + + let mut made = Item { + kind, + name, + defines, + start, + end, + is_test, + impl_type: None, + impl_trait: None, + methods, + refs: Refs::default(), + }; + collect_refs(item, &mut made.refs); + out.push(made); +} + +/// Break an `impl` into a header plus one item per method, or leave it whole +/// when it is short enough to move as a unit. +fn explode_impl( + i: &syn::ItemImpl, + span: proc_macro2::Span, + is_test: bool, + explode_over: usize, + out: &mut Vec, +) { + let ty = type_head(&i.self_ty); + let tr = i.trait_.as_ref().and_then(|(_, p, _)| path_head(p)); + let head = match (&tr, &ty) { + (Some(t), Some(y)) => format!("impl {t} for {y}"), + (None, Some(y)) => format!("impl {y}"), + _ => "impl".to_string(), + }; + let (start, end) = (span.start().line, span.end().line); + let methods: Vec<&syn::ImplItemFn> = i + .items + .iter() + .filter_map(|it| match it { + syn::ImplItem::Fn(f) => Some(f), + _ => None, + }) + .collect(); + + if end - start < explode_over || methods.len() < 2 { + let mut made = Item { + kind: Kind::Impl, + name: head, + defines: vec![], + start, + end, + is_test, + impl_type: ty.clone(), + impl_trait: tr, + methods: methods.iter().map(|f| f.sig.ident.to_string()).collect(), + refs: Refs::default(), + }; + RefVisitor { + refs: &mut made.refs, + } + .visit_item_impl(i); + rewrite_self(&mut made.refs, ty.as_deref()); + out.push(made); + return; + } + + let first = methods + .first() + .map(|f| f.span().start().line) + .unwrap_or(end); + let mut header = Item { + kind: Kind::Impl, + name: format!("{head} (header)"), + defines: vec![], + start, + end: first.saturating_sub(1).max(start), + is_test, + impl_type: ty.clone(), + impl_trait: tr.clone(), + methods: vec![], + refs: Refs::default(), + }; + { + let mut visitor = RefVisitor { + refs: &mut header.refs, + }; + visitor.visit_generics(&i.generics); + visitor.visit_type(&i.self_ty); + if let Some((_, path, _)) = &i.trait_ { + visitor.visit_path(path); + } + for it in &i.items { + if !matches!(it, syn::ImplItem::Fn(_)) { + visitor.visit_impl_item(it); + } + } + } + rewrite_self(&mut header.refs, ty.as_deref()); + out.push(header); + + let owner = ty.clone().unwrap_or_else(|| "impl".to_string()); + for f in methods { + let fspan = f.span(); + let name = f.sig.ident.to_string(); + let mut made = Item { + kind: Kind::Fn, + name: format!("{owner}::{name}"), + defines: vec![], + start: fspan.start().line, + end: fspan.end().line, + is_test: is_test || f.attrs.iter().any(is_test_attr), + impl_type: ty.clone(), + impl_trait: tr.clone(), + methods: vec![name], + refs: Refs::default(), + }; + RefVisitor { + refs: &mut made.refs, + } + .visit_impl_item_fn(f); + rewrite_self(&mut made.refs, ty.as_deref()); + out.push(made); + } +} + +/// `Self` means the type being implemented, and resolving it that way is what +/// links a method to the rest of its type. +fn rewrite_self(refs: &mut Refs, ty: Option<&str>) { + let Some(ty) = ty else { return }; + for segs in &mut refs.paths { + if segs.first().map(String::as_str) == Some("Self") { + segs[0] = ty.to_string(); + } + } +} + +/// The name at the head of a type, ignoring references and generics. +fn type_head(ty: &syn::Type) -> Option { + match ty { + syn::Type::Path(p) => path_head(&p.path), + syn::Type::Reference(r) => type_head(&r.elem), + syn::Type::Paren(p) => type_head(&p.elem), + syn::Type::Group(g) => type_head(&g.elem), + syn::Type::Slice(s) => type_head(&s.elem), + syn::Type::Array(a) => type_head(&a.elem), + syn::Type::Ptr(p) => type_head(&p.elem), + _ => None, + } +} + +fn path_head(path: &syn::Path) -> Option { + path.segments.last().map(|s| s.ident.to_string()) +} + +struct RefVisitor<'a> { + refs: &'a mut Refs, +} + +impl<'ast> Visit<'ast> for RefVisitor<'_> { + fn visit_path(&mut self, path: &'ast syn::Path) { + let segs: Vec = path.segments.iter().map(|s| s.ident.to_string()).collect(); + if !segs.is_empty() { + self.refs.paths.push(segs); + } + syn::visit::visit_path(self, path); + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + self.refs.methods.insert(call.method.to_string()); + syn::visit::visit_expr_method_call(self, call); + } + + fn visit_generic_param(&mut self, param: &'ast syn::GenericParam) { + match param { + syn::GenericParam::Type(t) => { + self.refs.generics.insert(t.ident.to_string()); + } + syn::GenericParam::Const(c) => { + self.refs.generics.insert(c.ident.to_string()); + } + syn::GenericParam::Lifetime(_) => {} + } + syn::visit::visit_generic_param(self, param); + } + + fn visit_macro(&mut self, mac: &'ast syn::Macro) { + // A macro body is not parseable as Rust, but its path is a name. + self.visit_path(&mac.path); + } +} + +fn collect_refs(item: &syn::Item, refs: &mut Refs) { + RefVisitor { refs }.visit_item(item); +} diff --git a/tools/xray-clusters/src/lib.rs b/tools/xray-clusters/src/lib.rs new file mode 100644 index 00000000..0ae11578 --- /dev/null +++ b/tools/xray-clusters/src/lib.rs @@ -0,0 +1,162 @@ +//! Counts the independent things inside a Rust source file. +//! +//! A file is read with `syn`, its top-level items become graph nodes, and an +//! edge is drawn from A to B when A's body names something B defines. The +//! connected components of that graph are the pieces the file could be split +//! into without either piece importing from the other. Names that resolve +//! outside the file are recorded against the module they come from, which is +//! what a split-out file would have to import. + +pub mod cluster; +pub mod item; +pub mod report; +pub mod scan; + +/// Parse and analyse one file's text. +pub fn analyse_source( + path: &str, + source: &str, + peel: usize, + limits: cluster::SplitLimits, +) -> syn::Result { + Ok(cluster::analyse( + path, + item::parse(source, limits.explode_impls_over)?, + peel, + limits, + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn clusters(src: &str) -> Vec> { + let a = analyse_source("t.rs", src, 0, Default::default()).expect("fixture parses"); + a.clusters + .iter() + .map(|c| c.items.iter().map(|&i| a.items[i].name.clone()).collect()) + .collect() + } + + #[test] + fn unconnected_functions_are_separate_clusters() { + let out = clusters("fn a() { let x = 1; }\nfn b() { let y = 2; }\n"); + assert_eq!(out.len(), 2); + } + + #[test] + fn a_call_joins_two_functions() { + let out = clusters("fn a() { b(); }\nfn b() {}\n"); + assert_eq!(out, vec![vec!["a".to_string(), "b".to_string()]]); + } + + #[test] + fn a_test_module_is_its_own_cluster() { + let src = r#" +fn a() { b(); } +fn b() {} +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn works() { a(); } +} +"#; + let a = analyse_source("t.rs", src, 0, Default::default()).expect("fixture parses"); + assert_eq!( + a.scores.cluster_count, 1, + "tests do not join the code graph" + ); + let test_clusters: Vec<_> = a.clusters.iter().filter(|c| c.is_test).collect(); + assert_eq!(test_clusters.len(), 1); + assert!(a.scores.test_lines > 0); + } + + #[test] + fn an_impl_joins_its_type() { + let src = "struct Zone;\nimpl Zone { fn name(&self) -> &str { \"z\" } }\nfn other() {}\n"; + let out = clusters(src); + assert_eq!(out.len(), 2); + assert!(out + .iter() + .any(|c| c.len() == 2 && c.contains(&"Zone".to_string()))); + } + + #[test] + fn a_foreign_trait_impl_stands_alone() { + let src = "impl std::fmt::Display for String { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Ok(()) } }\nfn a() {}\n"; + let a = analyse_source("t.rs", src, 0, Default::default()).expect("fixture parses"); + assert_eq!(a.scores.cluster_count, 2); + assert!(a.clusters.iter().any(|c| c.is_foreign_impl)); + } + + #[test] + fn imports_follow_the_cluster_that_uses_them() { + let src = "use std::collections::HashMap;\nfn a() -> HashMap { HashMap::new() }\nfn b() {}\n"; + let a = analyse_source("t.rs", src, 0, Default::default()).expect("fixture parses"); + let user = a + .clusters + .iter() + .find(|c| c.items.iter().any(|&i| a.items[i].name == "a")) + .expect("cluster for a"); + assert!(user.imports.contains("use std::collections::HashMap;")); + let other = a + .clusters + .iter() + .find(|c| c.items.iter().any(|&i| a.items[i].name == "b")) + .expect("cluster for b"); + assert!(other.imports.is_empty()); + } + + #[test] + fn contiguous_clusters_do_not_count_as_interleaved() { + let contiguous = "fn a1() { a2(); }\nfn a2() {}\nfn b1() { b2(); }\nfn b2() {}\n"; + let woven = "fn a1() { a2(); }\nfn b1() { b2(); }\nfn a2() {}\nfn b2() {}\n"; + let c = analyse_source("t.rs", contiguous, 0, Default::default()).unwrap(); + let w = analyse_source("t.rs", woven, 0, Default::default()).unwrap(); + assert_eq!(c.scores.cluster_count, 2); + assert_eq!(w.scores.cluster_count, 2); + assert_eq!(c.scores.interleaving, 0.0); + assert!(w.scores.interleaving > 0.0); + } + + #[test] + fn a_thin_link_between_two_halves_makes_two_cohesion_groups() { + // One connected component, because `b3` calls `a1` once. The dense + // wiring is inside each half, which is what the groups follow. + let src = " +fn a1() { a2(); a3(); } +fn a2() { a1(); a3(); } +fn a3() { a1(); a2(); } +fn b1() { b2(); b3(); } +fn b2() { b1(); b3(); } +fn b3() { b1(); b2(); a1(); } +"; + let a = analyse_source("t.rs", src, 0, Default::default()).unwrap(); + assert_eq!(a.scores.cluster_count, 1); + assert_eq!(a.scores.group_count, 2); + } + + #[test] + fn a_long_impl_is_read_one_method_at_a_time() { + let body = (0..8) + .map(|i| format!("fn m{i}(&self) -> u32 {{ {i} }}\n{}", "// pad\n".repeat(12))) + .collect::(); + let src = format!("struct Big;\nimpl Big {{\n{body}}}\n"); + let a = analyse_source("t.rs", &src, 0, Default::default()).unwrap(); + let named: Vec<&str> = a.items.iter().map(|i| i.name.as_str()).collect(); + assert!(named.contains(&"Big::m0"), "got {named:?}"); + assert!(named.contains(&"Big::m7"), "got {named:?}"); + } + + #[test] + fn peeling_a_hub_reveals_the_clusters_behind_it() { + let src = "struct Ctx;\nfn a(c: &Ctx) {}\nfn b(c: &Ctx) {}\nfn c_(c: &Ctx) {}\n"; + let whole = analyse_source("t.rs", src, 0, Default::default()).unwrap(); + assert_eq!(whole.scores.cluster_count, 1); + let peeled = analyse_source("t.rs", src, 1, Default::default()).unwrap(); + assert_eq!(peeled.peeled.len(), 1); + assert_eq!(peeled.scores.cluster_count, 3); + } +} diff --git a/tools/xray-clusters/src/main.rs b/tools/xray-clusters/src/main.rs new file mode 100644 index 00000000..4a7e619d --- /dev/null +++ b/tools/xray-clusters/src/main.rs @@ -0,0 +1,132 @@ +//! `xray-clusters` -- how many independent things a Rust file holds. + +use std::io::Write as _; +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use clap::Parser; +use xray_clusters::cluster::{Analysis, SplitLimits}; +use xray_clusters::{analyse_source, report, scan}; + +#[derive(Parser)] +#[command( + name = "xray-clusters", + about = "Finds the independent clusters inside a Rust source file", + version +)] +struct Args { + /// One file to partition in detail. + #[arg(long, value_name = "PATH")] + file: Option, + + /// Rank every file in the tree instead. + #[arg(long, conflicts_with = "file")] + all: bool, + + /// Where to walk for `--all`. + #[arg(long, default_value = ".", value_name = "DIR")] + root: PathBuf, + + /// Skip files shorter than this for `--all`. + #[arg(long, default_value_t = 300, value_name = "N")] + min_lines: usize, + + /// How many rows `--all` prints. 0 prints them all. + #[arg(long, default_value_t = 30, value_name = "N")] + top: usize, + + /// Set aside this many hub items before partitioning, which is what + /// moving a shared type out of the file would do. + #[arg(long, default_value_t = 0, value_name = "N")] + peel: usize, + + /// How many items to list per cluster in detail mode. + #[arg(long, default_value_t = 12, value_name = "N")] + max_items: usize, + + /// Smallest one-way split worth proposing. Under about 40 lines a split + /// is a stray helper rather than a module. + #[arg(long, default_value_t = 40, value_name = "N")] + split_min_lines: usize, + + /// Largest share of a file's code one split may hold, 0.0 to 1.0. Above + /// about 0.5 every file trivially splits into its entry points and + /// everything underneath them, which says nothing. + #[arg(long, default_value_t = 0.5, value_name = "F")] + split_max_share: f64, + + /// Break an `impl` longer than this into one item per method. Somewhere + /// between about 40 and 200 lines; below it a block moves as a unit. + #[arg(long, default_value_t = 60, value_name = "N")] + explode_impls_over: usize, + + /// Also read a file's earlier content from git, e.g. `--rev HEAD~1`. + #[arg(long, value_name = "REV", requires = "file")] + rev: Option, +} + +fn main() -> Result<()> { + let args = Args::parse(); + let limits = SplitLimits { + min_lines: args.split_min_lines, + max_share: args.split_max_share, + explode_impls_over: args.explode_impls_over, + }; + let mut out = std::io::stdout().lock(); + + if let Some(path) = &args.file { + let display = path.display().to_string(); + let source = match &args.rev { + Some(rev) => git_show(rev, &display)?, + None => std::fs::read_to_string(path).with_context(|| format!("reading {display}"))?, + }; + let analysis = analyse_source(&display, &source, args.peel, limits) + .map_err(|e| anyhow::anyhow!("{display}: {e}"))?; + write!(out, "{}", report::detail(&analysis, args.max_items))?; + return Ok(()); + } + + if !args.all { + // Neither mode asked for: say so on stderr and exit non-zero, so a + // script notices rather than reading an empty table as "no findings". + anyhow::bail!("pass --file or --all"); + } + + let mut rows: Vec = Vec::new(); + for (path, _) in scan::rust_files(&args.root, args.min_lines) { + let display = path + .strip_prefix(&args.root) + .unwrap_or(&path) + .display() + .to_string(); + let Ok(source) = std::fs::read_to_string(&path) else { + continue; + }; + match analyse_source(&display, &source, args.peel, limits) { + Ok(a) => rows.push(a), + Err(e) => eprintln!("skipped {display}: {e}"), + } + } + rows.sort_by_key(|row| std::cmp::Reverse(row.scores.rank())); + if args.top > 0 { + rows.truncate(args.top); + } + write!(out, "{}", report::table(&rows))?; + Ok(()) +} + +/// Read a file as it was at some revision, so a split can be checked against +/// the file it was split from. +fn git_show(rev: &str, path: &str) -> Result { + let spec = format!("{rev}:{path}"); + let output = std::process::Command::new("git") + .args(["show", &spec]) + .output() + .with_context(|| format!("running git show {spec}"))?; + anyhow::ensure!( + output.status.success(), + "git show {spec}: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + Ok(String::from_utf8(output.stdout)?) +} diff --git a/tools/xray-clusters/src/report.rs b/tools/xray-clusters/src/report.rs new file mode 100644 index 00000000..ff130737 --- /dev/null +++ b/tools/xray-clusters/src/report.rs @@ -0,0 +1,253 @@ +//! Prints an analysis. + +use std::fmt::Write as _; + +use crate::cluster::Analysis; + +/// The one-line-per-file ranking. +pub fn table(rows: &[Analysis]) -> String { + let mut out = String::new(); + let width = rows.iter().map(|a| a.path.len()).max().unwrap_or(4).max(4); + let _ = writeln!( + out, + "{:6} {:>6} {:>4} {:>4} {:>4} {:>6} {:>6} {:>9}", + "file", "lines", "code", "grp", "clus", "big", "share", "interl", "rank" + ); + for a in rows { + let s = &a.scores; + let _ = writeln!( + out, + "{:6} {:>6} {:>4} {:>4} {:>4} {:>5.0}% {:>6.2} {:>9}", + a.path, + s.lines, + s.code_lines, + s.group_count, + s.cluster_count, + s.substantial, + s.largest_cluster_share * 100.0, + s.interleaving, + s.rank(), + ); + } + out +} + +/// The full partition of one file. +pub fn detail(a: &Analysis, max_items: usize) -> String { + let mut out = String::new(); + let s = &a.scores; + let _ = writeln!(out, "{}", a.path); + let _ = writeln!( + out, + " {} lines, {} of code and {} of tests, in {} items", + s.lines, + s.code_lines, + s.test_lines, + a.items.len() + ); + let _ = writeln!( + out, + " {} cohesion groups, {} connected clusters ({} of 25+ lines), largest cluster holds {:.0}%", + s.group_count, + s.cluster_count, + s.substantial, + s.largest_cluster_share * 100.0 + ); + let _ = writeln!( + out, + " interleaving {:.2} ({} blocks in line order, {} if each cluster were contiguous)", + s.interleaving, s.blocks, s.cluster_count + ); + if !a.globs.is_empty() { + let _ = writeln!( + out, + " glob imports make some names unresolvable: {}", + a.globs.join(", ") + ); + } + if !a.peeled.is_empty() { + let names: Vec<&str> = a.peeled.iter().map(|&i| a.items[i].name.as_str()).collect(); + let _ = writeln!(out, " peeled into a shared spine: {}", names.join(", ")); + } + + for (n, c) in a.clusters.iter().enumerate() { + let spans: Vec = c + .spans + .iter() + .map(|(s, e)| format!("{s}-{e}")) + .collect::>(); + let tag = if c.is_test { + " [tests]" + } else if c.is_foreign_impl { + " [foreign impl]" + } else { + "" + }; + let _ = writeln!(out); + let _ = writeln!( + out, + "cluster {} -- {}{} ({} lines, {} items)", + n + 1, + c.title(&a.items), + tag, + c.lines, + c.items.len() + ); + let _ = writeln!(out, " lines: {}", spans.join(", ")); + for &i in c.items.iter().take(max_items) { + let it = &a.items[i]; + let _ = writeln!( + out, + " {:>5}-{:<5} {:<7} {}", + it.start, + it.end, + it.kind.label(), + it.name + ); + } + if c.items.len() > max_items { + let _ = writeln!(out, " ... {} more", c.items.len() - max_items); + } + if !c.imports.is_empty() { + let _ = writeln!(out, " a split-out file would need:"); + for line in &c.imports { + let _ = writeln!(out, " {line}"); + } + } + let mut ext: Vec<(&String, &usize)> = c.external.iter().collect(); + ext.sort_by(|a, b| b.1.cmp(a.1).then(a.0.cmp(b.0))); + if !ext.is_empty() { + let names: Vec = ext + .iter() + .take(8) + .map(|(m, n)| format!("{m} ({n})")) + .collect(); + let _ = writeln!(out, " outside modules: {}", names.join(", ")); + } + } + + if !a.groups.is_empty() { + let _ = writeln!(out); + let _ = writeln!( + out, + "cohesion groups -- the partition that cuts where references are" + ); + let _ = writeln!( + out, + "thinnest, allowing the two halves to `use` each other:" + ); + for (n, g) in a.groups.iter().enumerate() { + let spans: Vec = g.spans.iter().map(|(s, e)| format!("{s}-{e}")).collect(); + let _ = writeln!(out); + let _ = writeln!( + out, + "group {} -- {} ({} lines, {} items)", + n + 1, + g.title(&a.items), + g.lines, + g.items.len() + ); + let _ = writeln!(out, " lines: {}", spans.join(", ")); + for &i in g.items.iter().take(max_items) { + let it = &a.items[i]; + let _ = writeln!( + out, + " {:>5}-{:<5} {:<7} {}", + it.start, + it.end, + it.kind.label(), + it.name + ); + } + if g.items.len() > max_items { + let _ = writeln!(out, " ... {} more", g.items.len() - max_items); + } + if !g.imports.is_empty() { + let _ = writeln!(out, " a split-out file would need:"); + for line in &g.imports { + let _ = writeln!(out, " {line}"); + } + } + } + } + + if !a.sinks.is_empty() { + let _ = writeln!(out); + let _ = writeln!( + out, + "one-way splits -- each of these names nothing left behind, so the" + ); + let _ = writeln!(out, "remainder would only gain a `use` of the new module:"); + for (n, sink) in a.sinks.iter().enumerate() { + let spans: Vec = sink.spans.iter().map(|(s, e)| format!("{s}-{e}")).collect(); + let head = sink + .items + .iter() + .max_by_key(|&&i| a.items[i].lines()) + .map(|&i| a.items[i].name.as_str()) + .unwrap_or("?"); + let _ = writeln!(out); + let _ = writeln!( + out, + "split {} -- {} ({} lines, {} items, {} items left behind use it)", + n + 1, + head, + sink.lines, + sink.items.len(), + sink.used_by + ); + let face: Vec<&str> = sink + .boundary + .iter() + .map(|&i| a.items[i].name.as_str()) + .collect(); + let _ = writeln!( + out, + " the rest of the file would call: {}", + face.join(", ") + ); + let _ = writeln!(out, " lines: {}", spans.join(", ")); + for &i in sink.items.iter().take(max_items) { + let it = &a.items[i]; + let _ = writeln!( + out, + " {:>5}-{:<5} {:<7} {}", + it.start, + it.end, + it.kind.label(), + it.name + ); + } + if sink.items.len() > max_items { + let _ = writeln!(out, " ... {} more", sink.items.len() - max_items); + } + if !sink.imports.is_empty() { + let _ = writeln!(out, " the new file would need:"); + for line in &sink.imports { + let _ = writeln!(out, " {line}"); + } + } + } + } + + if !a.hubs.is_empty() { + let _ = writeln!(out); + let _ = writeln!( + out, + "hubs -- moving one of these out would break the rest up further:" + ); + for h in a.hubs.iter().take(8) { + let it = &a.items[h.item]; + let _ = writeln!( + out, + " {:<7} {:<32} {:>5} lines -> {} clusters, biggest {} lines", + it.kind.label(), + it.name, + it.lines(), + h.clusters, + h.largest + ); + } + } + out +} diff --git a/tools/xray-clusters/src/scan.rs b/tools/xray-clusters/src/scan.rs new file mode 100644 index 00000000..834b1945 --- /dev/null +++ b/tools/xray-clusters/src/scan.rs @@ -0,0 +1,38 @@ +//! Finds the Rust files in a tree worth looking at. + +use std::path::{Path, PathBuf}; + +use walkdir::WalkDir; + +/// Directories that hold code nobody here wrote, or nothing at all. +const SKIP: &[&str] = &["target", "vendor", "node_modules", ".git", "pkg", "dist"]; + +/// Every `.rs` file under `root` with at least `min_lines` lines. +pub fn rust_files(root: &Path, min_lines: usize) -> Vec<(PathBuf, usize)> { + let mut out = Vec::new(); + for entry in WalkDir::new(root) + .into_iter() + .filter_entry(|e| { + !e.file_name() + .to_str() + .is_some_and(|n| SKIP.contains(&n) || n.starts_with(".claude")) + }) + .filter_map(Result::ok) + { + if !entry.file_type().is_file() { + continue; + } + if entry.path().extension().and_then(|e| e.to_str()) != Some("rs") { + continue; + } + let Ok(text) = std::fs::read_to_string(entry.path()) else { + continue; + }; + let lines = text.lines().count(); + if lines >= min_lines { + out.push((entry.into_path(), lines)); + } + } + out.sort(); + out +}