Refuse a require_regexp that satisfies itself, which is the same accident going the other way - #74
Conversation
…dent going the other way
`validate_no_self_match` covered `regexp` and stopped there. Both are content
searches over a corpus that contains the rule's own declaration, and the same
accident reaches them in opposite directions:
regexp must find nothing, so a self-match is a finding that is
always there
require_regexp must find something, so a self-match is a PASS that is
always there
The second is the quieter one and the worse one. Reproduced: a rule requiring
`Copyright` in every file reports `bad.txt` and never reports
`policy/principles.toml`, because that file carries the word on its own
`require_regexp` line. It meets the requirement by naming it. It is the one file
in the corpus that cannot fail the rule, whatever else it does or stops doing,
and nothing says so -- a loud finding about the wrong file gets read, and a file
that has exempted itself is never mentioned again.
The message is not shared. The three cures are the same, and the sentence before
them is the opposite: a reader told that the file will be REPORTED, when what
actually happens is that it is exempt, goes looking for a finding that is not
there.
Measured over the same 82 policy files: 11 `require_regexp` rules, none of them
self-satisfying with the policy file in scope. Nothing that passes today starts
failing, and the fleet sweep with the release binary confirms it.
551 tests pass, clippy and fmt clean, and every lefthook pre-commit command was
run directly -- including the Python suite, which is what caught the exit-code
assumption in the last change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe configuration loader now detects self-matches for both ChangesSelf-match validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change adds a targeted refusal for self-satisfying Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
+ Coverage 90.34% 90.36% +0.01%
==========================================
Files 35 35
Lines 11147 11168 +21
==========================================
+ Hits 10071 10092 +21
Misses 1076 1076 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Follow-on to #73, which covered
regexpand stopped there.The same accident, going the other way
Both are content searches over a corpus that contains the rule's own declaration:
regexprequire_regexp#73 refused the first. The second is quieter and worse.
Reproduced
A rule requiring
Copyrightin every file, over a tree with one good file and one bad one:policy/principles.tomlis not reported -- it carries the word on its ownrequire_regexpline. It meets the requirement by naming it. It is the one file in the corpus that cannot fail this rule, whatever else it does or stops doing, and nothing ever says so.A loud finding about the wrong file gets read. A file that has exempted itself is never mentioned again.
Now:
The message is not shared, deliberately
The three cures are the same. The sentence before them is the opposite: a reader told the file will be reported, when what actually happens is that it is exempt, goes looking for a finding that is not there.
SelfMatchcarries which direction it is and composes the two halves.Measured
Same 82 policy files: 11
require_regexprules, none self-satisfying with the policy file in scope. Nothing that passes today starts failing; the release-binary fleet sweep confirms 0 newly refused.Verification
cargo test-- 551 pass, 0 fail (549 + 2)cargo clippy --all-targets,cargo fmt --check-- cleanpython -m unittest discover -s tests-- that suite is what caught the exit-code assumption in Refuse a rule that matches its own declaration and selects the file it is in #73 after CI did, so it is in the loop before pushing this timeNew tests: the refusal fires, names
require_regexp, says "exempt" and not "violating this rule"; and a requirement whose pattern is nowhere in its own section still loads.Summary by CodeRabbit