From aea5bf1556527d09634e2e2885da642bb58133e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Tue, 2 Jan 2024 11:59:38 +0100 Subject: [PATCH 1/3] feat: named unions --- prqlc/prql-compiler/src/codegen/types.rs | 31 +++++++++++++++++-- .../src/semantic/resolver/types.rs | 18 +++++------ .../tests/integration/resolving.rs | 25 +++++++++++++-- ...egration__resolving__resolve_types_04.snap | 9 ++++++ prqlc/prqlc-parser/src/types.rs | 24 +++++++++++++- 5 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap diff --git a/prqlc/prql-compiler/src/codegen/types.rs b/prqlc/prql-compiler/src/codegen/types.rs index 64708b967161..7e35c8ee9357 100644 --- a/prqlc/prql-compiler/src/codegen/types.rs +++ b/prqlc/prql-compiler/src/codegen/types.rs @@ -38,14 +38,24 @@ impl WriteSource for TyKind { Ident(ident) => ident.write(opt), Primitive(prim) => Some(prim.to_string()), Union(variants) => { - let variants: Vec<_> = variants.iter().map(|x| &x.1).collect(); + let parenthesize = + // never must be parenthesized + variants.is_empty() || + // named union must be parenthesized + variants.iter().any(|(n, _)| n.is_some()); - SeparatedExprs { + let variants: Vec<_> = variants.iter().map(|(n, t)| UnionVariant(n, t)).collect(); + let sep_exprs = SeparatedExprs { exprs: &variants, inline: " || ", line_end: " ||", + }; + + if parenthesize { + sep_exprs.write_between("(", ")", opt) + } else { + sep_exprs.write(opt) } - .write(opt) } Singleton(lit) => Some(lit.to_string()), Tuple(elements) => SeparatedExprs { @@ -101,3 +111,18 @@ impl WriteSource for TupleField { } } } + +struct UnionVariant<'a>(&'a Option, &'a Ty); + +impl WriteSource for UnionVariant<'_> { + fn write(&self, mut opt: WriteOpt) -> Option { + let mut r = String::new(); + if let Some(name) = &self.0 { + r += name; + r += " = "; + } + opt.consume_width(r.len() as u16); + r += &self.1.write(opt)?; + Some(r) + } +} diff --git a/prqlc/prql-compiler/src/semantic/resolver/types.rs b/prqlc/prql-compiler/src/semantic/resolver/types.rs index 873ca90ead56..d7b49647e717 100644 --- a/prqlc/prql-compiler/src/semantic/resolver/types.rs +++ b/prqlc/prql-compiler/src/semantic/resolver/types.rs @@ -244,13 +244,13 @@ pub(crate) fn normalize_type(ty: Ty) -> Ty { for (variant_name, variant_ty) in variants { let variant_ty = normalize_type(variant_ty); - // A | () = A + // (A || ()) = A // skip never - if variant_ty.is_never() { + if variant_ty.is_never() && variant_name.is_none() { continue; } - // A | A | B = A | B + // (A || A || B) = A || B // skip duplicates let already_included = res.iter().any(|(_, r)| is_super_type_of(r, &variant_ty)); if already_included { @@ -272,7 +272,7 @@ pub(crate) fn normalize_type(ty: Ty) -> Ty { TyKind::Difference { base, exclude } => { let (base, exclude) = match (*base, *exclude) { - // (A | B) - C = (A - C) | (B - C) + // (A || B) - C = (A - C) || (B - C) ( Ty { kind: TyKind::Union(variants), @@ -297,7 +297,7 @@ pub(crate) fn normalize_type(ty: Ty) -> Ty { ); return normalize_type(Ty { kind, name, span }); } - // (A - B) - C = A - (B | C) + // (A - B) - C = A - (B || C) ( Ty { kind: @@ -318,9 +318,9 @@ pub(crate) fn normalize_type(ty: Ty) -> Ty { // A - (B - C) = // = A & not (B & not C) - // = A & (not B | C) - // = (A & not B) | (A & C) - // = (A - B) | (A & C) + // = A & (not B || C) + // = (A & not B) || (A & C) + // = (A - B) || (A & C) ( a, Ty { @@ -450,7 +450,7 @@ pub(crate) fn normalize_type(ty: Ty) -> Ty { let base = Box::new(normalize_type(base)); let exclude = Box::new(normalize_type(exclude)); - // A - (A | B) = () + // A - (A || B) = () if let TyKind::Union(excluded) = &exclude.kind { for (_, e) in excluded { if base.as_ref() == e { diff --git a/prqlc/prql-compiler/tests/integration/resolving.rs b/prqlc/prql-compiler/tests/integration/resolving.rs index 5f87858bf804..6646baf83981 100644 --- a/prqlc/prql-compiler/tests/integration/resolving.rs +++ b/prqlc/prql-compiler/tests/integration/resolving.rs @@ -46,12 +46,11 @@ fn resolve_types_01() { } #[test] -#[ignore] fn resolve_types_02() { assert_snapshot!(resolve(r#" - type A = A || () + type A = int || () "#).unwrap(), @r###" - type A = A + type A = int "###) } @@ -63,3 +62,23 @@ fn resolve_types_03() { type A = {a = int, bool, b = text, float} "###) } + +#[test] +fn resolve_types_04() { + assert_snapshot!(resolve( + r#" + type Status = ( + Paid = () || + Unpaid = float || + Canceled = {reason = text, cancelled_at = timestamp} || + ) + "#, + ) + .unwrap(), @r###" + type Status = ( + Paid = () || + Unpaid = float || + {reason = text, cancelled_at = timestamp} || + ) + "###); +} diff --git a/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap b/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap new file mode 100644 index 000000000000..f4a1d548a032 --- /dev/null +++ b/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap @@ -0,0 +1,9 @@ +--- +source: prqlc/prql-compiler/tests/integration/resolving.rs +expression: "resolve(r#\"\n type Status = (\n Paid = () ||\n Unpaid = float ||\n Canceled = {reason = text, cancelled_at = timestamp} ||\n )\n \"#).unwrap()" +--- +type Status = + float || + {reason = text, cancelled_at = timestamp} || + + diff --git a/prqlc/prqlc-parser/src/types.rs b/prqlc/prqlc-parser/src/types.rs index d8073254936b..e063ca6107e3 100644 --- a/prqlc/prqlc-parser/src/types.rs +++ b/prqlc/prqlc-parser/src/types.rs @@ -81,6 +81,28 @@ pub fn type_expr() -> impl Parser { .map(TyKind::Tuple) .labelled("tuple"); + let union_parenthesized = ident_part() + .then_ignore(ctrl('=')) + .or_not() + .then(nested_type_expr.clone()) + .padded_by(new_line().repeated()) + .separated_by(just(Token::Or)) + .allow_trailing() + .then_ignore(new_line().repeated()) + .delimited_by(ctrl('('), ctrl(')')) + .recover_with(nested_delimiters( + Token::Control('('), + Token::Control(')'), + [ + (Token::Control('{'), Token::Control('}')), + (Token::Control('('), Token::Control(')')), + (Token::Control('['), Token::Control(']')), + ], + |_| vec![], + )) + .map(TyKind::Union) + .labelled("union"); + let array = nested_type_expr .map(Box::new) .padded_by(new_line().repeated()) @@ -98,7 +120,7 @@ pub fn type_expr() -> impl Parser { .map(TyKind::Array) .labelled("array"); - let term = choice((basic, ident, func, tuple, array)) + let term = choice((basic, ident, func, tuple, array, union_parenthesized)) .map_with_span(into_ty) .boxed(); From 6fca655984dab5f9c3af7315befcfc9b94ac40e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Tue, 2 Jan 2024 12:20:53 +0100 Subject: [PATCH 2/3] fix --- prqlc/Taskfile.yaml | 2 +- prqlc/prql-compiler/src/codegen/types.rs | 2 +- prqlc/prql-compiler/src/sql/gen_expr.rs | 4 ++-- prqlc/prql-compiler/tests/integration/error_messages.rs | 2 +- prqlc/prql-compiler/tests/integration/sql.rs | 2 +- prqlc/prqlc-parser/src/interpolation.rs | 8 ++++---- prqlc/prqlc/tests/snapshots/test__debug.snap | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/prqlc/Taskfile.yaml b/prqlc/Taskfile.yaml index ac2f064b90c8..4fc28cef69ec 100644 --- a/prqlc/Taskfile.yaml +++ b/prqlc/Taskfile.yaml @@ -17,7 +17,7 @@ tasks: cmds: - cmd: | # remove trailing whitespace - rg '\s+$' --files-with-matches --glob '!*.rs' . \ + rg '\s+$' --files-with-matches --glob '!*.{rs,snap}' . \ | xargs -I _ sh -c "echo Removing trailing whitespace from _ && sd '[\t ]+$' '' _" - cmd: | diff --git a/prqlc/prql-compiler/src/codegen/types.rs b/prqlc/prql-compiler/src/codegen/types.rs index 7e35c8ee9357..4ad1530787f6 100644 --- a/prqlc/prql-compiler/src/codegen/types.rs +++ b/prqlc/prql-compiler/src/codegen/types.rs @@ -38,7 +38,7 @@ impl WriteSource for TyKind { Ident(ident) => ident.write(opt), Primitive(prim) => Some(prim.to_string()), Union(variants) => { - let parenthesize = + let parenthesize = // never must be parenthesized variants.is_empty() || // named union must be parenthesized diff --git a/prqlc/prql-compiler/src/sql/gen_expr.rs b/prqlc/prql-compiler/src/sql/gen_expr.rs index 547d2e790990..a4985692e29d 100644 --- a/prqlc/prql-compiler/src/sql/gen_expr.rs +++ b/prqlc/prql-compiler/src/sql/gen_expr.rs @@ -813,8 +813,8 @@ pub(super) fn translate_operand( /// parentheses are not required. Some examples of when parentheses are not required: /// - `(a - b) - c` & `(a + b) - c` — as opposed to `a - (b - c)` /// - `a + (b - c)` & `a + (b + c)` — as opposed to `a - (b + c)` & `a - (b - c)` -/// -/// +/// +/// // // If it were possible to evaluate this with less context that would be // preferable, but it's not clear how to do that. (For example, even if we diff --git a/prqlc/prql-compiler/tests/integration/error_messages.rs b/prqlc/prql-compiler/tests/integration/error_messages.rs index 40ec3cf36a48..34b268fc1e56 100644 --- a/prqlc/prql-compiler/tests/integration/error_messages.rs +++ b/prqlc/prql-compiler/tests/integration/error_messages.rs @@ -324,7 +324,7 @@ fn date_to_text_with_column_format() { fn date_to_text_unsupported_chrono_item() { assert_display_snapshot!(compile(r#" prql target:sql.duckdb - + from [{d = @2021-01-01}] derive { d_str = d | date.to_text "%_j" diff --git a/prqlc/prql-compiler/tests/integration/sql.rs b/prqlc/prql-compiler/tests/integration/sql.rs index f18d7b396240..d1467ea3de26 100644 --- a/prqlc/prql-compiler/tests/integration/sql.rs +++ b/prqlc/prql-compiler/tests/integration/sql.rs @@ -318,7 +318,7 @@ fn test_precedence() { assert_display_snapshot!((compile(r###" from numbers derive { - sum_1 = a + b, + sum_1 = a + b, sum_2 = add a b, g = -a } diff --git a/prqlc/prqlc-parser/src/interpolation.rs b/prqlc/prqlc-parser/src/interpolation.rs index bf1799202f9d..cf9cd53648b9 100644 --- a/prqlc/prqlc-parser/src/interpolation.rs +++ b/prqlc/prqlc-parser/src/interpolation.rs @@ -27,7 +27,7 @@ fn parse_interpolate() { let span_base = ParserSpan::new(0, 0..0); assert_debug_snapshot!( - parse("concat({a})".to_string(), span_base).unwrap(), + parse("concat({a})".to_string(), span_base).unwrap(), @r###" [ String( @@ -55,7 +55,7 @@ fn parse_interpolate() { "###); assert_debug_snapshot!( - parse("print('{{hello}}')".to_string(), span_base).unwrap(), + parse("print('{{hello}}')".to_string(), span_base).unwrap(), @r###" [ String( @@ -65,7 +65,7 @@ fn parse_interpolate() { "###); assert_debug_snapshot!( - parse("concat('{{', a, '}}')".to_string(), span_base).unwrap(), + parse("concat('{{', a, '}}')".to_string(), span_base).unwrap(), @r###" [ String( @@ -75,7 +75,7 @@ fn parse_interpolate() { "###); assert_debug_snapshot!( - parse("concat('{{', {a}, '}}')".to_string(), span_base).unwrap(), + parse("concat('{{', {a}, '}}')".to_string(), span_base).unwrap(), @r###" [ String( diff --git a/prqlc/prqlc/tests/snapshots/test__debug.snap b/prqlc/prqlc/tests/snapshots/test__debug.snap index 3d38bd96c096..25e9715d6eee 100644 --- a/prqlc/prqlc/tests/snapshots/test__debug.snap +++ b/prqlc/prqlc/tests/snapshots/test__debug.snap @@ -7,10 +7,10 @@ info: - debug - resolve env: - RUST_BACKTRACE: "" CLICOLOR_FORCE: "" - NO_COLOR: "1" + RUST_BACKTRACE: "" RUST_LOG: "" + NO_COLOR: "1" stdin: from tracks --- success: true From 54d540d55ee1c67d6093da3d61f49461d55f74cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= Date: Tue, 2 Jan 2024 12:33:10 +0100 Subject: [PATCH 3/3] fix --- .../integration__resolving__resolve_types_04.snap | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap diff --git a/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap b/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap deleted file mode 100644 index f4a1d548a032..000000000000 --- a/prqlc/prql-compiler/tests/integration/snapshots/integration__resolving__resolve_types_04.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: prqlc/prql-compiler/tests/integration/resolving.rs -expression: "resolve(r#\"\n type Status = (\n Paid = () ||\n Unpaid = float ||\n Canceled = {reason = text, cancelled_at = timestamp} ||\n )\n \"#).unwrap()" ---- -type Status = - float || - {reason = text, cancelled_at = timestamp} || - -