ESLint 规则审查技能
像一位拥有数千条过往审查评论的维护者一样审查 lint 规则。 这位维护者的核心习惯是经验主义的:他们不单阅读规则,更尝试破坏它。 他们会删除一个条件,观察测试是否失败,并构思出没人想到的语法。 遵循此习惯。 有失败测试或存活变异体支持的结果,远胜于听起来合理的结果。而听起来合理的噪音正是使自动化审查被忽略的原因。
结果交给用户,由其决定如何处理。 将审查写入本地文件即为全部任务:发布到 GitHub 或推送任何内容,仅当用户单独要求时才进行。
1. 理解变更
确定审查对象:PR、分支或规则文件。 对于 PR,尽可能签出其头部提交,因为后续步骤需要运行代码。
在形成意见前先阅读以下内容:
- 规则、其测试及文档页面
- PR 描述及关联议题。因为规则应基于其目的而非其算法来评判
- 同一仓库中的两个或三个同级规则,以及
CONTRIBUTING文档
注意作者在描述中提出的任何问题。 审查应回答该问题。
仓库约定优先于本技能的参考资料。 参考资料来自多个项目,因此如果某个模式与仓库明确的方式冲突,请遵循仓库的约定。
参考资料较大,因此不要从头到尾阅读。 从 references/index.md 开始,该文件列出了所有模式,仅拉取与此变更相关的部分。 另请阅读 references/extra-checks.md,该文件简短,涵盖工具陷阱以及针对修复器、导入规则和 Flint 的检查。
2. 检查规则的设计
根据变更类型从索引的 design.md 列表中选择:
- 新规则: 是否应添加、其范围、与其他规则的重叠、类型信息、选项以及预设位置。
- 对既有规则的更改: semver 影响、需要相同更改的同级规则,以及更改是否服务于议题。 跳过其余部分。
如果 PR 粗糙,例如尚无测试或设计需要重新思考,则停止于高层审查并说明,跳过无法在没有测试的情况下提供信息的变异。 仍然在几个用例上尝试规则,因为任何阶段的真实错误都值得报告。 对即将更改的代码进行详细注释会浪费大家的时间。
3. 读取每行添加的代码
使用索引拉取与变更匹配的部分:
| 变更内容 | 查看参考 |
|---|---|
| 规则逻辑 | bugs.md, simplification.md, performance.md |
| 修复器或建议 | fixes.md |
| 消息 | messages.md |
| 测试 | test-method.md |
| 文档页面 | docs.md |
| 新名称、选项或规则 | naming.md |
在尝试破坏任何内容之前完成此步骤,因为一旦开始错误查找,它就会占据主导。 边读边做笔记,在步骤5中整理。
逐个遍历 diff 中添加的每行代码,并对每行提出以下问题:
- 名称: 不进一步阅读能否判断此标识符持有或做什么? 注意那些描述类型而非用途的名称,以及声称错误操作的名称。 如果需要二次阅读才能理解,请明确指出并建议更好的名称。
- 局部变量: 此变量是否仅使用一次,且紧接在设置之后? 建议内联它。
- 条件: 能否简化?例如
!x || x.kind !== y可变为x?.kind !== y。 嵌套或相邻的if通常可合并,布尔值的if通常可直接返回。 - 守卫: 考虑到 AST 和上方代码,此状态真的可能发生吗?
- 重用: 是否已有共享工具实现此功能?
在接受任何辅助函数前先检查。
如果辅助函数放在
create内部,参数是否会减少? - 长度: 是否有更少的代码能实现相同功能? 简化是过往审查中最大的一类。
- 值得做吗: 这段代码或
TODO是否为没人需要的功能做准备? 说明其可以删除。
如果代码看起来是 AI 辅助的,用更挑剔的眼光阅读,因为那里往往堆积着冗余代码。
检查每一条注释和文档语句。 Bug 查找会占用这部分时间,但它是真实审查评论的常规来源,因此请单独对每条新增注释和文档语句进行扫查。 不要问它是否正确,而是问它是否应该以当前形式存在:
- 此注释是否会过时? 关于测试覆盖率、历史或测试覆盖内容的注释会过时,因此建议删除它们。
- 它是否重复了代码? 建议删除它。
- 它是否是一个重要的已知捷径,例如上游代码的分支?
要求一个
TODO,如果存在关联议题则附上链接,并建议确切的注释内容。 - 最终用户会关心这条文档语句吗? 实现细节不属于文档页面。
- 此文档示例是否包含不必要的代码? 建议裁剪任何超出展示行为的代码。
- 语句是否精确? 注意那些听起来普适或永远正确但实际上并非如此的措辞,并建议重写。
以 ```suggestion 代码块的形式给出这些建议。
这对作者而言易于接受。
注意到好的方面。 指出任何有助于审查或处理棘手内容的地方,并具体提及,在审查正文中或作为 [Praise] 评论。
4. 尝试破坏它
此步骤产生最有价值的发现,因此不要跳过。 首先让规则的现有测试通过,以便有一个仅运行此规则测试的工作命令。
变异条件。 在仓库根目录运行捆绑脚本。 需要 Node 22.18 或更新版本,这可能与仓库固定的版本不同。 每个变异体是一个测试运行,因此在后台运行。 它会就地编辑规则,因此在此期间运行的任何其他内容必须使用临时名称保存的规则副本:
node <skill-dir>/scripts/mutate-conditions.ts --file <rule> --test "<test command>" --base <base ref>
--base 将变异限制在 PR 更改的行上,--diff <file> 使用 diff 实现相同目的。
两者都省略可变异整个新规则。
变异体按源码顺序运行,上限为 --max(默认为 60),因此如果摘要显示有任何未测试的,请提高该值。
如果测试从包文件夹运行,请在那运行脚本。--out <file> 保存完整结果,存活者列表始终最后打印。
它首先检查当规则抛出异常时测试是否失败,如果失败则停止,因为这意味着测试并未真正执行规则。
脚本完成后或被中断时会恢复文件。
如果它被直接终止,下一次运行将拒绝启动并打印原始文件的保存位置,因此在对规则进行更改前先比较两者,因为你可能已经编辑了它。
如果测试命令构建规则,请在之后重新构建,因为构建输出仍包含最后一个变异体。
脚本使用仓库的 typescript 解析,并且在 TypeScript 7 或更高版本上,它会打印如何通过 --typescript 传递旧副本。
未被测试发现的变异体称为存活者:每个存活者意味着测试缺失或代码是死代码,你需要判断是哪一种。
删除该条件并思考什么输入需要它。
如果你无法构造出输入,建议删除代码。
如果你能构造出,将其添加到下一步。
标记为 likelyTypeOnly 的存活者通常是仅为了编译器而存在的空 null 检查出口,因此最后查看它们,但也要检查该分支是否仅仅是未被测试。
如果测试运行器进行类型检查,变异体也可能仅因为无法编译而“失败”,因此在可能的情况下使用仅转换的命令。
该脚本仅涵盖常见变异。 还要手动尝试 PR 依赖的但脚本无法触及的任何内容,例如将参数替换为作用域中已有的类似变量。
比较 head 和 base。 对于对既有规则的更改,这是最重要的检查。
曾经工作的代码中的回归远比已经存在的缺陷重要,而且不运行两个版本就无法区分它们。
将 base 版本保存在规则旁边,例如使用 git show <base>:./<rule> > <rule-dir>/<name>.base-tmp.ts,或反向应用 diff。
然后将 assets/compare-rule.test.ts 复制到规则测试旁边,并调整其标记的行。
它通过比较报告计数对每个用例进行排序,并记录每个 fixture 中的消息、范围、修复输出、建议、崩溃和编译错误。
自行阅读 CHANGED OUTPUT 结果,因为更改的消息或修复可能是错误所在,并忽略任何带有 fixtureErrors 的用例,直到其 fixture 编译通过为止。
对于添加修复器的 PR,修复和建议输出是主要阅读内容,因此在这些用例上设置 expectedOutput。
如果 PR 添加了选项,设置 baseOptions 以便 base 版本的 schema 接受该用例。
没有它,该用例仅基于 head 进行判断。
对于新规则,同时将其作为 head 和 base 导入,以便 harness 报告 ok 或 WRONG 而不是比较版本。
如果 harness 不适合插件,改用仓库自己测试 harness 中的临时测试文件。
编写对抗性用例。 从 references/test-cases.md 中选择适合此规则涉及的语法和类型的类别,并编写文字用例。 在运行每个用例前确定正确行为,因为这能阻止你对结果进行合理化解释。 打印消息数据以及报告计数,因为消息中的错误类型名称也是一个 Bug。 将新语法中的用例与旧语法中的对照配对,例如符号键与相同用例使用字符串键。 查找:
- 崩溃
- 误报,其中有效代码被报告
- 漏报,其中无效代码未被报告
- 修复产生无效代码、改变行为或删除注释
- 报告范围超出违规代码
保留那些揭示了问题的用例,以便它们可以作为文字测试用例出现在审查中。
文字很重要:这位维护者要求手写用例,并反对 .map、工厂方法和生成测试。
证明重大建议。 如果你认为重新设计可以消除多个错误,请制作原型并运行现有测试。"267 of 268 tests pass, and here's why the other fails" 远比直觉有用。
清理。 在继续之前运行 git status 并删除所有临时文件,使仓库恢复到最初的样子。
5. 筛选发现
在写任何内容之前,逐一检查每个发现:
- Bug: 包含用例和实际发生的情况。
如果你无法复现,则删除它,或者如果你仍有具体理由怀疑它,降级为
[Question]。 - 顺序: 回归优先,然后是新代码中的 Bug,然后是 PR 涉及的既有 Bug。
- 单一根本原因: 如果重新设计可以修复多个 Bug,则将其合并为一条评论,并在其中列出用例。 仅当 Bug 无论如何都需要自己的修复时才保留单独评论,例如回归。
- 有争议的立场: 将这些视为非阻塞问题:节点上的
in检查、esquery使用、选项 schema 形状和import ts风格。 其他维护者有合理理由对此持有不同意见。 - 扩展或延后: 如果漏洞与规则概念相同且真实用户会遇到,则在此 PR 中要求修复。 否则建议创建后续议题。
- 添加或删除选项: 仅当规则否则会禁止有效代码,或真实用户要求时才要求选项。
- 既有代码: 除非问题严重,否则忽略 PR 未接触的问题,并标记为
[Non-Actionable]。 - 重复: 将重复项合并为一条评论,注明“此处及其他地方”。
噪音的代价高于遗漏:一个跳过了三条薄弱评论的作者也会跳过真正的那条。 阻塞性评论应仅限合并前必须更改的内容。 保持最多大约八条非阻塞评论,选择那些最能改进代码的,并在要求更改的每条评论中提供随时可应用的修改建议。 优先删除以下内容:
- 没有真正清晰度提升的品味问题,例如重述消息或折叠一个读起来还行的条件
- 描述中说明是有意为之的内容,例如计划工作的脚手架
- 你尝试过但找不到真实用例,且没有具体理由继续怀疑的 Bug
- 空白和引号样式
- 不增加价值的过程请求,例如为每个小的
TODO创建跟踪议题
没有可报告的内容也是好的结果,因此不要为了显得全面而编造发现。
将结论与阻塞评论相匹配。 如果没有阻塞,则批准并将其余作为建议留下。 这位维护者在可靠的 PR 上的许多审查是温暖的批准配有一两个小请求。
6. 编写审查
阅读 references/review-voice.md,然后在当前目录编写 lint-rule-review-skill-<pr-or-rule>.md,不提交它。
保持审查正文只有几句话,仅在回答作者问题或解释设计决策时才变长。
使用此结构:
# Review of <PR or rule>
**结论:** 批准 | 请求更改 | 仅高层级
<审查正文,遵循 review-voice.md 中的顺序>
## 内联评论
### `path/to/file.ts:42` · 阻塞
[Bug] <评论内容,如同发布时>
```ts
<文字失败的用例或建议>
```
证据:<你运行了什么以及结果如何,或“未复现”>
## 未检查
<任何你无法运行或验证的内容及其原因>
将每条评论标记为 阻塞 或 非阻塞,阻塞优先排序。"证据" 仅供用户参考,不会发布于…