walker: schema-existence ERROR diagnostics (ADR-0027 step B)
`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`.
This commit is contained in:
@@ -151,7 +151,7 @@ fn build_import(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
.map(|i| i.text.clone())
|
.map(|i| i.text.clone())
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
let target = path
|
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());
|
.map(|i| i.text.clone());
|
||||||
Ok(Command::App(AppCommand::Import {
|
Ok(Command::App(AppCommand::Import {
|
||||||
path: bare_path,
|
path: bare_path,
|
||||||
|
|||||||
@@ -316,7 +316,7 @@ const DELETE_SHAPE: Node = Node::Seq(DELETE_NODES);
|
|||||||
|
|
||||||
fn ident_text<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> {
|
fn ident_text<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> {
|
||||||
path.items.iter().find_map(|i| match &i.kind {
|
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,
|
_ => None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -450,7 +450,7 @@ fn build_insert(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
let table_idx = path
|
let table_idx = path
|
||||||
.items
|
.items
|
||||||
.iter()
|
.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 {
|
.ok_or_else(|| ValidationError {
|
||||||
message_key: "parse.error_wrapper",
|
message_key: "parse.error_wrapper",
|
||||||
args: vec![("detail", "missing table".to_string())],
|
args: vec![("detail", "missing table".to_string())],
|
||||||
@@ -498,6 +498,7 @@ fn build_insert(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
.filter_map(|i| match &i.kind {
|
.filter_map(|i| match &i.kind {
|
||||||
MatchedKind::Ident {
|
MatchedKind::Ident {
|
||||||
role: "insert_first_item",
|
role: "insert_first_item",
|
||||||
|
..
|
||||||
} => Some(i.text.clone()),
|
} => Some(i.text.clone()),
|
||||||
_ => None,
|
_ => None,
|
||||||
})
|
})
|
||||||
@@ -551,6 +552,7 @@ fn build_insert(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
.filter_map(|i| match &i.kind {
|
.filter_map(|i| match &i.kind {
|
||||||
MatchedKind::Ident {
|
MatchedKind::Ident {
|
||||||
role: "insert_first_item",
|
role: "insert_first_item",
|
||||||
|
..
|
||||||
} => Some(i.text.clone()),
|
} => Some(i.text.clone()),
|
||||||
_ => None,
|
_ => None,
|
||||||
})
|
})
|
||||||
@@ -619,7 +621,8 @@ fn collect_assignments(
|
|||||||
if matches!(
|
if matches!(
|
||||||
item.kind,
|
item.kind,
|
||||||
MatchedKind::Ident {
|
MatchedKind::Ident {
|
||||||
role: "update_set_column"
|
role: "update_set_column",
|
||||||
|
..
|
||||||
}
|
}
|
||||||
) {
|
) {
|
||||||
let column = item.text.clone();
|
let column = item.text.clone();
|
||||||
|
|||||||
@@ -475,7 +475,7 @@ const CHANGE_COLUMN: Node = Node::Seq(CHANGE_COLUMN_NODES);
|
|||||||
/// First ident whose role matches.
|
/// First ident whose role matches.
|
||||||
fn ident<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> {
|
fn ident<'a>(path: &'a MatchedPath, role: &str) -> Option<&'a str> {
|
||||||
path.items.iter().find_map(|i| match &i.kind {
|
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,
|
_ => None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -495,7 +495,7 @@ fn collect_idents(path: &MatchedPath, role: &str) -> Vec<String> {
|
|||||||
path.items
|
path.items
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|i| match &i.kind {
|
.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,
|
_ => None,
|
||||||
})
|
})
|
||||||
.collect()
|
.collect()
|
||||||
@@ -862,7 +862,7 @@ fn build_create_table(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
.items
|
.items
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|i| match &i.kind {
|
.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,
|
_ => None,
|
||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
@@ -870,7 +870,7 @@ fn build_create_table(path: &MatchedPath) -> Result<Command, ValidationError> {
|
|||||||
.items
|
.items
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|i| match &i.kind {
|
.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,
|
_ => None,
|
||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|||||||
@@ -525,7 +525,7 @@ impl<'a> ExprParser<'a> {
|
|||||||
.advance()
|
.advance()
|
||||||
.ok_or_else(|| drift_error("expected an operand"))?;
|
.ok_or_else(|| drift_error("expected an operand"))?;
|
||||||
match &item.kind {
|
match &item.kind {
|
||||||
MatchedKind::Ident { role: "expr_column" } => {
|
MatchedKind::Ident { role: "expr_column", .. } => {
|
||||||
Ok(Operand::Column(item.text.clone()))
|
Ok(Operand::Column(item.text.clone()))
|
||||||
}
|
}
|
||||||
MatchedKind::Word("null") => Ok(Operand::Literal(Value::Null)),
|
MatchedKind::Word("null") => Ok(Operand::Literal(Value::Null)),
|
||||||
|
|||||||
@@ -434,7 +434,7 @@ fn walk_ident(
|
|||||||
.push(canonical);
|
.push(canonical);
|
||||||
}
|
}
|
||||||
path.push(MatchedItem {
|
path.push(MatchedItem {
|
||||||
kind: MatchedKind::Ident { role },
|
kind: MatchedKind::Ident { role, source: src },
|
||||||
text,
|
text,
|
||||||
span: (start, end),
|
span: (start, end),
|
||||||
});
|
});
|
||||||
|
|||||||
+137
-3
@@ -330,6 +330,97 @@ pub fn input_verdict(
|
|||||||
outcome_severity.into_iter().chain(diag_severity).max()
|
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<outcome::Diagnostic> {
|
||||||
|
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<String> = 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`
|
/// What the grammar would accept at the end of `source`
|
||||||
/// (ADR-0024 §architecture, Phase F walker-driven completion).
|
/// (ADR-0024 §architecture, Phase F walker-driven completion).
|
||||||
///
|
///
|
||||||
@@ -604,14 +695,20 @@ pub fn walk<'a>(
|
|||||||
other => (other, None),
|
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 {
|
let result = WalkResult {
|
||||||
outcome: final_outcome,
|
outcome: final_outcome,
|
||||||
matched_path: path,
|
matched_path: path,
|
||||||
per_byte_class: per_byte,
|
per_byte_class: per_byte,
|
||||||
tail_expected,
|
tail_expected,
|
||||||
// Filled by the schema-existence and expression passes
|
diagnostics,
|
||||||
// (ADR-0027 §2); empty here at the structural layer.
|
|
||||||
diagnostics: Vec::new(),
|
|
||||||
};
|
};
|
||||||
(Some(result), cmd)
|
(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]
|
#[test]
|
||||||
fn walker_parses_insert_with_explicit_column_list() {
|
fn walker_parses_insert_with_explicit_column_list() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -109,8 +109,14 @@ pub enum MatchedKind {
|
|||||||
/// on it canonically.
|
/// on it canonically.
|
||||||
Word(&'static str),
|
Word(&'static str),
|
||||||
Punct(char),
|
Punct(char),
|
||||||
/// An `Ident` matched. The role identifies which slot.
|
/// An `Ident` matched. `role` identifies which slot;
|
||||||
Ident { role: &'static str },
|
/// `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,
|
NumberLit,
|
||||||
StringLit,
|
StringLit,
|
||||||
BlobLit,
|
BlobLit,
|
||||||
|
|||||||
@@ -37,6 +37,9 @@
|
|||||||
|
|
||||||
/// `(key, expected_placeholders)`. Sorted by key for grep-ability.
|
/// `(key, expected_placeholders)`. Sorted by key for grep-ability.
|
||||||
pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
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") ----
|
// ---- Already-exists collisions (anchor: "already exists") ----
|
||||||
("error.already_exists.column.headline", &["table", "column"]),
|
("error.already_exists.column.headline", &["table", "column"]),
|
||||||
("error.already_exists.relationship.headline", &["name"]),
|
("error.already_exists.relationship.headline", &["name"]),
|
||||||
|
|||||||
@@ -458,6 +458,14 @@ parse:
|
|||||||
mode: "mode simple | mode advanced"
|
mode: "mode simple | mode advanced"
|
||||||
messages: "messages | messages short | messages verbose"
|
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 lifecycle event notes -----------------------------------
|
||||||
project:
|
project:
|
||||||
rebuild_ok: "[ok] rebuild — {summary}"
|
rebuild_ok: "[ok] rebuild — {summary}"
|
||||||
|
|||||||
Reference in New Issue
Block a user