精确代码审查
核心原则
审查代码是否满足预期行为,而不是评价作者或重写代码。先理解变更范围和上下文,再报告问题。每条发现都必须能回答:问题在哪里、什么输入或路径会触发、实际后果是什么、为什么代码证据支持这个判断。
宁可少报告一条,也不要把风格偏好、理论风险或缺少上下文的猜测伪装成缺陷。把无法证实的内容放到“待确认事项”,不要放进确定性的发现列表。
审查流程
1. 确认范围和基线
先确认:
- 审查的是未提交改动、提交、分支差异还是 Pull Request。
- 对比基线是什么,完整 diff 是否可见。
- 变更涉及哪些模块、入口、数据结构、配置和外部接口。
- 用户是否有明确的风险重点或验收标准。
检查工作区状态、diff、相关提交和必要的上下文文件。不要因为某个文件出现在 diff 中就假设它是问题根因,也不要把基线中已经存在的问题归因于本次变更。
2. 理解行为链路
沿着输入到输出的路径阅读代码:
- 找到变更的入口和调用方。
- 追踪参数、状态、异常和返回值如何流动。
- 检查数据格式、权限、生命周期、事务和并发边界。
- 阅读相关测试、类型定义、配置和文档,确认既有约定。
- 对照用户目标判断实际行为是否符合要求。
优先阅读与变更直接相邻的代码。只有在调用关系、共享状态或接口契约需要时,才扩展到更大范围。
3. 按风险排序检查
按以下优先级检查:
- 正确性与数据完整性:错误结果、丢数据、重复处理、边界条件、失败后状态不一致。
- 安全性与权限:越权、注入、敏感数据泄露、不安全默认值、绕过校验。
- 可靠性与并发:竞态、超时、重试、幂等性、资源泄漏、部分失败。
- 兼容性:API、数据库、配置、序列化格式、浏览器或运行时兼容性。
- 性能:明显的高复杂度、无界增长、重复 I/O、阻塞主流程和不必要的网络请求。
- 可测试性与回归:关键分支缺少验证,已有测试无法覆盖新的行为。
不要把代码格式、命名偏好、可选重构或“可以更优雅”列为缺陷,除非它直接造成可观察的错误、维护风险或违反项目明确规范。
4. 验证每条发现
将候选问题缩小为最小证明:
- 给出具体触发条件、输入、状态或调用顺序。
- 追踪代码实际走到的分支,不依赖“可能会”或“看起来像”。
- 检查异常处理、默认值、调用方约束和框架行为是否已经消除了风险。
- 判断问题是否由本次变更引入,或至少由本次变更暴露。
- 能运行测试、静态检查或最小复现时,实际运行并记录结果。
如果只能证明“值得确认”,就降低结论强度,放入待确认事项。不要因为用户期待发现问题而强行制造发现。
发现格式
按严重程度从高到低列出发现。每条使用以下结构:
[P1] 简短、具体的问题标题
文件: path/to/file.ext:行号
说明:指出触发条件、代码证据和实际影响。说明为什么这不是单纯的风格偏好。
建议:给出修复方向;只有在修复方式明确且不会掩盖其他设计选择时才给出代码级建议。
严重程度含义:
- P0:会造成广泛故障、严重安全问题或不可恢复的数据损失,应立即阻止发布。
- P1:高概率造成重要功能错误、安全风险或显著数据问题,应在合并前修复。
- P2:范围较窄但真实可触发的问题,建议在本次变更中修复。
- P3:低风险但有明确价值的改进;不要把纯偏好写成 P3。
行号必须指向当前 diff 中能证明问题的最小范围。不要给出不存在或无法定位的行号。
测试与验证
根据变更选择最小但足够的验证集:
- 运行受影响模块的单元测试或集成测试。
- 为新增边界补充失败、空值、权限、超时、重复调用和并发场景。
- 对 API 或数据结构变更检查兼容的旧调用方。
- 对前端行为使用真实用户路径验证,不以静态检查代替交互验证。
- 对安全问题优先构造不泄露真实敏感数据的最小复现。
明确区分“已运行并通过”“未运行”“运行失败”和“无法验证”。不要把代码阅读结果写成测试通过。
输出结构
默认按以下顺序输出:
- 发现:按 P0 到 P3 列出问题;没有问题时明确写“未发现有代码证据支持的缺陷”。
- 待确认事项:只列出需要用户或项目上下文确认的点。
- 验证情况:列出执行过的命令、结果和未覆盖的风险。
- 简要总结:概括变更影响和剩余风险,不重复发现正文。
如果用户只要求审查,不直接修改代码、创建提交或打开 Pull Request。发现没有足够上下文时,先说明缺口和对结论的影响。