From 916acb1d1842db3c1b305ea8a9f9e7dd048d7793 Mon Sep 17 00:00:00 2001 From: kvlonge <273747162+kvlonge@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:48:47 +0000 Subject: [PATCH] fix(format): lay out CTE macros with comments between them A comment between two CTE macro placeholders stays in the SQL laid out without them, so the placeholders after it are now put back after it as a group of their own. Before, the whole run went back before the comment, the token check failed, and the query was declined with "native formatter could not restore CTE macro placeholders". --- .../src/sql_lint/_helpers/cte_macros.rs | 24 +++++++++++++++---- .../src/sql_lint/tests/test_layout_rules.rs | 7 ++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs b/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs index c55eccb..7fa720d 100644 --- a/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs +++ b/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs @@ -21,7 +21,7 @@ pub(super) struct WithoutCteMacros { original: Vec<(bool, String)>, } -/// Placeholders, with the commas taken out with them, that stood before one significant token. +/// Placeholders, with the commas taken out with them, that stood before one significant token with no comment between them; a comment between placeholders stays in the stripped SQL, and the placeholders after it make the next group. struct MacroGroup { /// The index, among the significant tokens of the stripped SQL, of the token they preceded. anchor: usize, @@ -63,13 +63,27 @@ pub(super) fn take_out_cte_macros( let previous_end = position .checked_sub(1) .map_or(0, |previous| tokens[significant[previous]].end); - let comments_before = tokens[..significant[position]] - .iter() - .filter(|token| token.start >= previous_end && is_comment(token)) - .count(); + let comments_until = |end: usize| { + tokens + .iter() + .filter(|token| { + token.start >= previous_end && token.end <= end && is_comment(token) + }) + .count() + }; + let mut comments_before = comments_until(tokens[significant[position]].start); let mut texts: Vec = Vec::new(); while removed.contains(&position) { let index = significant[position]; + let comments = comments_until(tokens[index].start); + if comments > comments_before && tokens[index].text(sql) != COMMA { + groups.push(MacroGroup { + anchor: kept, + comments_before, + texts: std::mem::take(&mut texts), + }); + comments_before = comments; + } stripped.push_str(&sql[cursor..tokens[index].start]); texts.push(tokens[index].text(sql).to_string()); cursor = tokens diff --git a/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs b/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs index 2c9d278..361bd46 100644 --- a/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs +++ b/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs @@ -121,6 +121,13 @@ fn given_clause_layout_rules_when_formatting_then_each_rule_applies_idempotently line_width: 100, expected_sql: "WITH a AS (\n SELECT 1 AS x\n),\n\n__sqlclean_interpolation_0__\n\n__sqlclean_interpolation_1__\n\npaid AS (\n SELECT *\n FROM a\n)\n\nSELECT *\nFROM paid", }, + test_types::LayoutRuleTestCase { + description: "CTE macro placeholders keep their place among the comments between them", + dialect: "duckdb", + sql: "WITH base AS (SELECT 1 AS order_id),\n-- First order stage.\n__sqlclean_interpolation_0__,\n-- Second order stage.\n__sqlclean_interpolation_1__,\n__sqlclean_interpolation_2__,\n-- Publish order identifiers.\nfinal AS (SELECT order_id FROM third_orders)\nSELECT * FROM final", + line_width: 100, + expected_sql: "WITH base AS (\n SELECT 1 AS order_id\n),\n\n-- First order stage.\n__sqlclean_interpolation_0__,\n\n-- Second order stage.\n__sqlclean_interpolation_1__,\n\n__sqlclean_interpolation_2__,\n\n-- Publish order identifiers.\nfinal AS (\n SELECT order_id\n FROM third_orders\n)\n\nSELECT *\nFROM final", + }, test_types::LayoutRuleTestCase { description: "a placeholder standing for a CTE's whole query keeps its place", dialect: "snowflake",