第 30 篇：第二轮审核整改——16 个问题点、2 个一票否决，到 PR 合并
本系列第 30 篇。第 29 篇把覆盖率做到 97.35% 后提交了复审，结果第二轮审核意见比第一轮猛得多：16 个问题点、7 个风险点、2 项触发"一票否决"的未实现功能。这篇记录我怎么用 12 笔提交逐条消化它们——顺带被上游回归测试揪出 3 个真实的生产缺陷——最终等来那句"PR 已合并"。
一、第二轮审核意见：比第一轮猛得多
第一轮意见只有 3 条（缺覆盖率报告 + 4 条冒烟断言），第二轮直接给了一张大清单：
| 类别 | 数量 | 最扎心的 |
|---|---|---|
| 问题点 | 16 条 | parse()/parseFile() 统一入口没实现，文档却声明可用 |
| 风险点 | 7 条 | 又漏了 1 条 @Expect(true, true)、三处 try/catch 吞异常 |
| 未实现功能 | 2 项 | 触发一票否决 ⚠️ |
| 文档勘误 | 4 条 | 文档方法名和代码对不上 |
最棘手的是「一票否决」那两项：
PropertyListParser.parse()/parseFile()——上游主推的统一入口（自动检测格式再分派），我移植时注释掉了，想着"三个子解析器都能直接用，统一入口以后再说"。但 feature_api.md 里却声明它可用。PropertyListConverter四个 convert 方法——全是桩代码,文档同样声明可用。
💡 教训：文档声明了什么，代码就必须有什么。评审不会听"以后再补"，只看声明与实现是否一致。要么实现，要么在 design.md 里声明平台差异并给出理由——含糊地带就是一票否决区。
二、先拔否决项：补齐 parse/parseFile 和 Converter
parse()/parseFile()：10 行 dispatch，为什么拖了这么久
实现本身不难——determineType() 早就有了，补个分派即可：
public static func parse(data: Array<UInt8>): NSObject {
match (PropertyListParser.determineType(data, 0)) {
case TYPE_BINARY => BinaryPropertyListParser.parse(data)
case TYPE_XML => XMLPropertyListParser.parse(data)
case TYPE_ASCII => ASCIIPropertyListParser.parse(data)
case _ => throw PropertyListFormatException("The given data is not a property list of a supported format.")
}
}parseFile(path) 读文件后委托 parse。补上后新写了 6 个分派测试（三格式分派、空输入、未知格式、parseFile 自动检测）。
PropertyListConverter：有了 parse，全盘皆活
Converter 的四个方法（convertToXml/Binary/ASCII/GnuStepASCII）本质都是「parseFile 读进来 → 用目标写出器写出去」。之前卡壳就是因为没有统一入口；parse 补齐后一小时就全部实现了，覆盖率从 0/1 拉到 16/17。
💡 心得：一票否决的两项其实是同一个欠账——统一入口。欠的账不会消失，只会在评审时连本带利要回来。
三、移植上游回归测试：30 个用例揪出 3 个真 Bug
第二轮意见里分量最重的整改，是把上游的 GoogleCodeIssueTest（11 个用例）和 IssueTest（19 个用例，含 Billion Laughs 攻击防护和 17 个 crash 崩溃样本）完整移植过来。这批测试全部通过 PropertyListParser.parseFile() 统一入口调用——和上游调用方式完全一致。
移植过程中，这些"上游多年积累的回归弹药"直接命中了我移植时引入的 3 个真实缺陷：
Bug 1：写出器把中文和 emoji 写坏了
writeToFile 我原来写的是按 Rune 码点 & 0xFF 截成单字节再写文件，循环区间还写成 0..size-1 丢了末尾字符。ASCII 字符看不出问题，但 testIssue22 的 emoji 用例一进来就原形毕露——任何含中文/emoji 的 plist 写出即损坏。修复为直接写 UTF-8 字节，XML/ASCII 写出器共 3 处同款缺陷一起修掉。
Bug 2：UTF-16BE 代理对没合并
BMP 之外的字符（emoji）在 UTF-16 里是一对代理项，NSString 解码时没把高低代理合并成单一码点。
Bug 3：畸形输入把内部异常"漏"出去了
上游 17 个 crash 崩溃样本喂进来，我的解析器直接泄漏 OverflowException、IllegalArgumentException 这类内部异常——而上游的约定是统一抛 PropertyListFormatException。修复时还学到一课：上游 Java 对恶意 trailer 的巨大数值靠 32 位 int 截断回绕来兜底，仓颉的 Int64 不会回绕，得写一个 toJavaInt 对齐这个语义，才能保证"和上游接受/拒绝同一批文件"。
💡 心得：移植测试比移植代码更能检验移植质量。上游的回归测试是多年真实 issue 沉淀出来的弹药库，跳过它们等于放弃了最便宜的质量保障。
四、其余整改：注释、断言、文档一致性
剩下的按清单逐条清掉：
- 过期注释：10 个源文件里还留着"XXX 尚未移植"的注释，而 XXX 早就实现了——自查一遍全部归位；
- 冒烟断言清零：第 28 篇声称修完 4 条
@Expect(true, true)，结果评审又翻出 1 条漏网的，连同三处catch (_: Exception) {}吞异常一起，改成具体行为断言和@ExpectThrows，最后 grep 全仓复核清零； - 文档勘误：feature_api.md 里
getFormatName实为getTypeName、NSNull.wrap()签名不符——全文与代码逐一复核修正； - UTF-32 检测：补了 ASCII 解析器的 UTF-32BE/LE BOM 自动检测。这里有个小陷阱：UTF-16LE BOM（
FF FE）恰好是 UTF-32LE BOM（FF FE 00 00）的前缀，检测顺序必须 UTF-32 在前； - CloneTest 深拷贝：补移植 testCloneIsDeep，验证篡改原对象后克隆不受影响。
五、复审说明：逐条回复，不留死角
整改完成后，我写了一份《复审说明》随 PR 提交——评审给的每一条，都用表格逐条回复整改结果和对应提交号。16 个问题点、7 个风险点、2 项否决项、4 条勘误，一条不落。
最终数字：
| 指标 | 整改前 | 整改后 |
|---|---|---|
| 测试用例 | 262 | 309（LTS/STS 双版本全绿） |
| 行覆盖率 | 97.35%（2794/2870） | 97.26%（2875/2956） |
| 提交数 | — | 12 笔（每条整改独立提交） |
覆盖率数字略降是因为分母变大了——本轮新增了 86 行实现代码（parse/parseFile、Converter、UTF-32、健壮性检查），这在复审说明里如实写明。
💡 心得：复审说明的价值在于把评审的检查成本降到最低。逐条对照、附上提交号，评审顺着表格点一遍就能核完——你替他省的每一分钟，都是 PR 合并的加速度。
六、合并了
提交复审后不久，收到消息：PR 已合并。
从第 1 篇读懂共建计划，到第 30 篇 PR 合并，这个从零开始的移植项目在官方仓库落地了。回头看两轮审核，其实是评审替我把"自以为完成"和"真正完成"之间的差距量了出来：
- 第一轮教我：覆盖率不是跑通就行，断言要有语义；
- 第二轮教我：文档即契约、上游测试即弹药、否决项即欠账。
七、这一篇的踩坑总结
| 坑 | 现象 | 解决 |
|---|---|---|
| 文档超前声明 | feature_api.md 声明了未实现的 API，触发一票否决 | 要么实现，要么 design.md 声明差异理由 |
| 写出器码点截断 | 中文/emoji 写出损坏 | 按 UTF-8 字节写出，不做 & 0xFF |
| 代理对未合并 | UTF-16BE 解码 emoji 乱码 | 高低代理合并为单一码点 |
| 内部异常泄漏 | crash 样本喂入抛 OverflowException | 统一转 PropertyListFormatException；toJavaInt 对齐 32 位回绕 |
| BOM 检测顺序 | UTF-16LE BOM 是 UTF-32LE BOM 的前缀 | UTF-32 检测放在 UTF-16 之前 |
下一篇：PR 合并只是代码入了库，制品还得发布到仓颉中心仓，让所有人 cjpm 一行依赖就能用上——那又是一段带着两个硬坑的旅程。