refactor: audit hygiene — shared constants, domain naming, idiom sweep - #95
Merged
Conversation
…etection The git-config override list was duplicated between --staged and --git-history, so adding the next override meant remembering both call sites (#88 had to add diff.relative twice). GIT_DIFF_FRAMING_ARGS is now one constant with the parser-dependency rationale attached. The (?s heuristic likewise lived in run_scan and seven test partitions — and the tests still carried the pre-#92 form that misses (?s:...). Detector::is_multiline() is now the single definition.
…esolution - detectors.toml was a literal at three discovery sites plus two error messages; DETECTORS_FILE_NAME is now the single spelling. - hook uninstall trusts the '# Installed by KeyWatch' marker, which the templates carried as unpinned text; a test now fails if a template loses it. - redact()'s prefix length (4) and short-match threshold (8) are named constants instead of magic numbers. - config discovery resolved HOME/USERPROFILE inline; utils::home_dir() (which hooks and display_path already rely on) is now the one implementation. Hooks keeps its own XDG/APPDATA-first order.
- ScanArgs' Default is derived now: ExitMode::Strict and OutputFormat::Json carry #[default], so a new flag no longer means editing a 25-line hand-written impl. - collect_files handed back anonymous (String, Option<String>) tuples whose second slot was 'the scan root'; a small ScanTarget struct names it. - scan_lines took a bare binary_detection: bool; a BinaryHandling enum makes both call sites self-describing. - CustomRule::description was parsed and never read (#[allow(dead_code)]); dropped. Configs carrying it keep parsing — serde ignores unknown keys. - utils.rs and report.rs had use statements below their first code blocks; all imports now sit at the top.
Finding.plugin_name stored a detector's name, but the domain says 'detector' everywhere else — 'plugin' survived in one field and the baseline schema. The in-code field is now detector_name; the JSON wire format keeps the historical plugin_name key (serde rename + alias) so report consumers are unaffected, and BaselineEntry keeps the versioned on-disk name. The staged parser's undiffable_files / scan_undiffable_blobs became unscannable_from_diff / scan_index_blobs: 'undiffable' is the cause (git cannot render the file), 'unscannable' is the outcome the report talks about — a distinction the two previous names blurred.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands all twelve findings from the constants/idiom/domain audit, one commit per group:
--stagedand--git-historyis oneGIT_DIFF_FRAMING_ARGSconstant (a new override previously had to be added to both call sites — fix(scan): make git-history and staged modes behave like the others #88 did exactly that); the(?smultiline heuristic is nowDetector::is_multiline()— production and the seven test partitions shared two divergent definitions, and the tests still carried the pre-fix: baseline integrity, clearer CLI errors, deterministic SARIF #92 form.detectors.tomlisDETECTORS_FILE_NAMEacross discovery and error messages; the hook-uninstall marker is pinned by a test (a template header edit would have silently brokenhook uninstall);redact()'s 4-char prefix and 8-char threshold are named constants; config discovery usesutils::home_dir()instead of a third inline HOME/USERPROFILE fallback.ScanArgs::defaultis derived (#[default]onExitMode::Strict/OutputFormat::Json);collect_filesreturns namedScanTargets instead of anonymous tuples;scan_linestakes aBinaryHandlingenum instead of a bare bool;CustomRule::description(parsed, never read) is dropped — configs carrying it keep parsing; stray mid-file imports moved to the top.Finding.plugin_name→detector_namein code — the JSON wire format still emits and acceptsplugin_name(serde rename + alias), so report consumers are unaffected;BaselineEntrykeeps the versioned on-disk name.undiffable_files/scan_undiffable_blobs→unscannable_from_diff/scan_index_blobs, with comments keeping the cause/outcome distinction ('undiffable' = git cannot render it; 'unscannable' = we could not read it).Tests
214 pass (one new: templates must carry the uninstall marker). fmt/clippy clean; self-scan clean and drift-free.