Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/relune-app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 20 additions & 0 deletions crates/relune-app/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,19 @@ pub struct ReviewRequest {
/// as an input error.
#[serde(default)]
pub severity_overrides: Vec<ReviewSeverityOverride>,
/// 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 {
Expand Down Expand Up @@ -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)]
Expand Down
93 changes: 92 additions & 1 deletion crates/relune-app/src/usecases/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,13 +25,25 @@ pub fn review(request: ReviewRequest) -> Result<ReviewResult, AppError> {

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);
Expand Down Expand Up @@ -295,6 +308,10 @@ fn parse_rule_id(value: &str) -> Result<ReviewRuleId, AppError> {
ReviewRuleId::parse(&normalized).map_err(AppError::input)
}

fn parse_explicit_rule_ids(rules: &[String]) -> Result<Vec<ReviewRuleId>, AppError> {
rules.iter().map(|s| parse_rule_id(s)).collect()
}

fn partition_suppressed(
findings: Vec<RiskFinding>,
except_tables: &[String],
Expand Down Expand Up @@ -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 = "
Expand Down
Loading
Loading