|
|
|
|
@@ -1,201 +1,94 @@
|
|
|
|
|
---
|
|
|
|
|
name: clean-code
|
|
|
|
|
description: Pragmatic coding standards - concise, direct, no over-engineering, no unnecessary comments
|
|
|
|
|
allowed-tools: Read, Write, Edit
|
|
|
|
|
version: 2.0
|
|
|
|
|
priority: CRITICAL
|
|
|
|
|
description: "Applies principles from Robert C. Martin's 'Clean Code'. Use this skill when writing, reviewing, or refactoring code to ensure high quality, readability, and maintainability. Covers naming, functions, comments, error handling, and class design."
|
|
|
|
|
user-invocable: true
|
|
|
|
|
risk: safe
|
|
|
|
|
source: "ClawForge (https://github.com/jackjin1997/ClawForge)"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
# Clean Code - Pragmatic AI Coding Standards
|
|
|
|
|
# Clean Code Skill
|
|
|
|
|
|
|
|
|
|
> **CRITICAL SKILL** - Be **concise, direct, and solution-focused**.
|
|
|
|
|
This skill embodies the principles of "Clean Code" by Robert C. Martin (Uncle Bob). Use it to transform "code that works" into "code that is clean."
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
## 🧠 Core Philosophy
|
|
|
|
|
> "Code is clean if it can be read, and enhanced by a developer other than its original author." — Grady Booch
|
|
|
|
|
|
|
|
|
|
## Core Principles
|
|
|
|
|
## When to Use
|
|
|
|
|
Use this skill when:
|
|
|
|
|
- **Writing new code**: To ensure high quality from the start.
|
|
|
|
|
- **Reviewing Pull Requests**: To provide constructive, principle-based feedback.
|
|
|
|
|
- **Refactoring legacy code**: To identify and remove code smells.
|
|
|
|
|
- **Improving team standards**: To align on industry-standard best practices.
|
|
|
|
|
|
|
|
|
|
| Principle | Rule |
|
|
|
|
|
|-----------|------|
|
|
|
|
|
| **SRP** | Single Responsibility - each function/class does ONE thing |
|
|
|
|
|
| **DRY** | Don't Repeat Yourself - extract duplicates, reuse |
|
|
|
|
|
| **KISS** | Keep It Simple - simplest solution that works |
|
|
|
|
|
| **YAGNI** | You Aren't Gonna Need It - don't build unused features |
|
|
|
|
|
| **Boy Scout** | Leave code cleaner than you found it |
|
|
|
|
|
## 1. Meaningful Names
|
|
|
|
|
- **Use Intention-Revealing Names**: `elapsedTimeInDays` instead of `d`.
|
|
|
|
|
- **Avoid Disinformation**: Don't use `accountList` if it's actually a `Map`.
|
|
|
|
|
- **Make Meaningful Distinctions**: Avoid `ProductData` vs `ProductInfo`.
|
|
|
|
|
- **Use Pronounceable/Searchable Names**: Avoid `genymdhms`.
|
|
|
|
|
- **Class Names**: Use nouns (`Customer`, `WikiPage`). Avoid `Manager`, `Data`.
|
|
|
|
|
- **Method Names**: Use verbs (`postPayment`, `deletePage`).
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
## 2. Functions
|
|
|
|
|
- **Small!**: Functions should be shorter than you think.
|
|
|
|
|
- **Do One Thing**: A function should do only one thing, and do it well.
|
|
|
|
|
- **One Level of Abstraction**: Don't mix high-level business logic with low-level details (like regex).
|
|
|
|
|
- **Descriptive Names**: `isPasswordValid` is better than `check`.
|
|
|
|
|
- **Arguments**: 0 is ideal, 1-2 is okay, 3+ requires a very strong justification.
|
|
|
|
|
- **No Side Effects**: Functions shouldn't secretly change global state.
|
|
|
|
|
|
|
|
|
|
## Naming Rules
|
|
|
|
|
## 3. Comments
|
|
|
|
|
- **Don't Comment Bad Code—Rewrite It**: Most comments are a sign of failure to express ourselves in code.
|
|
|
|
|
- **Explain Yourself in Code**:
|
|
|
|
|
```python
|
|
|
|
|
# Check if employee is eligible for full benefits
|
|
|
|
|
if employee.flags & HOURLY and employee.age > 65:
|
|
|
|
|
```
|
|
|
|
|
vs
|
|
|
|
|
```python
|
|
|
|
|
if employee.isEligibleForFullBenefits():
|
|
|
|
|
```
|
|
|
|
|
- **Good Comments**: Legal, Informative (regex intent), Clarification (external libraries), TODOs.
|
|
|
|
|
- **Bad Comments**: Mumbling, Redundant, Misleading, Mandated, Noise, Position Markers.
|
|
|
|
|
|
|
|
|
|
| Element | Convention |
|
|
|
|
|
|---------|------------|
|
|
|
|
|
| **Variables** | Reveal intent: `userCount` not `n` |
|
|
|
|
|
| **Functions** | Verb + noun: `getUserById()` not `user()` |
|
|
|
|
|
| **Booleans** | Question form: `isActive`, `hasPermission`, `canEdit` |
|
|
|
|
|
| **Constants** | SCREAMING_SNAKE: `MAX_RETRY_COUNT` |
|
|
|
|
|
## 4. Formatting
|
|
|
|
|
- **The Newspaper Metaphor**: High-level concepts at the top, details at the bottom.
|
|
|
|
|
- **Vertical Density**: Related lines should be close to each other.
|
|
|
|
|
- **Distance**: Variables should be declared near their usage.
|
|
|
|
|
- **Indentation**: Essential for structural readability.
|
|
|
|
|
|
|
|
|
|
> **Rule:** If you need a comment to explain a name, rename it.
|
|
|
|
|
## 5. Objects and Data Structures
|
|
|
|
|
- **Data Abstraction**: Hide the implementation behind interfaces.
|
|
|
|
|
- **The Law of Demeter**: A module should not know about the innards of the objects it manipulates. Avoid `a.getB().getC().doSomething()`.
|
|
|
|
|
- **Data Transfer Objects (DTO)**: Classes with public variables and no functions.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
## 6. Error Handling
|
|
|
|
|
- **Use Exceptions instead of Return Codes**: Keeps logic clean.
|
|
|
|
|
- **Write Try-Catch-Finally First**: Defines the scope of the operation.
|
|
|
|
|
- **Don't Return Null**: It forces the caller to check for null every time.
|
|
|
|
|
- **Don't Pass Null**: Leads to `NullPointerException`.
|
|
|
|
|
|
|
|
|
|
## Function Rules
|
|
|
|
|
## 7. Unit Tests
|
|
|
|
|
- **The Three Laws of TDD**:
|
|
|
|
|
1. Don't write production code until you have a failing unit test.
|
|
|
|
|
2. Don't write more of a unit test than is sufficient to fail.
|
|
|
|
|
3. Don't write more production code than is sufficient to pass the failing test.
|
|
|
|
|
- **F.I.R.S.T. Principles**: Fast, Independent, Repeatable, Self-Validating, Timely.
|
|
|
|
|
|
|
|
|
|
| Rule | Description |
|
|
|
|
|
|------|-------------|
|
|
|
|
|
| **Small** | Max 20 lines, ideally 5-10 |
|
|
|
|
|
| **One Thing** | Does one thing, does it well |
|
|
|
|
|
| **One Level** | One level of abstraction per function |
|
|
|
|
|
| **Few Args** | Max 3 arguments, prefer 0-2 |
|
|
|
|
|
| **No Side Effects** | Don't mutate inputs unexpectedly |
|
|
|
|
|
## 8. Classes
|
|
|
|
|
- **Small!**: Classes should have a single responsibility (SRP).
|
|
|
|
|
- **The Stepdown Rule**: We want the code to read like a top-down narrative.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Code Structure
|
|
|
|
|
|
|
|
|
|
| Pattern | Apply |
|
|
|
|
|
|---------|-------|
|
|
|
|
|
| **Guard Clauses** | Early returns for edge cases |
|
|
|
|
|
| **Flat > Nested** | Avoid deep nesting (max 2 levels) |
|
|
|
|
|
| **Composition** | Small functions composed together |
|
|
|
|
|
| **Colocation** | Keep related code close |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## AI Coding Style
|
|
|
|
|
|
|
|
|
|
| Situation | Action |
|
|
|
|
|
|-----------|--------|
|
|
|
|
|
| User asks for feature | Write it directly |
|
|
|
|
|
| User reports bug | Fix it, don't explain |
|
|
|
|
|
| No clear requirement | Ask, don't assume |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Anti-Patterns (DON'T)
|
|
|
|
|
|
|
|
|
|
| ❌ Pattern | ✅ Fix |
|
|
|
|
|
|-----------|-------|
|
|
|
|
|
| Comment every line | Delete obvious comments |
|
|
|
|
|
| Helper for one-liner | Inline the code |
|
|
|
|
|
| Factory for 2 objects | Direct instantiation |
|
|
|
|
|
| utils.ts with 1 function | Put code where used |
|
|
|
|
|
| "First we import..." | Just write code |
|
|
|
|
|
| Deep nesting | Guard clauses |
|
|
|
|
|
| Magic numbers | Named constants |
|
|
|
|
|
| God functions | Split by responsibility |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 🔴 Before Editing ANY File (THINK FIRST!)
|
|
|
|
|
|
|
|
|
|
**Before changing a file, ask yourself:**
|
|
|
|
|
|
|
|
|
|
| Question | Why |
|
|
|
|
|
|----------|-----|
|
|
|
|
|
| **What imports this file?** | They might break |
|
|
|
|
|
| **What does this file import?** | Interface changes |
|
|
|
|
|
| **What tests cover this?** | Tests might fail |
|
|
|
|
|
| **Is this a shared component?** | Multiple places affected |
|
|
|
|
|
|
|
|
|
|
**Quick Check:**
|
|
|
|
|
```
|
|
|
|
|
File to edit: UserService.ts
|
|
|
|
|
└── Who imports this? → UserController.ts, AuthController.ts
|
|
|
|
|
└── Do they need changes too? → Check function signatures
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
> 🔴 **Rule:** Edit the file + all dependent files in the SAME task.
|
|
|
|
|
> 🔴 **Never leave broken imports or missing updates.**
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Summary
|
|
|
|
|
|
|
|
|
|
| Do | Don't |
|
|
|
|
|
|----|-------|
|
|
|
|
|
| Write code directly | Write tutorials |
|
|
|
|
|
| Let code self-document | Add obvious comments |
|
|
|
|
|
| Fix bugs immediately | Explain the fix first |
|
|
|
|
|
| Inline small things | Create unnecessary files |
|
|
|
|
|
| Name things clearly | Use abbreviations |
|
|
|
|
|
| Keep functions small | Write 100+ line functions |
|
|
|
|
|
|
|
|
|
|
> **Remember: The user wants working code, not a programming lesson.**
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 🔴 Self-Check Before Completing (MANDATORY)
|
|
|
|
|
|
|
|
|
|
**Before saying "task complete", verify:**
|
|
|
|
|
|
|
|
|
|
| Check | Question |
|
|
|
|
|
|-------|----------|
|
|
|
|
|
| ✅ **Goal met?** | Did I do exactly what user asked? |
|
|
|
|
|
| ✅ **Files edited?** | Did I modify all necessary files? |
|
|
|
|
|
| ✅ **Code works?** | Did I test/verify the change? |
|
|
|
|
|
| ✅ **No errors?** | Lint and TypeScript pass? |
|
|
|
|
|
| ✅ **Nothing forgotten?** | Any edge cases missed? |
|
|
|
|
|
|
|
|
|
|
> 🔴 **Rule:** If ANY check fails, fix it before completing.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Verification Scripts (MANDATORY)
|
|
|
|
|
|
|
|
|
|
> 🔴 **CRITICAL:** Each agent runs ONLY their own skill's scripts after completing work.
|
|
|
|
|
|
|
|
|
|
### Agent → Script Mapping
|
|
|
|
|
|
|
|
|
|
| Agent | Script | Command |
|
|
|
|
|
|-------|--------|---------|
|
|
|
|
|
| **frontend-specialist** | UX Audit | `python ~/.claude/skills/frontend-design/scripts/ux_audit.py .` |
|
|
|
|
|
| **frontend-specialist** | A11y Check | `python ~/.claude/skills/frontend-design/scripts/accessibility_checker.py .` |
|
|
|
|
|
| **backend-specialist** | API Validator | `python ~/.claude/skills/api-patterns/scripts/api_validator.py .` |
|
|
|
|
|
| **mobile-developer** | Mobile Audit | `python ~/.claude/skills/mobile-design/scripts/mobile_audit.py .` |
|
|
|
|
|
| **database-architect** | Schema Validate | `python ~/.claude/skills/database-design/scripts/schema_validator.py .` |
|
|
|
|
|
| **security-auditor** | Security Scan | `python ~/.claude/skills/vulnerability-scanner/scripts/security_scan.py .` |
|
|
|
|
|
| **seo-specialist** | SEO Check | `python ~/.claude/skills/seo-fundamentals/scripts/seo_checker.py .` |
|
|
|
|
|
| **seo-specialist** | GEO Check | `python ~/.claude/skills/geo-fundamentals/scripts/geo_checker.py .` |
|
|
|
|
|
| **performance-optimizer** | Lighthouse | `python ~/.claude/skills/performance-profiling/scripts/lighthouse_audit.py <url>` |
|
|
|
|
|
| **test-engineer** | Test Runner | `python ~/.claude/skills/testing-patterns/scripts/test_runner.py .` |
|
|
|
|
|
| **test-engineer** | Playwright | `python ~/.claude/skills/webapp-testing/scripts/playwright_runner.py <url>` |
|
|
|
|
|
| **Any agent** | Lint Check | `python ~/.claude/skills/lint-and-validate/scripts/lint_runner.py .` |
|
|
|
|
|
| **Any agent** | Type Coverage | `python ~/.claude/skills/lint-and-validate/scripts/type_coverage.py .` |
|
|
|
|
|
| **Any agent** | i18n Check | `python ~/.claude/skills/i18n-localization/scripts/i18n_checker.py .` |
|
|
|
|
|
|
|
|
|
|
> ❌ **WRONG:** `test-engineer` running `ux_audit.py`
|
|
|
|
|
> ✅ **CORRECT:** `frontend-specialist` running `ux_audit.py`
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
### 🔴 Script Output Handling (READ → SUMMARIZE → ASK)
|
|
|
|
|
|
|
|
|
|
**When running a validation script, you MUST:**
|
|
|
|
|
|
|
|
|
|
1. **Run the script** and capture ALL output
|
|
|
|
|
2. **Parse the output** - identify errors, warnings, and passes
|
|
|
|
|
3. **Summarize to user** in this format:
|
|
|
|
|
|
|
|
|
|
```markdown
|
|
|
|
|
## Script Results: [script_name.py]
|
|
|
|
|
|
|
|
|
|
### ❌ Errors Found (X items)
|
|
|
|
|
- [File:Line] Error description 1
|
|
|
|
|
- [File:Line] Error description 2
|
|
|
|
|
|
|
|
|
|
### ⚠️ Warnings (Y items)
|
|
|
|
|
- [File:Line] Warning description
|
|
|
|
|
|
|
|
|
|
### ✅ Passed (Z items)
|
|
|
|
|
- Check 1 passed
|
|
|
|
|
- Check 2 passed
|
|
|
|
|
|
|
|
|
|
**Should I fix the X errors?**
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
4. **Wait for user confirmation** before fixing
|
|
|
|
|
5. **After fixing** → Re-run script to confirm
|
|
|
|
|
|
|
|
|
|
> 🔴 **VIOLATION:** Running script and ignoring output = FAILED task.
|
|
|
|
|
> 🔴 **VIOLATION:** Auto-fixing without asking = Not allowed.
|
|
|
|
|
> 🔴 **Rule:** Always READ output → SUMMARIZE → ASK → then fix.
|
|
|
|
|
## 9. Smells and Heuristics
|
|
|
|
|
- **Rigidity**: Hard to change.
|
|
|
|
|
- **Fragility**: Breaks in many places.
|
|
|
|
|
- **Immobility**: Hard to reuse.
|
|
|
|
|
- **Viscosity**: Hard to do the right thing.
|
|
|
|
|
- **Needless Complexity/Repetition**.
|
|
|
|
|
|
|
|
|
|
## 🛠️ Implementation Checklist
|
|
|
|
|
- [ ] Is this function smaller than 20 lines?
|
|
|
|
|
- [ ] Does this function do exactly one thing?
|
|
|
|
|
- [ ] Are all names searchable and intention-revealing?
|
|
|
|
|
- [ ] Have I avoided comments by making the code clearer?
|
|
|
|
|
- [ ] Am I passing too many arguments?
|
|
|
|
|
- [ ] Is there a failing test for this change?
|
|
|
|
|
|
sck_0