refactor: resolve all deferred tech debt from PRs #106 and #107#108
Merged
refactor: resolve all deferred tech debt from PRs #106 and #107#108
Conversation
…int) Add `skim lint` subcommand with four linter parsers following the three-tier degradation pattern (Full JSON, Degraded regex, Passthrough). - LintResult/LintIssue/LintGroup canonical types in output/canonical.rs - CommandType::Lint variant in analytics - eslint parser: --format json tier 1, default formatter regex tier 2 - ruff parser: --output-format json tier 1, text regex tier 2 - mypy parser: --output json NDJSON tier 1, text regex tier 2 - golangci-lint parser: --out-format json tier 1, text regex tier 2 - --json flag for structured JSON output on all linters - 9 rewrite rules (eslint, npx eslint, ruff, ruff check, mypy, python -m mypy, python3 -m mypy, golangci-lint, golangci-lint run) - E2E tests for all tiers, --json flag, help, unknown linter - Rewrite E2E tests for all 9 lint rules Co-Authored-By: Claude <noreply@anthropic.com>
…audit) Add `skim pkg` subcommand with four package manager parsers following the three-tier degradation pattern (JSON -> regex -> passthrough): - npm: install, audit, outdated, ls (JSON + regex tiers) - pnpm: install (regex), audit (JSON), outdated (JSON) - pip: install (regex), check (regex), list --outdated (JSON) - cargo: audit (JSON + regex tiers) Also adds: - PkgResult/PkgOperation canonical types in output/canonical.rs - CommandType::Pkg variant in analytics - 17 rewrite rules for npm/pnpm/pip/pip3/cargo audit - 22 E2E pkg parser tests + 18 E2E rewrite tests - 41 unit tests across all four parsers - 15 fixture files with realistic tool output Co-Authored-By: Claude <noreply@anthropic.com>
…loops, tighten assertions
- Extract `run_lint_json_mode` in lint/mod.rs to replace 4 duplicated
`run_json_mode` functions (~320 lines -> single shared helper)
- Add 64 MiB stdin limit via `.take()` in shared helper, consistent
with `run_parsed_command_with_mode`
- Preserve child process exit code instead of always returning SUCCESS
- Replace `?` operators in try_parse_json loops (eslint, ruff, golangci)
with `let-else { continue }` to avoid discarding previously parsed issues
- Fix rule count comment in rewrite.rs (26 -> 24)
Co-Authored-By: Claude <noreply@anthropic.com>
- eslint Tier 2 test: replace weak > 0 check with exact assert_eq counts - golangci Tier 1 test: replace sum assertion with individual error/warning counts - golangci E2E test: add count assertions for 1 error and 3 warnings - Add unit tests for group_issues (Info severity, empty input) - Add unit tests for extract_json_flag (present and absent) - Add E2E test for --show-stats with lint subcommand Co-Authored-By: Claude <noreply@anthropic.com>
- Add 34 unit tests for all 18 Pkg rewrite rules (cargo audit, npm audit/install/i/ci/outdated/list/ls, pnpm audit/install/i/outdated, pip install/check/list, pip3 install/check/list) including skip-flag and category assertions - Add two missing rewrite aliases: npm list -> skim pkg npm ls (skip: --json) and pnpm i -> skim pkg pnpm install (skip: none) - Remove unnecessary .cloned() deep copy in cargo audit JSON parser; use borrowed reference with empty vec fallback instead - Refactor combine_output into shared Cow-based helper in pkg/mod.rs to eliminate duplication across cargo/npm/pip/pnpm parsers Co-Authored-By: Claude <noreply@anthropic.com>
…ip args, fix vuln counts - Remove dead-code guards in pip and pnpm run() — replace unreachable let-else with .expect() since args.is_empty() is already checked - Apply cargo fmt to npm.rs via-array handling block Co-Authored-By: Claude <noreply@anthropic.com>
) Split npm.rs (787 lines) into npm/ directory with 5 files: - mod.rs (67 lines): dispatcher + re-exports - install.rs (249 lines): npm install parser + tests - audit.rs (270 lines): npm audit parser + tests - outdated.rs (165 lines): npm outdated parser + tests - ls.rs (156 lines): npm ls parser + tests Extract extract_vuln_detail() helper from cargo.rs try_parse_audit_json, reducing it from 91 to ~69 lines. Pure structural refactor — zero behavioral changes. All existing tests pass.
Filenames already communicate purpose after the split — section headers like "// npm install" are noise in dedicated files.
The function always returns Some since all fields have unwrap_or fallbacks. Updated doc to accurately reflect this behavior.
Replace truncating `as u32` casts with `u32::try_from(x).unwrap_or(u32::MAX)` in all four lint parsers (eslint, golangci, mypy, ruff) to prevent silent data loss on line numbers exceeding u32::MAX. Co-Authored-By: Claude <noreply@anthropic.com>
Add tests for extract_json_flag (flag present/absent) and combine_output (borrowed/owned paths) in pkg/mod.rs. Add three-tier integration tests (json->Full, garbage->Passthrough) for npm outdated and npm ls parsers, matching install.rs pattern. Co-Authored-By: Claude <noreply@anthropic.com>
- Change extract_vuln_detail return from Option<(String, &str)> to plain (String, &str) since all fields have unwrap_or fallbacks and the function never returns None - Replace `as usize` casts in npm install parser with usize::try_from().ok() to avoid silent truncation on 32-bit targets - Include co-located batch fixes: sanitize user input in error messages, use CARGO_TERM_COLOR env var, add three-tier integration tests Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unnecessary String allocation in LintResult::render by using direct write! calls with inline pluralization instead of format!() - Add test_parse_impl_text_produces_degraded to ruff, mypy, and golangci modules, matching the existing eslint pattern - Add E2E test for `skim lint eslint --json` with degraded tier input - Document golangci Tier 2 regex severity-defaults-to-Warning as a known limitation (text format lacks severity information) - Fix Cow<str> type mismatch in parse_impl Passthrough arms after combine_stdout_stderr was refactored to return Cow - Add stderr tier markers to run_lint_json_mode for observability Co-Authored-By: Claude <noreply@anthropic.com>
Collapse stderr lock/emit/drop, inline single-use variables, deduplicate render write calls, optimize group_issues ownership.
Deduplicate three identical u64-to-usize conversion chains into a single helper function.
…n_linter helper - Add OutputFormat enum (Text/Json) and output_format field to ParsedCommandConfig - Add to_json_envelope() on ParseResult<T: Serialize> for JSON serialization - Widen T bound to AsRef<str> + Serialize in run_parsed_command_with_mode - Add JSON output branch in run_parsed_command_with_mode - Extract LinterConfig + run_linter helper in lint/mod.rs - Migrate all 4 linters (eslint, ruff, mypy, golangci) to use run_linter - Delete run_lint_json_mode + LintJsonConfig (dead code) Resolves tech debt items 1-3 from PR #106 review. Co-Authored-By: Claude <noreply@anthropic.com>
…parser - Add PkgSubcommandConfig + run_pkg_subcommand helper in pkg/mod.rs - Migrate all 10 run_* functions to use the shared helper - Rewrite try_parse_audit_regex from triple-regex to block-based parsing (handles missing fields and reordered fields correctly) - Remove dead regex statics (RE_CRATE, RE_TITLE, RE_ADVISORY_ID) - Add failing tests for missing-field and reordered-field cases (TDD) Resolves tech debt items 5-6 from PR #107 review. Co-Authored-By: Claude <noreply@anthropic.com>
- De-duplicate stats/analytics logic in run_parsed_command_with_mode by extracting compressed content from the output_format match arms - Replace fully-qualified crate::cmd:: paths with proper use imports in lint/mod.rs and test/cargo.rs for consistency
…ith tech debt fixes Merges feat/104-lint-parsers and feat/105-pkg-parsers into a single consolidation branch. Resolves merge conflicts (additive — both PRs add independent modules, types, and tests to shared files). Post-merge fixes: - Add output_format field to pkg's ParsedCommandConfig construction - Add Serialize bound to run_pkg_subcommand generic parameter - Reconstruct mangled struct/impl/test boundaries in canonical.rs Tech debt resolved (items 1-3 from PR #106): - Unified lint JSON mode into shared run_parsed_command_with_mode - Extracted run_linter helper, migrated all 4 linters - Deleted run_lint_json_mode + LintJsonConfig Tech debt resolved (items 5-6 from PR #107): - Extracted run_pkg_subcommand helper, migrated all 10 functions - Block-based cargo audit parser replaces fragile triple-regex - Removed dead regex statics
This was referenced Mar 29, 2026
13 tasks
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
run_parsed_command_with_modeviaOutputFormatenum. Extractedrun_linterhelper. Migrated all 4 linters (eslint, ruff, mypy, golangci). Deletedrun_lint_json_mode+LintJsonConfig.run_pkg_subcommandhelper. Migrated all 10run_*functions across npm, pnpm, pip, cargo.Net reduction: ~150 lines of duplicated infrastructure eliminated.
Test plan
cargo test— all tests pass (913+ unit, 45+ integration, 11 doc tests)cargo clippy -- -D warnings— cleancargo fmt -- --check— clean