审查清单与标准
系统化的审查清单能够帮助审查者全面、高效地检查代码。本章提供多层次的审查清单,涵盖不同维度的检查要点。
通用审查清单
功能正确性
核心功能验证
- 代码实现了需求文档中描述的功能
- 所有业务逻辑分支都被正确处理
- 边界条件和异常情况有适当的处理
- 输入数据的验证和清理充分
- 输出结果符合预期格式
错误处理
- 潜在的错误场景被识别并处理
- 错误信息清晰且对用户友好
- 异常被正确捕获和处理,不会导致系统崩溃
- 资源在异常情况下也能正确释放
并发安全
- 共享资源的访问是线程安全的
- 锁的粒度适当,不会导致性能瓶颈
- 不存在死锁风险
- 原子操作的使用正确
代码质量
命名规范
- 变量名准确表达其用途和含义
- 函数名清晰描述其功能
- 类名符合其职责和领域概念
- 常量名使用大写和下划线
- 避免使用缩写,除非广为人知
代码结构
- 函数长度适中(建议不超过 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 规范
类型提示
- 函数参数有类型注解
- 返回值有类型注解
- 复杂数据结构使用
TypedDict或NamedTuple - 泛型使用正确
异常处理
- 捕获具体的异常类型
- 使用
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处理可能为空的值 -
?操作符正确使用 - 自定义错误类型提供有用信息
并发安全
-
Send和Synctrait 使用正确 -
Mutex、RwLock等同步原语使用恰当 - 避免数据竞争
- 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 行)
- 分模块逐步审查
- 必要时拆分审查任务
- 重点关注架构影响
- 进行集成测试
- 考虑向后兼容性
审查决策指南
必须阻止合并的情况
- 存在安全漏洞
- 功能实现与需求不符
- 关键路径缺少测试
- 破坏现有功能
- 代码无法编译或测试失败
可以批准的情况
- 所有阻塞性问题已解决
- 建议性问题有记录或计划解决
- 测试覆盖充分
- 代码符合规范
需要讨论的情况
- 设计选择有多种可行方案
- 涉及架构决策
- 性能影响不明确
- 与现有代码风格不一致
通过使用系统化的审查清单,审查者能够更全面、更一致地评估代码质量,减少遗漏,提高审查效率。