Skip to content

Refactor src/stdlib/command.rs to resolve low cohesion #233

@coderabbitai

Description

@coderabbitai

Context

Related PR: #231
Requested by: @leynos

The current src/stdlib/command.rs file has grown to 1,022 lines with 47 functions spanning at least 9 different responsibilities, exceeding the cohesion threshold of 3. This issue tracks the refactoring work to split the module into focused sub-modules.

Current State

  • File: src/stdlib/command.rs (1,022 lines, 47 functions, 9 responsibilities)
  • Issue: Low cohesion detected by CodeScene analysis

Target

  • Split into 8 sub-modules + coordinator under src/stdlib/command/
  • Preserve all existing functionality, tests, and platform-specific behaviour
  • Maintain existing public API surface via re-exports in mod.rs

Implementation Steps

1. Create Directory Structure

Create directory src/stdlib/command/ and move src/stdlib/command.rs to src/stdlib/command/mod_backup.rs

2. Create src/stdlib/command/config.rs

  • Move: CommandConfig, OutputMode, OutputStream, PipeSpec, PipeLimit, CommandOptions (struct and impl blocks)
  • Add derives/visibility as needed
  • Keep all methods and Default impl for CommandOptions

3. Create src/stdlib/command/error.rs

  • Move: CommandFailure enum with all variants and From<io::Error> impl
  • Move: ExitDetails, LimitExceeded structs with impl blocks
  • Move: All error constructor functions (spawn_error, io_error, broken_pipe_error, exit_error, timeout_error, output_limit_error, stream_path_error, command_error)
  • Move: append_exit_status, append_stderr helper functions
  • Import necessary types from super::config and super::context

4. Create src/stdlib/command/context.rs

  • Move: CommandContext, GrepCall, CommandLocation structs with all impl blocks
  • Import necessary types from super::config

5. Create src/stdlib/command/result.rs

  • Move: StdoutResult, PipeOutcome enums

6. Create src/stdlib/command/execution.rs

  • Move: run_command, run_program (cfg-gated), run_child, wait_for_exit
  • Move: SHELL, SHELL_ARGS, COMMAND_TIMEOUT constants
  • Import from super::{config, error, result, pipes, quote}

7. Create src/stdlib/command/pipes.rs

  • Move: spawn_pipe_reader, join_reader, read_pipe, create_empty_tempfile
  • Move: cleanup_readers, join_pipe_for_cleanup, handle_stdin_result
  • Import from super::{config, error, result}

8. Create src/stdlib/command/filters.rs

  • Move: execute_shell, execute_grep
  • Move: collect_flag_args, format_command, to_bytes
  • Import from super::{config, context, error, result, execution, quote}

9. Create src/stdlib/command/quote.rs

  • Move: quote function (both Windows and non-Windows versions with cfg gates)
  • Move: QuoteError enum with Display impl
  • Move: The quote_escapes_cmd_metacharacters test under #[cfg(test)] mod tests
  • Add necessary imports (fmt, shell_quote for non-Windows)

10. Create src/stdlib/command/mod.rs

  • Add module documentation (move from original file header)
  • Declare sub-modules: mod config; mod error; mod context; mod result; mod execution; mod pipes; mod filters; mod quote;
  • Add necessary imports (Arc, AtomicBool, minijinja types, etc.)
  • Move: register() function (import needed types from sub-modules)
  • Add pub(crate) use statements to re-export types used by parent stdlib module
  • Import value_from_bytes from super::value_from_bytes

11. Update Imports

  • Ensure each sub-module imports only what it needs from siblings via super::module_name
  • Use pub(crate) or pub(super) visibility appropriately
  • Maintain platform-specific cfg gates for Windows/non-Windows code

12. Verification

  • Run: make check-fmt
  • Run: make lint
  • Run: make test
  • Ensure all tests pass and no functionality changed

Important Notes

  • Keep platform-specific cfg attributes with their respective code
  • Preserve all comments and documentation
  • Use pub(crate) for items used outside the command module, pub(super) for items used between sub-modules
  • The public API surface should remain identical via re-exports in mod.rs
  • Each sub-module should have a single, clear responsibility, reducing the cohesion metric from 9 to 1–2 per module

Expected Outcome

After refactoring:

  • Each sub-module focuses on a single responsibility
  • Cohesion metric improves from 9 to 1–2 per module
  • All existing tests pass
  • Public API remains unchanged
  • Code is more maintainable and easier to navigate

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions