Code Smells Detector

Overview

This skill analyzes PHP codebases for code smells (symptoms of deeper problems) and generates detailed reports with severity levels and refactoring recommendations.

Code Smells Catalog

Detection Patterns

God Class Detection

# Large classes (>500 lines)
Grep: "^class " --glob "**/*.php"
# Then check file line counts

# Many public methods (>15)
Grep: "public function " --glob "**/*.php"
# Count per file

# Many dependencies (>8)
Grep: "__construct" --glob "**/*.php" -A 20
# Count constructor parameters

# Problematic names
Grep: "class.*Manager|class.*Handler|class.*Helper|class.*Util|class.*Processor" --glob "**/*.php"

Indicators:

• Class > 500 lines → CRITICAL
• Class > 15 public methods → CRITICAL
• Class > 8 constructor dependencies → WARNING
• Class name contains Manager, Handler, Helper, Util → INFO

Feature Envy Detection

# Methods using other class data excessively
Grep: "\$this->[a-z]+->get[A-Z]" --glob "**/*.php"

# Multiple calls to same foreign object
Grep: "\$[a-z]+->.*\$[a-z]+->" --glob "**/*.php"

# Getters called more than own methods
Grep: "function [a-z]+\(" --glob "**/*.php" -A 30
# Analyze method bodies for foreign vs own calls

Indicators:

• Method calls other object's methods > own methods → WARNING
• Multiple chained calls to foreign object → INFO
• Method only transforms data from another class → WARNING

Data Clumps Detection

# Repeated parameter groups in constructors
Grep: "__construct\(" --glob "**/*.php" -A 10
# Look for patterns: (string $x, string $y, string $z) appearing multiple times

# Repeated parameter groups in methods
Grep: "function [a-z]+\(" --glob "**/*.php"
# Look for same 3+ parameter combinations

# Multiple classes with same field groups
Grep: "(private|readonly) (string|int|float)" --glob "**/*.php"
# Detect repeated field patterns

Common Data Clumps:

• $street, $city, $zipCode, $country → Address Value Object
• $startDate, $endDate → DateRange Value Object
• $amount, $currency → Money Value Object
• $firstName, $lastName, $email → Contact/Person Value Object

Long Parameter List Detection

# Methods with many parameters
Grep: "function [a-z]+\(" --glob "**/*.php"
# Count parameters (comma-separated)

# Constructors with many parameters
Grep: "__construct\(" --glob "**/*.php" -A 15
# Count parameters

Thresholds:

• 4+ parameters → INFO
• 6+ parameters → WARNING
• 8+ parameters → CRITICAL

Long Method Detection

# Find method definitions and count lines until closing brace
Grep: "function [a-z]+\(" --glob "**/*.php" -A 60
# Analyze method length

# Nested control structures (indicator of complexity)
Grep: "if\s*\(.*\{.*if\s*\(" --glob "**/*.php" --multiline

Thresholds:

• 30+ lines → INFO
• 50+ lines → WARNING
• 100+ lines → CRITICAL

Primitive Obsession Detection

# String parameters that should be Value Objects
Grep: "string \$email|string \$phone|string \$url|string \$currency|string \$country" --glob "**/*.php"

# Integer amounts
Grep: "int \$amount|int \$price|int \$total|int \$money|int \$cents" --glob "**/*.php"

# Float for money
Grep: "float \$amount|float \$price|float \$money" --glob "**/*.php"

# String status/type
Grep: "string \$status|string \$type|string \$state" --glob "**/*.php"

# Magic strings
Grep: "=== 'pending'|=== 'active'|=== 'completed'|=== 'draft'" --glob "**/*.php"

Should be Value Objects:

• Email addresses → Email
• Phone numbers → PhoneNumber
• URLs → Url or Uri
• Money amounts → Money (with currency)
• Dates/periods → DateRange, Period
• Identifiers → UserId, OrderId, etc.
• Status/Type → Enum

Message Chains Detection

# Long getter chains
Grep: "->get[A-Z][a-z]+\(\)->get[A-Z][a-z]+\(\)" --glob "**/*.php"

# Triple or more chains
Grep: "->.*->.*->" --glob "**/*.php"

# Law of Demeter violations
Grep: "\$this->[a-z]+->get[A-Z].*->get[A-Z]" --glob "**/*.php"

Indicators:

• 2 chained getters → INFO
• 3+ chained getters → WARNING
• Chains in loops → CRITICAL

Inappropriate Intimacy Detection

# Direct public property access
Grep: "\$[a-z]+->(?!get|set|is|has|can)[a-z]+" --glob "**/*.php"

# Friend classes accessing private state (via reflection)
Grep: "ReflectionClass|ReflectionProperty|setAccessible" --glob "**/*.php"

# Classes knowing internal structure
Grep: "->getInternalState|->getRawData|->getFields" --glob "**/*.php"

Report Format

# Code Smells Analysis Report

## Summary

| Smell | Critical | Warning | Info |
|-------|----------|---------|------|
| God Class | X | X | - |
| Feature Envy | - | X | X |
| Data Clumps | - | X | - |
| Long Parameter List | X | X | X |
| Long Method | - | X | X |
| Primitive Obsession | - | X | X |
| Message Chains | - | X | X |
| Inappropriate Intimacy | - | X | - |

**Total Issues:** X critical, X warnings, X info

## Critical Issues

### SMELL-001: God Class
- **File:** `src/Service/OrderManager.php`
- **Lines:** 847
- **Public Methods:** 23
- **Dependencies:** 12
- **Issue:** Class has too many responsibilities
- **Refactoring:**
  - Extract `OrderValidator` (validation logic)
  - Extract `OrderNotifier` (notification logic)
  - Extract `OrderPriceCalculator` (pricing logic)
- **Skills:** `create-use-case`, `create-domain-service`

### SMELL-002: Long Parameter List
- **File:** `src/Domain/Order/Order.php:45`
- **Method:** `createOrder()`
- **Parameters:** 9
- **Issue:** Too many parameters, hard to maintain
- **Refactoring:** Introduce Parameter Object
- **Skills:** `create-dto`, `create-builder`

## Warning Issues

### SMELL-003: Data Clump
- **Files:**
  - `src/Domain/User/User.php:15` — $street, $city, $zipCode
  - `src/Domain/Company/Company.php:23` — $street, $city, $zipCode
  - `src/Application/DTO/CreateOrderDTO.php:8` — $street, $city, $zipCode
- **Issue:** Address fields repeated across 3 classes
- **Refactoring:** Extract Address Value Object
- **Skills:** `create-value-object`

### SMELL-004: Feature Envy
- **File:** `src/Service/ReportGenerator.php:89`
- **Method:** `generateUserReport()`
- **Issue:** Method makes 15 calls to User object, only 2 to own class
- **Refactoring:** Move method to User or create UserReportBuilder
- **Skills:** `create-domain-service`

### SMELL-005: Primitive Obsession
- **File:** `src/Domain/User/User.php:12`
- **Field:** `private string $email`
- **Issue:** Email should be Value Object for validation
- **Refactoring:** Create Email Value Object
- **Skills:** `create-value-object`

### SMELL-006: Message Chain
- **File:** `src/Application/Handler/CreateOrderHandler.php:34`
- **Code:** `$user->getCompany()->getAddress()->getCountry()`
- **Issue:** Law of Demeter violation, tight coupling
- **Refactoring:** Add shortcut method or delegate

## Info Issues

### SMELL-007: Long Method
- **File:** `src/Infrastructure/Repository/OrderRepository.php:78`
- **Method:** `findByComplexCriteria()`
- **Lines:** 45
- **Issue:** Method approaching complexity threshold
- **Refactoring:** Extract query builder or specification

## Refactoring Priority

1. **Immediate:** God Classes blocking testing
2. **High:** Data Clumps causing duplication
3. **Medium:** Long Parameter Lists
4. **Low:** Message Chains, minor smells

Remediation Skills

Quick Analysis Commands

# Full smell detection
echo "=== God Classes ===" && \
find . -name "*.php" -path "*/src/*" -exec wc -l {} \; | awk '$1 > 400' && \
echo "=== Long Parameter Lists ===" && \
grep -rn "function [a-z]*(" --include="*.php" src/ | grep -E "(\$[a-z]+,\s*){5,}" && \
echo "=== Primitive Obsession ===" && \
grep -rn "string \$email\|string \$phone\|int \$amount\|float \$price" --include="*.php" src/ && \
echo "=== Message Chains ===" && \
grep -rn "->get[A-Z].*->get[A-Z].*->get[A-Z]" --include="*.php" src/ && \
echo "=== Magic Strings ===" && \
grep -rn "=== '[a-z]*'\|== '[a-z]*'" --include="*.php" src/

Integration with Other Skills

This skill works alongside:

• analyze-solid-violations — SOLID violations overlap with some smells
• structural-auditor — architectural context for smells
• ddd-auditor — domain model quality assessment

References

Based on Martin Fowler's "Refactoring" catalog:

• https://refactoring.guru/refactoring/smells
• "Refactoring: Improving the Design of Existing Code" (Fowler)

When This Is Acceptable

• DTOs — Data classes are NOT a code smell; they serve a clear data transfer purpose
• Configuration classes — Classes with many constants/properties for configuration
• Builder pattern — Method chaining in builders creates apparent "Feature Envy" but is by design

False Positive Indicators

• Class has DTO, Request, Response, Config in its name
• Class implements a Builder pattern with fluent API
• "Long parameter list" is actually a constructor with proper dependency injection