Rust Refactor

Rust Refactoring Frameworks

This skill teaches you to look at working Rust code and see unnecessary complexity. Every refactoring starts with a diagnostic question, follows a transformation pattern, and ends with a self-review checklist.

The Two Refactoring Philosophies

Before touching code, decide which mode you are in.

Defensive mode: Add abstraction for safety. Split types. Create From bridges. Audit every consumer. Use when: security-critical code, type evolution, multi-crate dependencies.

Offensive mode: Delete indirection. Remove forwarded parameters. Collapse layers. Use when: the abstraction adds complexity without adding safety, the mediator just forwards, the parameter is always None.

Know which mode you are in. Do not add abstraction when you should be deleting, and do not delete safety layers when you should be adding them.

The 4 Diagnostic Questions

Run these BEFORE touching any code. They determine WHAT to refactor.

1. "Is this parameter just being forwarded?"

Signal: The function body only passes the parameter to another function. No local logic depends on it. Action: Remove it. Replace with ambient/global access at the point of actual use. Remove from the lowest layer first, fix compilation errors upward.

2. "Is this feature flag still needed?"

Signal: Stage is Stable or equivalent, default is enabled, no rollback planned. Action: Remove the flag. Delete the conditional. Rename gated functions (init_if_enabled -> init). Remove monitoring scaffolding.

3. "Is this file doing too many things?"

Signal: File over ~500 lines with impl blocks operating on different domains (e.g., threads AND logs AND jobs). Action: Split by domain (what it operates on), not by layer. See Module Decomposition Guide below.

4. "Does this struct have mutually exclusive optional fields?"

Signal: 2+ Option<T> fields where only one should be set at a time. Code like if a.is_some() { assert!(b.is_none()) } or fields named x_config that only apply to one mode. Action: Convert to an enum where each variant carries only its relevant data.

// BEFORE: invalid states representable
struct Auth { api_key: Option<String>, oauth_token: Option<String>, storage: Option<TokenStore> }
// AFTER: each variant carries only what it needs
enum Auth { ApiKey(String), OAuth { token: String, storage: TokenStore } }

The 6 Refactoring Transformations

Transformation 1: Forwarded Parameter -> Ambient Access

// BEFORE: metrics threaded through every signature, never used locally
pub async fn process_batch(db: &Database, config: &Config,
    metrics: Option<&MetricsClient>) {           // forwarded
    for item in db.pending_items(config).await? {
        process_item(db, item, metrics).await?;  // forwarded again
    }
}
// AFTER: ambient access at the single point of use
pub async fn process_batch(db: &Database, config: &Config) {
    for item in db.pending_items(config).await? {
        process_item(db, item).await?;
    }
}
pub async fn process_item(db: &Database, item: Item) {
    let result = transform(item)?;
    db.save(result).await?;
    if let Some(m) = metrics::global().as_ref() { m.counter("items_processed", 1); }
}

Coordination: Remove from the lowest layer first. Every intermediate commit must compile.

Transformation 2: Monolithic File -> Domain-Split Modules

// BEFORE: src/runtime.rs (950 lines, four concerns)
// AFTER:
src/runtime/mod.rs       (~30 lines: struct def, init, re-exports)
src/runtime/threads.rs   (~200 lines)
src/runtime/logs.rs      (~250 lines)
src/runtime/jobs.rs      (~300 lines)
src/runtime/cache.rs     (~120 lines)

Each file is an impl block extension of the same struct. Tests move WITH their code.

Transformation 3: Big-Bang Type Change -> Incremental From Bridge

// STEP 1: New types alongside old, with From bridges
impl From<&LegacyPolicy> for NetworkPolicy {
    fn from(value: &LegacyPolicy) -> Self {
        match value {
            LegacyPolicy::FullAccess => NetworkPolicy::Enabled,
            _ => NetworkPolicy::Restricted,
        }
    }
}
// STEP 2: Runtime carries both representations simultaneously
pub struct Permissions {
    pub legacy: LegacyPolicy,         // existing consumers keep working
    pub network_policy: NetworkPolicy, // new consumers use richer types
}

Stacked changes -- each step is a separate, independently compilable commit:

1. Add new types and From bridges
2. Plumb new types through runtime alongside old
3. Migrate consumers one at a time (one commit per subsystem)
4. Remove legacy type only after ALL consumers migrated

Transformation 4: Dead Feature Flag -> Clean Removal

// BEFORE: init_if_enabled(config) -> Option<Handle>
// AFTER:  init(config) -> Handle  (unconditional, renamed)

Full cleanup sequence:

1. Remove flag check from all call sites
2. Rename gated functions: init_if_enabled -> init
3. Simplify return types: Option<Handle> -> Handle
4. Move the flag to Stage::Removed in the features registry
5. Remove conditional branches entirely (do not just change the default)
6. Check #[cfg(feature = "...")] in Cargo.toml -- make those deps unconditional
7. Remove comparison/discrepancy metrics that tracked old vs new path
8. Delete the feature flag definition

Order: ship -> stabilize -> clean structure -> optimize -> remove flag -> remove scaffolding.