diff --git a/Cargo.lock b/Cargo.lock index 00b59b6..306e98b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2157,6 +2157,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", + "toml", "tracing", "web-time", ] diff --git a/crates/relune-app/Cargo.toml b/crates/relune-app/Cargo.toml index f0b64c7..f6e3f1e 100644 --- a/crates/relune-app/Cargo.toml +++ b/crates/relune-app/Cargo.toml @@ -28,6 +28,7 @@ introspect = ["dep:relune-introspect", "dep:tokio"] insta.workspace = true relune-testkit = { path = "../relune-testkit" } tempfile = "3" +toml = "1.1" [lints] workspace = true diff --git a/crates/relune-app/src/request.rs b/crates/relune-app/src/request.rs index 49e72f9..eea629b 100644 --- a/crates/relune-app/src/request.rs +++ b/crates/relune-app/src/request.rs @@ -644,6 +644,19 @@ pub struct ReviewRequest { /// as an input error. #[serde(default)] pub severity_overrides: Vec, + /// Effective SQL dialect used by lock-risk rule evaluation. + /// + /// Independent from the parser dialect carried inside each + /// `InputSource`: that one only governs lexing of the supplied SQL, + /// while this field is the single source for rule-evaluation + /// dialect. Lock-risk rules require this to resolve to `Postgres` + /// or `Mysql`; under `Auto` / `Sqlite` they are silently skipped + /// (an info-level diagnostic is emitted only when a lock-risk rule + /// was explicitly opted in via `rules`). + /// + /// Defaults to `SqlDialect::Auto`. + #[serde(default)] + pub dialect: SqlDialect, } impl ReviewRequest { @@ -704,6 +717,13 @@ impl ReviewRequest { self.severity_overrides = overrides; self } + + /// Set the effective dialect used by lock-risk rule evaluation. + #[must_use] + pub const fn with_dialect(mut self, dialect: SqlDialect) -> Self { + self.dialect = dialect; + self + } } #[cfg(test)] diff --git a/crates/relune-app/src/usecases/review.rs b/crates/relune-app/src/usecases/review.rs index 63fc0d5..076556a 100644 --- a/crates/relune-app/src/usecases/review.rs +++ b/crates/relune-app/src/usecases/review.rs @@ -6,6 +6,7 @@ use std::fmt::Write; use relune_core::{ EffectiveDialect, ReviewResult as CoreReviewResult, ReviewRuleId, ReviewRuleMetadata, ReviewSeverity, ReviewSeverityOverride, ReviewSummary, RiskFinding, diff_schemas, + lock_risk_skip_diagnostic, }; use crate::error::AppError; @@ -24,13 +25,25 @@ pub fn review(request: ReviewRequest) -> Result { let applied_rules = resolve_active_rules(&request.rules, &request.except_rules)?; let override_map = build_override_map(&request.severity_overrides)?; + let effective: EffectiveDialect = request.dialect.into(); let mut raw_findings = relune_core::run_rules( &schema_diff, &before_schema, &after_schema, &applied_rules, - EffectiveDialect::Auto, + effective, ); + + // Surface a single info-level diagnostic when the caller explicitly + // opted in to a lock-risk rule that the resolved dialect cannot + // evaluate. Default profiles stay silent to avoid CI noise. + let explicit_rule_ids = parse_explicit_rule_ids(&request.rules)?; + if let Some(diagnostic) = + lock_risk_skip_diagnostic(&explicit_rule_ids, &applied_rules, effective) + { + diagnostics.push(diagnostic); + } + apply_severity_overrides(&mut raw_findings, &override_map); let (findings, suppressed) = partition_suppressed(raw_findings, &request.except_tables); @@ -295,6 +308,10 @@ fn parse_rule_id(value: &str) -> Result { ReviewRuleId::parse(&normalized).map_err(AppError::input) } +fn parse_explicit_rule_ids(rules: &[String]) -> Result, AppError> { + rules.iter().map(|s| parse_rule_id(s)).collect() +} + fn partition_suppressed( findings: Vec, except_tables: &[String], @@ -587,6 +604,80 @@ mod tests { } } + #[test] + fn dialect_default_is_auto_and_emits_no_diagnostic() { + // Default profile (no `request.rules`) under `Auto` should + // produce zero findings and zero lock-risk skip diagnostics. + let sql = "CREATE TABLE users (id INT PRIMARY KEY);"; + let result = run(ReviewRequest::from_sql(sql, sql)); + assert!(result.diagnostics.is_empty()); + assert_eq!( + relune_core::SqlDialect::default(), + relune_core::SqlDialect::Auto, + ); + } + + #[test] + fn explicit_lock_risk_opt_in_under_auto_emits_skip_diagnostic() { + let sql = "CREATE TABLE users (id INT PRIMARY KEY);"; + let request = ReviewRequest::from_sql(sql, sql) + .with_rules(vec!["risk/add-index-on-large-table".to_string()]); + let result = run(request); + assert_eq!(result.diagnostics.len(), 1); + let diag = &result.diagnostics[0]; + assert_eq!(diag.severity, relune_core::Severity::Info); + assert!( + diag.message + .contains("Lock-risk review rules require an explicit --dialect"), + "unexpected message: {}", + diag.message, + ); + } + + #[test] + fn explicit_opt_in_under_postgres_does_not_emit_skip_diagnostic() { + let sql = "CREATE TABLE users (id INT PRIMARY KEY);"; + let request = ReviewRequest::from_sql(sql, sql) + .with_rules(vec!["risk/add-index-on-large-table".to_string()]) + .with_dialect(relune_core::SqlDialect::Postgres); + let result = run(request); + assert!(result.diagnostics.is_empty()); + } + + #[test] + fn explicit_opt_in_dialect_mismatch_emits_skip_diagnostic() { + let sql = "CREATE TABLE users (id INT PRIMARY KEY);"; + let request = ReviewRequest::from_sql(sql, sql) + .with_rules(vec!["risk/rewrite-table".to_string()]) + .with_dialect(relune_core::SqlDialect::Postgres); + let result = run(request); + assert_eq!(result.diagnostics.len(), 1); + assert!( + result.diagnostics[0] + .message + .contains("require dialect mysql"), + "unexpected message: {}", + result.diagnostics[0].message, + ); + } + + #[test] + fn except_rule_suppresses_skip_diagnostic_for_opted_in_lock_risk() { + // The user opted in via `--rules`, then removed the same rule + // via `--except-rule`. Nothing was actually attempted, so no + // skip diagnostic should fire (and the request must keep at + // least one active rule to avoid the "no rules remain" error). + let sql = "CREATE TABLE users (id INT PRIMARY KEY);"; + let request = ReviewRequest::from_sql(sql, sql) + .with_rules(vec![ + "risk/add-index-on-large-table".to_string(), + "risk/fk-without-index".to_string(), + ]) + .with_except_rules(vec!["risk/add-index-on-large-table".to_string()]); + let result = run(request); + assert!(result.diagnostics.is_empty()); + } + #[test] fn format_markdown_uses_bullet_per_finding() { let before = " diff --git a/crates/relune-app/tests/review_fixtures.rs b/crates/relune-app/tests/review_fixtures.rs index 0952a44..85bc9fe 100644 --- a/crates/relune-app/tests/review_fixtures.rs +++ b/crates/relune-app/tests/review_fixtures.rs @@ -1,83 +1,251 @@ //! Golden tests for the migration risk review pipeline. //! -//! Each subdirectory under `fixtures/review/` provides `before.sql` and -//! `after.sql`. The test runs the review pipeline and asserts the -//! resulting `findings` array matches the persisted `expected.json`. +//! Each fixture directory under `fixtures/review/` provides +//! `before.sql` and `after.sql`. The runner walks the directory tree +//! recursively so per-rule fixture groups can live in subdirectories +//! (`fixtures/review/lock-risk/-/...`). //! -//! Set `UPDATE_FIXTURES=1` to regenerate `expected.json` from the -//! current pipeline output. +//! An optional per-fixture `meta.toml` configures the request; an +//! optional `expected_diagnostics.json` locks down the diagnostics +//! payload. Fixtures without `meta.toml` keep running with default +//! settings and must produce zero diagnostics. +//! +//! Set `UPDATE_FIXTURES=1` to regenerate `expected.json` (and any +//! `expected_diagnostics.json` already present) from the current +//! pipeline output. use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; -use relune_app::{ReviewRequest, review}; +use relune_app::{InputSource, ReviewRequest, review}; +use relune_core::SqlDialect; use relune_testkit::workspace_root; +use serde::Deserialize; + +/// Optional per-fixture configuration loaded from `meta.toml`. +#[derive(Debug, Default, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "snake_case")] +struct FixtureMeta { + #[serde(default)] + dialect: Option, + #[serde(default)] + rules: Option>, + #[serde(default)] + except_rules: Option>, + #[serde(default)] + except_tables: Option>, +} #[test] fn review_golden_fixtures_match_expected_findings() { let root = workspace_root().join("fixtures").join("review"); - let mut entries: Vec<_> = fs::read_dir(&root) - .expect("fixtures/review directory should exist") - .filter_map(Result::ok) - .filter(|entry| entry.file_type().is_ok_and(|ft| ft.is_dir())) - .collect(); - entries.sort_by_key(std::fs::DirEntry::file_name); + let mut fixtures = Vec::new(); + collect_fixtures(&root, &root, &mut fixtures); + fixtures.sort_by(|a, b| a.relative.cmp(&b.relative)); assert!( - !entries.is_empty(), - "expected at least one review fixture directory" + !fixtures.is_empty(), + "expected at least one review fixture directory under {}", + root.display(), ); let update = std::env::var_os("UPDATE_FIXTURES").is_some(); let mut failures = Vec::new(); - for entry in entries { - let path = entry.path(); - let name = entry.file_name().to_string_lossy().into_owned(); - - let before_path = path.join("before.sql"); - let after_path = path.join("after.sql"); - let expected_path = path.join("expected.json"); + for fixture in fixtures { + if let Err(err) = run_fixture(&fixture, update) { + failures.push(err); + } + } - let before = read_or_skip(&before_path); - let after = read_or_skip(&after_path); + assert!( + failures.is_empty(), + "{} fixture mismatch(es):\n{}", + failures.len(), + failures.join("\n---\n"), + ); +} - let result = review(ReviewRequest::from_sql(before, after)) - .unwrap_or_else(|err| panic!("review failed for fixture {name}: {err:?}")); - let actual = serde_json::to_value(&result.review.findings).unwrap(); - let actual_pretty = serde_json::to_string_pretty(&actual).unwrap(); +struct Fixture { + /// Absolute path to the fixture directory. + path: PathBuf, + /// Path relative to `fixtures/review/`, used for stable ordering + /// and human-readable error messages (`lock-risk/foo/bar`). + relative: String, +} - if update { - fs::write(&expected_path, format!("{actual_pretty}\n")) - .unwrap_or_else(|err| panic!("failed to write {}: {err}", expected_path.display())); +fn collect_fixtures(root: &Path, dir: &Path, out: &mut Vec) { + let entries = fs::read_dir(dir) + .unwrap_or_else(|err| panic!("failed to read directory {}: {err}", dir.display())); + for entry in entries.filter_map(Result::ok) { + let path = entry.path(); + if !entry.file_type().is_ok_and(|ft| ft.is_dir()) { continue; } + // A directory is treated as a fixture iff both before.sql and + // after.sql exist; otherwise descend looking for leaf fixtures. + if path.join("before.sql").exists() && path.join("after.sql").exists() { + let relative = path.strip_prefix(root).map_or_else( + |_| path.to_string_lossy().into_owned(), + |p| p.to_string_lossy().into_owned(), + ); + out.push(Fixture { path, relative }); + } else { + collect_fixtures(root, &path, out); + } + } +} + +fn run_fixture(fixture: &Fixture, update: bool) -> Result<(), String> { + let before = read_required(&fixture.path.join("before.sql")); + let after = read_required(&fixture.path.join("after.sql")); + let meta = load_meta(&fixture.path.join("meta.toml")); + + let request = build_request(before, after, meta); + let result = review(request).map_err(|err| { + format!( + "fixture `{name}` failed to run: {err:?}", + name = fixture.relative + ) + })?; + + check_findings(fixture, &result.review.findings, update)?; + check_diagnostics(fixture, &result.diagnostics, update)?; + + Ok(()) +} + +fn build_request(before: String, after: String, meta: FixtureMeta) -> ReviewRequest { + let mut request = ReviewRequest::from_sql(before, after); + if let Some(dialect) = meta.dialect { + request = request.with_dialect(dialect); + request = with_parser_dialect(request, dialect); + } + if let Some(rules) = meta.rules { + request = request.with_rules(rules); + } + if let Some(except_rules) = meta.except_rules { + request = request.with_except_rules(except_rules); + } + if let Some(except_tables) = meta.except_tables { + request = request.with_except_tables(except_tables); + } + request +} + +fn check_findings( + fixture: &Fixture, + findings: &T, + update: bool, +) -> Result<(), String> { + let actual = serde_json::to_value(findings).unwrap(); + let actual_pretty = serde_json::to_string_pretty(&actual).unwrap(); + let path = fixture.path.join("expected.json"); + + if update { + write_expected(&path, &actual_pretty); + return Ok(()); + } + + let expected_pretty = fs::read_to_string(&path).map_err(|err| { + format!( + "fixture `{}` missing expected.json (set UPDATE_FIXTURES=1): {err}", + fixture.relative, + ) + })?; + let expected: serde_json::Value = serde_json::from_str(&expected_pretty).map_err(|err| { + format!( + "fixture `{}` has invalid JSON in expected.json: {err}", + fixture.relative, + ) + })?; + if actual != expected { + return Err(format!( + "fixture `{name}` findings mismatch.\n expected: {expected_pretty}\n actual: {actual_pretty}", + name = fixture.relative + )); + } + Ok(()) +} + +fn check_diagnostics( + fixture: &Fixture, + diagnostics: &[T], + update: bool, +) -> Result<(), String> { + let actual = serde_json::to_value(diagnostics).unwrap(); + let actual_pretty = serde_json::to_string_pretty(&actual).unwrap(); + let path = fixture.path.join("expected_diagnostics.json"); - let expected_pretty = fs::read_to_string(&expected_path).unwrap_or_else(|err| { - panic!( - "failed to read {} (set UPDATE_FIXTURES=1): {err}", - expected_path.display() - ) - }); - let expected: serde_json::Value = serde_json::from_str(&expected_pretty) - .unwrap_or_else(|err| panic!("invalid JSON in {}: {err}", expected_path.display())); - - if actual != expected { - failures.push(format!( - "fixture `{name}` mismatched.\n expected: {expected_pretty}\n actual: {actual_pretty}" + if !path.exists() { + if !diagnostics.is_empty() { + return Err(format!( + "fixture `{name}` produced unexpected diagnostics (add expected_diagnostics.json to lock them down): {actual_pretty}", + name = fixture.relative )); } + return Ok(()); } - assert!( - failures.is_empty(), - "{} fixture mismatch(es):\n{}", - failures.len(), - failures.join("\n---\n"), - ); + if update { + write_expected(&path, &actual_pretty); + return Ok(()); + } + + let expected_pretty = fs::read_to_string(&path).map_err(|err| { + format!( + "fixture `{}` failed to read expected_diagnostics.json: {err}", + fixture.relative, + ) + })?; + let expected: serde_json::Value = serde_json::from_str(&expected_pretty).map_err(|err| { + format!( + "fixture `{}` has invalid JSON in expected_diagnostics.json: {err}", + fixture.relative, + ) + })?; + if actual != expected { + return Err(format!( + "fixture `{name}` diagnostics mismatch.\n expected: {expected_pretty}\n actual: {actual_pretty}", + name = fixture.relative + )); + } + Ok(()) } -fn read_or_skip(path: &Path) -> String { +fn write_expected(path: &Path, pretty: &str) { + fs::write(path, format!("{pretty}\n")) + .unwrap_or_else(|err| panic!("failed to write {}: {err}", path.display())); +} + +fn read_required(path: &Path) -> String { fs::read_to_string(path) .unwrap_or_else(|err| panic!("failed to read fixture file {}: {err}", path.display())) } + +fn load_meta(path: &Path) -> FixtureMeta { + if !path.exists() { + return FixtureMeta::default(); + } + let text = fs::read_to_string(path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); + toml::from_str(&text) + .unwrap_or_else(|err| panic!("invalid meta.toml at {}: {err}", path.display())) +} + +/// Re-tag the parser dialect on every `SqlText` input so the lexer +/// uses the right reserved-word set when fixtures pin a dialect. +/// `SchemaJson` inputs are untouched. +fn with_parser_dialect(mut request: ReviewRequest, dialect: SqlDialect) -> ReviewRequest { + request.before = retag_parser_dialect(request.before, dialect); + request.after = retag_parser_dialect(request.after, dialect); + request +} + +fn retag_parser_dialect(input: InputSource, dialect: SqlDialect) -> InputSource { + match input { + InputSource::SqlText { sql, .. } => InputSource::sql_text_with_dialect(sql, dialect), + InputSource::SqlFile { path, .. } => InputSource::sql_file_with_dialect(path, dialect), + other => other, + } +} diff --git a/crates/relune-cli/src/commands/review.rs b/crates/relune-cli/src/commands/review.rs index 86bce43..90c4549 100644 --- a/crates/relune-cli/src/commands/review.rs +++ b/crates/relune-cli/src/commands/review.rs @@ -55,6 +55,7 @@ pub fn run_review( except_tables: merged.except_tables, deny, severity_overrides: merged.severity_overrides, + dialect: relune_core::SqlDialect::default(), }; let result = review(request) diff --git a/crates/relune-core/src/diagnostic.rs b/crates/relune-core/src/diagnostic.rs index f734715..696275a 100644 --- a/crates/relune-core/src/diagnostic.rs +++ b/crates/relune-core/src/diagnostic.rs @@ -206,4 +206,11 @@ pub mod codes { pub fn lint_orphan_table() -> DiagnosticCode { DiagnosticCode::new("LINT", 2) } + + /// Returns the code emitted when lock-risk review rules are skipped + /// because the effective dialect is not in their dialect scope. + #[must_use] + pub fn review_lock_risk_skipped() -> DiagnosticCode { + DiagnosticCode::new("REVIEW", 1) + } } diff --git a/crates/relune-core/src/lib.rs b/crates/relune-core/src/lib.rs index ba39c13..e5c786a 100644 --- a/crates/relune-core/src/lib.rs +++ b/crates/relune-core/src/lib.rs @@ -68,5 +68,5 @@ pub use model::{ }; pub use review::{ EffectiveDialect, ReviewResult, ReviewRuleId, ReviewRuleMetadata, ReviewSeverity, - ReviewSeverityOverride, ReviewSummary, RiskFinding, run_rules, + ReviewSeverityOverride, ReviewSummary, RiskFinding, lock_risk_skip_diagnostic, run_rules, }; diff --git a/crates/relune-core/src/review.rs b/crates/relune-core/src/review.rs index 9572a5e..2d21764 100644 --- a/crates/relune-core/src/review.rs +++ b/crates/relune-core/src/review.rs @@ -9,7 +9,9 @@ //! lint severity describes "schema quality". use crate::SqlDialect; +use crate::diagnostic::{Diagnostic, codes}; use serde::{Deserialize, Serialize}; +use std::collections::HashSet; use std::fmt; use std::str::FromStr; @@ -85,6 +87,105 @@ impl From for SqlDialect { } } +/// Build the info-level diagnostic that explains why explicitly +/// opted-in lock-risk rules produced no findings under the resolved +/// effective dialect. +/// +/// Returns `Some(diagnostic)` only when: +/// 1. `explicit_rule_ids` is non-empty (the user opted in to a specific +/// rule set via `--rules` / `[review.rules]` — default profile runs +/// return `None` to avoid CI noise), AND +/// 2. at least one `applied_rules` entry that is also in +/// `explicit_rule_ids` is dialect-scoped and would not fire under +/// `dialect`. +/// +/// The single returned diagnostic aggregates every skipped rule. The +/// caller pushes it into the result `diagnostics` vector untouched. +#[must_use] +pub fn lock_risk_skip_diagnostic( + explicit_rule_ids: &[ReviewRuleId], + applied_rules: &[ReviewRuleId], + dialect: EffectiveDialect, +) -> Option { + if explicit_rule_ids.is_empty() { + return None; + } + + let explicit: HashSet = explicit_rule_ids.iter().copied().collect(); + let mut skipped: Vec<(ReviewRuleId, &'static [SqlDialect])> = applied_rules + .iter() + .copied() + .filter(|rule| explicit.contains(rule)) + .filter_map(|rule| match rule.dialect_scope() { + DialectScope::Any => None, + DialectScope::OneOf(scopes) => { + if scope_matches(scopes, dialect) { + None + } else { + Some((rule, scopes)) + } + } + }) + .collect(); + + if skipped.is_empty() { + return None; + } + + skipped.sort_by_key(|(rule, _)| rule.as_str()); + let count = skipped.len(); + + let message = match dialect { + EffectiveDialect::Auto => format!( + "Lock-risk review rules require an explicit --dialect (postgres or mysql); skipped {count} rule(s)." + ), + EffectiveDialect::Sqlite => { + format!("Lock-risk review rules are not defined for sqlite; skipped {count} rule(s).") + } + EffectiveDialect::Postgres | EffectiveDialect::Mysql => { + // Case (b): effective dialect is lock-risk-capable but the + // opted-in rule's scope still does not include it. + let detail = if let [(rule, scopes)] = skipped.as_slice() { + format!( + "{} require dialect {}", + rule.as_str(), + join_dialects(scopes), + ) + } else { + skipped + .iter() + .map(|(rule, scopes)| { + format!("{} requires {}", rule.as_str(), join_dialects(scopes)) + }) + .collect::>() + .join("; ") + }; + format!( + "Lock-risk rule(s) {detail}; effective dialect is {dialect}. Skipped {count} rule(s).", + ) + } + }; + + Some(Diagnostic::info(codes::review_lock_risk_skipped(), message)) +} + +fn scope_matches(scopes: &[SqlDialect], dialect: EffectiveDialect) -> bool { + match dialect { + EffectiveDialect::Auto => false, + EffectiveDialect::Postgres => scopes.contains(&SqlDialect::Postgres), + EffectiveDialect::Mysql => scopes.contains(&SqlDialect::Mysql), + EffectiveDialect::Sqlite => scopes.contains(&SqlDialect::Sqlite), + } +} + +fn join_dialects(scopes: &[SqlDialect]) -> String { + scopes + .iter() + .map(SqlDialect::to_string) + .collect::>() + .join("|") +} + /// Severity scale for migration risk findings. /// /// Ordered `Info < Warning < Caution < Breaking` so callers can use `>=` @@ -766,4 +867,74 @@ mod tests { assert!(summary.has_findings_at_or_above(ReviewSeverity::Warning)); assert!(summary.has_findings_at_or_above(ReviewSeverity::Info)); } + + #[test] + fn lock_risk_skip_diagnostic_silent_on_default_profile() { + // Empty `explicit_rule_ids` represents the default profile path + // where every rule is active by default but no rule was opted + // in by name. The diagnostic must stay silent to avoid CI noise. + let applied = ReviewRuleId::all_rules().to_vec(); + assert!(lock_risk_skip_diagnostic(&[], &applied, EffectiveDialect::Auto).is_none()); + assert!(lock_risk_skip_diagnostic(&[], &applied, EffectiveDialect::Sqlite).is_none()); + } + + #[test] + fn lock_risk_skip_diagnostic_for_auto_dialect() { + let explicit = vec![ReviewRuleId::AddIndexOnLargeTable]; + let applied = explicit.clone(); + let diagnostic = lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Auto) + .expect("auto + lock-risk opt-in should produce a diagnostic"); + assert_eq!(diagnostic.severity, crate::Severity::Info); + assert_eq!(diagnostic.code, codes::review_lock_risk_skipped()); + assert_eq!( + diagnostic.message, + "Lock-risk review rules require an explicit --dialect (postgres or mysql); skipped 1 rule(s)." + ); + } + + #[test] + fn lock_risk_skip_diagnostic_for_sqlite() { + let explicit = vec![ + ReviewRuleId::AddIndexOnLargeTable, + ReviewRuleId::AlterColumnType, + ]; + let applied = explicit.clone(); + let diagnostic = lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Sqlite) + .expect("sqlite + lock-risk opt-in should produce a diagnostic"); + assert_eq!( + diagnostic.message, + "Lock-risk review rules are not defined for sqlite; skipped 2 rule(s)." + ); + } + + #[test] + fn lock_risk_skip_diagnostic_for_dialect_mismatch_single() { + let explicit = vec![ReviewRuleId::RewriteTable]; + let applied = explicit.clone(); + let diagnostic = lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Postgres) + .expect("postgres + mysql-only opt-in should produce a diagnostic"); + assert_eq!( + diagnostic.message, + "Lock-risk rule(s) risk/rewrite-table require dialect mysql; effective dialect is postgres. Skipped 1 rule(s)." + ); + } + + #[test] + fn lock_risk_skip_diagnostic_returns_none_when_scope_matches() { + let explicit = vec![ReviewRuleId::AddIndexOnLargeTable]; + let applied = explicit.clone(); + assert!( + lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Postgres).is_none() + ); + assert!(lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Mysql).is_none()); + } + + #[test] + fn lock_risk_skip_diagnostic_ignores_rules_dropped_from_applied() { + // User opted in but `--except-rule` removed it: nothing to + // diagnose because the rule was never going to be evaluated. + let explicit = vec![ReviewRuleId::AddIndexOnLargeTable]; + let applied: Vec = vec![]; + assert!(lock_risk_skip_diagnostic(&explicit, &applied, EffectiveDialect::Auto).is_none()); + } } diff --git a/crates/relune-wasm/src/request.rs b/crates/relune-wasm/src/request.rs index e8a0210..9a9e77a 100644 --- a/crates/relune-wasm/src/request.rs +++ b/crates/relune-wasm/src/request.rs @@ -507,6 +507,7 @@ impl WasmReviewRequest { except_tables: self.except_tables.clone(), deny: self.deny, severity_overrides: self.severity_overrides.clone(), + dialect: SqlDialect::default(), }) } }