code-smell-detector by rysweet/amplihack
npx skills add https://github.com/rysweet/amplihack --skill code-smell-detector本技能识别违反 amplihack 开发理念的反模式,并提供建设性、具体的修复方案。它确保代码保持极致的简洁性、模块化设计,以及零废话的实现。
Amplihack 开发理念专注于:
定义:不必要的抽象层、不提供明确价值的通用基类或接口。
为何不好:违反"极致简洁"原则——增加了复杂性却没有带来相应好处。使代码更难理解和维护。
危险信号:
:
广告位招租
在这里展示您的产品或服务
触达数万 AI 开发者,精准高效
# BAD: Over-abstracted
class DataProcessor(ABC):
@abstractmethod
def process(self, data):
pass
class SimpleDataProcessor(DataProcessor):
def process(self, data):
return data * 2
示例 - 修复后:
# GOOD: Direct implementation
def process_data(data):
"""Process data by doubling it."""
return data * 2
检测清单:
修复策略:
定义:深度继承链、多重继承或使代码流程模糊的复杂类层次结构。
为何不好:使代码难以跟踪,造成紧耦合,违反简洁性原则。谁做什么变得不清晰。
危险信号:
示例 - 异味:
# BAD: Complex inheritance
class Entity:
def save(self): pass
def load(self): pass
class TimestampedEntity(Entity):
def add_timestamp(self): pass
class AuditableEntity(TimestampedEntity):
def audit_log(self): pass
class User(AuditableEntity):
def authenticate(self): pass
示例 - 修复后:
# GOOD: Composition over inheritance
class User:
def __init__(self, storage, timestamp_service, audit_log):
self.storage = storage
self.timestamps = timestamp_service
self.audit = audit_log
def save(self):
self.storage.save(self)
self.timestamps.record()
self.audit.log("saved user")
检测清单:
修复策略:
定义:做太多事情、难以理解、测试和修改的函数。
为何不好:违反单一职责原则,使测试更困难,增加错误发生面,降低代码可重用性。
危险信号:
示例 - 异味:
# BAD: Large function doing multiple things
def process_user_data(user_dict, validate=True, save=True, notify=True, log=True):
if validate:
if not user_dict.get('email'):
raise ValueError("Email required")
if not '@' in user_dict['email']:
raise ValueError("Invalid email")
user = User(
name=user_dict['name'],
email=user_dict['email'],
phone=user_dict['phone']
)
if save:
db.save(user)
if notify:
email_service.send(user.email, "Welcome!")
if log:
logger.info(f"User {user.name} created")
# ... 30+ more lines of mixed concerns
return user
示例 - 修复后:
# GOOD: Separated concerns
def validate_user_data(user_dict):
"""Validate user data structure."""
if not user_dict.get('email'):
raise ValueError("Email required")
if '@' not in user_dict['email']:
raise ValueError("Invalid email")
def create_user(user_dict):
"""Create user object from data."""
return User(
name=user_dict['name'],
email=user_dict['email'],
phone=user_dict['phone']
)
def process_user_data(user_dict):
"""Orchestrate user creation workflow."""
validate_user_data(user_dict)
user = create_user(user_dict)
db.save(user)
email_service.send(user.email, "Welcome!")
logger.info(f"User {user.name} created")
return user
检测清单:
修复策略:
定义:模块/类直接依赖于具体实现而非抽象,使得它们难以测试和修改。
为何不好:一个模块的更改会破坏其他模块。难以独立测试。违反模块化原则。
危险信号:
db = Database())obj.service.repository.data)示例 - 异味:
# BAD: Tight coupling
class UserService:
def create_user(self, name, email):
db = Database() # Hardcoded dependency
user = db.save_user(name, email)
email_service = EmailService() # Hardcoded dependency
email_service.send(email, "Welcome!")
return user
def get_user(self, user_id):
db = Database()
return db.find_user(user_id)
示例 - 修复后:
# GOOD: Loose coupling via dependency injection
class UserService:
def __init__(self, db, email_service):
self.db = db
self.email = email_service
def create_user(self, name, email):
user = self.db.save_user(name, email)
self.email.send(email, "Welcome!")
return user
def get_user(self, user_id):
return self.db.find_user(user_id)
# Usage:
user_service = UserService(db=PostgresDB(), email_service=SMTPService())
检测清单:
Service())修复策略:
__all__ 导出(Python)定义:未通过 __all__ 明确定义其公共接口的 Python 模块。
为何不好:不清楚什么是公共的,什么是内部的。用户导入了私有实现细节。违反"接口"概念——连接点不清晰。
危险信号:
__init__.py 中没有 __all___function)仍然可访问示例 - 异味:
# BAD: No __all__ - unclear public interface
# module/__init__.py
from .core import process_data, _internal_helper
from .utils import validate_input, LOG_LEVEL
# What should users import? All of it? Only some?
示例 - 修复后:
# GOOD: Clear public interface via __all__
# module/__init__.py
from .core import process_data
from .utils import validate_input
__all__ = ['process_data', 'validate_input']
# Users know exactly what's public and what to use
检测清单:
__init__.py 中缺少 __all___ 为前缀)修复策略:
__init__.py 添加 __all___ 前缀__all____all__ 的存在/缺失对于每个潜在问题:
对于发现的每种异味:
检查:class X(ABC) 且恰好有 1 个具体实现
# BAD pattern detection
- Count implementations of abstract class
- If count <= 2 and not used as interface: FLAG
修复:移除抽象,使用直接实现
检查:类层次结构深度
# BAD pattern detection
- Follow inheritance chain: class -> parent -> grandparent...
- If depth > 2: FLAG
修复:使用组合替代
检查:所有函数体
# BAD pattern detection
- Count lines in function (excluding docstring)
- If > 50 lines: FLAG
- If > 3 nesting levels: FLAG
修复:提取辅助函数
检查:方法/函数内部的类实例化
# BAD pattern detection
- Search for "= ServiceName()" inside methods
- If found: FLAG
修复:作为构造函数参数传递
检查:Python 模块
# BAD pattern detection
- Look for __all__ definition
- If missing: FLAG
- If __all__ incomplete: FLAG
修复:定义明确的 __all__
# BAD
class StringUtils:
@staticmethod
def clean(s):
return s.strip().lower()
修复:使用直接函数
# GOOD
def clean_string(s):
return s.strip().lower()
# BAD
class UserManager:
def create(self): pass
def update(self): pass
def delete(self): pass
def validate(self): pass
def email(self): pass
修复:拆分为专注的服务
# GOOD
class UserService:
def __init__(self, db, email):
self.db = db
self.email = email
def create(self): pass
def update(self): pass
def delete(self): pass
def validate_user(user): pass
# BAD - 200 line function doing everything
def process_order(order_data, validate, save, notify, etc...):
# 200 lines mixing validation, transformation, DB, email, logging
修复:组合小函数
# GOOD
def process_order(order_data):
validate_order(order_data)
order = create_order(order_data)
save_order(order)
notify_customer(order)
log_creation(order)
# BAD
class Base:
def work(self): pass
class Middle(Base):
def work(self):
return super().work()
class Derived(Middle):
def work(self):
return super().work() # Which work()?
修复:使用清晰、可测试的组合
# GOOD
class Worker:
def __init__(self, validator, transformer):
self.validator = validator
self.transformer = transformer
def work(self, data):
self.validator.check(data)
return self.transformer.apply(data)
# BAD
def fetch_data(user_id):
db = Database() # Where's this coming from?
return db.query(f"SELECT * FROM users WHERE id={user_id}")
修复:显式注入依赖
# GOOD
def fetch_data(user_id, db):
return db.query(f"SELECT * FROM users WHERE id={user_id}")
# Or in a class:
class UserRepository:
def __init__(self, db):
self.db = db
def fetch(self, user_id):
return self.db.query(f"SELECT * FROM users WHERE id={user_id}")
User: Review this new authentication module for code smells.
Claude:
1. Scans all Python files in module
2. Checks for each smell type
3. Finds:
- Abstract base class with 1 implementation
- Large 120-line authenticate() function
- Missing __all__ in __init__.py
4. Provides specific fixes with before/after code
5. Explains philosophy violations
User: Find tight coupling in this user service.
Claude:
1. Traces all dependencies
2. Finds hardcoded Database() instantiation
3. Finds direct EmailService() creation
4. Shows dependency injection fix
5. Includes test example showing why it matters
User: This class hierarchy is too complex.
Claude:
1. Maps inheritance tree (finds 4 levels)
2. Shows each level doing what
3. Suggests composition approach
4. Provides before/after refactoring
5. Explains how it aligns with brick philosophy
__all__)使用本技能进行代码审查应:
何时使用本技能:
与以下工具配合良好:
~/.amplihack/.claude/context/PHILOSOPHY.md~/.amplihack/.claude/context/PATTERNS.md本技能通过以下方式帮助维护代码质量:
建设性地使用它——目标是学习和改进,而非批评。
每周安装次数
116
仓库
GitHub 星标数
39
首次出现
2026年1月23日
安全审计
安装于
opencode103
codex99
claude-code98
cursor96
github-copilot96
gemini-cli95
This skill identifies anti-patterns that violate amplihack's development philosophy and provides constructive, specific fixes. It ensures code maintains ruthless simplicity, modular design, and zero-BS implementations.
Amplihack Development Philosophy focuses on:
What It Is : Unnecessary layers of abstraction, generic base classes, or interfaces that don't provide clear value.
Why It's Bad : Violates "ruthless simplicity" - adds complexity without proportional benefit. Makes code harder to understand and maintain.
Red Flags :
Example - SMELL :
# BAD: Over-abstracted
class DataProcessor(ABC):
@abstractmethod
def process(self, data):
pass
class SimpleDataProcessor(DataProcessor):
def process(self, data):
return data * 2
Example - FIXED :
# GOOD: Direct implementation
def process_data(data):
"""Process data by doubling it."""
return data * 2
Detection Checklist :
Fix Strategy :
What It Is : Deep inheritance chains, multiple inheritance, or convoluted class hierarchies that obscure code flow.
Why It's Bad : Makes code hard to follow, creates tight coupling, violates simplicity principle. Who does what becomes unclear.
Red Flags :
Example - SMELL :
# BAD: Complex inheritance
class Entity:
def save(self): pass
def load(self): pass
class TimestampedEntity(Entity):
def add_timestamp(self): pass
class AuditableEntity(TimestampedEntity):
def audit_log(self): pass
class User(AuditableEntity):
def authenticate(self): pass
Example - FIXED :
# GOOD: Composition over inheritance
class User:
def __init__(self, storage, timestamp_service, audit_log):
self.storage = storage
self.timestamps = timestamp_service
self.audit = audit_log
def save(self):
self.storage.save(self)
self.timestamps.record()
self.audit.log("saved user")
Detection Checklist :
Fix Strategy :
What It Is : Functions that do too many things and are difficult to understand, test, and modify.
Why It's Bad : Violates single responsibility, makes testing harder, increases bug surface area, reduces code reusability.
Red Flags :
Example - SMELL :
# BAD: Large function doing multiple things
def process_user_data(user_dict, validate=True, save=True, notify=True, log=True):
if validate:
if not user_dict.get('email'):
raise ValueError("Email required")
if not '@' in user_dict['email']:
raise ValueError("Invalid email")
user = User(
name=user_dict['name'],
email=user_dict['email'],
phone=user_dict['phone']
)
if save:
db.save(user)
if notify:
email_service.send(user.email, "Welcome!")
if log:
logger.info(f"User {user.name} created")
# ... 30+ more lines of mixed concerns
return user
Example - FIXED :
# GOOD: Separated concerns
def validate_user_data(user_dict):
"""Validate user data structure."""
if not user_dict.get('email'):
raise ValueError("Email required")
if '@' not in user_dict['email']:
raise ValueError("Invalid email")
def create_user(user_dict):
"""Create user object from data."""
return User(
name=user_dict['name'],
email=user_dict['email'],
phone=user_dict['phone']
)
def process_user_data(user_dict):
"""Orchestrate user creation workflow."""
validate_user_data(user_dict)
user = create_user(user_dict)
db.save(user)
email_service.send(user.email, "Welcome!")
logger.info(f"User {user.name} created")
return user
Detection Checklist :
Fix Strategy :
What It Is : Modules/classes directly depend on concrete implementations instead of abstractions, making them hard to test and modify.
Why It's Bad : Changes in one module break others. Hard to test in isolation. Violates modularity principle.
Red Flags :
db = Database())obj.service.repository.data)Example - SMELL :
# BAD: Tight coupling
class UserService:
def create_user(self, name, email):
db = Database() # Hardcoded dependency
user = db.save_user(name, email)
email_service = EmailService() # Hardcoded dependency
email_service.send(email, "Welcome!")
return user
def get_user(self, user_id):
db = Database()
return db.find_user(user_id)
Example - FIXED :
# GOOD: Loose coupling via dependency injection
class UserService:
def __init__(self, db, email_service):
self.db = db
self.email = email_service
def create_user(self, name, email):
user = self.db.save_user(name, email)
self.email.send(email, "Welcome!")
return user
def get_user(self, user_id):
return self.db.find_user(user_id)
# Usage:
user_service = UserService(db=PostgresDB(), email_service=SMTPService())
Detection Checklist :
Service())Fix Strategy :
__all__ Exports (Python)What It Is : Python modules that don't explicitly define their public interface via __all__.
Why It's Bad : Unclear what's public vs internal. Users import private implementation details. Violates the "stud" concept - unclear connection points.
Red Flags :
__all__ in __init__.py_function) still accessibleExample - SMELL :
# BAD: No __all__ - unclear public interface
# module/__init__.py
from .core import process_data, _internal_helper
from .utils import validate_input, LOG_LEVEL
# What should users import? All of it? Only some?
Example - FIXED :
# GOOD: Clear public interface via __all__
# module/__init__.py
from .core import process_data
from .utils import validate_input
__all__ = ['process_data', 'validate_input']
# Users know exactly what's public and what to use
Detection Checklist :
__all__ in __init__.py_) exposedFix Strategy :
__all__ to every __init__.py___all____all__ presence/absenceFor each potential issue:
For each smell found:
Check : class X(ABC) with exactly 1 concrete implementation
# BAD pattern detection
- Count implementations of abstract class
- If count <= 2 and not used as interface: FLAG
Fix : Remove abstraction, use direct implementation
Check : Class hierarchy depth
# BAD pattern detection
- Follow inheritance chain: class -> parent -> grandparent...
- If depth > 2: FLAG
Fix : Use composition instead
Check : All function bodies
# BAD pattern detection
- Count lines in function (excluding docstring)
- If > 50 lines: FLAG
- If > 3 nesting levels: FLAG
Fix : Extract helper functions
Check : Class instantiation inside methods/functions
# BAD pattern detection
- Search for "= ServiceName()" inside methods
- If found: FLAG
Fix : Pass as constructor argument
Check : Python modules
# BAD pattern detection
- Look for __all__ definition
- If missing: FLAG
- If __all__ incomplete: FLAG
Fix : Define explicit __all__
# BAD
class StringUtils:
@staticmethod
def clean(s):
return s.strip().lower()
Fix : Use direct function
# GOOD
def clean_string(s):
return s.strip().lower()
# BAD
class UserManager:
def create(self): pass
def update(self): pass
def delete(self): pass
def validate(self): pass
def email(self): pass
Fix : Split into focused services
# GOOD
class UserService:
def __init__(self, db, email):
self.db = db
self.email = email
def create(self): pass
def update(self): pass
def delete(self): pass
def validate_user(user): pass
# BAD - 200 line function doing everything
def process_order(order_data, validate, save, notify, etc...):
# 200 lines mixing validation, transformation, DB, email, logging
Fix : Compose small functions
# GOOD
def process_order(order_data):
validate_order(order_data)
order = create_order(order_data)
save_order(order)
notify_customer(order)
log_creation(order)
# BAD
class Base:
def work(self): pass
class Middle(Base):
def work(self):
return super().work()
class Derived(Middle):
def work(self):
return super().work() # Which work()?
Fix : Use clear, testable composition
# GOOD
class Worker:
def __init__(self, validator, transformer):
self.validator = validator
self.transformer = transformer
def work(self, data):
self.validator.check(data)
return self.transformer.apply(data)
# BAD
def fetch_data(user_id):
db = Database() # Where's this coming from?
return db.query(f"SELECT * FROM users WHERE id={user_id}")
Fix : Inject dependencies explicitly
# GOOD
def fetch_data(user_id, db):
return db.query(f"SELECT * FROM users WHERE id={user_id}")
# Or in a class:
class UserRepository:
def __init__(self, db):
self.db = db
def fetch(self, user_id):
return self.db.query(f"SELECT * FROM users WHERE id={user_id}")
User: Review this new authentication module for code smells.
Claude:
1. Scans all Python files in module
2. Checks for each smell type
3. Finds:
- Abstract base class with 1 implementation
- Large 120-line authenticate() function
- Missing __all__ in __init__.py
4. Provides specific fixes with before/after code
5. Explains philosophy violations
User: Find tight coupling in this user service.
Claude:
1. Traces all dependencies
2. Finds hardcoded Database() instantiation
3. Finds direct EmailService() creation
4. Shows dependency injection fix
5. Includes test example showing why it matters
User: This class hierarchy is too complex.
Claude:
1. Maps inheritance tree (finds 4 levels)
2. Shows each level doing what
3. Suggests composition approach
4. Provides before/after refactoring
5. Explains how it aligns with brick philosophy
__all__)A code review using this skill should:
When to Use This Skill :
Works Well With :
~/.amplihack/.claude/context/PHILOSOPHY.md~/.amplihack/.claude/context/PATTERNS.mdThis skill helps maintain code quality by:
Use it constructively - the goal is learning and improvement, not criticism.
Weekly Installs
116
Repository
GitHub Stars
39
First Seen
Jan 23, 2026
Security Audits
Gen Agent Trust HubFailSocketPassSnykPass
Installed on
opencode103
codex99
claude-code98
cursor96
github-copilot96
gemini-cli95
Perl安全编程指南:输入验证、注入防护与安全编码实践
1,100 周安装
系统思维指南:6位产品领导者的框架与洞见,解决复杂问题的系统思维方法
1,100 周安装
创业构思指南:运用产品负责人框架生成与评估创业想法
1,100 周安装
AI产品战略指南:94位专家框架,助你制定AI产品决策与架构规划
1,100 周安装
Medusa 管理面板自定义开发指南:使用 Admin SDK 和 UI 组件构建扩展
1,100 周安装
Vue 3 专家技能包 | Composition API、Pinia、Nuxt 3 全栈开发指南
1,200 周安装
Atlassian MCP 服务器配置与使用指南:Jira JQL查询、Confluence CQL搜索与自动化工作流
1,100 周安装