From d9bf2de556f154264c51f3e6a7450de034bb98ea Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Thu, 24 Jul 2025 22:02:23 -0400 Subject: [PATCH] parser: fix cast validation issue This was incorrect and warning about things that are fine. --- .../squawk_syntax/src/ast/generated/nodes.rs | 4 + crates/squawk_syntax/src/postgresql.ungram | 2 +- ...ntax__test__alter_table_ok_validation.snap | 134 ++++++++++++++++++ ...wk_syntax__test__call_expr_validation.snap | 31 ---- crates/squawk_syntax/src/test.rs | 18 ++- crates/squawk_syntax/src/validation.rs | 15 -- .../test_data/validation/alter_table_ok.sql | 10 ++ .../test_data/validation/call_expr.sql | 2 - 8 files changed, 162 insertions(+), 54 deletions(-) create mode 100644 crates/squawk_syntax/src/snapshots/squawk_syntax__test__alter_table_ok_validation.snap delete mode 100644 crates/squawk_syntax/src/snapshots/squawk_syntax__test__call_expr_validation.snap create mode 100644 crates/squawk_syntax/test_data/validation/alter_table_ok.sql delete mode 100644 crates/squawk_syntax/test_data/validation/call_expr.sql diff --git a/crates/squawk_syntax/src/ast/generated/nodes.rs b/crates/squawk_syntax/src/ast/generated/nodes.rs index c9f8cb48..c02c927c 100644 --- a/crates/squawk_syntax/src/ast/generated/nodes.rs +++ b/crates/squawk_syntax/src/ast/generated/nodes.rs @@ -1445,6 +1445,10 @@ impl CastExpr { pub fn ty(&self) -> Option { support::child(&self.syntax) } + #[inline] + pub fn as_token(&self) -> Option { + support::token(&self.syntax, SyntaxKind::AS_KW) + } } #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/crates/squawk_syntax/src/postgresql.ungram b/crates/squawk_syntax/src/postgresql.ungram index ee8b8735..8b561850 100644 --- a/crates/squawk_syntax/src/postgresql.ungram +++ b/crates/squawk_syntax/src/postgresql.ungram @@ -70,7 +70,7 @@ CallExpr = Expr ArgList CastExpr = - Expr ColonColon Type + Expr (ColonColon? | 'as') Type ArrayExpr = 'array' '[' (Expr (',' Expr)*) ']' diff --git a/crates/squawk_syntax/src/snapshots/squawk_syntax__test__alter_table_ok_validation.snap b/crates/squawk_syntax/src/snapshots/squawk_syntax__test__alter_table_ok_validation.snap new file mode 100644 index 00000000..8f3257da --- /dev/null +++ b/crates/squawk_syntax/src/snapshots/squawk_syntax__test__alter_table_ok_validation.snap @@ -0,0 +1,134 @@ +--- +source: crates/squawk_syntax/src/test.rs +input_file: crates/squawk_syntax/test_data/validation/alter_table_ok.sql +--- +SOURCE_FILE@0..470 + COMMENT@0..66 "-- regression test fo ..." + WHITESPACE@66..67 "\n" + ALTER_TABLE@67..243 + ALTER_KW@67..72 "alter" + WHITESPACE@72..73 " " + TABLE_KW@73..78 "table" + WHITESPACE@78..79 " " + RELATION_NAME@79..92 + PATH@79..92 + PATH@79..85 + PATH_SEGMENT@79..85 + NAME_REF@79..85 + IDENT@79..85 "public" + DOT@85..86 "." + PATH_SEGMENT@86..92 + NAME_REF@86..92 + IDENT@86..92 "widget" + WHITESPACE@92..95 "\n " + ADD_CONSTRAINT@95..243 + ADD_KW@95..98 "add" + WHITESPACE@98..99 " " + CHECK_CONSTRAINT@99..233 + CONSTRAINT_KW@99..109 "constraint" + WHITESPACE@109..110 " " + NAME@110..136 + IDENT@110..136 "widget_config_schema_ ..." + WHITESPACE@136..137 " " + CHECK_KW@137..142 "check" + WHITESPACE@142..143 " " + L_PAREN@143..144 "(" + WHITESPACE@144..149 "\n " + CALL_EXPR@149..229 + FIELD_EXPR@149..178 + NAME_REF@149..155 + IDENT@149..155 "checks" + DOT@155..156 "." + NAME_REF@156..178 + IDENT@156..178 "is_widget_config_valid" + ARG_LIST@178..229 + L_PAREN@178..179 "(" + CAST_EXPR@179..220 + LITERAL@179..187 + STRING@179..187 "'widget'" + COLON_COLON@187..189 + COLON@187..188 ":" + COLON@188..189 ":" + FIELD_EXPR@189..220 + NAME_REF@189..201 + IDENT@189..201 "custom_types" + DOT@201..202 "." + NAME_REF@202..220 + IDENT@202..220 "widget_schema_type" + COMMA@220..221 "," + WHITESPACE@221..222 " " + NAME_REF@222..228 + IDENT@222..228 "config" + R_PAREN@228..229 ")" + WHITESPACE@229..232 "\n " + R_PAREN@232..233 ")" + WHITESPACE@233..234 " " + NOT_VALID@234..243 + NOT_KW@234..237 "not" + WHITESPACE@237..238 " " + VALID_KW@238..243 "valid" + SEMICOLON@243..244 ";" + WHITESPACE@244..246 "\n\n" + ALTER_TABLE@246..469 + ALTER_KW@246..251 "alter" + WHITESPACE@251..252 " " + TABLE_KW@252..257 "table" + WHITESPACE@257..258 " " + RELATION_NAME@258..280 + PATH@258..280 + PATH@258..264 + PATH_SEGMENT@258..264 + NAME_REF@258..264 + IDENT@258..264 "public" + DOT@264..265 "." + PATH_SEGMENT@265..280 + NAME_REF@265..280 + IDENT@265..280 "widget_instance" + WHITESPACE@280..283 "\n " + ADD_CONSTRAINT@283..469 + ADD_KW@283..286 "add" + WHITESPACE@286..287 " " + CHECK_CONSTRAINT@287..459 + CONSTRAINT_KW@287..297 "constraint" + WHITESPACE@297..298 " " + NAME@298..343 + IDENT@298..343 "widget_instance_confi ..." + WHITESPACE@343..344 " " + CHECK_KW@344..349 "check" + WHITESPACE@349..350 " " + L_PAREN@350..351 "(" + WHITESPACE@351..356 "\n " + CALL_EXPR@356..455 + FIELD_EXPR@356..385 + NAME_REF@356..362 + IDENT@356..362 "checks" + DOT@362..363 "." + NAME_REF@363..385 + IDENT@363..385 "is_widget_config_valid" + ARG_LIST@385..455 + L_PAREN@385..386 "(" + CAST_EXPR@386..436 + LITERAL@386..403 + STRING@386..403 "'widget_instance'" + COLON_COLON@403..405 + COLON@403..404 ":" + COLON@404..405 ":" + FIELD_EXPR@405..436 + NAME_REF@405..417 + IDENT@405..417 "custom_types" + DOT@417..418 "." + NAME_REF@418..436 + IDENT@418..436 "widget_schema_type" + COMMA@436..437 "," + WHITESPACE@437..438 " " + NAME_REF@438..454 + IDENT@438..454 "config_overrides" + R_PAREN@454..455 ")" + WHITESPACE@455..458 "\n " + R_PAREN@458..459 ")" + WHITESPACE@459..460 " " + NOT_VALID@460..469 + NOT_KW@460..463 "not" + WHITESPACE@463..464 " " + VALID_KW@464..469 "valid" + WHITESPACE@469..470 "\n" diff --git a/crates/squawk_syntax/src/snapshots/squawk_syntax__test__call_expr_validation.snap b/crates/squawk_syntax/src/snapshots/squawk_syntax__test__call_expr_validation.snap deleted file mode 100644 index 41e9b583..00000000 --- a/crates/squawk_syntax/src/snapshots/squawk_syntax__test__call_expr_validation.snap +++ /dev/null @@ -1,31 +0,0 @@ ---- -source: crates/squawk_syntax/src/test.rs -input_file: crates/squawk_syntax/test_data/validation/call_expr.sql ---- -SOURCE_FILE@0..97 - COMMENT@0..69 "-- Trino/Snowflake/Da ..." - WHITESPACE@69..70 "\n" - SELECT@70..95 - SELECT_CLAUSE@70..95 - SELECT_KW@70..76 "select" - WHITESPACE@76..77 " " - TARGET_LIST@77..95 - TARGET@77..95 - CALL_EXPR@77..95 - NAME_REF@77..85 - IDENT@77..85 "try_cast" - ARG_LIST@85..95 - L_PAREN@85..86 "(" - CAST_EXPR@86..94 - NAME_REF@86..87 - IDENT@86..87 "x" - WHITESPACE@87..88 " " - AS_KW@88..90 "as" - WHITESPACE@90..91 " " - NAME_REF@91..94 - INT_KW@91..94 "int" - R_PAREN@94..95 ")" - SEMICOLON@95..96 ";" - WHITESPACE@96..97 "\n" - -ERROR@86:94 "Invalid cast. Use `cast`, `treat`, or `::` instead." diff --git a/crates/squawk_syntax/src/test.rs b/crates/squawk_syntax/src/test.rs index 5a90b7a2..c27bd552 100644 --- a/crates/squawk_syntax/src/test.rs +++ b/crates/squawk_syntax/src/test.rs @@ -43,9 +43,17 @@ fn syntaxtest(fixture: Fixture<&str>) { assert_snapshot!(format!("{}_{}", test_name, parent_dir), buffer); }); - assert_ne!( - errors.len(), - 0, - "tests defined in the `syntax/test_data` must have errors." - ); + if test_name.ends_with("_ok") { + assert_eq!( + errors.len(), + 0, + "tests defined in the `syntax/test_data` ending with `_ok` can't have errors." + ); + } else { + assert_ne!( + errors.len(), + 0, + "tests defined in the `syntax/test_data` must have errors." + ); + } } diff --git a/crates/squawk_syntax/src/validation.rs b/crates/squawk_syntax/src/validation.rs index 2c673000..5ee45634 100644 --- a/crates/squawk_syntax/src/validation.rs +++ b/crates/squawk_syntax/src/validation.rs @@ -18,7 +18,6 @@ pub(crate) fn validate(root: &SyntaxNode, errors: &mut Vec) { ast::PrefixExpr(it) => validate_prefix_expr(it, errors), ast::ArrayExpr(it) => validate_array_expr(it, errors), ast::DropAggregate(it) => validate_drop_aggregate(it, errors), - ast::CallExpr(it) => validate_call_expr(it, errors), ast::JoinExpr(it) => validate_join_expr(it, errors), ast::Literal(it) => validate_literal(it, errors), ast::NonStandardParam(it) => validate_non_standard_param(it, errors), @@ -28,20 +27,6 @@ pub(crate) fn validate(root: &SyntaxNode, errors: &mut Vec) { } } -fn validate_call_expr(it: ast::CallExpr, acc: &mut Vec) { - let Some(arg_list) = it.arg_list() else { - return; - }; - for arg in arg_list.args() { - if let ast::Expr::CastExpr(cast_expr) = &arg { - acc.push(SyntaxError::new( - "Invalid cast. Use `cast`, `treat`, or `::` instead.", - cast_expr.syntax().text_range(), - )); - } - } -} - fn validate_create_table(it: ast::CreateTable, acc: &mut Vec) { let Some(arg_list) = it.table_arg_list() else { return; diff --git a/crates/squawk_syntax/test_data/validation/alter_table_ok.sql b/crates/squawk_syntax/test_data/validation/alter_table_ok.sql new file mode 100644 index 00000000..715620ad --- /dev/null +++ b/crates/squawk_syntax/test_data/validation/alter_table_ok.sql @@ -0,0 +1,10 @@ +-- regression test for https://github.com/sbdchd/squawk/issues/597 +alter table public.widget + add constraint widget_config_schema_check check ( + checks.is_widget_config_valid('widget'::custom_types.widget_schema_type, config) + ) not valid; + +alter table public.widget_instance + add constraint widget_instance_config_overrides_schema_check check ( + checks.is_widget_config_valid('widget_instance'::custom_types.widget_schema_type, config_overrides) + ) not valid diff --git a/crates/squawk_syntax/test_data/validation/call_expr.sql b/crates/squawk_syntax/test_data/validation/call_expr.sql deleted file mode 100644 index 6b60386e..00000000 --- a/crates/squawk_syntax/test_data/validation/call_expr.sql +++ /dev/null @@ -1,2 +0,0 @@ --- Trino/Snowflake/Databricks extension that Postgres doesn't support -select try_cast(x as int);