0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

Jev で関数名・コメントの嘘を探したら、本物は4割。でも PR 差分なら指摘はほぼ出なかった

0
Posted at

ドパガキ駆動型結論

私のようなドパガキのために先に結論を。

  • 関数名やコメントと中身の矛盾を見つける jev-lint を TypeScript リポジトリにかけた。875件、$2.95
  • 名前・コメント系の118件を1件ずつ仕分けたら、本物は44件(37%)。README の「5件に1件は誤り」より精度がだいぶ悪かった。ルールによる差も大きい
  • 確率の高さでは、本物と誤検知をあまり区別できなかった
  • PR の差分だけにかけると、1本あたりの指摘は0〜1件で、読める量に収まった。ただ、精度が上がったわけではなく、見る対象が少ないだけ
  • 全体スキャンの結果を全部信じるのは無理。ルールを選んで、PR の差分にかけるくらいがちょうどいい

あるある言いたい

関数名やコメントと中身が合っていない、ってよくありますよね。「〜付きで取得する」と書いてあるのに付いてこない。コメントが説明している処理が、もう無い。型チェックも ESLint も通るので、誰にも指摘されません。

mizchi さんの jev-lint の記事も、この問題から始まっています。

気づいたら関数名が実態とズレていたり、昔書いたテストケース名やコメントが嘘になっていた、という問題に遭遇したことはありませんか?

元記事は、作者自身のリポジトリの紹介です。この記事は、実際にリポジトリにかけて出てきた指摘を全部仕分けた記録です。知りたかったのは「実際どこまで信用できるのか」です。

Jev とjev-lint

JevはTypeSafe AI の分類器です。LLM のように文章を返すのではなく、はい/いいえの質問に確率で答えます。文章を生成しないぶん、1問がとても安いです。

jev-lint はこれを lint に使ったツールです。ast-grep で関数やコメントを抜き出し、「この関数の中身は、名前が約束していることと実質的に違うか?」のような1文の質問を Jev に投げます。確率がルールごとのしきい値を超えたら指摘になります。

実際にリポジトリにかけてみた

--dry-run (見積もりだけ・送信なし)で見ると、リポジトリ全体に全ルールで約 $7 でした。手持ちのクレジットが $5 だったので、バックエンドの中核(API・ビジネスロジック、DB アクセス、共通ユーティリティ、ドメインモデル)に絞りました。

判定対象: 約5.4万件、約3,000リクエスト
指摘: 875件(本番コード474件、テストコード401件)
費用: $2.95
本番コードで多かったのは「その失敗経路を通るテストが無い」(209件)、「catch が失敗を握りつぶしている」(53件)、「上のコメントが中身と合わない」(49件)でした。

875件を全部は見られないので、名前・コメント系の6ルールにかかった本番コードの118件に絞って仕分けました。ここでの「名前」は、関数・メソッド・型の名前のことです(変数名は見ていません)。残りの757件の当たり外れは見ていません。

本物はどれくらい?

118件を、実際のコードと突き合わせて仕分けました。1件ずつサブエージェントに読ませて判定させ、影響が大きそうなものは自分でも確認しています。

判定 件数
本物(名前やコメントの具体的な主張が、コードと矛盾する) 44(37%)
言いがかり(間違いではないが、直すほどではない) 36(31%)
誤検知(名前やコメントは正しい) 38(32%)

jev-lint の README には、作者のリポジトリで「5件に1件は誤りだった」とあります。私のでは本物が4割弱でした。

本物の例

名前も doc も「オーナー付き」なのに、オーナーを取っていない関数です。

/**
 * 指定したチームID配列に該当するプロジェクトと、各プロジェクトのオーナーを一括で取得する
 * @returns プロジェクト+オーナーの配列
 */
async getAllByTeamIdsWithOwner(teamIds: number[]): Promise<Project[]> {
  return await this.prisma.project.findMany({
    where: { team_id: { in: teamIds } },
    // include: { owner: true } が無い
  });
}

こちらは、名前もコメントも「公開済み記事の更新を禁止する」と言っているのに、本体は文字数のルールしか見ていない関数です。公開済みの検証は意図的に消されていて、本体には「公開済みに関する警告は出さない」と書いてありました。

/**
 * 公開済み記事の更新制約・文字数ルールの検証を行う
 * - 公開済み: タイトル/URL/カテゴリの更新は禁止
 * - 本文更新: 新しい本文は元の文字数の50%以上である必要
 */
private async validatePublishedAndLengthConstraints(/* ... */) {
  // 公開済みに関する警告は出さない(実際にエラーとなるケースのみ検証)
  // ...文字数ルールだけ
}

本物の44件のうち、すぐにバグに直結するものはありませんでした。危なそうなものも、今は呼び出し元が無いか、今の呼び方では踏まないものでした。

ルールでは当たり外れが大きい

ルール 判定対象 指摘 本物 言いがかり 誤検知 使うなら
メソッド名が約束と違う(method-name-promises) 436 3 3 0 0 残す
doc の失敗時の約束が違う(doc-errors-match-body) 199 24 9 14 1 残す。言いがかりは多い
上のコメントが中身と合わない(comment-describes-declaration) 1,529 49 19 7 23 残す。誤検知が多い
関数名が約束と違う(fn-name-promises) 2,222 19 7 6 6 残す
型名が中身の形と合わない(type-name-describes-shape) 1,142 14 5 2 7 様子見
純粋な計算に見えるのに副作用がある(pure-name-is-pure) 138 9 1 7 1 切る

pure-name-is-pure は、ほぼ言いがかりでした。doc-errors-match-body は、外部 API を呼ぶ関数が @throws を書いていない、という指摘が多く、間違いではないけど直すほどでもないものが大半です。

誤検知のよくあるパターン

  • コメントの {@link 関数名} が別ファイルの関数を指していると、「存在しない関数を参照している」と読まれる
  • ファイル内の区切りの見出しコメント(「// Helpers」など)を、直後の関数の説明として読む
  • コメントアウトされたコードを、説明文として読む
  • そのファイルの命名の慣習に従った名前を、文字どおりに読んで「名前と違う」と言う

確率の高さでは区別できない

指摘には確率がついているので、「高いものだけ見れば精度が上がるのでは」と思って比べてみました。

判定 確率の中央値
本物 0.69
言いがかり 0.67
誤検知 0.62

ほとんど差がありません。しきい値を一律に0.1上げると、誤検知は38件から13件に減りますが、本物も44件から30件に減ります。本物の割合は37%から48%に上がるものの、3割の本物を捨てることになります。

PRの差分だけにかけてみた

全体スキャンの結果を全部読むのは大変です。jev-lintには、差分だけを判定する jev-lint review --base <ブランチ> があり、元記事でも「PRごとにアドバイザリーとして挟むくらいなら現実的」と書かれていました。そこで、過去のPRで試してみました。ルールは、さっきの6本だけです。

見逃さないか

まず、本物の矛盾が入ったとわかっている過去のPRを2本取り出して、PRの時点で指摘できたかをみました。

PR 指摘 中身
A 1件 狙いの矛盾(@throws に書かれていない例外を投げるようになった関数)。ほかの指摘は無し
B 3件 狙いの矛盾(「テストはこの4メソッドを差し替える」とあるのに、型は5メソッドになった)/PR の前からあった古い矛盾/テスト補助関数の名前への言いがかり

狙いの2件は、どちらも指摘されていました。

PR B の2件目が少し面白くて、このPRが作った矛盾ではありませんでした。

// enum セレクト。現在値が選択肢に無い場合は「⚠ 生値」を先頭に足して値を失わせない
function renderSelect(rowIndex, col, value, options, blankLabel, /* この PR で引数を追加 */) {
  // ...
  {!known && value !== '' && <option value={value}>{`未登録: ${value}`}</option>}

コメントは「⚠ 生値」と言っていますが、足しているのは「未登録: 値」です。この PR がこの関数に引数を足して触ったので、差分の判定対象に入って見つかりました。関数を触ったついでに、その関数の古い矛盾も出てくるわけです。

ノイズが出るか

次に、最近マージされた PR のうち、本番コードの TypeScript を変更しているものを新しい順に5本選んで流しました。

PR 判定対象 指摘
C 28 0件
D 45 1件(テストの補助関数。名前が use で始まるので React の hook に見えるが、中身はモックの書き換え。言いがかり寄り)
E 4 0件
F 24 0件(巨大なテストファイル1件はトークン上限で判定できず)
G 12 0件

5本で指摘は1件だけでした。費用は7本合わせて $0.03 です。

精度が上がったわけではない

差分にかけると静かだったので、精度が良くなったのかと思いましたが、指摘が出る割合を比べると同じでした。

判定対象 指摘 割合
全体スキャン(本番コード・6ルール) 5,666 118 2.1%
PR7本の差分 247 5 2.0%

静かだったのは、差分だけなので見る対象が少なく、1本あたり0〜1件に収まったからです。出てくる指摘の当たり外れは、全体スキャンと同じくらいだと思っておいたほうがいいかもしれません。PR の数が7本だけで、どれも小さめなので、大きな PR でどうなるかも分かりません。

どこまで信用できるか

あくまでも試した範囲での結論です。

  • 全体スキャンの指摘は、本物が4割弱。一覧をそのまま信じて直すのは無理で、人が1件ずつ見る必要がある
  • 確率の高さでは本物と誤検知を分けられない。しきい値を上げると、本物も一緒に減る
  • ルールを選ぶと、だいぶましになる。pure-name-is-pure は外していい
  • PR の差分にかけると、1本あたり0〜1件なので、毎回読める。精度は変わらないので、指摘は「ここを見て」という合図くらいに受け取る
  • 関数を触った PR で、その関数の古い矛盾も出てくる。直すタイミングとしてはちょうどいい

おわりに

名前やコメントと実装の矛盾は、安く見つかります。ただ、見つかったものの6割は直さなくていいものでした。全体スキャンの一覧を全部信じるのではなく、ルールを選んで PR の差分にかけ、出てきたら人が見る。今のところ、それくらいの距離感が良さそうです。

ちなみに、Jev は1問が安いので、同じ質問を過去の全コミット分聞くこともできます。やってみたら、さっきのような矛盾が「どのコミットで生まれたか」まで分かりました。その話は次の記事で書きます。

参考

付録:ハマったところ

  • ディレクトリをまとめて渡すと ast-grep rejected the rule set で落ちることがあった。ディレクトリごとに分ければ動く
  • --cache を指定しないと、リポジトリ内に .jev-lint/ が作られる
  • 巨大なファイルは、1リクエストのトークン上限(max_tokens_exceeded)に当たって判定できない
  • 過去の PR にかけるときは、git worktree で PR の先頭を取り出して、分岐元を --base に渡した
0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?