uphold 1.7.0: a rule may not be about its own declaration - #75
Conversation
Two load-time refusals, one accident, reached from opposite directions. A policy
file is a tracked file, so a rule's `regexp` and `require_regexp` are inside the
corpus that rule scans, and an unanchored literal matches the line it is written
on:
regexp must find nothing, so a self-match is a finding that is always
there, naming the rule instead of the tree
require_regexp must find something, so a self-match is a pass that is always
there, exempting the policy file from the rule forever
Both are refused only when the rule ALSO selects the file its declaration is in.
That second half is the whole design: measured over 82 policy files and 178
`regexp` rules, 54 matched their own text and none of them selected the file it
was written in, so refusing on the text alone would have failed 54 working
rules. 11 `require_regexp` rules, none self-satisfying in scope. Nothing in the
fleet is refused by either check.
What this replaces is three workarounds that spread with nothing recording any
of them as the answer -- a narrow include, an explicit exclude, and breaking the
literal with a one-character class, nine of those in a single pattern. The third
is also applied where it does nothing: `^Sta[t]us:` appears in three
repositories and the dodge is unnecessary in all of them, because an anchored
pattern cannot match the key it is written under. REFERENCE.md now says so, so
the next author does not have to guess.
Documented pins move to v1.7.0 in README.md and hooks/lefthook.yml. Consumers
pinning v1.6.0 will fail `no-stale-hook-pins` at pre-push until they take this,
which is that rule working rather than a surprise.
|
Warning Review limit reached
Next review available in: 47 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
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 #75 +/- ##
=======================================
Coverage 90.36% 90.36%
=======================================
Files 35 35
Lines 11168 11168
=======================================
Hits 10092 10092
Misses 1076 1076 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Cuts 1.7.0 over #73 and #74, which are already on main.
What is in it
regexpmatching its own declaration -- a finding that is always thererequire_regexpmatching its own declaration -- a pass that is always thereBoth refuse only when the rule also selects the file its declaration is in. Measured across 82 policy files: 54 of 178
regexprules match their own text, none in scope; 11require_regexprules, none in scope. Zero consumers refused.In this PR
Cargo.toml/Cargo.lockto 1.7.0v1.7.0inREADME.md(3) andhooks/lefthook.yml(2)docs/REFERENCE.md: a new section under Rule shape describing both directions, the scope requirement, and the three cures -- including that an anchored pattern needs none of them, which makes the^Sta[t]us:dodge in three repositories deletableConsumers
no-stale-hook-pinsrefuses at pre-push once a newer tag exists, so every consumer pinned at v1.6.0 goes red until it takes this. That is the rule working. The fleet sweep follows the tag; 11 repositories are already red against v1.6.0 and will be fixed by the same pass.Verification
cargo test-- 551 pass, 0 failcargo clippy --all-targets,cargo fmt --check-- cleanuphold --versionreports 1.7.0