diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 4c47988..57d44bd 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -18,7 +18,9 @@ pub mod highlight; pub mod lex_helpers; pub mod outcome; -use crate::dsl::command::Command; +use crate::dsl::command::{ + Command, CompareOp, Expr, Operand, Predicate, RowFilter, +}; use crate::dsl::grammar; use crate::dsl::walker::context::WalkContext; use crate::dsl::walker::driver::{FailureKind, NodeWalkResult, walk_node}; @@ -421,6 +423,163 @@ fn schema_has_column( .is_some_and(|cols| cols.iter().any(|c| c.name.eq_ignore_ascii_case(column))) } +/// The WHERE expression of a filter command, if it has one. +const fn command_where_expr(command: &Command) -> Option<&Expr> { + match command { + Command::Update { + filter: RowFilter::Where(expr), + .. + } + | Command::Delete { + filter: RowFilter::Where(expr), + .. + } + | Command::ShowData { + filter: Some(expr), .. + } => Some(expr), + _ => None, + } +} + +/// A coarse span covering the WHERE clause — from the `where` +/// keyword to the end of input. The validity indicator reads +/// only the severity; the span is for the (eventual) +/// highlight overlay. +fn where_clause_span(path: &MatchedPath, source_len: usize) -> (usize, usize) { + path.items + .iter() + .find(|i| matches!(&i.kind, outcome::MatchedKind::Word("where"))) + .map_or((0, source_len), |w| (w.span.0, source_len)) +} + +/// WARNING diagnostics for a WHERE expression (ADR-0026 §7): +/// a type-mismatched comparison, or `= NULL` / `!= NULL`. +/// Both are valid and runnable — the warning is advisory. +fn expr_warnings( + expr: &Expr, + columns: &[crate::completion::TableColumn], + span: (usize, usize), +) -> Vec { + let mut out = Vec::new(); + collect_expr_warnings(expr, columns, span, &mut out); + out +} + +fn collect_expr_warnings( + expr: &Expr, + columns: &[crate::completion::TableColumn], + span: (usize, usize), + out: &mut Vec, +) { + match expr { + Expr::Or(terms) | Expr::And(terms) => { + for term in terms { + collect_expr_warnings(term, columns, span, out); + } + } + Expr::Not(inner) => collect_expr_warnings(inner, columns, span, out), + Expr::Predicate(predicate) => { + predicate_warnings(predicate, columns, span, out); + } + } +} + +fn predicate_warnings( + predicate: &Predicate, + columns: &[crate::completion::TableColumn], + span: (usize, usize), + out: &mut Vec, +) { + use outcome::{Diagnostic, Severity}; + let warn = |message: String| Diagnostic { + severity: Severity::Warning, + span, + message, + }; + match predicate { + Predicate::Compare { left, op, right } => { + // `= NULL` / `!= NULL`: valid syntax that is never + // true — the user almost certainly means IS NULL. + if matches!(op, CompareOp::Eq | CompareOp::NotEq) + && (is_null_literal(left) || is_null_literal(right)) + { + out.push(warn(crate::friendly::translate( + "diagnostic.eq_null", + &[], + ))); + } else if let Some(message) = + pair_type_mismatch(left, right, columns) + { + out.push(warn(message)); + } + } + Predicate::Between { + target, low, high, .. + } => { + for bound in [low, high] { + if let Some(message) = + pair_type_mismatch(target, bound, columns) + { + out.push(warn(message)); + } + } + } + Predicate::In { target, items, .. } => { + for item in items { + if let Some(message) = + pair_type_mismatch(target, item, columns) + { + out.push(warn(message)); + } + } + } + // `LIKE` is inherently a text-pattern test; flagging a + // non-text target is a future model extension. + // `IS [NOT] NULL` is the *correct* null test — never + // flagged. + Predicate::Like { .. } | Predicate::IsNull { .. } => {} + } +} + +const fn is_null_literal(operand: &Operand) -> bool { + matches!(operand, Operand::Literal(crate::dsl::value::Value::Null)) +} + +/// If one operand is a known column and the other a non-null +/// literal whose type the column cannot hold, the message for +/// a type-mismatch WARNING; otherwise `None` (column-to-column, +/// literal-to-literal, an unknown column — already an ERROR — +/// or a compatible pair). +fn pair_type_mismatch( + a: &Operand, + b: &Operand, + columns: &[crate::completion::TableColumn], +) -> Option { + let (column, literal) = match (a, b) { + (Operand::Column(c), Operand::Literal(v)) + | (Operand::Literal(v), Operand::Column(c)) => (c, v), + _ => return None, + }; + // `null` fits any column; `= NULL` is flagged separately. + if matches!(literal, crate::dsl::value::Value::Null) { + return None; + } + let ty = columns + .iter() + .find(|tc| tc.name.eq_ignore_ascii_case(column))? + .user_type; + if literal.bind_for_column(column, ty).is_ok() { + return None; + } + Some(crate::friendly::translate( + "diagnostic.type_mismatch", + &[ + ("column", column as &dyn std::fmt::Display), + ("type", &ty.keyword() as &dyn std::fmt::Display), + ], + )) +} + /// What the grammar would accept at the end of `source` /// (ADR-0024 §architecture, Phase F walker-driven completion). /// @@ -698,11 +857,25 @@ pub fn walk<'a>( // Schema-existence diagnostics (ADR-0027 §2) layer on top // of a structurally-valid parse; a parse that already // failed gets its ERROR verdict from `outcome`. - let diagnostics = if matches!(final_outcome, WalkOutcome::Match { .. }) { + let mut diagnostics = if matches!(final_outcome, WalkOutcome::Match { .. }) { schema_existence_diagnostics(&path, ctx.schema) } else { Vec::new() }; + // Expression WARNING diagnostics — type-mismatched + // comparisons and `= NULL` (ADR-0026 §7, surfaced through + // ADR-0027's model). Only a successfully-built command has + // a `where` expression to inspect. + if let Some(command) = &cmd + && let Some(expr) = command_where_expr(command) + { + let columns = ctx.current_table_columns.as_deref().unwrap_or(&[]); + diagnostics.extend(expr_warnings( + expr, + columns, + where_clause_span(&path, effective_source.len()), + )); + } let result = WalkResult { outcome: final_outcome, matched_path: path, @@ -1451,6 +1624,61 @@ mod tests { ); } + #[test] + fn input_verdict_type_mismatch_is_warning() { + // `Age` is int; comparing it with a text literal runs, + // but is flagged (ADR-0026 §7). + let schema = + schema_with("Customers", &[("id", Type::Int), ("Age", Type::Int)]); + assert_eq!( + super::input_verdict( + "delete from Customers where Age = 'hello'", + Some(&schema), + ), + Some(super::Severity::Warning), + ); + } + + #[test] + fn input_verdict_eq_null_is_warning() { + let schema = + schema_with("Customers", &[("id", Type::Int), ("Name", Type::Text)]); + assert_eq!( + super::input_verdict( + "delete from Customers where Name = null", + Some(&schema), + ), + Some(super::Severity::Warning), + ); + } + + #[test] + fn input_verdict_compatible_comparison_is_clean() { + let schema = + schema_with("Customers", &[("id", Type::Int), ("Name", Type::Text)]); + assert_eq!( + super::input_verdict( + "delete from Customers where id = 5", + Some(&schema), + ), + None, + ); + } + + #[test] + fn input_verdict_error_outranks_warning() { + // An unknown column (ERROR) alongside `= NULL` + // (WARNING) — the indicator shows the higher severity. + let schema = schema_with("Customers", &[("id", Type::Int)]); + assert_eq!( + super::input_verdict( + "delete from Customers where NoSuchCol = null", + Some(&schema), + ), + Some(super::Severity::Error), + ); + } + #[test] fn walker_parses_insert_with_explicit_column_list() { assert_eq!( diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 4d5a598..61c7e6f 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -38,6 +38,8 @@ /// `(key, expected_placeholders)`. Sorted by key for grep-ability. pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ // ---- Pre-submit diagnostics (ADR-0027) ---- + ("diagnostic.eq_null", &[]), + ("diagnostic.type_mismatch", &["column", "type"]), ("diagnostic.unknown_column", &["name", "table"]), ("diagnostic.unknown_table", &["name"]), // ---- Already-exists collisions (anchor: "already exists") ---- diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 0921ecb..b57c3ce 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -465,6 +465,10 @@ parse: diagnostic: unknown_table: "no such table: `{name}`" unknown_column: "no such column `{name}` on table `{table}`" + # WARNING-severity flags (ADR-0026 §7) — the command still + # runs, but the comparison is very likely not intended. + type_mismatch: "`{column}` is {type} — this comparison uses a value of a different type" + eq_null: "`= NULL` is never true — use `IS NULL` or `IS NOT NULL`" # ---- Project lifecycle event notes ----------------------------------- project: