From 4c75bd2957f61a4b585dbed2074e0c2f2158c7e9 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 01:53:03 +0100 Subject: [PATCH 01/10] perf(format): build syntax-protection sentinels only when a token needs one Protecting SQL for the layout parse no longer formats a sentinel name and upper-cases every token, and looks cast ranges up by start token instead of scanning them for each token. --- .../src/sql_lint/_helpers/formatter_syntax.rs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs index a6863ca386..2b015c66e2 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs @@ -101,9 +101,14 @@ pub(super) fn protect_syntax( expressions: bool, ) -> Result { let mut prefix = "__sqlbuild_format_".to_string(); - while sql.to_ascii_lowercase().contains(&prefix) { + let lowercase: String = sql.to_ascii_lowercase(); + while lowercase.contains(&prefix) { prefix.push('_'); } + let mut cast_type_ends: HashMap = HashMap::new(); + for (start, end) in cast_types { + let _ = cast_type_ends.entry(*start).or_insert(*end); + } let mut ranges: Vec<(usize, usize, String)> = Vec::new(); let cte_macros = cte_macro_indices(tokens); let depths = token_depths(tokens); @@ -112,23 +117,23 @@ pub(super) fn protect_syntax( let mut index = 0; while index < tokens.len() { let token = &tokens[index]; - let sentinel = format!("{prefix}{}__", ranges.len()); + let sentinel = || format!("{prefix}{}__", ranges.len()); if separators.contains(&index) { ranges.push((token.span.start, token.span.end, ",".to_string())); index += 1; continue; } let (end, replacement) = if cte_macros.contains(&index) { - (index, format!("{sentinel} AS (SELECT 1)")) + (index, format!("{} AS (SELECT 1)", sentinel())) } else if !expressions { index += 1; continue; - } else if let Some((_, end)) = cast_types.iter().find(|(start, _)| *start == index) { - (*end, sentinel) + } else if let Some(end) = cast_type_ends.get(&index) { + (*end, sentinel()) } else if token.token_type == TokenType::DColon { ( postfix_type_end(tokens, index + 1)?, - format!("::{sentinel}"), + format!("::{}", sentinel()), ) } else if token.token_type == TokenType::Colon && index > 0 @@ -144,7 +149,7 @@ pub(super) fn protect_syntax( ) }) { - (index + 1, format!(".{sentinel}")) + (index + 1, format!(".{}", sentinel())) } else if is_typed_literal(tokens, index) || (matches!(token.token_type, TokenType::Var | TokenType::Identifier) && tokens.get(index + 1).is_some_and(|next| { @@ -154,12 +159,14 @@ pub(super) fn protect_syntax( .get(index + 2) .is_some_and(|next| next.token_type == TokenType::Arrow)) { - (index + 1, sentinel) - } else if matches!(token.text.to_ascii_uppercase().as_str(), "MOD" | "POSITION") + (index + 1, sentinel()) + } else if (token.text.eq_ignore_ascii_case("MOD") + || token.text.eq_ignore_ascii_case("POSITION")) && tokens .get(index + 1) .is_some_and(|next| next.token_type == TokenType::LParen) { + let sentinel: String = sentinel(); if token.text.eq_ignore_ascii_case("POSITION") { let separator = (index + 2..tokens.len()) .take_while(|&position| { @@ -180,7 +187,7 @@ pub(super) fn protect_syntax( token.token_type, TokenType::String | TokenType::DollarString ) { - (index, format!("'{sentinel}'")) + (index, format!("'{}'", sentinel())) } else { index += 1; continue; From b881602017a4799f08c8418accab6d05dccda0db Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 01:53:03 +0100 Subject: [PATCH 02/10] perf(format): skip the idempotency re-print for unchanged SQL bodies A body the formatter leaves unchanged would be re-printed from the same text, tokens and layout oracle, so the re-print can only reproduce it. Changed bodies are still re-printed and refused when not idempotent. --- crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index c016eccf54..18d6e4263d 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -92,10 +92,11 @@ fn try_format_sql(request: FormatRequest) -> Result { wrap: wrap.as_ref(), }; let (formatted, formatted_tokens, oracle) = format_layout(&original, &layout, &parse_options)?; - if relayout((&formatted, &formatted_tokens), oracle.as_ref(), &layout)? != formatted { + let changed = formatted != original; + if changed && relayout((&formatted, &formatted_tokens), oracle.as_ref(), &layout)? != formatted + { return Err(NOT_IDEMPOTENT_FAILURE.to_string()); } - let changed = formatted != original; Ok(FormatResponse { version: LINT_API_VERSION, changed, From 9284e1131c4824f9227637a52dd5528b51e02e4a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:10:54 +0100 Subject: [PATCH 03/10] perf(format): restore protected SQL fragments from sentinel tokens only Restoring fragments after the layout parse indexed every token of the generated SQL by its lower-case text. Only tokens that start with the sentinel prefix can match a fragment, so only those are indexed. --- .../src/sql_lint/_helpers/formatter_syntax.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs index 2b015c66e2..bf77d4c8d2 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs @@ -8,6 +8,8 @@ use std::collections::{HashMap, HashSet}; pub(super) struct ProtectedSql { pub sql: String, + /// The lower-case prefix every sentinel name starts with. + prefix: String, fragments: Vec<(String, String)>, in_functions: HashSet, } @@ -19,6 +21,14 @@ impl ProtectedSql { .map_err(|error| error.to_string())?; let mut positions: HashMap> = HashMap::new(); for (index, token) in tokens.iter().enumerate() { + let prefixed = token + .text + .as_bytes() + .get(..self.prefix.len()) + .is_some_and(|start| start.eq_ignore_ascii_case(self.prefix.as_bytes())); + if !prefixed { + continue; + } positions .entry(token.text.to_ascii_lowercase()) .or_default() @@ -222,6 +232,7 @@ pub(super) fn protect_syntax( prepared.extend(&chars[cursor..]); Ok(ProtectedSql { sql: prepared, + prefix, fragments, in_functions, }) From 7b0e08f3d5dbd72bea41bb55febd4ce8fd722a8b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:12:22 +0100 Subject: [PATCH 04/10] perf(format): compare alignment tokens without building key strings Aligning authored tokens with the layout guide built a formatted key string, with an upper-case copy for words, for every token. Lexemes now carry a small key kind and compare their raw text directly, ignoring ASCII case for words, which matches exactly what the strings compared. --- .../src/sql_lint/_helpers/token_layout.rs | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs index 80d196c96e..4eab341da4 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs @@ -39,7 +39,7 @@ struct Lexeme { start: usize, end: usize, raw: String, - key: String, + key: LexemeKey, /// In the reserved-keyword and built-in fold set shared with the change fingerprint. foldable: bool, /// An unquoted authored word outside the name positions that keep reserved words exact. @@ -48,6 +48,25 @@ struct Lexeme { keyword: bool, } +/// How lexemes compare: words ignoring ASCII case, punctuation by type, the rest by raw text. +#[derive(Clone, Copy, PartialEq, Eq)] +enum LexemeKey { + Word, + Punctuation(TokenType), + Raw, +} + +impl Lexeme { + fn same_key(&self, other: &Lexeme) -> bool { + self.key == other.key + && match self.key { + LexemeKey::Word => self.raw.eq_ignore_ascii_case(&other.raw), + LexemeKey::Punctuation(_) => true, + LexemeKey::Raw => self.raw == other.raw, + } + } +} + /// An authored token the printer recased to the layout guide's keyword spelling. pub(crate) struct KeywordRecase { pub(crate) generated: String, @@ -339,17 +358,18 @@ fn lexemes(sql: &str, tokens: &[Token], dialect: Option<&Dialect>) -> Result Lexeme { - let key = if is_unquoted_word(&raw) { - format!("w:{}", raw.to_ascii_uppercase()) + let word = is_unquoted_word(&raw); + let key = if word { + LexemeKey::Word } else if raw .chars() .all(|character| character.is_ascii_punctuation()) { - format!("p:{:?}", token.token_type) + LexemeKey::Punctuation(token.token_type) } else { - format!("r:{raw}") + LexemeKey::Raw }; - let keyword = is_unquoted_word(&raw) + let keyword = word && !matches!( token.token_type, TokenType::Var | TokenType::Identifier | TokenType::QuotedIdentifier @@ -433,7 +453,7 @@ struct Alignment<'a> { impl Alignment<'_> { fn matches(&self, left: usize, right: usize) -> bool { - if self.authored[left].key != self.generated[right].key { + if !self.authored[left].same_key(&self.generated[right]) { return false; } if !is_closer(&self.authored[left].raw) { @@ -468,7 +488,7 @@ impl Alignment<'_> { continue; } let anchor = &self.authored[left + skip_left]; - let confirmations = if anchor.key.starts_with("p:") { + let confirmations = if matches!(anchor.key, LexemeKey::Punctuation(_)) { RESYNC_CONFIRMATION } else { 0 @@ -478,7 +498,7 @@ impl Alignment<'_> { self.authored.get(left + skip_left + offset), self.generated.get(right + skip_right + offset), ) { - (Some(before), Some(after)) => before.key == after.key, + (Some(before), Some(after)) => before.same_key(after), _ => true, } }); @@ -569,7 +589,7 @@ fn separators( separators[next] = if substitution { let oracle_gap = (previous_at + 1..next_at) .rev() - .find(|&at| generated[at].key == authored[last].key) + .find(|&at| generated[at].same_key(&authored[last])) .map_or_else(|| gap(next_at - 1, next_at), |at| gap(at, at + 1)); let unspaced = default_space(&authored[last], &authored[next]).is_empty(); if unspaced && !oracle_gap.contains('\n') { From e175203531b468ba0e5e6ed04c997612e34d939d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:13:44 +0100 Subject: [PATCH 05/10] perf(format): reuse the printer's token texts in the token check The token check read the authored and printed token texts twice each, once for the case-folded comparison and once for keyword recases. The printer already holds the authored texts and fold flags, so the check reuses them, compares them with the printed tokens without upper-case copies of the authored side, and reads the printed texts only when a keyword recase must be confirmed. --- .../src/sql_lint/_helpers/formatter.rs | 18 ++--- .../src/sql_lint/_helpers/token_layout.rs | 70 +++++++++++++++---- .../sql_lint/tests/test_authored_tokens.rs | 10 ++- 3 files changed, 72 insertions(+), 26 deletions(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index 18d6e4263d..390b5bc5c3 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -8,8 +8,8 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator}; use crate::sql_lint::_helpers::formatter_syntax::{cte_macro_indices, protect_syntax}; use crate::sql_lint::_helpers::printed_layout::{PrintedSql, WrapOptions, wrap_lines}; use crate::sql_lint::_helpers::token_layout::{ - AuthoredSql, KeywordRecase, OracleLayout, comments_in, neutralize_comments, oracle_layout, - print_authored_tokens, tokenize, verify_token_invariant, + AuthoredSql, KeywordRecase, OracleLayout, TokenTexts, comments_in, neutralize_comments, + oracle_layout, print_authored_tokens, tokenize, verify_printed_tokens, }; use crate::sql_lint::constants::{ CAST_TYPE_SEPARATOR_KEYWORD, CLOSE_PARENTHESIS, LINT_API_VERSION, OPEN_PARENTHESIS, @@ -138,10 +138,10 @@ fn format_layout( tokens: &tokens, comments: &comments, }; - let (printed, recases) = layout.print(&authored, &oracle)?; + let (printed, recases, authored_texts) = layout.print(&authored, &oracle)?; let printed_tokens = tokenize(&printed, dialect)?; - verify_token_invariant( - (sql, &tokens), + verify_printed_tokens( + (sql, &tokens, &authored_texts), (&printed, &printed_tokens), dialect, &recases, @@ -178,19 +178,19 @@ impl Layout<'_> { &self, authored: &AuthoredSql<'_>, oracle: &OracleLayout, - ) -> Result<(String, BTreeMap), String> { - let (printed, recases) = print_authored_tokens(authored, oracle, self.dialect)?; + ) -> Result<(String, BTreeMap, TokenTexts), String> { + let (printed, recases, texts) = print_authored_tokens(authored, oracle, self.dialect)?; let no_widths: HashMap = HashMap::new(); let token_widths = self.wrap.map_or(&no_widths, |wrap| wrap.token_widths); let Some(mut sql) = PrintedSql::parse(&printed, self.dialect, token_widths)? else { - return Ok((printed, recases)); + return Ok((printed, recases, texts)); }; sql.arrange_clauses(self.wrap.map_or(usize::MAX, |wrap| wrap.line_width)); if let Some(wrap) = self.wrap { wrap_lines(&mut sql, wrap.line_width); } sql.lead_operators_of_multiline_operands(); - Ok((sql.render(), recases)) + Ok((sql.render(), recases, texts)) } } diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs index 4eab341da4..986882eb29 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs @@ -67,6 +67,35 @@ impl Lexeme { } } +/// The source text and case-fold flag of every token, read once for the token check. +pub(crate) struct TokenTexts { + texts: Vec, + foldable: Vec, +} + +impl TokenTexts { + #[cfg(test)] + pub(crate) fn read(sql: &str, tokens: &[Token], dialect: &Dialect) -> Result { + let texts = token_texts(sql, tokens)?; + let foldable = foldable_tokens(&texts, tokens, dialect); + Ok(Self { texts, foldable }) + } + + /// Whether token `index` here, upper-cased when foldable, spells `canonical`. + fn same_canonical(&self, index: usize, canonical: &str) -> bool { + let (text, fold) = (&self.texts[index], self.foldable[index]); + text.len() == canonical.len() + && text.bytes().zip(canonical.bytes()).all(|(byte, expected)| { + expected + == if fold { + byte.to_ascii_uppercase() + } else { + byte + } + }) + } +} + /// An authored token the printer recased to the layout guide's keyword spelling. pub(crate) struct KeywordRecase { pub(crate) generated: String, @@ -198,7 +227,7 @@ pub(crate) fn print_authored_tokens( source: &AuthoredSql<'_>, oracle: &OracleLayout, dialect: &Dialect, -) -> Result<(String, BTreeMap), String> { +) -> Result<(String, BTreeMap, TokenTexts), String> { let AuthoredSql { sql, tokens, @@ -275,29 +304,42 @@ pub(crate) fn print_authored_tokens( } } } - Ok((output, recases)) + let mut texts = TokenTexts { + texts: Vec::with_capacity(authored.len()), + foldable: Vec::with_capacity(authored.len()), + }; + for lexeme in authored { + texts.texts.push(lexeme.raw); + texts.foldable.push(lexeme.foldable); + } + Ok((output, recases, texts)) } /// Refuse output that changes anything but layout, fold-set case, and recorded keyword recases. -pub(crate) fn verify_token_invariant( - (before, before_tokens): (&str, &[Token]), +pub(crate) fn verify_printed_tokens( + (before, before_tokens, before_texts): (&str, &[Token], &TokenTexts), (after, after_tokens): (&str, &[Token]), dialect: &Dialect, recases: &BTreeMap, ) -> Result<(), String> { - let before_canonical = canonical_tokens(before, before_tokens, dialect)?; let after_canonical = canonical_tokens(after, after_tokens, dialect)?; - if before_canonical.len() != after_canonical.len() { + if before_texts.texts.len() != after_canonical.len() { return Err(TOKEN_PRESERVATION_FAILURE.to_string()); } - let before_texts = token_texts(before, before_tokens)?; - let after_texts = token_texts(after, after_tokens)?; - for (index, (left, right)) in before_canonical.iter().zip(&after_canonical).enumerate() { - if left != right - && !recases - .get(&index) - .is_some_and(|recase| recase.allows(&before_texts[index], &after_texts[index])) - { + let mut after_texts: Option> = None; + for (index, canonical) in after_canonical.iter().enumerate() { + if before_texts.same_canonical(index, &canonical.text) { + continue; + } + let Some(recase) = recases.get(&index) else { + return Err(TOKEN_PRESERVATION_FAILURE.to_string()); + }; + let texts: &Vec = match after_texts { + Some(ref texts) => texts, + None => after_texts.insert(token_texts(after, after_tokens)?), + }; + let printed: &str = &texts[index]; + if !recase.allows(&before_texts.texts[index], printed) { return Err(TOKEN_PRESERVATION_FAILURE.to_string()); } } diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs index af36b480a1..cb29e9c288 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs @@ -2,7 +2,9 @@ use serde_json::{Value, json}; use std::collections::BTreeMap; -use crate::sql_lint::_helpers::token_layout::{KeywordRecase, tokenize, verify_token_invariant}; +use crate::sql_lint::_helpers::token_layout::{ + KeywordRecase, TokenTexts, tokenize, verify_printed_tokens, +}; use crate::sql_lint::main::formatter::format_json; use crate::sql_lint::tests::test_types; @@ -396,8 +398,10 @@ fn given_formatted_candidates_when_checking_token_invariant_then_only_layout_may .collect(); let before_tokens = tokenize(test_case.before, &dialect).expect("before tokenizes"); let after_tokens = tokenize(test_case.after, &dialect).expect("after tokenizes"); - let actual = verify_token_invariant( - (test_case.before, &before_tokens), + let before_texts = + TokenTexts::read(test_case.before, &before_tokens, &dialect).expect("before reads"); + let actual = verify_printed_tokens( + (test_case.before, &before_tokens, &before_texts), (test_case.after, &after_tokens), &dialect, &recases, From 7a0b9ae7940070f44d1bc8b49f7b34df1dc53733 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:14:31 +0100 Subject: [PATCH 06/10] perf(format): skip the CTE macro pass when the layout has no sentinels Laying out CTE macros tokenized the restored layout guide again for every body. A CTE macro token starts with the lint sentinel prefix, so the pass now returns at once when the text does not contain it. --- crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs | 4 ++++ .../sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index 390b5bc5c3..938f5a5c04 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -5,6 +5,7 @@ use polyglot_sql::tokens::{Token, TokenType}; use polyglot_sql::{ComplexityGuardOptions, Dialect, DialectType, ParseOptions}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; +use crate::lint_text::constants::LINT_SENTINEL_PREFIX; use crate::sql_lint::_helpers::formatter_syntax::{cte_macro_indices, protect_syntax}; use crate::sql_lint::_helpers::printed_layout::{PrintedSql, WrapOptions, wrap_lines}; use crate::sql_lint::_helpers::token_layout::{ @@ -223,6 +224,9 @@ fn layout_oracle( } fn layout_cte_macros(mut formatted: String, dialect: &Dialect) -> Result { + if !formatted.contains(LINT_SENTINEL_PREFIX) { + return Ok(formatted); + } let tokens = dialect .tokenize(&formatted) .map_err(|error| error.to_string())?; diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs index bf77d4c8d2..e59eb02800 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter_syntax.rs @@ -1,3 +1,4 @@ +use crate::lint_text::constants::LINT_SENTINEL_PREFIX; use crate::sql_lint::_helpers::engine::token_depths; use crate::sql_lint::constants::{ CTE_MACRO_PLACEHOLDER_LITERAL, POSITION_ARGUMENT_SEPARATOR, POSTFIX_CAST_PREFIX_TOKEN_COUNT, @@ -281,7 +282,7 @@ pub(super) fn cte_macro_indices(tokens: &[Token]) -> HashSet { scopes.insert(depths[index]); } else if token.token_type == TokenType::Select { scopes.remove(&depths[index]); - } else if token.text.starts_with("__sqb_lint_") + } else if token.text.starts_with(LINT_SENTINEL_PREFIX) && scopes.contains(&depths[index]) && index > 0 && matches!( From 69cedd1604e13922183cc9e3fc92d2c876bf6b84 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:16:52 +0100 Subject: [PATCH 07/10] perf(format): reuse header spans when relocation leaves a file unchanged Formatting headers scanned the file for header spans again after moving leading comments. When trimming and relocation changed nothing, the spans scanned from the original text are reused. --- .../sqlbuild-rules/src/dsl_headers/main/format_headers.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs b/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs index 978bad4747..c4277c4252 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs @@ -16,7 +16,11 @@ pub fn format_headers( let headers: Vec = scan_headers(text, false); let trimmed: Vec = without_trailing_whitespace(text, &headers); let (relocated, faults) = relocate_leading_comment(&trimmed, config, &headers, parser)?; - let relocated_headers: Vec = scan_headers(&relocated, false); + let relocated_headers: Vec = if relocated.as_slice() == text { + headers + } else { + scan_headers(&relocated, false) + }; let wrapped: Vec = wrap_descriptions(&relocated, &relocated_headers, config.line_width, parser)?; Ok((wrap_audits(&wrapped, config.line_width, parser)?, faults)) From b6764ff4202ac54a6999ffec777c294f20af79f7 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:23:04 +0100 Subject: [PATCH 08/10] test(format): count idempotency re-prints of formatted and changed bodies --- .../src/sql_lint/_helpers/formatter.rs | 8 ++++ crates/sqlbuild-rules/src/sql_lint/tests.rs | 2 + .../tests/test_idempotency_reprint.rs | 48 +++++++++++++++++++ .../src/sql_lint/tests/test_types.rs | 7 +++ 4 files changed, 65 insertions(+) create mode 100644 crates/sqlbuild-rules/src/sql_lint/tests/test_idempotency_reprint.rs diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index 938f5a5c04..2b7cd18ffc 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -17,6 +17,12 @@ use crate::sql_lint::constants::{ }; use crate::sql_lint::models::{FormatRequest, FormatResponse}; +#[cfg(test)] +thread_local! { + /// Idempotency re-prints on this thread, for tests that assert which bodies are re-printed. + pub(crate) static RELAYOUTS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + const NOT_IDEMPOTENT_FAILURE: &str = "native formatter output is not idempotent"; const UNSUPPORTED_SQL_FAILURE: &str = "native formatter could not safely restore literal or cast spelling"; @@ -156,6 +162,8 @@ fn relayout( oracle: Option<&OracleLayout>, layout: &Layout<'_>, ) -> Result { + #[cfg(test)] + RELAYOUTS.with(|relayouts| relayouts.set(relayouts.get() + 1)); let Some(oracle) = oracle.filter(|_| !tokens.is_empty()) else { return Ok(formatted.to_string()); }; diff --git a/crates/sqlbuild-rules/src/sql_lint/tests.rs b/crates/sqlbuild-rules/src/sql_lint/tests.rs index 5e5d936598..d1ff27056d 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests.rs @@ -14,6 +14,8 @@ mod format_corpus; mod function_call_spacing; #[path = "tests/helpers.rs"] mod helpers; +#[path = "tests/test_idempotency_reprint.rs"] +mod idempotency_reprint; #[path = "tests/test_layout_rules.rs"] mod layout_rules; #[path = "tests/test_line_wrap.rs"] diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_idempotency_reprint.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_idempotency_reprint.rs new file mode 100644 index 0000000000..edf70b5826 --- /dev/null +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_idempotency_reprint.rs @@ -0,0 +1,48 @@ +use serde_json::{Value, json}; + +use crate::sql_lint::_helpers::formatter::RELAYOUTS; +use crate::sql_lint::main::formatter::format_json; +use crate::sql_lint::tests::test_types; + +#[test] +fn given_formatted_and_unformatted_bodies_when_formatting_then_only_changed_bodies_are_reprinted() +-> Result<(), String> { + let test_cases = [ + test_types::ReprintTestCase { + description: "an already formatted body is not re-printed", + sql: "SELECT\n order_id,\n amount\nFROM orders\nWHERE amount > 0", + expected_changed: false, + expected_reprints: 0, + }, + test_types::ReprintTestCase { + description: "a body the formatter changes is re-printed once", + sql: "select order_id, amount from orders where amount > 0", + expected_changed: true, + expected_reprints: 1, + }, + ]; + for test_case in &test_cases { + RELAYOUTS.with(|relayouts| relayouts.set(0)); + let response = format_json( + &json!({"version": 1, "sql": test_case.sql, "dialect": "duckdb"}).to_string(), + )?; + let payload: Value = serde_json::from_str(&response).map_err(|error| error.to_string())?; + assert_eq!( + payload["formatted"], true, + "{}: {payload}", + test_case.description + ); + assert_eq!( + payload["changed"], test_case.expected_changed, + "{}: {payload}", + test_case.description + ); + assert_eq!( + RELAYOUTS.with(std::cell::Cell::get), + test_case.expected_reprints, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_types.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_types.rs index d036ebdecc..30f7edda4a 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_types.rs @@ -132,6 +132,13 @@ pub(crate) struct FormatTestCase { pub expected_changed: bool, } +pub(crate) struct ReprintTestCase { + pub description: &'static str, + pub sql: &'static str, + pub expected_changed: bool, + pub expected_reprints: usize, +} + pub(crate) struct InvalidRuleTestCase { pub description: &'static str, pub rule: &'static str, From 5754c5764974730cff6f646b247a0dcb77a10472 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:24:15 +0100 Subject: [PATCH 09/10] perf(format): skip the token check when printing leaves a body unchanged Printed text identical to the authored body tokenizes to the authored tokens, so the token check compares the body with itself and can only pass. The check and the printed-text tokenize now run only for bodies the printer changed. --- crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index 2b7cd18ffc..9e0072c389 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -146,6 +146,9 @@ fn format_layout( comments: &comments, }; let (printed, recases, authored_texts) = layout.print(&authored, &oracle)?; + if printed == sql { + return Ok((printed, tokens, Some(oracle))); + } let printed_tokens = tokenize(&printed, dialect)?; verify_printed_tokens( (sql, &tokens, &authored_texts), From f425596aa60f7981f9dbd81e8246f2818220a977 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 21 Oct 2026 03:44:46 +0100 Subject: [PATCH 10/10] fix(format): stop adding a newline after comments that end a SQL body A line comment after the last token was printed with a newline after it. Format runs trim a body's trailing whitespace and keep it apart, so each run added that newline again and the file never settled. The printer now ends the output at the comment, as the authored body did. --- .../src/sql_lint/_helpers/token_layout.rs | 2 +- .../sql_lint/tests/test_comment_formatter.rs | 42 +++++++++++++++++++ .../src/sql_lint/tests/test_sql_lint.rs | 2 +- .../sqlbuild/cli/commands/main/_test_types.py | 9 ++++ .../sqlbuild/cli/commands/main/test_format.py | 39 +++++++++++++++++ 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs index 986882eb29..82971be7fa 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs @@ -294,7 +294,7 @@ pub(crate) fn print_authored_tokens( } else { separators .get(index + 1) - .is_some_and(|next| next.starts_with('\n')) + .is_none_or(|next| next.starts_with('\n')) }; if !next_starts_line { output.push('\n'); diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_comment_formatter.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_comment_formatter.rs index 5519b0a18f..adc81ce1ed 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests/test_comment_formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_comment_formatter.rs @@ -51,3 +51,45 @@ fn given_leading_cte_comments_when_formatting_then_comments_stay_above_their_cte } Ok(()) } + +#[test] +fn given_trailing_comments_after_the_last_token_when_formatting_then_no_newline_is_added() +-> Result<(), String> { + let test_cases = [ + test_types::FormatTestCase { + description: "a trailing line comment ends the output without a newline", + sql: "SELECT 1 AS order_id\nFROM orders -- latest orders", + expected_sql: "SELECT 1 AS order_id\nFROM orders -- latest orders", + expected_changed: false, + }, + test_types::FormatTestCase { + description: "line comments after the last token keep no trailing newline", + sql: "select 1 as order_id from orders\n\n-- first note\n\n-- second note", + expected_sql: "SELECT 1 AS order_id\nFROM orders -- first note\n -- second note", + expected_changed: true, + }, + test_types::FormatTestCase { + description: "a trailing block comment ends the output", + sql: "SELECT 1 AS order_id\nFROM orders /* latest orders */", + expected_sql: "SELECT 1 AS order_id\nFROM orders /* latest orders */", + expected_changed: false, + }, + ]; + for test_case in &test_cases { + let response = format_json( + &json!({"version": 1, "sql": test_case.sql, "dialect": "duckdb"}).to_string(), + )?; + let payload: Value = serde_json::from_str(&response).map_err(|error| error.to_string())?; + assert_eq!( + payload["sql"], test_case.expected_sql, + "{}", + test_case.description + ); + assert_eq!( + payload["changed"], test_case.expected_changed, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_sql_lint.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_sql_lint.rs index 2c8bcbbde8..ab2d111445 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests/test_sql_lint.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_sql_lint.rs @@ -1890,7 +1890,7 @@ fn given_format_cases_when_formatting_then_output_matches() -> Result<(), String test_types::FormatTestCase { description: "trailing comment after a kept statement terminator", sql: "select a from items; -- retained", - expected_sql: "SELECT a\nFROM items; -- retained\n", + expected_sql: "SELECT a\nFROM items; -- retained", expected_changed: true, }, test_types::FormatTestCase { diff --git a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py index 2499250938..1a30877064 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -315,6 +315,15 @@ class FormatCompileIntegrationTestCase: expected_literal: str +@dataclass(frozen=True) +class TrailingCommentFormatIntegrationTestCase: + """A model body ending in comments that formatting must settle on in one run.""" + + description: str + body: str + expected_body: str + + @dataclass(frozen=True) class AuthoredSpellingFormatIntegrationTestCase: """One authored function or operator spelling that formatting must keep.""" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py index 398aa37bb7..f56f66e489 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py @@ -26,6 +26,7 @@ LeadingCteCommentFormatIntegrationTestCase, LineWidthWrapIntegrationTestCase, MixedFromValuesFormatIntegrationTestCase, + TrailingCommentFormatIntegrationTestCase, TypedNullFormatIntegrationTestCase, ) from tests.integration.src.sqlbuild.cli.commands.main.helpers import ( @@ -436,6 +437,44 @@ def test_given_description_width_when_formatting_then_wrapped_model_compiles( ) +@pytest.mark.parametrize( + "test_case", + [ + TrailingCommentFormatIntegrationTestCase( + description="a trailing line comment on the last line stays settled", + body="SELECT 1 AS order_id\nFROM orders -- latest orders\n", + expected_body="SELECT 1 AS order_id\nFROM orders -- latest orders\n", + ), + TrailingCommentFormatIntegrationTestCase( + description="comments after the last clause gain no blank line", + body="select 1 as order_id from orders\n\n-- first note\n\n-- second note\n", + expected_body="SELECT 1 AS order_id\nFROM orders -- first note\n -- second note\n", + ), + ], + ids=lambda case: case.description, +) +def test_given_trailing_comments_when_formatting_twice_then_second_run_changes_nothing( + test_case: TrailingCommentFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + header: str = 'MODEL (description "Orders placed by customers.");\n\n' + model: Path = tmp_path / "models" / "orders.sql" + model.parent.mkdir() + model.write_text(header + test_case.body, encoding="utf-8") + + first_exit: int = main(["--project-dir", str(tmp_path), "format", "--no-cache"]) + formatted_once: str = model.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(tmp_path), "format", "--no-cache"]) + check_exit: int = main(["--project-dir", str(tmp_path), "format", "--check", "--no-cache"]) + + assert (first_exit, second_exit, check_exit) == (0, 0, 0) + assert formatted_once == header + test_case.expected_body + assert model.read_text(encoding="utf-8") == formatted_once + + @pytest.mark.parametrize( "test_case", [