feat(review): Wire ReviewRequest.dialect and lock-risk skip diagnostic#124
Merged
feat(review): Wire ReviewRequest.dialect and lock-risk skip diagnostic#124
Conversation
Introduce DiagnosticCode::review_lock_risk_skipped (REVIEW001) and a public lock_risk_skip_diagnostic helper in relune-core. The helper returns an info-level Diagnostic when explicitly opted-in lock-risk rules cannot evaluate under the effective dialect, with distinct messages for Auto, Sqlite, and Postgres/Mysql scope mismatches. The helper preserves the pub(crate) visibility of DialectScope by keeping scope inspection inside the review module.
This comment has been minimized.
This comment has been minimized.
Schema reviewTip ✅ No risk findings — schema changes look safe to merge. |
Add a ReviewRequest.dialect field with a with_dialect builder, thread the resulting EffectiveDialect into run_rules instead of hard-coding Auto, and emit the REVIEW001 skip diagnostic when an explicitly requested lock-risk rule cannot run under the effective dialect. Default profiles (no explicit rules) remain silent to keep CI noise in check. Add CLI/wasm shims that pass SqlDialect::default() until the upcoming dialect plumbing tasks expose the user-supplied value.
The golden fixture runner now descends into subdirectories so per-rule fixtures can live under fixtures/review/<group>/<rule-dialect>/..., with each leaf detected by the presence of before.sql + after.sql. An optional per-fixture meta.toml configures dialect/rules/except filters, and an optional expected_diagnostics.json locks down the diagnostics payload (with UPDATE_FIXTURES=1 honored for both expected files). Fixtures without expected_diagnostics.json must keep producing no diagnostics, matching the existing flat fixtures.
mhiro2
force-pushed
the
feat/review-request-dialect
branch
from
fa1e50d to
bfc7c7a
Compare
Code Metrics Report
Details | | main (9e06a63) | #124 (02d98f7) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 94.5% | 94.5% | +0.0% |
| Files | 81 | 81 | 0 |
| Lines | 37510 | 37678 | +168 |
+ | Covered | 35464 | 35629 | +165 |
| Test Execution Time | 1m34s | 1m34s | 0s |Code coverage of files in pull request scope (93.6% → 93.9%)
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
ReviewRequest.dialect(with builder) and thread the resultingEffectiveDialectthrough the review usecase, replacing the hard-codedAuto.REVIEW001(review_lock_risk_skipped) and alock_risk_skip_diagnostichelper that emits an info-level diagnostic only when an explicitly opted-in lock-risk rule cannot evaluate under the effective dialect; the default profile (no--rules) stays silent.meta.toml, recursive directory walk, and optionalexpected_diagnostics.jsonso per-rule lock-risk fixtures can live underfixtures/review/<group>/<rule-dialect>/....Changes
DiagnosticCode::review_lock_risk_skipped()(REVIEW001) and a publiclock_risk_skip_diagnosticinrelune-corethat branches onAuto/Sqlite/Postgres|Mysqlscope mismatches with distinct messages.DialectScopepub(crate)by inspecting it inside the review module.dialect: SqlDialect(defaulting via serde) andwith_dialectbuilder onReviewRequest, then thread the resultingEffectiveDialectintorun_rulesinstead of the hard-codedAuto.SqlDialect::default()until the upcoming dialect-plumbing tasks expose the user-supplied value.fixtures/review/recursively and treat any directory containingbefore.sql+after.sqlas a leaf fixture so per-rule sets can live under nested groups.meta.toml(dialect/rules/except_rules/except_tables) and an optionalexpected_diagnostics.json, withUPDATE_FIXTURES=1regenerating both expected files.