name: 代码审查 description: 系统地评审拉取请求、功能实现和代码变更，以确保质量、可维护性、安全性和遵循最佳实践。当在合并前审查代码、进行同行评审、执行自我审查、审计代码质量、检查安全漏洞、确保一致的编码标准、验证测试覆盖、评估性能影响、评估架构决策，或提供建设性反馈以改善团队代码质量时使用。

代码审查 - 系统性代码质量分析

何时使用此技能

在合并到主分支前评审拉取请求
为团队成员进行同行代码评审
在提交代码评审前进行自我审查
审计代码质量和标准合规性
检查安全漏洞和不良实践
验证足够的测试覆盖存在
评估变更的性能影响
评估架构和设计决策
确保与项目编码标准一致
提供建设性反馈以改善代码质量
评审关键业务逻辑或敏感操作
检查常见的反模式和代码异味

核心原则

首先理解评审内容 - 在提出更改建议前，确保理解意图和上下文
具体且可操作 - 指向具体行并提供具体建议
平衡优点与改进 - 承认良好模式同时提出增强建议
关注影响 - 优先处理关键问题（安全性、正确性）而非风格偏好
教育而非仅纠正 - 解释为什么更改重要

评审清单

1. 正确性与逻辑

✓ 代码是否如声称的那样工作？
✓ 是否处理了边界情况（空值、空集、边界值）？
✓ 是否存在潜在的竞争条件或时序问题？
✓ 错误处理是否适当和完整？
✓ 假设是否已验证？

2. 安全性

✓ 对所有用户提供的数据进行输入验证？
✓ SQL注入、XSS、CSRF保护？
✓ 秘密/凭证是否适当保护（环境变量，非硬编码）？
✓ 认证和授权检查？
✓ 公共端点的速率限制？

3. 性能

✓ N+1查询问题？
✓ 不必要的数据库调用或API请求？
✓ 内存泄漏（事件监听器、订阅）？
✓ 大型数据集的适当分页？
✓ 高效算法（避免O(n²)当O(n log n)可能时）？

4. 可维护性

✓ 变量/函数名称是否清晰、描述性强？
✓ 函数是否做好一件事（单一职责）？
✓ DRY - 无复制粘贴重复？
✓ 魔术数字是否替换为命名常量？
✓ 复杂逻辑是否用注释解释？

5. 测试

✓ 测试是否覆盖正常路径和错误情况？
✓ 测试是否确定性（无不稳定测试）？
✓ 边界情况是否测试？
✓ 集成点是否适当模拟/存根？
✓ 测试名称是否描述验证内容？

6. 代码风格与标准

✓ 是否与项目约定一致？
✓ 是否遵循语言习惯？
✓ 无未使用导入或死代码？
✓ 是否抛出/返回适当错误类型？
✓ TypeScript类型是否具体（非'any'）？

评审流程

步骤1：高层评审

1. 阅读PR描述和链接问题
2. 理解更改背后的“为什么”
3. 扫描文件列表 - 范围是否匹配描述？
4. 检查缺失文件（测试、迁移、文档）

步骤2：深度代码评审

1. 首先评审关键路径（安全性、数据完整性）
2. 检查测试覆盖和质量
3. 寻找架构问题
4. 评审错误处理
5. 检查性能问题

步骤3：提供反馈

格式：[严重性] 问题 - 具体建议

示例：
[关键] 第45行的SQL注入漏洞
- 使用参数化查询而非字符串拼接
- 更改：`query = f"SELECT * FROM users WHERE id = {user_id}"`
- 为：`query = "SELECT * FROM users WHERE id = ?"` 带参数

[建议] 考虑将这个50行函数拆分为更小的部分
- 行100-150可以拆分为：
  - `validateInput()` (行100-120)
  - `processData()` (行121-140)  
  - `formatOutput()` (行141-150)

反馈严重性级别

[关键] - 安全问题、数据丢失风险、功能损坏
[主要] - 性能问题、错误处理不当、逻辑错误
[次要] - 代码异味、可维护性问题、风格不一致
[建议] - 改进建议、替代方法
[表扬] - 值得突出的良好模式

示例代码评审

拉取请求：添加用户认证端点

评审评论：

[关键] 密码更改端点缺少认证（行67）

// 当前 - 无认证检查
app.post('/change-password', (req, res) => {
  const { userId, newPassword } = req.body;
  updatePassword(userId, newPassword);
});

// 应为：
app.post('/change-password', requireAuth, (req, res) => {
  // 仅允许用户更改自己的密码
  if (req.user.id !== req.body.userId) {
    return res.status(403).json({ error: '禁止访问' });
  }
  const { newPassword } = req.body;
  updatePassword(req.user.id, newPassword);
});

[主要] 密码存储前未哈希（行23）

// 永不存储明文密码
await db.users.update({ password: req.body.password }); // ❌

// 使用bcrypt或argon2
const hashedPassword = await bcrypt.hash(req.body.password, 10);
await db.users.update({ passwordHash: hashedPassword }); // ✅

[次要] 令牌过期的魔术数字（行45）

const token = jwt.sign(payload, secret, { expiresIn: 3600 }); // ❌

// 使用命名常量
const TOKEN_EXPIRY_SECONDS = 60 * 60; // 1小时
const token = jwt.sign(payload, secret, { expiresIn: TOKEN_EXPIRY_SECONDS }); // ✅

[表扬] 优秀的输入验证（行12-20） 这里的zod模式全面，包含所有必要检查。这防止了畸形数据进入数据库。

常见的反模式标志

1. 静默失败

// 坏 - 错误消失
try {
  await criticalOperation();
} catch (e) {
  console.log('哎呀'); // ❌
}

// 好 - 适当的错误处理
try {
  await criticalOperation();
} catch (e) {
  logger.error('关键操作失败', { error: e, context: {...} });
  throw new CriticalOperationError('处理失败', { cause: e });
}

2. 回调地狱 / 末日金字塔

// 坏
getData((data) => {
  processData(data, (result) => {
    saveResult(result, (saved) => {
      // 3+层深 ❌
    });
  });
});

// 好 - 使用async/await
const data = await getData();
const result = await processData(data);
const saved = await saveResult(result);

3. 上帝函数

// 坏 - 函数做太多事
function handleUserRequest(req) {
  // 200行验证、处理、格式化、保存 ❌
}

// 好 - 拆分职责
function handleUserRequest(req) {
  const validated = validateRequest(req);
  const processed = processUserData(validated);
  const formatted = formatResponse(processed);
  return saveAndRespond(formatted);
}

何时阻止与批准并评论

阻止合并（请求更改）：

安全漏洞
数据丢失风险
功能损坏
缺少关键测试
主要性能问题

批准并评论：

风格改进
重构建议
次要性能优化
文档增强
改进测试建议

自动检查

手动评审前，确保自动检查通过：

✓ 代码检查（ESLint、Pylint等）
✓ 类型检查（TypeScript、mypy）
✓ 单元测试通过
✓ 集成测试通过
✓ 代码覆盖达到阈值
✓ 安全扫描（SAST）
✓ 依赖漏洞扫描

评审响应模板

## 总结
[对PR的高层评估]

## 关键问题
- [列出阻塞问题]

## 主要担忧  
- [列出重要但非阻塞问题]

## 建议
- [列出改进建议]

## 积极亮点
- [指出良好模式]

## 问题
- [关于意图或方法的澄清问题]

## 批准状态
- [ ] 批准 - 准备合并
- [ ] 批准并附小评论
- [ ] 请求更改 - 阻塞问题需解决

资源

记住：目标是提高代码质量，同时保持团队士气。要彻底但尊重，具体但不迂腐，始终解释建议背后的“为什么”。