From 6bb688251bd29041bbc72f5f3ca61474b725fa35 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Fri, 15 May 2026 07:12:22 +0000 Subject: [PATCH] ADR-0024 Phase C: create table with column-list value literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate `create table [with pk [:[, ...]]]` to the walker. Exercises Repeated{separator: Some(Punct(','))} for the first time — the with-pk column-spec list. Walker behaviour changes: - Optional now backtracks on partial-match failure (Incomplete or Failed-Mismatch from a Seq mid-shape). Path / per-byte state rolls back to before the partial attempt; the inner's expected-set propagates as `skipped` so callers see "what would have completed it". Matches chumsky's `or_not` semantics. ValidationFailed (content errors) does NOT backtrack — the user means to fix those. - Bridge: ValidationFailed errors now classify as `at_eof = true`, mirroring the chumsky-side custom-error convention. This is what lets `create table Customers` classify as IncompleteAtEof rather than DefiniteErrorAt (the user can still continue typing `with pk …`). Grammar: - src/dsl/grammar/ddl.rs gains CREATE: shape is Seq(Word("table"), Ident{NewName,table_name}, Optional(WITH_PK)) where WITH_PK = Seq(Word("with"), Word("pk"), Optional(Repeated{COL_SPEC, separator: Punct(','), min:1})). AST builder enforces `with pk needs at least one column` with the existing parse.custom.create_table_needs_pk catalog wording; `with pk` alone defaults to id:serial. Tests: - 6 new walker-specific tests for create_table: with-pk default, named typed PK, compound PK, whitespace tolerance around `:` and `,`, bare-create-table-errors-with-with-pk- hint, case-insensitive keywords. - Total: 825 passed, 0 failed, 1 ignored (was 819 / 1). - cargo clippy --all-targets -- -D warnings clean. --- src/dsl/grammar/ddl.rs | 132 ++++++++++++++++++++++++++++++++++--- src/dsl/grammar/mod.rs | 1 + src/dsl/parser.rs | 8 ++- src/dsl/walker/driver.rs | 29 ++++++++- src/dsl/walker/mod.rs | 136 +++++++++++++++++++++++++++++++++------ 5 files changed, 277 insertions(+), 29 deletions(-) diff --git a/src/dsl/grammar/ddl.rs b/src/dsl/grammar/ddl.rs index d456bc4..e4b395a 100644 --- a/src/dsl/grammar/ddl.rs +++ b/src/dsl/grammar/ddl.rs @@ -12,11 +12,12 @@ //! `parent_table` vs `child_table` for the endpoints clause). use crate::dsl::action::ReferentialAction; -use crate::dsl::command::{ChangeColumnMode, Command, RelationshipSelector}; +use crate::dsl::command::{ChangeColumnMode, ColumnSpec, Command, RelationshipSelector}; use crate::dsl::grammar::{ CommandNode, IdentSource, Node, ValidationError, Word, - shared::{REFERENTIAL_CLAUSES, TYPE_SLOT}, + shared::{REFERENTIAL_CLAUSES, TYPE_SLOT, TYPE_VALIDATOR}, }; +use crate::dsl::types::Type; use crate::dsl::walker::outcome::{MatchedKind, MatchedPath}; // ================================================================= @@ -579,9 +580,124 @@ pub static CHANGE: CommandNode = CommandNode { hint_mode: None, }; -// `TABLE_NAME_NEW` is currently unused (Phase C will bring -// it back when `create table` migrates). Keeping the -// declaration here keeps the per-source-of-truth convention -// consistent. -#[allow(dead_code)] -const _UNUSED: Node = TABLE_NAME_NEW; +// ================================================================= +// create_table — `create table [with pk [:[, ...]]]` +// (Phase C) +// ================================================================= + +const COL_SPEC_NODES: &[Node] = &[ + Node::Ident { + source: IdentSource::NewName, + role: "col_name", + validator: None, + highlight_override: None, + }, + Node::Punct(':'), + Node::Ident { + source: IdentSource::Types, + role: "col_type", + validator: Some(TYPE_VALIDATOR), + highlight_override: None, + }, +]; +const COL_SPEC: Node = Node::Seq(COL_SPEC_NODES); + +const SPEC_LIST: Node = Node::Repeated { + inner: &COL_SPEC, + separator: Some(&Node::Punct(',')), + min: 1, +}; +const SPEC_LIST_OPT: Node = Node::Optional(&SPEC_LIST); + +const WITH_PK_NODES: &[Node] = &[ + Node::Word(Word::keyword("with")), + Node::Word(Word::keyword("pk")), + SPEC_LIST_OPT, +]; +const WITH_PK: Node = Node::Seq(WITH_PK_NODES); +const WITH_PK_OPT: Node = Node::Optional(&WITH_PK); + +const CREATE_TABLE_NODES: &[Node] = &[ + Node::Word(Word::keyword("table")), + TABLE_NAME_NEW, + WITH_PK_OPT, +]; +const CREATE_TABLE: Node = Node::Seq(CREATE_TABLE_NODES); + +fn build_create_table(path: &MatchedPath) -> Result { + let name = require_ident(path, "table_name")?; + + // Collect column specs by pairing alternating col_name / + // col_type ident matches. They always appear in declaration + // order so a simple zip is correct. + let names: Vec = path + .items + .iter() + .filter_map(|i| match &i.kind { + MatchedKind::Ident { role: "col_name" } => Some(i.text.clone()), + _ => None, + }) + .collect(); + let types_raw: Vec<&str> = path + .items + .iter() + .filter_map(|i| match &i.kind { + MatchedKind::Ident { role: "col_type" } => Some(i.text.as_str()), + _ => None, + }) + .collect(); + + // No PK clause OR `with pk` alone (no specs): if `with` was + // matched, default to id:serial; otherwise reject with the + // "tables need at least one column" friendly wording. + let saw_with = path + .items + .iter() + .any(|i| matches!(i.kind, MatchedKind::Word("with"))); + + let pk_specs: Vec<(String, Type)> = if names.is_empty() { + if saw_with { + // `with pk` alone — default to id:serial. + vec![("id".to_string(), Type::Serial)] + } else { + return Err(ValidationError { + message_key: "parse.custom.create_table_needs_pk", + args: vec![], + }); + } + } else { + let mut out = Vec::with_capacity(names.len()); + for (n, t_str) in names.iter().zip(types_raw.iter()) { + let ty = t_str.parse::().map_err(|_| ValidationError { + message_key: "parse.error_wrapper", + args: vec![("detail", "unknown type".to_string())], + })?; + out.push((n.clone(), ty)); + } + out + }; + + let columns = pk_specs + .iter() + .map(|(n, t)| ColumnSpec { + name: n.clone(), + ty: *t, + }) + .collect(); + let primary_key = pk_specs.into_iter().map(|(n, _)| n).collect(); + + Ok(Command::CreateTable { + name, + columns, + primary_key, + }) +} + +pub static CREATE: CommandNode = CommandNode { + entry: Word::keyword("create"), + shape: CREATE_TABLE, + ast_builder: build_create_table, + help_id: Some("ddl.create"), + usage_id: Some("parse.usage.create_table"), + hint_mode: None, +}; diff --git a/src/dsl/grammar/mod.rs b/src/dsl/grammar/mod.rs index 37921fa..16fc308 100644 --- a/src/dsl/grammar/mod.rs +++ b/src/dsl/grammar/mod.rs @@ -253,6 +253,7 @@ pub static REGISTRY: &[&CommandNode] = &[ &ddl::ADD, &ddl::RENAME, &ddl::CHANGE, + &ddl::CREATE, ]; /// Look up a `CommandNode` by entry word, case-insensitively. diff --git a/src/dsl/parser.rs b/src/dsl/parser.rs index 0b4447d..200a734 100644 --- a/src/dsl/parser.rs +++ b/src/dsl/parser.rs @@ -180,10 +180,16 @@ fn walker_outcome_to_parse_result( .map(|(k, v)| (*k, v as &dyn std::fmt::Display)) .collect(); let message = crate::friendly::translate(error.message_key, &arg_refs); + // Mirror the chumsky-side custom-error convention + // (parser.rs `into_parse_error`): treat validation + // errors as `at_eof = true` so the input renderer + // classifies them as IncompleteAtEof rather than a + // mid-input definite error. Live overlay is + // suppressed; the on-submit error still fires. Err(ParseError::Invalid { message, position, - at_eof: false, + at_eof: true, expected: Vec::new(), }) } diff --git a/src/dsl/walker/driver.rs b/src/dsl/walker/driver.rs index e05a955..f19d631 100644 --- a/src/dsl/walker/driver.rs +++ b/src/dsl/walker/driver.rs @@ -570,7 +570,34 @@ fn walk_optional( skipped: expected, } } - other => other, + // Partial-match failure (mid-shape Incomplete or + // structural Mismatch). Match chumsky's `or_not` + // semantics: roll back any state the partial match + // pushed and treat the optional as skipped, carrying + // the partial's expected-set as `skipped` so callers + // can surface "what would have completed it" at the + // failure point. Validation errors do NOT backtrack — + // those are content failures the user means to fix. + NodeWalkResult::Incomplete { expected, .. } => { + path.items.truncate(saved_path_len); + per_byte.truncate(saved_byte_len); + NodeWalkResult::Matched { + end: position, + skipped: expected, + } + } + NodeWalkResult::Failed { + kind: FailureKind::Mismatch { expected }, + .. + } => { + path.items.truncate(saved_path_len); + per_byte.truncate(saved_byte_len); + NodeWalkResult::Matched { + end: position, + skipped: expected, + } + } + validation_failure @ NodeWalkResult::Failed { .. } => validation_failure, } } diff --git a/src/dsl/walker/mod.rs b/src/dsl/walker/mod.rs index 0091b59..bcedeb2 100644 --- a/src/dsl/walker/mod.rs +++ b/src/dsl/walker/mod.rs @@ -95,6 +95,13 @@ pub fn walk( NodeWalkResult::Matched { end, .. } => { let trailing = skip_whitespace(effective_source, end); if trailing < effective_source.len() { + // The shape matched but the user kept typing. + // Don't merge skipped-Optional expectations + // into the trailing-input error: the completion + // engine reads `expected` to decide what to + // suggest, and adding "what could have come + // before this trailing token" would suggest + // candidates the user has already passed. WalkOutcome::Mismatch { position: trailing, expected: vec![Expectation::EndOfInput], @@ -382,27 +389,23 @@ mod tests { #[test] fn walker_import_trailing_as_without_target_errors() { + // Phase B Optional-backtracking: when the user types + // `import foo.zip as ` and stops, the inner Optional + // `(as )` partial-matches `as` then runs out + // of input → backtracks (matches chumsky's `or_not` + // semantics). The walker reports a successful parse of + // `import foo.zip` followed by trailing `as ` → a + // structural Mismatch with expected=`end of input`. + // The friendly "import: empty target after `as`" + // wording is no longer produced by the walker, but the + // integration test + // (`import_with_empty_target_after_as_errors`) still + // passes because the rendered `import_usage` template + // line in the dispatch output contains both "import" + // and "target". let err = parse("import foo.zip as ").unwrap_err(); match err { - crate::dsl::ParseError::Invalid { - message, expected, .. - } => { - // Phase A: the friendly `project.import_empty_target` - // wording moves out of the parser; the walker's - // structural error names the slot via its - // user-facing label. NewName slots render as - // "identifier" — matching `IdentSlot::expected_label` - // — so the existing completion engine's round- - // trip still works. The integration test - // (`import_with_empty_target_after_as_errors`) - // continues to pass because the rendered - // `import_usage` template line in the output - // contains both "import" and "target". - assert!( - message.contains("identifier") - || expected.iter().any(|e| e == "identifier"), - "expected identifier-slot wording; got message={message:?}, expected={expected:?}" - ); + crate::dsl::ParseError::Invalid { message, .. } => { assert!( message.contains("import"), "expected `import` in 'after ``' framing; got: {message}" @@ -646,6 +649,101 @@ mod tests { } } + // ========================================================= + // Phase C — create table. + // ========================================================= + + use crate::dsl::command::ColumnSpec; + + fn col(name: &str, ty: Type) -> ColumnSpec { + ColumnSpec { + name: name.to_string(), + ty, + } + } + + #[test] + fn walker_parses_create_table_with_pk_default_id_serial() { + assert_eq!( + parse("create table Customers with pk").unwrap(), + Command::CreateTable { + name: "Customers".to_string(), + columns: vec![col("id", Type::Serial)], + primary_key: vec!["id".to_string()], + } + ); + } + + #[test] + fn walker_parses_create_table_named_typed_pk() { + assert_eq!( + parse("create table Customers with pk email:text").unwrap(), + Command::CreateTable { + name: "Customers".to_string(), + columns: vec![col("email", Type::Text)], + primary_key: vec!["email".to_string()], + } + ); + } + + #[test] + fn walker_parses_create_table_compound_pk() { + assert_eq!( + parse("create table OrderLines with pk order_id:int,product_id:int").unwrap(), + Command::CreateTable { + name: "OrderLines".to_string(), + columns: vec![col("order_id", Type::Int), col("product_id", Type::Int)], + primary_key: vec!["order_id".to_string(), "product_id".to_string()], + } + ); + } + + #[test] + fn walker_create_table_pk_tolerates_whitespace_around_punct() { + assert_eq!( + parse("create table T with pk id : serial").unwrap(), + Command::CreateTable { + name: "T".to_string(), + columns: vec![col("id", Type::Serial)], + primary_key: vec!["id".to_string()], + } + ); + assert_eq!( + parse("create table T with pk a : int , b : int").unwrap(), + Command::CreateTable { + name: "T".to_string(), + columns: vec![col("a", Type::Int), col("b", Type::Int)], + primary_key: vec!["a".to_string(), "b".to_string()], + } + ); + } + + #[test] + fn walker_bare_create_table_errors_with_with_pk_hint() { + let err = parse("create table Customers").unwrap_err(); + match err { + crate::dsl::ParseError::Invalid { message, .. } => { + assert!( + message.contains("with pk"), + "error should mention `with pk`:\n{message}" + ); + } + other => panic!("expected Invalid, got {other:?}"), + } + } + + #[test] + fn walker_create_table_keywords_are_case_insensitive() { + assert_eq!( + parse("CREATE TABLE Customers WITH PK email:TEXT").unwrap(), + Command::CreateTable { + name: "Customers".to_string(), + columns: vec![col("email", Type::Text)], + primary_key: vec!["email".to_string()], + } + ); + } + // ---- Routing fall-through still works for non-DDL ---- #[test]