125 lines
4.5 KiB
Markdown
125 lines
4.5 KiB
Markdown
---
|
|
name: cleanup
|
|
description: Use when the user asks to clean up, lint, or review uncommitted code for common code smells — duplicate logic, magic numbers, unclear naming, dead code, style inconsistencies. Fixes issues without changing any runtime behavior.
|
|
---
|
|
|
|
# Cleanup
|
|
|
|
Review and fix code quality issues in the current working tree without altering any logic or behavior.
|
|
|
|
## When To Use
|
|
|
|
- The user asks to clean up, tidy, or lint uncommitted changes
|
|
- The user wants a code smell review before releasing or committing
|
|
- The user mentions magic numbers, duplicate logic, dead code, or naming issues
|
|
|
|
Do not refactor architecture, add features, or change behavior.
|
|
|
|
## Scope
|
|
|
|
If the user specifies a file or directory, check only that. Otherwise check all uncommitted changes (`git diff HEAD`).
|
|
|
|
Only report issues present in **newly added or modified** lines of this diff — do not audit unchanged code.
|
|
|
|
## Checklist
|
|
|
|
### 1. Duplicate Logic
|
|
- Identical or near-identical code blocks appearing in multiple places
|
|
- A function/helper that already exists but is re-implemented elsewhere instead of being reused
|
|
- Repeated DOM queries, regex literals, or template strings within the same file
|
|
|
|
### 2. Magic Numbers / Magic Strings
|
|
- Bare numeric literals used in calculations (offsets, timeouts, sizes, thresholds) without a named constant
|
|
- Hardcoded strings (IDs, status values, URL fragments) scattered through logic
|
|
- Exceptions: `0`, `1`, `-1`, `100`, `""` and other idiomatically clear values are fine
|
|
|
|
### 3. Naming Issues
|
|
- Cryptic abbreviations (`or_`, `tmp2`, `x2`)
|
|
- Names that do not match actual behavior
|
|
- The same concept referred to by different names in different places
|
|
|
|
### 4. Dead Code
|
|
- Commented-out code blocks (3+ lines)
|
|
- Variables, parameters, or imports declared but never used
|
|
- Branches that can never execute
|
|
|
|
### 5. Style Inconsistencies
|
|
- Trailing whitespace
|
|
- Mixed quote styles or indentation within the same file
|
|
- Inconsistent blank-line usage (multiple consecutive blank lines, etc.)
|
|
|
|
### 6. Other
|
|
- Private helper functions that should be exported but are not, causing callers to duplicate the implementation
|
|
- Overly verbose conditions that can be simplified without changing logic
|
|
|
|
## Steps
|
|
|
|
### Step 1 — Get the file list
|
|
|
|
```bash
|
|
git diff HEAD --name-only
|
|
```
|
|
|
|
Filter to the user-specified path if one was provided.
|
|
|
|
### Step 2 — Read and analyze each file
|
|
|
|
Read the full file (not just the diff) with the Read tool. For each file, record every issue found: filename, line number, category, and suggested fix.
|
|
|
|
### Step 3 — Report findings before touching anything
|
|
|
|
Print a structured list:
|
|
|
|
```
|
|
Found N issues:
|
|
|
|
[file] js/foo.js
|
|
· L34, L78: Duplicate logic — same DOM query implemented twice; extract to getPanel()
|
|
· L91: Magic number — bare 14 used as pixel offset; name it TOOLTIP_OFFSET
|
|
|
|
[file] js/bar.js
|
|
· L12: Naming — variable `or_` is unclear; rename to outerR, outerG, outerB
|
|
...
|
|
```
|
|
|
|
If no issues are found, output "No code smells detected. Code quality looks good." and stop.
|
|
|
|
### Step 4 — Fix each issue
|
|
|
|
Use the Edit tool for **minimal, targeted changes**:
|
|
|
|
- **Duplicate logic**: extract to a shared constant or function; update all call sites
|
|
- **Magic number/string**: declare `const NAME = value` near the top of the relevant scope; replace all usages
|
|
- **Naming**: rename the variable/function; update all references
|
|
- **Dead code**: delete it
|
|
- **Trailing whitespace / style**: fix in place
|
|
- **Unexported helper**: add `export`; update callers to import instead of re-implementing
|
|
|
|
Principles:
|
|
- Only fix issues identified in the checklist — no extra improvements
|
|
- Keep each Edit as small as possible
|
|
- After fixing, verify the old bad pattern is gone with grep
|
|
|
|
### Step 5 — Summary
|
|
|
|
```
|
|
Cleanup complete:
|
|
|
|
Fixed N issues:
|
|
✓ earth.js — extracted duplicate vertexShader into ATMOS_VERTEX_SHADER constant
|
|
✓ main.js — extracted TOOLTIP_CURSOR_OFFSET = 14 (4 references updated)
|
|
✓ controls.js — exported updateLayerButtonState; removed duplicate implementation in main.js
|
|
...
|
|
|
|
Skipped (needs manual review):
|
|
! foo.js L45 — large commented-out block; confirm it is safe to delete
|
|
```
|
|
|
|
## Constraints
|
|
|
|
- **Do not** change function signatures, exported interfaces, or public APIs (unless the issue is a missing export)
|
|
- **Do not** add new features, abstractions, or parameters
|
|
- **Do not** rewrite comments (only delete commented-out dead code)
|
|
- **Do not** touch test file logic
|
|
- If a magic number's intent is uncertain, skip it and flag it in the summary
|