fix: 正規表現ビジュアライザの不正正規表現エラーを日本語化 (#489) - #536
Conversation
不正な正規表現入力時に JS エンジンの英語 SyntaxError (例: `Invalid regular expression: /(/: Unterminated group`)が そのまま表示されていた問題を修正。 - parse.ts の parseRegex で SyntaxError を catch し、日本語見出し付き (「正規表現が不正です: 〈詳細〉」)へ変換して再 throw。V8 系の重複する `Invalid regular expression: ` 接頭辞は除去し、不正箇所・理由の英語詳細は 情報量があるため残す。他ツールの ErrorMessage 表示と一貫 - parse.test.ts に日本語見出し / 詳細保持 / フラグエラーの assertion を追加 (旧実装に当てると fail することを陽性対照として確認済み) - regex-visualizer.spec.ts の既存エラー E2E を強化し、alert に日本語見出しが 表示され英語接頭辞が出ないことを assert - docs/tools.md の構文解析の項にエラー日本語化の挙動を追記 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🖼️ Visual Regression Test 結果
|
fumtas1k
left a comment
There was a problem hiding this comment.
多角的観点でレビューしました。総じて scope が狭く狙いの明確な良い fix で、merge OK と判断します(self-approve 不可のため COMMENT で投稿)。以下、観点別の所感と軽微な提案です。
✅ セキュリティ
- 日本語見出しを付けるだけの string 整形で新規攻撃面なし。
ErrorMessageは通常の props 経由(dangerouslySetInnerHTML不使用)なので、エラーメッセージに pattern の一部 (/(/等) が含まれても XSS にはならない。 - E2E は
withProductionCsp配下で実行されており、production CSP 下でも alert 表示が崩れないことが担保されている。 getErrorMessageのinstanceof Error分岐により prototype 混入経路もケア済み。
✅ ロジック
parseToRegExpTree内の nativenew RegExpと regexp-tree 自身のどちらが throw しても catch 一箇所で吸収できており、責務が綺麗。^Invalid regular expression:\s*をiflag 付きで stripping しているのは V8 系の大文字ゆらぎや trailing space に強く、妥当。useDebouncedTransformのfallbackErrorはe instanceof Errorでない場合の安全網として残しており、二重保険として正しい配置(PR 本文の補足通り)。parseRegexが日本語化 Error を throw すれば実質 fallback は発火しない。transformはparseRegex→analyzeRedos→buildRailroadの順次評価で、parseRegexの throw で後続まで到達しないため、recheck / railroad 由来の英語 raw error が混入する経路は実用上塞がれている。
🟡 アーキテクチャ(軽微な提案)
- 日本語化を
parseRegex内に閉じた設計は妥当ですが、parseToRegExpTreeはexportされており(railroad / redos モジュールが共有する想定)、将来別経路から直接呼ばれた場合に英語 raw error が UI に漏れる余地は残ります。yagni でも問題ないですが、もし将来 hardening するならparseToRegExpTree側に日本語化を寄せる選択肢もあります(現状の transform 構成では実害なし)。 let ast;はparseToRegExpTreeの戻り値型が推論される一方、明示するならlet ast: ReturnType<typeof parseToRegExpTree>;の方が読み手に親切。astro checkを通過しているので blocker ではありません。
✅ フロントエンドデザイン / UI
- 既存
ErrorMessage(variant="block") を踏襲し、他ツールと表示一貫性が保たれている。 - 「日本語見出し + 英語詳細」の方針は issue #489 の「要検討」を踏まえた合理的な落としどころ。
Unterminated group等の詳細語彙の和訳化は scope 外として妥当(やるなら別 issue 化推奨)。 - DADS 観点(青基調・semantic token)に関する変更なし。
✅ テスト
- 陽性対照あり: 「日本語変換を外すと 3 件 fail する」とローカルで確認済みである旨が本文に明記されており、
docs/shared-agent-rules.md3 章のガード/検知機構ルールに合致。test-gatesskill の要件を満たします。 - engine 依存になりがちな詳細文言は
message.length > '正規表現が不正です: '.lengthで「何か残っている」ことだけを assert しており、CI 差分への耐性が考慮されている。 - E2E が
toContainText('正規表現が不正です')とnot.toContainText('Invalid regular expression:')の両方を assert しており、見出し追加と prefix 除去の両方を抑え込めている。
🟡 任意の追補案(必須ではない)
toJapaneseRegexError(e)でeが Error 以外(例: 文字列)の場合に'正規表現が不正です: 構文を確認してください'が返る挙動を 1 行でユニットテスト化すると、fallback 経路の regression も抑え込めます(実用上発火しないため任意)。- もし
Unterminated group等の頻出英語詳細の和訳化を将来検討するなら、P3の別 issue として登録するのが分かりやすいです。
CI は test / visual-regression success、e2e は in_progress の状態でレビューしています。e2e が green になれば merge 可と判断します。
Generated by Claude Code
PR #536 レビュー (Arch#2) 対応。`let ast;` を `let ast: ReturnType<typeof parseToRegExpTree>;` にし、try 前宣言時の 型を読み手に明示する。挙動変更なし。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
レビューありがとうございます。軽微/任意の提案について対応・判断しました。 対応(commit b3d401d)
据え置き(理由明示)
CI は test / visual-regression / e2e いずれも success です。 |
fumtas1k
left a comment
There was a problem hiding this comment.
Re-review しました。前回レビューの指摘事項対応を確認します。
対応確認
| 項目 | カテゴリ | 対応 | 備考 |
|---|---|---|---|
parseToRegExpTree 側への日本語化集約 |
Arch #1 | 未対応(任意) | 現状 transform 構成では実害なし。OK |
let ast の型注釈明示 |
Arch #2 | ✅ 対応(b3d401d) |
let ast: ReturnType<typeof parseToRegExpTree>; で適切 |
| Error 以外 fallback の unit test | 任意 #1 | 未対応(任意) | 実用上発火しない経路。OK |
| 詳細語彙の和訳化 | 任意 #2 | 未対応(任意) | 別 issue 化推奨という提案のみ。OK |
追加レビュー(差分 b3d401d)
- ✅
ReturnType<typeof parseToRegExpTree>は同モジュール内のヘルパー関数の戻り値型を直接参照するため、parseRegExpTree本体の戻り値型が変わっても自動追随する。読み手にとってもastの型推論経路が明示され、try 前宣言パターンの可読性が向上。 - ✅ commit メッセージが Conventional Commits の
refactor:で日本語、挙動変更なしを明示しており規約準拠。 - ✅ CI:
test✅ /visual-regression✅ /e2ein_progress。挙動変更なしの型注釈追加のみなので e2e も green 見込み。
総評
スコープを保ったまま読みやすさだけ底上げする、健全な review fix です。e2e が green になれば merge OK と判断します(self-approve 不可のため引き続き COMMENT 形式)。
Generated by Claude Code
概要
正規表現ビジュアライザ(
/tools/regex-visualizer)で不正な正規表現を入力した際、JS エンジンの英語 SyntaxError(例:Invalid regular expression: /(/: Unterminated group)がそのまま表示されていた問題を修正します(Closes #489)。変更内容
src/utils/regex-visualizer/parse.ts:parseRegexでparseToRegExpTreeの throw を catch し、日本語見出し付き(「正規表現が不正です: 〈詳細〉」)へ変換して再 throw。V8 系の重複するInvalid regular expression:接頭辞は除去し、不正箇所・理由を示す英語詳細は情報量があるため残す(issue の「日本語見出し + 英語詳細」方針)。他ツールのErrorMessage表示と一貫。parse.test.ts: 日本語見出しで始まること / 英語詳細を保持し重複接頭辞を除去すること / フラグエラーも日本語化されることを assert。regex-visualizer.spec.ts: 既存のエラー E2E を強化し、role="alert"に「正規表現が不正です」が表示されInvalid regular expression:が出ないことを assert。docs/tools.md: 構文解析の項にエラー日本語化の挙動を追記。検証
node_modules/.bin/astro check→ 0 errorsnpm run test→ 91 files / 1651 passednpm run test:e2e -- regex-visualizer.spec.ts -g "不正な正規表現でエラーが出る"→ pass補足
useDebouncedTransformのfallbackErrorは Error 以外の throw 用の安全網としてそのまま残しています(通常経路は parse.ts が日本語 Error を throw)。🤖 Generated with Claude Code