From 8163ed4871249c005ee7ddcf920f76d739ca229e Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Sun, 2 Aug 2026 10:49:19 +0200 Subject: [PATCH 1/2] feat(config): Add per-analysis ignore globs for dupes, health, and dead-code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The top-level `ignore` list applies to every command, so accepting an intentional duplicate meant losing dead-code and health coverage for the whole file. Each command's config section now takes an `ignore` list of its own — `deadCode.ignore`, `dupes.ignore`, and `health.ignore` — unioned with the top-level list and applied only to that command, resolved against the config file's directory with the same glob rules. Health's excluded counts and footer include the scoped globs, and unknown fields in the new `deadCode` block fail loudly like everywhere else. Closes #32 --- README.md | 12 +- _typos.toml | 2 + docs/commands/dead-code.mdx | 5 +- docs/commands/dupes.mdx | 16 +- docs/commands/health.mdx | 9 +- docs/configuration.mdx | 52 ++++- docs/reference/json-output.mdx | 2 +- docs/suppressing-findings.mdx | 17 +- src/commands/dead_code.rs | 8 +- src/commands/dupes.rs | 5 +- src/commands/health.rs | 5 +- src/commands/mod.rs | 132 ++++++++++++- src/config.rs | 87 +++++++++ tests/cli.rs | 178 ++++++++++++++++++ .../Handlers/PingHandler.cs | 41 ++++ .../dead_code_scoped_ignore_json/Program.cs | 10 + .../Services/PingService.cs | 37 ++++ .../dead_code_scoped_ignore_json/StillDead.cs | 10 + .../dead_code_scoped_ignore_json/roe.json | 5 + .../Handlers/PingHandler.cs | 41 ++++ .../dead_code_scoped_ignore_yaml/Program.cs | 10 + .../Services/PingService.cs | 37 ++++ .../dead_code_scoped_ignore_yaml/StillDead.cs | 10 + .../dead_code_scoped_ignore_yaml/roe.yaml | 3 + .../Legacy/OldShippingService.cs | 61 ++++++ .../PaymentService.cs | 37 ++++ .../dupes_scoped_ignore_json/Program.cs | 10 + .../Vendor/VendorShipping.cs | 47 +++++ .../dupes_scoped_ignore_json/roe.json | 6 + .../Legacy/OldShippingService.cs | 61 ++++++ .../PaymentService.cs | 37 ++++ .../dupes_scoped_ignore_yaml/Program.cs | 10 + .../Vendor/VendorShipping.cs | 47 +++++ .../dupes_scoped_ignore_yaml/roe.yaml | 5 + .../health_scoped_ignore/Lib/Ignored.cs | 41 ++++ .../fixtures/health_scoped_ignore/Lib/Kept.cs | 32 ++++ .../health_scoped_ignore/Lib/Lib.csproj | 7 + tests/fixtures/health_scoped_ignore/roe.json | 5 + 38 files changed, 1103 insertions(+), 37 deletions(-) create mode 100644 tests/fixtures/dead_code_scoped_ignore_json/Handlers/PingHandler.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_json/Program.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_json/Services/PingService.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_json/StillDead.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_json/roe.json create mode 100644 tests/fixtures/dead_code_scoped_ignore_yaml/Handlers/PingHandler.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_yaml/Program.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_yaml/Services/PingService.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_yaml/StillDead.cs create mode 100644 tests/fixtures/dead_code_scoped_ignore_yaml/roe.yaml create mode 100644 tests/fixtures/dupes_scoped_ignore_json/Legacy/OldShippingService.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_json/PaymentService.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_json/Program.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_json/Vendor/VendorShipping.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_json/roe.json create mode 100644 tests/fixtures/dupes_scoped_ignore_yaml/Legacy/OldShippingService.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_yaml/PaymentService.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_yaml/Program.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_yaml/Vendor/VendorShipping.cs create mode 100644 tests/fixtures/dupes_scoped_ignore_yaml/roe.yaml create mode 100644 tests/fixtures/health_scoped_ignore/Lib/Ignored.cs create mode 100644 tests/fixtures/health_scoped_ignore/Lib/Kept.cs create mode 100644 tests/fixtures/health_scoped_ignore/Lib/Lib.csproj create mode 100644 tests/fixtures/health_scoped_ignore/roe.json diff --git a/README.md b/README.md index 2c9cd63..cb28e23 100644 --- a/README.md +++ b/README.md @@ -442,11 +442,15 @@ walking up from the analysis root to the nearest directory containing one "roots": ["MyApp.Program.Main"], "libraryProjects": ["MyLib"], "ignore": ["Migrations/**", "Generated/", "**/*.designer.cs"], + "deadCode": { + "ignore": ["Plugins/**"] + }, "dupes": { "mode": "semantic", "minTokens": 100, "minLines": 10, - "minOccurrences": 2 + "minOccurrences": 2, + "ignore": ["**/InsurerUnionQuery.cs"] }, "health": { "maxComplexity": 15, @@ -469,7 +473,11 @@ explicit `--aggressive`, `--root`, or `--library` always wins, otherwise the config's value applies, otherwise the built-in default (`false` / no extra roots / no extra library projects). The same `ignore` list also applies to `roe dupes`, since a duplicate spans multiple files and doesn't map cleanly -onto a single-line inline suppression comment. +onto a single-line inline suppression comment. Each command's section takes +an `ignore` list of its own with the same rules, unioned with the top-level +one and applied only to that command — so accepting an intentional duplicate +through `dupes.ignore` doesn't cost the file its dead-code and health +coverage. The `dupes` block sets defaults for `roe dupes`' matching mode and thresholds, and the `health` block does the same for `roe health`'s diff --git a/_typos.toml b/_typos.toml index f2a73e9..08a1ffa 100644 --- a/_typos.toml +++ b/_typos.toml @@ -8,3 +8,5 @@ extend-exclude = ["Cargo.lock", "tests/fixtures/", "tests/snapshots/"] # `rejects_unknown_fields` in src/config.rs to prove serde rejects near-miss # keys instead of silently ignoring them. agressive = "agressive" +# Same idea for the `ignore` field, used by `rejects_unknown_dead_code_fields`. +ignor = "ignor" diff --git a/docs/commands/dead-code.mdx b/docs/commands/dead-code.mdx index 74420f9..5d95648 100644 --- a/docs/commands/dead-code.mdx +++ b/docs/commands/dead-code.mdx @@ -50,7 +50,10 @@ never flagged even though no executable in the workspace uses it: roe dead-code --library MyLib ``` -Both can also be set persistently in a [config file](/configuration). +Both can also be set persistently in a [config file](/configuration). A +config's `deadCode.ignore` glob list drops every dead-code finding in +matching files without affecting the other commands — see +[Suppressing findings](/suppressing-findings). ## What counts as an entry point diff --git a/docs/commands/dupes.mdx b/docs/commands/dupes.mdx index b9427af..cf1ab65 100644 --- a/docs/commands/dupes.mdx +++ b/docs/commands/dupes.mdx @@ -56,6 +56,16 @@ roe dupes --mode semantic path/to/solution ## Ignoring files Duplicates span multiple files, so there's no inline suppression comment for -them. Instead, use the `ignore` glob list in a -[config file](/configuration) — it applies to every command, dropping every -finding in matching files. +them. Instead, use ignore globs in a [config file](/configuration): the +top-level `ignore` list applies to every command, and the `dupes.ignore` +list adds patterns only the duplicate analysis honours — so accepting an +intentional duplicate doesn't cost the file its dead-code and health +coverage. + +```json roe.json +{ + "dupes": { + "ignore": ["**/InsurerUnionQuery.cs"] + } +} +``` diff --git a/docs/commands/health.mdx b/docs/commands/health.mdx index 0b86609..bba150c 100644 --- a/docs/commands/health.mdx +++ b/docs/commands/health.mdx @@ -115,8 +115,8 @@ or a `??` whose fallback contains a ternary is counted on its own merits. The footer's `N project(s), N file(s), N symbol(s) scanned` counts what was *eligible to be reported*, not what was parsed. Anything ruled out up front — test projects under `--exclude-tests`, files matched by the config's -[`ignore` globs](/configuration) — is subtracted, and named on a line of its -own: +[`ignore` globs](/configuration) (top-level or `health.ignore`) — is +subtracted, and named on a line of its own: ```text found 8 issues across 7 locations in 6 files — 2 project(s), 118 file(s), 1204 symbol(s) scanned in 153 ms @@ -398,5 +398,6 @@ public void ParseEverything(string input) ``` Circular dependencies span multiple files, so — like duplicates — they have -no inline comment. Use the `ignore` globs in a -[config file](/configuration) instead. +no inline comment. Use ignore globs in a [config file](/configuration) +instead — `health.ignore` scopes the suppression to health alone, so the +file keeps its dead-code and dupes coverage. diff --git a/docs/configuration.mdx b/docs/configuration.mdx index 4f20972..cd6817e 100644 --- a/docs/configuration.mdx +++ b/docs/configuration.mdx @@ -27,11 +27,15 @@ error. "roots": ["MyApp.Program.Main"], "libraryProjects": ["MyLib"], "ignore": ["Migrations/**", "Generated/", "**/*.designer.cs"], + "deadCode": { + "ignore": ["Plugins/**"] + }, "dupes": { "mode": "semantic", "minTokens": 100, "minLines": 10, - "minOccurrences": 2 + "minOccurrences": 2, + "ignore": ["**/InsurerUnionQuery.cs"] }, "health": { "maxComplexity": 15, @@ -41,7 +45,8 @@ error. "maxFileLines": 750, "maxTypeMembers": 25, "excludeTests": true, - "baseline": "roe-baseline.json" + "baseline": "roe-baseline.json", + "ignore": ["**/GeneratedModels.cs"] } } ``` @@ -51,9 +56,10 @@ error. | `aggressive` | boolean | Also flag enum members and public settable auto-properties. Default for the `--aggressive` flag. | | `roots` | string[] | Fully-qualified symbol names to treat as extra entry-point roots. Default for `--root`. | | `libraryProjects` | string[] | Project names to always treat in library mode (public API counts as used). Default for `--library`. | -| `ignore` | string[] | Glob patterns; every finding in a matching file is dropped. Applies to all three commands. | -| `dupes` | object | Matching mode and thresholds for [`roe dupes`](/commands/dupes). Every key is optional. | -| `health` | object | Thresholds and the baseline path for [`roe health`](/commands/health). Every key is optional. | +| `ignore` | string[] | Glob patterns; every finding in a matching file is dropped. Applies to all three commands; each command's section can add its own. | +| `deadCode` | object | Extra `ignore` globs applied only to [`roe dead-code`](/commands/dead-code). | +| `dupes` | object | Matching mode, thresholds, and extra `ignore` globs for [`roe dupes`](/commands/dupes). Every key is optional. | +| `health` | object | Thresholds, the baseline path, and extra `ignore` globs for [`roe health`](/commands/health). Every key is optional. | Unknown fields are rejected, so a typo fails loudly instead of being silently ignored. @@ -69,6 +75,7 @@ Every field under `dupes` is optional and corresponds to a flag on | `minTokens` | number | `--min-tokens` | `50` | | `minLines` | number | `--min-lines` | `5` | | `minOccurrences` | number | `--min-occurrences` | `2` | +| `ignore` | string[] | — | none | This block is what calibrates a combined run: [`roe check`](/commands/check) takes no dupes flags of its own, so raising `minTokens` here is how a one-line @@ -89,6 +96,7 @@ Every field under `health` is optional and corresponds to a flag on | `maxTypeMembers` | number | `--max-type-members` | `20` | | `excludeTests` | boolean | `--exclude-tests` | `false` | | `baseline` | string | `--baseline` | none | +| `ignore` | string[] | — | none | Committing these is usually better than passing six flags on every CI invocation, and it keeps local runs and CI in agreement. @@ -108,9 +116,33 @@ full-backlog run. A trailing `/` matches the whole directory, so `"Generated/"` needs no `**`. Patterns containing `..` are unsupported and produce a warning. -The same `ignore` list also applies to `roe dupes` and to `roe health`'s -circular dependencies, since both span multiple files and don't map cleanly -onto a single-line [inline suppression comment](/suppressing-findings). +The top-level `ignore` list applies to all three commands — including +`roe dupes` and `roe health`'s circular dependencies, which span multiple +files and don't map cleanly onto a single-line +[inline suppression comment](/suppressing-findings). + +Each command's section takes an `ignore` list of its own with the same +rules, unioned with the top-level list and applied only to that command. +That keeps a suppression as narrow as the exception it accepts: ignoring an +intentional duplicate through `dupes.ignore` doesn't cost the file its +dead-code and health coverage. + +```yaml roe.yaml +ignore: + - "Generated/**" +deadCode: + ignore: + - "Plugins/**" +dupes: + ignore: + - "**/InsurerUnionQuery.cs" +health: + ignore: + - "**/GeneratedModels.cs" +``` + +A scoped list only ever adds patterns — it can't re-include a file the +top-level list ignores — and an empty one is a no-op. ## Precedence @@ -136,6 +168,10 @@ Dupes settings follow the same per-field order: `--min-tokens 100` overrides carries a value, so unlike `--aggressive` and `--exclude-tests` it has no on-only nuance — `--mode exact` overrides a config's `"semantic"` cleanly. +Ignore lists sit outside this order entirely: they have no CLI flag, and a +command's own `ignore` list is unioned with the top-level one rather than +replacing it. + To confirm a config-only `excludeTests` or `ignore` actually applied, read the `roe health` footer: the scanned counts narrow by whatever was excluded, and an `excluded:` line names it. See diff --git a/docs/reference/json-output.mdx b/docs/reference/json-output.mdx index 3d8e1f4..04885a7 100644 --- a/docs/reference/json-output.mdx +++ b/docs/reference/json-output.mdx @@ -216,7 +216,7 @@ including its own `version` and `root`, so anything already written against | --- | --- | | `version` | Schema version, `1`. | | `root` | The analysis root path. | -| `summary` | Scan totals: `projects`, `filesScanned`, `symbols`, one count per check (`highComplexity`, `highCognitiveComplexity`, `longMethods`, `tooManyParameters`, `largeFiles`, `largeTypes`, `circularDependencies`), and `elapsedMs`. `commitsWalked` is present only when `--hotspots` was passed. The three scan totals count what was eligible to be reported, so they narrow under `--exclude-tests` and the config's `ignore` globs. | +| `summary` | Scan totals: `projects`, `filesScanned`, `symbols`, one count per check (`highComplexity`, `highCognitiveComplexity`, `longMethods`, `tooManyParameters`, `largeFiles`, `largeTypes`, `circularDependencies`), and `elapsedMs`. `commitsWalked` is present only when `--hotspots` was passed. The three scan totals count what was eligible to be reported, so they narrow under `--exclude-tests` and the config's `ignore` globs (top-level and `health.ignore`). | | `summary.baselined` | How many findings and cycles a [baseline](/commands/health#baselines) hid — they are counted nowhere else in this document. Present only when a baseline was in force, so `0` means "the baseline is fully ratcheted" and absent means "no baseline was used". | | `summary.excluded` | What was ruled out before any check ran: `testProjects` (an array of project names, present under `--exclude-tests`) and `ignoredFiles` (a count). Omitted entirely when nothing was excluded. | | `findings[].kind` | `high-complexity`, `high-cognitive-complexity`, `long-method`, `too-many-parameters`, `large-file`, or `large-type` — the same names used by [inline suppressions](/suppressing-findings). | diff --git a/docs/suppressing-findings.mdx b/docs/suppressing-findings.mdx index cea0d7a..47fb177 100644 --- a/docs/suppressing-findings.mdx +++ b/docs/suppressing-findings.mdx @@ -78,10 +78,25 @@ To drop every finding in matching files — for `dead-code`, `dupes`, and } ``` +To keep a suppression as narrow as the exception it accepts, scope it to one +command instead: each command's section takes an `ignore` list of its own, +unioned with the top-level one and applied only to that command. Accepting +an intentional duplicate this way doesn't cost the file its dead-code and +health coverage: + +```json roe.json +{ + "dupes": { + "ignore": ["**/InsurerUnionQuery.cs"] + } +} +``` + See [Configuration](/configuration) for glob semantics and config discovery. Duplicate groups and circular dependencies span multiple files, so neither maps onto a single-line comment and neither has an inline suppression. - Ignore globs are the only way to silence them. + Ignore globs — top-level, `dupes.ignore`, or `health.ignore` — are the only + way to silence them. diff --git a/src/commands/dead_code.rs b/src/commands/dead_code.rs index fe50393..49076a7 100644 --- a/src/commands/dead_code.rs +++ b/src/commands/dead_code.rs @@ -121,13 +121,11 @@ pub(crate) fn execute_extracted( .warnings .extend(context.warnings.iter().cloned()); - if let Some(resolved) = &context.config - && let Some(ignore) = &resolved.config.ignore - { + if let Some((patterns, dir)) = context.ignore_for(commands::Analysis::DeadCode) { suppress::apply_config_ignores( &mut analysis.result, - ignore, - &resolved.dir, + &patterns, + dir, &mut analysis.workspace.warnings, ); } diff --git a/src/commands/dupes.rs b/src/commands/dupes.rs index b8bc23d..e101425 100644 --- a/src/commands/dupes.rs +++ b/src/commands/dupes.rs @@ -109,13 +109,16 @@ pub(crate) fn execute(context: &Context, args: &DupesArgs) -> anyhow::Result Option<(&[String], &Path)> { - self.config.as_ref().and_then(|resolved| { - resolved - .config - .ignore - .as_ref() - .map(|patterns| (patterns.as_slice(), resolved.dir.as_path())) - }) + /// The union of the config's top-level `ignore` globs and the given + /// analysis's own list, paired with the directory they resolve against. + /// `None` when the union is empty. + /// + /// Built in one place so every consumer of an analysis's globs — filters, + /// summaries, footers — sees the same list and their counts can't drift. + pub fn ignore_for(&self, analysis: Analysis) -> Option<(Vec, &Path)> { + let resolved = self.config.as_ref()?; + let config = &resolved.config; + let scoped = match analysis { + Analysis::DeadCode => config.dead_code.as_ref().and_then(|c| c.ignore.as_deref()), + Analysis::Dupes => config.dupes.as_ref().and_then(|c| c.ignore.as_deref()), + Analysis::Health => config.health.as_ref().and_then(|c| c.ignore.as_deref()), + }; + + let mut patterns = config.ignore.clone().unwrap_or_default(); + patterns.extend(scoped.into_iter().flatten().cloned()); + + if patterns.is_empty() { + return None; + } + + Some((patterns, resolved.dir.as_path())) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{DeadCodeConfig, DupesConfig, HealthConfig, ResolvedConfig, RoeConfig}; + + fn context_with(config: RoeConfig) -> Context { + Context { + root: PathBuf::from("/repo"), + config: Some(ResolvedConfig { + path: PathBuf::from("/repo/roe.json"), + dir: PathBuf::from("/repo"), + config, + }), + warnings: Vec::new(), + } + } + + #[test] + fn ignore_for_unions_top_level_and_scoped_globs() { + let context = context_with(RoeConfig { + ignore: Some(vec!["Generated/".to_string()]), + dupes: Some(DupesConfig { + ignore: Some(vec!["Legacy/".to_string()]), + ..Default::default() + }), + ..Default::default() + }); + + let (patterns, dir) = context.ignore_for(Analysis::Dupes).expect("a union"); + + assert_eq!( + patterns, + vec!["Generated/".to_string(), "Legacy/".to_string()] + ); + assert_eq!(dir, Path::new("/repo")); + } + + #[test] + fn ignore_for_scopes_each_list_to_its_own_analysis() { + let context = context_with(RoeConfig { + dead_code: Some(DeadCodeConfig { + ignore: Some(vec!["Plugins/".to_string()]), + }), + dupes: Some(DupesConfig { + ignore: Some(vec!["Legacy/".to_string()]), + ..Default::default() + }), + health: Some(HealthConfig { + ignore: Some(vec!["Models/".to_string()]), + ..Default::default() + }), + ..Default::default() + }); + + let (dead_code, _) = context.ignore_for(Analysis::DeadCode).expect("patterns"); + let (dupes, _) = context.ignore_for(Analysis::Dupes).expect("patterns"); + let (health, _) = context.ignore_for(Analysis::Health).expect("patterns"); + + assert_eq!(dead_code, vec!["Plugins/".to_string()]); + assert_eq!(dupes, vec!["Legacy/".to_string()]); + assert_eq!(health, vec!["Models/".to_string()]); + } + + #[test] + fn ignore_for_returns_none_when_nothing_is_configured() { + let no_config = Context { + root: PathBuf::from("/repo"), + config: None, + warnings: Vec::new(), + }; + assert!(no_config.ignore_for(Analysis::Dupes).is_none()); + + let empty = context_with(RoeConfig::default()); + assert!(empty.ignore_for(Analysis::Health).is_none()); + } + + #[test] + fn ignore_for_treats_an_empty_scoped_list_as_a_no_op() { + let context = context_with(RoeConfig { + dupes: Some(DupesConfig { + ignore: Some(Vec::new()), + ..Default::default() + }), + ..Default::default() + }); + + assert!(context.ignore_for(Analysis::Dupes).is_none()); } } diff --git a/src/config.rs b/src/config.rs index f3b27f4..843de6a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -21,6 +21,9 @@ pub struct RoeConfig { /// matching files have all their findings suppressed. A pattern ending in /// `/` also matches everything under that directory. pub ignore: Option>, + /// Extra `ignore` globs applied only to `roe dead-code`. + #[serde(rename = "deadCode")] + pub dead_code: Option, /// Defaults for `roe dupes`' matching mode and thresholds, so a combined /// `roe check` — which takes no dupes flags — can be calibrated. pub dupes: Option, @@ -29,6 +32,15 @@ pub struct RoeConfig { pub health: Option, } +/// The `deadCode` block of a config file. +#[derive(Debug, Default, Clone, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct DeadCodeConfig { + /// Extra ignore globs applied only to `roe dead-code`, unioned with the + /// top-level `ignore` list and resolved with the same rules. + pub ignore: Option>, +} + /// The `health` block of a config file. Every field is optional; an absent /// one falls back to the built-in default. #[derive(Debug, Default, Clone, Deserialize)] @@ -44,6 +56,9 @@ pub struct HealthConfig { /// Path to a baseline file, relative to this config file's own directory /// the way `ignore` globs are. pub baseline: Option, + /// Extra ignore globs applied only to `roe health`, unioned with the + /// top-level `ignore` list and resolved with the same rules. + pub ignore: Option>, } /// The `dupes` block of a config file. Every field is optional; an absent one @@ -51,6 +66,9 @@ pub struct HealthConfig { #[derive(Debug, Default, Clone, Deserialize)] #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct DupesConfig { + /// Extra ignore globs applied only to `roe dupes`, unioned with the + /// top-level `ignore` list and resolved with the same rules. + pub ignore: Option>, pub min_lines: Option, pub min_occurrences: Option, pub min_tokens: Option, @@ -729,6 +747,75 @@ mod tests { assert_eq!(dupes.min_tokens, Some(100)); } + #[test] + fn parses_dupes_ignore_globs() { + let config: RoeConfig = + serde_json::from_str(r#"{"dupes": {"ignore": ["Legacy/"]}}"#).expect("valid json"); + let dupes = config.dupes.expect("a dupes block"); + + assert_eq!(dupes.ignore, Some(vec!["Legacy/".to_string()])); + assert_eq!(dupes.min_tokens, None); + } + + #[test] + fn parses_health_ignore_globs() { + let config: RoeConfig = + serde_json::from_str(r#"{"health": {"ignore": ["**/GeneratedModels.cs"]}}"#) + .expect("valid json"); + let health = config.health.expect("a health block"); + + assert_eq!( + health.ignore, + Some(vec!["**/GeneratedModels.cs".to_string()]) + ); + } + + #[test] + fn parses_dead_code_ignore_globs() { + let config: RoeConfig = + serde_json::from_str(r#"{"deadCode": {"ignore": ["Handlers/"]}}"#).expect("valid json"); + let dead_code = config.dead_code.expect("a deadCode block"); + + assert_eq!(dead_code.ignore, Some(vec!["Handlers/".to_string()])); + } + + #[test] + fn parses_yaml_scoped_ignores() { + // The exact shape issue #32 asked for. + let config: RoeConfig = serde_yaml_ng::from_str(concat!( + "ignore:\n", + " - \"Generated/**\"\n", + "dupes:\n", + " ignore:\n", + " - \"**/DashboardGetPlatformPolicyMembers.cs\"\n", + "health:\n", + " ignore:\n", + " - \"**/GeneratedModels.cs\"\n", + )) + .expect("valid yaml"); + + assert_eq!(config.ignore, Some(vec!["Generated/**".to_string()])); + assert_eq!( + config.dupes.expect("a dupes block").ignore, + Some(vec!["**/DashboardGetPlatformPolicyMembers.cs".to_string()]) + ); + assert_eq!( + config.health.expect("a health block").ignore, + Some(vec!["**/GeneratedModels.cs".to_string()]) + ); + } + + #[test] + fn rejects_unknown_dead_code_fields() { + // `ignor` is a near-miss of the real `ignore` field — it must fail + // loudly rather than leave the user thinking a suppression is in + // force when it isn't. It is declared in `_typos.toml` so the spell + // checker leaves it alone. + let error = serde_json::from_str::(r#"{"deadCode": {"ignor": []}}"#) + .expect_err("typo should not be silently ignored"); + assert!(error.to_string().contains("unknown field")); + } + #[test] fn rejects_unknown_dupes_fields() { // `minToken` is a near-miss of the real `minTokens` field — it must diff --git a/tests/cli.rs b/tests/cli.rs index 508ce72..75cc18e 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -108,6 +108,44 @@ fn config_ignore_yaml_filters_out_ignored_folder() { assert!(!stdout.contains("Ignored")); } +#[test] +fn dead_code_scoped_ignore_json_drops_findings_in_matching_files() { + let output = roe() + .args(["dead-code", &fixture("dead_code_scoped_ignore_json")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("StillDead"), "got {stdout}"); + assert!(!stdout.contains("Handlers"), "got {stdout}"); +} + +#[test] +fn dead_code_scoped_ignore_yaml_drops_findings_in_matching_files() { + let output = roe() + .args(["dead-code", &fixture("dead_code_scoped_ignore_yaml")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("StillDead"), "got {stdout}"); + assert!(!stdout.contains("Handlers"), "got {stdout}"); +} + +/// The scoped list must not leak into the other analyses: the ignored +/// handler's duplicate occurrence still counts. +#[test] +fn dead_code_scoped_ignore_leaves_dupes_findings_alone() { + let output = roe() + .args(["dupes", &fixture("dead_code_scoped_ignore_json")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("found 1 duplicate group"), "got {stdout}"); + assert!(stdout.contains("PingHandler.cs"), "got {stdout}"); +} + #[test] fn config_resolution_walks_up_to_parent_directory() { // roe.json lives at the fixture root; --path points at a nested @@ -295,6 +333,65 @@ fn dupes_config_ignore_yaml_drops_the_ignored_occurrence() { assert!(stdout.contains("no duplicate code found")); } +#[test] +fn dupes_scoped_ignore_json_drops_the_duplicate() { + // `Legacy/` comes from `dupes.ignore` and `Vendor/` from the top-level + // list; only their union leaves a single occurrence, which is below the + // occurrence threshold. + let output = roe() + .args(["dupes", &fixture("dupes_scoped_ignore_json")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(0)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("no duplicate code found"), "got {stdout}"); +} + +#[test] +fn dupes_scoped_ignore_yaml_drops_the_duplicate() { + let output = roe() + .args(["dupes", &fixture("dupes_scoped_ignore_yaml")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(0)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("no duplicate code found"), "got {stdout}"); +} + +/// The acceptance criterion of the scoped-ignore feature: suppressing an +/// intentional duplicate must not cost the file its dead-code coverage. +#[test] +fn dupes_scoped_ignore_leaves_dead_code_findings_alone() { + let output = roe() + .args(["dead-code", &fixture("dupes_scoped_ignore_json")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Legacy/OldShippingService.cs"), + "got {stdout}" + ); + // The top-level list still applies to every command. + assert!(!stdout.contains("Vendor"), "got {stdout}"); +} + +#[test] +fn dupes_scoped_ignore_leaves_health_findings_alone() { + let output = roe() + .args([ + "health", + &fixture("dupes_scoped_ignore_json"), + "--max-complexity", + "2", + ]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("OldShippingService"), "got {stdout}"); +} + #[test] fn dupes_config_thresholds_json_reveal_a_short_clone() { let output = roe() @@ -833,6 +930,69 @@ fn health_ignore_globs_drop_findings_and_the_cycles_they_touch() { ); } +#[test] +fn health_scoped_ignore_drops_findings_and_counts_the_exclusion() { + let output = roe() + .args([ + "health", + &fixture("health_scoped_ignore"), + "--format", + "json", + "--max-complexity", + "2", + ]) + .output() + .expect("command runs"); + + let stdout = normalize(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(&stdout).expect("valid JSON"); + + let names: Vec<&str> = parsed["findings"] + .as_array() + .expect("findings array") + .iter() + .filter_map(|finding| finding["name"].as_str()) + .collect(); + assert_eq!(names, vec!["Lib.KeptWidget.Branchy"], "got {stdout}"); + + let summary = &parsed["summary"]; + assert_eq!(summary["filesScanned"], 1, "got {stdout}"); + assert_eq!(summary["excluded"]["ignoredFiles"], 1, "got {stdout}"); +} + +#[test] +fn health_scoped_ignore_shows_in_the_footer() { + let output = roe() + .args([ + "health", + &fixture("health_scoped_ignore"), + "--max-complexity", + "2", + ]) + .output() + .expect("command runs"); + + let stdout = normalize(&output.stdout); + + assert!( + stdout.contains("excluded: 1 ignored file"), + "the footer must count what the scoped glob dropped, got:\n{stdout}" + ); +} + +/// The scoped list must not leak into the other analyses: the health-ignored +/// file's dead code is still reported. +#[test] +fn health_scoped_ignore_leaves_dead_code_findings_alone() { + let output = roe() + .args(["dead-code", &fixture("health_scoped_ignore")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("Lib/Ignored.cs"), "got {stdout}"); +} + /// A writable path for a baseline the test owns. `CARGO_TARGET_TMPDIR` is /// per-crate and cleaned by `cargo clean`, so nothing leaks into the fixtures. fn scratch(name: &str) -> std::path::PathBuf { @@ -1164,6 +1324,24 @@ fn check_reads_the_dupes_config_for_the_combined_run() { assert!(stdout.contains("duplicate group"), "got {stdout}"); } +/// One shared context feeds all three analyses, so each must apply its own +/// scoped list on top of the top-level one. +#[test] +fn check_applies_scoped_ignores_per_analysis() { + let output = roe() + .args([&fixture("dupes_scoped_ignore_json")]) + .output() + .expect("command runs"); + assert_eq!(output.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("no duplicate code found"), "got {stdout}"); + assert!( + stdout.contains("Legacy/OldShippingService.cs"), + "got {stdout}" + ); + assert!(!stdout.contains("Vendor"), "got {stdout}"); +} + /// The combined JSON report must state the mode that actually ran, not a /// hard-coded `exact`. #[test] diff --git a/tests/fixtures/dead_code_scoped_ignore_json/Handlers/PingHandler.cs b/tests/fixtures/dead_code_scoped_ignore_json/Handlers/PingHandler.cs new file mode 100644 index 0000000..751a2b9 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_json/Handlers/PingHandler.cs @@ -0,0 +1,41 @@ +using System; +using System.Threading.Tasks; + +namespace Messaging.Handlers; + +// Matched only by the `deadCode.ignore` glob: this class is unused, but its +// finding must be dropped — while its retry-method occurrence must still +// appear in the duplicate report, because the scoped glob applies to +// dead-code alone. +internal class PingHandler +{ + public Task HandleAsync(string requestId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Handling ping {requestId}"); + await Task.CompletedTask; + }, maxAttempts: 2); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_json/Program.cs b/tests/fixtures/dead_code_scoped_ignore_json/Program.cs new file mode 100644 index 0000000..38d2c6a --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_json/Program.cs @@ -0,0 +1,10 @@ +namespace Messaging; + +public static class Program +{ + public static void Main() + { + var pings = new Services.PingService(); + pings.PingAsync("gateway-1").Wait(); + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_json/Services/PingService.cs b/tests/fixtures/dead_code_scoped_ignore_json/Services/PingService.cs new file mode 100644 index 0000000..3f169a5 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_json/Services/PingService.cs @@ -0,0 +1,37 @@ +using System; +using System.Threading.Tasks; + +namespace Messaging.Services; + +public class PingService +{ + public Task PingAsync(string hostId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Pinging {hostId}"); + await Task.CompletedTask; + }, maxAttempts: 3); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_json/StillDead.cs b/tests/fixtures/dead_code_scoped_ignore_json/StillDead.cs new file mode 100644 index 0000000..b795644 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_json/StillDead.cs @@ -0,0 +1,10 @@ +namespace Messaging; + +// Not covered by any ignore glob, so this finding must survive the +// `deadCode.ignore` filtering that drops Handlers/. +internal class StillDead +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_json/roe.json b/tests/fixtures/dead_code_scoped_ignore_json/roe.json new file mode 100644 index 0000000..c48960e --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_json/roe.json @@ -0,0 +1,5 @@ +{ + "deadCode": { + "ignore": ["Handlers/"] + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_yaml/Handlers/PingHandler.cs b/tests/fixtures/dead_code_scoped_ignore_yaml/Handlers/PingHandler.cs new file mode 100644 index 0000000..751a2b9 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_yaml/Handlers/PingHandler.cs @@ -0,0 +1,41 @@ +using System; +using System.Threading.Tasks; + +namespace Messaging.Handlers; + +// Matched only by the `deadCode.ignore` glob: this class is unused, but its +// finding must be dropped — while its retry-method occurrence must still +// appear in the duplicate report, because the scoped glob applies to +// dead-code alone. +internal class PingHandler +{ + public Task HandleAsync(string requestId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Handling ping {requestId}"); + await Task.CompletedTask; + }, maxAttempts: 2); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_yaml/Program.cs b/tests/fixtures/dead_code_scoped_ignore_yaml/Program.cs new file mode 100644 index 0000000..38d2c6a --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_yaml/Program.cs @@ -0,0 +1,10 @@ +namespace Messaging; + +public static class Program +{ + public static void Main() + { + var pings = new Services.PingService(); + pings.PingAsync("gateway-1").Wait(); + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_yaml/Services/PingService.cs b/tests/fixtures/dead_code_scoped_ignore_yaml/Services/PingService.cs new file mode 100644 index 0000000..3f169a5 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_yaml/Services/PingService.cs @@ -0,0 +1,37 @@ +using System; +using System.Threading.Tasks; + +namespace Messaging.Services; + +public class PingService +{ + public Task PingAsync(string hostId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Pinging {hostId}"); + await Task.CompletedTask; + }, maxAttempts: 3); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_yaml/StillDead.cs b/tests/fixtures/dead_code_scoped_ignore_yaml/StillDead.cs new file mode 100644 index 0000000..b795644 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_yaml/StillDead.cs @@ -0,0 +1,10 @@ +namespace Messaging; + +// Not covered by any ignore glob, so this finding must survive the +// `deadCode.ignore` filtering that drops Handlers/. +internal class StillDead +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dead_code_scoped_ignore_yaml/roe.yaml b/tests/fixtures/dead_code_scoped_ignore_yaml/roe.yaml new file mode 100644 index 0000000..f30dde0 --- /dev/null +++ b/tests/fixtures/dead_code_scoped_ignore_yaml/roe.yaml @@ -0,0 +1,3 @@ +deadCode: + ignore: + - "Handlers/" diff --git a/tests/fixtures/dupes_scoped_ignore_json/Legacy/OldShippingService.cs b/tests/fixtures/dupes_scoped_ignore_json/Legacy/OldShippingService.cs new file mode 100644 index 0000000..d5911c6 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_json/Legacy/OldShippingService.cs @@ -0,0 +1,61 @@ +using System; +using System.Threading.Tasks; + +namespace Billing.Legacy; + +// Matched only by the `dupes.ignore` glob: its retry-method occurrence must +// vanish from the duplicate report, while this file's dead-code finding and +// the health finding for Route stay. +public class OldShippingService +{ + public Task DispatchAsync(string orderId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Dispatching order {orderId}"); + await Task.CompletedTask; + }, maxAttempts: 5); + } + + public string Route(bool express, bool oversized, bool fragile) + { + if (express) + { + return "air"; + } + else if (oversized && fragile) + { + return "special"; + } + + return "ground"; + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} + +internal class LegacyLedger +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_json/PaymentService.cs b/tests/fixtures/dupes_scoped_ignore_json/PaymentService.cs new file mode 100644 index 0000000..6117741 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_json/PaymentService.cs @@ -0,0 +1,37 @@ +using System; +using System.Threading.Tasks; + +namespace Billing; + +public class PaymentService +{ + public Task ChargeAsync(string customerId, decimal amount) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Charging {customerId} for {amount:C}"); + await Task.CompletedTask; + }, maxAttempts: 3); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_json/Program.cs b/tests/fixtures/dupes_scoped_ignore_json/Program.cs new file mode 100644 index 0000000..79b6449 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_json/Program.cs @@ -0,0 +1,10 @@ +namespace Billing; + +public static class Program +{ + public static void Main() + { + var payments = new PaymentService(); + payments.ChargeAsync("customer-1", 25m).Wait(); + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_json/Vendor/VendorShipping.cs b/tests/fixtures/dupes_scoped_ignore_json/Vendor/VendorShipping.cs new file mode 100644 index 0000000..14fcaa9 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_json/Vendor/VendorShipping.cs @@ -0,0 +1,47 @@ +using System; +using System.Threading.Tasks; + +namespace Billing.Vendor; + +// Matched by the top-level `ignore` glob, which applies to every command: +// neither the duplicate occurrence nor this file's dead-code finding may +// appear anywhere. +public class VendorShipping +{ + public Task ShipAsync(string parcelId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Shipping parcel {parcelId}"); + await Task.CompletedTask; + }, maxAttempts: 4); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} + +internal class VendorLedger +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_json/roe.json b/tests/fixtures/dupes_scoped_ignore_json/roe.json new file mode 100644 index 0000000..97304db --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_json/roe.json @@ -0,0 +1,6 @@ +{ + "ignore": ["Vendor/"], + "dupes": { + "ignore": ["Legacy/"] + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_yaml/Legacy/OldShippingService.cs b/tests/fixtures/dupes_scoped_ignore_yaml/Legacy/OldShippingService.cs new file mode 100644 index 0000000..d5911c6 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_yaml/Legacy/OldShippingService.cs @@ -0,0 +1,61 @@ +using System; +using System.Threading.Tasks; + +namespace Billing.Legacy; + +// Matched only by the `dupes.ignore` glob: its retry-method occurrence must +// vanish from the duplicate report, while this file's dead-code finding and +// the health finding for Route stay. +public class OldShippingService +{ + public Task DispatchAsync(string orderId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Dispatching order {orderId}"); + await Task.CompletedTask; + }, maxAttempts: 5); + } + + public string Route(bool express, bool oversized, bool fragile) + { + if (express) + { + return "air"; + } + else if (oversized && fragile) + { + return "special"; + } + + return "ground"; + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} + +internal class LegacyLedger +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_yaml/PaymentService.cs b/tests/fixtures/dupes_scoped_ignore_yaml/PaymentService.cs new file mode 100644 index 0000000..6117741 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_yaml/PaymentService.cs @@ -0,0 +1,37 @@ +using System; +using System.Threading.Tasks; + +namespace Billing; + +public class PaymentService +{ + public Task ChargeAsync(string customerId, decimal amount) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Charging {customerId} for {amount:C}"); + await Task.CompletedTask; + }, maxAttempts: 3); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_yaml/Program.cs b/tests/fixtures/dupes_scoped_ignore_yaml/Program.cs new file mode 100644 index 0000000..79b6449 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_yaml/Program.cs @@ -0,0 +1,10 @@ +namespace Billing; + +public static class Program +{ + public static void Main() + { + var payments = new PaymentService(); + payments.ChargeAsync("customer-1", 25m).Wait(); + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_yaml/Vendor/VendorShipping.cs b/tests/fixtures/dupes_scoped_ignore_yaml/Vendor/VendorShipping.cs new file mode 100644 index 0000000..14fcaa9 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_yaml/Vendor/VendorShipping.cs @@ -0,0 +1,47 @@ +using System; +using System.Threading.Tasks; + +namespace Billing.Vendor; + +// Matched by the top-level `ignore` glob, which applies to every command: +// neither the duplicate occurrence nor this file's dead-code finding may +// appear anywhere. +public class VendorShipping +{ + public Task ShipAsync(string parcelId) + { + return ExecuteWithRetryAsync(async () => + { + Console.WriteLine($"Shipping parcel {parcelId}"); + await Task.CompletedTask; + }, maxAttempts: 4); + } + + private async Task ExecuteWithRetryAsync(Func action, int maxAttempts) + { + var attempt = 0; + + while (true) + { + try + { + await action(); + return; + } + catch (Exception exception) when (attempt < maxAttempts) + { + attempt++; + var delay = TimeSpan.FromSeconds(Math.Pow(2, attempt)); + Console.WriteLine($"Retry {attempt} after {exception.Message}, waiting {delay}"); + await Task.Delay(delay); + } + } + } +} + +internal class VendorLedger +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/dupes_scoped_ignore_yaml/roe.yaml b/tests/fixtures/dupes_scoped_ignore_yaml/roe.yaml new file mode 100644 index 0000000..7cf1f25 --- /dev/null +++ b/tests/fixtures/dupes_scoped_ignore_yaml/roe.yaml @@ -0,0 +1,5 @@ +ignore: + - "Vendor/" +dupes: + ignore: + - "Legacy/" diff --git a/tests/fixtures/health_scoped_ignore/Lib/Ignored.cs b/tests/fixtures/health_scoped_ignore/Lib/Ignored.cs new file mode 100644 index 0000000..4162474 --- /dev/null +++ b/tests/fixtures/health_scoped_ignore/Lib/Ignored.cs @@ -0,0 +1,41 @@ +namespace Lib; + +// Matched by the config's `health.ignore` glob. The findings in here and the +// cycle entirely contained in here must be dropped from the health report — +// while this file's dead-code finding survives, because the scoped glob +// applies to health alone. +public class IgnoredWidget +{ + public void Branchy(bool a, bool b, bool c) + { + if (a) + { + DoA(); + } + else if (b && c) + { + DoB(); + } + } + + private void DoA() { } + + private void DoB() { } +} + +public class IgnoredGamma +{ + public IgnoredDelta Delta; +} + +public class IgnoredDelta +{ + public IgnoredGamma Gamma; +} + +internal class IgnoredLedger +{ + public void DoNothing() + { + } +} diff --git a/tests/fixtures/health_scoped_ignore/Lib/Kept.cs b/tests/fixtures/health_scoped_ignore/Lib/Kept.cs new file mode 100644 index 0000000..7e598ba --- /dev/null +++ b/tests/fixtures/health_scoped_ignore/Lib/Kept.cs @@ -0,0 +1,32 @@ +namespace Lib; + +// Neither this file nor its cycle is covered by the config's ignore glob, so +// both must survive the filtering that drops Ignored.cs. +public class KeptWidget +{ + public void Branchy(bool a, bool b, bool c) + { + if (a) + { + DoA(); + } + else if (b && c) + { + DoB(); + } + } + + private void DoA() { } + + private void DoB() { } +} + +public class KeptAlpha +{ + public KeptBeta Beta; +} + +public class KeptBeta +{ + public KeptAlpha Alpha; +} diff --git a/tests/fixtures/health_scoped_ignore/Lib/Lib.csproj b/tests/fixtures/health_scoped_ignore/Lib/Lib.csproj new file mode 100644 index 0000000..c92aa9b --- /dev/null +++ b/tests/fixtures/health_scoped_ignore/Lib/Lib.csproj @@ -0,0 +1,7 @@ + + + Exe + net8.0 + enable + + diff --git a/tests/fixtures/health_scoped_ignore/roe.json b/tests/fixtures/health_scoped_ignore/roe.json new file mode 100644 index 0000000..cd45471 --- /dev/null +++ b/tests/fixtures/health_scoped_ignore/roe.json @@ -0,0 +1,5 @@ +{ + "health": { + "ignore": ["Lib/Ignored.cs"] + } +} From 83416b0dddca18416e79b549ec7bf170fa522626 Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Sun, 2 Aug 2026 10:50:26 +0200 Subject: [PATCH 2/2] docs: Add an agent-facing skill describing roe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An Anthropic Agent Skill that teaches coding agents when and how to run roe: command surface, JSON schema, suppression mechanics, config precedence, and the baseline ratchet — current with the config's `dupes` block and the per-analysis ignore globs. --- skills/roe/SKILL.md | 257 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 skills/roe/SKILL.md diff --git a/skills/roe/SKILL.md b/skills/roe/SKILL.md new file mode 100644 index 0000000..6713e27 --- /dev/null +++ b/skills/roe/SKILL.md @@ -0,0 +1,257 @@ +--- +name: roe +description: >- + Analyze C#/.NET codebases with the roe CLI: find dead code (unused types, + members, and files), duplicated code, and code-health problems (complexity, + size, coupling) using pure static analysis — no build required. Use this + skill whenever you are working in a repository containing .cs, .sln, or + .csproj files and the task involves deleting unused code, cleaning up, + refactoring, hunting copy-paste duplication, auditing code quality or + complexity, or gating a CI pipeline on code quality — even if the user + never mentions roe by name. +--- + +# Using roe + +roe is a codebase-intelligence CLI for C#. It parses source with tree-sitter +and never compiles or runs anything, so it works on codebases that don't +build. It is deliberately conservative: when in doubt, code is marked *used*. +A missed finding is cheap; a false positive is treated as a bug. That means +findings are high-confidence and generally safe to act on. + +roe only analyzes C# — it scans `.cs`, `.sln`, and `.csproj` files. Generated +code (`*.g.cs`, `*.Designer.cs`, `` headers, `obj/`) is read +for references but never flagged. + +## Check availability, install if needed + +```bash +roe --version +``` + +If missing, install one of: + +```bash +dotnet tool install --global roe # recommended; .NET SDK 8.0+ +npm install --global roe-cli # npm package is roe-cli, binary is roe +dnx roe . # one-shot, .NET SDK 10+ +npx roe-cli . # one-shot via npm +``` + +roe is not on crates.io; prebuilt binaries are also on GitHub Releases. + +## Core workflow + +Always use `--format json` — the output is a stable, versioned schema built +for machines. Human format is for terminals. + +```bash +roe --format json [PATH] # PATH = directory, .sln, or .csproj; defaults to cwd +``` + +A bare `roe` (or `roe check`) runs all three analyses and prints **one** +combined JSON document: `{version, root, deadCode, dupes, health}`. Each +nested value is exactly what the individual command would emit. + +Exit codes: + +| Code | Meaning | +| --- | --- | +| `0` | Clean — no findings. | +| `1` | Findings were reported. For the combined run: any analysis found something. | +| `2` | Error (`error: …` on stderr) — bad path, malformed config, etc. | + +Warnings appear on stderr as `warning: …` and never affect the exit code. + +After fixing findings, **re-run roe**. Deleting dead code cascades: code that +was only referenced by the deleted code becomes newly dead on the next run. +Repeat until clean, and verify with the project's own build and tests. + +## Commands + +### `roe check [PATH]` (the default) + +Runs dead-code, dupes, and health together. Takes only `-f/--format` and +`--config` — per-analysis tuning deliberately lives in the config file, not +flags: the `dupes` block calibrates the duplicate analysis and the `health` +block its thresholds. In the combined output, `dupes.mode` states the mode +that actually ran (`exact` unless the config says otherwise) and +`health.hotspots` is always empty. + +### `roe dead-code [PATH]` + +Finds unused types, members, and files. + +| Flag | Effect | +| --- | --- | +| `--aggressive` | Also flag enum members and public settable auto-properties. | +| `--root ` | Extra entry-point root (repeatable). Use for code reached in ways static analysis can't see: string-based reflection, external plugin hosts. | +| `--library ` | Treat this project's public API as used (repeatable). Use when the project is consumed outside the workspace — e.g. a DLL referenced by a Unity project. | + +Entry points are never flagged: `static Main` / top-level statements, +ASP.NET controllers and actions, test methods, anything carrying a non-inert +attribute, reflection-scan contracts (`AddHostedService()`-style), and the +public API of library-mode projects. + +### `roe dupes [PATH]` + +Finds duplicated code blocks. + +| Flag | Default | Effect | +| --- | --- | --- | +| `--mode ` | `exact` | `semantic` normalizes identifiers and numeric literals, so renamed-but-structurally-identical blocks match too. | +| `--min-tokens ` | `50` | Minimum token-run length to report. | +| `--min-lines ` | `5` | Minimum line span of the shortest occurrence. | +| `--min-occurrences ` | `2` | Minimum number of copies. | +| `--no-code` | | Hide the duplicated source in human output. | + +`--mode` and the three thresholds fall back to the config file's `dupes` +block before their built-in defaults, so a committed calibration also +applies to a flagless `roe check`. + +### `roe health [PATH]` + +Flags complexity, size, and coupling issues. Thresholds are +strictly-greater-than: a value equal to the threshold is clean. + +| Flag | Default | Flags … | +| --- | --- | --- | +| `--max-complexity ` | `10` | methods/properties above this cyclomatic complexity. | +| `--max-cognitive ` | `15` | above this cognitive complexity (nesting-weighted). | +| `--max-method-lines ` | `40` | method bodies spanning more lines than this. | +| `--max-parameters ` | `5` | methods declared with more required parameters than this. | +| `--max-file-lines ` | `750` | files longer than this. | +| `--max-type-members ` | `20` | types with more members than this. | + +Other flags: `--exclude-tests` (skip test projects — long arrange/act/assert +methods are normal there), `--sort `, `--limit ` (truncates +the human report only; JSON is never truncated), `--hotspots --top ` +(ranks complex-and-frequently-changed files from git history; informational, +never affects the exit code; requires a real git checkout, so CI needs +`fetch-depth: 0`), and `--baseline` / `--write-baseline` (see below). + +Circular dependencies between types are always reported (as `cycles` in +JSON) — there is no threshold for them. + +## Reading the JSON + +Schema `version` is `1`; changes are additive only. Keys are camelCase, +optional fields are omitted, and `file` paths are relative to `root`. + +Each report has a `summary` (scan totals) and `findings` array. Every finding +carries `kind`, `name` (fully-qualified symbol, or file path for file-level +findings), `file`, `line`, `column`. Health findings add `metric` and +`threshold` — `metric / threshold` is the severity. Dupes reports have +`groups[]`, each with `tokenCount`, `lineCount`, and `occurrences[]` +(`file`, `startLine`..`endColumn`). + +The `kind` values (identical to the inline-suppression rule names): +`unused-type`, `unused-member`, `unused-file`, `high-complexity`, +`high-cognitive-complexity`, `long-method`, `too-many-parameters`, +`large-file`, `large-type`. + +## Suppressing false positives + +Prefer fixing real findings over suppressing. When a finding genuinely is a +false positive (e.g. code invoked via string-based reflection), first +consider whether `--root`/`roots` describes the situation better — a root +documents *why* the code is alive, a suppression just silences the report. + +Inline comments work eslint-style, checked after the first `//` (so `///` +doc-comments work too): + +```csharp +// roe-ignore-next-line unused-type +internal class LegacyHelper { } + +public void DoWork() { } // roe-ignore-line long-method +``` + +The rule list is optional (a bare marker suppresses any kind) and +comma-separated (`// roe-ignore-next-line high-complexity,long-method`). +Dead-file and large-file findings are pinned at line 1, so their markers +work from anywhere in the file. + +Duplicate groups and circular dependencies span multiple files and have no +inline suppression — use config `ignore` globs for those. Prefer the scoped +form (`dupes.ignore`, `health.ignore`, `deadCode.ignore`) over the top-level +list when accepting one command's finding: it keeps the file covered by the +other analyses. + +## Config file + +roe looks for `roe.json`, `roe.yaml`, or `roe.yml`, walking up from the +analysis root (nearest wins), or takes `--config ` explicitly. Unknown +fields are a hard error, so typos fail loudly. + +```json +{ + "aggressive": false, + "roots": ["App.Jobs.NightlyCleanupJob"], + "libraryProjects": ["App.Sdk"], + "ignore": ["Migrations/**", "Generated/"], + "deadCode": { + "ignore": ["Plugins/**"] + }, + "dupes": { + "mode": "exact", + "minTokens": 50, + "minLines": 5, + "minOccurrences": 2, + "ignore": ["**/InsurerUnionQuery.cs"] + }, + "health": { + "maxComplexity": 10, + "maxCognitive": 15, + "maxMethodLines": 40, + "maxParameters": 5, + "maxFileLines": 750, + "maxTypeMembers": 20, + "excludeTests": true, + "baseline": ".roe-baseline.json", + "ignore": ["**/GeneratedModels.cs"] + } +} +``` + +`ignore` globs resolve relative to the config file's directory, and a +trailing `/` means the whole subtree (`Generated/` ≡ `Generated/**`). The +top-level list applies to all three analyses; each command section's own +`ignore` list is unioned with it and applies to that command alone — the +narrow tool for accepting one command's finding without silencing the +others. + +Precedence is CLI flag → config → built-in default, with gotchas: + +- `--aggressive` and `--exclude-tests` are OR'd with the config — a config + `true` cannot be turned off from the CLI. +- `--root` and `--library` **replace** the config's `roots`/`libraryProjects` + lists entirely rather than merging with them. +- The `dupes` settings are plain per-field overrides (`--mode exact` beats a + config `"semantic"`), and ignore lists have no CLI flag at all — scoped + lists only ever add to the top-level one. + +## Adopting roe on a legacy codebase + +Don't try to fix hundreds of pre-existing health findings at once. Ratchet +instead: + +```bash +roe health --write-baseline .roe-baseline.json # record current debt; always exits 0 +git add .roe-baseline.json +roe health --baseline .roe-baseline.json # from now on: only NEW debt fails +``` + +Baselines match findings by `(kind, name)` — not line numbers — so ordinary +edits don't invalidate them. Stale entries produce a warning, which is the +cue to regenerate the baseline. `--write-baseline` conflicts with +`--baseline` by design: writing through a filter would record a lie. + +## Limitations to keep in mind + +- C# only. Razor/`.cshtml` files are not parsed — build the project first so + the generated sources in `obj/` stand in for them. +- Name-based static analysis can't see string-based reflection or external + consumers; declare those with `roots` / `libraryProjects` instead of + suppressing the findings. +- Files over 5 MB are skipped.