代码评审清单

代码评审是最常见的工程实践之一。好的代码评审不只是找 bug——它传播知识、统一风格、发现设计问题、降低单人风险。坏的代码评审（过于严苛或流于形式）会拖慢团队、制造对立。

要点

代码评审不只是找 bug，还传播知识、统一风格、发现设计问题
评审者应假设善意，区分「必须改」和「建议改」
提交者应自测通过、PR 规模合理、描述清楚
重点检查：正确性、可读性、测试覆盖、安全、性能

1. 适用时机

每个 Pull Request / Merge Request
重大重构每个阶段
引入新依赖或工具
修复关键 bug 后

不需要代码评审的场景：

自动生成的代码（配置、schema、lock 文件）
紧急 hotfix（但事后必须补评审）
纯格式调整（用 linter 自动处理，不要浪费人力）

2. 评审前准备（提交者）

提交者应该在请求评审前完成自检，减少来回：

PR 标题清晰，描述说明改了什么、为什么改
PR 规模合理（建议不超过 400 行，超过应拆分）
自测通过（本地跑通、单元测试通过）
Lint 和格式化已执行（不要让人肉评审格式问题）
相关文档已更新（README、API 文档、变更日志）
依赖方已通知（如果有接口变更）

3. 核心检查项

3.1 正确性

代码真的解决了它声称要解决的问题（对照 PR 描述）
边界情况处理（空值、零值、极大值、并发）
错误处理完整（异常捕获、错误传播、用户提示）
没有引入新的 bug（回归测试覆盖）
没有硬编码（魔法数字、写死的 URL、密钥）

TypeScript 示例：边界情况处理

// 坏的代码：没有处理边界情况
function getUser(userId: string) {
  return db.users.find(userId)
}
 
// 好的代码：处理各种边界情况
async function getUser(userId: string): Promise<User | null> {
  // 校验输入
  if (!userId || typeof userId !== 'string') {
    throw new ValidationError('userId 必须是字符串')
  }
 
  // 处理空值
  const user = await db.users.find(userId)
 
  if (!user) {
    return null  // 明确返回 null，不是 undefined
  }
 
  return user
}
 
// 坏的代码：硬编码
function calculateDiscount(price: number) {
  return price * 0.1  // 魔法数字
}
 
// 好的代码：使用常量
const DISCOUNT_RATE = 0.1
 
function calculateDiscount(price: number) {
  if (price < 0) {
    throw new ValidationError('价格不能为负')
  }
  return price * DISCOUNT_RATE
}

3.2 可读性

命名清晰（变量名、函数名、类名能表达意图）
函数职责单一（一个函数做一件事，不超过 50 行）
代码结构清晰（逻辑顺序、层次分明）
必要的注释有（解释 why，不是复述 what）
复杂逻辑有拆解或详细注释

TypeScript 示例：命名与函数拆分

// 坏的命名
function processData(data: any) {
  const x = data.filter(...)
  const y = x.map(...)
  return y
}
 
// 好的命名
function filterActiveUsers(users: User[]): User[] {
  return users.filter(user => user.status === 'active')
}
 
function formatUserDisplayName(user: User): string {
  return `${user.firstName} ${user.lastName}`
}
 
// 坏的函数：一个函数做太多事
async function createOrder(input: CreateOrderInput) {
  // 校验输入
  if (!input.userId) throw new Error('userId is required')
  if (!input.items.length) throw new Error('items is required')
 
  // 检查库存
  for (const item of input.items) {
    const stock = await db.inventory.find(item.productId)
    if (stock.quantity < item.quantity) {
      throw new Error('库存不足')
    }
  }
 
  // 创建订单
  const order = await db.orders.create({
    userId: input.userId,
    items: input.items,
    total: input.items.reduce((sum, item) => sum + item.price * item.quantity, 0)
  })
 
  // 扣减库存
  for (const item of input.items) {
    await db.inventory.update(item.productId, {
      quantity: stock.quantity - item.quantity
    })
  }
 
  // 发送通知
  await sendEmail(input.userId, '订单创建成功')
 
  return order
}
 
// 好的函数：拆分成小函数
async function createOrder(input: CreateOrderInput): Promise<Order> {
  validateCreateOrderInput(input)
  await checkInventory(input.items)
 
  const order = await db.transaction(async (tx) => {
    const order = await createOrderRecord(tx, input)
    await deductInventory(tx, input.items)
    return order
  })
 
  await notifyOrderCreated(input.userId, order.id)
 
  return order
}
 
function validateCreateOrderInput(input: CreateOrderInput): void {
  if (!input.userId) {
    throw new ValidationError('userId is required')
  }
  if (!input.items.length) {
    throw new ValidationError('items is required')
  }
}
 
async function checkInventory(items: OrderItem[]): Promise<void> {
  for (const item of items) {
    const stock = await db.inventory.find(item.productId)
    if (!stock || stock.quantity < item.quantity) {
      throw new InsufficientStockError(item.productId)
    }
  }
}

3.3 设计与抽象

没有过度设计（不要为「未来可能需要」而抽象）
没有欠设计（不要把逻辑全堆在一起）
复用合理（已有的工具函数/组件用上了吗？）
依赖方向正确（上层不依赖下层细节）
模块边界清楚（跨模块调用是否合理？）

TypeScript 示例：避免过度设计

// 过度设计：为「未来可能需要」而抽象
interface DataFetcher<T> {
  fetch(id: string): Promise<T>
}
 
class UserFetcher implements DataFetcher<User> {
  async fetch(id: string): Promise<User> {
    return db.users.find(id)
  }
}
 
class UserRepository {
  private fetcher: DataFetcher<User>
 
  constructor(fetcher: DataFetcher<User>) {
    this.fetcher = fetcher
  }
 
  async findById(id: string): Promise<User | null> {
    return this.fetcher.fetch(id)
  }
}
 
// 实际上只需要：
class UserRepository {
  async findById(id: string): Promise<User | null> {
    return db.users.find(id)
  }
}
 
// 如果未来真的需要抽象，再引入也不迟
// YAGNI: You Aren't Gonna Need It

3.4 性能

没有明显的性能问题（循环里的 IO、N+1 查询）
大数据量场景考虑（分页、流式处理、批量操作）
内存使用合理（不要一次性加载整个大文件）
缓存使用得当（该缓存的缓存了，TTL 合理）
异步处理合理（不阻塞主线程）

TypeScript 示例：N+1 查询问题

// 坏的代码：N+1 查询
async function getOrdersWithUsers() {
  const orders = await db.orders.findMany()
 
  // N+1 问题：每个订单都查询一次用户
  return orders.map(async (order) => {
    const user = await db.users.find(order.userId)  // N 次查询
    return { ...order, user }
  })
}
 
// 好的代码：批量查询
async function getOrdersWithUsers() {
  const orders = await db.orders.findMany()
 
  // 收集所有 userId
  const userIds = [...new Set(orders.map(o => o.userId))]
 
  // 一次查询所有用户
  const users = await db.users.findMany({
    where: { id: { in: userIds } }
  })
 
  // 在内存中关联
  const userMap = new Map(users.map(u => [u.id, u]))
 
  return orders.map(order => ({
    ...order,
    user: userMap.get(order.userId)
  }))
}
 
// 坏的代码：循环里的 IO
async function sendNotifications(userIds: string[]) {
  for (const userId of userIds) {
    await sendEmail(userId, '通知内容')  // 串行发送，慢
  }
}
 
// 好的代码：批量处理
async function sendNotifications(userIds: string[]) {
  // 并发发送，但要控制并发数
  const batchSize = 10
  for (let i = 0; i < userIds.length; i += batchSize) {
    const batch = userIds.slice(i, i + batchSize)
    await Promise.all(batch.map(userId => sendEmail(userId, '通知内容')))
  }
}

3.5 安全

输入校验到位（长度、类型、格式、范围）
没有 SQL 注入、XSS、命令注入风险
敏感数据不打印日志、不暴露给前端
认证和授权检查正确
依赖库版本无已知漏洞

TypeScript 示例：安全检查

// 坏的代码：SQL 注入风险
app.get('/users', async (c) => {
  const name = c.req.query('name')
  const users = await db.execute(`SELECT * FROM users WHERE name = '${name}'`)  // 危险！
  return c.json(users)
})
 
// 好的代码：参数化查询
app.get('/users', async (c) => {
  const name = c.req.query('name')
 
  // 使用 ORM 或参数化查询
  const users = await db.users.findMany({
    where: { name }
  })
 
  return c.json(users)
})
 
// 坏的代码：日志泄露敏感信息
console.log('User login:', { email: user.email, password: user.password })
 
// 好的代码：脱敏日志
console.log('User login:', { email: user.email, password: '***' })
 
// 坏的代码：没有权限检查
app.delete('/users/:id', async (c) => {
  const userId = c.req.param('id')
  await db.users.delete(userId)
  return c.json({ success: true })
})
 
// 好的代码：权限检查
app.delete('/users/:id', async (c) => {
  const userId = c.req.param('id')
  const currentUser = c.get('user')
 
  // 检查权限：只能删除自己，或管理员可以删除任何人
  if (currentUser.id !== userId && currentUser.role !== 'admin') {
    return c.json({ error: '无权限' }, 403)
  }
 
  await db.users.delete(userId)
  return c.json({ success: true })
})

3.6 测试

新代码有对应的测试
测试覆盖核心逻辑和关键边界
测试命名清晰（描述测试场景和期望结果）
测试不依赖外部状态（独立、可重复）
修复 bug 的 PR 有回归测试

TypeScript 示例：测试编写

// 好的测试：描述清晰、覆盖边界
describe('calculateDiscount', () => {
  it('应该正常计算折扣', () => {
    expect(calculateDiscount(100)).toBe(10)
  })
 
  it('应该处理零价格', () => {
    expect(calculateDiscount(0)).toBe(0)
  })
 
  it('应该拒绝负数价格', () => {
    expect(() => calculateDiscount(-100)).toThrow('价格不能为负')
  })
 
  it('应该拒绝非数字输入', () => {
    expect(() => calculateDiscount('abc' as any)).toThrow('价格必须是数字')
  })
})
 
// 坏的测试：没有描述、不独立
test('test1', () => {
  // 依赖外部状态
  global.config = { discountRate: 0.1 }
  expect(calculateDiscount(100)).toBe(10)
})

3.7 可观测性

关键操作有日志（INFO 级别）
错误有详细日志（ERROR 级别，带上下文）
日志级别使用正确（DEBUG/INFO/WARN/ERROR）
日志不包含敏感信息
关键流程有 tracing（trace id 贯穿）

3.8 兼容与维护

不破坏向后兼容（除非有明确理由和迁移方案）
废弃的功能有标记和迁移说明
配置项有文档和默认值
没有引入新的技术债（或明确记录 TODO）

4. 评审者应该做的

心态

假设善意：代码背后有你不了解的约束
问问题，不要下结论：「这里为什么不用 X？」比「这里应该用 X」更合适
区分「必须改」和「建议改」：用 [nit]、[suggestion]、[question] 标签
肯定做得好的地方：不要只挑毛病

评审顺序

先看 PR 描述和上下文（为什么改？要解决什么问题？）
看测试（理解预期行为）
看主路径（核心逻辑）
看边界和错误处理
看细节（命名、注释、格式）

反馈分级

[blocking]：必须改才能合并（bug、安全问题、设计错误）
[suggestion]：建议改，不改也能合并（更好的命名、更清晰的结构）
[nit]：无关紧要的小问题（格式、措辞）
[question]：想了解背景，不一定需要改
[praise]：做得好的地方，值得鼓励

5. 评审会上容易漏的点

错误处理：问「这里失败了会怎样？用户看到什么？」
边界情况：问「如果输入是 null / 空数组 / 超大值呢？」
测试覆盖：问「这个改动有测试吗？覆盖核心路径了吗？」
日志与监控：问「出问题了怎么排查？」
向后兼容：问「老客户端/老数据还能用吗？」

6. 通过标准

代码正确实现预期功能
没有 blocking 级别的未解决问题
代码可读、可维护、符合团队风格
测试覆盖充分
安全和性能无明显问题
至少 1-2 位评审者 approve

7. 反模式

以下是代码评审中的反模式：

LGTM 党：不看代码直接 approve，评审形同虚设
吹毛求疵：纠结于无关紧要的格式问题，忽略真正的设计缺陷
自我炫耀：故意写难懂的代码显示水平
防守性编程过度：每个变量都判空、每个分支都 try-catch
评论攻击：对人不对事，制造对立

8. 维护建议

代码评审清单需要随团队实践更新：

团队风格变化时（如从 JS 切到 TS），调整相关检查项
出现代码相关的事故后，把根因对应的检查项加进来
团队规模扩大时，强化可读性和文档要求
每半年复审一次，删除过时的检查项