Skip to content

Refactor to reduce CRAP #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aleyan merged 8 commits into from
May 20, 2026
Merged

Refactor to reduce CRAP #164

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ab7ea9
Initial refactor
aleyan May 20, 2026
2693937
fix noise
aleyan May 20, 2026
7bf3218
a little less crap
aleyan May 20, 2026
eeb6124
lints
aleyan May 20, 2026
9430061
refactor: encapsulate DiscoveredTaskDefinitions fields and add explic…
aleyan May 20, 2026
f5bd76c
baseline
aleyan May 20, 2026
a6f5adf
fixups
aleyan May 20, 2026
bb44624
fix
aleyan May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 51 additions & 35 deletions cargo_crap_baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,10 @@
{
"file": "./src/commands/list.rs",
"function": "execute",
"line": 20,
"cyclomatic": 72.0,
"coverage": 0.0,
"crap": 5256.0
},
{
"file": "./src/commands/mcp.rs",
"function": "execute",
"line": 243,
"cyclomatic": 19.0,
"line": 22,
"cyclomatic": 43.0,
"coverage": 0.0,
"crap": 380.0
"crap": 1892.0
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

High CRAP score remains after refactoring.

The execute function in ./src/commands/list.rs still has a CRAP score of 1892.0 with cyclomatic complexity of 43. While the PR refactored verbose reporting to reduce duplication, the core execute function remains highly complex with zero test coverage.

Consider breaking down the execute function into smaller helper functions and adding unit tests to reduce this risk metric below the typical threshold of 30.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cargo_crap_baseline.json` around lines 8 - 11, The execute function in
./src/commands/list.rs has excessive cyclomatic complexity and zero test
coverage; refactor it by extracting smaller, well-named helper functions (e.g.,
parse_list_args, collect_packages, filter_packages, compute_package_metrics, and
format_list_output) to move independent match arms, loops, and conditional logic
out of execute, reduce branching inside execute to high-level orchestration, and
then add unit tests for each helper plus focused integration tests for execute
(using mocks/stubs for IO and external dependencies) to cover all branches and
error paths so the overall cyclomatic and CRAP metrics fall below the threshold.

},
{
"file": "./src/mcp/job_manager.rs",
Expand All @@ -26,6 +18,14 @@
"coverage": 24.691358024691358,
"crap": 388.19513360843445
},
{
"file": "./src/commands/mcp.rs",
"function": "execute",
"line": 243,
"cyclomatic": 19.0,
"coverage": 0.0,
"crap": 380.0
},
{
"file": "./src/mcp/server.rs",
"function": "DelaMcpServer::call_tool",
Expand Down Expand Up @@ -109,7 +109,7 @@
{
"file": "./src/types.rs",
"function": "TaskRunner::get_command",
"line": 199,
"line": 192,
"cyclomatic": 21.0,
"coverage": 100.0,
"crap": 21.0
Expand Down Expand Up @@ -165,7 +165,7 @@
{
"file": "./src/types.rs",
"function": "TaskRunner::short_name",
"line": 242,
"line": 235,
"cyclomatic": 19.0,
"coverage": 95.23809523809523,
"crap": 19.038980671633734
Expand Down Expand Up @@ -221,7 +221,7 @@
{
"file": "./src/commands/list.rs",
"function": "format_task_entry",
"line": 356,
"line": 231,
"cyclomatic": 15.0,
"coverage": 83.33333333333334,
"crap": 16.041666666666664
Expand All @@ -234,14 +234,6 @@
"coverage": 92.3076923076923,
"crap": 15.10241238051889
},
{
"file": "./src/task_discovery/support.rs",
"function": "set_definition",
"line": 12,
"cyclomatic": 14.0,
"coverage": 87.5,
"crap": 14.3828125
},
{
"file": "./src/allowlist.rs",
"function": "evaluate_task_against_allowlist",
Expand Down Expand Up @@ -925,7 +917,7 @@
{
"file": "./src/commands/list.rs",
"function": "format_task_entry_with_source",
"line": 445,
"line": 320,
"cyclomatic": 3.0,
"coverage": 87.5,
"crap": 3.017578125
Expand Down Expand Up @@ -1077,7 +1069,7 @@
{
"file": "./src/commands/list.rs",
"function": "task_source_label",
"line": 458,
"line": 333,
"cyclomatic": 3.0,
"coverage": 100.0,
"crap": 3.0
Expand Down Expand Up @@ -1125,7 +1117,7 @@
{
"file": "./src/commands/list.rs",
"function": "format_definition_path_for_display",
"line": 472,
"line": 347,
"cyclomatic": 2.0,
"coverage": 80.0,
"crap": 2.032
Expand All @@ -1141,7 +1133,7 @@
{
"file": "./src/commands/list.rs",
"function": "format_runner_path_for_display",
"line": 480,
"line": 355,
"cyclomatic": 2.0,
"coverage": 85.71428571428571,
"crap": 2.011661807580175
Expand Down Expand Up @@ -1205,7 +1197,7 @@
{
"file": "./src/types.rs",
"function": "deserialize_path",
"line": 315,
"line": 298,
"cyclomatic": 2.0,
"coverage": 100.0,
"crap": 2.0
Expand Down Expand Up @@ -1605,7 +1597,7 @@
{
"file": "./src/task_discovery.rs",
"function": "discover_tasks",
"line": 67,
"line": 51,
"cyclomatic": 2.0,
"coverage": 100.0,
"crap": 2.0
Expand Down Expand Up @@ -1690,26 +1682,42 @@
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/types.rs",
"function": "DiscoveredTaskDefinitions::insert",
"line": 139,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/types.rs",
"function": "DiscoveredTaskDefinitions::get",
"line": 146,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/types.rs",
"function": "Task::definition_path",
"line": 187,
"line": 180,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/types.rs",
"function": "Task::allowlist_path",
"line": 192,
"line": 185,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/types.rs",
"function": "serialize_path",
"line": 308,
"line": 291,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
Expand Down Expand Up @@ -1794,18 +1802,26 @@
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/task_discovery/support.rs",
"function": "set_definition",
"line": 12,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/task_discovery/support.rs",
"function": "handle_discovery_error",
"line": 36,
"line": 16,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/task_discovery/support.rs",
"function": "handle_discovery_success",
"line": 57,
"line": 37,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
Expand Down Expand Up @@ -2309,15 +2325,15 @@
{
"file": "./src/task_discovery.rs",
"function": "DiscoveredTasks::new",
"line": 52,
"line": 36,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
},
{
"file": "./src/task_discovery.rs",
"function": "DiscoveredTasks::add_task",
"line": 57,
"line": 41,
"cyclomatic": 1.0,
"coverage": 100.0,
"crap": 1.0
Expand Down
6 changes: 3 additions & 3 deletions dev_docs/project_plan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,10 @@ Advanced MCP features for better editor integration and real-time feedback.

## Technical Debt & Code Cleanup (Phase 11)

- [ ] **[DTKT-202]** Refactor `DiscoveredTaskDefinitions` to use a grouped map collection (`BTreeMap<TaskDefinitionType, Vec<TaskDefinitionFile>>`) instead of hardcoded fields. This preserves predictable output ordering while robustly capturing multiple definitions per parser (e.g., included Makefiles or workspace-local `turbo.json` configs).
- [ ] **[DTKT-203]** Migrate all dependent discovery and output layers to iterate dynamically over the new collection, allowing new parsers (e.g., TravisCi, CMake, Justfile) to correctly store their statuses.
- [x] **[DTKT-202]** Refactor `DiscoveredTaskDefinitions` to use a grouped map collection (`BTreeMap<TaskDefinitionType, Vec<TaskDefinitionFile>>`) instead of hardcoded fields. This preserves predictable output ordering while robustly capturing multiple definitions per parser (e.g., included Makefiles or workspace-local `turbo.json` configs).
- [x] **[DTKT-203]** Migrate all dependent discovery and output layers to iterate dynamically over the new collection, allowing new parsers (e.g., TravisCi, CMake, Justfile) to correctly store their statuses.

- [ ] **[DTKT-204]** Consolidate the 100+ lines of duplicated `TaskFileStatus` match formatting blocks in `src/commands/list.rs` into a single loop mapping over the refactored `DiscoveredTaskDefinitions`.
- [x] **[DTKT-204]** Consolidate the 100+ lines of duplicated `TaskFileStatus` match formatting blocks in `src/commands/list.rs` into a single loop mapping over the refactored `DiscoveredTaskDefinitions`.

- [ ] **[DTKT-205]** Deprecate the hacky `test_println!` macro and abstract CLI formatting: Inject a `&mut dyn std::io::Write` interface (or an abstract output buffer) across commands.
- [ ] **[DTKT-206]** Update tests in `list.rs` to pass a mock buffer and write explicit assertions against the output strings instead of discarding stdout.
Expand Down
Loading
Loading