changan-su/forsion/code-review

代码审查

审查流程

1. 先看全貌再看细节:用 run_bash 跑 git diff(或 git diff main...HEAD)拿到完整改动;改动大时先 git diff --stat 看波及面。
2. 理解意图:从 commit message、PR 描述或用户说明确认"这次改动想达成什么",审查是对照意图而不是空对代码。
3. 追踪调用方:改了函数签名/行为时,用 search_files 找所有调用点确认没有遗漏更新,例如 search_files("functionName\\(", "src/");再用 glob_files 找同名测试文件确认测试是否同步改了。
4. 分维度过清单(见下),逐文件审。
5. 按规范输出反馈(见末尾)。

维度一:正确性(最高优先)

• 边界条件:空数组/空字符串/null/undefined/0/负数/超长输入是否处理?循环边界有没有差一(off-by-one)?
• 错误路径:每个可能失败的调用(IO、网络、解析)是否处理失败分支?catch 之后是吞掉、还是正确传播/记日志?
• 状态一致:提前 return 的分支是否跳过了必要的清理/解锁/计数?
• 类型陷阱:隐式类型转换(== vs ===)、字符串数字混算、时区/编码假设。
• 逻辑取反:复杂布尔表达式建议手算一组真值确认;!(a && b) 这类容易写错。

维度二:安全

• 注入:SQL 是否参数化?shell 命令是否拼接了用户输入?路径是否校验防穿越(../)?HTML 输出是否转义防 XSS?
• 密钥:diff 里 grep 一遍 key|secret|token|password,硬编码凭证一律 blocker。
• 认证授权:新接口是否过了鉴权中间件?是否校验了资源归属(横向越权)?
• 反序列化/eval:对不可信输入用 eval、pickle.loads、yaml.load(非 safe)直接标 blocker。

维度三:性能

• N+1:循环里发请求/查库,改为批量。
• 不必要的全量:大文件整读、大表全扫、列表上 O(n²) 嵌套查找(可用 Map/Set 化为 O(n))。
• 资源泄漏:文件句柄、连接、定时器、事件监听是否在所有路径(含异常路径)上释放?Python 用 with,JS 注意 clearInterval/removeEventListener。
• 并发:共享可变状态有没有竞态?check-then-act(先查再写)在并发下是否安全?异步函数有没有漏 await?

维度四:可读性与维护性

• 命名是否表达意图;魔法数字是否需要常量化。
• 函数是否单一职责;超过约 50 行或嵌套超 3 层考虑拆分。
• 是否有重复代码可提取;是否引入了与现有代码风格不一致的新模式。
• 注释:解释"为什么"而非"做了什么";过期注释比没注释更糟。
• 测试:新逻辑有没有测试覆盖?测试是否覆盖了边界而不只是 happy path?

常见 bug 模式速查

反馈输出规范

• 每条问题必须带定位:文件路径:行号,引用关键代码行。
• 指出问题的同时给出可执行的修法(写出修改后的代码或明确步骤),不只说"这里有问题"。
• 按严重度分级输出:
• [Blocker] 必须修:正确性 bug、安全漏洞、数据丢失风险。
• [Should] 应该修:性能隐患、缺测试、错误处理不当。
• [Nit] 可选:命名、风格、注释。
• 格式示例:

[Blocker] server/routes/user.ts:42 — SQL 拼接了 req.query.id,存在注入。
修法:改用参数化查询 db.query('SELECT ... WHERE id = $1', [id])。

• 没有问题的维度明确说"已检查,未发现问题",让用户知道覆盖面。
• 最后给一句总评:能否合入、需要先修哪几条。