code-review-checklist-zh：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
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描述和关联问题
• 了解要解决的问题是什么
• 检查CI/CD中的测试是否通过
• 拉取分支并在本地运行

功能

• 代码解决了所陈述的问题
• 边界情况得到处理
• 错误处理恰当
• 用户输入经过验证
• 无逻辑错误

安全

• 无SQL注入漏洞
• 无XSS漏洞
• 身份验证/授权正确
• 敏感数据受到保护
• 无硬编码密钥

性能

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

代码质量

• 代码可读且清晰
• 命名具有描述性
• 函数聚焦且短小
• 无代码重复
• 遵循项目约定

测试

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

文档

• 代码注释解释原因，而非内容
• API文档已更新
• README已在必要时更新
• 破坏性变更已文档化
• 必要时提供迁移指南

Git

• 提交信息清晰
• 无合并冲突
• 分支与主分支保持同步
• 未提交不必要的文件
• .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 Code Review Guidelines
• OWASP Top 10
• Code Review Best Practices
• How to Review Code

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