rust-refactor

Files (3)

• AGENTS.md
• SKILL.md
• metadata.json

SKILL.md (first lines)

# 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.

```rust
// 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

```rust
// 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

```rust
// 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

From pproenca/dot-skills, MIT.

More from pproenca/dot-skills