# 代码审查

## 输入要求

1. **代码片段**（必填）：需要审查的代码
2. **编程语言**（必填）：代码使用的语言
3. **上下文说明**（可选）：这段代码的功能、所在的模块
4. **审查重点**（可选）：如"重点关注安全性"、"检查内存泄漏"

### 输出示例

````
## 代码审查报告

**审查文件**：e/admin/ecmsmod.php
**编程语言**：PHP
**代码行数**：412 行
**问题总数**：3 个（致命 0 / 严重 1 / 警告 1 / 建议 1）

## 严重问题（建议修复）

### 问题1：ehash 校验缺失导致 CSRF 风险
**位置**：DoEditMod 函数 / 第 88-102 行
**原代码**：
```php
if($_POST['enews']=='EditMod'){ ... }   // 未校验 hCheckEcmsRHash()
```
**问题描述**：后台写操作未做 rhash 校验，可被跨站请求伪造触发非预期修改。
**影响**：攻击者可构造页面诱导管理员点击，静默修改模型配置。
**修复建议**：
```php
hCheckEcmsRHash();
if($_POST['enews']=='EditMod'){ ... }
```

## 警告与建议
- 第 130 行 `fetch1()` 未做 false 守卫，建议 `?: []`。

## 审查总结
- **整体评分**：82/100
- **优先修复**：问题1（上线前必须修）
````

## 执行流程

### 第一步：Spec一致性检查

审查前先检查代码是否符合spec：

- **如有spec**：逐条检查spec中的scenario是否被正确实现
- **发现不一致**：标注spec与实现的偏差，建议修正spec或代码
- **无spec**：提示建议在审查后使用 `spec-driven-development` 补全spec

```
✓ Spec一致性检查:
- Scenario 1: [通过/失败] — [说明]
- Scenario 2: [通过/失败] — [说明]
```

### 第二步：项目规范检查

检查代码是否符合项目记忆中的规范：

- 命名约定（snake_case / camelCase / PascalCase）
- 错误处理模式（异常 vs 返回码）
- 导入排序、文件组织等风格规范
- **发现新规范**：提示记录到项目记忆

### 第三步：代码理解

- 识别编程语言和框架
- 理解代码的功能和逻辑
- 识别代码的上下文（函数、类、模块）

### 第四步：多维度审查

| 维度             | 检查内容                                | 权重 |
| ---------------- | --------------------------------------- | ---- |
| **Bug风险**      | 空指针、数组越界、资源未释放、并发问题  | 30%  |
| **安全性**       | SQL注入、XSS、敏感信息硬编码、权限绕过  | 25%  |
| **性能**         | 时间复杂度、内存泄漏、N+1查询、循环内IO | 20%  |
| **可维护性**     | 命名规范、函数长度、圈复杂度、重复代码  | 15%  |
| **最佳实践**     | 语言特性利用、设计模式、错误处理        | 10%  |
| **Karpathy维度** | 简洁优先、精准修改、编码前思考          | 附加 |

**小片段快速审查模式**：

当用户只提供单个函数、短代码片段或明确要求“快速看看”时，先按以下四象限完成轻量审查；发现高风险问题后再展开完整分级报告：

| 快速象限         | 检查重点                                     | 输出要求                         |
| ---------------- | -------------------------------------------- | -------------------------------- |
| 代码规范与可读性 | 命名、缩进、函数长度、嵌套层级、注释有效性   | 只指出影响理解或维护的具体问题   |
| 潜在 Bug         | 边界条件、空值处理、数组索引、资源释放、并发 | 必须说明触发条件和可能后果       |
| 性能分析         | 算法复杂度、重复计算、循环内 IO/SQL/网络请求 | 能量化则量化，不能量化则标注原因 |
| 安全性考量       | 注入、XSS、敏感信息、权限校验、反序列化风险  | 命中即提升为严重或致命候选       |

快速模式不得降低审查标准；只是压缩输出篇幅，不省略 Bug、安全和性能判断。

### 第五步：Karpathy维度审查

在常规审查之外，按Karpathy编码哲学进行专项审查：

**简洁优先（Simplicity First）**：

- [ ] 是否包含超出需求的功能？
- [ ] 是否存在单次使用的抽象（Strategy模式做简单计算）？
- [ ] 是否可以用更少的代码实现（200行能否变50行）？
- [ ] 是否包含推测性功能（缓存、验证、通知等未被要求的）？

**精准修改（Surgical Changes）**（针对修改的代码）：

- [ ] 是否只修改了必要的代码？
- [ ] 是否"顺手"改进了相邻代码、注释或格式？
- [ ] 是否保持了原有代码风格？
- [ ] 每一行修改是否能追溯到需求？

**编码前思考（Think Before Coding）**：

- [ ] 代码中是否有未陈述的假设？
- [ ] 是否有硬编码的"魔法值"没有说明原因？
- [ ] 函数/类设计是否考虑了多种使用场景？

Karpathy维度问题不纳入 severity 分级，单独标注为 **「Karpathy建议」**。

### 第六步：问题分级

| 级别                   | 说明                               | 必须修复 |
| ---------------------- | ---------------------------------- | -------- |
| **致命（Critical）**   | 会导致系统崩溃、数据丢失、安全漏洞 | 是       |
| **严重（Major）**      | 明显Bug、性能严重下降、安全隐患    | 是       |
| **警告（Warning）**    | 潜在风险、代码异味、不符合规范     | 建议     |
| **建议（Suggestion）** | 可优化点、风格问题、最佳实践       | 可选     |

### 第七步：生成修复建议

对每个问题：

- 指出具体代码位置（行号/函数名）
- 说明问题原因和影响
- 提供修复后的代码示例
- 说明修复后的收益

**Karpathy建议的修复优先级**：

- 过度设计 → 先简化再审查
- 顺手重构 → 回滚无关修改
- 隐藏假设 → 添加参数校验或文档

### 第八步：记录到项目记忆

审查完成后，将发现的问题和规范沉淀到项目记忆（参见 `project-memory-management.md`）：

- 记录反复出现的坏味道模式（Convention Capture 模式）：提炼为团队编码规范避免复发
- 记录安全/性能问题的修复模式（Decision Record 模式）：形成可复用的检查清单
- 若审查中发现 spec 与实现偏差，反馈到 spec 同步更新
- 更新已知问题清单，标注高风险模块供后续审查重点关注

## 实战请求示例

### 示例一：小片段快速审查

```text
帮我快速审查这段 PHP 代码。
重点：SQL 注入、XSS、PHP 8.2 兼容、是否有明显逻辑 Bug。
范围：只看我贴出的函数，不扩展到其他文件。
输出：按阻塞/严重/一般/建议分级；如果没有明显问题，请明确写“未发现阻塞问题”。
验证：说明是否需要 php -l 或单元测试。
```

优先加载：`code-review.md`；如果出现数据库访问，再协同 `mysql-database.md`。

### 示例二：功能合并前审查

```text
帮我审查这个订单导出功能的改动。
范围：Controller、Service、导出任务、前端轮询接口。
重点：权限、CSRF、Init-Step-Poll、防重复提交、超时恢复、错误提示。
要求：先列阻塞问题，再列可优化项；不要直接重构，除非影响主线可用性。
```

优先加载：`code-review.md`、`api-design.md`、`test-generation.md`。

## 输出格式

````
## 代码审查报告

**审查文件**：[文件名]
**编程语言**：[语言]
**代码行数**：[X行]
**问题总数**：[X]个（致命[X] / 严重[X] / 警告[X] / 建议[X]）

## 致命问题（必须修复）

### 问题1：[问题标题]
**位置**：[函数名/行号范围]
**原代码**：
```[代码片段]```
**问题描述**：[具体说明]
**影响**：[可能导致什么后果]
**修复建议**：
```[修复后的代码]```

## 严重问题（建议修复）

### 问题N：[问题标题]
...

## 警告与建议

...

## 审查总结

- **整体评分**：[X]/100
- **主要风险**：[一句话总结]
- **优先修复**：[Top 3问题]
- **正向评价**：[代码中的亮点]
````

**无明显问题输出规范**：

如果完成审查后未发现有效问题，不得强行制造建议，必须明确输出：

```
## 代码审查报告

**审查结论**：未发现明显问题
**已覆盖维度**：可读性 / Bug风险 / 性能 / 安全性 / 项目规范
**残余风险**：[缺少运行环境、依赖上下文、测试结果或业务规则时说明；无则写"无"]
**建议验证**：[可选的测试、lint、静态分析或人工验收步骤]
```

## 质量标准

- 每个问题必须指出具体代码位置，不能只说"这段代码"
- 修复建议必须是可运行的代码，不能是伪代码或描述
- 致命问题必须说明可能导致的具体后果（如"可能导致SQL注入，攻击者可获取全表数据"）
- 不得误报（把正确代码标记为问题），不确定时标注"疑似"
- 安全审查必须覆盖OWASP Top 10相关风险
- 性能审查必须量化影响
- 涉及 MySQL/SQL 时必须检查参数绑定、索引策略、EXPLAIN 证据、事务边界、DDL 风险和回滚方案
- **证伪门禁**：每个结论交付前，先构造一个反例尝试推翻它；构造不出反例才允许交付。跑通 ≠ 正确。环检测三次没写对，根因不是没清单，而是每次写完都没人攻击它；本门禁把"跑通就交付"逼成"证伪后交付"

### 性能反模式审查清单

当用户要求代码优化、性能优化、架构优化、慢接口排查，或审查中发现性能嫌疑点时，追加以下检查：

| 反模式            | 审查信号                                                       | 风险判定                                                   | 修复方向                                           |
| ----------------- | -------------------------------------------------------------- | ---------------------------------------------------------- | -------------------------------------------------- |
| N+1 查询          | 循环体内出现数据库查询、ORM `find/select/where`、外部 API 请求 | 数据量增长时调用次数线性放大，严重时可导致超时或连接池耗尽 | 批量查询、预加载、JOIN、聚合、一次性查询后内存分组 |
| 同步阻塞 I/O      | 高并发接口中出现同步文件读写、同步网络调用、长 CPU 任务        | 请求线程被阻塞，吞吐下降，尾延迟升高                       | 异步化、后台任务、队列、流式处理、连接池           |
| 无界缓存/内存泄漏 | 全局 Map/dict/Object/static 集合持续写入，缺少 TTL、容量、清理 | 长时间运行后内存持续增长，可能 OOM                         | TTL、容量上限、LRU、生命周期清理、弱引用或托管缓存 |
| 无上限数据拉取    | 列表接口无分页、`.all()` / `findAll()` / `SELECT *` 拉全量     | 大数据量下内存、网络和数据库压力失控                       | 强制分页、游标、最大 page size、按需字段           |
| 索引失效          | `DATE(column)`、`LIKE '%xxx'`、隐式类型转换、函数包裹索引列    | 查询退化为全表扫描或大范围扫描                             | 改写查询条件、调整索引、提供 EXPLAIN 验证          |

性能问题只允许在有证据时定性为结论；静态扫描命中但缺少 benchmark、profile、EXPLAIN 或日志时，必须标注为"疑似"并给出最小验证动作。

### 长任务反模式审查清单

当用户要求实现或审查长任务（批量导入/导出/生成静态页/重建索引/采集同步/清理缓存/>3 秒或 >100 条处理）时，追加以下检查：

| 反模式             | 审查信号                                               | 风险判定                                       | 修复方向                                                        |
| ------------------ | ------------------------------------------------------ | ---------------------------------------------- | --------------------------------------------------------------- |
| 单请求同步跑长任务 | 批量操作在单个 HTTP 请求内同步执行，无分批、无进度返回 | PHP 超时/内存耗尽、用户无进度感知、无法取消    | 改 `Init → Step → Poll`：Init 建任务、Step 分批、Poll 查状态    |
| Step 非幂等        | Step 重试会重复处理同一批数据                          | 数据重复、状态错乱                             | 用 `last_id`/游标/offset 保证每批唯一，重复调用结果一致         |
| 进度仅存内存       | 任务进度存 PHP 变量/session 而非 DB/缓存/任务文件      | 进程重启/多实例下进度丢失，Poll 永远 `running` | 进度持久化到数据库/缓存/任务文件，不能只依赖 PHP 内存           |
| 无取消入口         | 前端有取消按钮但后端无 cancel 端点或忽略取消标记       | 用户无法中止错误任务，资源空耗                 | 提供 cancel 端点，Step 检测取消标记后优雅退出                   |
| 无超时/僵尸保护    | 无 `updated_at` 阈值检测，`running` 任务卡死无兜底     | 僵尸任务永远 `running`，前端轮询死循环         | Poll 检测 `running` 超阈值→标记 `failed`（僵尸）或允许 `resume` |
| 并发不安全         | 多 Step 并发/多用户操作同任务无锁                      | 数据竞争、覆盖                                 | 任务级锁或串行 Step                                             |

长任务问题只允许在有实现证据时定性为结论；静态扫描命中但缺少接口契约、进度持久化证据或取消实现时，必须标注为"疑似"并给出最小验证动作。

### MySQL 索引审查清单

| 检查项       | 判定标准                                           |
| ------------ | -------------------------------------------------- |
| 查询路径     | 索引必须对应 WHERE/JOIN/ORDER BY/GROUP BY/分页路径 |
| 联合索引顺序 | 必须说明等值、区分度、范围、排序、覆盖字段顺序     |
| EXPLAIN      | 高频/慢查询必须提供 `key/type/rows/Extra` 证据     |
| 覆盖索引     | 只允许覆盖高频小字段列表，禁止塞入大字段           |
| 冗余索引     | 检查 `(a)` 与 `(a,b)`、唯一索引与普通索引重复      |
| 写入成本     | 新增索引必须评估 INSERT/UPDATE/DELETE 成本         |
| 删除索引     | 必须有依赖 SQL、风险说明和回滚 SQL                 |

## CMS 安全审查清单

当审查对象为 PHP+MySQL CMS 代码时，在「多维度审查」之外追加以下专项检查：

| 检查项                 | 检查方法                                                          | 判定标准                                 |
| ---------------------- | ----------------------------------------------------------------- | ---------------------------------------- |
| SQL 注入（CMS 上下文） | 检查是否使用 CMS 官方数据访问层，是否存在 `$_GET/$_POST` 直拼 SQL | 必须使用参数绑定或 CMS API               |
| XSS（CMS 模板）        | 检查模板输出是否默认转义，PHP 直接输出是否 `htmlspecialchars`     | 所有用户输入输出前必须转义               |
| CSRF                   | 检查表单提交和状态变更操作是否有 Token 验证                       | POST/PUT/DELETE 必须验证 CSRF Token      |
| 文件上传               | 检查是否有 MIME 校验 + 扩展名白名单 + 重命名 + 非执行目录         | 四项缺一不可                             |
| 反序列化               | 检查是否有 `unserialize()` 对不可信数据的调用                     | 禁止，改用 JSON                          |
| 密码存储               | 检查密码是否用 `password_hash()`/`password_verify()`              | 禁止 MD5/SHA1                            |
| Include 注入           | 检查是否有 `include $user_input`                                  | 路径必须白名单或固定                     |
| 数组键引号             | 检查 `$arr[key]` 无引号访问                                       | 必须加引号 `$arr['key']`，PHP 8 致命错误 |
| 类型比较               | 检查是否使用 `==` 而非 `===`                                      | 统一 `===`                               |

详见 `cms-development.md` 安全红线章节。

## Java 专项审查清单

当审查对象为 Java/Spring 代码时，在「多维度审查」之外追加以下专项检查：

| 检查项      | 检查方法                                                         | 判定标准                                                |
| ----------- | ---------------------------------------------------------------- | ------------------------------------------------------- |
| 正确性      | 检查边界条件、空值、集合访问、Optional 使用、异常路径            | 不得依赖隐式非空假设，异常路径必须可解释                |
| 安全性      | 检查 SQL 拼接、XSS、反序列化、敏感信息、鉴权注解                 | MyBatis 禁止 `${}` 拼接外部输入，接口必须有权限边界     |
| 性能        | 检查 N+1 查询、循环内 IO/SQL、重复对象创建、分页缺失             | 高频路径必须说明复杂度和数据量影响                      |
| 健壮性      | 检查异常处理、日志上下文、事务边界、连接池/超时                  | 禁止空 catch；事务边界必须在业务方法层清晰表达          |
| 并发安全    | 检查共享可变状态、线程池配置、锁粒度、CompletableFuture Executor | 共享状态必须有线程安全策略，线程池不得使用隐式默认配置  |
| Spring 分层 | 检查 Controller/Service/Repository/DTO/Entity 职责               | Controller 不写业务细节，Entity 不直接作为长期 API 响应 |
| JVM 风险    | 检查大对象、缓存无界、线程泄漏、GC 压力和资源关闭                | 性能判断必须有耗时、GC、线程、堆或连接池证据            |

详见 `java-development.md` 的源码分析、Spring Boot 结构、并发与 JVM 章节。

### 算法正确性审查清单

当用户要求审查图遍历、环检测、递归、状态机、拓扑排序、依赖解析，或发现递归/DFS 代码时，在「多维度审查」之外追加以下专项检查：

| 检查项              | 检查方法                                                                                                                                          | 判定标准                                                                                              |
| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- |
| 不变量（invariant） | 检查是否**显式声明**了「不变量命题 + 终止度量 + 守恒关系」三件套（推导方法见 `karpathy-coding-guidelines.md` 不变量推导三步法），且实现能断言它们 | 关键算法必须交出三件套并过自检（可判定/初始成立/归纳保持/充分性），不能只靠"流程跑通"或"写了个不变量" |
| DFS 回边判定        | 检查回边识别是否基于"在递归栈中"而非"已访问"，避免把横叉边/前向边误判为环                                                                         | 回边必须只在当前 DFS 路径中闭合才计为环                                                               |
| 方向语义解耦        | 检查遍历方向（down/up/both）与边有向性是否被编码耦合                                                                                              | 遍历方向必须独立于边的有向性，反方向遍历结果应一致                                                    |
| 结构边界覆盖        | 检查测试是否覆盖 0/1 节点、自环、互环、三环、菱形 DAG、平行边                                                                                     | 见 `test-generation.md` 算法边界用例清单                                                              |
| 死代码检测          | 检查是否存在永不被调用的函数/分支、恒真/恒假条件、不可达代码                                                                                      | 标记死代码，要求删除或说明保留理由                                                                    |

图与递归类缺陷的典型漏网形态是"流程跑通但结果错"，审查必须断言具体结果（环数、环成员、是否标记、是否重复），不得仅以"能运行"判定正确。

## 关联 reference

- **code-generation**（代码生成）— 审查来自 `code-generation` 产出的代码时，重点验证生成是否偏离 Spec 约束和编码规范
- **karpathy-coding-guidelines**（Karpathy编码规范）— 用 `karpathy-coding-guidelines` 深入评估代码是否符合编码哲学
- **frontend-design**（前端设计）— 前端页面审查时参考 `frontend-design` 的交互状态、响应式、可访问性和浏览器验证清单
- **bug-diagnosis**（Bug诊断）— 审查发现的具体Bug可用 `bug-diagnosis` 深入分析根因
- **refactoring**（重构建议）— 审查发现的代码异味可用 `refactoring` 获取详细重构方案
- **test-generation**（测试用例生成）— 修复后可用 `test-generation` 补充回归测试
- **cms-development**（CMS二次开发）— PHP+MySQL CMS 场景参考 `cms-development.md` 的安全红线和兼容性检查
- **java-development**（Java专项参考）— Java/Spring 场景参考 `java-development.md` 的分层结构、异常处理、并发和 JVM 检查
- **mysql-database**（MySQL数据库）— 审查 SQL 注入、慢查询、索引滥用、事务边界和迁移风险时参考 `mysql-database.md`
- **project-memory-management**（项目记忆管理）— 记录反复出现的坏味道、安全/性能问题修复模式和已知问题清单

## 失败回退机制

| 步骤                     | 失败条件                         | 回退目标                                       | 最大重试  | 不可恢复时升级路径                                       |
| ------------------------ | -------------------------------- | ---------------------------------------------- | --------- | -------------------------------------------------------- |
| 第一步：Spec一致性检查   | Spec文档不可用或无关联           | 跳过一致性检查，从代码反推预期行为             | 0         | 标注"无Spec对照"，仅审查内部一致性和通用规范             |
| 第二步：项目规范检查     | 项目规范文档缺失或代码文件不可读 | 跳过规范加载，直接分析代码结构，标注"规范缺失" | 2         | 基于通用最佳实践审查，建议补充项目规范                   |
| 第三步：代码理解         | 代码量过大超出分析能力           | 分批审查，逐文件输出，最后汇总                 | 0（分批） | 告知用户分批结果，建议缩小审查范围                       |
| 第四步：多维度审查       | 某维度缺少足够上下文             | 标注"待验证"，跳过该维度                       | 1         | 输出已覆盖维度的审查结果，缺项标注为"建议补充后重新审查" |
| 第五步：Karpathy维度审查 | 简洁性判断主观争议               | 标注为"建议评估"，附正反观点                   | 0         | 交由用户/团队讨论决策                                    |
| 第六步：问题分级         | 严重性边界模糊（中/高难判）      | 默认升级一级（保守策略），标注"存疑"           | 1         | 所有存疑项统一标记为"需确认"                             |
| 第七步：生成修复建议     | 修复方案可能引入新问题           | 标注风险提示，建议回归测试范围                 | 2         | 输出修复建议+单元测试覆盖要求，由开发者自行实施          |
| 第八步：记录到项目记忆   | 项目记忆系统不可用               | 输出审查报告和规范沉淀到本地文件               | 1         | 标注"审查结果未沉淀"，提示用户手动保存                   |