feat(review): Thread EffectiveDialect through run_rules and stub lock-risk checks#123
Merged
feat(review): Thread EffectiveDialect through run_rules and stub lock-risk checks#123
Conversation
This comment has been minimized.
This comment has been minimized.
Schema reviewTip ✅ No risk findings — schema changes look safe to merge. |
run_rules now takes an EffectiveDialect argument so the dispatcher can gate per-rule evaluation against the dialect resolved by the caller. RuleContext carries the dialect through the pass and exposes a new rule_active(rule, selected) helper that combines the existing selected set check with ReviewRuleId::dialect_scope; every existing dispatch site now goes through it instead of reaching for selected.contains directly. Phase 4 lock-risk rules with DialectScope::OneOf will plug into the same helper, but for now Phase 1-3 rules keep their previous behavior because they all carry DialectScope::Any. The relune-app review use case hard-codes EffectiveDialect::Auto for this commit; Phase 4 task 3 introduces ReviewRequest.dialect and swaps the hard-coded value for request.dialect.into(). The dead_code allows on DialectScope and dialect_scope come off now that they have a real caller.
Add empty stub bodies for the four Phase 4 lock-risk rule checks (check_add_index_on_large_table, check_add_fk_on_existing, check_alter_column_type_lock, check_rewrite_table) and dispatch them from run_rules through the existing context.rule_active gate. The stubs return zero findings, so behavior is unchanged for every dialect and every rule selection; the value of this commit is locking down the function signatures, parameter shapes, and dispatch sites that Phase 4 task 4 will fill in. AlterColumnType deliberately runs alongside check_type_narrow on the same ColumnDiff::Modified because the two rules cover lock and data-correctness angles independently, and check_rewrite_table dispatches once per modified table since its trigger conditions are table-level. The missing_const_for_fn allow on each stub and too_many_lines on run_rules are local; they come off naturally when task 4 lands the finding-construction logic.
mhiro2
force-pushed
the
feat/review-run-rules-dialect
branch
from
2d724d6 to
da56d8b
Compare
Code Metrics Report
Details | | main (3a88bcf) | #123 (7675645) | +/- |
|---------------------|----------------|----------------|-------|
- | Coverage | 94.6% | 94.5% | -0.2% |
| Files | 81 | 81 | 0 |
| Lines | 37420 | 37510 | +90 |
+ | Covered | 35431 | 35464 | +33 |
+ | Test Execution Time | 1m49s | 1m25s | -24s |Code coverage of files in pull request scope (93.1% → 91.0%)
Reported by octocov |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_rulesto acceptEffectiveDialectso dialect-scoped rules can gate themselves at evaluation time.RuleContextand add arule_activehelper that consultsReviewRuleId::dialect_scope()instead of rawselected.contains(...)checks.Changes
dialect: EffectiveDialectparameter torun_rulesand propagate it intoRuleContextso rule dispatch can honor the active SQL dialect.selected.contains(...)predicates with a newRuleContext::rule_activehelper that defers toReviewRuleId::dialect_scope()for dialect-scoped rules.relune-appreview usecase callsite and the in-crate test helper to passEffectiveDialect::Auto, drop the now-redundantdead_codeallow onDialectScope, and scrub stale roadmap-task references from doc comments / tests /action/review.sh.check_add_index_on_large_table,check_add_fk_on_existing,check_alter_column_type_lock, andcheck_rewrite_tableso the four lock-risk rule ids dispatch throughrun_rulesahead of their detection logic.RuleContext::rule_active, keeping the call graph ready for the upcoming rule implementations.