跳到主要内容

审查清单与标准

系统化的审查清单能够帮助审查者全面、高效地检查代码。本章提供多层次的审查清单,涵盖不同维度的检查要点。

通用审查清单

功能正确性

核心功能验证

  • 代码实现了需求文档中描述的功能
  • 所有业务逻辑分支都被正确处理
  • 边界条件和异常情况有适当的处理
  • 输入数据的验证和清理充分
  • 输出结果符合预期格式

错误处理

  • 潜在的错误场景被识别并处理
  • 错误信息清晰且对用户友好
  • 异常被正确捕获和处理,不会导致系统崩溃
  • 资源在异常情况下也能正确释放

并发安全

  • 共享资源的访问是线程安全的
  • 锁的粒度适当,不会导致性能瓶颈
  • 不存在死锁风险
  • 原子操作的使用正确

代码质量

命名规范

  • 变量名准确表达其用途和含义
  • 函数名清晰描述其功能
  • 类名符合其职责和领域概念
  • 常量名使用大写和下划线
  • 避免使用缩写,除非广为人知

代码结构

  • 函数长度适中(建议不超过 50 行)
  • 类的职责单一,符合单一职责原则
  • 代码层次清晰,避免深层嵌套(建议不超过 3 层)
  • 重复代码被提取为公共函数或组件

可读性

  • 代码逻辑清晰,易于理解
  • 复杂的算法有适当的注释说明
  • 魔法数字被提取为命名常量
  • 代码组织符合项目的结构约定

设计合理性

架构合规

  • 代码符合项目的架构设计
  • 分层清晰,职责边界明确
  • 模块间的依赖关系合理
  • 接口设计符合开闭原则

设计原则

  • 遵循 SOLID 原则
  • 避免过度设计,保持简单
  • 设计模式的使用恰当
  • 代码易于扩展和维护

安全性

输入验证

  • 所有外部输入都经过验证
  • SQL 注入风险被消除(使用参数化查询)
  • XSS 攻击被防范(输出编码)
  • 文件上传有类型和大小限制

认证授权

  • 敏感操作需要身份验证
  • 权限控制正确实现
  • 会话管理安全
  • 密码等敏感信息加密存储

数据安全

  • 敏感数据不在日志中明文输出
  • 敏感数据不在 URL 中传输
  • 加密算法和密钥管理正确
  • 个人隐私数据处理符合法规

性能

算法效率

  • 算法复杂度合理
  • 不存在明显的性能瓶颈
  • 大数据量处理有分页或流式处理
  • 循环中没有重复计算

资源使用

  • 数据库连接正确释放
  • 文件句柄正确关闭
  • 内存使用合理,无内存泄漏
  • 缓存使用得当

测试

测试覆盖

  • 核心逻辑有单元测试覆盖
  • 边界条件有测试用例
  • 错误场景有测试覆盖
  • 测试覆盖率符合团队标准

测试质量

  • 测试用例独立,不相互依赖
  • 测试数据准备和清理完善
  • 测试命名清晰,描述测试意图
  • 测试运行速度快

语言特定检查清单

Java 审查要点

语言特性使用

  • 使用 Optional 处理可能为空的情况
  • 优先使用 Stream API 处理集合操作
  • 使用 try-with-resources 确保资源释放
  • 字符串拼接使用 StringBuilder(循环中)

集合使用

  • 选择合适的集合类型
  • 初始化时指定合适的容量
  • 使用不可变集合保护数据
  • 并发场景使用线程安全集合

异常处理

  • 使用具体的异常类型,避免捕获 Exception
  • 不要忽略异常(空的 catch 块)
  • 自定义异常提供有意义的错误信息
  • 异常链保持完整

示例

// 不好的实践
try {
processFile(file);
} catch (Exception e) {
// 忽略异常
}

// 好的实践
try {
processFile(file);
} catch (IOException e) {
logger.error("Failed to process file: {}", file.getName(), e);
throw new BusinessException("文件处理失败", e);
}

Python 审查要点

Pythonic 写法

  • 使用列表推导式替代简单循环
  • 使用生成器处理大数据
  • 使用 with 语句管理资源
  • 遵循 PEP 8 规范

类型提示

  • 函数参数有类型注解
  • 返回值有类型注解
  • 复杂数据结构使用 TypedDictNamedTuple
  • 泛型使用正确

异常处理

  • 捕获具体的异常类型
  • 使用 try/except/else/finally 结构
  • 异常信息包含有用的上下文
  • 不要滥用异常控制流程

示例

# 不好的实践
def get_user(user_id):
try:
return db.query(user_id)
except:
return None

# 好的实践
from typing import Optional

def get_user(user_id: int) -> Optional[User]:
"""根据用户ID获取用户信息。

Args:
user_id: 用户ID

Returns:
用户对象,如果不存在则返回 None
"""
try:
return db.query(User).filter_by(id=user_id).first()
except DatabaseError as e:
logger.error(f"Failed to query user {user_id}: {e}")
raise UserServiceError(f"查询用户失败") from e

JavaScript/TypeScript 审查要点

异步处理

  • 使用 async/await 替代回调
  • 正确处理 Promise 错误
  • 避免回调地狱
  • 异步操作有超时处理

类型安全(TypeScript)

  • 避免使用 any 类型
  • 接口定义清晰完整
  • 泛型使用合理
  • 严格模式开启

现代特性

  • 使用解构赋值
  • 使用模板字符串
  • 使用箭头函数(适当场景)
  • 使用模块化导入导出

示例

// 不好的实践
function fetchUserData(userId, callback) {
fetch('/api/users/' + userId)
.then(function(response) {
return response.json();
})
.then(function(data) {
callback(null, data);
})
.catch(function(error) {
callback(error);
});
}

// 好的实践
interface User {
id: number;
name: string;
email: string;
}

async function fetchUserData(userId: number): Promise<User> {
try {
const response = await fetch(`/api/users/${userId}`);
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
return await response.json();
} catch (error) {
logger.error(`Failed to fetch user ${userId}:`, error);
throw new UserFetchError('获取用户数据失败', { cause: error });
}
}

Go 审查要点

错误处理

  • 错误被正确处理,不忽略
  • 错误信息包含有用的上下文
  • 使用自定义错误类型传递额外信息
  • 错误链保持完整

并发安全

  • Channel 使用正确
  • Goroutine 泄漏被避免
  • 共享变量有适当的同步机制
  • Context 用于取消和超时控制

示例

// 不好的实践
func process(data []byte) error {
result, err := doWork(data)
if err != nil {
return err
}
return saveResult(result)
}

// 好的实践
func process(ctx context.Context, data []byte) error {
result, err := doWork(ctx, data)
if err != nil {
return fmt.Errorf("doWork failed: %w", err)
}

// 使用 timeout 避免长时间阻塞
saveCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()

if err := saveResult(saveCtx, result); err != nil {
return fmt.Errorf("saveResult failed: %w", err)
}
return nil
}

Rust 审查要点

所有权系统

  • 借用检查规则正确使用
  • 生命周期注解必要且正确
  • 避免不必要的克隆
  • 使用适当的智能指针

错误处理

  • 使用 Result 处理可恢复错误
  • 使用 Option 处理可能为空的值
  • ? 操作符正确使用
  • 自定义错误类型提供有用信息

并发安全

  • SendSync trait 使用正确
  • MutexRwLock 等同步原语使用恰当
  • 避免数据竞争
  • Channel 用于跨线程通信

示例

// 不好的实践
fn process(data: &mut Vec<u32>) -> Vec<u32> {
let mut result = Vec::new();
for item in data.iter() {
if *item > 10 {
result.push(*item);
}
}
result
}

// 好的实践
fn process(data: &[u32]) -> Vec<u32> {
data.iter()
.filter(|&&x| x > 10)
.copied()
.collect()
}

// 使用自定义错误类型
#[derive(Debug)]
enum ProcessError {
InvalidInput(String),
ComputationFailed,
}

impl std::error::Error for ProcessError {}

impl std::fmt::Display for ProcessError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ProcessError::InvalidInput(msg) => write!(f, "Invalid input: {}", msg),
ProcessError::ComputationFailed => write!(f, "Computation failed"),
}
}
}

安全专项检查清单

SQL 注入防护

  • 所有数据库查询使用参数化查询或 ORM
  • 用户输入不直接拼接到 SQL 语句
  • 动态表名/列名有白名单验证
  • ORM 使用正确,避免 N+1 查询问题

示例对比

// 不安全
String query = "SELECT * FROM users WHERE id = " + userId;
stmt.executeQuery(query);

// 安全
PreparedStatement ps = conn.prepareStatement(
"SELECT * FROM users WHERE id = ?"
);
ps.setInt(1, userId);

XSS 防护

  • 输出到 HTML 时进行编码
  • 使用内容安全策略(CSP)
  • 富文本内容使用白名单过滤
  • HTTPOnly 和 Secure 标记正确设置
// React 默认会转义内容,这是安全的
return <div>{userInput}</div>;

// 需要渲染 HTML 时使用 DOMPurify
import DOMPurify from 'dompurify';
const sanitized = DOMPurify.sanitize(userHtml);
return <div dangerouslySetInnerHTML={{ __html: sanitized }} />;

CSRF 防护

  • 敏感操作使用 CSRF Token
  • SameSite Cookie 正确设置
  • CORS 配置正确
  • 验证请求来源

认证与授权

  • 密码使用强哈希算法(bcrypt、Argon2)
  • Session ID 足够随机且安全存储
  • 权限检查在服务端执行
  • 最小权限原则遵循

性能专项检查清单

数据库

  • 必要的索引已创建
  • 查询使用 EXPLAIN 分析
  • 避免 SELECT *,只查询需要的字段
  • 批量操作替代循环单条操作
  • 分页处理大数据量
-- 不好的查询
SELECT * FROM orders, order_items
WHERE orders.id = order_items.order_id;

-- 好的查询
SELECT o.id, o.created_at, o.total
FROM orders o
WHERE o.created_at > '2024-01-01'
LIMIT 100;

缓存

  • 缓存键命名规范
  • 缓存过期时间设置合理
  • 缓存穿透有防护(空值缓存、布隆过滤器)
  • 缓存雪崩有应对策略
  • 缓存击穿有保护机制

API 设计

  • 响应数据精简
  • 压缩传输内容(Gzip)
  • 批量接口减少请求次数
  • 分页接口限制返回数量
  • 异步处理耗时操作

PR 审查关注点

小型 PR(< 100 行)

  • 快速浏览整体实现
  • 验证核心逻辑
  • 检查边界情况
  • 确认测试覆盖

中型 PR(100-400 行)

  • 理解变更背景和目的
  • 逐文件审查实现细节
  • 评估设计合理性
  • 检查测试质量
  • 关注安全性和性能

大型 PR(> 400 行)

  • 分模块逐步审查
  • 必要时拆分审查任务
  • 重点关注架构影响
  • 进行集成测试
  • 考虑向后兼容性

审查决策指南

必须阻止合并的情况

  1. 存在安全漏洞
  2. 功能实现与需求不符
  3. 关键路径缺少测试
  4. 破坏现有功能
  5. 代码无法编译或测试失败

可以批准的情况

  1. 所有阻塞性问题已解决
  2. 建议性问题有记录或计划解决
  3. 测试覆盖充分
  4. 代码符合规范

需要讨论的情况

  1. 设计选择有多种可行方案
  2. 涉及架构决策
  3. 性能影响不明确
  4. 与现有代码风格不一致

通过使用系统化的审查清单,审查者能够更全面、更一致地评估代码质量,减少遗漏,提高审查效率。