code-review-checklist

代码审查清单

概述

提供一份系统化的清单，用于开展全面的代码审查。这项技能可帮助审查者确保代码质量、发现缺陷、识别安全问题，并保持整个代码库的一致性。

何时使用这项技能

• 在审查拉取请求时使用
• 在进行代码审计时使用
• 在为团队建立代码审查标准时使用
• 在培训新开发者了解代码审查实践时使用
• 在你希望确保审查中不遗漏任何内容时使用
• 在创建代码审查文档时使用

工作原理

第 1 步：理解背景

在审查代码之前，我会帮助你理解：

• 这段代码解决了什么问题？
• 需求是什么？
• 更改了哪些文件，为什么更改？
• 是否有相关的 issue 或工单？
• 测试策略是什么？

第 2 步：审查功能

检查代码是否正确工作：

• 它是否解决了所述问题？
• 是否处理了边界情况？
• 错误处理是否恰当？
• 是否存在任何逻辑错误？
• 它是否符合需求？

第 3 步：审查代码质量

评估代码可维护性：

• 代码是否易读且清晰？
• 命名是否具有描述性？
• 结构是否合理？
• 函数/方法是否职责聚焦？
• 是否存在不必要的复杂性？

第 4 步：审查安全性

检查安全问题：

• 输入是否经过验证？
• 敏感数据是否受到保护？
• 是否存在 SQL 注入风险？
• 认证/授权是否正确？
• 依赖项是否安全？

第 5 步：审查性能

查找性能问题：

• 是否存在不必要的循环？
• 数据库访问是否经过优化？
• 是否存在内存泄漏？
• 缓存使用是否恰当？
• 是否存在 N+1 查询问题？

第 6 步：审查测试

验证测试覆盖率：

• 是否为新代码编写了测试？
• 测试是否覆盖边界情况？
• 测试是否有意义？
• 所有测试是否通过？
• 测试覆盖率是否足够？

示例

示例 1：功能审查清单

## Functionality Review

### Requirements
- [ ] Code solves the stated problem
- [ ] All acceptance criteria are met
- [ ] Edge cases are handled
- [ ] Error cases are handled
- [ ] User input is validated

### Logic
- [ ] No logical errors or bugs
- [ ] Conditions are correct (no off-by-one errors)
- [ ] Loops terminate correctly
- [ ] Recursion has proper base cases
- [ ] State management is correct

### Error Handling
- [ ] Errors are caught appropriately
- [ ] Error messages are clear and helpful
- [ ] Errors don't expose sensitive information
- [ ] Failed operations are rolled back
- [ ] Logging is appropriate

### Example Issues to Catch:

**❌ Bad - Missing validation:**
\`\`\`javascript
function createUser(email, password) {
  // No validation!
  return db.users.create({ email, password });
}
\`\`\`

**✅ Good - Proper validation:**
\`\`\`javascript
function createUser(email, password) {
  if (!email || !isValidEmail(email)) {
    throw new Error('Invalid email address');
  }
  if (!password || password.length < 8) {
    throw new Error('Password must be at least 8 characters');
  }
  return db.users.create({ email, password });
}
\`\`\`

示例 2：安全审查清单

## Security Review

### Input Validation
- [ ] All user inputs are validated
- [ ] SQL injection is prevented (use parameterized queries)
- [ ] XSS is prevented (escape output)
- [ ] CSRF protection is in place
- [ ] File uploads are validated (type, size, content)

### Authentication & Authorization
- [ ] Authentication is required where needed
- [ ] Authorization checks are present
- [ ] Passwords are hashed (never stored plain text)
- [ ] Sessions are managed securely
- [ ] Tokens expire appropriately

### Data Protection
- [ ] Sensitive data is encrypted
- [ ] API keys are not hardcoded
- [ ] Environment variables are used for secrets
- [ ] Personal data follows privacy regulations
- [ ] Database credentials are secure

### Dependencies
- [ ] No known vulnerable dependencies
- [ ] Dependencies are up to date
- [ ] Unnecessary dependencies are removed
- [ ] Dependency versions are pinned

### Example Issues to Catch:

**❌ Bad - SQL injection risk:**
\`\`\`javascript
const query = \`SELECT * FROM users WHERE email = '\${email}'\`;
db.query(query);
\`\`\`

**✅ Good - Parameterized query:**
\`\`\`javascript
const query = 'SELECT * FROM users WHERE email = $1';
db.query(query, [email]);
\`\`\`

**❌ Bad - Hardcoded secret:**
\`\`\`javascript
const API_KEY = 'sk_live_abc123xyz';
\`\`\`

**✅ Good - Environment variable:**
\`\`\`javascript
const API_KEY = process.env.API_KEY;
if (!API_KEY) {
  throw new Error('API_KEY environment variable is required');
}
\`\`\`

示例 3：代码质量审查清单

## Code Quality Review

### Readability
- [ ] Code is easy to understand
- [ ] Variable names are descriptive
- [ ] Function names explain what they do
- [ ] Complex logic has comments
- [ ] Magic numbers are replaced with constants

### Structure
- [ ] Functions are small and focused
- [ ] Code follows DRY principle (Don't Repeat Yourself)
- [ ] Proper separation of concerns
- [ ] Consistent code style
- [ ] No dead code or commented-out code

### Maintainability
- [ ] Code is modular and reusable
- [ ] Dependencies are minimal
- [ ] Changes are backwards compatible
- [ ] Breaking changes are documented
- [ ] Technical debt is noted

### Example Issues to Catch:

**❌ Bad - Unclear naming:**
\`\`\`javascript
function calc(a, b, c) {
  return a * b + c;
}
\`\`\`

**✅ Good - Descriptive naming:**
\`\`\`javascript
function calculateTotalPrice(quantity, unitPrice, tax) {
  return quantity * unitPrice + tax;
}
\`\`\`

**❌ Bad - Function doing too much:**
\`\`\`javascript
function processOrder(order) {
  // Validate order
  if (!order.items) throw new Error('No items');
  
  // Calculate total
  let total = 0;
  for (let item of order.items) {
    total += item.price * item.quantity;
  }
  
  // Apply discount
  if (order.coupon) {
    total *= 0.9;
  }
  
  // Process payment
  const payment = stripe.charge(total);
  
  // Send email
  sendEmail(order.email, 'Order confirmed');
  
  // Update inventory
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

**✅ Good - Separated concerns:**
\`\`\`javascript
function processOrder(order) {
  validateOrder(order);
  const total = calculateOrderTotal(order);
  const payment = processPayment(total);
  sendOrderConfirmation(order.email);
  updateInventory(order.items);
  
  return { orderId: order.id, total };
}
\`\`\`

最佳实践

✅ 应该这样做

• 审查小型变更 - 较小的 PR 更容易进行彻底审查
• 先检查测试 - 验证测试是否通过并覆盖新代码
• 运行代码 - 尽可能在本地进行测试
• 提出问题 - 不要假设，要请求澄清
• 建设性反馈 - 提出改进建议，而不只是批评
• 关注重要问题 - 不要纠结于细枝末节的风格问题
• 使用自动化工具 - Linter、格式化工具、安全扫描器
• 审查文档 - 检查文档是否已更新
• 考虑性能 - 思考规模和效率
• 检查回归 - 确保现有功能仍然正常

❌ 不要这样做

• 不要未阅读就批准 - 要真正审查代码
• 不要含糊其辞 - 提供具体反馈并附带示例
• 不要忽视安全 - 安全问题至关重要
• 不要跳过测试 - 未经测试的代码会引发问题
• 不要无礼 - 要保持尊重和专业
• 不要橡皮图章式批准 - 每次审查都应创造价值
• 不要在疲惫时审查 - 否则你会遗漏重要问题
• 不要忘记背景 - 理解整体情况

完整审查清单

审查前

• 阅读 PR 描述和关联的 issue
• 理解正在解决的问题是什么
• 检查 CI/CD 中的测试是否通过
• 拉取分支并在本地运行

功能

• 代码解决了所述问题
• 边界情况已处理
• 错误处理恰当
• 用户输入已验证
• 没有逻辑错误

安全

• 没有 SQL 注入漏洞
• 没有 XSS 漏洞
• 认证/授权正确
• 敏感数据受到保护
• 没有硬编码密钥

性能

• 没有不必要的数据库查询
• 没有 N+1 查询问题
• 使用了高效算法
• 没有内存泄漏
• 缓存使用恰当

代码质量

• 代码可读且清晰
• 命名具有描述性
• 函数职责单一且短小
• 没有代码重复
• 遵循项目约定

测试

• 新代码有测试
• 测试覆盖边界情况
• 测试有意义
• 所有测试通过
• 测试覆盖率足够

文档

• 代码注释解释为什么，而不是做什么
• API 文档已更新
• 如有需要，README 已更新
• 破坏性变更已记录
• 如有需要，提供迁移指南

Git

• 提交信息清晰
• 没有合并冲突
• 分支与 main 保持同步
• 没有提交不必要的文件
• .gitignore 配置正确

常见陷阱

问题：缺少边界情况

症状： 代码在正常路径下可以工作，但在边界情况下失败 解决方案： 提出“如果……会怎样？”的问题

• 如果输入为 null 会怎样？
• 如果数组为空会怎样？
• 如果用户未认证会怎样？
• 如果网络请求失败会怎样？

问题：安全漏洞

症状： 代码暴露安全风险 解决方案： 使用安全检查清单

• 运行安全扫描器（npm audit、Snyk）
• 检查 OWASP Top 10
• 验证所有输入
• 使用参数化查询
• 永远不要信任用户输入

问题：测试覆盖不足

症状： 新代码没有测试或测试不充分 解决方案： 要求所有新代码都有测试

• 函数的单元测试
• 功能的集成测试
• 边界情况测试
• 错误情况测试

问题：代码不清晰

症状： 审查者无法理解代码在做什么 解决方案： 请求改进

• 更好的变量名
• 解释性注释
• 更小的函数
• 清晰的结构

审查评论模板

请求更改

**Issue:** [Describe the problem]

**Current code:**
\`\`\`javascript
// Show problematic code
\`\`\`

**Suggested fix:**
\`\`\`javascript
// Show improved code
\`\`\`

**Why:** [Explain why this is better]

提问

**Question:** [Your question]

**Context:** [Why you're asking]

**Suggestion:** [If you have one]

表扬优秀代码

**Nice!** [What you liked]

This is great because [explain why]

相关技能

• @requesting-code-review - 准备代码以供审查
• @receiving-code-review - 处理审查反馈
• @systematic-debugging - 调试审查中发现的问题
• @test-driven-development - 确保代码有测试

其他资源

• Google 代码审查指南
• OWASP Top 10
• 代码审查最佳实践
• 如何审查代码

---

专业提示： 每次审查都使用清单模板，以确保一致性和全面性。根据团队的具体需求进行定制！