code-review

code-review

描述：自动化代码质量分析、静态分析、安全扫描和最佳实践执行

类别：代码质量保障

复杂度：高（多工具集成与分析）

---

目的

进行全面的代码审查，确保代码质量、安全性、可维护性，并符合架构决策（ADR）和规范（SPEC）。在代码进入生产环境之前识别问题。

---

功能

1. 静态分析

• Lint 检查：集成 pylint、ruff、flake8
• 类型检查：使用 mypy、pyright 确保类型安全
• 代码风格：PEP 8 合规性、格式问题
• 复杂度指标：圈复杂度、认知复杂度
• 代码异味：重复代码、过长的方法、上帝类

2. 安全漏洞扫描

• 依赖项漏洞：bandit、安全检查
• 安全反模式：SQL 注入、XSS、CSRF 风险
• 密钥检测：硬编码凭据、API 密钥
• OWASP Top 10：常见安全漏洞
• CWE 映射：通用弱点枚举（CWE）参考

3. 最佳实践执行

• 设计模式：正确使用模式
• SOLID 原则：检测违规情况
• DRY 原则：代码重复分析
• 错误处理：异常管理审查
• 资源管理：内存泄漏、文件句柄泄漏

4. 代码覆盖率分析

• 行覆盖率：已执行代码行的百分比
• 分支覆盖率：条件路径覆盖率
• 函数覆盖率：已测试与未测试的函数
• 缺口识别：未覆盖的关键路径

5. 架构合规性

• ADR 验证：代码与架构决策保持一致
• SPEC 合规性：实现符合规范
• 依赖规则：无循环依赖，分层合理
• API 契约遵循情况：符合 CTR 定义

6. 性能分析

• 低效算法：可使用 O(n) 时却使用 O(n²)
• 内存使用：过度分配
• 数据库查询：N+1 查询问题
• 缓存机会：重复执行开销较大的操作

7. 文档质量

• 文档字符串覆盖率：缺失或不充分的文档
• 注释质量：过时或有误导性的注释
• API 文档：公共方法文档
• 类型提示：缺少类型注解

8. 自动修复建议

• 可自动修复的问题：格式、导入、简单重构
• 重构建议：提取方法、简化条件
• 性能优化：建议更快的替代方案
• 安全补丁：修复常见漏洞

---

代码审查工作流

graph TD
    A[Code Submission] --> B[Static Analysis]
    B --> C{Linting Pass?}
    C -->|Fail| D[Report Linting Issues]
    C -->|Pass| E[Type Checking]

    E --> F{Type Safety?}
    F -->|Fail| G[Report Type Errors]
    F -->|Pass| H[Security Scan]

    H --> I{Security Issues?}
    I -->|Critical| J[Block Merge]
    I -->|Warning| K[Flag for Review]
    I -->|Pass| L[Complexity Analysis]

    L --> M{Complexity High?}
    M -->|Yes| N[Suggest Refactoring]
    M -->|No| O[Coverage Analysis]

    O --> P{Coverage < 90%?}
    P -->|Yes| Q[Request More Tests]
    P -->|No| R[ADR Compliance]

    R --> S{ADR Violations?}
    S -->|Yes| T[Report Violations]
    S -->|No| U[SPEC Compliance]

    U --> V{SPEC Mismatch?}
    V -->|Yes| W[Report Mismatches]
    V -->|No| X[Generate Review Report]

    D --> X
    G --> X
    J --> X
    K --> X
    N --> X
    Q --> X
    T --> X
    W --> X

    X --> Y[Review Complete]

---

使用说明

审查单个文件

code-review analyze --file src/auth/service.py

输出：

=== Code Review Report: src/auth/service.py ===

Issues Found: 8

[CRITICAL] Security Issue (Line 45)
- SQL Injection vulnerability in user_login()
- Direct string concatenation in SQL query
- CWE-89: SQL Injection
→ Fix: Use parameterized queries

[ERROR] Type Safety (Line 78)
- Missing return type annotation for validate_token()
- mypy: Function return type cannot be inferred
→ Fix: Add return type hint -> bool

[WARNING] Complexity (Line 112-156)
- Cyclomatic complexity: 12 (threshold: 10)
- Method authenticate_user() too complex
→ Suggestion: Extract validation logic to separate methods

[WARNING] Code Duplication (Lines 201-215, 234-248)
- 15 lines duplicated with 95% similarity
→ Suggestion: Extract common logic to validate_password_strength()

[INFO] Documentation (Line 89)
- Missing docstring for public method reset_password()
→ Suggestion: Add docstring with parameter and return descriptions

[INFO] Performance (Line 167)
- N+1 query detected in get_user_permissions()
→ Suggestion: Use JOIN or prefetch related data

Coverage: 87% (Below 90% threshold)
- Uncovered lines: 45-47, 89-91, 134-136
→ Action: Add tests for error handling paths

ADR Compliance: 1 violation
- ADR-003 requires JWT tokens, code uses session cookies
→ Action: Update implementation to use JWT

Total Score: 72/100 (Acceptable with improvements)

审查整个模块

code-review analyze --module src/auth

审查已更改的文件（Git）

code-review analyze --diff HEAD~1..HEAD

审查并自动修复

code-review analyze --file src/auth/service.py --auto-fix

---

分析类别

1. 安全分析

工具：bandit、safety、semgrep

检查项：

• SQL 注入漏洞
• XSS 攻击向量
• CSRF 令牌验证
• 硬编码密钥
• 不安全的随机数
• 路径遍历风险
• 命令注入
• 不安全的反序列化
• 弱加密

严重级别：

• CRITICAL：存在即时安全风险（阻止合并）
• HIGH：严重漏洞（必须修复）
• MEDIUM：潜在安全问题（需要审查）
• LOW：违反安全最佳实践

示例：

# CRITICAL: SQL Injection
query = f"SELECT * FROM users WHERE username = '{username}'"  # ❌

# FIX
query = "SELECT * FROM users WHERE username = %s"
cursor.execute(query, (username,))  # ✓

# CRITICAL: Hardcoded Secret
API_KEY = "sk-1234567890abcdef"  # ❌

# FIX
API_KEY = os.environ.get('API_KEY')  # ✓

---

2. 代码质量分析

工具：pylint、ruff、radon

检查项：

• PEP 8 合规性
• 圈复杂度
• 可维护性指数
• 代码重复
• 死代码
• 未使用的导入/变量
• 魔法数字
• 过长的函数/类

复杂度阈值：

• A (1-5)：简单，易于维护
• B (6-10)：复杂度适中
• C (11-20)：复杂度高（建议重构）
• D (21-50)：复杂度极高（必须重构）
• F (>50)：复杂度极端（立即重构）

示例：

# Complexity: 12 (High)
def process_order(order):  # ❌
    if order.status == 'pending':
        if order.payment_verified:
            if order.items_available:
                if order.shipping_valid:
                    # ... nested logic
                    pass

# FIX: Complexity: 4 (Simple)
def process_order(order):  # ✓
    if not can_process_order(order):
        return False

    return execute_order_processing(order)

def can_process_order(order):
    return (order.status == 'pending' and
            order.payment_verified and
            order.items_available and
            order.shipping_valid)

---

3. 类型安全分析

工具：mypy、pyright

检查项：

• 缺少类型提示
• 类型不兼容
• 无效的类型转换
• 可选值处理
• 泛型类型用法
• 协议合规性

示例：

# Type error: Missing return type
def calculate_total(items):  # ❌
    return sum(item.price for item in items)

# FIX
def calculate_total(items: list[Item]) -> Decimal:  # ✓
    return sum(item.price for item in items)

# Type error: None not handled
def get_user(user_id: int) -> User:  # ❌
    return database.query(User).get(user_id)  # May return None

# FIX
def get_user(user_id: int) -> User | None:  # ✓
    return database.query(User).get(user_id)

---

4. 性能分析

检查项：

• 低效算法
• 内存泄漏
• N+1 查询问题
• 不必要的循环
• 缺少缓存
• 阻塞式 I/O 操作
• 大量内存分配

示例：

# Performance issue: N+1 queries
def get_users_with_permissions():  # ❌
    users = User.query.all()
    for user in users:
        user.permissions = Permission.query.filter_by(user_id=user.id).all()
    return users

# FIX: Single query with JOIN
def get_users_with_permissions():  # ✓
    return User.query.options(
        joinedload(User.permissions)
    ).all()

# Performance issue: Inefficient algorithm O(n²)
def find_duplicates(items):  # ❌
    duplicates = []
    for i, item1 in enumerate(items):
        for item2 in items[i+1:]:
            if item1 == item2:
                duplicates.append(item1)
    return duplicates

# FIX: O(n) with set
def find_duplicates(items):  # ✓
    seen = set()
    duplicates = set()
    for item in items:
        if item in seen:
            duplicates.add(item)
        seen.add(item)
    return list(duplicates)

---

5. 最佳实践分析

检查项：

• SOLID 原则
• DRY（不要重复自己）
• KISS（保持简单）
• YAGNI（你不会需要它）
• 错误处理模式
• 资源管理
• 日志记录实践

示例：

# Violation: Single Responsibility Principle
class UserManager:  # ❌
    def create_user(self, data):
        # Validates data
        # Saves to database
        # Sends email
        # Logs action
        pass

# FIX: Separate responsibilities
class UserValidator:  # ✓
    def validate(self, data): pass

class UserRepository:
    def save(self, user): pass

class EmailService:
    def send_welcome_email(self, user): pass

class AuditLogger:
    def log_user_creation(self, user): pass

---

6. 文档分析

检查项：

• 缺少文档字符串
• 文档字符串不完整
• 注释已过时
• 类型提示文档
• API 文档
• 使用示例

示例：

# Poor documentation
def process(data):  # ❌
    # Process the data
    return result

# Good documentation
def process_user_registration(  # ✓
    user_data: dict[str, Any]
) -> User:
    """
    Process new user registration with validation and persistence.

    Args:
        user_data: Dictionary containing user registration information
            Required keys: username, email, password
            Optional keys: full_name, phone_number

    Returns:
        User: Newly created user instance with assigned ID

    Raises:
        ValidationError: If user_data fails validation
        DatabaseError: If user cannot be persisted
        DuplicateUserError: If username or email already exists

    Example:
        >>> user = process_user_registration({
        ...     'username': 'johndoe',
        ...     'email': '[email protected]',
        ...     'password': 'SecureP@ss123'
        ... })
        >>> user.id
        12345
    """
    validated_data = UserValidator.validate(user_data)
    user = User.create(validated_data)
    return UserRepository.save(user)

---

7. ADR 合规性分析

检查项：

• 是否遵循架构决策
• 是否符合技术栈要求
• 设计模式的使用
• API 设计标准
• 数据流模式

示例：

# ADR-003: All authentication must use JWT tokens

# Violation
@app.route('/login', methods=['POST'])  # ❌
def login():
    # ... authentication logic
    session['user_id'] = user.id  # Using sessions, not JWT
    return jsonify({'success': True})

# Compliant
@app.route('/login', methods=['POST'])  # ✓
def login():
    # ... authentication logic
    token = jwt.encode(
        {'user_id': user.id, 'exp': datetime.utcnow() + timedelta(hours=1)},
        SECRET_KEY,
        algorithm='HS256'
    )
    return jsonify({'token': token})

---

8. SPEC 合规性分析

检查项：

• 实现是否符合规范
• API 签名是否正确
• 数据结构是否符合架构
• 业务逻辑是否正确
• 错误处理是否符合规范

示例：

# SPEC-AUTH-V1: Password validation requirements

# SPEC violation: Missing uppercase requirement
def validate_password(password: str) -> bool:  # ❌
    return (
        len(password) >= 8 and
        any(c.islower() for c in password) and
        any(c.isdigit() for c in password)
    )

# SPEC compliant
def validate_password(password: str) -> bool:  # ✓
    return (
        8 <= len(password) <= 20 and
        any(c.isupper() for c in password) and  # Missing requirement
        any(c.islower() for c in password) and
        any(c.isdigit() for c in password) and
        any(c in '!@#$%^&*' for c in password)
    )

---

审查评分计算

score = (
    security_score * 0.30 +      # 30% weight
    quality_score * 0.25 +        # 25% weight
    type_safety_score * 0.15 +    # 15% weight
    coverage_score * 0.15 +       # 15% weight
    documentation_score * 0.10 +  # 10% weight
    performance_score * 0.05      # 5% weight
)

# Score categories
90-100: Excellent (merge approved)
80-89:  Good (minor improvements recommended)
70-79:  Acceptable (improvements required)
60-69:  Needs work (significant issues)
<60:    Poor (major rework needed)

---

工具访问权限

所需工具：

• Read：读取源代码文件
• Bash：执行分析工具（pylint、mypy、bandit）
• Grep：搜索模式和违规项
• Glob：查找代码文件

所需库：

• pylint
• ruff
• mypy / pyright
• bandit
• safety
• radon
• coverage
• semgrep

---

集成点

与 test-automation 集成

• 验证测试覆盖率是否达到阈值
• 找出未测试的代码路径
• 验证测试质量

与 doc-validator 集成

• 检查代码文档质量
• 验证代码中的可追溯性引用
• 确保代码注释符合标准

与 security-audit 集成

• 共享安全漏洞发现
• 协调安全修复
• 跟踪安全指标

与 refactor-flow 集成

• 找出重构机会
• 提供复杂度指标
• 提出代码改进建议

---

自动修复能力

安全的自动修复（自动应用）

• 导入排序
• 代码格式化（black、autopep8）
• 删除行尾空格
• 补充缺少的空行
• 删除未使用的导入
• 添加简单的类型提示

建议的修复（需要批准）

• 提取方法重构
• 降低复杂度
• 性能优化
• 安全漏洞补丁
• 补充文档

---

CI/CD 集成

# .github/workflows/code-review.yml
name: Automated Code Review

on: [pull_request]

jobs:
  code-review:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - name: Run code review
        run: |
          code-review analyze --diff origin/main..HEAD

      - name: Check quality gate
        run: |
          if [ $CODE_REVIEW_SCORE -lt 70 ]; then
            echo "Code quality below threshold"
            exit 1
          fi

---

最佳实践

1. 尽早且频繁地审查：不要等到 PR 阶段
2. 立即修复关键问题：优先处理安全性和正确性问题
3. 持续跟踪指标：监控代码质量趋势
4. 在 CI/CD 中自动化：防止质量差的代码合并
5. 确定问题优先级：重点关注影响大的问题
6. 提供背景信息：解释问题的重要性
7. 提出解决方案：不要只指出问题
8. 平衡严格程度：不要因轻微的风格问题阻碍进度

---

局限性

1. 无法理解业务逻辑意图
2. 可能产生误报（需要人工审查）
3. 无法检测所有安全漏洞
4. 性能分析仅限于明显的模式
5. ADR/SPEC 合规性需要手动定义规则
6. 需要安装和配置工具

---

后续改进

1. 由 AI 驱动的代码理解和建议
2. 从历史代码审查中学习
3. 用于项目特定标准的自定义规则引擎
4. 可视化代码质量仪表板
5. 自动更新依赖项
6. 预测性 Bug 检测
7. 代码异味趋势分析

---

成功标准

• 严重安全漏洞为零
• 代码质量评分 ≥80/100
• 类型安全：公开 API 的类型提示覆盖率为 100%
• 覆盖率 ≥90%
• 复杂度：没有复杂度 >10 的函数
• ADR 违规为零
• SPEC 不匹配为零

---

说明

• 审查报告保存在 reports/code-review/
• 历史指标记录在 metrics/code-quality.json 中
• 自动修复需要获得批准，除非另有配置
• 安全问题始终会阻止合并
• 可通过 .code-review.yml 配置文件按项目自定义