ESLint ルールレビュースキル は何をしますか?
何千もの過去のレビューコメントを持つメンテナーのようにlintルールをレビューします。 そのメンテナーの特徴的な習慣は経験主義的です。ルールを読むだけでなく、それを壊そうと試みます。 条件を削除してテストが失敗するか確認し、誰も考えつかなかった構文を考案します。 ここではその習慣に従います。 失敗するテストや生き残ったミュータントによって裏付けられた発見は、もっともらしいだけの発見よりはるかに価値があり、もっともらしいだけのノイズこそが自動レビューが無視される原因です。
発見結果はユーザーに渡され、ユーザーがどうするかを決定します。 レビューをローカルファイルに書き出すことだけが仕事です。GitHubへの投稿やプッシュは、ユーザーが別途依頼した場合にのみ行われます。
1. 変更を理解する
レビュー対象が何か(PR、ブランチ、ルールファイル)を特定します。 PRの場合は、可能であればHEADコミットをチェックアウトします。後のステップでコードを実行するためです。
意見を形成する前に以下を読みます:
- ルール、そのテスト、ドキュメントページ
- PRの説明とリンクされたIssue(ルールはその目的に対して判断され、独自のアルゴリズムに対して判断されるわけではありません)
- 同じリポジトリの2〜3個の類似ルール、および
CONTRIBUTINGドキュメント
説明で作者が尋ねている質問に注意します。 レビューはそれに答えるべきです。
リポジトリの慣習はこのスキルのリファレンスより優先されます。 リファレンスは複数のプロジェクトから来ているため、パターンがリポジトリで明確に行われていることと矛盾する場合は、リポジトリに従います。
リファレンスは大規模なので、最初から最後まで読まないでください。 references/index.mdから開始します。そこにはすべてのパターンがリストされており、この変更に該当するセクションだけを抽出します。 またreferences/extra-checks.mdも読んでください。これは短く、ツールの落とし穴と、fixer、importルール、Flintのチェックをカバーしています。
2. ルールの設計を確認する
変更の種類に応じて、インデックスのdesign.mdリストから選択します:
- 新規ルール: 属するかどうか、スコープ、他のルールとの重複、型情報、オプション、プリセットの配置。
- 既存ルールの変更: semverへの影響、同じ変更が必要な類似ルール、変更がIssueに役立つかどうか。 残りはスキップします。
PRが荒い場合(まだテストがない、設計を再考する必要があるなど)は、ハイレベルなレビューで止めてその旨を伝え、ミューテーションはスキップします。テストなしでは何もわかりません。 それでもルールをいくつかのケースで試してみてください。実際のバグはどの段階でも報告する価値があります。 変更されようとしているコードに関する詳細なコメントは、皆の時間の無駄です。
3. すべての追加行を読む
インデックスを使用して、変更された内容に一致するセクションを取得します:
| 変更内容 | 参照先 |
|---|---|
| ルールロジック | bugs.md, simplification.md, performance.md |
| Fixerまたはsuggestion | fixes.md |
| メッセージ | messages.md |
| テスト | test-method.md |
| ドキュメントページ | docs.md |
| 新しい名前、オプション、ルール | naming.md |
これは何かを壊そうとする前に行います。バグハンティングは始まるとそれを優先するからです。 進みながらメモを取り、ステップ5で整理します。
差分を追加された行ごとに順に見ていき、それぞれについて以下を問います:
- 名前: これ以上読まなくても、この識別子が何を保持しているか、または何をするかわかりますか? 型ではなく目的を説明する名前、間違ったアクションを主張する名前に注意してください。 理解するのに二度読む必要があった場合は、その旨を伝え、より良い名前を提案します。
- ローカル変数: この変数は、設定された直後に一度だけ使用されていますか? インライン化を提案します。
- 条件式: 折りたためますか?
!x || x.kind !== yはx?.kind !== yになります。 ネストまたは隣接するifはしばしばマージでき、ブール値のifはしばしば直接returnできます。 - ガード条件: ASTとその上のコードを考慮して、この状態は本当に発生し得ますか?
- 再利用: 共有ユーティリティがこれをすでに行っていますか?
ヘルパーを受け入れる前に確認してください。
そのヘルパーが
create内部にあった場合、必要なパラメータは少なくなりますか? - 長さ: 同じことを行うより少ないコードはありますか? 単純化は過去のレビューで最大のカテゴリです。
- やる価値: このコードや
TODOは誰も必要としないものを計画していますか? 削除できると言います。
コードがAI支援に見える場合は、より識別力のある目で読んでください。そこにパディング(冗長なコード)がたまりがちだからです。
すべてのコメントとドキュメントの文を精査します。 バグハンティングはこれを押しのけますが、実際のレビューコメントの定期的なソースなので、追加されたコメントとドキュメントの行ごとに別々のパスとして行います。 正しいかどうかを問うのではなく、書かれたままそこにあるべきかどうかを問います:
- このコメントは古くなりますか? テストカバレッジ、履歴、テストの対象に関するメモは古くなるので、削除を提案します。
- コードを言い換えていますか? 削除を提案します。
- 上流コードのフォークなど、重要な既知のショートカットですか?
TODOを求め、存在する場合はIssueリンクも付け、正確なコメントを提案します。 - エンドユーザーはこのドキュメントの文を気にしますか? 実装の詳細はページに属しません。
- このドキュメントの例には不要なコードが含まれていますか? 動作を示すのに必要なもの以外は削除することを提案します。
- 文は正確ですか? 普遍的または常に真実であるように聞こえるが実際はそうでない表現に注意し、書き直しを提案します。
これらは `suggestion` ブロックとして提供します。
作者にとって受け入れが素早く行えます。
良い点に注目します。 レビューに役立った点や、難しいことを処理している点に注意し、レビュー本文または [Praise] コメントとして具体的に言及します。
4. 壊そうと試みる
このステップは最も価値の高い発見を生み出すので、スキップしないでください。 まずルールの既存のテストが通ることを確認し、このルールのテストだけを実行する作業コマンドを用意します。
条件をミューテーションする。 リポジトリルートからバンドルされたスクリプトを実行します。 Node 22.18以降が必要です。これはリポジトリが固定しているバージョンと異なる場合があります。 各ミュータントは1回のテスト実行なので、バックグラウンドで実行します。 ルールをその場で編集するため、その間に実行する他のものは、一時的な名前で保存したルールのコピーを使用する必要があります:
node <skill-dir>/scripts/mutate-conditions.ts --file <rule> --test "<test command>" --base <base ref>
--base はミュータントをPRが変更した行に制限し、--diff <file> は差分から同じことを行います。
両方を省略すると、新しいルール全体をミューテーションします。
ミュータントはソース順に実行され、--max(デフォルト60)までなので、サマリーにテストされていないものが表示されたら増やします。
テストがパッケージフォルダから実行される場合は、そこからスクリプトを実行します。--out <file> で完全な結果を保存し、生存者の短いリストが最後に常に表示されます。
最初に、ルールがスローしたときにテストが失敗することを確認し、失敗しない場合は停止します。テストがルールを実行していないからです。
スクリプトは終了時または停止時にファイルを復元します。
完全に強制終了された場合、次の実行は開始を拒否し、オリジナルが保存された場所を表示するため、ルールを変更する前に比較します。その間に編集した可能性があるためです。
テストコマンドがルールをビルドする場合、ビルド後に再ビルドします。ビルド出力には最後のミュータントが残っているためです。
リポジトリのtypescriptで解析し、TypeScript 7以降では--typescriptで古いコピーを渡す方法を表示します。
テストが気付かないミュータントは生存者です。それぞれは、テストが欠けているか、コードがデッドであることを意味し、どちらかを特定する必要があります。
条件を削除し、それを必要とする入力を考えます。
構築できない場合は、コードの削除を推奨します。
構築できる場合は、次のステップに追加します。
likelyTypeOnlyとマークされた生存者は、多くの場合コンパイラのためだけに存在する裸のnullチェック終了なので、最後に確認しますが、ブランチが単にテストされていないわけではないことを確認します。
テストランナーが型チェックを行う場合、ミュータントはコンパイルされないという理由だけで「失敗」することもあるので、可能な場合はトランスパイルのみのコマンドを使用します。
スクリプトは一般的なミューテーションのみをカバーします。 PRが依存しているもので、スクリプトが到達できないものも手動で試します。例えば、引数をスコープ内の類似変数と交換するなどです。
HEADとベースを比較します。 既存ルールの変更の場合、これが最も重要なチェックです。
以前動作していたコードのリグレッションは、すでにあったギャップよりもはるかに重要であり、両方のバージョンを実行しなければ区別できません。
ベースバージョンをルールの横に保存します。例えば、git show <base>:./<rule> > <rule-dir>/<name>.base-tmp.ts を使用するか、差分を逆適用します。
次に assets/compare-rule.test.ts をルールのテストの横にコピーし、マークされた行を調整します。
これは各ケースをレポート数を比較してソートし、各フィクスチャのメッセージ、範囲、fix出力、suggestion、クラッシュ、コンパイルエラーも記録します。
CHANGED OUTPUTの結果は自分で読んでください。変更されたメッセージやfixがバグである可能性があり、fixtureErrorsがあるケースは、フィクスチャがコンパイルされるまで無視します。
fixerを追加するPRの場合、fixとsuggestionの出力が主に読むものなので、それらのケースにexpectedOutputを設定します。
PRがオプションを追加する場合は、baseOptionsを設定して、ベースバージョンのスキーマがそのケースを受け入れるようにします。
そうしないと、ケースはHEADのみで判断されます。
新しいルールの場合は、headとbaseの両方としてインポートし、ハーネスがバージョンを比較する代わりにokまたはWRONGを報告するようにします。
ハーネスがプラグインに合わない場合は、代わりにリポジトリ独自のテストハーネスで一時的なテストファイルを使用します。
敵対的なケースを書きます。 references/test-cases.md から、このルールが扱う構文と型に合ったカテゴリを選び、リテラルなケースを書きます。 各ケースを実行する前に正しい動作を決定します。そうすることで、返ってきた結果を合理化するのを防げます。 レポート数だけでなくメッセージデータも出力します。メッセージ内の間違った型名もバグだからです。 新しい構文のケースと、古い構文の制御ケースをペアにします。例えば、シンボルキーと文字列キーを使用した同じケースなどです。 以下を探します:
- クラッシュ
- 誤検知(正当なコードが報告される)
- 見逃し(不正なコードが報告されない)
- 不正なコードを生成したり、動作を変更したり、コメントを削除するfix
- 問題のコードよりも広いレポート範囲
何かを露呈したケースは、レビューにリテラルなテストケースとして含められるように保持します。
リテラルが重要です。このメンテナーは手書きのケースを求め、.map、ファクトリ、生成されたテストを拒否します。
大きな提案を証明します。 リデザインがいくつかのバグを除去すると思う場合は、プロトタイプを作成し、既存のテストを実行します。「268テスト中267が合格し、残り1つが失敗する理由はこれです」は、勘よりもはるかに有用です。
クリーンアップします。 git statusを実行し、次に進む前にすべての一時ファイルを削除し、リポジトリを元の状態に戻します。
5. 指摘事項をフィルタリングする
何かを書く前に、各指摘事項を確認します:
- バグ: ケースと実際に起こったことを含めます。
再現できなかった場合は削除するか、特定の理由がまだある場合に限り
[Question]に格下げします。 - 順序: リグレッションが最初、次に新しいコードのバグ、次にPRが触れる既存のバグ。
- 単一の根本原因: リデザインが複数のバグを修正する場合は、それを1つのコメントにし、その中にケースをリストします。 バグがどちらにせよ独自の修正を必要とする場合(リグレッションなど)のみ、別のコメントを保持します。
- 争点のある立場: これらは非ブロッキングの質問として扱います: ノードの
inチェック、esqueryの使用、オプションスキーマの形状、import tsスタイル。 他のメンテナーも合理的にこれらに不同意してきました。 - 拡大または延期: ギャップがルールと同じ概念であり、実際のユーザーが遭遇する場合は、このPRでそれを求めます。 そうでなければフォローアップIssueを提案します。
- オプションの追加または削除: ルールが有効なコードを禁止する場合、または実際のユーザーが求めた場合にのみ、オプションを求めます。
- 既存のコード: PRが触れなかった問題は、深刻でない限り除外し、
[Non-Actionable]とマークします。 - 重複: 繰り返しを「ここおよび他の場所」という1つのコメントにマージします。
ノイズは見逃しよりもコストがかかります。3つの弱いコメントをスキップした作者は、実際のコメントもスキップします。 ブロッキングコメントは、マージ前に変更が必要なものだけにすべきです。 ノンブロッキングコメントは多くても8つ程度にし、コードを最も改善するものを選び、コメントが変更を要求する場所にはすぐに適用可能なsuggestionを付けます。 以下を最初に削除します:
- 明確さに実質的な利点がない好みの問題(メッセージの言い換え、問題なく読める条件式の折りたたみなど)
- 説明が意図的であると述べているもの(計画された作業の足場など)
- 実際のケースを見つけられず、疑う具体的な理由も残っていないバグ
- 空白と引用符のスタイル
- 何も追加しないプロセスの要求(小さな
TODOごとの追跡Issueなど)
報告することが何もないのは良い結果なので、徹底的に見せかけるために発見をでっち上げないでください。
評決をブロッキングコメントに一致させます。 何もブロックしなければ、承認し、残りはsuggestionとして残します。 このメンテナーからのしっかりしたPRへの多くのレビューは、1、2の小さなリクエストを伴う温かい承認です。
6. レビューを書く
references/review-voice.mdを読み、現在のディレクトリにlint-rule-review-skill-<pr-or-rule>.mdを書き込みます。コミットはしません。
レビュー本文は数文に抑え、作者の質問に答えたり設計上の判断を説明する場合にのみ長くします。
以下の構造を使用します:
# <PRまたはルール> のレビュー
**評決:** approve | request changes | high-level only
<レビュー本文。review-voice.mdの順序に従う>
## インラインコメント
### `path/to/file.ts:42` · blocking
[Bug] <投稿される形のコメント>
```ts
<リテラルな失敗ケースまたはsuggestion>
```
証拠: <実行したものと起こったこと、または「再現せず」>
## 未チェック
<実行または検証できなかったものとその理由>
各コメントをblockingまたはnon-blockingとマークし、ブロッキングを最初に並べます。「証拠」はユーザー向けであり、実際の投稿には含まれません。
証拠フィールドの後に改行と<詳細>ブロックを追加します。詳細ブロックには、コメントを生成するために実行した生のコマンド出力を含めます。これにより、ユーザーは必要な場合にいつでも詳細を確認できます。