From 67151a4a6d3c23a798ed22b0793dd79cc3262fd7 Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:18:11 +0300 Subject: [PATCH 1/9] fix(providers): honour PATHEXT on Windows when detecting providers find_program joined the bare catalog name (cargo, tsc, ...) and checked is_file, so on Windows cargo.exe / tsc.cmd were never found and plumb doctor reported every provider as missing. Bare names are now tried with the executable extensions from PATHEXT (Windows only; kept to .com/.exe/.bat/.cmd, which std::process::Command can start), directory order first, then extension order. Unix keeps the exact-name + execute-bit lookup. detect_with_exts injects the list so the Windows rules are tested on Linux; one cfg(windows) test covers PATHEXT. --- crates/core/src/providers.rs | 87 ++++++++++++- crates/core/tests/providers_exe_ext.rs | 173 +++++++++++++++++++++++++ 2 files changed, 255 insertions(+), 5 deletions(-) create mode 100644 crates/core/tests/providers_exe_ext.rs diff --git a/crates/core/src/providers.rs b/crates/core/src/providers.rs index aa147fa..b82bab4 100644 --- a/crates/core/src/providers.rs +++ b/crates/core/src/providers.rs @@ -199,13 +199,82 @@ pub fn default_search_path() -> Vec { .unwrap_or_default() } -fn find_program(program: &str, search: &[PathBuf], root: &Path) -> Option { +/// Windows `PATHEXT` default, used when the variable is unset or yields nothing usable. +pub const DEFAULT_PATHEXT: &str = ".COM;.EXE;.BAT;.CMD"; + +/// Extensions `std::process::Command` can start on Windows (`.bat`/`.cmd` go through +/// `cmd.exe`). Other `PATHEXT` entries (`.js`, `.vbs`, `.msc`, ...) need a host program, +/// so a provider found only under such a name could not be run and is not reported. +const RUNNABLE_EXTS: &[&str] = &[".com", ".exe", ".bat", ".cmd"]; + +/// Parse a `PATHEXT`-style list (`.COM;.EXE;...`) into lower-case, dot-prefixed, +/// de-duplicated extensions in their original order, keeping only `RUNNABLE_EXTS`. +pub fn parse_pathext(raw: &str) -> Vec { + let mut out: Vec = vec![]; + for part in raw.split(';') { + let part = part.trim(); + if part.is_empty() { + continue; + } + let mut e = part.to_ascii_lowercase(); + if !e.starts_with('.') { + e.insert(0, '.'); + } + if RUNNABLE_EXTS.contains(&e.as_str()) && !out.contains(&e) { + out.push(e); + } + } + out +} + +/// Executable extensions to try after a bare program name: from `PATHEXT` on Windows, +/// none elsewhere (Unix looks up the exact name and checks the execute bit). +pub fn executable_extensions() -> Vec { + #[cfg(windows)] + { + let raw = std::env::var("PATHEXT").unwrap_or_default(); + let exts = parse_pathext(&raw); + if exts.is_empty() { + parse_pathext(DEFAULT_PATHEXT) + } else { + exts + } + } + #[cfg(not(windows))] + { + Vec::new() + } +} + +/// File names to try for `program`: the exact name when there is no extension list or the +/// name already ends in a listed extension; otherwise `program` + each extension, in order. +/// The bare name is deliberately not tried when extensions apply: npm puts an +/// extensionless sh shim next to the `.cmd` one, and Windows cannot run the sh shim. +fn candidate_names(program: &str, exe_exts: &[String]) -> Vec { + let lower = program.to_ascii_lowercase(); + if exe_exts.is_empty() || exe_exts.iter().any(|e| lower.ends_with(e.as_str())) { + return vec![program.to_string()]; + } + exe_exts.iter().map(|e| format!("{program}{e}")).collect() +} + +fn find_program( + program: &str, + search: &[PathBuf], + root: &Path, + exe_exts: &[String], +) -> Option { let mut dirs: Vec = search.to_vec(); // project-local JS tooling (tsc, scip-typescript, ast-grep) installed by npm dirs.push(root.join("node_modules/.bin")); + let names = candidate_names(program, exe_exts); + // directory order first, then extension order: the same precedence as cmd.exe for d in dirs { - let p = d.join(program); - if p.is_file() { + for name in &names { + let p = d.join(name); + if !p.is_file() { + continue; + } #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -253,12 +322,20 @@ fn project_exts(root: &Path) -> std::collections::HashSet { } /// Which providers are installed and relevant. Looks for executables only; runs nothing. +/// On Windows a bare name such as `cargo` is matched as `cargo.exe`, `cargo.cmd`, ... +/// following `PATHEXT` (see `executable_extensions`). pub fn detect(root: &Path, search: &[PathBuf]) -> Vec { + detect_with_exts(root, search, &executable_extensions()) +} + +/// `detect` with an explicit executable-extension list (`[]` = exact names, the Unix rule). +/// Lets the Windows lookup rules be tested on any platform. +pub fn detect_with_exts(root: &Path, search: &[PathBuf], exe_exts: &[String]) -> Vec { let exts = project_exts(root); CATALOG .iter() .map(|e| { - let found = find_program(e.program, search, root); + let found = find_program(e.program, search, root, exe_exts); Detected { id: e.id.into(), kind: e.kind, @@ -308,7 +385,7 @@ pub fn run_scip_indexers( runs.push(run); continue; }; - let Some(prog) = find_program(e.program, search, &root) else { + let Some(prog) = find_program(e.program, search, &root, &executable_extensions()) else { run.status = "skipped".into(); run.note = Some(format!("`{}` not found; install: {}", e.program, e.install)); runs.push(run); diff --git a/crates/core/tests/providers_exe_ext.rs b/crates/core/tests/providers_exe_ext.rs new file mode 100644 index 0000000..7d46875 --- /dev/null +++ b/crates/core/tests/providers_exe_ext.rs @@ -0,0 +1,173 @@ +//! Provider detection must find Windows executables (`cargo.exe`, npm `.cmd` shims) without +//! changing the exact-name lookup used on Unix. The extension list is injected so the Windows +//! rules run on every platform; one `cfg(windows)` test covers the real `PATHEXT` path. +use plumbgraph_core::providers::{self, Detected}; +use std::path::{Path, PathBuf}; + +/// Create an executable file: mode 0755 on Unix, plain file elsewhere. +fn exe(dir: &Path, name: &str) -> PathBuf { + let p = dir.join(name); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, "#!/bin/sh\nexit 0\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + p +} + +fn project() -> tempfile::TempDir { + let t = tempfile::tempdir().unwrap(); + std::fs::write(t.path().join("main.rs"), "fn main() {}\n").unwrap(); + t +} + +fn get<'a>(det: &'a [Detected], id: &str) -> &'a Detected { + det.iter().find(|d| d.id == id).unwrap() +} + +fn file_name(d: &Detected) -> String { + Path::new(d.path.as_deref().unwrap_or_default()) + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default() +} + +fn win_exts() -> Vec { + [".com", ".exe", ".bat", ".cmd"] + .iter() + .map(|s| s.to_string()) + .collect() +} + +#[test] +fn windows_extensions_find_exe_and_cmd_programs() { + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "cargo.exe"); + exe(bin.path(), "cargo-clippy.exe"); + // npm installs an extensionless sh shim next to the .cmd one; only the .cmd runs on Windows + exe(bin.path(), "tsc"); + exe(bin.path(), "tsc.cmd"); + exe(bin.path(), "semgrep.bat"); + let t = project(); + exe(t.path(), "node_modules/.bin/ast-grep.cmd"); + let det = providers::detect_with_exts(t.path(), &[bin.path().to_path_buf()], &win_exts()); + + let cargo = get(&det, "cargo-check"); + assert!(cargo.available, "{cargo:?}"); + assert_eq!(file_name(cargo), "cargo.exe"); + assert!(get(&det, "clippy").available); + assert_eq!( + file_name(get(&det, "tsc")), + "tsc.cmd", + "the sh shim cannot run on Windows" + ); + assert_eq!(file_name(get(&det, "semgrep")), "semgrep.bat"); + assert_eq!( + file_name(get(&det, "ast-grep")), + "ast-grep.cmd", + "project-local node_modules/.bin" + ); + assert!(!get(&det, "ruff").available, "nothing named ruff exists"); +} + +#[test] +fn windows_extension_order_follows_the_list() { + // PATHEXT order decides between two candidates in the same directory, as cmd.exe does + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "ruff.cmd"); + exe(bin.path(), "ruff.exe"); + let t = project(); + let det = providers::detect_with_exts(t.path(), &[bin.path().to_path_buf()], &win_exts()); + assert_eq!(file_name(get(&det, "ruff")), "ruff.exe"); + // and the first directory on the search path wins over a better extension later on + let first = tempfile::tempdir().unwrap(); + exe(first.path(), "ruff.bat"); + let det = providers::detect_with_exts( + t.path(), + &[first.path().to_path_buf(), bin.path().to_path_buf()], + &win_exts(), + ); + assert_eq!(file_name(get(&det, "ruff")), "ruff.bat"); +} + +#[test] +fn no_extension_list_keeps_exact_name_lookup() { + // the Unix behaviour: `cargo.exe` is not `cargo` + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "cargo.exe"); + exe(bin.path(), "ruff"); + let t = project(); + let det = providers::detect_with_exts(t.path(), &[bin.path().to_path_buf()], &[]); + assert!(!get(&det, "cargo-check").available); + let ruff = get(&det, "ruff"); + assert!(ruff.available, "{ruff:?}"); + assert_eq!(file_name(ruff), "ruff"); +} + +#[test] +fn pathext_parsing_normalises_and_keeps_only_runnable_extensions() { + assert_eq!( + providers::parse_pathext(".COM;.EXE;;.Bat ; cmd;.EXE;.VBS;.JS;.PS1;.MSC"), + vec![".com", ".exe", ".bat", ".cmd"], + "case folded, blanks and duplicates dropped, dot added, non-runnable hosts dropped" + ); + assert_eq!( + providers::parse_pathext(".CMD;.EXE"), + vec![".cmd", ".exe"], + "user order is kept" + ); + assert!(providers::parse_pathext("").is_empty()); + assert!(providers::parse_pathext(".JS;.VBS").is_empty()); + assert_eq!( + providers::parse_pathext(providers::DEFAULT_PATHEXT), + vec![".com", ".exe", ".bat", ".cmd"] + ); +} + +#[test] +fn a_non_runnable_extension_is_not_reported_as_installed() { + // `ruff.js` would need a script host; Command cannot start it + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "ruff.js"); + let t = project(); + let exts = providers::parse_pathext(".COM;.EXE;.BAT;.CMD;.JS"); + let det = providers::detect_with_exts(t.path(), &[bin.path().to_path_buf()], &exts); + assert!(!get(&det, "ruff").available); +} + +#[test] +fn platform_extension_list_matches_the_platform() { + let exts = providers::executable_extensions(); + if cfg!(windows) { + assert!(exts.iter().any(|e| e == ".exe"), "{exts:?}"); + } else { + assert!( + exts.is_empty(), + "Unix lookup must stay exact-name: {exts:?}" + ); + } +} + +#[cfg(unix)] +#[test] +fn unix_default_detection_is_exact_name_only() { + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "cargo.exe"); + let t = project(); + let det = providers::detect(t.path(), &[bin.path().to_path_buf()]); + assert!(!get(&det, "cargo-check").available); +} + +#[cfg(windows)] +#[test] +fn real_windows_detection_finds_cargo_exe() { + let bin = tempfile::tempdir().unwrap(); + exe(bin.path(), "cargo.exe"); + let t = project(); + let det = providers::detect(t.path(), &[bin.path().to_path_buf()]); + let cargo = get(&det, "cargo-check"); + assert!(cargo.available, "{cargo:?}"); + assert!(file_name(cargo).eq_ignore_ascii_case("cargo.exe")); +} From f0b9ed6a9fe446a15a5595f36013d3fe43bbf07c Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:20:12 +0300 Subject: [PATCH 2/9] fix(init): print a valid TOML Codex snippet on Windows The snippet interpolated root.display() into a TOML basic string. On Windows canonicalize() returns \\?\C:\..., and \\, \U, \o etc. are escape sequences in basic strings, so the printed block did not parse. The verbatim prefix is now stripped (\\?\C:\ -> C:\, \\?\UNC\ -> \\) and the path is encoded by toml_string: a literal string when possible, otherwise a basic string with quotes, backslashes and control characters escaped. Paths in the init report use the same plain form. Tests parse the snippet with the toml crate. --- crates/core/src/init.rs | 68 +++++++++++++++++--- crates/core/tests/init_snippet.rs | 101 ++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 7 deletions(-) create mode 100644 crates/core/tests/init_snippet.rs diff --git a/crates/core/src/init.rs b/crates/core/src/init.rs index 091688a..680b350 100644 --- a/crates/core/src/init.rs +++ b/crates/core/src/init.rs @@ -43,7 +43,7 @@ fn server_entry() -> Value { } fn merge_json(path: &Path, rep: &mut InitReport, dry: bool) -> Result<()> { - let rel = path.display().to_string(); + let rel = shown(path); let mut doc: Value = match std::fs::read_to_string(path) { Ok(s) => serde_json::from_str(&s) .with_context(|| format!("{rel} exists but is not valid JSON; not touching it"))?, @@ -82,7 +82,7 @@ fn merge_json(path: &Path, rep: &mut InitReport, dry: bool) -> Result<()> { } fn upsert_block(path: &Path, rep: &mut InitReport, dry: bool, create: bool) -> Result<()> { - let rel = path.display().to_string(); + let rel = shown(path); let existing = std::fs::read_to_string(path).ok(); if existing.is_none() && !create { return Ok(()); @@ -106,6 +106,62 @@ fn upsert_block(path: &Path, rep: &mut InitReport, dry: bool, create: bool) -> R Ok(()) } +/// Drop the Windows verbatim prefix that `canonicalize` adds: `\\?\C:\x` -> `C:\x`, +/// `\\?\UNC\srv\share` -> `\\srv\share`. Other verbatim forms (`\\?\Volume{..}\`) have no +/// plain spelling and are returned unchanged, as is every non-verbatim path. +pub fn strip_verbatim(p: &str) -> String { + if let Some(rest) = p.strip_prefix(r"\\?\UNC\") { + return format!(r"\\{rest}"); + } + if let Some(rest) = p.strip_prefix(r"\\?\") { + let b = rest.as_bytes(); + if b.len() >= 2 && b[0].is_ascii_alphabetic() && b[1] == b':' { + return rest.to_string(); + } + } + p.to_string() +} + +/// `p` for display and for config files (no verbatim prefix). +fn shown(p: &Path) -> String { + strip_verbatim(&p.display().to_string()) +} + +/// Encode `s` as a TOML string: a literal string (`'C:\x'`, no escapes, so Windows +/// backslashes stay readable) when that can hold it, else a basic string with every +/// character TOML requires escaped (`"`, `\`, control characters). +pub fn toml_string(s: &str) -> String { + if !s.contains('\'') && !s.chars().any(|c| c.is_control() && c != '\t') { + return format!("'{s}'"); + } + let mut out = String::with_capacity(s.len() + 2); + out.push('"'); + for c in s.chars() { + match c { + '"' => out.push_str("\\\""), + '\\' => out.push_str("\\\\"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + '\u{8}' => out.push_str("\\b"), + '\u{c}' => out.push_str("\\f"), + c if c.is_control() => out.push_str(&format!("\\u{:04X}", c as u32)), + c => out.push(c), + } + } + out.push('"'); + out +} + +/// The `~/.codex/config.toml` block registering the server for `root` (a path as +/// `canonicalize` returns it; a Windows verbatim prefix is removed). +pub fn codex_snippet(root: &str) -> String { + format!( + "# Codex: add to ~/.codex/config.toml (global; plumb does not edit it)\n[mcp_servers.plumbgraph]\ncommand = \"plumb\"\nargs = [\"mcp\", \"--root\", {}]\n", + toml_string(&strip_verbatim(root)) + ) +} + /// `agents`: which to configure; empty = everything detected (`AGENTS.md` is always written). pub fn init(root: &Path, agents: &[String], dry_run: bool) -> Result { let root = root.canonicalize()?; @@ -129,10 +185,8 @@ pub fn init(root: &Path, agents: &[String], dry_run: bool) -> Result merge_json(&root.join(".cursor/mcp.json"), &mut rep, dry_run)?; } if want("codex", codex || !agents.is_empty()) || agents.is_empty() { - rep.snippets.push(format!( - "# Codex: add to ~/.codex/config.toml (global; plumb does not edit it)\n[mcp_servers.plumbgraph]\ncommand = \"plumb\"\nargs = [\"mcp\", \"--root\", \"{}\"]\n", - root.display() - )); + rep.snippets + .push(codex_snippet(&root.display().to_string())); } // keep the local index out of version control, but keep the baseline let gi = root.join(".gitignore"); @@ -150,7 +204,7 @@ pub fn init(root: &Path, agents: &[String], dry_run: bool) -> Result n.push_str(".plumbgraph/\n"); std::fs::write(&gi, n)?; } - rep.written.push(gi.display().to_string()); + rep.written.push(shown(&gi)); } if rep.written.is_empty() && rep.notes.is_empty() { rep.notes.push("already configured".into()); diff --git a/crates/core/tests/init_snippet.rs b/crates/core/tests/init_snippet.rs new file mode 100644 index 0000000..1be9725 --- /dev/null +++ b/crates/core/tests/init_snippet.rs @@ -0,0 +1,101 @@ +//! `plumb init` prints a Codex `config.toml` block. It must be valid TOML for every project +//! path, including Windows verbatim paths (`\\?\C:\...`) that `canonicalize` returns. +use plumbgraph_core::init::{codex_snippet, init}; + +/// Parse the snippet as TOML and return `mcp_servers.plumbgraph.args`. +fn args_of(snippet: &str) -> Vec { + let doc: toml::Table = toml::from_str(snippet) + .unwrap_or_else(|e| panic!("snippet is not valid TOML: {e}\n---\n{snippet}")); + let server = &doc["mcp_servers"]["plumbgraph"]; + assert_eq!(server["command"].as_str(), Some("plumb")); + server["args"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() +} + +#[test] +fn codex_snippet_is_valid_toml_for_a_windows_verbatim_path() { + let args = args_of(&codex_snippet(r"\\?\C:\Users\ofekv\Desktop\repos\Wraith")); + assert_eq!( + args, + ["mcp", "--root", r"C:\Users\ofekv\Desktop\repos\Wraith"] + ); +} + +#[test] +fn codex_snippet_maps_verbatim_unc_to_a_plain_unc_path() { + let args = args_of(&codex_snippet(r"\\?\UNC\fileserver\share\proj")); + assert_eq!(args[2], r"\\fileserver\share\proj"); +} + +#[test] +fn codex_snippet_round_trips_awkward_paths() { + for p in [ + "/home/dev/my project", + r"C:\Users\O'Brien\code", + r#"C:\Users\x\"quoted"\dir"#, + r"D:\a\b\c", + "/tmp/it's \"both\"", + "/tmp/tab\there", + "/tmp/new\nline", + "/tmp/bell\u{7}\u{7f}", + "/tmp/ünïcødé/日本", + r"\\server\share\already-plain", + ] { + let args = args_of(&codex_snippet(p)); + assert_eq!(args[2], p, "round trip of {p:?}"); + } +} + +#[test] +fn plain_windows_paths_use_a_readable_literal_string() { + let s = codex_snippet(r"\\?\C:\Users\me\proj"); + assert!( + s.contains(r#"args = ["mcp", "--root", 'C:\Users\me\proj']"#), + "{s}" + ); +} + +#[test] +fn strip_verbatim_only_touches_verbatim_drive_and_unc_paths() { + use plumbgraph_core::init::strip_verbatim; + assert_eq!(strip_verbatim(r"\\?\C:\x"), r"C:\x"); + assert_eq!(strip_verbatim(r"\\?\d:\"), r"d:\"); + assert_eq!(strip_verbatim(r"\\?\UNC\srv\share"), r"\\srv\share"); + // no plain spelling exists for a volume GUID path: keep it + assert_eq!( + strip_verbatim(r"\\?\Volume{0b1c}\dir"), + r"\\?\Volume{0b1c}\dir" + ); + assert_eq!(strip_verbatim(r"\\?\"), r"\\?\"); + assert_eq!(strip_verbatim(r"C:\x"), r"C:\x"); + assert_eq!(strip_verbatim("/home/u/p"), "/home/u/p"); + assert_eq!( + strip_verbatim(r"\\.\C:\x"), + r"\\.\C:\x", + "device paths untouched" + ); +} + +#[test] +fn init_prints_a_parseable_codex_snippet_without_a_verbatim_prefix() { + let t = tempfile::tempdir().unwrap(); + let rep = init(t.path(), &["codex".to_string()], true).unwrap(); + let snippet = rep + .snippets + .iter() + .find(|s| s.contains("mcp_servers.plumbgraph")) + .expect("codex snippet"); + let root = &args_of(snippet)[2]; + assert!(!root.starts_with(r"\\?\"), "{root}"); + assert!( + std::path::Path::new(root).is_dir(), + "the printed root must be a usable path: {root}" + ); + for w in &rep.written { + assert!(!w.starts_with(r"\\?\"), "reported paths are plain: {w}"); + } +} From e5a65817d0d70624939e96dd70e3c4b8be4bbe04 Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:29:19 +0300 Subject: [PATCH 3/9] feat(csharp): Unity engine entry points for dead code MonoBehaviour messages (Awake, Start, Update, OnTriggerEnter2D, ...) are called by the engine by name, so every one was reported as dead code (0.71 medium) and so was the component class. - non-static message methods of classes deriving from MonoBehaviour, ScriptableObject, NetworkBehaviour, StateMachineBehaviour, UIBehaviour (Editor/EditorWindow in Unity files), or deriving from anything under Assets/, are entry points - [RuntimeInitializeOnLoadMethod], [InitializeOnLoadMethod], [InitializeOnLoad], [MenuItem], [ContextMenu], ... are entry points in Unity files (Assets/, Packages/, or a UnityEngine/UnityEditor using or qualified name in code; comments and strings do not count) - public methods of those classes keep a finding with a 0.25 penalty and an fp_risk about UnityEvent bindings in scenes/prefabs (new SymbolFact.framework_risk, stored in a new column; schema 3, extractor 8) Golden fixture tests/fixtures/unity; docs/PACKS.md documents the rules. --- crates/core/src/deadcode.rs | 11 +- crates/core/src/extract.rs | 240 ++++++++++++++++++ crates/core/src/model.rs | 4 + crates/core/src/store.rs | 16 +- .../unity/Assets/Editor/BuildTools.cs | 28 ++ .../unity/Assets/Scripts/Character.cs | 11 + .../fixtures/unity/Assets/Scripts/Enemy.cs | 13 + .../unity/Assets/Scripts/GameSettings.cs | 13 + .../fixtures/unity/Assets/Scripts/MathUtil.cs | 9 + .../unity/Assets/Scripts/Net/NetPlayer.cs | 12 + .../fixtures/unity/Assets/Scripts/Player.cs | 48 ++++ .../tests/fixtures/unity/Server/Worker.cs | 21 ++ crates/core/tests/golden/unity_default.txt | 13 + crates/core/tests/golden/unity_low.txt | 15 ++ crates/core/tests/unity_entry_points.rs | 229 +++++++++++++++++ docs/PACKS.md | 12 +- 16 files changed, 685 insertions(+), 10 deletions(-) create mode 100644 crates/core/tests/fixtures/unity/Assets/Editor/BuildTools.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/Character.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/Enemy.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/GameSettings.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/MathUtil.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/Net/NetPlayer.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/Player.cs create mode 100644 crates/core/tests/fixtures/unity/Server/Worker.cs create mode 100644 crates/core/tests/golden/unity_default.txt create mode 100644 crates/core/tests/golden/unity_low.txt create mode 100644 crates/core/tests/unity_entry_points.rs diff --git a/crates/core/src/deadcode.rs b/crates/core/src/deadcode.rs index 7f3ae38..96357d0 100644 --- a/crates/core/src/deadcode.rs +++ b/crates/core/src/deadcode.rs @@ -1,12 +1,13 @@ //! Dead-code analysis: reachability from entry points over the tier-0 reference graph, //! then a confidence score per unreachable symbol. //! -//! Confidence model (documented in docs/DEAD_CODE.md): +//! Confidence model: //! `score = clamp((0.70 + bonuses) * (1 - sum(penalties)), 0, 0.99)` //! bonuses: private symbol +0.10, name occurs nowhere else in the indexed code +0.15. //! penalties: dynamic-access markers in file 0.20, name appears in a string literal 0.15, -//! decorated/attributed 0.30, exported (app mode) 0.10, imported-by-name elsewhere 0.20, -//! file has syntax errors 0.10. +//! decorated/attributed 0.30, method of a subclass 0.25, framework risk (e.g. a public +//! Unity component method that a UnityEvent may call) 0.25, exported (app mode) 0.10, +//! imported-by-name elsewhere 0.20, file has syntax errors 0.10. //! Levels: high >= 0.90, medium >= 0.70, low otherwise (hidden by default). use crate::store::{Graph, SymRow}; @@ -556,6 +557,10 @@ fn score( pen += 0.25; fp.push("method of a class that extends/implements another type: it may override or implement a base/framework method that is called from outside the indexed code".into()); } + if let Some(r) = &s.framework_risk { + pen += 0.25; + fp.push(sanitize(r)); + } if s.exported { pen += 0.10; fp.push("exported/public: may be used by consumers outside the indexed code".into()); diff --git a/crates/core/src/extract.rs b/crates/core/src/extract.rs index 357fb1c..fb2c9e2 100644 --- a/crates/core/src/extract.rs +++ b/crates/core/src/extract.rs @@ -263,6 +263,216 @@ const CL_NEUTRAL_ANNOTATIONS: &[&str] = &[ "Nullable", ]; +/// Unity base types whose subclasses receive engine messages by method name. Names that are +/// only used by Unity; `Editor`/`EditorWindow` are generic and need `UNITY_EDITOR_BASES` context. +const UNITY_BASES: &[&str] = &[ + "MonoBehaviour", + "ScriptableObject", + "NetworkBehaviour", + "StateMachineBehaviour", + "UIBehaviour", +]; +const UNITY_EDITOR_BASES: &[&str] = &["Editor", "EditorWindow"]; + +/// Methods the Unity engine calls by name (reflection on the message name, not a virtual call). +/// MonoBehaviour / ScriptableObject messages, physics (3D and 2D), rendering, input, animation, +/// StateMachineBehaviour, Editor/EditorWindow and ISerializationCallbackReceiver callbacks. +const UNITY_MESSAGES: &[&str] = &[ + "Awake", + "Start", + "Update", + "LateUpdate", + "FixedUpdate", + "OnEnable", + "OnDisable", + "OnDestroy", + "OnValidate", + "Reset", + "OnGUI", + "OnApplicationFocus", + "OnApplicationPause", + "OnApplicationQuit", + "OnBecameVisible", + "OnBecameInvisible", + "OnCollisionEnter", + "OnCollisionStay", + "OnCollisionExit", + "OnCollisionEnter2D", + "OnCollisionStay2D", + "OnCollisionExit2D", + "OnTriggerEnter", + "OnTriggerStay", + "OnTriggerExit", + "OnTriggerEnter2D", + "OnTriggerStay2D", + "OnTriggerExit2D", + "OnControllerColliderHit", + "OnJointBreak", + "OnJointBreak2D", + "OnParticleCollision", + "OnParticleTrigger", + "OnParticleSystemStopped", + "OnMouseDown", + "OnMouseUp", + "OnMouseUpAsButton", + "OnMouseEnter", + "OnMouseExit", + "OnMouseOver", + "OnMouseDrag", + "OnPreCull", + "OnPreRender", + "OnPostRender", + "OnRenderImage", + "OnRenderObject", + "OnWillRenderObject", + "OnDrawGizmos", + "OnDrawGizmosSelected", + "OnAnimatorMove", + "OnAnimatorIK", + "OnAudioFilterRead", + "OnTransformChildrenChanged", + "OnTransformParentChanged", + "OnBeforeTransformParentChanged", + "OnRectTransformDimensionsChange", + "OnCanvasGroupChanged", + "OnStateEnter", + "OnStateExit", + "OnStateUpdate", + "OnStateMove", + "OnStateIK", + "OnStateMachineEnter", + "OnStateMachineExit", + "OnInspectorGUI", + "OnSceneGUI", + "CreateInspectorGUI", + "CreateGUI", + "OnSelectionChange", + "OnFocus", + "OnLostFocus", + "OnHierarchyChange", + "OnProjectChange", + "OnInspectorUpdate", + "OnBeforeSerialize", + "OnAfterDeserialize", +]; + +/// Unity attributes that make the engine/editor call the method (or, for `InitializeOnLoad`, +/// run the class's static constructor). Matched without a trailing `Attribute`. +const UNITY_ENTRY_ATTRIBUTES: &[&str] = &[ + "RuntimeInitializeOnLoadMethod", + "InitializeOnLoadMethod", + "InitializeOnLoad", + "MenuItem", + "ContextMenu", + "DidReloadScripts", + "PostProcessBuild", + "PostProcessScene", + "OnOpenAsset", +]; + +/// Unity context for a C# file: under `Assets/` or `Packages/` (a Unity project layout), or +/// code (not comments or strings) that imports or names the `UnityEngine`/`UnityEditor` +/// namespaces: a `using` directive or a qualified name such as `UnityEditor.MenuItem`. +fn unity_file(rel_path: &str, root: Node, src: &[u8]) -> bool { + let under = |dir: &str| { + rel_path.starts_with(&format!("{dir}/")) || rel_path.contains(&format!("/{dir}/")) + }; + if under("Assets") || under("Packages") { + return true; + } + let names_unity = |t: &str| { + let t = t.trim().trim_start_matches("global::"); + ["UnityEngine", "UnityEditor"] + .iter() + .any(|ns| t == *ns || t.starts_with(&format!("{ns}."))) + }; + let mut stack = vec![root]; + while let Some(n) = stack.pop() { + match n.kind() { + "using_directive" => { + let mut c = n.walk(); + if n.named_children(&mut c) + .any(|ch| names_unity(text(ch, src))) + { + return true; + } + } + "qualified_name" | "alias_qualified_name" if names_unity(text(n, src)) => return true, + "comment" + | "string_literal" + | "verbatim_string_literal" + | "raw_string_literal" + | "interpolated_string_expression" => {} + _ => { + let mut c = n.walk(); + stack.extend(n.children(&mut c)); + } + } + } + false +} + +fn under_assets(rel_path: &str) -> bool { + rel_path.starts_with("Assets/") || rel_path.contains("/Assets/") +} + +/// Base type names (last path segment, generics stripped) of a C# class declaration. +fn cs_base_names(class_node: Node, src: &[u8]) -> Vec { + let mut out = vec![]; + let mut c = class_node.walk(); + for ch in class_node.children(&mut c) { + if ch.kind() != "base_list" { + continue; + } + let mut cc = ch.walk(); + for b in ch.named_children(&mut cc) { + let t = text(b, src); + let head = t.split(['<', '(']).next().unwrap_or(t).trim(); + let last = head.rsplit(['.', ':']).next().unwrap_or(head).trim(); + if !last.is_empty() { + out.push(last.to_string()); + } + } + } + out +} + +/// How a C# class relates to Unity: `Some(reason)` when its methods may be called by the engine. +fn unity_class( + class_node: Node, + src: &[u8], + rel_path: &str, + in_unity_file: bool, +) -> Option { + let bases = cs_base_names(class_node, src); + if let Some(b) = bases.iter().find(|b| UNITY_BASES.contains(&b.as_str())) { + return Some(format!("derives from {b}")); + } + if in_unity_file { + if let Some(b) = bases + .iter() + .find(|b| UNITY_EDITOR_BASES.contains(&b.as_str())) + { + return Some(format!("derives from {b}")); + } + } + if !bases.is_empty() && under_assets(rel_path) { + // indirect subclass (`Enemy : Character : MonoBehaviour`): the chain is not resolved, + // so any derived class in a Unity project's Assets/ is treated as possibly a component + return Some(format!( + "derives from {} under Assets/ (possibly an indirect MonoBehaviour)", + bases[0] + )); + } + None +} + +fn unity_attribute(anns: &[String]) -> Option<&str> { + anns.iter() + .map(|a| a.strip_suffix("Attribute").unwrap_or(a.as_str())) + .find(|a| UNITY_ENTRY_ATTRIBUTES.contains(a)) +} + const GO_WELL_KNOWN_METHODS: &[&str] = &[ "String", "Error", @@ -516,6 +726,9 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { || rel_path.starts_with("bin/") || rel_path.contains("examples/"); + let is_csharp = pack.manifest.id == "csharp"; + let cs_unity_file = is_csharp && unity_file(rel_path, root, src); + let mut syms: Vec = Vec::with_capacity(defs.len()); for (i, d) in defs.iter().enumerate() { let mut name = text(d.name_node, src).to_string(); @@ -573,6 +786,7 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { let mut entry_cond: Option = None; let mut decorated = false; let mut subclass_method = false; + let mut framework_risk: Option = None; match family { Family::Python => { @@ -690,6 +904,31 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { entry = Some("program entry point".into()); } } + if is_csharp { + if let Some(a) = unity_attribute(&anns).filter(|_| cs_unity_file) { + if entry.is_none() { + entry = + Some(format!("Unity `[{a}]`: invoked by the engine/editor")); + } + } + let unity_owner = parent + .filter(|&p| syms[p].kind == "class") + .and_then(|p| unity_class(defs[p].def, src, rel_path, cs_unity_file)); + if let (Some(why), true) = (unity_owner, kind == "method" && !has("static")) + { + if UNITY_MESSAGES.contains(&name.as_str()) { + if entry.is_none() { + entry = Some(format!( + "Unity message `{name}` (class {why}): called by the engine by name" + )); + } + } else if entry.is_none() && has("public") { + framework_risk = Some(format!( + "public method of a Unity class ({why}): may be bound to a UnityEvent (e.g. Button.onClick) or called via SendMessage in a scene/prefab, which is not indexed" + )); + } + } + } } } Family::Rust => { @@ -872,6 +1111,7 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { entry_cond, decorated, subclass_method, + framework_risk, keep, parent, }); diff --git a/crates/core/src/model.rs b/crates/core/src/model.rs index f0d8edd..ec6ec94 100644 --- a/crates/core/src/model.rs +++ b/crates/core/src/model.rs @@ -23,6 +23,10 @@ pub struct SymbolFact { pub decorated: bool, /// Method of a class that extends/implements another type (may override a base/framework method). pub subclass_method: bool, + /// A framework may call it from outside the indexed code (e.g. a UnityEvent bound in a + /// scene). The text becomes a dead-code `fp_risk` and lowers confidence. + #[serde(default)] + pub framework_risk: Option, /// `plumb:keep(reason)` annotation text. pub keep: Option, /// Index of the lexically enclosing symbol in the same file. diff --git a/crates/core/src/store.rs b/crates/core/src/store.rs index 97e8c02..976d6d3 100644 --- a/crates/core/src/store.rs +++ b/crates/core/src/store.rs @@ -6,9 +6,9 @@ use rusqlite::{params, Connection}; use std::collections::HashMap; use std::path::Path; -pub const SCHEMA_VERSION: &str = "2"; +pub const SCHEMA_VERSION: &str = "3"; /// Bump when extraction logic changes so cached facts are re-extracted. -pub const EXTRACTOR_VERSION: &str = "7"; +pub const EXTRACTOR_VERSION: &str = "8"; pub struct Store { pub conn: Connection, @@ -23,7 +23,7 @@ CREATE TABLE IF NOT EXISTS file( CREATE TABLE IF NOT EXISTS symbol( id INTEGER PRIMARY KEY, file_id INTEGER NOT NULL REFERENCES file(id) ON DELETE CASCADE, idx INTEGER NOT NULL, kind TEXT NOT NULL, name TEXT NOT NULL, qname TEXT NOT NULL, start_line INTEGER, end_line INTEGER, - exported INTEGER, is_test INTEGER, entry TEXT, entry_cond TEXT, decorated INTEGER, subclass INTEGER NOT NULL DEFAULT 0, keep TEXT, parent_idx INTEGER); + exported INTEGER, is_test INTEGER, entry TEXT, entry_cond TEXT, decorated INTEGER, subclass INTEGER NOT NULL DEFAULT 0, keep TEXT, parent_idx INTEGER, framework_risk TEXT); CREATE INDEX IF NOT EXISTS symbol_name ON symbol(name); CREATE INDEX IF NOT EXISTS symbol_file ON symbol(file_id); CREATE TABLE IF NOT EXISTS ref( @@ -172,7 +172,7 @@ impl Store { let Some(f) = facts else { return Ok(id) }; { let mut st = self.conn.prepare_cached( - "INSERT INTO symbol(file_id,idx,kind,name,qname,start_line,end_line,exported,is_test,entry,entry_cond,decorated,subclass,keep,parent_idx) VALUES(?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15)", + "INSERT INTO symbol(file_id,idx,kind,name,qname,start_line,end_line,exported,is_test,entry,entry_cond,decorated,subclass,keep,parent_idx,framework_risk) VALUES(?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12,?13,?14,?15,?16)", )?; for (i, s) in f.symbols.iter().enumerate() { st.execute(params![ @@ -190,7 +190,8 @@ impl Store { s.decorated, s.subclass_method, s.keep, - s.parent.map(|p| p as i64) + s.parent.map(|p| p as i64), + s.framework_risk ])?; } } @@ -264,6 +265,7 @@ pub struct SymRow { pub subclass: bool, pub keep: Option, pub parent_idx: Option, + pub framework_risk: Option, } #[derive(Debug, Clone)] @@ -336,7 +338,7 @@ impl Graph { })? { g.files.push(r?); } - let mut st = c.prepare("SELECT id,file_id,idx,kind,name,qname,start_line,end_line,exported,is_test,entry,entry_cond,decorated,subclass,keep,parent_idx FROM symbol ORDER BY file_id, idx")?; + let mut st = c.prepare("SELECT id,file_id,idx,kind,name,qname,start_line,end_line,exported,is_test,entry,entry_cond,decorated,subclass,keep,parent_idx,framework_risk FROM symbol ORDER BY file_id, idx")?; for r in st.query_map([], |r| { Ok(SymRow { id: r.get(0)?, @@ -355,6 +357,7 @@ impl Graph { subclass: r.get(13)?, keep: r.get(14)?, parent_idx: r.get(15)?, + framework_risk: r.get(16)?, }) })? { g.symbols.push(r?); @@ -445,6 +448,7 @@ mod tests { entry_cond: None, decorated: false, subclass_method: false, + framework_risk: None, keep: None, parent: None, }); diff --git a/crates/core/tests/fixtures/unity/Assets/Editor/BuildTools.cs b/crates/core/tests/fixtures/unity/Assets/Editor/BuildTools.cs new file mode 100644 index 0000000..e4ffe13 --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Editor/BuildTools.cs @@ -0,0 +1,28 @@ +using UnityEditor; +using UnityEngine; + +namespace Game.EditorTools +{ + internal static class BuildTools + { + [MenuItem("Tools/Bake Lighting")] + private static void Bake() { } + + [UnityEditor.MenuItemAttribute("Tools/Clear Cache")] + private static void ClearCache() { } + + [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSceneLoad)] + private static void Boot() { } + + [InitializeOnLoadMethod] + private static void OnEditorLoad() { } + + private static void OrphanTool() { } + } + + [InitializeOnLoad] + internal class StartupHook + { + static StartupHook() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/Character.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/Character.cs new file mode 100644 index 0000000..0b84199 --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/Character.cs @@ -0,0 +1,11 @@ +using UnityEngine; + +namespace Game +{ + public abstract class Character : MonoBehaviour + { + protected virtual void Awake() { } + + private void UnusedCharacterHelper() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/Enemy.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/Enemy.cs new file mode 100644 index 0000000..51db63a --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/Enemy.cs @@ -0,0 +1,13 @@ +namespace Game +{ + // Indirect MonoBehaviour (through Character, in another file): recognised because it + // derives from something and lives under Assets/. + internal class Enemy : Character + { + void Update() { Chase(); } + void OnTriggerEnter(UnityEngine.Collider other) { } + + private void Chase() { } + private void EnemyDeadHelper() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/GameSettings.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/GameSettings.cs new file mode 100644 index 0000000..b52afde --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/GameSettings.cs @@ -0,0 +1,13 @@ +using UnityEngine; + +namespace Game +{ + [CreateAssetMenu(menuName = "Game/Settings")] + public class GameSettings : ScriptableObject + { + void OnEnable() { } + void OnValidate() { } + + private void StaleSettingsHelper() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/MathUtil.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/MathUtil.cs new file mode 100644 index 0000000..460c39f --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/MathUtil.cs @@ -0,0 +1,9 @@ +namespace Game +{ + // Under Assets/ but derives from nothing: Unity never calls these by name. + internal static class MathUtil + { + private static void Update() { } + private static void Start() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/Net/NetPlayer.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/Net/NetPlayer.cs new file mode 100644 index 0000000..98f69da --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/Net/NetPlayer.cs @@ -0,0 +1,12 @@ +using Mirror; + +namespace Game.Net +{ + internal class NetPlayer : NetworkBehaviour + { + void Start() { } + void Update() { } + + private void NetDeadHelper() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/Player.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/Player.cs new file mode 100644 index 0000000..dc9214a --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/Player.cs @@ -0,0 +1,48 @@ +using UnityEngine; +using UnityEngine.Events; + +namespace Game +{ + // Attached to a GameObject in a scene: nothing in code references this class. + public class Player : MonoBehaviour + { + [SerializeField] private float speed = 3f; + [SerializeField] private UnityEvent onDied; + + private void Awake() { } + void Start() { Init(); } + void Update() { Move(); } + void LateUpdate() { } + void FixedUpdate() { } + void OnEnable() { } + void OnDisable() { } + void OnDestroy() { } + void OnCollisionEnter(Collision c) { } + void OnCollisionStay(Collision c) { } + void OnCollisionExit(Collision c) { } + void OnTriggerEnter(Collider other) { } + void OnTriggerStay(Collider other) { } + void OnTriggerExit(Collider other) { } + void OnCollisionEnter2D(Collision2D c) { } + void OnCollisionStay2D(Collision2D c) { } + void OnCollisionExit2D(Collision2D c) { } + void OnTriggerEnter2D(Collider2D other) { } + void OnTriggerStay2D(Collider2D other) { } + void OnTriggerExit2D(Collider2D other) { } + void OnGUI() { } + void OnValidate() { } + void Reset() { } + + [ContextMenu("Respawn")] + private void RespawnFromInspector() { } + + private void Init() { } + private void Move() { transform.Translate(Vector3.forward * speed); } + + // Bound to Button.onClick in the scene (not indexed): only a low-confidence lead. + public void Jump() { } + + // Real dead code: nothing calls it and Unity does not either. + private void NeverCalled() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Server/Worker.cs b/crates/core/tests/fixtures/unity/Server/Worker.cs new file mode 100644 index 0000000..6c1a32d --- /dev/null +++ b/crates/core/tests/fixtures/unity/Server/Worker.cs @@ -0,0 +1,21 @@ +using System; + +namespace Server +{ + // Not Unity: outside Assets/, no Unity base type. Same method names must still be reported. + // (Mentioning UnityEngine or UnityEditor in a comment must not make this a Unity file.) + internal class Worker : IDisposable + { + private const string Note = "UnityEditor.MenuItem in a string does not count either"; + + public void Dispose() { } + void Start() { } + void Update() { } + [MenuItem] + void Menu() { } + } + + // Not reported: tier-0 resolves by name, so `[UnityEditor.MenuItemAttribute]` in + // Assets/Editor/BuildTools.cs (a live method) links to this same-named class. + internal class MenuItemAttribute : Attribute { } +} diff --git a/crates/core/tests/golden/unity_default.txt b/crates/core/tests/golden/unity_default.txt new file mode 100644 index 0000000..9f32a9b --- /dev/null +++ b/crates/core/tests/golden/unity_default.txt @@ -0,0 +1,13 @@ +high 0.95 dead-code class MathUtil Assets/Scripts/MathUtil.cs:4 +high 0.95 dead-code class Worker Server/Worker.cs:7 +high 0.95 dead-code method BuildTools.OrphanTool Assets/Editor/BuildTools.cs:20 +high 0.95 dead-code method MathUtil.Start Assets/Scripts/MathUtil.cs:7 +high 0.95 dead-code method MathUtil.Update Assets/Scripts/MathUtil.cs:6 +medium 0.71 dead-code method Character.UnusedCharacterHelper Assets/Scripts/Character.cs:9 +medium 0.71 dead-code method Enemy.EnemyDeadHelper Assets/Scripts/Enemy.cs:11 +medium 0.71 dead-code method GameSettings.StaleSettingsHelper Assets/Scripts/GameSettings.cs:11 +medium 0.71 dead-code method NetPlayer.NetDeadHelper Assets/Scripts/Net/NetPlayer.cs:10 +medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Player.cs:46 +medium 0.71 dead-code method Worker.Dispose Server/Worker.cs:11 +medium 0.71 dead-code method Worker.Start Server/Worker.cs:12 +medium 0.71 dead-code method Worker.Update Server/Worker.cs:13 diff --git a/crates/core/tests/golden/unity_low.txt b/crates/core/tests/golden/unity_low.txt new file mode 100644 index 0000000..980c477 --- /dev/null +++ b/crates/core/tests/golden/unity_low.txt @@ -0,0 +1,15 @@ +high 0.95 dead-code class MathUtil Assets/Scripts/MathUtil.cs:4 +high 0.95 dead-code class Worker Server/Worker.cs:7 +high 0.95 dead-code method BuildTools.OrphanTool Assets/Editor/BuildTools.cs:20 +high 0.95 dead-code method MathUtil.Start Assets/Scripts/MathUtil.cs:7 +high 0.95 dead-code method MathUtil.Update Assets/Scripts/MathUtil.cs:6 +low 0.34 dead-code method Player.Jump Assets/Scripts/Player.cs:43 +low 0.43 dead-code method Worker.Menu Server/Worker.cs:14 +medium 0.71 dead-code method Character.UnusedCharacterHelper Assets/Scripts/Character.cs:9 +medium 0.71 dead-code method Enemy.EnemyDeadHelper Assets/Scripts/Enemy.cs:11 +medium 0.71 dead-code method GameSettings.StaleSettingsHelper Assets/Scripts/GameSettings.cs:11 +medium 0.71 dead-code method NetPlayer.NetDeadHelper Assets/Scripts/Net/NetPlayer.cs:10 +medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Player.cs:46 +medium 0.71 dead-code method Worker.Dispose Server/Worker.cs:11 +medium 0.71 dead-code method Worker.Start Server/Worker.cs:12 +medium 0.71 dead-code method Worker.Update Server/Worker.cs:13 diff --git a/crates/core/tests/unity_entry_points.rs b/crates/core/tests/unity_entry_points.rs new file mode 100644 index 0000000..327eed6 --- /dev/null +++ b/crates/core/tests/unity_entry_points.rs @@ -0,0 +1,229 @@ +//! Unity C#: engine-invoked methods (MonoBehaviour messages, Unity attributes) are entry +//! points; public component methods are only low-confidence leads (UnityEvent bindings live in +//! scenes/prefabs, which are not indexed). Fixture: `tests/fixtures/unity`. +mod common; +use common::*; +use plumbgraph_core::ops::{run_dead_code, DeadCodeParams, ScipOpts, Target}; +use plumbgraph_core::Finding; + +fn all_findings() -> Vec { + let t = copy_fixture("unity"); + let dbdir = tempfile::tempdir().unwrap(); + run_dead_code( + &Target { + root: t.path().to_path_buf(), + db: Some(dbdir.path().join("i.db")), + }, + &DeadCodeParams { + min_confidence: 0.0, + scip: ScipOpts::disabled(), + ..Default::default() + }, + ) + .unwrap() + .findings +} + +fn symbols(f: &[Finding]) -> Vec { + let mut v: Vec = f.iter().filter_map(|x| x.symbol.clone()).collect(); + v.sort(); + // positive control: proves the `Class.Method` naming the absence checks below rely on + // (C# namespaces are not part of the qualified name), so they cannot pass vacuously + assert!(v.iter().any(|q| q == "Player.NeverCalled"), "{v:?}"); + v +} + +const MESSAGES: &[&str] = &[ + "Awake", + "Start", + "Update", + "LateUpdate", + "FixedUpdate", + "OnEnable", + "OnDisable", + "OnDestroy", + "OnCollisionEnter", + "OnCollisionStay", + "OnCollisionExit", + "OnTriggerEnter", + "OnTriggerStay", + "OnTriggerExit", + "OnCollisionEnter2D", + "OnCollisionStay2D", + "OnCollisionExit2D", + "OnTriggerEnter2D", + "OnTriggerStay2D", + "OnTriggerExit2D", + "OnGUI", + "OnValidate", + "Reset", +]; + +#[test] +fn monobehaviour_messages_are_entry_points() { + let s = symbols(&all_findings()); + for m in MESSAGES { + let q = format!("Player.{m}"); + assert!(!s.contains(&q), "false positive {q}: {s:?}"); + } + // the class itself is live (its messages are), and so is what they call + for q in ["Player", "Player.Init", "Player.Move"] { + assert!(!s.contains(&q.to_string()), "false positive {q}: {s:?}"); + } +} + +#[test] +fn scriptableobject_networkbehaviour_and_indirect_subclasses_under_assets() { + let s = symbols(&all_findings()); + for q in [ + "GameSettings.OnEnable", + "GameSettings.OnValidate", + "NetPlayer.Start", + "NetPlayer.Update", + "Character.Awake", + // Enemy : Character (a MonoBehaviour declared in another file), under Assets/ + "Enemy.Update", + "Enemy.OnTriggerEnter", + "Enemy.Chase", + ] { + assert!(!s.contains(&q.to_string()), "false positive {q}: {s:?}"); + } +} + +#[test] +fn unity_attributes_are_entry_points() { + let s = symbols(&all_findings()); + for q in [ + "BuildTools.Bake", + "BuildTools.ClearCache", + "BuildTools.Boot", + "BuildTools.OnEditorLoad", + "StartupHook", + "Player.RespawnFromInspector", + ] { + assert!(!s.contains(&q.to_string()), "false positive {q}: {s:?}"); + } +} + +#[test] +fn real_dead_code_in_unity_projects_is_still_reported() { + let s = symbols(&all_findings()); + for q in [ + "Player.NeverCalled", + "Character.UnusedCharacterHelper", + "Enemy.EnemyDeadHelper", + "GameSettings.StaleSettingsHelper", + "NetPlayer.NetDeadHelper", + "BuildTools.OrphanTool", + // same names, but no Unity base type: not called by the engine + "MathUtil.Update", + "MathUtil.Start", + "Worker.Start", + "Worker.Update", + // `[MenuItem]` outside a Unity project is just an attribute + "Worker.Menu", + ] { + assert!(s.contains(&q.to_string()), "missing {q}: {s:?}"); + } +} + +#[test] +fn public_component_methods_are_low_confidence_with_a_unityevent_risk() { + let f = all_findings(); + let jump = f + .iter() + .find(|x| x.symbol.as_deref() == Some("Player.Jump")) + .expect("Jump is still reported, as a lead"); + assert_eq!(jump.level.as_str(), "low", "{jump:?}"); + assert!( + jump.fp_risks.iter().any(|r| r.contains("UnityEvent")), + "{:?}", + jump.fp_risks + ); + // private helpers in the same class do not get that risk + let never = f + .iter() + .find(|x| x.symbol.as_deref() == Some("Player.NeverCalled")) + .unwrap(); + assert!(!never.fp_risks.iter().any(|r| r.contains("UnityEvent"))); + assert_ne!(never.level.as_str(), "low", "{never:?}"); +} + +#[test] +fn serialize_field_fields_are_never_reported() { + // C# fields are not extracted as symbols, so `[SerializeField]` fields cannot be + // reported; this guards that if field extraction is ever added. + let s = symbols(&all_findings()); + assert!( + !s.iter() + .any(|q| q.ends_with(".speed") || q.ends_with(".onDied")), + "{s:?}" + ); +} + +#[test] +fn outside_assets_unity_context_comes_from_code_not_comments_or_strings() { + let t = tempfile::tempdir().unwrap(); + let w = |rel: &str, c: &str| { + let p = t.path().join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, c).unwrap(); + }; + // a real `using UnityEditor;` (e.g. an embedded package outside Assets/) + w( + "Tools/Real.cs", + "using UnityEditor;\nstatic class RealTools {\n [MenuItem(\"X/Y\")]\n static void RealMenu() { }\n}\n", + ); + // a fully qualified attribute, no using + w( + "Tools/Qualified.cs", + "static class QualifiedTools {\n [global::UnityEditor.MenuItem(\"X/Z\")]\n static void QualifiedMenu() { }\n}\n", + ); + // only a comment, a string and a look-alike namespace mention Unity + w( + "Tools/Fake.cs", + "// using UnityEditor;\nusing UnityEditorTools;\nstatic class FakeTools {\n const string S = \"UnityEngine.Debug\";\n [MenuItem]\n static void FakeMenu() { }\n}\n", + ); + let r = run_dead_code( + &Target { + root: t.path().to_path_buf(), + db: None, + }, + &DeadCodeParams { + min_confidence: 0.0, + scip: ScipOpts::disabled(), + ..Default::default() + }, + ) + .unwrap(); + let s: Vec = r.findings.iter().filter_map(|f| f.symbol.clone()).collect(); + assert!(!s.contains(&"RealTools.RealMenu".to_string()), "{s:?}"); + assert!( + !s.contains(&"QualifiedTools.QualifiedMenu".to_string()), + "{s:?}" + ); + assert!(s.contains(&"FakeTools.FakeMenu".to_string()), "{s:?}"); +} + +#[test] +fn unity_golden_default() { + let t = copy_fixture("unity"); + let dbdir = tempfile::tempdir().unwrap(); + let r = run_dead_code( + &Target { + root: t.path().to_path_buf(), + db: Some(dbdir.path().join("i.db")), + }, + &DeadCodeParams { + scip: ScipOpts::disabled(), + ..Default::default() + }, + ) + .unwrap(); + assert_golden("unity_default", &golden_lines(&r.findings)); +} + +#[test] +fn unity_golden_include_low() { + assert_golden("unity_low", &golden_lines(&all_findings())); +} diff --git a/docs/PACKS.md b/docs/PACKS.md index 804e46b..67c5c54 100644 --- a/docs/PACKS.md +++ b/docs/PACKS.md @@ -20,6 +20,16 @@ Tier-0 only (tree-sitter, name-based). What each pack knows: |---|---|---|---|---| | Go | func, method, struct, interface, type, const | capitalised name | `main`, `init`, well-known interface methods (`String`, `Error`, `ServeHTTP`, …); `*_test.go` | package path matched to a directory of the project (suffix match), else stdlib (no dot in first segment) / external | | Java | class, interface, enum, record, annotation type, method, constructor | `public` (members also need an exported parent; interface members are public) | `static main`, `@Override`, constructors; `@Test`-style annotations, `src/test/**`; other annotations mark a symbol *decorated* (lower confidence) | qualified name matched to `a/b/C.java` or a package directory; `java.*`/`javax.*` stdlib | -| C# | class, interface, struct, enum, record, method, constructor | `public` | `static Main`, `override`, constructors; `[Fact]`/`[Test]`/`[TestMethod]`-style attributes | `using` is namespace based and namespaces are not indexed: `System.*`/`Microsoft.*` = stdlib, everything else `unresolved` | +| C# | class, interface, struct, enum, record, method, constructor | `public` | `static Main`, `override`, constructors; `[Fact]`/`[Test]`/`[TestMethod]`-style attributes; Unity messages and attributes (below) | `using` is namespace based and namespaces are not indexed: `System.*`/`Microsoft.*` = stdlib, everything else `unresolved` | + +### Unity (C#) + +The engine calls MonoBehaviour messages by name, so tier-0 sees no caller. Rules (golden fixture: `crates/core/tests/fixtures/unity`, tests: `crates/core/tests/unity_entry_points.rs`): + +- **Message methods are entry points** (`Awake`, `Start`, `Update`, `LateUpdate`, `FixedUpdate`, `OnEnable`, `OnDisable`, `OnDestroy`, `OnValidate`, `Reset`, `OnGUI`, `OnCollision*`/`OnTrigger*` and their `2D` variants, plus the rest of the list in `UNITY_MESSAGES` in `crates/core/src/extract.rs`) when they are non-static methods of a class that derives directly from `MonoBehaviour`, `ScriptableObject`, `NetworkBehaviour`, `StateMachineBehaviour` or `UIBehaviour` (`Editor`/`EditorWindow` only in a Unity file, see below), **or** that derives from anything and lives under `Assets/` (the inheritance chain is not resolved, so `Enemy : Character : MonoBehaviour` is recognised this way). The class is then live too. +- **Attributes are entry points** in a Unity file: `[RuntimeInitializeOnLoadMethod]`, `[InitializeOnLoadMethod]`, `[InitializeOnLoad]` (class), `[MenuItem]`, `[ContextMenu]`, `[DidReloadScripts]`, `[PostProcessBuild]`, `[PostProcessScene]`, `[OnOpenAsset]` (with or without the `Attribute` suffix). A Unity file is one under `Assets/` or `Packages/`, or whose code (not comments or strings) has a `using UnityEngine…`/`using UnityEditor…` directive or a `UnityEngine.`/`UnityEditor.` qualified name. +- **Public methods of such classes** stay findings but get a 0.25 penalty and the fp_risk "may be bound to a UnityEvent (e.g. Button.onClick) or called via SendMessage in a scene/prefab, which is not indexed". Scenes, prefabs and `.asset` files are not parsed, so plumb cannot tell a bound handler from dead code. +- `[SerializeField]` fields: C# fields are not extracted, so they are never reported (tested, so this is noticed if field extraction is added). +- Not covered: messages invoked through `SendMessage("Name")` / `Invoke("Name")` strings are only a string-mention penalty; `Editor`/`EditorWindow` subclasses outside a Unity file; no Unity project or editor was run to produce these rules (they come from the Unity scripting reference). Not covered: fields/properties/events, local functions, Go package-level `var`, generics-aware resolution, partial classes across files, DI containers/reflection (only marker strings lower confidence), registry checks for Go/Maven/NuGet (`check-deps` skips these languages). Dead-code calls to interface-implementing methods without `@Override`/`override` are flagged *subclass method* (lower confidence), not proven live. From 779119befddec89852ebd9139bd5963964f282ff Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:30:45 +0300 Subject: [PATCH 4/9] fix(cli): accept map --budget as an alias of --tokens plumb map --budget 1500 failed with 'unexpected argument'; --tokens was the only spelling although the help, output and docs all call it a budget. --budget is now a visible clap alias (shown in --help), passing both is a usage error (exit 2), and the README quickstart shows the flag. --- README.md | 2 +- crates/cli/src/lib.rs | 2 +- crates/cli/tests/cli.rs | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1e06957..eace928 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Nothing is published yet (no crate, no binaries). Build from source with Rust 1. git clone https://github.com/grloper/plumbgraph && cd plumbgraph cargo install --path crates/cli --locked # installs `plumb` (and `plumbgraph`) -plumb map . # ranked, token-budgeted overview +plumb map . --tokens 1500 # ranked overview under a hard token budget (--budget is an alias) plumb dead-code . # add --lib for libraries (exports = public API) plumb check-deps . --online # sends package names only to the registries plumb verify . # one pass/fail gate (exit 1 on new findings) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 6ebc094..c13a437 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -186,7 +186,7 @@ enum Cmd { #[arg(default_value = ".")] path: PathBuf, /// Hard token budget (estimated as chars/4) - #[arg(long, default_value_t = 1500)] + #[arg(long, visible_alias = "budget", default_value_t = 1500)] tokens: usize, /// Boost files changed vs this git revision (use `--changed` alone for HEAD) #[arg(long, num_args = 0..=1, default_missing_value = "HEAD", value_name = "REV")] diff --git a/crates/cli/tests/cli.rs b/crates/cli/tests/cli.rs index 966f33f..2140f74 100644 --- a/crates/cli/tests/cli.rs +++ b/crates/cli/tests/cli.rs @@ -479,6 +479,41 @@ fn map_and_impact_commands() { assert_eq!(out.status.code(), Some(2)); } +#[test] +fn map_budget_is_an_alias_of_tokens() { + let t = tempfile::tempdir().unwrap(); + write(t.path(), "lib.py", "def core():\n return 1\n"); + let run = |args: &[&str]| { + plumb() + .arg("map") + .arg(t.path()) + .args(args) + .output() + .unwrap() + }; + let by_budget = run(&["--budget", "300"]); + assert!( + by_budget.status.success(), + "{}", + String::from_utf8_lossy(&by_budget.stderr) + ); + let by_tokens = run(&["--tokens", "300"]); + assert_eq!(by_budget.stdout, by_tokens.stdout, "same flag, same output"); + assert!(String::from_utf8_lossy(&by_budget.stdout).contains("budget 300")); + // giving the budget twice under both names is a usage error, not a silent override + assert_eq!( + run(&["--budget", "300", "--tokens", "500"]).status.code(), + Some(2) + ); + // the alias is visible in --help + let help = plumb().args(["map", "--help"]).output().unwrap(); + let help = String::from_utf8_lossy(&help.stdout); + assert!( + help.contains("--tokens") && help.contains("--budget"), + "{help}" + ); +} + // ---------------------------------------------------------------- v1: verify, doctor, init #[test] From f55122978e956069d5e41e41ce1dd209f29df292 Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:34:00 +0300 Subject: [PATCH 5/9] fix(init): safe .gitignore handling, shown in --dry-run - --dry-run now says which lines it would add, not just 'would write .gitignore' - writes '.plumbgraph/*' + '!.plumbgraph/allow.toml' instead of '.plumbgraph/': excluding the directory made the documented, committed allow-list impossible to commit - recognises existing spellings (/.plumbgraph, **/.plumbgraph/*, BOM) instead of appending a duplicate - works for a project in a subdirectory of a repo (and worktrees, where .git is a file) - never rewrites a .gitignore it cannot read as UTF-8: it was read as empty and overwritten, losing e.g. a UTF-16 file from PowerShell 5 - keeps CRLF line endings - new --no-gitignore option --- README.md | 5 +- crates/cli/src/lib.rs | 7 +- crates/cli/tests/cli.rs | 32 ++++++ crates/core/src/init.rs | 112 +++++++++++++++--- crates/core/tests/init_gitignore.rs | 172 ++++++++++++++++++++++++++++ 5 files changed, 308 insertions(+), 20 deletions(-) create mode 100644 crates/core/tests/init_gitignore.rs diff --git a/README.md b/README.md index eace928..e22a80e 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,12 @@ plumb dead-code . # add --lib for libraries (exports = public AP plumb check-deps . --online # sends package names only to the registries plumb verify . # one pass/fail gate (exit 1 on new findings) plumb verify . --run --online # also run diagnostics, semgrep, ast-grep (trusted code only) -plumb init # write AGENTS.md block + MCP config for your agent +plumb init --dry-run # preview: AGENTS.md block, MCP config, .gitignore lines +plumb init # write them ``` +In a git work tree `plumb init` adds `.plumbgraph/*` and `!.plumbgraph/allow.toml` to `/.gitignore` (the index is a local cache; the allow-list is meant to be committed). An existing `.plumbgraph` entry is left alone, a `.gitignore` that is not UTF-8 is never rewritten, and `--no-gitignore` skips this step. + `--json` on any command gives machine-readable output; `--fail-on high|medium|low` sets the exit code. ### Use with an agent (MCP) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index c13a437..62b9799 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -278,8 +278,12 @@ enum Cmd { /// claude, cursor, codex or all (default: whatever is detected) #[arg(long = "agent")] agents: Vec, + /// Show what would be written (including the .gitignore lines) without writing #[arg(long)] dry_run: bool, + /// Do not add `.plumbgraph/*` and `!.plumbgraph/allow.toml` to .gitignore + #[arg(long)] + no_gitignore: bool, }, /// Serve the tools over MCP (stdio, JSON-RPC) Mcp { @@ -989,8 +993,9 @@ nothing was executed. `plumb enrich` runs SCIP indexers; `plumb verify --run` ru path, agents, dry_run, + no_gitignore, } => { - let r = plumbgraph_core::init::init(&path, &agents, dry_run)?; + let r = plumbgraph_core::init::init_with(&path, &agents, dry_run, !no_gitignore)?; if json { print_json(&r)?; } else { diff --git a/crates/cli/tests/cli.rs b/crates/cli/tests/cli.rs index 2140f74..eb88908 100644 --- a/crates/cli/tests/cli.rs +++ b/crates/cli/tests/cli.rs @@ -514,6 +514,38 @@ fn map_budget_is_an_alias_of_tokens() { ); } +#[test] +fn init_dry_run_shows_the_gitignore_lines() { + let t = project(); + git(t.path(), &["init", "-q"]); + let out = plumb() + .args(["init"]) + .arg(t.path()) + .arg("--dry-run") + .output() + .unwrap(); + assert!(out.status.success()); + let text = String::from_utf8_lossy(&out.stdout); + assert!( + text.contains("would add to") && text.contains(".plumbgraph/*"), + "{text}" + ); + assert!(text.contains("!.plumbgraph/allow.toml"), "{text}"); + assert!( + !t.path().join(".gitignore").exists(), + "dry run writes nothing" + ); + let out = plumb() + .args(["init"]) + .arg(t.path()) + .args(["--no-gitignore"]) + .output() + .unwrap(); + assert!(out.status.success()); + assert!(!t.path().join(".gitignore").exists()); + assert!(String::from_utf8_lossy(&out.stdout).contains("--no-gitignore")); +} + // ---------------------------------------------------------------- v1: verify, doctor, init #[test] diff --git a/crates/core/src/init.rs b/crates/core/src/init.rs index 680b350..81a95f6 100644 --- a/crates/core/src/init.rs +++ b/crates/core/src/init.rs @@ -163,7 +163,18 @@ pub fn codex_snippet(root: &str) -> String { } /// `agents`: which to configure; empty = everything detected (`AGENTS.md` is always written). +/// Also adds `GITIGNORE_LINES` to `.gitignore` inside a git work tree. pub fn init(root: &Path, agents: &[String], dry_run: bool) -> Result { + init_with(root, agents, dry_run, true) +} + +/// `init` with `gitignore = false` for `--no-gitignore` (the report says what to add by hand). +pub fn init_with( + root: &Path, + agents: &[String], + dry_run: bool, + gitignore: bool, +) -> Result { let root = root.canonicalize()?; let mut rep = InitReport::default(); let want = |name: &str, detected: bool| { @@ -188,26 +199,91 @@ pub fn init(root: &Path, agents: &[String], dry_run: bool) -> Result rep.snippets .push(codex_snippet(&root.display().to_string())); } - // keep the local index out of version control, but keep the baseline - let gi = root.join(".gitignore"); - let cur = std::fs::read_to_string(&gi).unwrap_or_default(); - if root.join(".git").exists() - && !cur - .lines() - .any(|l| l.trim() == ".plumbgraph/" || l.trim() == ".plumbgraph") - { - if !dry_run { - let mut n = cur.clone(); - if !n.is_empty() && !n.ends_with('\n') { - n.push('\n'); - } - n.push_str(".plumbgraph/\n"); - std::fs::write(&gi, n)?; - } - rep.written.push(shown(&gi)); - } + ensure_gitignore(&root, &mut rep, dry_run, gitignore)?; if rep.written.is_empty() && rep.notes.is_empty() { rep.notes.push("already configured".into()); } Ok(rep) } + +/// Lines `init` adds to `/.gitignore`. The index and SCIP files are local caches; the +/// allow-list `.plumbgraph/allow.toml` is meant to be committed. `/*` rather than `/`: +/// git cannot re-include a file whose parent directory is excluded. +pub const GITIGNORE_LINES: &[&str] = &[".plumbgraph/*", "!.plumbgraph/allow.toml"]; + +/// Does an existing `.gitignore` already exclude `.plumbgraph` in some spelling +/// (`.plumbgraph`, `/.plumbgraph/`, `.plumbgraph/*`, `**/.plumbgraph/**`, ...)? +fn gitignore_covers_plumbgraph(text: &str) -> bool { + text.lines().any(|l| { + let l = l.trim_start_matches('\u{feff}').trim(); + if l.starts_with('#') || l.starts_with('!') { + return false; + } + let l = l.trim_start_matches('/'); + let l = l.strip_prefix("**/").unwrap_or(l); + let l = l + .strip_suffix("/**") + .or_else(|| l.strip_suffix("/*")) + .unwrap_or(l); + l.trim_end_matches('/') == ".plumbgraph" + }) +} + +/// Add `GITIGNORE_LINES` to `/.gitignore` when `root` is inside a git work tree and the +/// file does not already cover `.plumbgraph`. Never rewrites a file it cannot read as UTF-8. +fn ensure_gitignore(root: &Path, rep: &mut InitReport, dry: bool, enabled: bool) -> Result<()> { + let wanted = GITIGNORE_LINES.join(" and "); + if !enabled { + rep.notes.push(format!( + "--no-gitignore: .gitignore not touched; to keep the local index out of git add {wanted}" + )); + return Ok(()); + } + // `.git` is a directory in a normal checkout and a file in worktrees and submodules + if !root.ancestors().any(|a| a.join(".git").exists()) { + rep.notes + .push("not inside a git work tree: .gitignore not touched".into()); + return Ok(()); + } + let gi = root.join(".gitignore"); + let cur = match std::fs::read(&gi) { + Ok(bytes) => match String::from_utf8(bytes) { + Ok(s) => s, + Err(_) => { + rep.notes.push(format!( + "{}: not UTF-8 (UTF-16?); left as is. Add {wanted} yourself", + shown(&gi) + )); + rep.unchanged.push(shown(&gi)); + return Ok(()); + } + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(e) => return Err(e).with_context(|| format!("reading {}", shown(&gi))), + }; + if gitignore_covers_plumbgraph(&cur) { + rep.unchanged.push(shown(&gi)); + return Ok(()); + } + rep.notes.push(format!( + "{} {}: {wanted}", + if dry { "would add to" } else { "added to" }, + shown(&gi) + )); + if !dry { + let nl = if cur.contains("\r\n") { "\r\n" } else { "\n" }; + let mut n = cur; + if !n.is_empty() && !n.ends_with('\n') { + n.push_str(nl); + } + n.push_str("# plumbgraph: local index and caches (the allow-list stays committed)"); + n.push_str(nl); + for l in GITIGNORE_LINES { + n.push_str(l); + n.push_str(nl); + } + std::fs::write(&gi, n)?; + } + rep.written.push(shown(&gi)); + Ok(()) +} diff --git a/crates/core/tests/init_gitignore.rs b/crates/core/tests/init_gitignore.rs new file mode 100644 index 0000000..d057208 --- /dev/null +++ b/crates/core/tests/init_gitignore.rs @@ -0,0 +1,172 @@ +//! `plumb init` keeps the local index (`.plumbgraph/index.db`, SCIP files) out of git while +//! the allow-list `.plumbgraph/allow.toml` stays committable, and says so in `--dry-run`. +use plumbgraph_core::init::init; +use std::path::Path; +use std::process::Command; + +fn git(dir: &Path, args: &[&str]) -> std::process::Output { + Command::new("git") + .arg("-C") + .arg(dir) + .args(args) + .output() + .unwrap() +} + +fn repo() -> tempfile::TempDir { + let t = tempfile::tempdir().unwrap(); + assert!(git(t.path(), &["init", "-q"]).status.success()); + t +} + +/// Is `rel` ignored by git in `dir`? (`git check-ignore` exits 0 when it is.) +fn ignored(dir: &Path, rel: &str) -> bool { + git(dir, &["check-ignore", "-q", "--no-index", rel]) + .status + .success() +} + +fn gitignore(dir: &Path) -> String { + std::fs::read_to_string(dir.join(".gitignore")).unwrap_or_default() +} + +#[test] +fn index_is_ignored_but_the_allow_list_stays_committable() { + let t = repo(); + init(t.path(), &[], false).unwrap(); + assert!( + ignored(t.path(), ".plumbgraph/index.db"), + "{}", + gitignore(t.path()) + ); + assert!(ignored(t.path(), ".plumbgraph/scip/scip-python.scip")); + assert!( + !ignored(t.path(), ".plumbgraph/allow.toml"), + "allow.toml is documented as committed: {}", + gitignore(t.path()) + ); +} + +#[test] +fn dry_run_reports_the_gitignore_lines_and_writes_nothing() { + let t = repo(); + let rep = init(t.path(), &[], true).unwrap(); + assert!(!t.path().join(".gitignore").exists()); + let said = format!("{:?}", rep.notes); + assert!(said.contains(".plumbgraph/*"), "{said}"); + assert!(said.contains("!.plumbgraph/allow.toml"), "{said}"); +} + +#[test] +fn equivalent_existing_patterns_are_respected() { + for existing in [ + ".plumbgraph/\n", + "/.plumbgraph\n", + "**/.plumbgraph/*\n!**/.plumbgraph/allow.toml\n", + ".plumbgraph/*\r\n", + ] { + let t = repo(); + std::fs::write(t.path().join(".gitignore"), existing).unwrap(); + let rep = init(t.path(), &[], false).unwrap(); + assert_eq!(gitignore(t.path()), existing, "left untouched"); + assert!( + !rep.written.iter().any(|w| w.ends_with(".gitignore")), + "{rep:?}" + ); + } +} + +#[test] +fn a_project_in_a_subdirectory_of_a_repo_gets_its_own_entry() { + let t = repo(); + let sub = t.path().join("services/api"); + std::fs::create_dir_all(&sub).unwrap(); + init(&sub, &[], false).unwrap(); + assert!(ignored(t.path(), "services/api/.plumbgraph/index.db")); + assert!(!ignored(t.path(), "services/api/.plumbgraph/allow.toml")); + assert!( + !t.path().join(".gitignore").exists(), + "the repo root is not touched" + ); +} + +#[test] +fn crlf_gitignore_keeps_its_line_endings() { + let t = repo(); + std::fs::write(t.path().join(".gitignore"), "target/\r\n").unwrap(); + init(t.path(), &[], false).unwrap(); + let g = gitignore(t.path()); + assert!(g.starts_with("target/\r\n"), "{g:?}"); + assert!( + !g.replace("\r\n", "").contains('\n'), + "no bare LF mixed in: {g:?}" + ); + assert!(ignored(t.path(), ".plumbgraph/index.db")); +} + +#[test] +fn a_gitignore_that_is_not_utf8_is_never_overwritten() { + // what Windows PowerShell 5 writes for `echo target/ > .gitignore`: UTF-16LE with a BOM + let t = repo(); + let mut utf16: Vec = vec![0xFF, 0xFE]; + for u in "target/\r\n".encode_utf16() { + utf16.extend_from_slice(&u.to_le_bytes()); + } + std::fs::write(t.path().join(".gitignore"), &utf16).unwrap(); + let rep = init(t.path(), &[], false).unwrap(); + assert_eq!( + std::fs::read(t.path().join(".gitignore")).unwrap(), + utf16, + "user content must survive" + ); + assert!( + rep.notes.iter().any(|n| n.contains("not UTF-8")), + "{:?}", + rep.notes + ); +} + +#[test] +fn bom_prefixed_existing_entry_is_recognised() { + let t = repo(); + let existing = "\u{feff}.plumbgraph/\n"; + std::fs::write(t.path().join(".gitignore"), existing).unwrap(); + init(t.path(), &[], false).unwrap(); + assert_eq!(gitignore(t.path()), existing); +} + +#[test] +fn no_gitignore_option_leaves_it_alone_and_says_what_to_add() { + let t = repo(); + let rep = plumbgraph_core::init::init_with(t.path(), &[], false, false).unwrap(); + assert!(!t.path().join(".gitignore").exists()); + let said = format!("{:?}", rep.notes); + assert!( + said.contains("--no-gitignore") && said.contains(".plumbgraph/*"), + "{said}" + ); + // the agent files are still written + assert!(t.path().join("AGENTS.md").is_file()); +} + +#[test] +fn second_run_is_a_no_op() { + let t = repo(); + init(t.path(), &[], false).unwrap(); + let first = gitignore(t.path()); + let again = init(t.path(), &[], false).unwrap(); + assert_eq!(gitignore(t.path()), first); + assert!(again.written.is_empty(), "{again:?}"); +} + +#[test] +fn outside_git_nothing_is_written_and_the_report_says_why() { + let t = tempfile::tempdir().unwrap(); + let rep = init(t.path(), &[], false).unwrap(); + assert!(!t.path().join(".gitignore").exists()); + assert!( + rep.notes.iter().any(|n| n.contains("not inside a git")), + "{:?}", + rep.notes + ); +} From 9611a0a13a90043de3d28d294357593106b57d81 Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:38:39 +0300 Subject: [PATCH 6/9] docs: Windows install and PATH setup, with exactly what was tested Documents cargo install --path crates/cli --locked on Windows, adding %USERPROFILE%\.cargo\bin to the user PATH (rustup does it; Chocolatey's Rust does not), and the one machine/toolchain this was tried on (Windows 11, Rust 1.91.1 x86_64-pc-windows-gnu, no MSVC, no 1.85, no CI), including the 10 tests that fail there on main as well. --- README.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e22a80e..96f3c53 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,23 @@ In a git work tree `plumb init` adds `.plumbgraph/*` and `!.plumbgraph/allow.tom `--json` on any command gives machine-readable output; `--fail-on high|medium|low` sets the exit code. +### Windows + +The same `cargo install --path crates/cli --locked` works from PowerShell; it puts `plumb.exe` and `plumbgraph.exe` in `%USERPROFILE%\.cargo\bin`. The rustup installer adds that directory to `PATH`; other Rust installs (Chocolatey, for example) do not, and `cargo install` then prints `be sure to add ...\.cargo\bin to your PATH`. Add it for your user and open a new terminal: + +```powershell +$bin = "$env:USERPROFILE\.cargo\bin" +$user = [Environment]::GetEnvironmentVariable('Path', 'User') +if (($user -split ';') -notcontains $bin) { + [Environment]::SetEnvironmentVariable('Path', ($user.TrimEnd(';') + ';' + $bin), 'User') +} +# new terminal: +plumb --version +plumb doctor . +``` + +What was tested on Windows, and only this: Windows 11 Home 64-bit (build 26200) with Rust 1.91.1 `x86_64-pc-windows-gnu` from Chocolatey (no rustup, so `rust-toolchain.toml`'s 1.85 was not used, and the MSVC target was not tried). `cargo install --path crates/cli --locked` built and installed; `plumb index`, `map`, `dead-code`, `check-deps`, `verify`, `weakening`, `doctor`, `init --dry-run` and `plumb mcp` (driven by one MCP host) ran on a real repository. `cargo test --workspace` on that machine: every test passes except 10 that also fail on `main` there (LSP/diagnostics tests that need a real `python3`, which was only the Microsoft Store stub, and the closed-stdout-pipe test, which gets Windows error 232 instead of a broken pipe). CI runs on Linux only. + ### Use with an agent (MCP) ```bash @@ -129,7 +146,7 @@ Use them together: if you need precise navigation and editing today, an LSP-back - Go, Java, C# are tier-0 only; C# was not benchmarked on a real repository. - SCIP: rust-analyzer's SCIP output, scip-go, scip-java and scip-dotnet were not run; scip-python and scip-typescript only on toys. - MCP: stdio only, tools and resources only (no prompts). Tested with the official TypeScript SDK and the MCP inspector CLI; agent hosts were not tested. -- `weakening` is line-based and heuristic. No Windows testing. No releases or packages yet. +- `weakening` is line-based and heuristic. Windows: one machine, manual runs, no CI (see [Windows](#windows)). No releases or packages yet. ## Documentation From eaad227b6be09cc55de054474902b656bc216dca Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:43:18 +0300 Subject: [PATCH 7/9] fix(csharp,init): tighten Unity and .gitignore rules after review Found while checking the change on a real Unity project (Unity's open-project-1) and re-reading the diff: - Assets/ indirect rule: a class whose first base is an interface (IDisposable, IComparable) cannot be a MonoBehaviour; its Update is no longer treated as an engine message - AddItemsToMenu (IHasCustomMenu, called by the editor on EditorWindows) added to the message list - UnityEvent fp_risk now also names Animation Events, which is what many still-reported public handlers on the real project are - init: any existing .gitignore rule mentioning .plumbgraph (including a negation such as !.plumbgraph/) means the user manages it; the appended lines would otherwise win as the last match --- crates/core/src/extract.rs | 16 +++++++++---- crates/core/src/init.rs | 23 +++++++------------ .../unity/Assets/Editor/SceneBrowser.cs | 15 ++++++++++++ .../fixtures/unity/Assets/Scripts/Pooled.cs | 13 +++++++++++ .../tests/fixtures/unity/Server/TextEditor.cs | 11 +++++++++ crates/core/tests/golden/unity_default.txt | 9 ++++++++ crates/core/tests/golden/unity_low.txt | 9 ++++++++ crates/core/tests/init_gitignore.rs | 4 ++++ crates/core/tests/unity_entry_points.rs | 9 ++++++++ docs/PACKS.md | 4 ++-- 10 files changed, 92 insertions(+), 21 deletions(-) create mode 100644 crates/core/tests/fixtures/unity/Assets/Editor/SceneBrowser.cs create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/Pooled.cs create mode 100644 crates/core/tests/fixtures/unity/Server/TextEditor.cs diff --git a/crates/core/src/extract.rs b/crates/core/src/extract.rs index fb2c9e2..e7c9052 100644 --- a/crates/core/src/extract.rs +++ b/crates/core/src/extract.rs @@ -276,7 +276,8 @@ const UNITY_EDITOR_BASES: &[&str] = &["Editor", "EditorWindow"]; /// Methods the Unity engine calls by name (reflection on the message name, not a virtual call). /// MonoBehaviour / ScriptableObject messages, physics (3D and 2D), rendering, input, animation, -/// StateMachineBehaviour, Editor/EditorWindow and ISerializationCallbackReceiver callbacks. +/// StateMachineBehaviour, Editor/EditorWindow (incl. IHasCustomMenu) and +/// ISerializationCallbackReceiver callbacks. const UNITY_MESSAGES: &[&str] = &[ "Awake", "Start", @@ -352,6 +353,7 @@ const UNITY_MESSAGES: &[&str] = &[ "OnHierarchyChange", "OnProjectChange", "OnInspectorUpdate", + "AddItemsToMenu", "OnBeforeSerialize", "OnAfterDeserialize", ]; @@ -456,9 +458,15 @@ fn unity_class( return Some(format!("derives from {b}")); } } - if !bases.is_empty() && under_assets(rel_path) { + // .NET naming: `IDisposable`, `IComparable`. A class lists its base class first and + // cannot reach MonoBehaviour through an interface. + let interface_like = |b: &str| { + let mut c = b.chars(); + c.next() == Some('I') && c.next().is_some_and(|x| x.is_ascii_uppercase()) + }; + if under_assets(rel_path) && bases.first().is_some_and(|b| !interface_like(b)) { // indirect subclass (`Enemy : Character : MonoBehaviour`): the chain is not resolved, - // so any derived class in a Unity project's Assets/ is treated as possibly a component + // so a class with a base class in a Unity project's Assets/ may be a component return Some(format!( "derives from {} under Assets/ (possibly an indirect MonoBehaviour)", bases[0] @@ -924,7 +932,7 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { } } else if entry.is_none() && has("public") { framework_risk = Some(format!( - "public method of a Unity class ({why}): may be bound to a UnityEvent (e.g. Button.onClick) or called via SendMessage in a scene/prefab, which is not indexed" + "public method of a Unity class ({why}): may be bound to a UnityEvent (e.g. Button.onClick), an Animation Event or SendMessage in a scene, prefab or animation clip, which are not indexed" )); } } diff --git a/crates/core/src/init.rs b/crates/core/src/init.rs index 81a95f6..7e856a7 100644 --- a/crates/core/src/init.rs +++ b/crates/core/src/init.rs @@ -211,26 +211,19 @@ pub fn init_with( /// git cannot re-include a file whose parent directory is excluded. pub const GITIGNORE_LINES: &[&str] = &[".plumbgraph/*", "!.plumbgraph/allow.toml"]; -/// Does an existing `.gitignore` already exclude `.plumbgraph` in some spelling -/// (`.plumbgraph`, `/.plumbgraph/`, `.plumbgraph/*`, `**/.plumbgraph/**`, ...)? -fn gitignore_covers_plumbgraph(text: &str) -> bool { +/// Does the user already manage `.plumbgraph` in this `.gitignore`? Any rule (not a comment) +/// that mentions it counts: an exclusion in any spelling (`/.plumbgraph/`, `**/.plumbgraph/*`, +/// `.plumbgraph*`) or a negation (`!.plumbgraph/`), which our appended lines would override, +/// because the last matching rule wins. +fn gitignore_mentions_plumbgraph(text: &str) -> bool { text.lines().any(|l| { let l = l.trim_start_matches('\u{feff}').trim(); - if l.starts_with('#') || l.starts_with('!') { - return false; - } - let l = l.trim_start_matches('/'); - let l = l.strip_prefix("**/").unwrap_or(l); - let l = l - .strip_suffix("/**") - .or_else(|| l.strip_suffix("/*")) - .unwrap_or(l); - l.trim_end_matches('/') == ".plumbgraph" + !l.starts_with('#') && l.contains(".plumbgraph") }) } /// Add `GITIGNORE_LINES` to `/.gitignore` when `root` is inside a git work tree and the -/// file does not already cover `.plumbgraph`. Never rewrites a file it cannot read as UTF-8. +/// file has no rule about `.plumbgraph` yet. Never rewrites a file it cannot read as UTF-8. fn ensure_gitignore(root: &Path, rep: &mut InitReport, dry: bool, enabled: bool) -> Result<()> { let wanted = GITIGNORE_LINES.join(" and "); if !enabled { @@ -261,7 +254,7 @@ fn ensure_gitignore(root: &Path, rep: &mut InitReport, dry: bool, enabled: bool) Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), Err(e) => return Err(e).with_context(|| format!("reading {}", shown(&gi))), }; - if gitignore_covers_plumbgraph(&cur) { + if gitignore_mentions_plumbgraph(&cur) { rep.unchanged.push(shown(&gi)); return Ok(()); } diff --git a/crates/core/tests/fixtures/unity/Assets/Editor/SceneBrowser.cs b/crates/core/tests/fixtures/unity/Assets/Editor/SceneBrowser.cs new file mode 100644 index 0000000..846fb6d --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Editor/SceneBrowser.cs @@ -0,0 +1,15 @@ +using UnityEditor; +using UnityEngine; + +namespace Game.EditorTools +{ + // EditorWindow + IHasCustomMenu: the editor calls OnGUI and AddItemsToMenu. + internal class SceneBrowser : EditorWindow, IHasCustomMenu + { + void OnGUI() { } + + public void AddItemsToMenu(GenericMenu menu) { } + + private void UnusedBrowserHelper() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/Pooled.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/Pooled.cs new file mode 100644 index 0000000..0afc5de --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/Pooled.cs @@ -0,0 +1,13 @@ +using System; + +namespace Game +{ + // Under Assets/ but only implements interfaces: a C# class cannot reach MonoBehaviour + // through an interface, so Unity never calls Update here. + internal class Pooled : IDisposable, IComparable + { + public void Dispose() { } + public int CompareTo(Pooled other) { return 0; } + void Update() { } + } +} diff --git a/crates/core/tests/fixtures/unity/Server/TextEditor.cs b/crates/core/tests/fixtures/unity/Server/TextEditor.cs new file mode 100644 index 0000000..aa66007 --- /dev/null +++ b/crates/core/tests/fixtures/unity/Server/TextEditor.cs @@ -0,0 +1,11 @@ +namespace Server +{ + internal class Editor { } + + // `Editor` is a generic name: outside a Unity file it is not UnityEditor.Editor. + internal class TextEditor : Editor + { + void OnGUI() { } + void OnInspectorGUI() { } + } +} diff --git a/crates/core/tests/golden/unity_default.txt b/crates/core/tests/golden/unity_default.txt index 9f32a9b..06177e0 100644 --- a/crates/core/tests/golden/unity_default.txt +++ b/crates/core/tests/golden/unity_default.txt @@ -1,4 +1,6 @@ high 0.95 dead-code class MathUtil Assets/Scripts/MathUtil.cs:4 +high 0.95 dead-code class Pooled Assets/Scripts/Pooled.cs:7 +high 0.95 dead-code class TextEditor Server/TextEditor.cs:6 high 0.95 dead-code class Worker Server/Worker.cs:7 high 0.95 dead-code method BuildTools.OrphanTool Assets/Editor/BuildTools.cs:20 high 0.95 dead-code method MathUtil.Start Assets/Scripts/MathUtil.cs:7 @@ -8,6 +10,13 @@ medium 0.71 dead-code method Enemy.EnemyDeadHelper Assets/Scripts/En medium 0.71 dead-code method GameSettings.StaleSettingsHelper Assets/Scripts/GameSettings.cs:11 medium 0.71 dead-code method NetPlayer.NetDeadHelper Assets/Scripts/Net/NetPlayer.cs:10 medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Player.cs:46 +medium 0.71 dead-code method Pooled.CompareTo Assets/Scripts/Pooled.cs:10 +medium 0.71 dead-code method Pooled.Dispose Assets/Scripts/Pooled.cs:9 +medium 0.71 dead-code method Pooled.Update Assets/Scripts/Pooled.cs:11 +medium 0.71 dead-code method SceneBrowser.UnusedBrowserHelper Assets/Editor/SceneBrowser.cs:13 +medium 0.71 dead-code method TextEditor.OnGUI Server/TextEditor.cs:8 +medium 0.71 dead-code method TextEditor.OnInspectorGUI Server/TextEditor.cs:9 medium 0.71 dead-code method Worker.Dispose Server/Worker.cs:11 medium 0.71 dead-code method Worker.Start Server/Worker.cs:12 medium 0.71 dead-code method Worker.Update Server/Worker.cs:13 +medium 0.80 dead-code class Editor Server/TextEditor.cs:3 diff --git a/crates/core/tests/golden/unity_low.txt b/crates/core/tests/golden/unity_low.txt index 980c477..098c756 100644 --- a/crates/core/tests/golden/unity_low.txt +++ b/crates/core/tests/golden/unity_low.txt @@ -1,4 +1,6 @@ high 0.95 dead-code class MathUtil Assets/Scripts/MathUtil.cs:4 +high 0.95 dead-code class Pooled Assets/Scripts/Pooled.cs:7 +high 0.95 dead-code class TextEditor Server/TextEditor.cs:6 high 0.95 dead-code class Worker Server/Worker.cs:7 high 0.95 dead-code method BuildTools.OrphanTool Assets/Editor/BuildTools.cs:20 high 0.95 dead-code method MathUtil.Start Assets/Scripts/MathUtil.cs:7 @@ -10,6 +12,13 @@ medium 0.71 dead-code method Enemy.EnemyDeadHelper Assets/Scripts/En medium 0.71 dead-code method GameSettings.StaleSettingsHelper Assets/Scripts/GameSettings.cs:11 medium 0.71 dead-code method NetPlayer.NetDeadHelper Assets/Scripts/Net/NetPlayer.cs:10 medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Player.cs:46 +medium 0.71 dead-code method Pooled.CompareTo Assets/Scripts/Pooled.cs:10 +medium 0.71 dead-code method Pooled.Dispose Assets/Scripts/Pooled.cs:9 +medium 0.71 dead-code method Pooled.Update Assets/Scripts/Pooled.cs:11 +medium 0.71 dead-code method SceneBrowser.UnusedBrowserHelper Assets/Editor/SceneBrowser.cs:13 +medium 0.71 dead-code method TextEditor.OnGUI Server/TextEditor.cs:8 +medium 0.71 dead-code method TextEditor.OnInspectorGUI Server/TextEditor.cs:9 medium 0.71 dead-code method Worker.Dispose Server/Worker.cs:11 medium 0.71 dead-code method Worker.Start Server/Worker.cs:12 medium 0.71 dead-code method Worker.Update Server/Worker.cs:13 +medium 0.80 dead-code class Editor Server/TextEditor.cs:3 diff --git a/crates/core/tests/init_gitignore.rs b/crates/core/tests/init_gitignore.rs index d057208..7fd599e 100644 --- a/crates/core/tests/init_gitignore.rs +++ b/crates/core/tests/init_gitignore.rs @@ -64,6 +64,10 @@ fn equivalent_existing_patterns_are_respected() { "/.plumbgraph\n", "**/.plumbgraph/*\n!**/.plumbgraph/allow.toml\n", ".plumbgraph/*\r\n", + // the user deliberately commits the index: appending `.plumbgraph/*` would win + // (last match) and silently undo that + "!.plumbgraph/\n", + ".plumbgraph*\n", ] { let t = repo(); std::fs::write(t.path().join(".gitignore"), existing).unwrap(); diff --git a/crates/core/tests/unity_entry_points.rs b/crates/core/tests/unity_entry_points.rs index 327eed6..17928af 100644 --- a/crates/core/tests/unity_entry_points.rs +++ b/crates/core/tests/unity_entry_points.rs @@ -85,6 +85,9 @@ fn scriptableobject_networkbehaviour_and_indirect_subclasses_under_assets() { "Enemy.Update", "Enemy.OnTriggerEnter", "Enemy.Chase", + // EditorWindow in a Unity file, incl. the IHasCustomMenu callback + "SceneBrowser.OnGUI", + "SceneBrowser.AddItemsToMenu", ] { assert!(!s.contains(&q.to_string()), "false positive {q}: {s:?}"); } @@ -122,6 +125,12 @@ fn real_dead_code_in_unity_projects_is_still_reported() { "Worker.Update", // `[MenuItem]` outside a Unity project is just an attribute "Worker.Menu", + "SceneBrowser.UnusedBrowserHelper", + // under Assets/, but only interfaces as bases: cannot be a MonoBehaviour + "Pooled.Update", + // a class merely named `Editor` outside a Unity file + "TextEditor.OnGUI", + "TextEditor.OnInspectorGUI", ] { assert!(s.contains(&q.to_string()), "missing {q}: {s:?}"); } diff --git a/docs/PACKS.md b/docs/PACKS.md index 67c5c54..53a54b1 100644 --- a/docs/PACKS.md +++ b/docs/PACKS.md @@ -26,9 +26,9 @@ Tier-0 only (tree-sitter, name-based). What each pack knows: The engine calls MonoBehaviour messages by name, so tier-0 sees no caller. Rules (golden fixture: `crates/core/tests/fixtures/unity`, tests: `crates/core/tests/unity_entry_points.rs`): -- **Message methods are entry points** (`Awake`, `Start`, `Update`, `LateUpdate`, `FixedUpdate`, `OnEnable`, `OnDisable`, `OnDestroy`, `OnValidate`, `Reset`, `OnGUI`, `OnCollision*`/`OnTrigger*` and their `2D` variants, plus the rest of the list in `UNITY_MESSAGES` in `crates/core/src/extract.rs`) when they are non-static methods of a class that derives directly from `MonoBehaviour`, `ScriptableObject`, `NetworkBehaviour`, `StateMachineBehaviour` or `UIBehaviour` (`Editor`/`EditorWindow` only in a Unity file, see below), **or** that derives from anything and lives under `Assets/` (the inheritance chain is not resolved, so `Enemy : Character : MonoBehaviour` is recognised this way). The class is then live too. +- **Message methods are entry points** (`Awake`, `Start`, `Update`, `LateUpdate`, `FixedUpdate`, `OnEnable`, `OnDisable`, `OnDestroy`, `OnValidate`, `Reset`, `OnGUI`, `OnCollision*`/`OnTrigger*` and their `2D` variants, plus the rest of the list in `UNITY_MESSAGES` in `crates/core/src/extract.rs`) when they are non-static methods of a class that derives directly from `MonoBehaviour`, `ScriptableObject`, `NetworkBehaviour`, `StateMachineBehaviour` or `UIBehaviour` (`Editor`/`EditorWindow` only in a Unity file, see below), **or** that lives under `Assets/` and whose first base type is a class rather than an interface (`I` + capital letter, the .NET convention). The inheritance chain is not resolved, so `Enemy : Character : MonoBehaviour` is recognised this way, while `Pool : IDisposable` is not. The class is then live too. - **Attributes are entry points** in a Unity file: `[RuntimeInitializeOnLoadMethod]`, `[InitializeOnLoadMethod]`, `[InitializeOnLoad]` (class), `[MenuItem]`, `[ContextMenu]`, `[DidReloadScripts]`, `[PostProcessBuild]`, `[PostProcessScene]`, `[OnOpenAsset]` (with or without the `Attribute` suffix). A Unity file is one under `Assets/` or `Packages/`, or whose code (not comments or strings) has a `using UnityEngine…`/`using UnityEditor…` directive or a `UnityEngine.`/`UnityEditor.` qualified name. -- **Public methods of such classes** stay findings but get a 0.25 penalty and the fp_risk "may be bound to a UnityEvent (e.g. Button.onClick) or called via SendMessage in a scene/prefab, which is not indexed". Scenes, prefabs and `.asset` files are not parsed, so plumb cannot tell a bound handler from dead code. +- **Public methods of such classes** stay findings but get a 0.25 penalty and the fp_risk "may be bound to a UnityEvent (e.g. Button.onClick), an Animation Event or SendMessage in a scene, prefab or animation clip, which are not indexed". Scenes, prefabs, animation clips and `.asset` files are not parsed, so plumb cannot tell a bound handler from dead code. - `[SerializeField]` fields: C# fields are not extracted, so they are never reported (tested, so this is noticed if field extraction is added). - Not covered: messages invoked through `SendMessage("Name")` / `Invoke("Name")` strings are only a string-mention penalty; `Editor`/`EditorWindow` subclasses outside a Unity file; no Unity project or editor was run to produce these rules (they come from the Unity scripting reference). From 562b219328e366dde813625875995867620767a3 Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:46:08 +0300 Subject: [PATCH 8/9] fix(csharp): Unity callback interfaces are entry points ISerializationCallbackReceiver (OnBeforeSerialize/OnAfterDeserialize), IHasCustomMenu (AddItemsToMenu) and the UI EventSystem handler interfaces (IPointerClickHandler.OnPointerClick, IDragHandler.OnDrag, ...) are called by Unity on any implementing type, not only on components. Found on open-project-1: SceneSelector.Storage, a plain [Serializable] class, reported its serialization callbacks as dead. --- crates/core/src/extract.rs | 44 +++++++++++++++++++ .../fixtures/unity/Assets/Scripts/SaveData.cs | 22 ++++++++++ crates/core/tests/golden/unity_default.txt | 1 + crates/core/tests/golden/unity_low.txt | 1 + crates/core/tests/unity_entry_points.rs | 7 +++ docs/PACKS.md | 1 + 6 files changed, 76 insertions(+) create mode 100644 crates/core/tests/fixtures/unity/Assets/Scripts/SaveData.cs diff --git a/crates/core/src/extract.rs b/crates/core/src/extract.rs index e7c9052..2cf410a 100644 --- a/crates/core/src/extract.rs +++ b/crates/core/src/extract.rs @@ -358,6 +358,35 @@ const UNITY_MESSAGES: &[&str] = &[ "OnAfterDeserialize", ]; +/// Unity interfaces whose methods the engine calls on any implementing type (not only +/// components): serialization callbacks, the editor's custom menu, and UI EventSystem handlers. +const UNITY_CALLBACK_INTERFACES: &[(&str, &str)] = &[ + ("ISerializationCallbackReceiver", "OnBeforeSerialize"), + ("ISerializationCallbackReceiver", "OnAfterDeserialize"), + ("IHasCustomMenu", "AddItemsToMenu"), + ("IPointerEnterHandler", "OnPointerEnter"), + ("IPointerExitHandler", "OnPointerExit"), + ("IPointerDownHandler", "OnPointerDown"), + ("IPointerUpHandler", "OnPointerUp"), + ("IPointerClickHandler", "OnPointerClick"), + ("IPointerMoveHandler", "OnPointerMove"), + ( + "IInitializePotentialDragHandler", + "OnInitializePotentialDrag", + ), + ("IBeginDragHandler", "OnBeginDrag"), + ("IDragHandler", "OnDrag"), + ("IEndDragHandler", "OnEndDrag"), + ("IDropHandler", "OnDrop"), + ("IScrollHandler", "OnScroll"), + ("IUpdateSelectedHandler", "OnUpdateSelected"), + ("ISelectHandler", "OnSelect"), + ("IDeselectHandler", "OnDeselect"), + ("IMoveHandler", "OnMove"), + ("ISubmitHandler", "OnSubmit"), + ("ICancelHandler", "OnCancel"), +]; + /// Unity attributes that make the engine/editor call the method (or, for `InitializeOnLoad`, /// run the class's static constructor). Matched without a trailing `Attribute`. const UNITY_ENTRY_ATTRIBUTES: &[&str] = &[ @@ -919,6 +948,21 @@ pub fn extract(pack: &Pack, rel_path: &str, source: &str) -> Result { Some(format!("Unity `[{a}]`: invoked by the engine/editor")); } } + if entry.is_none() && cs_unity_file && kind == "method" { + if let Some(p) = parent + .filter(|&p| matches!(syms[p].kind.as_str(), "class" | "struct")) + { + let bases = cs_base_names(defs[p].def, src); + if let Some((iface, _)) = UNITY_CALLBACK_INTERFACES + .iter() + .find(|(i, m)| *m == name && bases.iter().any(|b| b == i)) + { + entry = Some(format!( + "Unity callback `{name}` of `{iface}`: called by the engine" + )); + } + } + } let unity_owner = parent .filter(|&p| syms[p].kind == "class") .and_then(|p| unity_class(defs[p].def, src, rel_path, cs_unity_file)); diff --git a/crates/core/tests/fixtures/unity/Assets/Scripts/SaveData.cs b/crates/core/tests/fixtures/unity/Assets/Scripts/SaveData.cs new file mode 100644 index 0000000..f7964ab --- /dev/null +++ b/crates/core/tests/fixtures/unity/Assets/Scripts/SaveData.cs @@ -0,0 +1,22 @@ +using System; +using UnityEngine; +using UnityEngine.EventSystems; + +namespace Game +{ + // Not a MonoBehaviour: Unity's serializer calls these because of the interface. + [Serializable] + internal class SaveData : ISerializationCallbackReceiver + { + public void OnBeforeSerialize() { } + public void OnAfterDeserialize() { } + public void UnusedSaveHelper() { } + } + + // The EventSystem calls interface methods on components. + internal class ClickTarget : MonoBehaviour, IPointerClickHandler, IDragHandler + { + public void OnPointerClick(PointerEventData eventData) { } + public void OnDrag(PointerEventData eventData) { } + } +} diff --git a/crates/core/tests/golden/unity_default.txt b/crates/core/tests/golden/unity_default.txt index 06177e0..eeba3b8 100644 --- a/crates/core/tests/golden/unity_default.txt +++ b/crates/core/tests/golden/unity_default.txt @@ -13,6 +13,7 @@ medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Playe medium 0.71 dead-code method Pooled.CompareTo Assets/Scripts/Pooled.cs:10 medium 0.71 dead-code method Pooled.Dispose Assets/Scripts/Pooled.cs:9 medium 0.71 dead-code method Pooled.Update Assets/Scripts/Pooled.cs:11 +medium 0.71 dead-code method SaveData.UnusedSaveHelper Assets/Scripts/SaveData.cs:13 medium 0.71 dead-code method SceneBrowser.UnusedBrowserHelper Assets/Editor/SceneBrowser.cs:13 medium 0.71 dead-code method TextEditor.OnGUI Server/TextEditor.cs:8 medium 0.71 dead-code method TextEditor.OnInspectorGUI Server/TextEditor.cs:9 diff --git a/crates/core/tests/golden/unity_low.txt b/crates/core/tests/golden/unity_low.txt index 098c756..98a73dc 100644 --- a/crates/core/tests/golden/unity_low.txt +++ b/crates/core/tests/golden/unity_low.txt @@ -15,6 +15,7 @@ medium 0.71 dead-code method Player.NeverCalled Assets/Scripts/Playe medium 0.71 dead-code method Pooled.CompareTo Assets/Scripts/Pooled.cs:10 medium 0.71 dead-code method Pooled.Dispose Assets/Scripts/Pooled.cs:9 medium 0.71 dead-code method Pooled.Update Assets/Scripts/Pooled.cs:11 +medium 0.71 dead-code method SaveData.UnusedSaveHelper Assets/Scripts/SaveData.cs:13 medium 0.71 dead-code method SceneBrowser.UnusedBrowserHelper Assets/Editor/SceneBrowser.cs:13 medium 0.71 dead-code method TextEditor.OnGUI Server/TextEditor.cs:8 medium 0.71 dead-code method TextEditor.OnInspectorGUI Server/TextEditor.cs:9 diff --git a/crates/core/tests/unity_entry_points.rs b/crates/core/tests/unity_entry_points.rs index 17928af..cad814d 100644 --- a/crates/core/tests/unity_entry_points.rs +++ b/crates/core/tests/unity_entry_points.rs @@ -88,6 +88,11 @@ fn scriptableobject_networkbehaviour_and_indirect_subclasses_under_assets() { // EditorWindow in a Unity file, incl. the IHasCustomMenu callback "SceneBrowser.OnGUI", "SceneBrowser.AddItemsToMenu", + // Unity callback interfaces, also on classes that are not components + "SaveData.OnBeforeSerialize", + "SaveData.OnAfterDeserialize", + "ClickTarget.OnPointerClick", + "ClickTarget.OnDrag", ] { assert!(!s.contains(&q.to_string()), "false positive {q}: {s:?}"); } @@ -128,6 +133,8 @@ fn real_dead_code_in_unity_projects_is_still_reported() { "SceneBrowser.UnusedBrowserHelper", // under Assets/, but only interfaces as bases: cannot be a MonoBehaviour "Pooled.Update", + // implementing a callback interface does not make other methods entry points + "SaveData.UnusedSaveHelper", // a class merely named `Editor` outside a Unity file "TextEditor.OnGUI", "TextEditor.OnInspectorGUI", diff --git a/docs/PACKS.md b/docs/PACKS.md index 53a54b1..df78846 100644 --- a/docs/PACKS.md +++ b/docs/PACKS.md @@ -27,6 +27,7 @@ Tier-0 only (tree-sitter, name-based). What each pack knows: The engine calls MonoBehaviour messages by name, so tier-0 sees no caller. Rules (golden fixture: `crates/core/tests/fixtures/unity`, tests: `crates/core/tests/unity_entry_points.rs`): - **Message methods are entry points** (`Awake`, `Start`, `Update`, `LateUpdate`, `FixedUpdate`, `OnEnable`, `OnDisable`, `OnDestroy`, `OnValidate`, `Reset`, `OnGUI`, `OnCollision*`/`OnTrigger*` and their `2D` variants, plus the rest of the list in `UNITY_MESSAGES` in `crates/core/src/extract.rs`) when they are non-static methods of a class that derives directly from `MonoBehaviour`, `ScriptableObject`, `NetworkBehaviour`, `StateMachineBehaviour` or `UIBehaviour` (`Editor`/`EditorWindow` only in a Unity file, see below), **or** that lives under `Assets/` and whose first base type is a class rather than an interface (`I` + capital letter, the .NET convention). The inheritance chain is not resolved, so `Enemy : Character : MonoBehaviour` is recognised this way, while `Pool : IDisposable` is not. The class is then live too. +- **Callback interfaces** in a Unity file: methods of `ISerializationCallbackReceiver` (`OnBeforeSerialize`, `OnAfterDeserialize`), `IHasCustomMenu` (`AddItemsToMenu`) and the UI EventSystem handlers (`IPointerClickHandler.OnPointerClick`, `IDragHandler.OnDrag`, ...; full list in `UNITY_CALLBACK_INTERFACES`) are entry points when the class or struct lists that interface, whether or not it is a component. - **Attributes are entry points** in a Unity file: `[RuntimeInitializeOnLoadMethod]`, `[InitializeOnLoadMethod]`, `[InitializeOnLoad]` (class), `[MenuItem]`, `[ContextMenu]`, `[DidReloadScripts]`, `[PostProcessBuild]`, `[PostProcessScene]`, `[OnOpenAsset]` (with or without the `Attribute` suffix). A Unity file is one under `Assets/` or `Packages/`, or whose code (not comments or strings) has a `using UnityEngine…`/`using UnityEditor…` directive or a `UnityEngine.`/`UnityEditor.` qualified name. - **Public methods of such classes** stay findings but get a 0.25 penalty and the fp_risk "may be bound to a UnityEvent (e.g. Button.onClick), an Animation Event or SendMessage in a scene, prefab or animation clip, which are not indexed". Scenes, prefabs, animation clips and `.asset` files are not parsed, so plumb cannot tell a bound handler from dead code. - `[SerializeField]` fields: C# fields are not extracted, so they are never reported (tested, so this is noticed if field extraction is added). From ac8bf5ad40cdfd60ab99d385013c0e60c9496c2c Mon Sep 17 00:00:00 2001 From: grloper Date: Sat, 10 Oct 2026 11:48:17 +0300 Subject: [PATCH 9/9] chore: CHANGELOG [Unreleased] 1.0.1, test-count baseline 159 -> 194 Raises the test-integrity baseline by the 35 tests added on this branch (no test removed or weakened). README: the Windows section now says which binary ran which command. --- CHANGELOG.md | 18 ++++++++++++++++++ README.md | 2 +- scripts/test-count-baseline.txt | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19c4d92..fce5722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## [Unreleased] - 1.0.1 + +Fixes for problems found by using v1.0.0 on real projects (Windows, Unity). Not released; the crate version is still 1.0.0. + +### Fixed +- Windows: `plumb doctor` (and `enrich`, rule-engine detection in `verify --run`) reported every provider as missing because lookup used the bare name (`cargo`), never `cargo.exe` / `tsc.cmd`. Bare names are now tried with the `PATHEXT` extensions that `std::process::Command` can start (`.com`, `.exe`, `.bat`, `.cmd`); Unix lookup is unchanged. +- `plumb init` printed a Codex `config.toml` snippet with a `\\?\C:\...` path inside a TOML basic string, which does not parse. The verbatim prefix is stripped (`\\?\UNC\` becomes `\\`) and the path is written as a TOML literal string, or an escaped basic string when it contains `'` or control characters. Report paths no longer show `\\?\` either. +- `plumb map --budget N` was rejected; `--budget` is now a visible alias of `--tokens`. +- `plumb init` and `.gitignore`: `--dry-run` shows the lines it would add; it writes `.plumbgraph/*` + `!.plumbgraph/allow.toml` (the previous `.plumbgraph/` made the committed allow-list impossible to commit); any existing rule mentioning `.plumbgraph` (including `!.plumbgraph/`) is respected; it works in a subdirectory of a repository; CRLF files keep CRLF; a `.gitignore` that is not UTF-8 (for example UTF-16 from PowerShell 5) is no longer read as empty and overwritten. + +### Added +- C# / Unity dead-code entry points: MonoBehaviour/ScriptableObject/NetworkBehaviour/StateMachineBehaviour/UIBehaviour messages (`Awake`, `Start`, `Update`, `OnTriggerEnter2D`, ...), Unity attributes (`[RuntimeInitializeOnLoadMethod]`, `[MenuItem]`, `[InitializeOnLoad]`, ...), and Unity callback interfaces (`ISerializationCallbackReceiver`, `IHasCustomMenu`, UI EventSystem handlers). Public methods of Unity classes stay findings with a 0.25 penalty and a UnityEvent / Animation Event fp_risk. Rules and limits: `docs/PACKS.md`. On Unity's open-project-1 scripts (commit 608eac9) dead-code findings went from 371 to 166 (medium 97 to 28); on Newtonsoft.Json (52fa3ae) they are identical before and after (204). +- `plumb init --no-gitignore`. +- README: Windows install and PATH setup, and exactly what was tested on Windows. + +### Changed +- Index schema 3 / extractor 8 (new `framework_risk` column): existing indexes are rebuilt once. + ## [1.0.0] - 2026-10-10 ### Added diff --git a/README.md b/README.md index 96f3c53..88c7bf7 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ plumb --version plumb doctor . ``` -What was tested on Windows, and only this: Windows 11 Home 64-bit (build 26200) with Rust 1.91.1 `x86_64-pc-windows-gnu` from Chocolatey (no rustup, so `rust-toolchain.toml`'s 1.85 was not used, and the MSVC target was not tried). `cargo install --path crates/cli --locked` built and installed; `plumb index`, `map`, `dead-code`, `check-deps`, `verify`, `weakening`, `doctor`, `init --dry-run` and `plumb mcp` (driven by one MCP host) ran on a real repository. `cargo test --workspace` on that machine: every test passes except 10 that also fail on `main` there (LSP/diagnostics tests that need a real `python3`, which was only the Microsoft Store stub, and the closed-stdout-pipe test, which gets Windows error 232 instead of a broken pipe). CI runs on Linux only. +What was tested on Windows, and only this: Windows 11 Home 64-bit (build 26200) with Rust 1.91.1 `x86_64-pc-windows-gnu` from Chocolatey (no rustup, so `rust-toolchain.toml`'s 1.85 was not used, and the MSVC target was not tried). `cargo install --path crates/cli --locked` built and installed both v1.0.0 and the v1.0.1 changes. On one real repository, v1.0.0 ran `index`, `map`, `dead-code`, `check-deps`, `verify`, `weakening`, `doctor` and `plumb mcp` (driven by one MCP host), and the v1.0.1 build ran `doctor`, `map --budget` and `init --dry-run`. `cargo test --workspace` on that machine: every test passes except 10 that also fail on `main` there (LSP/diagnostics tests that need a real `python3`, which was only the Microsoft Store stub, and the closed-stdout-pipe test, which gets Windows error 232 instead of a broken pipe). CI runs on Linux only. ### Use with an agent (MCP) diff --git a/scripts/test-count-baseline.txt b/scripts/test-count-baseline.txt index 3f7d191..205a12b 100644 --- a/scripts/test-count-baseline.txt +++ b/scripts/test-count-baseline.txt @@ -1 +1 @@ -159 +194