code-review-checklist

代码审查清单

概述

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

何时使用此技能

• 审查拉取请求时
• 开展代码审计时
• 为团队制定代码审查标准时
• 培训新开发者掌握代码审查实践时
• 希望确保审查时没有遗漏时
• 编写代码审查文档时

工作原理

步骤 1：了解背景

审查代码前，我会帮助你了解：

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

步骤 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
async 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');
  }
  const passwordHash = await hashPassword(password); // configured password-hashing library
  return db.users.create({ email, passwordHash });
}
\`\`\`

示例 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 leakedToken = '[redacted live key]';
\`\`\`

**✅ 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 };
}
\`\`\`

“分离关注点”顺序示例仅用于展示结构：实际的支付/库存/电子邮件操作需要等待失败结果、确保幂等性，并采用事务/outbox 或补偿策略。仅验证本身并不能确保密码存储安全。清单用于记录已检查、已测试或无法检查的项目；不要勾选未观察到的检查项，也不要把扫描器未报告问题当作问题不存在的证明。

最佳实践

✅ 推荐做法

• 审查小改动 - 较小的 PR 更容易全面审查
• 先检查测试 - 确认测试通过，且覆盖了新代码
• 运行代码 - 尽可能在本地进行测试
• 提出问题 - 不要自行假设，应请求澄清
• 提供建设性意见 - 提出改进建议，而不只是批评
• 关注重要问题 - 不要对细枝末节的风格问题吹毛求疵
• 使用自动化工具 - 如代码检查工具、格式化工具和安全扫描器
• 审查文档 - 检查文档是否已更新
• 考虑性能 - 思考可扩展性和效率
• 检查回归问题 - 确保现有功能仍能正常工作

❌ 不推荐做法

• 不要未读代码就批准 - 应实际审查代码
• 不要含糊其辞 - 提供具体反馈和示例
• 不要忽视安全性 - 安全问题至关重要
• 不要跳过测试 - 未经测试的代码会引发问题
• 不要无礼 - 保持尊重和专业
• 不要敷衍了事 - 每次审查都应有所助益
• 疲惫时不要审查 - 否则容易漏掉重要问题
• 不要忘记背景 - 了解整体情况

完整审查清单

审查前

• 阅读 PR 描述和关联的问题
• 了解要解决的问题
• 检查 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
• 代码审查最佳实践
• 如何审查代码

---

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

限制

• 仅当任务明确符合上述范围时，才使用此技能。
• 不要将输出视为针对特定环境的验证、测试或专家审查的替代品。
• 如果缺少必要的输入、权限、安全边界或成功标准，请暂停并请求澄清。