代码审查

何时使用此技能

• 审查拉取请求
• 检查代码质量
• 对实现提供反馈
• 识别潜在错误
• 提出改进建议
• 安全审计
• 性能分析

指示

步骤 1：理解上下文

阅读拉取请求描述：

• 此更改的目标是什么？
• 它解决了哪些问题？
• 是否有任何特殊考虑？

检查范围：

• 更改了多少个文件？
• 更改类型是什么？（功能、错误修复、重构）
• 是否包含测试？

步骤 2：高层次审查

架构和设计：

• 方法是否合理？
• 是否与现有模式一致？
• 是否有更简单的替代方案？
• 代码是否放在正确的位置？

代码组织：

• 关注点是否分离清晰？
• 抽象层次是否恰当？
• 文件/文件夹结构是否合理？

步骤 3：详细代码审查

命名：

• 变量：描述性强、有意义的名称
• 函数：基于动词、目的清晰
• 类：基于名词、单一职责
• 常量：真正的常量使用大写字母
• 避免使用缩写，除非广为人知

函数：

• 单一职责
• 长度合理（理想情况下 < 50 行）
• 输入和输出清晰
• 最小化副作用
• 适当的错误处理

类和对象：

• 单一职责原则
• 开闭原则
• 里氏替换原则
• 接口隔离原则
• 依赖倒置原则

错误处理：

• 所有错误都被捕获并处理
• 有意义的错误信息
• 适当的日志记录
• 没有静默失败
• 对界面用户友好的错误信息

代码质量：

• 无代码重复
• 无死代码
• 无注释掉的代码
• 无魔法数字
• 格式一致

步骤 4：安全审查

输入验证：

• 所有用户输入均已验证
• 类型检查
• 范围检查
• 格式验证

认证与授权：

• 适当的认证检查
• 对敏感操作进行授权
• 会话管理
• 密码处理（哈希、加盐）

数据保护：

• 无硬编码密钥
• 敏感数据加密
• 防止 SQL 注入
• 防止跨站脚本攻击
• 跨站请求伪造保护

依赖项：

• 无易受攻击的包
• 依赖项版本最新
• 最小化依赖项使用

步骤 5：性能审查

算法：

• 算法选择恰当
• 时间复杂度合理
• 空间复杂度合理
• 无不必要的循环

数据库：

• 查询效率高
• 索引使用恰当
• 防止 N+1 查询
• 使用连接池

缓存：

• 缓存策略恰当
• 缓存失效已处理
• 内存使用合理

资源管理：

• 文件正确关闭
• 连接已释放
• 防止内存泄漏

步骤 6：测试审查

测试覆盖率：

• 为新代码编写了单元测试
• 必要时编写了集成测试
• 覆盖了边缘情况
• 测试了错误情况

测试质量：

• 测试可读
• 测试可维护
• 测试是确定性的
• 无测试间依赖
• 测试数据设置/清理恰当

测试命名：

# 好
def test_user_creation_with_valid_data_succeeds():
    pass

# 差
def test1():
    pass

步骤 7：文档审查

代码注释：

• 复杂逻辑已解释
• 无显而易见的注释
• 待办事项有关联工单
• 注释准确

函数文档：

def calculate_total(items: List[Item], tax_rate: float) -> Decimal:
    """
    计算包含税费的总价。

    参数:
        items: 需要计算总价的商品列表
        tax_rate: 用小数表示的税率（例如 0.1 表示 10%）

    返回:
        包含税费的总价

    抛出:
        ValueError: 如果税率为负数
    """
    pass

README/文档：

• 必要时更新了 README
• 更新了 API 文档
• 如果有破坏性变更，提供了迁移指南

步骤 8：提供反馈

要有建设性：

✅ 好：
“考虑将此逻辑提取到一个单独的函数中，以提高可测试性和可重用性：

def validate_email(email: str) -> bool:
    return '@' in email and '.' in email.split('@')[1]

这将使其更易于测试并在整个代码库中重用。”

❌ 差：
“这是错的。重写它。”

要具体：

✅ 好：
“在第 45 行，这个查询可能导致 N+1 问题。考虑使用
.select_related('author') 在单个查询中获取相关对象。”

❌ 差：
“这里有性能问题。”

对问题排序：

• 🔴 严重：安全、数据丢失、主要错误
• 🟡 重要：性能、可维护性
• 🟢 可有可无：风格、微小改进

肯定好的工作：

“这里很好地运用了策略模式！这使得将来添加新的支付方式变得容易。”

审查清单

功能性

• 代码完成了它应该完成的功能
• 处理了边缘情况
• 处理了错误情况
• 无明显错误

代码质量

• 命名清晰、描述性强
• 函数短小且专注
• 无代码重复
• 与代码库风格一致
• 无代码坏味

安全性

• 输入验证
• 无硬编码密钥
• 认证/授权
• 无 SQL 注入漏洞
• 无跨站脚本攻击漏洞

性能

• 无明显瓶颈
• 算法高效
• 数据库查询得当
• 资源管理得当

测试

• 包含测试
• 测试覆盖率高
• 测试可维护
• 测试了边缘情况

文档

• 代码是自文档化的
• 必要处有注释
• 文档已更新
• 记录了破坏性变更

常见问题

反模式

上帝类：

# 差：一个类做所有事
class UserManager:
    def create_user(self): pass
    def send_email(self): pass
    def process_payment(self): pass
    def generate_report(self): pass

魔法数字：

# 差
if user.age > 18:
    pass

# 好
MINIMUM_AGE = 18
if user.age > MINIMUM_AGE:
    pass

深层嵌套：

# 差
if condition1:
    if condition2:
        if condition3:
            if condition4:
                # 深度嵌套的代码

# 好（提前返回）
if not condition1:
    return
if not condition2:
    return
if not condition3:
    return
if not condition4:
    return
# 扁平代码

安全漏洞

SQL 注入：

# 差
query = f"SELECT * FROM users WHERE id = {user_id}"

# 好
query = "SELECT * FROM users WHERE id = %s"
cursor.execute(query, (user_id,))

跨站脚本攻击：

// 差
element.innerHTML = userInput;

// 好
element.textContent = userInput;

硬编码密钥：

# 差
API_KEY = "sk-1234567890abcdef"

# 好
API_KEY = os.environ.get("API_KEY")

最佳实践

1. 及时审查：不要让作者久等
2. 保持尊重：关注代码而非个人
3. 解释原因：不要只说哪里错了
4. 建议替代方案：展示更好的方法
5. 使用示例：代码示例能澄清反馈
6. 把握重点：关注重要问题
7. 肯定好的工作：正面反馈也很重要
8. 先审查自己的代码：发现明显问题
9. 使用自动化工具：让工具捕捉风格问题
10. 保持一致：对所有代码应用相同标准

要使用的工具

代码检查器：

• Python: pylint, flake8, black
• JavaScript: eslint, prettier
• Go: golint, gofmt
• Rust: clippy, rustfmt

安全：

• Bandit (Python)
• npm audit (Node.js)
• OWASP Dependency-Check

代码质量：

• SonarQube
• CodeClimate
• Codacy

参考资料

• Google 代码审查指南
• OWASP Top 10
• 代码整洁之道 by 罗伯特·C·马丁

示例

示例 1：基本用法

示例 2：高级用法