From 827b47f88f9dfb67fe1bfe29812232c74e60ec4b Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Tue, 19 May 2026 07:15:58 +0000 Subject: [PATCH] walker: schema-existence ERROR diagnostics (ADR-0027 step B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MatchedKind::Ident` now carries its `IdentSource`. A post-walk pass over a structurally-valid parse flags a matched `Tables` ident that is absent from the schema, or a `Columns` ident absent from the table in scope, as an ERROR diagnostic — the command parses but would fail at execution (ADR-0027 §2). New behaviour: an unknown table / column used to parse cleanly and fail only when run. Column scope is resolved by one left-to-right pass over the matched path (every command places its table ident before the columns that belong to it); an unknown table clears the scope, so its columns are not cascaded into a second diagnostic. New catalog keys `diagnostic.unknown_table` / `diagnostic.unknown_column`. --- src/dsl/grammar/app.rs | 2 +- src/dsl/grammar/data.rs | 9 +- src/dsl/grammar/ddl.rs | 8 +- src/dsl/grammar/expr.rs | 2 +- src/dsl/walker/driver.rs | 2 +- src/dsl/walker/mod.rs | 140 +++++++++++++++++++++++++++++++- src/dsl/walker/outcome.rs | 10 ++- src/friendly/keys.rs | 3 + src/friendly/strings/en-US.yaml | 8 ++ 9 files changed, 169 insertions(+), 15 deletions(-) diff --git a/src/dsl/grammar/app.rs b/src/dsl/grammar/app.rs index fdee2ef..74a1d70 100644 --- a/src/dsl/grammar/app.rs +++ b/src/dsl/grammar/app.rs @@ -151,7 +151,7 @@ fn build_import(path: &MatchedPath) -> Result { .map(|i| i.text.clone()) .unwrap_or_default(); let target = path - .find(|i| matches!(&i.kind, MatchedKind::Ident { role } if *role == "target")) + .find(|i| matches!(&i.kind, MatchedKind::Ident { role, .. } if *role == "target")) .map(|i| i.text.clone()); Ok(Command::App(AppCommand::Import { path: bare_path, diff --git a/src/dsl/grammar/data.rs b/src/dsl/grammar/data.rs index a2eef8d..f75d98c 100644 --- a/src/dsl/grammar/data.rs +++ b/src/dsl/grammar/data.rs @@ -316,7 +316,7 @@ const DELETE_SHAPE: Node = Node::Seq(DELETE_NODES); fn ident_text<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> { path.items.iter().find_map(|i| match &i.kind { - MatchedKind::Ident { role: r } if *r == role => Some(i.text.as_str()), + MatchedKind::Ident { role: r, .. } if *r == role => Some(i.text.as_str()), _ => None, }) } @@ -450,7 +450,7 @@ fn build_insert(path: &MatchedPath) -> Result { let table_idx = path .items .iter() - .position(|i| matches!(&i.kind, MatchedKind::Ident { role: "table_name" })) + .position(|i| matches!(&i.kind, MatchedKind::Ident { role: "table_name", .. })) .ok_or_else(|| ValidationError { message_key: "parse.error_wrapper", args: vec![("detail", "missing table".to_string())], @@ -498,6 +498,7 @@ fn build_insert(path: &MatchedPath) -> Result { .filter_map(|i| match &i.kind { MatchedKind::Ident { role: "insert_first_item", + .. } => Some(i.text.clone()), _ => None, }) @@ -551,6 +552,7 @@ fn build_insert(path: &MatchedPath) -> Result { .filter_map(|i| match &i.kind { MatchedKind::Ident { role: "insert_first_item", + .. } => Some(i.text.clone()), _ => None, }) @@ -619,7 +621,8 @@ fn collect_assignments( if matches!( item.kind, MatchedKind::Ident { - role: "update_set_column" + role: "update_set_column", + .. } ) { let column = item.text.clone(); diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index a492104..97d15f2 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -475,7 +475,7 @@ const CHANGE_COLUMN: Node = Node::Seq(CHANGE_COLUMN_NODES); /// First ident whose role matches. fn ident<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> { path.items.iter().find_map(|i| match &i.kind { - MatchedKind::Ident { role: r } if *r == role => Some(i.text.as_str()), + MatchedKind::Ident { role: r, .. } if *r == role => Some(i.text.as_str()), _ => None, }) } @@ -495,7 +495,7 @@ fn collect_idents(path: &MatchedPath, role: &str) -> Vec { path.items .iter() .filter_map(|i| match &i.kind { - MatchedKind::Ident { role: r } if *r == role => Some(i.text.clone()), + MatchedKind::Ident { role: r, .. } if *r == role => Some(i.text.clone()), _ => None, }) .collect() @@ -862,7 +862,7 @@ fn build_create_table(path: &MatchedPath) -> Result { .items .iter() .filter_map(|i| match &i.kind { - MatchedKind::Ident { role: "col_name" } => Some(i.text.clone()), + MatchedKind::Ident { role: "col_name", .. } => Some(i.text.clone()), _ => None, }) .collect(); @@ -870,7 +870,7 @@ fn build_create_table(path: &MatchedPath) -> Result { .items .iter() .filter_map(|i| match &i.kind { - MatchedKind::Ident { role: "col_type" } => Some(i.text.as_str()), + MatchedKind::Ident { role: "col_type", .. } => Some(i.text.as_str()), _ => None, }) .collect(); diff --git a/src/dsl/grammar/expr.rs b/src/dsl/grammar/expr.rs index 38c2ade..c6a70d5 100644 --- a/src/dsl/grammar/expr.rs +++ b/src/dsl/grammar/expr.rs @@ -525,7 +525,7 @@ impl<'a> ExprParser<'a> { .advance() .ok_or_else(|| drift_error("expected an operand"))?; match &item.kind { - MatchedKind::Ident { role: "expr_column" } => { + MatchedKind::Ident { role: "expr_column", .. } => { Ok(Operand::Column(item.text.clone())) } MatchedKind::Word("null") => Ok(Operand::Literal(Value::Null)), diff --git a/src/dsl/walker/driver.rs b/src/dsl/walker/driver.rs index 6af3dff..900d9bb 100644 --- a/src/dsl/walker/driver.rs +++ b/src/dsl/walker/driver.rs @@ -434,7 +434,7 @@ fn walk_ident( .push(canonical); } path.push(MatchedItem { - kind: MatchedKind::Ident { role }, + kind: MatchedKind::Ident { role, source: src }, text, span: (start, end), }); diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 794d119..4c47988 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -330,6 +330,97 @@ pub fn input_verdict( outcome_severity.into_iter().chain(diag_severity).max() } +/// Schema-existence diagnostics (ADR-0027 §2). +/// +/// A matched `IdentSource::Tables` token whose name is not in +/// the schema — or a `Columns` token absent from the table in +/// scope — is an ERROR: the command parses but would fail at +/// execution. Runs only on a structural `Match`. +/// +/// Column scope is resolved by a single left-to-right pass: +/// every command places its table ident before the columns +/// that belong to it (a qualified `T.c` puts `T` immediately +/// before `c`), so the most recent valid `Tables` ident is the +/// table a subsequent `Columns` ident is checked against. An +/// unknown table clears the scope, so its columns are not +/// cascaded into a second diagnostic. +fn schema_existence_diagnostics( + path: &MatchedPath, + schema: Option<&crate::completion::SchemaCache>, +) -> Vec { + use crate::dsl::grammar::IdentSource; + use outcome::{Diagnostic, MatchedKind, Severity}; + + let Some(schema) = schema else { + return Vec::new(); + }; + let mut diagnostics = Vec::new(); + let mut current_table: Option = None; + for item in &path.items { + let MatchedKind::Ident { source, .. } = item.kind else { + continue; + }; + match source { + IdentSource::Tables => { + if schema_has_table(schema, &item.text) { + current_table = Some(item.text.clone()); + } else { + current_table = None; + diagnostics.push(Diagnostic { + severity: Severity::Error, + span: item.span, + message: crate::friendly::translate( + "diagnostic.unknown_table", + &[("name", &item.text as &dyn std::fmt::Display)], + ), + }); + } + } + IdentSource::Columns => { + if let Some(table) = current_table.as_deref() + && !schema_has_column(schema, table, &item.text) + { + diagnostics.push(Diagnostic { + severity: Severity::Error, + span: item.span, + message: crate::friendly::translate( + "diagnostic.unknown_column", + &[ + ("name", &item.text as &dyn std::fmt::Display), + ("table", &table as &dyn std::fmt::Display), + ], + ), + }); + } + } + // Invented names (`NewName`), closed sets (`Types`), + // and the other entity kinds are not schema-checked + // here (ADR-0027 §2 scopes the check to tables and + // columns). + IdentSource::NewName + | IdentSource::Relationships + | IdentSource::Indexes + | IdentSource::Types + | IdentSource::Free => {} + } + } + diagnostics +} + +fn schema_has_table(schema: &crate::completion::SchemaCache, name: &str) -> bool { + schema.tables.iter().any(|t| t.eq_ignore_ascii_case(name)) +} + +fn schema_has_column( + schema: &crate::completion::SchemaCache, + table: &str, + column: &str, +) -> bool { + schema + .columns_for_table(table) + .is_some_and(|cols| cols.iter().any(|c| c.name.eq_ignore_ascii_case(column))) +} + /// What the grammar would accept at the end of `source` /// (ADR-0024 §architecture, Phase F walker-driven completion). /// @@ -604,14 +695,20 @@ pub fn walk<'a>( other => (other, None), }; + // 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 { .. }) { + schema_existence_diagnostics(&path, ctx.schema) + } else { + Vec::new() + }; let result = WalkResult { outcome: final_outcome, matched_path: path, per_byte_class: per_byte, tail_expected, - // Filled by the schema-existence and expression passes - // (ADR-0027 §2); empty here at the structural layer. - diagnostics: Vec::new(), + diagnostics, }; (Some(result), cmd) } @@ -1317,6 +1414,43 @@ mod tests { ); } + #[test] + fn input_verdict_unknown_table_is_error() { + // The command parses, but the table does not exist — + // an ERROR diagnostic (ADR-0027 §2). + let schema = schema_with("Customers", &[("id", Type::Int)]); + assert_eq!( + super::input_verdict("show data NoSuchTable", Some(&schema)), + Some(super::Severity::Error), + ); + } + + #[test] + fn input_verdict_unknown_column_is_error() { + let schema = + schema_with("Customers", &[("id", Type::Int), ("Name", Type::Text)]); + assert_eq!( + super::input_verdict( + "show data Customers where NoSuchCol = 1", + Some(&schema), + ), + Some(super::Severity::Error), + ); + } + + #[test] + fn input_verdict_known_table_and_column_is_clean() { + let schema = + schema_with("Customers", &[("id", Type::Int), ("Name", Type::Text)]); + assert_eq!( + super::input_verdict( + "show data Customers where id = 1", + Some(&schema), + ), + None, + ); + } + #[test] fn walker_parses_insert_with_explicit_column_list() { assert_eq!( diff --git a/src/dsl/walker/outcome.rs b/src/dsl/walker/outcome.rs index 1192134..9e1449a 100644 --- a/src/dsl/walker/outcome.rs +++ b/src/dsl/walker/outcome.rs @@ -109,8 +109,14 @@ pub enum MatchedKind { /// on it canonically. Word(&'static str), Punct(char), - /// An `Ident` matched. The role identifies which slot. - Ident { role: &'static str }, + /// An `Ident` matched. `role` identifies which slot; + /// `source` is the slot's `IdentSource` — the post-walk + /// schema-existence check (ADR-0027 §2) reads it to decide + /// whether a matched name must exist in the schema. + Ident { + role: &'static str, + source: IdentSource, + }, NumberLit, StringLit, BlobLit, diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 439cb47..4d5a598 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -37,6 +37,9 @@ /// `(key, expected_placeholders)`. Sorted by key for grep-ability. pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ + // ---- Pre-submit diagnostics (ADR-0027) ---- + ("diagnostic.unknown_column", &["name", "table"]), + ("diagnostic.unknown_table", &["name"]), // ---- Already-exists collisions (anchor: "already exists") ---- ("error.already_exists.column.headline", &["table", "column"]), ("error.already_exists.relationship.headline", &["name"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 0c235f1..0921ecb 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -458,6 +458,14 @@ parse: mode: "mode simple | mode advanced" messages: "messages | messages short | messages verbose" +# ---- Pre-submit diagnostics (ADR-0027) ------------------------------- +# Surfaced by the validity indicator and the hint panel before +# a command is submitted, so a learner sees the problem without +# having to run the command and read the engine's error. +diagnostic: + unknown_table: "no such table: `{name}`" + unknown_column: "no such column `{name}` on table `{table}`" + # ---- Project lifecycle event notes ----------------------------------- project: rebuild_ok: "[ok] rebuild — {summary}"