diff --git a/.claude/custom-lint-rules.toml b/.claude/custom-lint-rules.toml index 8e99b082..e05d1e60 100644 --- a/.claude/custom-lint-rules.toml +++ b/.claude/custom-lint-rules.toml @@ -587,3 +587,54 @@ yaml = [ "no_jj_template_first_line_detects_yaml_pattern", "no_jj_template_first_line_yaml_skips_empty_keyword", ] + +# ─── ルール⑫: jj revset で `"master..@"` literal の hardcode 禁止 ─── +# +# 由来: PR #195 で `assert_descriptions_absent_in_pr_range` / `assert_descriptions_present_in_pr_range` +# helper が `"master..@"` を hardcode しており、alternative branch ("main" 等) test の追加時に +# silent breakage となる問題が CR Major 指摘 (commit 9663dd68 で前 2 関数を default_branch 引数化済)。 +# 同型 hardcode が `count_empty_in_pr_range` にも残留し、pre-push simplicity reviewer F-1 で +# non-blocking 観測。手動 grep / reviewer 判断で塞ぐと再発するため、決定論的防止層 +# (ADR-007 の正規表現層) として機械検出する。 +# +# Pattern scope: jj revset 固有構文のため false positive 極小。`"master..@"` literal は +# 通常の Rust 文脈 (path / URL / 通常文字列) には出現しない。 +# +# Self-exclusion (重要): 本 TOML は extensions = ["rs"] により対象外。 +# `src/hooks-post-tool-linter/src/main.rs` の test fixture は build_*_fixture helper で +# `format!("\"{}..@\"", branch)` のように runtime 組立 (existing build_write_discard_fixture +# pattern を踏襲)、main.rs source 内に literal `master..@` を書かないこと。 +# verify: `grep -n 'master\.\.@' src/hooks-post-tool-linter/src/main.rs` → 0 hit が期待値。 + +[[rules]] +id = "no-hardcoded-jj-revset-range" +pattern = 'master\.\.@' +severity = "warning" +message = "jj revset の `master..@` literal が hardcode されています。default branch 引数化を検討してください" +why = "PR #195 で test helper 3 関数のうち 2 関数が `master..@` hardcode → CR Major 指摘で default_branch 引数化済、残 1 関数 (count_empty_in_pr_range) は `\"empty() & (master..@)\"` 形式で non-blocking 観測。alternative branch (`main` 等) variant test 追加時の silent breakage 防止 (ADR-021 § Revset Composability)。決定論的防止層 (ADR-007)。FP リスクは Bundle Z #B-α (comment-lint-rust) で非 doc comments が既に禁止されているため軽微 (jj revset 固有 token)" + +extensions = ["rs"] + +[rules.fix] +strategy = "default_branch 引数を受けて `format!(\"{}..@\", default_branch)` で組立" +steps = [ + "対象関数の signature に `default_branch: &str` を追加", + "revset literal を `format!(\"{}..@\", default_branch)` または `format!(\"empty() & ({}..@)\", default_branch)` 形式に変更", + "caller を `\"master\"` 引数で更新 (既存挙動を保持)", + "alternative branch (`\"main\"`) variant の sanity check test を companion helper group 全関数に対して追加", +] + +[rules.example] +bad = 'let revset = "master..@";' +good = 'let revset = format!("{}..@", default_branch);' + +[rules.test_coverage] +# rule⑫ は rs のみ (主要拡張子)。positive (hardcode 検出) + negative (parameterized 形式 skip) の variant を網羅。 + +[rules.test_coverage.main_ext_tests] +rs = [ + "no_hardcoded_jj_revset_range_detects_simple_hardcode", + "no_hardcoded_jj_revset_range_detects_within_empty_filter", + "no_hardcoded_jj_revset_range_skips_parameterized_format", + "no_hardcoded_jj_revset_range_skips_other_branch_literal", +] diff --git a/src/cli-pr-monitor/src/fix_commit.rs b/src/cli-pr-monitor/src/fix_commit.rs index d07d3a2f..257b361b 100644 --- a/src/cli-pr-monitor/src/fix_commit.rs +++ b/src/cli-pr-monitor/src/fix_commit.rs @@ -599,12 +599,13 @@ mod tests { } } - fn count_empty_in_pr_range(repo_dir: &std::path::Path) -> usize { + fn count_empty_in_pr_range(repo_dir: &std::path::Path, default_branch: &str) -> usize { + let revset = format!("empty() & ({}..@)", default_branch); let out = std::process::Command::new("jj") .args([ "log", "-r", - "empty() & (master..@)", + &revset, "--no-graph", "-T", "change_id ++ \"\\n\"", @@ -618,7 +619,7 @@ mod tests { .count() } - /// 統合: `master..@` 範囲に空 commit が無いとき sweep は no-op (非空 commit を保持)。 + /// 統合: PR 範囲 (`..@`) に空 commit が無いとき sweep は no-op (非空 commit を保持)。 #[test] #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] fn integration_sweep_empty_commits_no_op_when_no_empty_in_range() { @@ -641,7 +642,7 @@ mod tests { ); } - /// 統合: `master..@` 範囲の複数空 commit を sweep が全て abandon する。 + /// 統合: PR 範囲 (`..@`) の複数空 commit を sweep が全て abandon する。 /// PR #174 `kqvluqyv` 事例の最小再現。 #[test] #[ignore = "integration: requires jj in PATH; run via `cargo test -- --ignored --test-threads=1`"] @@ -654,7 +655,7 @@ mod tests { build_jj_empty_with_description(repo_dir, label); } assert!( - count_empty_in_pr_range(repo_dir) >= 2, + count_empty_in_pr_range(repo_dir, "master") >= 2, "前提: sweep 前に空 commit が 2 件以上" ); @@ -717,6 +718,11 @@ mod tests { build_jj_empty_with_description(repo_dir, "fix(review): empty under main"); + assert!( + count_empty_in_pr_range(repo_dir, "main") >= 1, + "前提: sweep 前に 'main' 範囲で空 commit が 1 件以上 (helper の default_branch 引数が main で機能していること)" + ); + let _guard = enter_repo(repo_dir); sweep_empty_commits_in_pr_range("main"); diff --git a/src/hooks-post-tool-linter/src/main.rs b/src/hooks-post-tool-linter/src/main.rs index 74d810a8..5a98558e 100644 --- a/src/hooks-post-tool-linter/src/main.rs +++ b/src/hooks-post-tool-linter/src/main.rs @@ -2733,6 +2733,90 @@ extensions = ["ts", "js"] assert_eq!(violations.len(), 1); } + fn no_hardcoded_jj_revset_range_rule() -> CustomRule { + make_test_rule( + "no-hardcoded-jj-revset-range", + r"master\.\.@", + &["rs"], + ) + } + + fn build_hardcoded_revset_fixture(branch: &str) -> String { + format!( + "fn count() {{ let revset = \"{}..@\"; let _ = revset; }}\n", + branch + ) + } + + fn build_empty_filter_revset_fixture(branch: &str) -> String { + format!( + "fn count() {{ let revset = \"empty() & ({}..@)\"; let _ = revset; }}\n", + branch + ) + } + + fn build_parameterized_revset_fixture() -> String { + "fn count(default_branch: &str) { let revset = format!(\"{}..@\", default_branch); let _ = revset; }\n" + .to_string() + } + + #[test] + fn no_hardcoded_jj_revset_range_detects_simple_hardcode() { + let dir = tempfile::tempdir().unwrap(); + let file = write_file( + dir.path(), + "fix_commit.rs", + &build_hardcoded_revset_fixture("master"), + ); + let rules = compile_test_rules(vec![no_hardcoded_jj_revset_range_rule()]); + let violations = run_custom_rules(file.to_str().unwrap(), &rules); + assert_eq!(violations.len(), 1); + } + + #[test] + fn no_hardcoded_jj_revset_range_detects_within_empty_filter() { + let dir = tempfile::tempdir().unwrap(); + let file = write_file( + dir.path(), + "fix_commit.rs", + &build_empty_filter_revset_fixture("master"), + ); + let rules = compile_test_rules(vec![no_hardcoded_jj_revset_range_rule()]); + let violations = run_custom_rules(file.to_str().unwrap(), &rules); + assert_eq!(violations.len(), 1); + } + + #[test] + fn no_hardcoded_jj_revset_range_skips_parameterized_format() { + let dir = tempfile::tempdir().unwrap(); + let file = write_file( + dir.path(), "fix_commit.rs", &build_parameterized_revset_fixture()); + let rules = compile_test_rules(vec![no_hardcoded_jj_revset_range_rule()]); + let violations = run_custom_rules(file.to_str().unwrap(), &rules); + assert!( + violations.is_empty(), + "format!(\"{{}}..@\", default_branch) の parameterized 形式は violation 対象外であるべき。実際: {:?}", + violations + ); + } + + #[test] + fn no_hardcoded_jj_revset_range_skips_other_branch_literal() { + let dir = tempfile::tempdir().unwrap(); + let file = write_file( + dir.path(), + "fix_commit.rs", + &build_hardcoded_revset_fixture("main"), + ); + let rules = compile_test_rules(vec![no_hardcoded_jj_revset_range_rule()]); + let violations = run_custom_rules(file.to_str().unwrap(), &rules); + assert!( + violations.is_empty(), + "default branch 以外 (本 case: 'main') の hardcode は narrow scope 設計により対象外。実際: {:?}", + violations + ); + } + fn collect_rust_files(root: &std::path::Path, out: &mut Vec) { let entries = match std::fs::read_dir(root) { Ok(e) => e,