diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index c7d5c5a..2c6c662 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -1106,6 +1106,72 @@ fn compound_arity_diagnostics( diagnostics } +/// `auto_column_overridden` WARNING (ADR-0033 §8.2, sub-phase 3i). +/// +/// A SQL `INSERT` whose explicit column list names a `serial` or +/// `shortid` column is providing a value for a column the playground +/// auto-generates (`serial` via the engine rowid, `shortid` via the +/// worker post-fill). That bypasses the auto-counter / generator and +/// can collide with later auto-generated values — pedagogically worth +/// a nudge, but advisory: the statement still runs (ADR-0027 §1). +/// +/// The conflict-target column list of an `ON CONFLICT (…)` uses a +/// distinct role (`conflict_target_column`), so it is not mistaken +/// for an inserted column here. +fn dml_auto_column_diagnostics( + path: &MatchedPath, + schema: Option<&crate::completion::SchemaCache>, +) -> Vec { + use crate::dsl::grammar::IdentSource; + use crate::dsl::types::Type; + use outcome::{Diagnostic, MatchedKind, Severity}; + + let Some(schema) = schema else { + return Vec::new(); + }; + let Some(target) = path.items.iter().find_map(|it| match it.kind { + MatchedKind::Ident { + source: IdentSource::Tables, + role: "insert_target_table", + } => Some(it.text.as_str()), + _ => None, + }) else { + return Vec::new(); + }; + let Some(cols) = schema.columns_for_table(target) else { + return Vec::new(); + }; + + let mut diagnostics = Vec::new(); + for item in &path.items { + let MatchedKind::Ident { + source: IdentSource::Columns, + role: "insert_column", + } = item.kind + else { + continue; + }; + if let Some(col) = cols + .iter() + .find(|c| c.name.eq_ignore_ascii_case(&item.text)) + && matches!(col.user_type, Type::Serial | Type::ShortId) + { + diagnostics.push(Diagnostic { + severity: Severity::Warning, + span: item.span, + message: crate::friendly::translate( + "diagnostic.auto_column_overridden", + &[ + ("column", &item.text as &dyn std::fmt::Display), + ("type", &col.user_type as &dyn std::fmt::Display), + ], + ), + }); + } + } + diagnostics +} + /// SQL-expression predicate-warning pass (ADR-0032 §11.6 — the /// Phase-1 carry-over gap closure). /// @@ -2167,6 +2233,9 @@ fn walk_one_command<'a>( // operator slot is highlighted rather than the engine // wording shown at execution time. d.extend(compound_arity_diagnostics(&path)); + // ADR-0033 §8.2 — WARNING when a SQL INSERT lists a + // serial/shortid (auto-generated) column explicitly. + d.extend(dml_auto_column_diagnostics(&path, ctx.schema)); // ADR-0032 §10.3 / §11.2 — diagnostics emitted during // the walk by node handlers with direct context the // post-walk passes can't reconstruct (primarily the @@ -3990,6 +4059,54 @@ mod tests { ); } + #[test] + fn auto_column_overridden_fires_on_explicit_serial() { + // ADR-0033 §8.2: listing a serial column explicitly warns. + let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); + let diags = diag_keys("sqlinsert into t (id, name) values (5, 'x')", &schema); + assert!( + diags.iter().any(|d| d.contains("auto-generated")), + "explicit serial value should warn; got {diags:?}", + ); + } + + #[test] + fn auto_column_overridden_fires_on_explicit_shortid() { + let schema = schema_with("t", &[("id", Type::ShortId), ("name", Type::Text)]); + let diags = diag_keys("sqlinsert into t (id, name) values ('abc', 'x')", &schema); + assert!( + diags.iter().any(|d| d.contains("auto-generated")), + "explicit shortid value should warn; got {diags:?}", + ); + } + + #[test] + fn auto_column_overridden_silent_when_omitted() { + // Negative: omitting the auto-gen column does not warn. + let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); + let diags = diag_keys("sqlinsert into t (name) values ('x')", &schema); + assert!( + !diags.iter().any(|d| d.contains("auto-generated")), + "omitting the serial column must not warn; got {diags:?}", + ); + } + + #[test] + fn auto_column_overridden_conflict_target_not_flagged() { + // The ON CONFLICT (id) target names `id` but isn't an + // *inserted* value, so it must not trigger the warning; + // only the inserted `id` in the column list would. + let schema = schema_with("t", &[("id", Type::Serial), ("name", Type::Text)]); + let diags = diag_keys( + "sqlinsert into t (name) values ('x') on conflict (id) do nothing", + &schema, + ); + assert!( + !diags.iter().any(|d| d.contains("auto-generated")), + "conflict-target serial column must not warn; got {diags:?}", + ); + } + #[test] fn upsert_excluded_resolves_in_do_update() { // ADR-0033 §9: inside a DO UPDATE action, `excluded.col` diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 46f34a5..72203c9 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -39,6 +39,7 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ // ---- Pre-submit diagnostics (ADR-0027) ---- ("diagnostic.ambiguous_column", &["column", "qualifiers"]), + ("diagnostic.auto_column_overridden", &["column", "type"]), ("diagnostic.compound_arity_mismatch", &["op", "left_n", "right_n"]), ("diagnostic.cte_arity_mismatch", &["cte", "declared", "actual"]), ("diagnostic.duplicate_cte", &["name"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index d6df488..6fb47bb 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -501,6 +501,8 @@ diagnostic: cte_arity_mismatch: "CTE `{cte}` declares {declared} columns but its body has {actual}" compound_arity_mismatch: "`{op}` requires both sides to have the same number of columns — left has {left_n}, right has {right_n}" duplicate_cte: "duplicate `WITH` table name: `{name}`" + # ADR-0033 §8 — Phase-3 DML diagnostic keys. + auto_column_overridden: "column `{column}` is auto-generated (`{type}`); providing an explicit value bypasses the auto-counter and may collide with later auto-generated values" # Engine-error translations: an engine-rejected SQL statement # reaches the friendly-error layer (ADR-0019) and these keys