From 063159db09cf5957e6d9569dea02b18cea9dc8d5 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 19 Jun 2024 17:45:40 -0700 Subject: [PATCH] feat(fmt): An attempt at aesthetic items into PL This is an attempt to get around the complications of managing lexer + parser output, which #4397 has hit in a few incarnations by just adding comments ('aesthetics') to PL. This very very nearly works -- with chumsky we can create a function that wraps anything that might have a comment, implement a trait on the AST items that contain it, and away we go (though it did require a lot of debugging in the end). This would then be really easy to write back out. I think there's literally a single case where it doesn't work -- where a comment doesn't come directly before or directly after an AST item -- in the final trailing comma of a tuple or array. So tests fail at the moment. Next we need to consider: - Can we workaround that one case? We don't actually care about whether there's a trailing comma, so we could likely hack around it... - Are there actually other cases of this model failing? I know this approach -- of putting aesthetic items into AST -- is not generally favored, and it's really rare that there's even a single case of something not working. --- prqlc/prqlc-ast/src/expr.rs | 24 ++++- prqlc/prqlc-ast/src/lib.rs | 8 ++ prqlc/prqlc-ast/src/stmt.rs | 42 +++++++- prqlc/prqlc-parser/src/expr.rs | 44 +++++---- prqlc/prqlc-parser/src/interpolation.rs | 4 + prqlc/prqlc-parser/src/lib.rs | 63 ++++++++---- ...qlc_parser__test__pipeline_parse_tree.snap | 8 ++ prqlc/prqlc-parser/src/stmt.rs | 51 +++++++--- prqlc/prqlc-parser/src/test.rs | 11 ++- prqlc/prqlc-parser/src/types.rs | 2 +- prqlc/prqlc/src/semantic/ast_expand.rs | 6 ++ .../tests/integration/ast_code_matches.rs | 95 ++++++++++++++++++- ...__queries__debug_lineage__aggregation.snap | 5 + ...n__queries__debug_lineage__arithmetic.snap | 2 + ...gration__queries__debug_lineage__cast.snap | 2 + ..._queries__debug_lineage__date_to_text.snap | 5 + ...ion__queries__debug_lineage__distinct.snap | 2 + ...__queries__debug_lineage__distinct_on.snap | 2 + ..._queries__debug_lineage__genre_counts.snap | 3 + ...on__queries__debug_lineage__group_all.snap | 2 + ...n__queries__debug_lineage__group_sort.snap | 2 + ..._debug_lineage__group_sort_limit_take.snap | 3 + ...ueries__debug_lineage__invoice_totals.snap | 3 + ...tion__queries__debug_lineage__loop_01.snap | 3 + ...__queries__debug_lineage__math_module.snap | 3 + ...on__queries__debug_lineage__pipelines.snap | 4 + ...ion__queries__debug_lineage__read_csv.snap | 4 + ...ueries__debug_lineage__set_ops_remove.snap | 2 + ...gration__queries__debug_lineage__sort.snap | 8 +- ...ation__queries__debug_lineage__switch.snap | 3 + ...gration__queries__debug_lineage__take.snap | 2 + ...__queries__debug_lineage__text_module.snap | 4 + ...ation__queries__debug_lineage__window.snap | 10 ++ 33 files changed, 370 insertions(+), 62 deletions(-) diff --git a/prqlc/prqlc-ast/src/expr.rs b/prqlc/prqlc-ast/src/expr.rs index 36c7ed0adc2e..e908bb6aae04 100644 --- a/prqlc/prqlc-ast/src/expr.rs +++ b/prqlc/prqlc-ast/src/expr.rs @@ -11,8 +11,8 @@ pub use self::ident::Ident; pub use self::ops::{BinOp, UnOp}; pub use self::token::{Literal, ValueAndUnit}; use super::token; -use crate::span::Span; -use crate::Ty; +use crate::{span::Span, WithAesthetics}; +use crate::{TokenKind, Ty}; impl Expr { pub fn new>(kind: K) -> Self { @@ -20,6 +20,8 @@ impl Expr { kind: kind.into(), span: None, alias: None, + aesthetics_before: Vec::new(), + aesthetics_after: Vec::new(), } } } @@ -38,6 +40,24 @@ pub struct Expr { #[serde(skip_serializing_if = "Option::is_none")] pub alias: Option, + + // Maybe should be Token? + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_before: Vec, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_after: Vec, +} + +impl WithAesthetics for Expr { + fn with_aesthetics( + mut self, + aesthetics_before: Vec, + aesthetics_after: Vec, + ) -> Self { + self.aesthetics_before = aesthetics_before; + self.aesthetics_after = aesthetics_after; + self + } } #[derive(Debug, EnumAsInner, PartialEq, Clone, Serialize, Deserialize, strum::AsRefStr)] diff --git a/prqlc/prqlc-ast/src/lib.rs b/prqlc/prqlc-ast/src/lib.rs index ec8101ccf097..c42368980fbe 100644 --- a/prqlc/prqlc-ast/src/lib.rs +++ b/prqlc/prqlc-ast/src/lib.rs @@ -9,3 +9,11 @@ pub use span::*; pub use stmt::*; pub use token::*; pub use types::*; + +pub trait WithAesthetics { + fn with_aesthetics( + self, + aesthetics_before: Vec, + aethetics_after: Vec, + ) -> Self; +} diff --git a/prqlc/prqlc-ast/src/stmt.rs b/prqlc/prqlc-ast/src/stmt.rs index 45fd0cd78182..2ac135cf2c92 100644 --- a/prqlc/prqlc-ast/src/stmt.rs +++ b/prqlc/prqlc-ast/src/stmt.rs @@ -4,7 +4,7 @@ use enum_as_inner::EnumAsInner; use semver::VersionReq; use serde::{Deserialize, Serialize}; -use crate::{expr::Expr, Ident, Span, Ty}; +use crate::{expr::Expr, Ident, Span, TokenKind, Ty, WithAesthetics}; #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Default)] pub struct QueryDef { @@ -31,6 +31,26 @@ pub struct Stmt { #[serde(skip_serializing_if = "Vec::is_empty", default)] pub annotations: Vec, + + // Maybe should be Token? + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_before: Vec, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_after: Vec, +} + +impl WithAesthetics for Stmt { + fn with_aesthetics( + self, + aesthetics_before: Vec, + aesthetics_after: Vec, + ) -> Self { + Stmt { + aesthetics_before, + aesthetics_after, + ..self + } + } } #[derive(Debug, EnumAsInner, PartialEq, Clone, Serialize, Deserialize)] @@ -73,6 +93,24 @@ pub struct ImportDef { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Annotation { pub expr: Box, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_before: Vec, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub aesthetics_after: Vec, +} + +impl WithAesthetics for Annotation { + fn with_aesthetics( + self, + aesthetics_before: Vec, + aesthetics_after: Vec, + ) -> Self { + Annotation { + aesthetics_before, + aesthetics_after, + ..self + } + } } impl Stmt { @@ -81,6 +119,8 @@ impl Stmt { kind, span: None, annotations: Vec::new(), + aesthetics_before: Vec::new(), + aesthetics_after: Vec::new(), } } } diff --git a/prqlc/prqlc-parser/src/expr.rs b/prqlc/prqlc-parser/src/expr.rs index eb7e3b391b3a..85921f6a2042 100644 --- a/prqlc/prqlc-parser/src/expr.rs +++ b/prqlc/prqlc-parser/src/expr.rs @@ -10,7 +10,7 @@ use super::interpolation; use crate::err::parse_error::PError; use crate::types::type_expr; -pub fn expr_call() -> impl Parser { +pub fn expr_call() -> impl Parser + Clone { let expr = expr(); lambda_func(expr.clone()).or(func_call(expr)) @@ -27,7 +27,9 @@ pub fn expr() -> impl Parser + Clone { .map(|x| x.to_string()) .map(ExprKind::Internal); - let nested_expr = pipeline(lambda_func(expr.clone()).or(func_call(expr.clone()))).boxed(); + let nested_expr = with_aesthetics( + pipeline(lambda_func(expr.clone()).or(func_call(expr.clone()))).boxed(), + ); let tuple = ident_part() .then_ignore(ctrl('=')) @@ -122,18 +124,20 @@ pub fn expr() -> impl Parser + Clone { let param = select! { TokenKind::Param(id) => ExprKind::Param(id) }; - let term = choice(( - literal, - internal, - tuple, - array, - interpolation, - ident_kind, - case, - param, - )) - .map_with_span(into_expr) - .or(pipeline) + let term = with_aesthetics( + choice(( + literal, + internal, + tuple, + array, + interpolation, + ident_kind, + case, + param, + )) + .map_with_span(into_expr) + .or(pipeline), + ) .boxed(); // indirections @@ -229,9 +233,9 @@ pub fn expr() -> impl Parser + Clone { }) } -pub fn pipeline(expr: E) -> impl Parser +pub fn pipeline(expr: E) -> impl Parser + Clone where - E: Parser, + E: Parser + Clone, { // expr has to be a param, because it can be either a normal expr() or // a recursive expr called from within expr() @@ -264,7 +268,7 @@ where pub fn binary_op_parser<'a, Term, Op>( term: Term, op: Op, -) -> impl Parser + 'a +) -> impl Parser + 'a + Clone where Term: Parser + 'a, Op: Parser + 'a, @@ -290,7 +294,7 @@ where .boxed() } -fn func_call(expr: E) -> impl Parser +fn func_call(expr: E) -> impl Parser + Clone where E: Parser + Clone, { @@ -342,7 +346,7 @@ where .labelled("function call") } -fn lambda_func(expr: E) -> impl Parser +fn lambda_func(expr: E) -> impl Parser + Clone where E: Parser + Clone + 'static, { @@ -402,7 +406,7 @@ where .labelled("function definition") } -pub fn ident() -> impl Parser { +pub fn ident() -> impl Parser + Clone { ident_part() .separated_by(ctrl('.')) .at_least(1) diff --git a/prqlc/prqlc-parser/src/interpolation.rs b/prqlc/prqlc-parser/src/interpolation.rs index 12af7f14f46f..109fbde7b641 100644 --- a/prqlc/prqlc-parser/src/interpolation.rs +++ b/prqlc/prqlc-parser/src/interpolation.rs @@ -99,6 +99,8 @@ fn parse_interpolate() { 0:8-9, ), alias: None, + aesthetics_before: [], + aesthetics_after: [], }, format: None, }, @@ -144,6 +146,8 @@ fn parse_interpolate() { 0:14-15, ), alias: None, + aesthetics_before: [], + aesthetics_after: [], }, format: None, }, diff --git a/prqlc/prqlc-parser/src/lib.rs b/prqlc/prqlc-parser/src/lib.rs index 91a408f0a10a..072f7b8e2704 100644 --- a/prqlc/prqlc-parser/src/lib.rs +++ b/prqlc/prqlc-parser/src/lib.rs @@ -23,6 +23,7 @@ pub fn parse_source(source: &str, source_id: u16) -> Result, Vec Result, Vec = tokens.map(|tokens| { - tokens.into_iter().filter(|token| { - !matches!( - token.kind, - TokenKind::Comment(_) | TokenKind::LineWrap(_) | TokenKind::DocComment(_) - ) - }) - }); - - let ast = if let Some(semantic_tokens) = semantic_tokens { - let stream = prepare_stream(semantic_tokens, source, source_id); + let ast = if let Some(tokens) = tokens { + let stream = prepare_stream(tokens.into_iter(), source, source_id); - let (ast, parse_errors) = ::chumsky::Parser::parse_recovery(&stmt::source(), stream); + let (ast, parse_errors) = + // ::chumsky::Parser::parse_recovery_verbose(&stmt::source(), stream); + ::chumsky::Parser::parse_recovery(&stmt::source(), stream); log::debug!("parse errors: {:?}", parse_errors); errors.extend(parse_errors.into_iter().map(|e| e.into())); @@ -72,16 +64,16 @@ pub fn lex_source(source: &str) -> Result> { mod common { use chumsky::prelude::*; - use prqlc_ast::expr::*; use prqlc_ast::stmt::*; use prqlc_ast::token::*; use prqlc_ast::Span; use prqlc_ast::Ty; use prqlc_ast::TyKind; + use prqlc_ast::{expr::*, WithAesthetics}; use crate::err::parse_error::PError; - pub fn ident_part() -> impl Parser { + pub fn ident_part() -> impl Parser + Clone { return select! { TokenKind::Ident(ident) => ident, TokenKind::Keyword(ident) if &ident == "module" => ident, @@ -112,6 +104,8 @@ mod common { kind, span: Some(span), annotations, + aesthetics_before: Vec::new(), + aesthetics_after: Vec::new(), } } @@ -128,6 +122,43 @@ mod common { ..Ty::new(kind) } } + + pub fn aesthetic() -> impl Parser + Clone { + select! { + TokenKind::Comment(comment) => TokenKind::Comment(comment), + TokenKind::LineWrap(lw) => TokenKind::LineWrap(lw), + TokenKind::DocComment(dc) => TokenKind::DocComment(dc), + } + } + + pub fn with_aesthetics(parser: P) -> impl Parser + Clone + where + P: Parser + Clone, + O: WithAesthetics, + { + // We can have newlines between the aesthetics and the actual token to + // cover a case like `# foo` here: + // + // ```prql + // # foo + // + // from bar + // # baz + // select artists + // ``` + // + // ...but not after the aesthetics after the token; since we don't want + // to eat the newline after `from bar` + // + let aesthetics_before = aesthetic().then_ignore(new_line().repeated()).repeated(); + let aesthetics_after = aesthetic().separated_by(new_line()); + + aesthetics_before.then(parser).then(aesthetics_after).map( + |((aesthetics_before, inner), aesthetics_after)| { + inner.with_aesthetics(aesthetics_before, aesthetics_after) + }, + ) + } } /// Convert the output of the lexer into the input of the parser. Requires diff --git a/prqlc/prqlc-parser/src/snapshots/prqlc_parser__test__pipeline_parse_tree.snap b/prqlc/prqlc-parser/src/snapshots/prqlc_parser__test__pipeline_parse_tree.snap index 6d6d8c6c3c77..f3b9a1d1fc86 100644 --- a/prqlc/prqlc-parser/src/snapshots/prqlc_parser__test__pipeline_parse_tree.snap +++ b/prqlc/prqlc-parser/src/snapshots/prqlc_parser__test__pipeline_parse_tree.snap @@ -24,6 +24,8 @@ expression: "parse_single(r#\"\nfrom employees\nfilter country == \"USA\" right: Literal: String: USA + aesthetics_after: + - Comment: " Each line transforms the previous result." - FuncCall: name: Ident: derive @@ -36,12 +38,16 @@ expression: "parse_single(r#\"\nfrom employees\nfilter country == \"USA\" right: Ident: payroll_tax alias: gross_salary + aesthetics_before: + - Comment: " This adds columns / variables." - Binary: left: Ident: gross_salary op: Add right: Ident: benefits_cost + aesthetics_after: + - Comment: " Variables can use other variables." alias: gross_cost - FuncCall: name: @@ -71,6 +77,8 @@ expression: "parse_single(r#\"\nfrom employees\nfilter country == \"USA\" Ident: average args: - Ident: salary + aesthetics_before: + - Comment: " Aggregate each group to a single row" - FuncCall: name: Ident: average diff --git a/prqlc/prqlc-parser/src/stmt.rs b/prqlc/prqlc-parser/src/stmt.rs index c63c25a2b20f..2036aea3f9d7 100644 --- a/prqlc/prqlc-parser/src/stmt.rs +++ b/prqlc/prqlc-parser/src/stmt.rs @@ -11,11 +11,11 @@ use semver::VersionReq; use super::common::{ctrl, ident_part, into_stmt, keyword, new_line}; use super::expr::{expr, expr_call, ident, pipeline}; -use crate::err::parse_error::PError; use crate::types::type_expr; +use crate::{common::with_aesthetics, err::parse_error::PError}; pub fn source() -> impl Parser, Error = PError> { - query_def() + with_aesthetics(query_def()) .or_not() .chain(module_contents()) .then_ignore(end()) @@ -34,19 +34,40 @@ fn module_contents() -> impl Parser, Error = PError> { .then_ignore(new_line().repeated()) .map(|expr| Annotation { expr: Box::new(expr), + aesthetics_before: Vec::new(), + aesthetics_after: Vec::new(), }); - annotation - .repeated() - .then(choice((module_def, type_def(), import_def(), var_def()))) - .map_with_span(into_stmt) - .separated_by(new_line().repeated().at_least(1)) - .allow_leading() - .allow_trailing() + // TODO: I think some duplication here; we allow for potential + // newlines before each item here, but then also have `.allow_leading` + // below — since now we can get newlines after a comment between the + // aesthetic item and the stmt... So a bit messy + let stmt_kind = new_line().repeated().ignore_then(choice(( + module_def, + type_def(), + import_def(), + var_def(), + ))); + + // Two wrapping of `with_aesthetics` — the first for the whole block, + // and the second for just the annotation; if there's a comment between + // the annotation and the code. + with_aesthetics( + with_aesthetics(annotation) + .repeated() + // TODO: do we need this? I think possibly we get an additional + // error when we remove it; check (because it seems redundant...). + .then_ignore(new_line().repeated()) + .then(stmt_kind) + .map_with_span(into_stmt), + ) + .separated_by(new_line().repeated().at_least(1)) + .allow_leading() + .allow_trailing() }) } -fn query_def() -> impl Parser { +fn query_def() -> impl Parser + Clone { new_line() .repeated() .ignore_then(keyword("prql")) @@ -115,8 +136,10 @@ fn query_def() -> impl Parser { .labelled("query header") } -fn var_def() -> impl Parser { - let let_ = keyword("let") +fn var_def() -> impl Parser + Clone { + let let_ = new_line() + .repeated() + .ignore_then(keyword("let")) .ignore_then(ident_part()) .then(type_expr().delimited_by(ctrl('<'), ctrl('>')).or_not()) .then(ctrl('=').ignore_then(expr_call()).map(Box::new).or_not()) @@ -153,7 +176,7 @@ fn var_def() -> impl Parser { let_.or(main_or_into) } -fn type_def() -> impl Parser { +fn type_def() -> impl Parser + Clone { keyword("type") .ignore_then(ident_part()) .then(ctrl('=').ignore_then(type_expr()).or_not()) @@ -161,7 +184,7 @@ fn type_def() -> impl Parser { .labelled("type definition") } -fn import_def() -> impl Parser { +fn import_def() -> impl Parser + Clone { keyword("import") .ignore_then(ident_part().then_ignore(ctrl('=')).or_not()) .then(ident()) diff --git a/prqlc/prqlc-parser/src/test.rs b/prqlc/prqlc-parser/src/test.rs index 4f32615a4d09..e6a8b58dadaf 100644 --- a/prqlc/prqlc-parser/src/test.rs +++ b/prqlc/prqlc-parser/src/test.rs @@ -404,6 +404,8 @@ fn test_basic_exprs() { args: - Ident: a span: "0:28-36" + aesthetics_before: + - Comment: " this is a comment" "###); assert_yaml_snapshot!(parse_expr( "join side:left country (id==employee_id)" @@ -1827,7 +1829,7 @@ fn test_dates() { #[test] fn test_multiline_string() { assert_yaml_snapshot!(parse_single(r##" - derive x = r#"r-string test"# + derive x = r"r-string test" "##).unwrap(), @r###" --- - VarDef: @@ -1838,9 +1840,10 @@ fn test_multiline_string() { name: Ident: derive args: - - Ident: r + - Literal: + String: r-string test alias: x - span: "0:9-39" + span: "0:9-37" "### ) } @@ -1928,6 +1931,8 @@ fn test_allowed_idents() { op: EqSelf expr: Ident: employee_id + aesthetics_after: + - Comment: " table with leading underscore" - FuncCall: name: Ident: filter diff --git a/prqlc/prqlc-parser/src/types.rs b/prqlc/prqlc-parser/src/types.rs index 7444842b2338..5c227a02efaf 100644 --- a/prqlc/prqlc-parser/src/types.rs +++ b/prqlc/prqlc-parser/src/types.rs @@ -5,7 +5,7 @@ use super::common::*; use crate::err::parse_error::PError; use crate::expr::ident; -pub fn type_expr() -> impl Parser { +pub fn type_expr() -> impl Parser + Clone { recursive(|nested_type_expr| { let basic = select! { TokenKind::Literal(lit) => TyKind::Singleton(lit), diff --git a/prqlc/prqlc/src/semantic/ast_expand.rs b/prqlc/prqlc/src/semantic/ast_expand.rs index 44d9a1a9cb91..f09ec287444b 100644 --- a/prqlc/prqlc/src/semantic/ast_expand.rs +++ b/prqlc/prqlc/src/semantic/ast_expand.rs @@ -292,6 +292,8 @@ pub fn restrict_expr(expr: pl::Expr) -> ast::Expr { kind: restrict_expr_kind(expr.kind), span: expr.span, alias: expr.alias, + aesthetics_before: vec![], + aesthetics_after: vec![], } } @@ -455,12 +457,16 @@ fn restrict_stmt(stmt: pl::Stmt) -> ast::Stmt { .into_iter() .map(restrict_annotation) .collect(), + aesthetics_before: vec![], + aesthetics_after: vec![], } } pub fn restrict_annotation(value: pl::Annotation) -> ast::Annotation { ast::Annotation { expr: restrict_expr_box(value.expr), + aesthetics_before: vec![], + aesthetics_after: vec![], } } diff --git a/prqlc/prqlc/tests/integration/ast_code_matches.rs b/prqlc/prqlc/tests/integration/ast_code_matches.rs index fea731d35cb4..61a61ecf87bb 100644 --- a/prqlc/prqlc/tests/integration/ast_code_matches.rs +++ b/prqlc/prqlc/tests/integration/ast_code_matches.rs @@ -17,6 +17,51 @@ fn test_expr_ast_code_matches() { -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Clone, PartialEq, Serialize, Deserialize)] @@ .. @@ + - // Maybe should be Token? + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_before: Vec, + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_after: Vec, + -} + + /// Unique identificator of the node. Set exactly once during semantic::resolve. + + #[serde(skip_serializing_if = "Option::is_none")] + + pub id: Option, + @@ .. @@ + -impl WithAesthetics for Expr { + - fn with_aesthetics( + - mut self, + - aesthetics_before: Vec, + - aesthetics_after: Vec, + - ) -> Self { + - self.aesthetics_before = aesthetics_before; + - self.aesthetics_after = aesthetics_after; + - self + - } + + /// For [Ident]s, this is id of node referenced by the ident + + #[serde(skip_serializing_if = "Option::is_none")] + + pub target_id: Option, + + + + /// Type of expression this node represents. + + /// [None] means that type should be inferred. + + #[serde(skip_serializing_if = "Option::is_none")] + + pub ty: Option, + + + + /// Information about where data of this expression will come from. + + /// + + /// Currently, this is used to infer relational pipeline frames. + + /// Must always exists if ty is a relation. + + #[serde(skip_serializing_if = "Option::is_none")] + + pub lineage: Option, + + + + #[serde(skip)] + + pub needs_window: bool, + + + + /// When true on [ExprKind::Tuple], this list will be flattened when placed + + /// in some other list. + + // TODO: maybe we should have a special ExprKind instead of this flag? + + #[serde(skip)] + + pub flatten: bool, + @@ .. @@ - Ident(String), - Indirection { - base: Box, @@ -38,6 +83,8 @@ fn test_expr_ast_code_matches() { - Binary(BinaryExpr), - Unary(UnaryExpr), @@ .. @@ + -} + - -#[derive(Debug, EnumAsInner, PartialEq, Clone, Serialize, Deserialize)] -pub enum IndirectionKind { - Name(String), @@ -50,8 +97,7 @@ fn test_expr_ast_code_matches() { - pub left: Box, - pub op: BinOp, - pub right: Box, - -} - - + @@ .. @@ -#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] -pub struct UnaryExpr { - pub op: UnOp, @@ -59,11 +105,12 @@ fn test_expr_ast_code_matches() { -} - @@ .. @@ + - -#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] -pub struct GenericTypeParam { - /// Assigned name of this generic type argument. - pub name: String, - - + @@ .. @@ - pub domain: Vec, -} - @@ -92,8 +139,48 @@ fn test_stmt_ast_code_matches() { &read_to_string("../prqlc/src/ir/pl/stmt.rs").unwrap(), ), @r###" @@ .. @@ + - + - // Maybe should be Token? + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_before: Vec, + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_after: Vec, + @@ .. @@ + -impl WithAesthetics for Stmt { + - fn with_aesthetics( + - self, + - aesthetics_before: Vec, + - aesthetics_after: Vec, + - ) -> Self { + - Stmt { + - aesthetics_before, + - aesthetics_after, + - ..self + - } + - } + -} + - + @@ .. @@ - pub kind: VarDefKind, @@ .. @@ + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_before: Vec, + - #[serde(skip_serializing_if = "Vec::is_empty")] + - pub aesthetics_after: Vec, + -} + - + -impl WithAesthetics for Annotation { + - fn with_aesthetics( + - self, + - aesthetics_before: Vec, + - aesthetics_after: Vec, + - ) -> Self { + - Annotation { + - aesthetics_before, + - aesthetics_after, + - ..self + - } + - } -} - -impl Stmt { @@ -102,6 +189,8 @@ fn test_stmt_ast_code_matches() { - kind, - span: None, - annotations: Vec::new(), + - aesthetics_before: Vec::new(), + - aesthetics_after: Vec::new(), - } - } "### diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__aggregation.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__aggregation.snap index 56eb48f37e81..930c9963253c 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__aggregation.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__aggregation.snap @@ -258,3 +258,8 @@ ast: args: - Ident: empty_name span: 1:102-244 + aesthetics_before: + - !Comment ' mssql:skip' + - !Comment ' mysql:skip' + - !Comment ' clickhouse:skip' + - !Comment ' glaredb:skip (the string_agg function is not supported)' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__arithmetic.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__arithmetic.snap index 29fd7ffed5bf..145276b511f4 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__arithmetic.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__arithmetic.snap @@ -1159,3 +1159,5 @@ ast: args: - Ident: id span: 1:13-833 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__cast.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__cast.snap index 673866095522..0e090ab209a3 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__cast.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__cast.snap @@ -187,3 +187,5 @@ ast: - Literal: Integer: 20 span: 1:13-106 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__date_to_text.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__date_to_text.snap index a2423ce4dc50..8de3ba831ddd 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__date_to_text.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__date_to_text.snap @@ -546,3 +546,8 @@ ast: String: 100%% in %d days alias: d12 span: 1:57-719 + aesthetics_before: + - !Comment ' generic:skip' + - !Comment ' glaredb:skip' + - !Comment ' sqlite:skip' + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct.snap index d2c9f6f98c33..efc1d91446a0 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct.snap @@ -209,3 +209,5 @@ ast: Ident: tracks field: Star span: 1:13-91 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct_on.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct_on.snap index 5b1eb4ad31d4..109c88863b91 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct_on.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__distinct_on.snap @@ -264,3 +264,5 @@ ast: Ident: genre_id - Ident: media_type_id span: 1:13-160 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__genre_counts.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__genre_counts.snap index d8a39e5b9d1c..960ab255aa48 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__genre_counts.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__genre_counts.snap @@ -116,6 +116,9 @@ ast: - Ident: name alias: a span: 1:117-185 + aesthetics_before: + - !Comment ' clickhouse:skip (ClickHouse prefers aliases to column names https://github.com/PRQL/prql/issues/2827)' + - !Comment ' mssql:test' - VarDef: kind: Main name: main diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_all.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_all.snap index a664ec8a01f4..67750c5dc693 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_all.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_all.snap @@ -307,3 +307,5 @@ ast: args: - Ident: album_id span: 1:13-161 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort.snap index 69119a7b38af..6b1bb688413a 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort.snap @@ -297,3 +297,5 @@ ast: alias: d1 - Ident: n1 span: 1:13-151 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort_limit_take.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort_limit_take.snap index b6d9be2bc15f..a4a91ab9f0b2 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort_limit_take.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__group_sort_limit_take.snap @@ -373,3 +373,6 @@ ast: expr: Ident: milliseconds span: 1:76-252 + aesthetics_before: + - !Comment ' Compute the 3 longest songs for each genre and sort by genre' + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__invoice_totals.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__invoice_totals.snap index 4054030d0bb4..8cebc7fa91b2 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__invoice_totals.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__invoice_totals.snap @@ -901,3 +901,6 @@ ast: - Literal: Integer: 20 span: 1:131-792 + aesthetics_before: + - !Comment ' clickhouse:skip (clickhouse doesn''t have lag function)' + - !DocComment ' Calculate a number of metrics about the sales of tracks in each city.' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__loop_01.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__loop_01.snap index 96f527899b53..29e8542c9e12 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__loop_01.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__loop_01.snap @@ -332,3 +332,6 @@ ast: args: - Ident: n span: 1:162-257 + aesthetics_before: + - !Comment ' clickhouse:skip (DB::Exception: Syntax error)' + - !Comment ' glaredb:skip (DataFusion does not support recursive CTEs https://github.com/apache/arrow-datafusion/issues/462)' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__math_module.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__math_module.snap index da55144526c2..3d6cbcffc293 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__math_module.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__math_module.snap @@ -757,3 +757,6 @@ ast: Integer: 2 alias: total_square span: 1:82-788 + aesthetics_before: + - !Comment ' mssql:test' + - !Comment ' sqlite:skip (see https://github.com/rusqlite/rusqlite/issues/1211)' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__pipelines.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__pipelines.snap index db6ae5f060e6..2e193153c425 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__pipelines.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__pipelines.snap @@ -308,3 +308,7 @@ ast: - Ident: name - Ident: composer span: 1:166-298 + aesthetics_before: + - !Comment ' sqlite:skip (Only works on Sqlite implementations which have the extension' + - !Comment ' installed' + - !Comment ' https://stackoverflow.com/questions/24037982/how-to-use-regexp-in-sqlite)' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__read_csv.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__read_csv.snap index 009ebf55c985..961711aa69c0 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__read_csv.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__read_csv.snap @@ -66,3 +66,7 @@ ast: args: - Ident: media_type_id span: 1:43-111 + aesthetics_before: + - !Comment ' sqlite:skip' + - !Comment ' postgres:skip' + - !Comment ' mysql:skip' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__set_ops_remove.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__set_ops_remove.snap index a36f100dd7fb..32c9c55189d5 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__set_ops_remove.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__set_ops_remove.snap @@ -276,6 +276,8 @@ ast: named_params: [] generic_type_params: [] span: 1:13-79 + aesthetics_before: + - !Comment ' mssql:test' - VarDef: kind: Main name: main diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__sort.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__sort.snap index 4f92fb305d05..48ab859dc5a0 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__sort.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__sort.snap @@ -26,7 +26,7 @@ frames: table: - default_db - employees -- - 1:145-215 +- - 1:92-215 - columns: - !All input_id: 128 @@ -170,7 +170,7 @@ nodes: - 119 - id: 143 kind: 'TransformCall: Join' - span: 1:145-215 + span: 1:92-215 children: - 138 - 119 @@ -256,6 +256,8 @@ ast: - FuncCall: name: Ident: join + aesthetics_before: + - !Comment ' joining may use HashMerge, which can undo ORDER BY' args: - Ident: employees alias: manager @@ -292,3 +294,5 @@ ast: Ident: manager field: !Name first_name span: 1:13-272 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__switch.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__switch.snap index 7af6fc2334b6..c4057740161f 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__switch.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__switch.snap @@ -212,3 +212,6 @@ ast: - Literal: Integer: 10 span: 1:89-255 + aesthetics_before: + - !Comment ' glaredb:skip (May be a bag of String type conversion for Postgres Client)' + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__take.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__take.snap index f2d9b74335bb..cba8322bda57 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__take.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__take.snap @@ -103,3 +103,5 @@ ast: Literal: Integer: 5 span: 1:13-52 + aesthetics_before: + - !Comment ' mssql:test' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__text_module.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__text_module.snap index a298ce5786ab..37394d37b0ff 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__text_module.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__text_module.snap @@ -632,3 +632,7 @@ ast: - Literal: String: os span: 1:113-589 + aesthetics_before: + - !Comment ' mssql:test' + - !Comment ' glaredb:skip — TODO: started raising an error on 2024-05-20; see `window.prql`' + - !Comment ' for more details' diff --git a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__window.snap b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__window.snap index 665a6da67969..f01c9ddba58e 100644 --- a/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__window.snap +++ b/prqlc/prqlc/tests/integration/snapshots/integration__queries__debug_lineage__window.snap @@ -459,3 +459,13 @@ ast: Literal: Integer: 22 span: 1:762-1021 + aesthetics_before: + - !Comment ' mssql:skip Conversion("cannot interpret I64(Some(1)) as an i32 value")'', connection.rs:200:34' + - !Comment ' duckdb:skip problems with DISTINCT ON (duckdb internal error: [with INPUT_TYPE = int; RESULT_TYPE = unsigned char]: Assertion `min_val <= input'' failed.)' + - !Comment ' clickhouse:skip problems with DISTINCT ON' + - !Comment ' postgres:skip problems with DISTINCT ON' + - !Comment ' glaredb:skip — TODO: started raising an error on 2024-05-20, from https://github.com/PRQL/prql/actions/runs/9154902656/job/25198160283:' + - !Comment ' ERROR: This feature is not implemented: Unsupported ast node in sqltorel:' + - !Comment ' Substring { expr: Identifier(Ident { value: "title", quote_style: None }),' + - !Comment ' substring_from: Some(Value(Number("2", false))), substring_for:' + - !Comment ' Some(Value(Number("5", false))), special: true }'