Skip to content

feat(cli,format): --suggest-ignores flag (#84) - #218

Merged
aram-devdocs merged 7 commits into
mainfrom
claude/issue-84-suggest-ignores
May 6, 2026
Merged

feat(cli,format): --suggest-ignores flag (#84)#218
aram-devdocs merged 7 commits into
mainfrom
claude/issue-84-suggest-ignores

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • Adds plumb lint --suggest-ignores, a default-off flag that appends a suggested .plumbignore block to the rendered output.
  • Pretty format appends a footer listing one line per (rule_id, selector) tuple. JSON format adds a suggested_ignores array to the envelope. SARIF intentionally ignores the flag.
  • Output is sorted by (rule_id, selector) and the JSON run_id is unchanged, so determinism holds whether the flag is on or off.

Closes #84.

Test plan

  • cargo test --workspace --all-features (4 new CLI integration tests + 7 new plumb-format unit tests, all green)
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • just determinism-check (3 runs of lint --format json byte-identical)
  • Manual: lint --format json --suggest-ignores 3-run byte-diff also identical
  • cargo deny check
  • mdbook build + cargo doc --workspace --no-deps
  • just validate

🤖 Generated with Claude Code

Add `plumb lint --suggest-ignores` so a noisy first run on an existing
codebase produces a ready-made `.plumbignore` starter. Default off.

Pretty format appends a footer block listing one entry per
`(rule_id, selector)` tuple, sorted by `(rule_id, selector)` so two
runs produce byte-identical output. JSON format adds a
`suggested_ignores` array to the existing envelope; the `run_id`
hashes only the violations payload so toggling the flag does not
shift it. SARIF intentionally ignores the flag — Code Scanning
consumers parse the strict 2.1.0 schema.

Closes #84
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

I have enough context for a complete review. Here's the full analysis:


PR Review: feat(cli,format): --suggest-ignores flag (#84)

File buckets touched

  • plumb-format — new public API surface (formatters)
  • plumb-cli — flag wiring, render helper, integration tests
  • docs — new cli/suggest-ignores.md page

1. Determinism

suggested_ignores() — deduplicates via BTreeSet<(String, String)> then converts to Vec. Sorted by (rule_id, selector). Correct. ✓

suggested_ignores_json() — builds each object by inserting rule_id before selector (alphabetical order). The comment at plumb-format/src/lib.rs:155 already establishes the pattern: alphabetical insertion makes the output stable whether or not serde_json's preserve_order feature is active. The new function follows that invariant correctly. ✓

json_with_suggested_ignores() — inserts envelope keys in order: plumb_version, run_id, stats, suggested_ignores, summary, violations. Alphabetically: p < r < st < su-g < su-m < v. ✓

run_id independence — the run digest is computed from canonical_sorted(&sorted), which is independent of suggest_ignores. Toggling the flag leaves the digest unchanged, as the doc promises. ✓

No determinism violations found.


2. Workspace layering

  • Changes are in plumb-format (depends only on plumb-core) and plumb-cli. No new inter-crate dependencies. ✓
  • #![forbid(unsafe_code)] respected. No unsafe added. ✓
  • No println!/eprintln! in plumb-format. ✓

3. Error handling

  • suggested_ignores() and pretty_with_suggested_ignores() are infallible; both carry #[must_use]. ✓
  • json_with_suggested_ignores() returns Result<String, serde_json::Error> and has # Errors doc. ✓
  • let _ = writeln!(out, ...) in append_suggested_ignores_block — writing to a String is infallible; the discard is correct. ✓
  • No unwrap/expect anywhere in library code. ✓

4. Test coverage

Unit tests in plumb-format:

  • Dedup across viewports ✓
  • Sort order ✓
  • Empty slice ✓
  • Pretty footer content ✓
  • JSON array sorted ✓
  • Byte determinism ✓

CLI integration tests:

  • --suggest-ignores absent → footer absent ✓
  • Pretty format footer content + exit code ✓
  • JSON array present + exit code ✓
  • JSON without flag → array absent ✓

No golden snapshot (insta) test, but this feature is an output-shape extension, not a rule-engine path, so unit + integration coverage is appropriate. ✓


5. Documentation

docs/src/cli/suggest-ignores.md — clear, no AI-tell phrases. Examples are concrete. SARIF non-support is documented with a rationale. ✓

docs/src/cli.md table entry updated. SUMMARY.md updated. ✓


Punch list

crates/plumb-format/src/lib.rs:252 — warning (UX, not correctness):
append_suggested_ignores_block uses violations.len() as the "would suppress N violations" count, but pairs (the actual entries) may be shorter when the same (rule_id, selector) fires on multiple viewports. A user seeing "would suppress 4 violations" but only 2 entries in the block will be confused. The docs do not explain this. Consider either: (a) using pairs.len() and rewording to "would suppress violations from N selector(s)", or (b) adding a parenthetical in the docs that clarifies one entry suppresses all viewports. Current wording is technically accurate but not self-evident.

crates/plumb-cli/src/main.rs:148 — nit:
#[arg(long, default_value_t = false)]default_value_t = false is redundant for a bool field (clap defaults bools to false). Harmless, but adds noise.

crates/plumb-cli/src/commands/watch.rs:327 — observation (not an issue):
clone_lint_args now copies suggest_ignores, but the Command::Watch arm in main.rs hardcodes suggest_ignores: false. The forwarding is future-proof but watch doesn't document that --suggest-ignores is intentionally absent. No change needed, just noting for awareness.


Verdict: APPROVE

aram-devdocs and others added 4 commits May 6, 2026 02:16
…st-ignores

# Conflicts:
#	crates/plumb-cli/tests/cli_integration.rs
…st-ignores

# Conflicts:
#	crates/plumb-cli/src/commands/lint.rs
#	crates/plumb-cli/src/main.rs
…s 1:1)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…st-ignores

# Conflicts:
#	crates/plumb-cli/src/commands/watch.rs
#	crates/plumb-cli/src/main.rs
@aram-devdocs
aram-devdocs merged commit e6e0ee8 into main May 6, 2026
16 checks passed
@aram-devdocs
aram-devdocs deleted the claude/issue-84-suggest-ignores branch May 6, 2026 12:22
@aram-devdocs aram-devdocs mentioned this pull request May 6, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): plumb lint --suggest-ignores

1 participant