diff --git a/crates/sqlquack-rules/src/conversion/_helpers/queries/query.rs b/crates/sqlquack-rules/src/conversion/_helpers/queries/query.rs index 9f31153c1..2723d2c36 100644 --- a/crates/sqlquack-rules/src/conversion/_helpers/queries/query.rs +++ b/crates/sqlquack-rules/src/conversion/_helpers/queries/query.rs @@ -607,7 +607,19 @@ fn convert_projection( let _rule = site.context.log.rule(COMMENT); let annotated = payload_object(payload, &construct)?; require_understood(annotated, &["this", "trailing_comments"], &construct)?; - convert_projection(field(annotated, "this", &construct)?, site, named) + let this = field(annotated, "this", &construct)?; + match node_kind(this) { + Some((kind, _)) if named && !matches!(kind, "star" | "alias" | "column") => { + let refusal = missing( + Construct::new(ConstructFamily::Syntax, UNALIASED_OUTPUT_NAME_CONSTRUCT), + format!( + "Snowflake names this `{kind}` column after its text, comment included" + ), + ); + Err(site.context.locator.placed(refusal, this)) + } + _ => convert_projection(this, site, named), + } } Some(("alias", payload)) => { let _rule = site.context.log.rule(ALIAS); diff --git a/crates/sqlquack-rules/src/conversion/_helpers/support/tree.rs b/crates/sqlquack-rules/src/conversion/_helpers/support/tree.rs index 32afd251b..e9752cea2 100644 --- a/crates/sqlquack-rules/src/conversion/_helpers/support/tree.rs +++ b/crates/sqlquack-rules/src/conversion/_helpers/support/tree.rs @@ -18,6 +18,7 @@ const COSMETIC_FIELDS: &[&str] = &[ "comments", "leading_comments", "pre_alias_comments", + "post_select_comments", "left_comments", "operator_comments", "right_comments", @@ -194,13 +195,65 @@ pub(crate) fn integer_literal(node: &Value) -> Option { /// The tree conversion reads, positions stripped and quoted identifiers folded if case is ignored. pub(crate) fn session_tree(positioned: &Value, ignore_quoted_case: bool) -> Result { - let mut tree = strip_positions(positioned); + let mut tree = without_comment_wrappers(&strip_positions(positioned)); if ignore_quoted_case { fold_quoted(&mut tree)?; } Ok(tree) } +/// `value` with each node that only wrapped a comment replaced by the node it wrapped. +fn without_comment_wrappers(value: &Value) -> Value { + unwrap_comments(value, false) +} + +/// `value` with its comment wrappers unwrapped, its own kept if `keep_wrapper`. +fn unwrap_comments(value: &Value, keep_wrapper: bool) -> Value { + match value { + Value::Object(fields) => match fields.get("annotated").and_then(Value::as_object) { + Some(annotated) if !keep_wrapper && fields.len() == 1 && annotated.len() == 1 => { + annotated + .get("this") + .map_or_else(|| value.clone(), without_comment_wrappers) + } + _ => Value::Object( + fields + .iter() + .map(|(key, field)| (key.clone(), unwrap_field(key, field))) + .collect(), + ), + }, + Value::Array(items) => Value::Array(items.iter().map(without_comment_wrappers).collect()), + other => other.clone(), + } +} + +/// A node's field unwrapped; a SELECT keeps its select items' wrappers, which name their columns. +fn unwrap_field(key: &str, field: &Value) -> Value { + match (key, field) { + ("select", Value::Object(clauses)) => Value::Object( + clauses + .iter() + .map(|(name, clause)| (name.clone(), unwrap_clause(name, clause))) + .collect(), + ), + _ => without_comment_wrappers(field), + } +} + +/// A SELECT clause with comment wrappers unwrapped, except around the select items themselves. +fn unwrap_clause(name: &str, clause: &Value) -> Value { + match (name, clause) { + ("expressions", Value::Array(items)) => Value::Array( + items + .iter() + .map(|item| unwrap_comments(item, true)) + .collect(), + ), + _ => without_comment_wrappers(clause), + } +} + /// Upper-cases the name of each quoted identifier under `value`. fn fold_quoted(value: &mut Value) -> Result<(), String> { match value { diff --git a/crates/sqlquack/src/analysis/tests/test_analyze.rs b/crates/sqlquack/src/analysis/tests/test_analyze.rs index 84eccf154..74a28b0e9 100644 --- a/crates/sqlquack/src/analysis/tests/test_analyze.rs +++ b/crates/sqlquack/src/analysis/tests/test_analyze.rs @@ -92,6 +92,32 @@ fn given_sql_and_a_schema_when_analyzing_then_it_converts_or_refuses_with_a_cate converted: true, }, }, + AnalyzeTestCase { + description: "a block comment straight after SELECT is dropped", + sql: "SELECT /* c */ amount AS v FROM orders", + expected_summary: converted(), + }, + AnalyzeTestCase { + description: "a line comment straight after SELECT is dropped", + sql: "SELECT -- note\n amount AS v FROM orders", + expected_summary: converted(), + }, + AnalyzeTestCase { + description: "a comment after SELECT inside a CTE and a subquery is dropped", + sql: "WITH x AS (SELECT /* c */ amount FROM orders) \ + SELECT v FROM (SELECT /* d */ amount AS v FROM x) AS s", + expected_summary: converted(), + }, + AnalyzeTestCase { + description: "a comment after a literal argument leaves the literal a literal", + sql: "SELECT ROUND(amount, 1 /* c */) AS v FROM orders", + expected_summary: converted(), + }, + AnalyzeTestCase { + description: "an unaliased literal carrying a comment has no recorded column name", + sql: "SELECT 'x' /* c */ FROM orders", + expected_summary: refused(RefusalCategory::MissingRule), + }, AnalyzeTestCase { description: "comments after select items and inside expressions are dropped", sql: "SELECT amount /* net */ + 1 AS gross, -- trailing\n customer_id -- last\n \