diff --git a/Makefile b/Makefile index d718172570..0dc9b83fe5 100644 --- a/Makefile +++ b/Makefile @@ -164,6 +164,7 @@ E2E_DUCKDB_CLI_PATHS := \ tests/e2e/src/sqlbuild/cli/commands/main/databricks \ tests/e2e/src/sqlbuild/cli/commands/main/dbt \ tests/e2e/src/sqlbuild/cli/commands/main/debug \ + tests/e2e/src/sqlbuild/cli/commands/main/format/test_description_layout.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_cache.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_example_projects.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_rule_fixes.py \ diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs index 90a597e4ba..46bfffd2b0 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs @@ -10,7 +10,6 @@ use crate::sql_macros::_helpers::parameters::{ with_placeholders, }; use crate::sql_macros::constants::{ARGUMENT_CODE, INVALID_DEFINITION_CODE}; -use crate::sql_macros::errors::LexError; use crate::sql_macros::errors::SqlMacroError; use crate::sql_macros::models::{ ArgumentShape, BodyExpansion, ParameterKind, RenderedArgument, SqlMacroDefinition, @@ -335,6 +334,6 @@ fn without_leading_keyword(text: &str) -> String { Some(first) => text[first.end..].trim().to_owned(), None => text.trim().to_owned(), }, - Err(LexError::UnclosedQuote(_) | LexError::UnclosedComment(_)) => text.trim().to_owned(), + Err(_) => text.trim().to_owned(), } } diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs index 513b9e32d6..e644af2802 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs @@ -10,6 +10,12 @@ use crate::sql_macros::constants::{ use crate::sql_macros::errors::LexError; use crate::sql_macros::errors::SqlMacroError; use crate::sql_macros::models::{ParameterKind, SqlMacroLocation, SqlMacroParameter}; +use sqlbuild_sqltext::header_strings::constants::{ + DOUBLE_QUOTE_INSIDE_HELP, DOUBLE_QUOTE_INSIDE_MESSAGE, SINGLE_QUOTE_INSIDE_HELP, + SINGLE_QUOTE_INSIDE_MESSAGE, TRIPLE_QUOTE_INSIDE_HELP, TRIPLE_QUOTE_INSIDE_MESSAGE, + UNCLOSED_TRIPLE_QUOTE_HELP, UNCLOSED_TRIPLE_QUOTE_MESSAGE, +}; +use sqlbuild_sqltext::header_strings::main::prose_string::prose_string; /// What the header declares and where the body starts. pub(crate) struct Header { @@ -42,6 +48,18 @@ impl HeaderParser<'_> { let scanned: Vec = tokens(source).map_err(|error| match error { LexError::UnclosedQuote(at) => self.error(at, "unclosed quote in SQL macro file"), LexError::UnclosedComment(at) => self.error(at, "unclosed comment in SQL macro file"), + LexError::QuoteInside(at) => self + .error(at, DOUBLE_QUOTE_INSIDE_MESSAGE) + .helped(DOUBLE_QUOTE_INSIDE_HELP), + LexError::SingleQuoteInside(at) => self + .error(at, SINGLE_QUOTE_INSIDE_MESSAGE) + .helped(SINGLE_QUOTE_INSIDE_HELP), + LexError::TripleQuoteInside(at) => self + .error(at, TRIPLE_QUOTE_INSIDE_MESSAGE) + .helped(TRIPLE_QUOTE_INSIDE_HELP), + LexError::UnclosedTripleQuote(at) => self + .error(at, UNCLOSED_TRIPLE_QUOTE_MESSAGE) + .helped(UNCLOSED_TRIPLE_QUOTE_HELP), })?; let starts_with_header: bool = scanned .first() @@ -89,7 +107,7 @@ impl HeaderParser<'_> { match key_text.as_str() { DESCRIPTION_KEY => match value { [text] if matches!(text.kind, TokenKind::Quoted | TokenKind::Text) => { - header.description = Some(unquoted(text.text(source))); + header.description = Some(self.description(text)?); Ok(header) } _ => Err(self.error(key.start, "description must be one quoted string")), @@ -139,6 +157,14 @@ impl HeaderParser<'_> { } } + /// The folded prose of the description string `text`, which lexing found well quoted. + fn description(&self, text: &Token) -> Result { + let raw: Vec = text.text(self.source).chars().collect(); + prose_string(&raw, 0) + .map(|prose| prose.value) + .map_err(|_| self.error(text.start, "description must be one quoted string")) + } + fn parameter(&self, tokens: &[Token]) -> Result { let source: &str = self.source; let name: &Token = &tokens[0]; diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/lexing.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/lexing.rs index d17c54dddc..2fc6bdd706 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/lexing.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/lexing.rs @@ -1,8 +1,14 @@ //! A comment- and quote-aware SQL token scan, and the `@...` constructs it finds. use sqlbuild_render::macro_calls::main::opens_bare_call::opens_bare_call; +use sqlbuild_sqltext::header_strings::errors::{ProseError, TripleQuoteError}; +use sqlbuild_sqltext::header_strings::main::closes_inside_text::closes_inside_text; +use sqlbuild_sqltext::header_strings::main::follows_description_key::follows_description_key; +use sqlbuild_sqltext::header_strings::main::opens_triple_quote::opens_triple_quote; +use sqlbuild_sqltext::header_strings::main::prose_end::prose_end; +use sqlbuild_sqltext::header_strings::main::triple_quoted_end::triple_quoted_end; -use crate::sql_macros::constants::SQL_KIND_WORD; +use crate::sql_macros::constants::{HEADER_KEYWORD, SQL_KIND_WORD}; use crate::sql_macros::errors::LexError; /// What one token is. @@ -63,6 +69,7 @@ fn scanned_tokens(sql: &str, backslash_escapes: bool) -> Result, LexE let bytes: &[u8] = sql.as_bytes(); let mut found: Vec = Vec::new(); let mut index: usize = 0; + let mut header: HeaderScan = HeaderScan::Before; while index < bytes.len() { let byte: u8 = bytes[index]; if byte.is_ascii_whitespace() { @@ -85,7 +92,16 @@ fn scanned_tokens(sql: &str, backslash_escapes: bool) -> Result, LexE continue; } let start: usize = index; + let in_header: bool = matches!(header, HeaderScan::Inside(_)); let kind: TokenKind = match byte { + b'\'' if in_header => { + index = header_string_end(bytes, index)?; + TokenKind::Text + } + b'"' if in_header => { + index = header_string_end(bytes, index)?; + TokenKind::Quoted + } b'\'' => { index = quote_end(bytes, index, backslash_escapes) .ok_or(LexError::UnclosedQuote(start))?; @@ -130,15 +146,77 @@ fn scanned_tokens(sql: &str, backslash_escapes: bool) -> Result, LexE TokenKind::Operator } }; - found.push(Token { + let token: Token = Token { kind, start, end: index, - }); + }; + header = header.after(sql, &token, found.is_empty()); + found.push(token); } Ok(found) } +/// Where a scan is relative to the `MACRO (...)` header that may start the text. +#[derive(Clone, Copy)] +enum HeaderScan { + Before, + /// The first token is `MACRO`. + Keyword, + /// Inside the header, at this bracket depth. + Inside(usize), + Outside, +} + +impl HeaderScan { + fn after(self, sql: &str, token: &Token, first: bool) -> Self { + let opens: bool = + token.kind == TokenKind::Punct && matches!(token.text(sql), "(" | "[" | "{"); + let closes: bool = + token.kind == TokenKind::Punct && matches!(token.text(sql), ")" | "]" | "}"); + match self { + Self::Before if first && token.is_word(sql, HEADER_KEYWORD) => Self::Keyword, + Self::Keyword if token.is(sql, "(") => Self::Inside(1), + Self::Inside(depth) if opens => Self::Inside(depth + 1), + Self::Inside(1) if closes => Self::Outside, + Self::Inside(depth) if closes => Self::Inside(depth - 1), + Self::Inside(depth) => Self::Inside(depth), + _ => Self::Outside, + } + } +} + +/// The lexing error of a prose string that does not end where it should. +fn prose_lex_error(error: ProseError) -> LexError { + match error { + ProseError::Unterminated(start) => LexError::UnclosedQuote(start), + ProseError::DoubleQuoteInside(position) => LexError::QuoteInside(position), + ProseError::SingleQuoteInside(position) => LexError::SingleQuoteInside(position), + ProseError::TripleQuoteInside(position) => LexError::TripleQuoteInside(position), + ProseError::UnclosedTripleQuote(start) => LexError::UnclosedTripleQuote(start), + } +} + +/// The end of the header string at `start`: a description by prose rules, else as a SQL string. +fn header_string_end(bytes: &[u8], start: usize) -> Result { + if follows_description_key(bytes, start) { + return prose_end(bytes, start).map_err(prose_lex_error); + } + if opens_triple_quote(bytes, start) { + let (content_end, end) = triple_quoted_end(bytes, start).map_err(|error| match error { + TripleQuoteError::Unclosed => LexError::UnclosedTripleQuote(start), + TripleQuoteError::Inside(position) => LexError::TripleQuoteInside(position), + })?; + if closes_inside_text(bytes, end) { + return Err(LexError::TripleQuoteInside(content_end)); + } + return Ok(end); + } + quoted_end(bytes, start) + .or_else(|| quote_end(bytes, start, true)) + .ok_or(LexError::UnclosedQuote(start)) +} + fn is_operator_byte(byte: u8) -> bool { matches!( byte, diff --git a/crates/sqlbuild-analysis/src/sql_macros/errors.rs b/crates/sqlbuild-analysis/src/sql_macros/errors.rs index 7825772743..9317468ea7 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/errors.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/errors.rs @@ -88,4 +88,12 @@ impl From for ExpansionFailure { pub(crate) enum LexError { UnclosedQuote(usize), UnclosedComment(usize), + /// A `"` that ends a `"..."` header string inside its text, at this offset. + QuoteInside(usize), + /// A `'` that ends a `'...'` header description inside its text, at this offset. + SingleQuoteInside(usize), + /// A `"""` that ends a `"""..."""` header string inside its text, at this offset. + TripleQuoteInside(usize), + /// A `"""` header string that opens at this offset and never closes. + UnclosedTripleQuote(usize), } diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs index 29c6fa4329..c63694a6a4 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs @@ -2,9 +2,12 @@ use crate::sql_macros::main::parse_sql_macro::parse_sql_macro; use crate::sql_macros::main::sql_macro_cycle::sql_macro_cycle; use crate::sql_macros::tests::helpers::owned; use crate::sql_macros::tests::test_types::{ - CycleTestCase, DefinitionErrorTestCase, DefinitionTestCase, + CycleTestCase, DefinitionErrorTestCase, DefinitionTestCase, MacroDescriptionTestCase, + MacroQuoteErrorTestCase, }; +const QUOTE_MESSAGE: &str = "a double quote ends this \"...\" string early; use \"\"\"…\"\"\" for text containing double quotes"; + #[test] fn given_valid_macro_files_when_parsing_then_each_parameter_and_the_body_are_read() { let test_cases = [ @@ -285,6 +288,125 @@ fn given_invalid_macro_files_when_parsing_then_each_is_rejected() { } } +#[test] +fn given_macro_descriptions_when_parsing_then_the_folded_text_is_stored() { + let test_cases = [ + MacroDescriptionTestCase { + description: "a triple-quoted description on one line keeps its quotes", + source: "MACRO (description \"\"\"Matches \"Miss\", it's.\"\"\", returns VARCHAR);\n'x'\n", + expected_description: "Matches \"Miss\", it's.", + }, + MacroDescriptionTestCase { + description: "a triple-quoted block across lines is folded", + source: "MACRO (\n description \"\"\"\n True for \"Mrs. C. O'Brien\",\n any case.\n \"\"\",\n returns VARCHAR,\n);\n'x'\n", + expected_description: "True for \"Mrs. C. O'Brien\", any case.", + }, + MacroDescriptionTestCase { + description: "a backslash escapes a quote in a single-quoted description", + source: "MACRO (\n description 'it\\'s here',\n arguments (v VARCHAR),\n returns VARCHAR,\n);\n\nLOWER(@v)\n", + expected_description: "it's here", + }, + MacroDescriptionTestCase { + description: "a doubled quote is one quote in a single-quoted description", + source: "MACRO (\n description 'it''s',\n returns VARCHAR,\n);\n'x'\n", + expected_description: "it's", + }, + MacroDescriptionTestCase { + description: "a double-quoted description may end in a backslash", + source: "MACRO (\n description \"ends in C:\\\",\n returns VARCHAR,\n);\n'x'\n", + expected_description: "ends in C:\\", + }, + MacroDescriptionTestCase { + description: "a wrapped description stores the same as one line", + source: "MACRO (\n description \"True for a woman's title,\nany case.\",\n returns VARCHAR,\n);\n'it''s'\n", + expected_description: "True for a woman's title, any case.", + }, + ]; + + for test_case in test_cases { + let definition = parse_sql_macro("m", "macros/sql/m.sql", test_case.source) + .expect(test_case.description); + + assert_eq!( + definition.description.as_deref(), + Some(test_case.expected_description), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_misquoted_macro_descriptions_when_parsing_then_the_quote_is_located() { + let test_cases = [ + MacroQuoteErrorTestCase { + description: "an escaped quote on one line fails at the quote", + source: "MACRO (\n description \"Matches a title (\\\"Miss\\\").\",\n arguments (value VARCHAR),\n returns VARCHAR,\n);\n@value\n", + expected_message: QUOTE_MESSAGE, + expected_location: (2, 34), + }, + MacroQuoteErrorTestCase { + description: "apostrophes elsewhere no longer let an escaped quote through", + source: "MACRO (\n description \"True when a name starts with a woman's title. Any case,\nwith or without a dot (\\\"MISS ANNA PARKER\\\", \\\"Mrs. C. O'Brien\\\").\",\n arguments (value VARCHAR),\n returns VARCHAR,\n);\n@value\n", + expected_message: QUOTE_MESSAGE, + expected_location: (3, 25), + }, + MacroQuoteErrorTestCase { + description: "a bare quote that ends the string early fails where it is", + source: "MACRO (description \"Matches \"Miss\".\", returns VARCHAR);\n'x'\n", + expected_message: QUOTE_MESSAGE, + expected_location: (1, 29), + }, + MacroQuoteErrorTestCase { + description: "doubled double quotes fail at the second quote", + source: "MACRO (description \"say \"\"hi\"\" now\", returns VARCHAR);\n'x'\n", + expected_message: QUOTE_MESSAGE, + expected_location: (1, 25), + }, + MacroQuoteErrorTestCase { + description: "a lone quote inside a single-quoted description", + source: "MACRO (description 'it's here', returns VARCHAR);\n'x'\n", + expected_message: "a single quote ends this '...' string early", + expected_location: (1, 23), + }, + MacroQuoteErrorTestCase { + description: "an unclosed triple quote reports where it started", + source: "MACRO (\n description \"\"\"never closed,\n returns VARCHAR,\n);\n'x'\n", + expected_message: "this \"\"\" string is never closed", + expected_location: (2, 15), + }, + MacroQuoteErrorTestCase { + description: "a triple quote inside a triple-quoted string", + source: "MACRO (description \"\"\"a \"\"\"b\"\"\" c\"\"\", returns VARCHAR);\n'x'\n", + expected_message: "\"\"\" inside a \"\"\"...\"\"\" string ends it early", + expected_location: (1, 25), + }, + ]; + + for test_case in test_cases { + let error = parse_sql_macro("m", "macros/sql/m.sql", test_case.source) + .expect_err(test_case.description); + let location = error + .locations + .first() + .map(|location| (location.line, location.column)); + + assert_eq!(error.code, "P015", "{}", test_case.description); + assert!( + error.message.starts_with(test_case.expected_message), + "{}: {}", + test_case.description, + error.message + ); + assert_eq!( + location, + Some(test_case.expected_location), + "{}", + test_case.description + ); + } +} + #[test] fn given_macros_calling_each_other_when_checking_then_the_cycle_is_named() { let test_cases = [ diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs index 47970c78e9..5058bb8c6e 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs @@ -83,3 +83,16 @@ pub(super) struct OutputNamesTestCase { pub(super) sql: &'static str, pub(super) expected_names: &'static [(Option<&'static str>, usize, &'static str)], } + +pub(super) struct MacroDescriptionTestCase { + pub(super) description: &'static str, + pub(super) source: &'static str, + pub(super) expected_description: &'static str, +} + +pub(super) struct MacroQuoteErrorTestCase { + pub(super) description: &'static str, + pub(super) source: &'static str, + pub(super) expected_message: &'static str, + pub(super) expected_location: (usize, usize), +} diff --git a/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs b/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs index 608f499c4e..d42b7281e8 100644 --- a/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs +++ b/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs @@ -2,6 +2,7 @@ use crate::models::{DiscoveryFailure, FailureKind}; use sqlbuild_sqltext::compiler::main::model_header_errors::header_nesting_failure; +use sqlbuild_sqltext::header_strings::main::quote_failure::quote_failure; /// One header that failed to parse, with the one-based file line its text starts on. pub(crate) struct FailedHeader<'a> { @@ -11,11 +12,34 @@ pub(crate) struct FailedHeader<'a> { pub(crate) file_path: &'a str, pub(crate) text: &'a str, pub(crate) line: usize, + /// The one-based column of the header text's first character. + pub(crate) column: usize, +} + +/// The one-based line and column of byte offset `start` in `contents`. +pub(crate) fn header_position(contents: &str, start: usize) -> (usize, usize) { + let before: &str = &contents[..start]; + let line_start: usize = before.rfind('\n').map_or(0, |newline| newline + 1); + ( + before.matches('\n').count() + 1, + before[line_start..].chars().count() + 1, + ) } /// The invalid-syntax failure for `error`; a nesting error names its file line and a fix. pub(crate) fn header_syntax_failure(header: &FailedHeader<'_>, error: &str) -> DiscoveryFailure { let (statement_name, file_path) = (header.statement_name, header.file_path); + if let Some(quote) = quote_failure(error, header.text, (header.line, header.column)) { + return DiscoveryFailure { + kind: header.kind, + message: format!( + "{statement_name}(...) in '{file_path}:{}:{}' contains invalid SQLBuild header \ + syntax: {}", + quote.line, quote.column, quote.message + ), + help: Some(quote.help), + }; + } let Some(nesting) = header_nesting_failure(error, header.text, header.line) else { return DiscoveryFailure::new( header.kind, diff --git a/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs b/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs index 255f8f8582..a8f94322a0 100644 --- a/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs +++ b/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs @@ -17,20 +17,21 @@ pub(crate) struct StatementHeader<'a> { pub(crate) supported_keys: &'a [String], } -/// `parse_header_values` then `reject_unsupported_header_keys`; `header_line` is one-based. +/// `parse_header_values` then `reject_unsupported_header_keys`, locating errors from `header_start`. pub(crate) fn parse_statement_header( contract: &StatementHeader<'_>, header: &str, - header_line: usize, + header_start: (usize, usize), ) -> Result, DiscoveryFailure> { + let header_line: usize = header_start.0; let values: Vec<(String, AuthoredValue)> = match parse_one(header) { - (_, _, Some(error)) => return Err(syntax_failure(contract, header, header_line, &error)), + (_, _, Some(error)) => return Err(syntax_failure(contract, header, header_start, &error)), (Some(AuthoredValue::Map(values)), _, None) => values, _ => { return Err(syntax_failure( contract, header, - header_line, + header_start, "Native MODEL header parser returned neither values nor an error", )); } @@ -57,7 +58,7 @@ pub(crate) fn parse_statement_header( fn syntax_failure( contract: &StatementHeader<'_>, header: &str, - header_line: usize, + (line, column): (usize, usize), error: &str, ) -> DiscoveryFailure { header_syntax_failure( @@ -66,7 +67,8 @@ fn syntax_failure( statement_name: contract.statement_name, file_path: contract.file_path, text: header, - line: header_line, + line, + column, }, error, ) diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_files.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_files.rs index 8837f04444..fd8ddd4a87 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_files.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_files.rs @@ -106,7 +106,13 @@ impl AuditParse<'_> { .iter() .filter(|character| **character == '\n') .count(); - let header_values: Vec<(String, AuthoredValue)> = self.header(&header, header_line)?; + let line_start: usize = block_chars[..=open_index] + .iter() + .rposition(|character| *character == '\n') + .map_or(0, |newline| newline + 1); + let header_column: usize = open_index + 2 - line_start; + let header_values: Vec<(String, AuthoredValue)> = + self.header(&header, (header_line, header_column))?; let evaluation_mode: &'static str = evaluation_mode(&header_values, file_path)?; let body: &[char] = &block_chars[body_start..]; let mut measure_sql: Option = None; @@ -154,7 +160,7 @@ impl AuditParse<'_> { fn header( &self, header: &str, - header_line: usize, + header_start: (usize, usize), ) -> Result, ParseStop> { let file_path: &str = self.file_path; let values: Vec<(String, AuthoredValue)> = parse_statement_header( @@ -166,7 +172,7 @@ impl AuditParse<'_> { supported_keys: &self.options.audit_keys, }, header, - header_line, + header_start, )?; for key in STRING_KEYS { if let Some(value) = get(&values, key) diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_text.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_text.rs index 053c22260d..639f617bb7 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_text.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/audit_text.rs @@ -1,5 +1,9 @@ //! The code-point scanning helpers Python's audit parser uses: comments, quotes and keywords. +use sqlbuild_sqltext::header_strings::main::follows_description_key::follows_description_key; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; +use sqlbuild_sqltext::header_strings::main::opens_triple_quote::opens_triple_quote; + const SQL_QUOTES: [char; 3] = ['\'', '"', '`']; /// Audit text as code points, with the Python semantics its keyword checks use. @@ -23,6 +27,16 @@ impl AuditText<'_> { (start..self.len()).find(|index| self.starts_with(*index, needle)) } + /// The end of the `"""..."""` or `description` string at `index`, if one opens there. + fn header_string_end(&self, index: usize) -> Option { + let header_string: bool = matches!(self.chars[index], '\'' | '"') + && (opens_triple_quote(self.chars, index) + || follows_description_key(self.chars, index)); + header_string + .then(|| header_quoted_end(self.chars, index)) + .flatten() + } + /// `_code_characters`: positions outside quotes and comments, from `start`. pub(crate) fn code_positions(&self, start: usize) -> Vec { let mut positions: Vec = Vec::new(); @@ -42,6 +56,10 @@ impl AuditText<'_> { .map_or(length, |close| close + 2); continue; } + if let Some(end) = self.header_string_end(index) { + index = end; + continue; + } if SQL_QUOTES.contains(&character) { index += 1; while index < length { diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs index f4881dfc6f..765a9dd5b3 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs @@ -1,11 +1,12 @@ //! Python's `_parse_declaration_headers`: split a file into `KIND(...);` headers and parse each. -use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure}; +use crate::_helpers::header_syntax::{FailedHeader, header_position, header_syntax_failure}; use crate::declaration_files::_helpers::checks::python_values::failure; use crate::declaration_files::_helpers::checks::stops::ParseStop; use crate::models::FailureKind; use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; use sqlbuild_sqltext::compiler::models::AuthoredValue; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; const DECLARATION_KINDS: [&str; 3] = ["ENUM", "CONSTANT", "SCHEMA"]; @@ -55,12 +56,14 @@ pub(crate) fn declaration_headers( ))); } let header_start: usize = open_index + 1; + let (line, column) = header_position(contents, header_start); let header = FailedHeader { kind: FailureKind::Declaration, statement_name: expected_kind, file_path, text: &contents[header_start..close_index], - line: contents[..header_start].matches('\n').count() + 1, + line, + column, }; let syntax_failure = |error: &str| ParseStop::Failed(header_syntax_failure(&header, error)); let (values, column_offsets) = match parse_one(header.text) { @@ -98,27 +101,18 @@ fn declaration_start(contents: &str, start: usize) -> Option<(&'static str, usiz (contents.as_bytes().get(open_index) == Some(&b'(')).then_some((kind, open_index)) } -/// `_find_closing_parenthesis`: quotes skip backslash escapes; `None` when the header never closes. +/// `_find_closing_parenthesis`: header strings skipped; `None` when the header never closes. fn closing_parenthesis(contents: &str, open_index: usize) -> Option { let bytes: &[u8] = contents.as_bytes(); let mut depth: usize = 1; - let mut quote: Option = None; let mut index: usize = open_index + 1; while index < bytes.len() { let byte: u8 = bytes[index]; - if let Some(open_quote) = quote { - if byte == b'\\' { - index += 1 + next_char_len(contents, index + 1); + match byte { + b'\'' | b'"' => { + index = header_quoted_end(bytes, index)?; continue; } - if byte == open_quote { - quote = None; - } - index += 1; - continue; - } - match byte { - b'\'' | b'"' => quote = Some(byte), b'(' => depth += 1, b')' => { depth -= 1; @@ -133,14 +127,6 @@ fn closing_parenthesis(contents: &str, open_index: usize) -> Option { None } -/// The UTF-8 length of the character at `index`, or 0 at the end. -fn next_char_len(contents: &str, index: usize) -> usize { - contents - .get(index..) - .and_then(|rest| rest.chars().next()) - .map_or(0, char::len_utf8) -} - /// The byte offset of the first non-whitespace character at or after `start`. fn skip_whitespace(contents: &str, start: usize) -> usize { contents[start..] diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/function_files.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/function_files.rs index 3b5bfa481b..c7252386d5 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/function_files.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/function_files.rs @@ -1,5 +1,6 @@ //! Python's `parse_function_sql`: a `FUNCTION(...);` header and the SQL body after it. +use crate::_helpers::header_syntax::header_position; use crate::_helpers::statement_headers::{StatementHeader, parse_statement_header}; use crate::declaration_files::_helpers::checks::stops::ParseStop; use crate::declaration_files::models::{DeclarationFileOptions, FunctionFile}; @@ -33,7 +34,7 @@ pub(crate) fn parse_function_file( supported_keys: &options.function_keys, }, &contents[header_start..header_end], - contents[..header_start].matches('\n').count() + 1, + header_position(&contents, header_start), )?; let body_sql: String = str::trim(&contents[sql_start..]).to_owned(); if body_sql.is_empty() { diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/hook_files.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/hook_files.rs index f94fe1e44f..90eb6cfda5 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/hook_files.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/hook_files.rs @@ -1,5 +1,6 @@ //! Python's `parse_sql_hook_file`: exactly one `HOOK(...);` header and the SQL after it. +use crate::_helpers::header_syntax::header_position; use crate::_helpers::statement_headers::{StatementHeader, parse_statement_header}; use crate::declaration_files::_helpers::checks::python_values::{ WordRules, failure, get, non_empty_str, python_str, @@ -9,6 +10,7 @@ use crate::declaration_files::models::{DeclarationFileOptions, HookFile}; use crate::models::FailureKind; use sqlbuild_core::text::main::python_cleandoc::python_cleandoc; use sqlbuild_sqltext::compiler::models::AuthoredValue; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; const HOOK_KEYWORD: &str = "HOOK"; const DESCRIPTION_KEY: &str = "description"; @@ -35,7 +37,7 @@ pub(crate) fn parse_hook_file( supported_keys: &options.hook_keys, }, header, - contents[..header_start].matches('\n').count() + 1, + header_position(&contents, header_start), )?; let description: Option = match get(&header_values, DESCRIPTION_KEY) { Some(value) => { @@ -81,28 +83,19 @@ fn split_hook_file(contents: &str, file_path: &str) -> Result<(usize, usize, usi } let header_start: usize = index + 1; let mut depth: usize = 1; - let mut quote: Option = None; index = header_start; while index < bytes.len() { let byte: u8 = bytes[index]; - if let Some(open_quote) = quote { - if byte == b'\\' { - index += 1 + next_char_len(contents, index + 1); - continue; - } - if byte == open_quote { - if bytes.get(index + 1) == Some(&open_quote) { - index += 2; - continue; - } - quote = None; + if SQL_QUOTES.contains(&byte) { + index = if byte == b'`' { + backtick_end(bytes, index) + } else { + header_quoted_end(bytes, index) } - index += 1; + .unwrap_or(bytes.len()); continue; } - if SQL_QUOTES.contains(&byte) { - quote = Some(byte); - } else if byte == b'(' { + if byte == b'(' { depth += 1; } else if byte == b')' { depth -= 1; @@ -127,11 +120,18 @@ fn split_hook_file(contents: &str, file_path: &str) -> Result<(usize, usize, usi Ok((header_start, header_end, index + 1)) } -fn next_char_len(contents: &str, index: usize) -> usize { - contents - .get(index..) - .and_then(|rest| rest.chars().next()) - .map_or(0, char::len_utf8) +/// The end of the backtick-quoted text at `start`: doubled backticks and backslashes escape. +fn backtick_end(bytes: &[u8], start: usize) -> Option { + let mut index: usize = start + 1; + while index < bytes.len() { + match bytes[index] { + b'\\' => index += 2, + b'`' if bytes.get(index + 1) == Some(&b'`') => index += 2, + b'`' => return Some(index + 1), + _ => index += 1, + } + } + None } fn skip_whitespace(contents: &str, start: usize) -> usize { diff --git a/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs b/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs index 29c20d9d54..e7e0ad7486 100644 --- a/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs +++ b/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs @@ -1,7 +1,7 @@ //! Parse one model file: header checks, model-local declaration checks and their messages. use crate::_helpers::header_keys::{UnsupportedKeys, unsupported_keys_failure}; -use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure}; +use crate::_helpers::header_syntax::{FailedHeader, header_position, header_syntax_failure}; use crate::_helpers::locations::header_column_locations; use crate::declaration_files::main::bare_number_failure::bare_number_failure; use crate::model_files::_helpers::output_columns::output_column_locations; @@ -29,13 +29,14 @@ pub(crate) fn parse_model_file( ))); }; let header = &contents[header_start..header_end]; - let header_line: usize = contents[..header_start].matches('\n').count() + 1; + let (header_line, header_column) = header_position(&contents, header_start); let failed_header = FailedHeader { kind: FailureKind::ModelSql, statement_name: "MODEL", file_path, text: header, line: header_line, + column: header_column, }; let (header_values, column_offsets) = match parse_one(header) { (_, _, Some(error)) => return Err(header_syntax_failure(&failed_header, &error)), diff --git a/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs b/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs index 8d0fcfc666..d844321ed7 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs @@ -1,5 +1,6 @@ //! Parse a scenario file header; Python builds the scenario from the parsed values. +use crate::_helpers::header_syntax::header_position; use crate::_helpers::statement_headers::{StatementHeader, parse_statement_header}; use crate::models::{DiscoveryFailure, FailureKind}; use crate::sql_tests::models::{DiscoveredScenarioFile, SqlTestFileOptions}; @@ -34,7 +35,7 @@ pub(crate) fn parse_scenario_file( supported_keys: &options.scenario_keys, }, &contents[header_start..header_end], - contents[..header_start].matches('\n').count() + 1, + header_position(&contents, header_start), )?; let sql_body: String = python_cleandoc(&contents[sql_start..]); let span_start: usize = contents[..sql_start].chars().count(); diff --git a/crates/sqlbuild-discovery/src/sql_tests/_helpers/test_blocks.rs b/crates/sqlbuild-discovery/src/sql_tests/_helpers/test_blocks.rs index 9a12dce126..05450a90dd 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/_helpers/test_blocks.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/_helpers/test_blocks.rs @@ -1,5 +1,6 @@ //! Split a SQL test file into `TEST` blocks and parse each block header. +use crate::_helpers::header_syntax::header_position; use crate::_helpers::statement_headers::{StatementHeader, parse_statement_header}; use crate::models::{DiscoveryFailure, FailureKind}; use crate::sql_tests::models::{DiscoveredSqlTestFile, SqlTestBlock, SqlTestFileOptions}; @@ -43,8 +44,8 @@ pub(crate) fn parse_sql_test_file( .get(index + 1) .map_or(contents.len(), |next| next.start); let header: &str = &contents[block.header_start..block.header_end]; - let header_line: usize = contents[..block.header_start].matches('\n').count() + 1; - match parse_statement_header(&contract, header, header_line) { + let header_start: (usize, usize) = header_position(&contents, block.header_start); + match parse_statement_header(&contract, header, header_start) { Ok(header_values) => { let body_start: usize = block.end.min(next_start); let authored: &str = diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs index 93945309cb..2fb32307ac 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs @@ -4,7 +4,9 @@ use std::path::PathBuf; use std::time::Instant; use pyo3::exceptions::PyException; -use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; +use pyo3::prelude::{ + Bound, PyAny, PyAnyMethods, PyErr, PyModule, PyModuleMethods, PyResult, Python, +}; use pyo3::types::{PyDict, PyString}; use pyo3::{pyfunction, wrap_pyfunction}; use sqlbuild_core::cli_text::models::CliStyle; @@ -24,6 +26,7 @@ use crate::bindings::_helpers::format_command::files::FormatChange; use crate::bindings::_helpers::format_command::report::lint_report; use crate::bindings::_helpers::format_command::run::{FormatOutcome, FormatRequest, run_format}; use crate::bindings::_helpers::format_command::selection::{LintInputs, resolve_lint_inputs}; +use crate::bindings::_helpers::format_command::stages::quote_fixable_file_count; use crate::bindings::_helpers::frontier::encoder::emit_error; use crate::bindings::_helpers::presentation_text::terminal::stdout_supports_color; use crate::bindings::_helpers::records::fields::field as native_field; @@ -51,6 +54,8 @@ struct Resolved<'py> { selected: Option>, discovered: Option>, has_fixture_candidates: bool, + /// Why `--fix` could not discover the project; descriptions that escape quotes may cause it. + discovery_error: Option, } fn string_tuple(value: &Bound<'_, PyAny>) -> PyResult> { @@ -81,6 +86,7 @@ fn resolve_inputs<'py>( selected: None, discovered: None, has_fixture_candidates: false, + discovery_error: None, }; if !scoped { let directory: PathBuf = project_dir.extract()?; @@ -109,6 +115,10 @@ fn resolve_inputs<'py>( resolved.selected = selected; resolved.discovered = Some(discovered); } + Err(error) if fix && !scoped && error.is_instance_of::(py) => { + resolved.discovery_error = Some(error); + return Ok(resolved); + } Err(error) if scoped || fix || !error.is_instance_of::(py) => { return Err(error); } @@ -116,10 +126,17 @@ fn resolve_inputs<'py>( } } if fix { - resolved.discovered = Some(discover_project_inputs( + match discover_project_inputs( py, Some(keywords(py, &[("project_dir", project_dir.clone())])?), - )?); + ) { + Ok(discovered) => resolved.discovered = Some(discovered), + Err(error) if error.is_instance_of::(py) => { + resolved.discovered = None; + resolved.discovery_error = Some(error); + } + Err(error) => return Err(error), + } } Ok(resolved) } @@ -213,6 +230,12 @@ fn run_command( selectors, (flags.fix, &config.settings.dialect), )?; + if let Some(error) = &resolved.discovery_error { + let directory: PathBuf = project_dir.extract()?; + if quote_fixable_file_count(&directory).map_err(value_error)? == 0 { + return Err(error.clone_ref(py)); + } + } let lint: Bound<'_, PyAny> = lint_config(py, &config, &resolved.dialect)?; let fixture_discovered: Option> = if flags.fix { resolved.discovered.clone() @@ -229,6 +252,7 @@ fn run_command( write: !(flags.check || flags.diff), fix: flags.fix, cache: !(flags.no_cache || flags.fix), + discovery_error: resolved.discovery_error.as_ref(), }; let (outcome, status, elapsed) = format_with_status(py, &request, flags.use_color)?; let root: PathBuf = project_dir.extract()?; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs index f6b68e23ce..3d26ef1018 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs @@ -4,10 +4,12 @@ use std::collections::{BTreeSet, HashMap}; use std::fs; use std::path::{Path, PathBuf}; -use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; +use pyo3::exceptions::PyException; +use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyErr, PyResult, Python}; use pyo3::types::{PyBool, PyDict, PyDictMethods}; use pyo3::{IntoPyObject, IntoPyObjectExt}; use sqlbuild_cache::digest::types::ContentDigest; +use sqlbuild_core::constants::PROJECT_CONFIG_FILENAME; use sqlbuild_core::format_command::main::collect_lint_files::collect_lint_files; use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; use sqlbuild_rules::lint_text::main::format_sql_bodies::format_sql_bodies; @@ -27,11 +29,12 @@ use crate::bindings::_helpers::format_command::files::{ FormatChange, PendingWrite, persist_changes, }; use crate::bindings::_helpers::format_command::stages::{ - each_file, format_header_text, lint_header_text, on_format_workers, parse_faults, + QuoteFix, each_file, format_header_text, lint_header_text, on_format_workers, parse_faults, + quote_fixed_text, }; -use crate::bindings::_helpers::records::construct::error_class; +use crate::bindings::_helpers::records::construct::{error_class, new_record_with}; use crate::bindings::_helpers::records::fields::field as native_field; -use crate::bindings::_helpers::records::ids::ErrorId; +use crate::bindings::_helpers::records::ids::{ErrorId, RecordId}; use crate::bindings::_helpers::rules::dsl_headers::rules_config; use crate::bindings::_helpers::rules::fixture_formatting::format_redundant_fixture_nulls; use crate::bindings::_helpers::rules::lint_pipeline::{ @@ -41,6 +44,14 @@ use crate::bindings::_helpers::rules::lint_text::{relative_parts, unsafe_fault}; use crate::bindings::types::CompilerDetach; const DESCRIPTION_PRESENT: &str = "description-present"; +/// The fix that rewrites descriptions escaping or doubling double quotes as `"""` strings. +const DESCRIPTION_QUOTES: &str = "description-quotes"; +const DESCRIPTION_QUOTES_REASON: &str = + "Escaped or doubled double quotes rewritten as a \"\"\"...\"\"\" string holding real quotes"; +const DESCRIPTION_QUOTES_REFUSED_REASON: &str = + "Quotes left as written: the file has header syntax errors the rewrite does not fix"; +const APPLIED: &str = "applied"; +const REFUSED: &str = "refused"; const LINE_FEED: &str = "\n"; const CARRIAGE_RETURN_LINE_FEED: &str = "\r\n"; @@ -56,6 +67,8 @@ pub(crate) struct FormatRequest<'a, 'py> { pub(crate) fix: bool, /// Whether files a previous run found clean are skipped, and clean files remembered. pub(crate) cache: bool, + /// Why `--fix` could not discover the project, which Rule fixes need. + pub(crate) discovery_error: Option<&'a PyErr>, } /// The outcome of one format run, with Python violations and Rule fix reports. @@ -591,6 +604,77 @@ fn uncached_entries( Ok((entries, kept)) } +/// One applied `description-quotes` fix report per rewritten description, in file order. +fn quote_fix_reports<'py>( + py: Python<'py>, + entries: &[Entry], + fixes: &[Option], +) -> PyResult>> { + let mut reports: Vec> = Vec::new(); + for (entry, fix) in entries.iter().zip(fixes) { + let Some(fix) = fix else { + continue; + }; + let (status, reason): (&str, &str) = if fix.text.is_some() { + (APPLIED, DESCRIPTION_QUOTES_REASON) + } else { + (REFUSED, DESCRIPTION_QUOTES_REFUSED_REASON) + }; + for line in &fix.lines { + reports.push(new_record_with( + py, + RecordId::RuleFixResult, + &[ + ( + "file_path", + entry.path.clone().into_pyobject(py)?.into_any(), + ), + ("code", DESCRIPTION_QUOTES.into_pyobject(py)?.into_any()), + ("line", line.into_pyobject(py)?.into_any()), + ("status", status.into_pyobject(py)?.into_any()), + ("reason", reason.into_pyobject(py)?.into_any()), + ], + )?); + } + } + Ok(reports) +} + +/// The project-file warning that Rule fixes, which compile the saved files, wait for quote fixes. +fn rule_fixes_skipped<'py>(py: Python<'py>, directory: &Path) -> PyResult> { + new_record_with( + py, + RecordId::LintViolation, + &[ + ( + "file_path", + directory + .join(PROJECT_CONFIG_FILENAME) + .into_pyobject(py)? + .into_any(), + ), + ("line", 1_i64.into_pyobject(py)?.into_any()), + ("column", 1_i64.into_pyobject(py)?.into_any()), + ("code", "format-fix-skipped".into_pyobject(py)?.into_any()), + ( + "message", + "Rule fixes were skipped: the project compiles only once the rewritten \ + descriptions are saved" + .into_pyobject(py)? + .into_any(), + ), + ("severity", "warning".into_pyobject(py)?.into_any()), + ("engine", "native".into_pyobject(py)?.into_any()), + ( + "remediation", + "Rerun `sqb format --fix` to apply Rule fixes." + .into_pyobject(py)? + .into_any(), + ), + ], + ) +} + /// Format the project's authored SQL files and report what remains. pub(crate) fn run_format<'py>(request: &FormatRequest<'_, 'py>) -> PyResult> { let py: Python<'py> = request.project_dir.py(); @@ -639,6 +723,15 @@ pub(crate) fn run_format<'py>(request: &FormatRequest<'_, 'py>) -> PyResult
> = native(py, || { each_file(&entries, |entry| lint_header_text(&entry.original, config)) })?; + let quote_fixes: Vec> = if request.fix { + native(py, || { + each_file(&entries, |entry| quote_fixed_text(&entry.original, config)) + })? + } else { + vec![None; entries.len()] + }; + let quote_reports: Vec> = quote_fix_reports(py, &entries, "e_fixes)?; + let quotes_fixed: bool = quote_fixes.iter().flatten().any(|fix| fix.text.is_some()); let mut run: Run<'_, '_, 'py> = Run { request, py, @@ -649,14 +742,25 @@ pub(crate) fn run_format<'py>(request: &FormatRequest<'_, 'py>) -> PyResult return Err(error.clone_ref(py)), + Some(_) => (quote_reports, vec![rule_fixes_skipped(py, &run.directory)?]), + None => match run.plan_rule_fixes() { + Ok((verified, faults)) => { + (quote_reports.into_iter().chain(verified).collect(), faults) + } + Err(error) if error.is_instance_of::(py) && quotes_fixed => { + let skipped: Bound<'py, PyAny> = rule_fixes_skipped(py, &run.directory)?; + (quote_reports, vec![skipped]) + } + Err(error) => return Err(error), + }, + } } else { (Vec::new(), Vec::new()) }; @@ -783,3 +887,26 @@ pub(crate) fn run_format<'py>(request: &FormatRequest<'_, 'py>) -> PyResult], lints: &[Vec]) -> Vec { + let mut safe: Vec = Vec::new(); + for (index, (fix, violations)) in quote_fixes.iter().zip(lints).enumerate() { + let fixed: bool = fix.as_ref().is_some_and(|fix| fix.text.is_some()); + if fixed || parse_faults(violations).is_empty() { + safe.push(index); + } + } + safe +} + +/// The quote-fixed text of each file a quote fix rewrote, by index. +fn quote_fixed_texts(quote_fixes: Vec>) -> HashMap { + let mut fixed: HashMap = HashMap::new(); + for (index, fix) in quote_fixes.into_iter().enumerate() { + if let Some(text) = fix.and_then(|fix| fix.text) { + fixed.insert(index, text); + } + } + fixed +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs index 7d263fb0ce..d7fa409d79 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs @@ -1,10 +1,15 @@ //! The per-file native work of a format run: header lint and header formatting. +use std::path::{Path, PathBuf}; use std::sync::OnceLock; use rayon::ThreadPool; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use sqlbuild_analysis::sql_macros::errors::SqlMacroError; +use sqlbuild_analysis::sql_macros::main::parse_sql_macro::parse_sql_macro; +use sqlbuild_core::format_command::main::collect_lint_files::collect_lint_files; use sqlbuild_rules::dsl_headers::constants::RULE_HEADER_PARSE; +use sqlbuild_rules::dsl_headers::main::fix_escaped_quotes::fix_escaped_quotes; use sqlbuild_rules::dsl_headers::main::format_headers::format_headers; use sqlbuild_rules::dsl_headers::main::scan_headers::scan_headers; use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; @@ -12,6 +17,12 @@ use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; use crate::bindings::_helpers::rules::dsl_headers::header_violations; use crate::bindings::_helpers::rules::native_header_parser::NativeHeaderParser; +/// The header kind of a SQL macro file. +const MACRO_HEADER_KIND: &str = "MACRO"; +/// The name and path a rewritten macro file is checked under; neither affects whether it parses. +const MACRO_CHECK_NAME: &str = "rewritten"; +const MACRO_CHECK_PATH: &str = "rewritten.sql"; + /// Parsers recurse deeply on nested SQL, so format workers get the stack the main thread has. const FORMAT_WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; @@ -52,6 +63,70 @@ pub(crate) fn parse_faults(violations: &[HeaderViolation]) -> Vec, + /// The one-based lines of the descriptions the fixes rewrite. + pub(crate) lines: Vec, +} + +/// Whether every header of `text`, `MACRO` headers included, parses. +fn headers_parse(text: &str, config: HeaderRulesConfig) -> Result { + if !parse_faults(&lint_header_text(text, config)?).is_empty() { + return Ok(false); + } + let characters: Vec = text.chars().collect(); + let defines_macro: bool = scan_headers(&characters, false) + .iter() + .any(|header| header.kind == MACRO_HEADER_KIND); + if !defines_macro { + return Ok(true); + } + match parse_sql_macro(MACRO_CHECK_NAME, MACRO_CHECK_PATH, text) { + Ok(_) => Ok(true), + Err(SqlMacroError { .. }) => Ok(false), + } +} + +/// The description quote fixes of `text`, applied only when the whole rewritten file parses. +pub(crate) fn quote_fixed_text( + text: &str, + config: HeaderRulesConfig, +) -> Result, String> { + let characters: Vec = text.chars().collect(); + let (fixed, lines) = fix_escaped_quotes(&characters); + if lines.is_empty() { + return Ok(None); + } + let fixed: String = fixed.into_iter().collect(); + let parses: bool = headers_parse(&fixed, config)?; + Ok(Some(QuoteFix { + text: parses.then_some(fixed), + lines, + })) +} + +/// The number of authored SQL files of the project with a description escaping or doubling quotes. +pub(crate) fn quote_fixable_file_count(directory: &Path) -> Result { + let files: Vec<(PathBuf, String)> = + collect_lint_files(directory, None).map_err(|error| error.to_string())?; + Ok(files + .iter() + .filter(|(_, text)| { + (text.contains("\\\"") || text.contains("\"\"")) && escapes_quotes(text) + }) + .count()) +} + +/// Whether a header description of `text` escapes or doubles quotes that a fix can rewrite. +fn escapes_quotes(text: &str) -> bool { + !fix_escaped_quotes(&text.chars().collect::>()) + .1 + .is_empty() +} + /// `text` with header formatting fixes applied, and the faults relocation reported. pub(crate) fn format_header_text( text: &str, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs index 64879da3a3..c534fddb24 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs @@ -9,6 +9,8 @@ use sqlbuild_rules::dsl_headers::types::HeaderParser; use sqlbuild_sqltext::compiler::main::header_word_value::header_word_value; use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; use sqlbuild_sqltext::compiler::models::{AuthoredValue, HeaderWord}; +use sqlbuild_sqltext::header_strings::main::quote_failure::quote_failure; +use sqlbuild_sqltext::header_strings::models::QuoteFailure; use crate::bindings::_helpers::boundary::panics::value_error; use crate::bindings::_helpers::records::construct::error_class; @@ -53,6 +55,7 @@ impl HeaderParser for NativeHeaderParser { ParseFailure { message, help: None, + position: None, }, ); } @@ -62,12 +65,14 @@ impl HeaderParser for NativeHeaderParser { return ParseOutcome::Parsed(value); } let error: String = error.unwrap_or_else(|| "None".to_owned()); + let quote: Option = quote_failure(&error, inner, (1, 1)); let (detail, help, _) = header_failure(&error, inner, None); let failure: ParseFailure = ParseFailure { message: format!( "{kind}(...) in '{SENTINEL_PATH}' contains invalid SQLBuild header syntax: {detail}" ), help, + position: quote.map(|quote| quote.position), }; ParseOutcome::Rejected(failure) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/sqltext/model_headers.rs b/crates/sqlbuild-python/src/bindings/_helpers/sqltext/model_headers.rs index b8dd61632e..e71636d6c5 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/sqltext/model_headers.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/sqltext/model_headers.rs @@ -110,6 +110,17 @@ pub(crate) fn header_failure( header, header_line.unwrap_or(1), ); + if let Some(quote) = sqlbuild_sqltext::header_strings::main::quote_failure::quote_failure( + error, + header, + (header_line.unwrap_or(1), 1), + ) { + return ( + quote.message, + Some(quote.help), + header_line.map(|_| quote.line), + ); + } match (nesting, header_line) { (Some(nesting), Some(_)) => (nesting.message, Some(nesting.help), Some(nesting.line)), (Some(nesting), None) => (error.to_owned(), Some(nesting.help), None), diff --git a/crates/sqlbuild-refactor/src/refactoring/_helpers/contracts/model_types.rs b/crates/sqlbuild-refactor/src/refactoring/_helpers/contracts/model_types.rs index 32448adbb1..ca902368d6 100644 --- a/crates/sqlbuild-refactor/src/refactoring/_helpers/contracts/model_types.rs +++ b/crates/sqlbuild-refactor/src/refactoring/_helpers/contracts/model_types.rs @@ -1,9 +1,13 @@ //! MODEL header column types: rendering, matching and the authored span of a declared type. +use sqlbuild_sqltext::header_strings::constants::TRIPLE_QUOTE_LENGTH; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; +use sqlbuild_sqltext::header_strings::main::opens_triple_quote::opens_triple_quote; + use crate::refactoring::_helpers::scanning::chars::slice; use crate::refactoring::constants::{ - CONTRACT_CLOSERS, CONTRACT_HEADER_WORD_EXTRA, CONTRACT_MODEL_HEADER_ESCAPE, CONTRACT_OPENERS, - CONTRACT_QUOTES, CONTRACT_TYPE_KEY, + CONTRACT_CLOSERS, CONTRACT_HEADER_WORD_EXTRA, CONTRACT_OPENERS, CONTRACT_QUOTES, + CONTRACT_TYPE_KEY, }; fn is_type_name_start(character: char) -> bool { @@ -103,20 +107,9 @@ fn is_header_word_character(character: char) -> bool { char::is_alphanumeric(character) || CONTRACT_HEADER_WORD_EXTRA.contains(&character) } +/// The end of the header string at `start`, by the shared header string rules, or `end`. fn quoted_value_end(text: &[char], start: usize, end: usize) -> usize { - let quote = text[start]; - let mut index = start + 1; - while index < end { - if text[index] == CONTRACT_MODEL_HEADER_ESCAPE { - index += 2; - continue; - } - if text[index] == quote { - return index + 1; - } - index += 1; - } - end + header_quoted_end(&text[..end], start).map_or(end, |value_end| value_end.min(end)) } fn unquoted_type_value_end(text: &[char], start: usize, end: usize) -> usize { @@ -196,6 +189,15 @@ pub(crate) fn declared_type_span( value_start += 1; } let (value_end, authored) = match text.get(value_start) { + Some(_) if opens_triple_quote(text, value_start) => { + let value_end = quoted_value_end(text, value_start, metadata_end); + let inner = slice( + text, + value_start + TRIPLE_QUOTE_LENGTH, + value_end.saturating_sub(TRIPLE_QUOTE_LENGTH), + ); + (value_end, inner) + } Some(quote) if CONTRACT_QUOTES.contains(quote) => { let value_end = quoted_value_end(text, value_start, metadata_end); let inner = slice(text, value_start + 1, value_end.saturating_sub(1)); diff --git a/crates/sqlbuild-refactor/src/refactoring/_helpers/edits/header_edits.rs b/crates/sqlbuild-refactor/src/refactoring/_helpers/edits/header_edits.rs index c1c0a50c3e..784be1caaf 100644 --- a/crates/sqlbuild-refactor/src/refactoring/_helpers/edits/header_edits.rs +++ b/crates/sqlbuild-refactor/src/refactoring/_helpers/edits/header_edits.rs @@ -5,6 +5,7 @@ use sqlbuild_model_config::header_metadata::constants::AUDIT_OPTION_KEYS; use sqlbuild_sqltext::compiler::main::model_header_matching::match_batch; use sqlbuild_sqltext::compiler::main::model_header_tokenizing::tokenize_one; use sqlbuild_sqltext::compiler::main::statement_header_matching::match_statement_header; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; use crate::refactoring::_helpers::edits::text_edits::{text_edit, whole_word_offsets}; use crate::refactoring::_helpers::scanning::chars::{find, rfind, slice, starts_with}; @@ -118,21 +119,10 @@ pub(crate) fn span_tokens( /// The offset just past the quoted value that opens at `start` (`quoted_value_end`). fn quoted_value_end(text: &[char], start: usize) -> usize { - let Some("e) = text.get(start) else { + if start >= text.len() { return text.len(); - }; - let mut index = start + 1; - while index < text.len() { - if text[index] == '\\' && index + 1 < text.len() { - index += 2; - continue; - } - if text[index] == quote { - return index + 1; - } - index += 1; } - text.len() + header_quoted_end(text, start).map_or(text.len(), |end| end.min(text.len())) } fn is_separator(token: &HeaderToken) -> bool { diff --git a/crates/sqlbuild-refactor/src/refactoring/constants.rs b/crates/sqlbuild-refactor/src/refactoring/constants.rs index 6a317bd16f..eea18227e3 100644 --- a/crates/sqlbuild-refactor/src/refactoring/constants.rs +++ b/crates/sqlbuild-refactor/src/refactoring/constants.rs @@ -88,7 +88,6 @@ pub const CONTRACT_TYPE_KEY: &str = "type"; pub const CONTRACT_YAML_COLUMNS_KEY: &str = "columns"; pub const CONTRACT_YAML_LIST_NAME: &str = "- name:"; pub const CONTRACT_YAML_COMMENT: &str = " #"; -pub const CONTRACT_MODEL_HEADER_ESCAPE: char = '\\'; pub const CONTRACT_HEADER_WORD_EXTRA: [char; 2] = ['_', '$']; pub const CONTRACT_QUOTES: [char; 2] = ['\'', '"']; pub const CONTRACT_OPENERS: [char; 3] = ['(', '[', '{']; diff --git a/crates/sqlbuild-refactor/src/refactoring/tests/test_contract_edits.rs b/crates/sqlbuild-refactor/src/refactoring/tests/test_contract_edits.rs index 780f3acb04..4de52dca4b 100644 --- a/crates/sqlbuild-refactor/src/refactoring/tests/test_contract_edits.rs +++ b/crates/sqlbuild-refactor/src/refactoring/tests/test_contract_edits.rs @@ -22,6 +22,18 @@ fn given_nested_type_key_when_locating_declared_type_then_returns_top_level_valu declared_type: "INT", expected_span: Some((40, 43)), }, + DeclaredTypeSpanTestCase { + description: "a triple-quoted description with quotes and a type word is skipped", + contents: "amount (description \"\"\"x \"type INT\" y\"\"\", type INT)", + declared_type: "INT", + expected_span: Some((47, 50)), + }, + DeclaredTypeSpanTestCase { + description: "a description ending in a backslash is skipped", + contents: "amount (description \"under C:\\\", type INT)", + declared_type: "INT", + expected_span: Some((38, 41)), + }, DeclaredTypeSpanTestCase { description: "a quoted type matches after unescaping", contents: "amount (type 'a\\'b')", diff --git a/crates/sqlbuild-rules/src/configuration/_helpers/loading.rs b/crates/sqlbuild-rules/src/configuration/_helpers/loading.rs index 6737b8cf28..dcf05f036f 100644 --- a/crates/sqlbuild-rules/src/configuration/_helpers/loading.rs +++ b/crates/sqlbuild-rules/src/configuration/_helpers/loading.rs @@ -8,6 +8,7 @@ use crate::constants::{ use crate::models::RulesConfig; use fensu_policy::policy::types::RuleCodeGrammar; use sqlbuild_core::constants::PROJECT_CONFIG_FILENAME; +use sqlbuild_sqltext::header_strings::main::fold_prose::fold_prose; use std::path::Path; pub(crate) fn load(project_dir: &Path) -> Result { @@ -30,7 +31,7 @@ pub(crate) fn load(project_dir: &Path) -> Result RulesConfig::default(), Some(value) => { if let Some(error) = value.as_table().and_then(removed_settings) { @@ -44,9 +45,45 @@ pub(crate) fn load(project_dir: &Path) -> Result Result<(), String> { let table = value .as_table() diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/formatting.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/formatting.rs index 2eaed5d2ef..253cc699bb 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/formatting.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/formatting.rs @@ -4,17 +4,23 @@ use sqlbuild_core::text::main::python_splitlines::python_splitlines; use sqlbuild_core::text::main::python_strip::python_strip; use crate::dsl_headers::_helpers::audits::wrap_header; -use crate::dsl_headers::_helpers::quoted::{ - decode_description, description_value_span, escape_quoted, split_outside_quotes, +use sqlbuild_sqltext::header_strings::constants::TRIPLE_QUOTE; +use sqlbuild_sqltext::header_strings::main::fold_prose::fold_prose; +use sqlbuild_sqltext::header_strings::main::prose_string::prose_string; + +use crate::dsl_headers::_helpers::macro_header::same_macro; +use crate::dsl_headers::_helpers::prose_layout::{ + canonical_prose, closing_width, description_string_span, }; +use crate::dsl_headers::_helpers::quoted::split_outside_quotes; use crate::dsl_headers::_helpers::rules::violation_at; use crate::dsl_headers::_helpers::text::{ - chars, line_count, line_starts, lstrip, rstrip, slice, string, tail, + chars, find, line_count, line_starts, lstrip, rfind_before, rstrip, slice, string, tail, }; -use crate::dsl_headers::_helpers::wrap::{wrap_description, wrap_header_description}; +use crate::dsl_headers::_helpers::wrap::wrap_description; use crate::dsl_headers::constants::{ - DESCRIPTION_HEADER_KINDS, DESCRIPTION_KEY, HEADER_INDENT, MODEL_KIND, - RULE_LEADING_COMMENT_DESCRIPTION, + DESCRIPTION_KEY, HEADER_INDENT, MODEL_KIND, RULE_LEADING_COMMENT_DESCRIPTION, + SQL_MACRO_HEADER_KIND, }; use crate::dsl_headers::main::inner_header_text::inner_header_text; use crate::dsl_headers::main::scan_headers::scan_headers; @@ -96,15 +102,20 @@ fn relocated_description( (description, vec![fault]) } -/// The `description "..."` line inserted after a model header's opening parenthesis. -fn description_insertion(description: &[char]) -> Vec { - let escaped: String = string(description) - .replace('\\', "\\\\") - .replace('"', "\\\"") - .replace("\r\n", "\n"); - chars(&format!( - "\n{HEADER_INDENT}{DESCRIPTION_KEY} \"{escaped}\"," - )) +/// The `description` line inserted after a model header's `(`, if the comment can be quoted. +fn description_insertion(description: &[char]) -> Option> { + let value: String = fold_prose(&string(description)); + if value.contains(TRIPLE_QUOTE) { + return None; + } + let quoted: String = if value.contains('"') { + format!("{TRIPLE_QUOTE}{value}{TRIPLE_QUOTE}") + } else { + format!("\"{value}\"") + }; + Some(chars(&format!( + "\n{HEADER_INDENT}{DESCRIPTION_KEY} {quoted}," + ))) } /// A model header's leading comment moved into its description, with any length fault. @@ -131,7 +142,9 @@ pub(crate) fn relocate_leading_comment( return Ok(unchanged()); } let (description, faults) = relocated_description(&comment, config, (text, header.start)); - let insertion: Vec = description_insertion(&description); + let Some(insertion) = description_insertion(&description) else { + return Ok(unchanged()); + }; let mut updated: Vec = match header_text.iter().position(|character| *character == '(') { Some(open) => [&header_text[..=open], &insertion, &header_text[open + 1..]].concat(), None => header_text.to_vec(), @@ -140,53 +153,88 @@ pub(crate) fn relocate_leading_comment( Ok((updated, faults)) } +/// Every top-level description of `headers` of the `kinds` in its canonical layout. pub(crate) fn wrap_descriptions( text: &[char], headers: &[HeaderSpan], - line_width: i64, + (kinds, line_width): (&[&str], i64), parser: &P, ) -> Result, P::Error> { let mut updated: Vec = text.to_vec(); for header in headers.iter().rev() { - if !DESCRIPTION_HEADER_KINDS.contains(&header.kind.as_str()) { + if !kinds.contains(&header.kind.as_str()) { continue; } let header_text: Vec = slice(&updated, header.start, header.end).to_vec(); - let Some((start, end)) = description_value_span(&header_text) else { + let Some(rewritten) = laid_out_description(&header.kind, &header_text, line_width, parser)? + else { continue; }; - let values: P::Values = - match parser.parse(&header.kind, &string(inner_header_text(&header_text))) { - ParseOutcome::Parsed(values) => values, - ParseOutcome::Rejected(_) => continue, - ParseOutcome::Raised(error, _) => return Err(error), - }; - if parser.description(&values)?.is_none() { - continue; - } - let quote: char = header_text[start - 1]; - let wrapped: Vec = wrap_header_description( - &decode_description(slice(&header_text, start, end), quote), - &header_text, - (start, end), - line_width, - ); - let rewritten: Vec = [ - slice(&header_text, 0, start), - &escape_quoted(&wrapped, quote), - tail(&header_text, end), - ] - .concat(); - updated = [ + let candidate: Vec = [ slice(&updated, 0, header.start), &rewritten, tail(&updated, header.end), ] .concat(); + if header.kind == SQL_MACRO_HEADER_KIND && !same_macro(&updated, &candidate) { + continue; + } + updated = candidate; } Ok(updated) } +/// `header_text` with its description laid out canonically, or `None` when nothing changes. +fn laid_out_description( + kind: &str, + header_text: &[char], + line_width: i64, + parser: &P, +) -> Result>, P::Error> { + let Some((start, end)) = description_string_span(header_text) else { + return Ok(None); + }; + let Ok(prose) = prose_string(header_text, start) else { + return Ok(None); + }; + if header_text[start] == '\'' && keeps_single_quotes(header_text, (start, end), line_width) { + return Ok(None); + } + let Some(rendered) = canonical_prose(&prose.value, header_text, (start, end), line_width) + else { + return Ok(None); + }; + if chars(&rendered) == slice(header_text, start, end) { + return Ok(None); + } + let rewritten: Vec = [ + slice(header_text, 0, start), + &chars(&rendered), + tail(header_text, end), + ] + .concat(); + if kind != SQL_MACRO_HEADER_KIND && !same_values(kind, (&rewritten, header_text), parser)? { + return Ok(None); + } + Ok(Some(rewritten)) +} + +/// Whether a `'...'` description stays as written: on one line that fits, with no double quote. +fn keeps_single_quotes( + header_text: &[char], + (start, end): (usize, usize), + line_width: i64, +) -> bool { + let authored: &[char] = slice(header_text, start, end); + if authored.contains(&'\n') || authored.contains(&'"') { + return false; + } + let line_start: usize = rfind_before(header_text, '\n', start).map_or(0, |found| found + 1); + let line_end: usize = find(header_text, &['\n'], end).unwrap_or(header_text.len()); + let width: usize = end - line_start + closing_width(slice(header_text, end, line_end)); + i64::try_from(width).is_ok_and(|width| width <= line_width) +} + /// Whether two header texts parse to equal values; a header syntax error equals nothing. fn same_values( kind: &str, diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/macro_header.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/macro_header.rs index 79d2b463ad..0ad631ecfd 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/macro_header.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/macro_header.rs @@ -1,5 +1,7 @@ -//! The canonical layout of a SQL macro's `MACRO (...)` header. +//! The canonical layout of a SQL macro's `MACRO (...)` header, and whether a layout kept its meaning. +use sqlbuild_analysis::sql_macros::main::parse_sql_macro::parse_sql_macro; +use sqlbuild_analysis::sql_macros::models::SqlMacroDefinition; use sqlbuild_core::text::main::python_isspace::python_isspace; use crate::dsl_headers::_helpers::quoted::quoted_value_end; @@ -192,3 +194,29 @@ fn has_comment(header: &[char]) -> bool { } false } + +/// The macro `text` defines with every line and column cleared, if it parses. +fn positionless_macro(text: &[char]) -> Option { + let Ok(mut definition) = parse_sql_macro("formatted", "formatted.sql", &string(text)) else { + return None; + }; + definition.body_line = 0; + definition.body_column = 0; + for parameter in &mut definition.parameters { + parameter.line = 0; + parameter.column = 0; + } + Some(definition) +} + +/// Whether `before` and `after` both parse to the same macro; a parse failure equals nothing. +pub(crate) fn same_macro(before: &[char], after: &[char]) -> bool { + positionless_macro(before) + .is_some_and(|definition| positionless_macro(after) == Some(definition)) +} + +/// Whether `before` defines a macro that `after` no longer defines the same. +pub(crate) fn changes_macro(before: &[char], after: &[char]) -> bool { + positionless_macro(before) + .is_some_and(|definition| positionless_macro(after) != Some(definition)) +} diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/mod.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/mod.rs index a893c2572c..b27147fcd6 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/mod.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/mod.rs @@ -1,6 +1,8 @@ pub(crate) mod audits; pub(crate) mod formatting; pub(crate) mod macro_header; +pub(crate) mod prose_layout; +pub(crate) mod quote_fix; pub(crate) mod quoted; pub(crate) mod rules; pub(crate) mod scan; diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/prose_layout.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/prose_layout.rs new file mode 100644 index 0000000000..6265ac45a1 --- /dev/null +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/prose_layout.rs @@ -0,0 +1,160 @@ +//! The canonical layout of a header description: one line when it fits, else a `"""` block. + +use sqlbuild_sqltext::header_strings::constants::{PARAGRAPH_BREAK, TRIPLE_QUOTE}; +use sqlbuild_sqltext::header_strings::main::prose_string::prose_string; + +use crate::dsl_headers::_helpers::quoted::description_value_span; +use crate::dsl_headers::_helpers::text::{ + chars, find, line_count, rfind_before, rstrip, slice, string, tail, +}; +use crate::dsl_headers::_helpers::wrap::wrap_description; +use crate::dsl_headers::constants::HEADER_INDENT; + +/// The span of the top-level `description` string, from its opening to past its closing quote. +pub(crate) fn description_string_span(header_text: &[char]) -> Option<(usize, usize)> { + let (value_start, _) = description_value_span(header_text)?; + let start: usize = value_start - 1; + let Ok(prose) = prose_string(header_text, start) else { + return None; + }; + Some((start, prose.end)) +} + +/// The breaks of one paragraph: lines filled to `width`, broken only at single spaces. +fn paragraph_lines(paragraph: &str, width: usize) -> Vec { + let characters: Vec = paragraph.chars().collect(); + let mut words: Vec = Vec::new(); + let mut word_start: usize = 0; + for index in 1..characters.len().saturating_sub(1) { + if characters[index] == ' ' && characters[index - 1] != ' ' && characters[index + 1] != ' ' + { + words.push(string(&characters[word_start..index])); + word_start = index + 1; + } + } + words.push(string(&characters[word_start..])); + let mut lines: Vec = Vec::new(); + let mut line: String = String::new(); + for word in words { + if !line.is_empty() && line.chars().count() + 1 + word.chars().count() > width { + lines.push(std::mem::take(&mut line)); + } + if !line.is_empty() { + line.push(' '); + } + line.push_str(&word); + } + lines.push(line); + lines +} + +/// `value` as an indented `"""` block whose closing delimiter lines up with `indent`. +fn block(value: &str, indent: &str, line_width: usize) -> String { + let content_indent: String = format!("{indent}{HEADER_INDENT}"); + let width: usize = line_width + .saturating_sub(content_indent.chars().count()) + .max(1); + let mut lines: Vec = Vec::new(); + for (index, paragraph) in value.split(PARAGRAPH_BREAK).enumerate() { + if index > 0 { + lines.push(String::new()); + } + lines.extend( + paragraph_lines(paragraph, width) + .into_iter() + .map(|line| format!("{content_indent}{line}")), + ); + } + format!( + "{TRIPLE_QUOTE}\n{}\n{indent}{TRIPLE_QUOTE}", + lines.join("\n") + ) +} + +/// The canonical text of the description string at `span`, or `None` if none stores `value`. +pub(crate) fn canonical_prose( + value: &str, + header_text: &[char], + (start, end): (usize, usize), + line_width: i64, +) -> Option { + let width: usize = usize::try_from(line_width).unwrap_or(0); + let line_start: usize = rfind_before(header_text, '\n', start).map_or(0, |found| found + 1); + let line_end: usize = find(header_text, &['\n'], end).unwrap_or(header_text.len()); + let prefix: usize = start - line_start; + let suffix: usize = closing_width(slice(header_text, end, line_end)); + let indent: String = header_text[line_start..start] + .iter() + .take_while(|character| matches!(character, ' ' | '\t')) + .collect(); + if value.contains(TRIPLE_QUOTE) { + return None; + } + let after: &[char] = tail(header_text, end); + let one_line: Option = (!value.contains('\n')).then(|| { + let double: String = format!("\"{value}\""); + if !value.contains('"') && stores_in_place(&double, value, after) { + double + } else { + format!("{TRIPLE_QUOTE}{value}{TRIPLE_QUOTE}") + } + }); + let rendered: String = match one_line { + Some(line) if prefix + line.chars().count() + suffix <= width => line, + _ => block(value, &indent, width), + }; + stores_in_place(&rendered, value, after).then_some(rendered) +} + +/// The width a description's line adds after it: its separator, plus closing brackets that end it. +pub(crate) fn closing_width(rest: &[char]) -> usize { + let rest: &[char] = rstrip(rest); + if rest + .iter() + .all(|character| matches!(character, ',' | ')' | ';' | ' ' | '\t')) + { + return rest.len(); + } + let spaces: usize = rest + .iter() + .take_while(|character| matches!(character, ' ' | '\t')) + .count(); + if rest.get(spaces) == Some(&',') { + spaces + 1 + } else { + 0 + } +} + +/// Whether `rendered`, followed by the header text `after` it, reads as exactly `value`. +fn stores_in_place(rendered: &str, value: &str, after: &[char]) -> bool { + let rendered: Vec = chars(rendered); + let followed: Vec = [rendered.as_slice(), after].concat(); + prose_string(&followed, 0) + .is_ok_and(|stored| stored.end == rendered.len() && stored.value == value) +} + +/// The lines the description-length rule counts for `value` at `span` of `header_text`. +pub(crate) fn prose_length( + value: &str, + header_text: &[char], + (start, end): (usize, usize), + line_width: i64, +) -> usize { + match canonical_prose(value, header_text, (start, end), line_width) { + Some(rendered) if !rendered.contains('\n') => 1, + _ => { + let line_start: usize = + rfind_before(header_text, '\n', start).map_or(0, |found| found + 1); + let line_end: usize = find(header_text, &['\n'], end).unwrap_or(header_text.len()); + let prefix: i64 = i64::try_from(start + 1 - line_start).unwrap_or(i64::MAX); + let suffix: i64 = i64::try_from(rstrip(slice(header_text, end, line_end)).len() + 1) + .unwrap_or(i64::MAX); + line_count(&wrap_description( + &chars(value), + (line_width - prefix - suffix).max(1), + (line_width - suffix).max(1), + )) + } + } +} diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/quote_fix.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/quote_fix.rs new file mode 100644 index 0000000000..d8b44d02a1 --- /dev/null +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/quote_fix.rs @@ -0,0 +1,115 @@ +//! Header descriptions that escape or double their double quotes, and their `"""` rewrites. + +use sqlbuild_core::text::main::python_isspace::python_isspace; +use sqlbuild_sqltext::header_strings::constants::{LONGEST_CLOSING_RUN, TRIPLE_QUOTE}; +use sqlbuild_sqltext::header_strings::main::closes_inside_text::closes_inside_text; +use sqlbuild_sqltext::header_strings::main::fold_prose::fold_prose; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; +use sqlbuild_sqltext::header_strings::main::opens_triple_quote::opens_triple_quote; +use sqlbuild_sqltext::header_strings::main::prose_end::prose_end; +use sqlbuild_sqltext::header_strings::main::prose_string::prose_string; + +use crate::dsl_headers::_helpers::text::{chars, is_word}; +use crate::dsl_headers::constants::{DESCRIPTION_KEY, QUOTES}; +use crate::dsl_headers::models::HeaderSpan; + +/// The end of the `"..."` string at `start` read with `\"` and `""` as quotes in the text. +fn escaped_end(text: &[char], start: usize) -> Option { + let mut index: usize = start + 1; + while index < text.len() { + let pair: bool = matches!(text[index], '\\' | '"') && text.get(index + 1) == Some(&'"'); + if pair { + index += 2; + continue; + } + if text[index] == '"' { + return Some(index + 1); + } + index += 1; + } + None +} + +/// The inside of a `"..."` string with `\"` and `""` as real quotes, if it has one and no `\`. +fn unescaped(raw: &[char]) -> Option { + let mut text: String = String::with_capacity(raw.len()); + let mut escaped: bool = false; + let mut index: usize = 0; + while index < raw.len() { + let pair: bool = matches!(raw[index], '\\' | '"') && raw.get(index + 1) == Some(&'"'); + if pair { + escaped = true; + text.push('"'); + index += 2; + continue; + } + if raw[index] == '\\' { + return None; + } + text.push(raw[index]); + index += 1; + } + escaped.then_some(text) +} + +/// `"""text"""` when it stores the same folded text and closes where it should. +fn triple_quoted(text: &str, after: &[char]) -> Option> { + let trailing: usize = text.chars().rev().take_while(|unit| *unit == '"').count(); + if text.contains(TRIPLE_QUOTE) || trailing + TRIPLE_QUOTE.len() > LONGEST_CLOSING_RUN { + return None; + } + let rewritten: Vec = chars(&format!("{TRIPLE_QUOTE}{text}{TRIPLE_QUOTE}")); + let Ok(prose) = prose_string(&rewritten, 0) else { + return None; + }; + let followed: Vec = [rewritten.as_slice(), after].concat(); + (prose.end == rewritten.len() + && prose.value == fold_prose(text) + && !closes_inside_text(&followed, rewritten.len())) + .then_some(rewritten) +} + +/// The `description` string spans of one header that escape or double a quote, and rewrites. +pub(crate) fn header_fixes(text: &[char], header: &HeaderSpan) -> Vec<(usize, usize, Vec)> { + let mut fixes: Vec<(usize, usize, Vec)> = Vec::new(); + let key: Vec = chars(DESCRIPTION_KEY); + let mut index: usize = header.start; + while index < header.end { + if QUOTES.contains(&text[index]) { + index = header_quoted_end(&text[..header.end], index).unwrap_or(header.end); + continue; + } + let at_key: bool = text[index..header.end].starts_with(&key) + && !(index > 0 && is_word(text[index - 1])) + && !text + .get(index + key.len()) + .is_some_and(|next| is_word(*next)); + if !at_key { + index += 1; + continue; + } + let mut start: usize = index + key.len(); + while start < header.end && python_isspace(text[start]) { + start += 1; + } + index = start; + if text.get(start) != Some(&'"') || opens_triple_quote(text, start) { + continue; + } + let bounded: &[char] = &text[..header.end]; + if let Ok(end) = prose_end(bounded, start) { + index = end; + continue; + } + let Some(end) = escaped_end(bounded, start) else { + continue; + }; + let rewrite: Option> = unescaped(&text[start + 1..end - 1]) + .and_then(|unquoted| triple_quoted(&unquoted, &text[end..header.end])); + if let Some(rewrite) = rewrite { + fixes.push((start, end, rewrite)); + } + index = end; + } + fixes +} diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/quoted.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/quoted.rs index 8b76cc1b14..6a17299ee4 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/quoted.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/quoted.rs @@ -3,27 +3,14 @@ use sqlbuild_core::text::main::python_isalnum::python_isalnum; use sqlbuild_core::text::main::python_isspace::python_isspace; -use crate::dsl_headers::_helpers::text::{is_alpha, string}; -use crate::dsl_headers::constants::{DESCRIPTION_KEY, ESCAPE, QUOTES}; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; -const MAX_CODEPOINT: u32 = 0x0010_FFFF; -const SURROGATES: std::ops::RangeInclusive = 0xD800..=0xDFFF; +use crate::dsl_headers::_helpers::text::{is_alpha, string}; +use crate::dsl_headers::constants::{DESCRIPTION_KEY, QUOTES}; -/// The offset just past the quoted value that opens at `start`. +/// The offset just past the quoted value that opens at `start`, or the end when it never closes. pub(crate) fn quoted_value_end(text: &[char], start: usize) -> usize { - let quote: char = text[start]; - let mut index: usize = start + 1; - while index < text.len() { - if text[index] == ESCAPE && index + 1 < text.len() { - index += 2; - continue; - } - if text[index] == quote { - return index + 1; - } - index += 1; - } - text.len() + header_quoted_end(text, start).map_or(text.len(), |end| end.min(text.len())) } /// The span of the top-level `description` string value inside the header parentheses. @@ -76,95 +63,18 @@ pub(crate) fn description_value_span(text: &[char]) -> Option<(usize, usize)> { None } -/// The text of a quoted description value with its escapes decoded. -pub(crate) fn decode_description(value: &[char], quote: char) -> Vec { - let mut decoded: Vec = Vec::with_capacity(value.len()); - let mut index: usize = 0; - while index < value.len() { - let character: char = value[index]; - if character != ESCAPE || index + 1 >= value.len() { - decoded.push(character); - index += 1; - continue; - } - let escaped: char = value[index + 1]; - if escaped == ESCAPE || escaped == quote { - decoded.push(escaped); - index += 2; - continue; - } - let simple: Option = match escaped { - 'b' => Some('\u{8}'), - 'f' => Some('\u{c}'), - 'n' => Some('\n'), - 'r' => Some('\r'), - 't' => Some('\t'), - _ => None, - }; - if let Some(simple) = simple { - decoded.push(simple); - index += 2; - continue; - } - let width: usize = match escaped { - 'u' => 4, - 'U' => 8, - _ => 0, - }; - let digits: &[char] = super::text::slice(value, index + 2, index + 2 + width); - if width > 0 && digits.len() == width && digits.iter().all(char::is_ascii_hexdigit) { - let codepoint: Option = match u32::from_str_radix(&string(digits), 16) { - Ok(codepoint) if codepoint <= MAX_CODEPOINT && !SURROGATES.contains(&codepoint) => { - char::from_u32(codepoint) - } - Ok(_) | Err(_) => None, - }; - if let Some(decoded_character) = codepoint { - decoded.push(decoded_character); - index += width + 2; - continue; - } - } - decoded.extend([ESCAPE, escaped]); - index += 2; - } - decoded -} - -/// `value` with backslashes and `quote` escaped for a quoted header value. -pub(crate) fn escape_quoted(value: &[char], quote: char) -> Vec { - let mut escaped: Vec = Vec::with_capacity(value.len()); - for character in value { - if *character == ESCAPE || *character == quote { - escaped.push(ESCAPE); - } - escaped.push(*character); - } - escaped -} - /// `text` split on newlines that are outside quoted values. pub(crate) fn split_outside_quotes(text: &[char]) -> Vec<&[char]> { let mut lines: Vec<&[char]> = Vec::new(); let mut line_start: usize = 0; - let mut in_quote: Option = None; let mut index: usize = 0; while index < text.len() { let character: char = text[index]; - if let Some(quote) = in_quote { - if character == ESCAPE && index + 1 < text.len() { - index += 2; - continue; - } - if character == quote { - in_quote = None; - } - index += 1; + if QUOTES.contains(&character) { + index = quoted_value_end(text, index); continue; } - if QUOTES.contains(&character) { - in_quote = Some(character); - } else if character == '\n' { + if character == '\n' { lines.push(&text[line_start..index]); line_start = index + 1; } diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/rules.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/rules.rs index 5531a419de..fb563d9cc5 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/rules.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/rules.rs @@ -1,10 +1,12 @@ //! The header rules of one header: parseable values, a short present description, no trailing spaces. -use crate::dsl_headers::_helpers::quoted::{ - decode_description, description_value_span, split_outside_quotes, +use sqlbuild_sqltext::header_strings::main::prose_string::prose_string; + +use crate::dsl_headers::_helpers::prose_layout::{description_string_span, prose_length}; +use crate::dsl_headers::_helpers::quoted::split_outside_quotes; +use crate::dsl_headers::_helpers::text::{ + chars, line_count, position, rstrip, slice, string, strip, }; -use crate::dsl_headers::_helpers::text::{line_count, position, rstrip, slice, string, strip}; -use crate::dsl_headers::_helpers::wrap::wrap_header_description; use crate::dsl_headers::constants::{ DESCRIPTION_HEADER_KINDS, DESCRIPTION_REQUIRED_HEADER_KINDS, RULE_DESCRIPTION_LENGTH, RULE_DESCRIPTION_PRESENT, RULE_HEADER_PARSE, RULE_HEADER_WHITESPACE, SQL_MACRO_HEADER_KIND, @@ -33,11 +35,22 @@ pub(crate) fn violation_at( } } -fn parse_fault(starts: &[usize], header: &HeaderSpan, failure: ParseFailure) -> HeaderViolation { +fn parse_fault( + (text, starts): (&[char], &[usize]), + header: &HeaderSpan, + failure: ParseFailure, +) -> HeaderViolation { let kind: &str = &header.kind; + let open: Option = slice(text, header.start, header.end) + .iter() + .position(|character| *character == '('); + let offset: usize = match (failure.position, open) { + (Some(position), Some(open)) => header.start + open + 1 + position, + _ => header.start, + }; violation_at( starts, - header.start, + offset, (RULE_HEADER_PARSE, HeaderSeverity::Fault), ( format!("{kind}() header could not be parsed: {}", failure.message), @@ -62,25 +75,23 @@ pub(crate) fn value_violations( let values: P::Values = match parser.parse(kind, &string(inner_header_text(header_text))) { ParseOutcome::Parsed(values) => values, ParseOutcome::Rejected(failure) | ParseOutcome::Raised(_, failure) => { - return Ok(vec![parse_fault(starts, header, failure)]); + return Ok(vec![parse_fault((text, starts), header, failure)]); } }; let mut violations: Vec = Vec::new(); let described: bool = DESCRIPTION_HEADER_KINDS.contains(&kind); let span: Option<(usize, usize)> = described - .then(|| description_value_span(header_text)) + .then(|| description_string_span(header_text)) .flatten(); - let description: Option> = match (parser.description(&values)?, span) { - (Some(_), Some((start, end))) => Some(decode_description( - slice(header_text, start, end), - header_text[start - 1], - )), - (description, _) => description.map(|description| description.chars().collect()), + let authored: Option = span.and_then(|(start, _)| authored_prose(header_text, start)); + let description: Option = match parser.description(&values)? { + Some(_) if authored.is_some() => authored, + description => description, }; if DESCRIPTION_REQUIRED_HEADER_KINDS.contains(&kind) && description .as_deref() - .is_none_or(|description| strip(description).is_empty()) + .is_none_or(|description| strip(&chars(description)).is_empty()) { violations.push(violation_at( starts, @@ -96,14 +107,12 @@ pub(crate) fn value_violations( )); } if described && let Some(description) = description { - let formatted: Vec = match span { - Some(span) => { - wrap_header_description(&description, header_text, span, config.line_width) - } - None => description, + let lines: usize = match span { + Some(span) => prose_length(&description, header_text, span, config.line_width), + None => line_count(&chars(&description)), }; let limit: usize = config.max_description_lines; - if line_count(&formatted) > limit { + if lines > limit { violations.push(violation_at( starts, header.start, @@ -138,3 +147,11 @@ pub(crate) fn whitespace_violation( ) }) } + +/// The folded text of the prose string at `start`, if it reads. +fn authored_prose(header_text: &[char], start: usize) -> Option { + let Ok(prose) = prose_string(header_text, start) else { + return None; + }; + Some(prose.value) +} diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs index e87e7c25da..f07338d232 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs @@ -1,9 +1,10 @@ //! Line-leading parenthesized DSL regions, found past comments and SQL strings. use sqlbuild_core::text::main::python_isspace::python_isspace; +use sqlbuild_sqltext::header_strings::main::header_quoted_end::header_quoted_end; use crate::dsl_headers::_helpers::text::{find, is_word}; -use crate::dsl_headers::constants::{ESCAPE, QUOTES}; +use crate::dsl_headers::constants::QUOTES; use crate::dsl_headers::models::HeaderSpan; /// The end of the SQL string quoted by `quote` that opens at `start`; `''` escapes a quote. @@ -72,23 +73,15 @@ fn header_end(text: &[char], body_start: usize) -> Option { return None; } let mut depth: i64 = 0; - let mut in_quote: Option = None; loop { - let found: usize = (index..text.len()).find(|position| match in_quote { - None => matches!(text[*position], '(' | ')' | '\'' | '"'), - Some(quote) => text[*position] == ESCAPE || text[*position] == quote, - })?; - index = found; + index = (index..text.len()) + .find(|position| matches!(text[*position], '(' | ')' | '\'' | '"'))?; let character: char = text[index]; - if in_quote.is_some() { - if character == ESCAPE { - index += 2; - continue; - } - in_quote = None; - } else if QUOTES.contains(&character) { - in_quote = Some(character); - } else if character == '(' { + if QUOTES.contains(&character) { + index = header_quoted_end(text, index)?; + continue; + } + if character == '(' { depth += 1; } else { depth -= 1; diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/wrap.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/wrap.rs index 9d33c62f64..79969b7f23 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/wrap.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/wrap.rs @@ -2,7 +2,7 @@ use sqlbuild_core::text::main::python_isspace::python_isspace; -use crate::dsl_headers::_helpers::text::{chars, find, rfind_before, rstrip, slice, strip}; +use crate::dsl_headers::_helpers::text::{chars, strip}; fn is_ascii_space(character: char) -> bool { matches!(character, '\t' | '\n' | '\u{b}' | '\u{c}' | '\r' | ' ') @@ -110,23 +110,3 @@ pub(crate) fn wrap_description(description: &[char], first_width: i64, width: i6 } wrapped.join(&['\n', '\n'][..]) } - -/// A description wrapped to fit the header line holding its value at `span`. -pub(crate) fn wrap_header_description( - description: &[char], - header_text: &[char], - (value_start, value_end): (usize, usize), - line_width: i64, -) -> Vec { - let line_start: usize = - rfind_before(header_text, '\n', value_start).map_or(0, |found| found + 1); - let line_end: usize = find(header_text, &['\n'], value_end).unwrap_or(header_text.len()); - let suffix: i64 = - i64::try_from(rstrip(slice(header_text, value_end, line_end)).len()).unwrap_or(i64::MAX); - let prefix: i64 = i64::try_from(value_start - line_start).unwrap_or(i64::MAX); - wrap_description( - description, - (line_width - prefix - suffix).max(1), - (line_width - suffix).max(1), - ) -} diff --git a/crates/sqlbuild-rules/src/dsl_headers/constants.rs b/crates/sqlbuild-rules/src/dsl_headers/constants.rs index b06d93ce6c..096b32df99 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/constants.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/constants.rs @@ -5,6 +5,9 @@ pub const HEADER_KINDS: [&str; 10] = [ ]; pub const MEASUREMENT_KINDS: [&str; 2] = ["EVIDENCE", "MEASURE"]; pub(crate) const DESCRIPTION_HEADER_KINDS: [&str; 2] = ["MODEL", "SCENARIO"]; +/// Headers whose top-level description `sqb format` lays out; `MACRO` is laid out after its header. +pub(crate) const PROSE_LAYOUT_HEADER_KINDS: [&str; 5] = + ["FUNCTION", "HOOK", "MODEL", "SCENARIO", "SCHEMA"]; pub(crate) const DESCRIPTION_REQUIRED_HEADER_KINDS: [&str; 4] = ["MODEL", "SCENARIO", "FUNCTION", "HOOK"]; pub(crate) const MODEL_KIND: &str = "MODEL"; @@ -21,4 +24,3 @@ pub(crate) const HEADER_TERMINATOR: &str = ";"; pub(crate) const AUDITS_KEYWORD: &str = "audits"; pub(crate) const AUDIT_WRAP_MAX_PASSES: usize = 64; pub(crate) const QUOTES: [char; 2] = ['\'', '"']; -pub(crate) const ESCAPE: char = '\\'; diff --git a/crates/sqlbuild-rules/src/dsl_headers/main/fix_escaped_quotes.rs b/crates/sqlbuild-rules/src/dsl_headers/main/fix_escaped_quotes.rs new file mode 100644 index 0000000000..a121067234 --- /dev/null +++ b/crates/sqlbuild-rules/src/dsl_headers/main/fix_escaped_quotes.rs @@ -0,0 +1,27 @@ +//! `fix_escaped_quotes`: descriptions escaping or doubling quotes rewritten as `"""` strings. + +use crate::dsl_headers::_helpers::quote_fix::header_fixes; +use crate::dsl_headers::_helpers::text::{line_starts, position}; +use crate::dsl_headers::main::scan_headers::scan_headers; + +/// `text` with each description escaping or doubling quotes rewritten as `"""..."""`, and lines. +pub fn fix_escaped_quotes(text: &[char]) -> (Vec, Vec) { + let starts: Vec = line_starts(text); + let mut fixes: Vec<(usize, usize, Vec)> = Vec::new(); + for header in scan_headers(text, false) { + fixes.extend(header_fixes(text, &header)); + } + let lines: Vec = fixes + .iter() + .map(|(start, _, _)| position(*start, &starts).0) + .collect(); + let mut fixed: Vec = Vec::with_capacity(text.len()); + let mut cursor: usize = 0; + for (start, end, rewrite) in fixes { + fixed.extend(&text[cursor..start]); + fixed.extend(rewrite); + cursor = end; + } + fixed.extend(&text[cursor..]); + (fixed, lines) +} 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 a6e30fc6b7..00fa69e00a 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/main/format_headers.rs @@ -3,7 +3,9 @@ use crate::dsl_headers::_helpers::formatting::{ relocate_leading_comment, without_trailing_whitespace, wrap_audits, wrap_descriptions, }; +use crate::dsl_headers::_helpers::macro_header::changes_macro; use crate::dsl_headers::_helpers::macro_header::layout_macro_headers; +use crate::dsl_headers::constants::{PROSE_LAYOUT_HEADER_KINDS, SQL_MACRO_HEADER_KIND}; use crate::dsl_headers::main::scan_headers::scan_headers; use crate::dsl_headers::models::{HeaderRulesConfig, HeaderSpan, HeaderViolation}; use crate::dsl_headers::types::HeaderParser; @@ -22,12 +24,29 @@ pub fn format_headers( } else { scan_headers(&relocated, false) }; - let wrapped: Vec = - wrap_descriptions(&relocated, &relocated_headers, config.line_width, parser)?; + let wrapped: Vec = wrap_descriptions( + &relocated, + &relocated_headers, + (&PROSE_LAYOUT_HEADER_KINDS, config.line_width), + parser, + )?; let audited: Vec = wrap_audits(&wrapped, config.line_width, parser)?; let macro_headers: Vec = scan_headers(&audited, false); + let mut laid_out: Vec = layout_macro_headers(&audited, ¯o_headers, config.line_width); + let defines_macro: bool = macro_headers + .iter() + .any(|header| header.kind == SQL_MACRO_HEADER_KIND); + if defines_macro && changes_macro(&audited, &laid_out) { + laid_out = audited; + } + let laid_out_headers: Vec = scan_headers(&laid_out, false); Ok(( - layout_macro_headers(&audited, ¯o_headers, config.line_width), + wrap_descriptions( + &laid_out, + &laid_out_headers, + (&[SQL_MACRO_HEADER_KIND], config.line_width), + parser, + )?, faults, )) } diff --git a/crates/sqlbuild-rules/src/dsl_headers/main/mod.rs b/crates/sqlbuild-rules/src/dsl_headers/main/mod.rs index 327f9ec2f3..63a895c416 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/main/mod.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/main/mod.rs @@ -1,3 +1,4 @@ +pub mod fix_escaped_quotes; pub mod format_headers; pub mod inner_header_text; pub mod lint_headers; diff --git a/crates/sqlbuild-rules/src/dsl_headers/models.rs b/crates/sqlbuild-rules/src/dsl_headers/models.rs index 235d36f011..ba74096e7a 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/models.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/models.rs @@ -38,6 +38,8 @@ pub struct HeaderRulesConfig { pub struct ParseFailure { pub message: String, pub help: Option, + /// The character offset in the text between the parentheses the error is at, if known. + pub position: Option, } /// One header parse: values, a header syntax error, or another error the caller raised. diff --git a/crates/sqlbuild-rules/src/dsl_headers/tests/helpers.rs b/crates/sqlbuild-rules/src/dsl_headers/tests/helpers.rs index 23b2e444fe..17c4b61349 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/tests/helpers.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/tests/helpers.rs @@ -1,3 +1,58 @@ +use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; +use sqlbuild_sqltext::compiler::models::AuthoredValue; + +use crate::dsl_headers::main::inner_header_text::inner_header_text; +use crate::dsl_headers::main::lint_headers::lint_headers; +use crate::dsl_headers::main::scan_headers::scan_headers; +use crate::dsl_headers::models::HeaderRulesConfig; +use crate::dsl_headers::tests::test_types::NativeParser; + pub(crate) fn chars(text: &str) -> Vec { text.chars().collect() } + +/// The description each header of `text` stores, in order. +pub(crate) fn stored_descriptions(text: &[char]) -> Vec> { + scan_headers(text, false) + .iter() + .map(|header| { + let inner: String = inner_header_text(&text[header.start..header.end]) + .iter() + .collect(); + let (Some(AuthoredValue::Map(entries)), _, None) = parse_one(&inner) else { + return None; + }; + entries + .iter() + .find(|(key, _)| key == "description") + .and_then(|(_, value)| description_text(value)) + }) + .collect() +} + +/// The text of a `description` value, if it is a string. +fn description_text(value: &AuthoredValue) -> Option { + let AuthoredValue::String(text) = value else { + return None; + }; + Some(text.clone()) +} + +/// The codes of the header violations of `text` at `(line_width, max_description_lines)`. +pub(crate) fn lint_codes( + text: &str, + (line_width, max_description_lines): (i64, usize), +) -> Vec<&'static str> { + let characters: Vec = chars(text); + let config = HeaderRulesConfig { + line_width, + max_description_lines, + }; + let Ok(violations) = lint_headers( + &characters, + &scan_headers(&characters, false), + config, + &NativeParser, + ); + violations.iter().map(|violation| violation.code).collect() +} diff --git a/crates/sqlbuild-rules/src/dsl_headers/tests/test_dsl_headers.rs b/crates/sqlbuild-rules/src/dsl_headers/tests/test_dsl_headers.rs index 831e793f98..ad0f2c0f42 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/tests/test_dsl_headers.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/tests/test_dsl_headers.rs @@ -1,13 +1,14 @@ -use crate::dsl_headers::_helpers::quoted::decode_description; use crate::dsl_headers::_helpers::wrap::wrap_description; +use crate::dsl_headers::main::fix_escaped_quotes::fix_escaped_quotes; use crate::dsl_headers::main::format_headers::format_headers; use crate::dsl_headers::main::measurement_body_ranges::measurement_body_ranges; use crate::dsl_headers::main::scan_headers::scan_headers; use crate::dsl_headers::main::sql_body_ranges::sql_body_ranges; use crate::dsl_headers::models::HeaderRulesConfig; -use crate::dsl_headers::tests::helpers::chars; +use crate::dsl_headers::tests::helpers::{chars, lint_codes, stored_descriptions}; use crate::dsl_headers::tests::test_types::{ - CompactParser, FormatTestCase, ProseTestCase, ScanTestCase, + CompactParser, FormatTestCase, LengthTestCase, NativeParser, ProseTestCase, QuoteFixTestCase, + ScanTestCase, }; const MODEL_FILE: &str = "-- leading note\n-- second line\nMODEL (\n name orders, \n audits [not_null(column 'order_id'), unique(column 'order_id'), accepted_values(column 'status', values ['a', 'b'])]\n);\nSELECT 1 /* MODEL ( */ , 'MODEL (x)'\n TEST (\"a\\\")\" ) ;\nSELECT 2\n"; @@ -123,30 +124,259 @@ fn given_sql_macro_headers_when_formatting_then_each_is_laid_out_canonically_and } #[test] -fn given_description_text_when_wrapping_and_decoding_then_matches_python() { +fn given_description_text_when_wrapping_then_matches_python() { let test_cases = [ProseTestCase { - description: "paragraphs, an unbreakable word and every escape form", + description: "paragraphs and an unbreakable word", prose: " The orders\tmodel joins every customer to supercalifragilisticexpialidociously long words.\n \n Second paragraph here.", - escaped: r#"a\"b\\c\nd\u00e9\U0001F600\uD800\x"#, expected_wrapped: "The orders model\njoins every customer to\nsupercalifragilisticexpialidociously\nlong words.\n\nSecond paragraph here.", - expected_decoded: "a\"b\\c\nd\u{e9}\u{1f600}\\uD800\\x", }]; for test_case in test_cases { let wrapped: String = wrap_description(&chars(test_case.prose), 20, 30) .into_iter() .collect(); - let decoded: String = decode_description(&chars(test_case.escaped), '"') - .into_iter() - .collect(); assert_eq!( wrapped, test_case.expected_wrapped, "{}", test_case.description ); + } +} + +#[test] +fn given_descriptions_when_formatting_then_layout_is_canonical_idempotent_and_text_unchanged() { + let test_cases = [ + FormatTestCase { + description: "a short wrapped description is joined onto one line", + text: "MODEL (\n description \"Orders placed\n by day.\",\n materialized table,\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"Orders placed by day.\",\n materialized table,\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a description that fits stays one line", + text: "MODEL (\n description \"Orders placed by day.\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"Orders placed by day.\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a long description becomes an indented block wrapped at the width", + text: "MODEL (\n description \"True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case, with or without a dot.\",\n materialized table,\n);\nSELECT 1\n", + line_width: 60, + expected_text: "MODEL (\n description \"\"\"\n True when a name starts with a woman's title: Mme, Mlle,\n Miss, Mrs, Ms. Any case, with or without a dot.\n \"\"\",\n materialized table,\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "continuation lines at column zero become a block", + text: "MODEL (\n description \"True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case,\nwith or without a dot ('MISS ANNA PARKER', 'Mrs. C. O'Brien', 'Ms.Robinson').\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"\"\"\n True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case, with or\n without a dot ('MISS ANNA PARKER', 'Mrs. C. O'Brien', 'Ms.Robinson').\n \"\"\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a short description with double quotes uses one-line triple quotes", + text: "MODEL (\n description 'Matches \"Miss\".',\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"\"\"Matches \"Miss\".\"\"\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a long description with double quotes becomes a block", + text: "MODEL (\n description \"\"\"True when a name starts with a title, with or without a dot (\"MISS ANNA PARKER\", \"Mrs. C. O'Brien\").\"\"\",\n);\nSELECT 1\n", + line_width: 80, + expected_text: "MODEL (\n description \"\"\"\n True when a name starts with a title, with or without a dot (\"MISS ANNA\n PARKER\", \"Mrs. C. O'Brien\").\n \"\"\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "paragraphs stay separated by a blank line", + text: "MODEL (\n description \"First paragraph about orders.\n\n Second paragraph about refunds.\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"\"\"\n First paragraph about orders.\n\n Second paragraph about refunds.\n \"\"\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a one-line triple-quoted description without quotes becomes plain", + text: "MODEL (\n description \"\"\"Orders by day.\"\"\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"Orders by day.\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a one-line single-quoted description that fits is kept", + text: "MODEL (\n description 'Orders by day.',\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description 'Orders by day.',\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "an escaped quote is left for format --fix", + text: "MODEL (\n description \"Matches \\\"Miss\\\".\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"Matches \\\"Miss\\\".\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a long MACRO description becomes a block under its laid-out header", + text: "MACRO (description \"True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case.\", arguments (value VARCHAR), returns BOOLEAN);\n\n@value IS NOT NULL\n", + line_width: 60, + expected_text: "MACRO (\n description \"\"\"\n True when a name starts with a woman's title: Mme, Mlle,\n Miss, Mrs, Ms. Any case.\n \"\"\",\n arguments (value VARCHAR),\n returns BOOLEAN,\n);\n\n@value IS NOT NULL\n", + }, + FormatTestCase { + description: "a long MACRO description with a doubled quote keeps its text", + text: "MACRO (\n description 'it''s a word word word word word word word word word end',\n arguments (v VARCHAR),\n returns VARCHAR,\n);\n\nLOWER(@v)\n", + line_width: 50, + expected_text: "MACRO (\n description \"\"\"\n it's a word word word word word word word word\n word end\n \"\"\",\n arguments (v VARCHAR),\n returns VARCHAR,\n);\n\nLOWER(@v)\n", + }, + FormatTestCase { + description: "a long model description with an escaped single quote keeps its text", + text: "MODEL (\n description 'it\\'s a word word word word word word word word word end',\n);\nSELECT 1\n", + line_width: 50, + expected_text: "MODEL (\n description \"\"\"\n it's a word word word word word word word word\n word end\n \"\"\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a description ending in a backslash stays as written", + text: "MODEL (\n description \"Files under C:\\\",\n);\nSELECT 1\n", + line_width: 100, + expected_text: "MODEL (\n description \"Files under C:\\\",\n);\nSELECT 1\n", + }, + FormatTestCase { + description: "a short description stays when later fields make its line long", + text: "FUNCTION (description 'Test function is_large_order.', arguments (amount_cents INTEGER), returns BOOLEAN);\n\namount_cents > 100\n", + line_width: 100, + expected_text: "FUNCTION (description 'Test function is_large_order.', arguments (amount_cents INTEGER), returns BOOLEAN);\n\namount_cents > 100\n", + }, + FormatTestCase { + description: "a description too long by itself becomes a block before later fields", + text: "FUNCTION (description \"Whether an order is large enough to need review.\", returns BOOLEAN);\n\nTRUE\n", + line_width: 50, + expected_text: "FUNCTION (description \"\"\"\n Whether an order is large enough to need review.\n\"\"\", returns BOOLEAN);\n\nTRUE\n", + }, + FormatTestCase { + description: "a SCHEMA description is laid out", + text: "SCHEMA (\n name order_columns,\n description \"Columns every\n order model shares.\",\n columns (order_id (type BIGINT)),\n);\n", + line_width: 100, + expected_text: "SCHEMA (\n name order_columns,\n description \"Columns every order model shares.\",\n columns (order_id (type BIGINT)),\n);\n", + }, + ]; + + for test_case in test_cases { + let config = HeaderRulesConfig { + line_width: test_case.line_width, + max_description_lines: 10, + }; + let text: Vec = chars(test_case.text); + let Ok((formatted, _)) = format_headers(&text, config, &NativeParser); + let Ok((again, _)) = format_headers(&formatted, config, &NativeParser); + + assert_eq!( + formatted.iter().collect::(), + test_case.expected_text, + "{}", + test_case.description + ); + assert_eq!(again, formatted, "idempotent: {}", test_case.description); + assert_eq!( + stored_descriptions(&formatted), + stored_descriptions(&text), + "stored text unchanged: {}", + test_case.description + ); + } +} + +#[test] +fn given_escaped_quotes_in_descriptions_when_fixing_then_triple_quotes_hold_real_quotes() { + let test_cases = [ + QuoteFixTestCase { + description: "a model description", + text: "MODEL (\n description \"Matches a title (\\\"Miss\\\").\",\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"\"\"Matches a title (\"Miss\").\"\"\",\n);\nSELECT 1\n", + expected_lines: vec![2], + }, + QuoteFixTestCase { + description: "a wrapped macro description with apostrophes", + text: "MACRO (\n description \"A woman's title,\nwith a dot (\\\"Mrs. C. O'Brien\\\").\",\n returns VARCHAR,\n);\n'x'\n", + expected_text: "MACRO (\n description \"\"\"A woman's title,\nwith a dot (\"Mrs. C. O'Brien\").\"\"\",\n returns VARCHAR,\n);\n'x'\n", + expected_lines: vec![2], + }, + QuoteFixTestCase { + description: "a column description, and text ending in a quote", + text: "MODEL (\n description \"Orders.\",\n columns (title (description \"Says \\\"hi\\\"\")),\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"Orders.\",\n columns (title (description \"\"\"Says \"hi\"\"\"\")),\n);\nSELECT 1\n", + expected_lines: vec![3], + }, + QuoteFixTestCase { + description: "doubled quotes in a model description", + text: "MODEL (\n description \"say \"\"hi\"\" now\",\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"\"\"say \"hi\" now\"\"\",\n);\nSELECT 1\n", + expected_lines: vec![2], + }, + QuoteFixTestCase { + description: "doubled quotes in a macro description", + text: "MACRO (\n description \"say \"\"hi\"\" now\",\n returns VARCHAR,\n);\n'x'\n", + expected_text: "MACRO (\n description \"\"\"say \"hi\" now\"\"\",\n returns VARCHAR,\n);\n'x'\n", + expected_lines: vec![2], + }, + QuoteFixTestCase { + description: "a description that already reads is left alone", + text: "MODEL (\n description \"Files under C:\\\",\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"Files under C:\\\",\n);\nSELECT 1\n", + expected_lines: vec![], + }, + QuoteFixTestCase { + description: "another backslash escape is not rewritten", + text: "MODEL (\n description \"Path C:\\\\data \\\"x\\\"\",\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"Path C:\\\\data \\\"x\\\"\",\n);\nSELECT 1\n", + expected_lines: vec![], + }, + QuoteFixTestCase { + description: "text that would hold three quotes in a row is not rewritten", + text: "MODEL (\n description \"a \\\"\\\"\\\" b\",\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"a \\\"\\\"\\\" b\",\n);\nSELECT 1\n", + expected_lines: vec![], + }, + QuoteFixTestCase { + description: "a SQL string that escapes a quote is left alone", + text: "MODEL (\n description \"Orders.\",\n audits [expression(predicate \"name <> \\\"x\\\"\")],\n);\nSELECT 1\n", + expected_text: "MODEL (\n description \"Orders.\",\n audits [expression(predicate \"name <> \\\"x\\\"\")],\n);\nSELECT 1\n", + expected_lines: vec![], + }, + ]; + + for test_case in test_cases { + let (fixed, lines) = fix_escaped_quotes(&chars(test_case.text)); + + assert_eq!( + fixed.iter().collect::(), + test_case.expected_text, + "{}", + test_case.description + ); + assert_eq!(lines, test_case.expected_lines, "{}", test_case.description); + } +} + +#[test] +fn given_description_near_the_line_limit_when_linting_then_block_layout_counts_as_before() { + let words: &str = + "aaaa bbbb cccc dddd eeee ffff gggg hhhh iiii jjjj kkkk llll mmmm nnnn oooo pppp qqqq rrrr"; + let test_cases = [ + LengthTestCase { + description: "three lines wide at the full width stay within the limit", + text: format!("MODEL (\n description \"{words}\",\n);\nSELECT 1\n"), + expected_too_long: false, + }, + LengthTestCase { + description: "the same words laid out as an indented block stay within the limit", + text: "MODEL (\n description \"\"\"\n aaaa bbbb cccc dddd eeee ffff gggg\n hhhh iiii jjjj kkkk llll mmmm nnnn\n oooo pppp qqqq rrrr\n \"\"\",\n);\nSELECT 1\n" + .to_owned(), + expected_too_long: false, + }, + LengthTestCase { + description: "more words than fit in three lines exceed the limit", + text: format!( + "MODEL (\n description \"{words} ssss tttt uuuu vvvv\",\n);\nSELECT 1\n" + ), + expected_too_long: true, + }, + ]; + + for test_case in test_cases { assert_eq!( - decoded, test_case.expected_decoded, + lint_codes(&test_case.text, (40, 3)).contains(&"description-length"), + test_case.expected_too_long, "{}", test_case.description ); diff --git a/crates/sqlbuild-rules/src/dsl_headers/tests/test_types.rs b/crates/sqlbuild-rules/src/dsl_headers/tests/test_types.rs index 47af0e7211..4a1d24ae42 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/tests/test_types.rs @@ -1,8 +1,44 @@ use std::convert::Infallible; -use crate::dsl_headers::models::ParseOutcome; +use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; +use sqlbuild_sqltext::compiler::models::AuthoredValue; + +use crate::dsl_headers::models::{ParseFailure, ParseOutcome}; use crate::dsl_headers::types::HeaderParser; +/// Parses a header with the compiler's header parser. +pub(crate) struct NativeParser; + +impl HeaderParser for NativeParser { + type Values = AuthoredValue; + type Error = Infallible; + + fn parse(&self, _kind: &str, inner: &str) -> ParseOutcome { + match parse_one(inner) { + (Some(value), _, None) => ParseOutcome::Parsed(value), + (_, _, error) => ParseOutcome::Rejected(ParseFailure { + message: error.unwrap_or_default(), + help: None, + position: None, + }), + } + } + + fn description(&self, values: &AuthoredValue) -> Result, Infallible> { + Ok(match values { + AuthoredValue::Map(entries) => entries.iter().find_map(|(key, value)| match value { + AuthoredValue::String(text) if key == "description" => Some(text.clone()), + _ => None, + }), + _ => None, + }) + } + + fn same(&self, left: &AuthoredValue, right: &AuthoredValue) -> Result { + Ok(left == right) + } +} + /// Parses a header to its text without whitespace or trailing commas. pub(crate) struct CompactParser; @@ -45,7 +81,18 @@ pub(crate) struct FormatTestCase { pub(crate) struct ProseTestCase { pub(crate) description: &'static str, pub(crate) prose: &'static str, - pub(crate) escaped: &'static str, pub(crate) expected_wrapped: &'static str, - pub(crate) expected_decoded: &'static str, +} + +pub(crate) struct QuoteFixTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) expected_text: &'static str, + pub(crate) expected_lines: Vec, +} + +pub(crate) struct LengthTestCase { + pub(crate) description: &'static str, + pub(crate) text: String, + pub(crate) expected_too_long: bool, } diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/matching.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/matching.rs index b6e8ff5832..889673043c 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/matching.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/matching.rs @@ -1,4 +1,7 @@ -//! Exact native equivalent of the possessive Python `MODEL(...)` header regular expression. +//! The possessive `MODEL(...)` header match, with `"""..."""` header strings. + +use crate::header_strings::main::header_quoted_end::header_quoted_end; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; /// Offsets of one matched header: header start, header end, and SQL start. pub(crate) type HeaderMatchOffsets = (usize, usize, usize); @@ -50,6 +53,9 @@ fn scan_header_body(text: &str, start: usize) -> usize { let mut index = start; while let Some(&byte) = bytes.get(index) { index = match byte { + b'"' if opens_triple_quote(bytes, index) => { + header_quoted_end(bytes, index).unwrap_or_else(|| unclosed_quote_end(text, index)) + } b'"' | b'\'' => quoted_string_end(text, index, byte) .unwrap_or_else(|| unclosed_quote_end(text, index)), b')' => { diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs index 729b8f901b..7b6d9d100f 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs @@ -8,6 +8,18 @@ use crate::constants::{ COLUMNS_KEY, CONSTANT_CALL, INLINE_SQL_HOOK, MAX_HEADER_NESTING_DEPTH, OUTSIDE_KEY, SQL_HOOK, THRESHOLDS_KEY, TYPE_KEY, }; +use crate::header_strings::constants::{ + AUDITS_KEY, DESCRIPTION_KEY, POSITION_SUFFIX, TRIPLE_QUOTE_INSIDE, TRIPLE_QUOTE_LENGTH, + UNCLOSED_TRIPLE_QUOTE, +}; +use crate::header_strings::errors::TripleQuoteError; +use crate::header_strings::main::closes_inside_text::closes_inside_text; +use crate::header_strings::main::follows_description_key::follows_description_key; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; +use crate::header_strings::main::prose_end::prose_end; +use crate::header_strings::main::prose_error_message::prose_error_message; +use crate::header_strings::main::prose_string::prose_string; +use crate::header_strings::main::triple_quoted_end::triple_quoted_end; use sqlbuild_core::constants::{CLOSE_PAREN, OPEN_PAREN}; pub(crate) const MAX_TOKENIZER_WORKERS: usize = 4; @@ -32,14 +44,16 @@ pub(crate) fn parse_batch(headers: &[String]) -> Result, } pub(crate) fn parse_one(header: &str) -> HeaderParseResult { - match tokenize(header).and_then(HeaderParser::parse) { + let characters: Vec = header.chars().collect(); + match tokenize(&characters).and_then(|tokens| HeaderParser::parse(tokens, &characters)) { Ok((value, offsets)) => (Some(value), Some(offsets), None), Err(error) => (None, None, Some(error)), } } pub(crate) fn tokenize_one(header: &str) -> Result, String> { - tokenize(header) + let characters: Vec = header.chars().collect(); + tokenize(&characters) } pub(crate) fn build_tokenizer_pool(header_count: usize) -> Result { @@ -50,8 +64,10 @@ pub(crate) fn build_tokenizer_pool(header_count: usize) -> Result { tokens: Vec, + /// The header text the token positions index, which prose values are read from. + characters: &'a [char], index: usize, column_offsets: Vec, /// The open containers, at most `MAX_HEADER_NESTING_DEPTH`, which bounds the recursion. @@ -65,6 +81,10 @@ struct ParseValueOptions { nested_columns: bool, nested_metadata: bool, record_nested_columns: bool, + /// The value of an `audits` key, whose audit calls have a prose `description` option. + audit_list: bool, + /// An item of an `audits` list. + audit_call: bool, } #[derive(Clone, Copy, Default)] @@ -75,12 +95,18 @@ struct ParseMapOptions<'a> { column_entries: bool, column_metadata: bool, record_column_entries: bool, + /// Whether a `description` string here is prose, stored folded. + prose_descriptions: bool, } -impl HeaderParser { - fn parse(tokens: Vec) -> Result<(AuthoredValue, Vec), String> { +impl<'a> HeaderParser<'a> { + fn parse( + tokens: Vec, + characters: &'a [char], + ) -> Result<(AuthoredValue, Vec), String> { let mut parser = Self { tokens, + characters, index: 0, column_offsets: Vec::new(), depth: 0, @@ -88,7 +114,10 @@ impl HeaderParser { let value = if parser.peek().0 == END_TOKEN { AuthoredValue::Map(Vec::new()) } else { - AuthoredValue::Map(parser.parse_map(ParseMapOptions::default())?) + AuthoredValue::Map(parser.parse_map(ParseMapOptions { + prose_descriptions: true, + ..ParseMapOptions::default() + })?) }; parser.expect_end()?; Ok((value, parser.column_offsets)) @@ -128,6 +157,12 @@ impl HeaderParser { self.parse_hook_field(&key)? } else if options.column_metadata && key == TYPE_KEY { self.parse_type()? + } else if options.prose_descriptions + && key == DESCRIPTION_KEY + && self.peek().0 == STRING_TOKEN + { + let position: usize = self.advance().2; + AuthoredValue::String(prose_string(self.characters, position)?.value) } else { self.parse_value(ParseValueOptions { threshold_policy: key == THRESHOLDS_KEY, @@ -136,6 +171,8 @@ impl HeaderParser { nested_columns: key == COLUMNS_KEY, nested_metadata: options.column_entries, record_nested_columns: options.end.is_none() && key == COLUMNS_KEY, + audit_list: key == AUDITS_KEY, + audit_call: false, })? }; values.push((key, value)); @@ -205,14 +242,18 @@ impl HeaderParser { } return Ok(parse_word(token.1)); } + let items: ParseValueOptions = ParseValueOptions { + audit_call: options.audit_list, + ..ParseValueOptions::default() + }; if self.match_symbol("[") { return self.nested(token.2, |parser| { - Ok(AuthoredValue::List(parser.parse_sequence("]")?)) + Ok(AuthoredValue::List(parser.parse_sequence("]", items)?)) }); } if self.match_symbol("{") { return self.nested(token.2, |parser| { - Ok(AuthoredValue::Set(parser.parse_sequence("}")?)) + Ok(AuthoredValue::Set(parser.parse_sequence("}", items)?)) }); } if self.match_symbol("(") { @@ -224,6 +265,7 @@ impl HeaderParser { column_entries: options.nested_columns, column_metadata: options.nested_metadata, record_column_entries: options.record_nested_columns, + prose_descriptions: options.nested_metadata, })?)) }); } @@ -255,6 +297,7 @@ impl HeaderParser { end: Some(")"), threshold_policy: options.threshold_policy, allow_outside: options.allow_outside, + prose_descriptions: options.audit_call, ..ParseMapOptions::default() }) })?; @@ -279,13 +322,17 @@ impl HeaderParser { value } - fn parse_sequence(&mut self, end: &str) -> Result, String> { + fn parse_sequence( + &mut self, + end: &str, + items: ParseValueOptions, + ) -> Result, String> { let mut values: Vec = Vec::new(); while !self.at_end_symbol(Some(end)) { if self.match_symbol(",") { continue; } - values.push(self.parse_value(ParseValueOptions::default())?); + values.push(self.parse_value(items)?); self.match_symbol(","); } self.consume_symbol(end)?; @@ -509,76 +556,145 @@ fn parse_word(value: String) -> AuthoredValue { } } -fn tokenize(header: &str) -> Result, String> { - let characters: Vec = header.chars().collect(); - let mut tokens: Vec = Vec::new(); - let mut index = 0; - while index < characters.len() { - let character = characters[index]; - if char::is_whitespace(character) { - index += 1; - continue; - } - if starts_comment(&characters, index) { - index = comment_end(&characters, index)?; - continue; - } - if starts_template(&characters, index) { - let template_end = find_character(&characters, '}', index + 2) - .ok_or_else(|| format!("unterminated template value at position {index}"))?; - tokens.push(( - WORD_TOKEN, - characters[index..=template_end].iter().collect(), - index, - )); - index = template_end + 1; - continue; - } - if is_symbol(character) || character == ':' { - tokens.push((SYMBOL_TOKEN, character.to_string(), index)); - index += 1; - continue; - } - if matches!(character, '\'' | '"') { - let (value, next_index) = read_quoted_string(&characters, index)?; - tokens.push((STRING_TOKEN, value, index)); - index = next_index; - continue; - } +/// The tokens of `characters`; when they fail after a misquoted description, that error. +fn tokenize(characters: &[char]) -> Result, String> { + let mut scan: TokenScan<'_> = TokenScan { + characters, + prose_error: None, + }; + scan.tokens() + .map_err(|error| scan.prose_error.take().unwrap_or(error)) +} - let start = index; +/// One tokenization of a header, remembering the first description that is misquoted as prose. +struct TokenScan<'a> { + characters: &'a [char], + prose_error: Option, +} + +impl TokenScan<'_> { + fn tokens(&mut self) -> Result, String> { + let characters: &[char] = self.characters; + let mut tokens: Vec = Vec::new(); + let mut index = 0; while index < characters.len() { - if starts_template(&characters, index) { - let template_end = find_character(&characters, '}', index + 2) + let character = characters[index]; + if char::is_whitespace(character) { + index += 1; + continue; + } + if starts_comment(characters, index) { + index = comment_end(characters, index)?; + continue; + } + if starts_template(characters, index) { + let template_end = find_character(characters, '}', index + 2) .ok_or_else(|| format!("unterminated template value at position {index}"))?; + tokens.push(( + WORD_TOKEN, + characters[index..=template_end].iter().collect(), + index, + )); index = template_end + 1; continue; } - let next = characters[index]; - if char::is_whitespace(next) - || is_symbol(next) - || next == ':' - || starts_comment(&characters, index) - { - break; + if is_symbol(character) || character == ':' { + tokens.push((SYMBOL_TOKEN, character.to_string(), index)); + index += 1; + continue; } - if matches!(next, '\'' | '"') { - let quote_name = if next == '\'' { "single" } else { "double" }; + if opens_triple_quote(characters, index) { + let (content_end, end) = triple_quoted_string(characters, index)?; + tokens.push(( + STRING_TOKEN, + characters[index + TRIPLE_QUOTE_LENGTH..content_end] + .iter() + .collect(), + index, + )); + index = end; + continue; + } + if matches!(character, '\'' | '"') { + let (value, next_index) = self.quoted_token(index)?; + tokens.push((STRING_TOKEN, value, index)); + index = next_index; + continue; + } + + let start = index; + while index < characters.len() { + if starts_template(characters, index) { + let template_end = + find_character(characters, '}', index + 2).ok_or_else(|| { + format!("unterminated template value at position {index}") + })?; + index = template_end + 1; + continue; + } + let next = characters[index]; + if char::is_whitespace(next) + || is_symbol(next) + || next == ':' + || starts_comment(characters, index) + { + break; + } + if matches!(next, '\'' | '"') { + let quote_name = if next == '\'' { "single" } else { "double" }; + return Err(format!( + "unexpected {quote_name} quote inside bare value at position {index}; quote the whole value" + )); + } + index += 1; + } + if start == index { return Err(format!( - "unexpected {quote_name} quote inside bare value at position {index}; quote the whole value" + "unexpected character '{character}' at position {start}" )); } - index += 1; + tokens.push((WORD_TOKEN, characters[start..index].iter().collect(), start)); } - if start == index { - return Err(format!( - "unexpected character '{character}' at position {start}" - )); + tokens.push((END_TOKEN, String::new(), characters.len())); + Ok(tokens) + } + + /// The value and end of the quoted string at `start`; a description's ends where prose ends. + fn quoted_token(&mut self, start: usize) -> Result<(String, usize), String> { + let characters: &[char] = self.characters; + let read: Result<(String, usize), String> = read_quoted_string(characters, start); + if !follows_description_key(characters, start) { + return read; + } + let end: usize = match prose_end(characters, start) { + Ok(end) => end, + Err(error) => { + self.prose_error + .get_or_insert_with(|| prose_error_message(error, characters[start])); + return read; + } + }; + match read { + Ok((value, read_end)) if read_end == end => Ok((value, end)), + _ => Ok((characters[start + 1..end - 1].iter().collect(), end)), + } + } +} + +/// The text end and end of the `"""` string at `start`, checked not to close inside its text. +fn triple_quoted_string(characters: &[char], start: usize) -> Result<(usize, usize), String> { + let (content_end, end) = triple_quoted_end(characters, start).map_err(|error| match error { + TripleQuoteError::Unclosed => format!("{UNCLOSED_TRIPLE_QUOTE}{POSITION_SUFFIX}{start}"), + TripleQuoteError::Inside(position) => { + format!("{TRIPLE_QUOTE_INSIDE}{POSITION_SUFFIX}{position}") } - tokens.push((WORD_TOKEN, characters[start..index].iter().collect(), start)); + })?; + if closes_inside_text(characters, end) { + return Err(format!( + "{TRIPLE_QUOTE_INSIDE}{POSITION_SUFFIX}{content_end}" + )); } - tokens.push((END_TOKEN, String::new(), characters.len())); - Ok(tokens) + Ok((content_end, end)) } fn read_quoted_string(characters: &[char], start: usize) -> Result<(String, usize), String> { diff --git a/crates/sqlbuild-sqltext/src/compiler/tests.rs b/crates/sqlbuild-sqltext/src/compiler/tests.rs index c98ba89828..ccf5e81b02 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests.rs @@ -1,5 +1,7 @@ #[path = "tests/test_declaration_references.rs"] mod declaration_references; +#[path = "tests/test_header_strings.rs"] +mod header_strings; #[path = "tests/helpers.rs"] mod helpers; #[path = "tests/test_model_columns.rs"] diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs index 0326016fed..660f9d5fb6 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs @@ -286,3 +286,41 @@ pub(crate) fn locations( }) .collect() } + +/// The string at `path`: map keys, call names, or list indexes in brackets. +pub(crate) fn string_at(value: &AuthoredValue, path: &[&str]) -> Option { + let Some((first, rest)) = path.split_first() else { + let AuthoredValue::String(text) = value else { + return None; + }; + return Some(text.clone()); + }; + string_at(child(value, first)?, rest) +} + +/// The value under map key `key`, or list index `key` in brackets, of `value`. +fn child<'a>(value: &'a AuthoredValue, key: &str) -> Option<&'a AuthoredValue> { + let AuthoredValue::Map(entries) = value else { + let AuthoredValue::List(items) = value else { + return None; + }; + return items.get(key.trim_matches(['[', ']']).parse::().ok()?); + }; + entries + .iter() + .find(|(entry_key, _)| entry_key == key) + .map(|(_, entry)| entry) +} + +/// The string `header` parses to at `path`, or its parse error. +pub(crate) fn parsed_string(header: &str, path: &[&str]) -> Result { + let (value, _, error) = parse_one(header); + error.map_or_else( + || { + value + .and_then(|value| string_at(&value, path)) + .ok_or_else(|| format!("no string at {path:?}")) + }, + Err, + ) +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_header_strings.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_header_strings.rs new file mode 100644 index 0000000000..6e339aead5 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_header_strings.rs @@ -0,0 +1,164 @@ +use crate::compiler::tests::helpers::parsed_string; +use crate::compiler::tests::test_types::HeaderStringTestCase; + +#[test] +fn given_header_strings_when_parsing_then_prose_is_folded_and_other_text_kept_as_written() { + let test_cases = [ + HeaderStringTestCase { + description: "a wrapped description is folded", + header: "description \"Orders placed\nby day, one row each.\"", + path: &["description"], + expected_value: Ok("Orders placed by day, one row each."), + }, + HeaderStringTestCase { + description: "a triple-quoted description on one line keeps its quotes", + header: r#"description """Matches "Miss".""", materialized table"#, + path: &["description"], + expected_value: Ok(r#"Matches "Miss"."#), + }, + HeaderStringTestCase { + description: "a triple-quoted block is folded with paragraphs", + header: "description \"\"\"\n A \"quoted\" word,\n it's fine.\n\n Second.\n \"\"\",\n materialized table", + path: &["description"], + expected_value: Ok("A \"quoted\" word, it's fine.\n\nSecond."), + }, + HeaderStringTestCase { + description: "the same text on one line or wrapped stores the same", + header: "description \"True for a woman's title: Mme, Mlle,\n Miss (any case).\"", + path: &["description"], + expected_value: Ok("True for a woman's title: Mme, Mlle, Miss (any case)."), + }, + HeaderStringTestCase { + description: "a column description is prose", + header: "columns (order_id (type BIGINT, description \"The order\n key.\"))", + path: &["columns", "order_id", "description"], + expected_value: Ok("The order key."), + }, + HeaderStringTestCase { + description: "an audit's description option is prose", + header: "audits [not_null(column 'id', description \"Every row\n has one.\")]", + path: &["audits", "[0]", "not_null", "description"], + expected_value: Ok("Every row has one."), + }, + HeaderStringTestCase { + description: "a description may end in a backslash", + header: "description \"ends in C:\\\",\n materialized table", + path: &["description"], + expected_value: Ok("ends in C:\\"), + }, + HeaderStringTestCase { + description: "a one-line description keeps doubled backslashes", + header: r#"description "Path C:\\temp""#, + path: &["description"], + expected_value: Ok(r"Path C:\\temp"), + }, + HeaderStringTestCase { + description: "a backslash escapes a quote in a single-quoted description", + header: r"description 'it\'s fine', materialized table", + path: &["description"], + expected_value: Ok("it's fine"), + }, + HeaderStringTestCase { + description: "a doubled quote is one quote in a single-quoted description", + header: "description 'it''s fine', materialized table", + path: &["description"], + expected_value: Ok("it's fine"), + }, + HeaderStringTestCase { + description: "doubled double quotes in a description fail at the second quote", + header: r#"description "say ""hi"" now""#, + path: &["description"], + expected_value: Err(r#"double quote inside a "..." string at position 17"#), + }, + HeaderStringTestCase { + description: "a single quote inside a single-quoted description fails", + header: "description 'it's fine'", + path: &["description"], + expected_value: Err("single quote inside a '...' string at position 15"), + }, + HeaderStringTestCase { + description: "adjacent list strings outside descriptions read as before", + header: r#"description "x", tags ["a""b"]"#, + path: &["tags", "[1]"], + expected_value: Ok("b"), + }, + HeaderStringTestCase { + description: "a string followed by another kind of quote reads as before", + header: r#"description "x", tags ["x"'y']"#, + path: &["tags", "[1]"], + expected_value: Ok("y"), + }, + HeaderStringTestCase { + description: "a meta value keeps its line breaks", + header: "meta (description \"kept\n as written\")", + path: &["meta", "description"], + expected_value: Ok("kept\n as written"), + }, + HeaderStringTestCase { + description: "an audit predicate keeps its line breaks", + header: "audits [expression(predicate \"a > 0\n AND b > 0\")]", + path: &["audits", "[0]", "expression", "predicate"], + expected_value: Ok("a > 0\n AND b > 0"), + }, + HeaderStringTestCase { + description: "a triple-quoted SQL string keeps backslashes and quotes", + header: r#"name_pattern """^"x"\d+$""""#, + path: &["name_pattern"], + expected_value: Ok(r#"^"x"\d+$"#), + }, + HeaderStringTestCase { + description: "a non-prose string still reads backslash escapes", + header: r#"name_pattern "a\"b""#, + path: &["name_pattern"], + expected_value: Ok("a\"b"), + }, + HeaderStringTestCase { + description: "an escaped double quote in a description fails at the quote", + header: r#"description "Matches a title (\"Miss\").""#, + path: &["description"], + expected_value: Err(r#"double quote inside a "..." string at position 31"#), + }, + HeaderStringTestCase { + description: "a wrapped escaped quote with apostrophes fails the same way", + header: "description \"A woman's title, with or without a dot\n(\\\"MISS ANNA PARKER\\\", \\\"Mrs. C. O'Brien\\\").\"", + path: &["description"], + expected_value: Err(r#"double quote inside a "..." string at position 54"#), + }, + HeaderStringTestCase { + description: "a bare double quote ends the string where text runs on", + header: r#"description "Matches "Miss" here""#, + path: &["description"], + expected_value: Err(r#"double quote inside a "..." string at position 21"#), + }, + HeaderStringTestCase { + description: "a triple quote inside a triple-quoted string", + header: r#"description """He said """hi""" then""""#, + path: &["description"], + expected_value: Err(r#"""" inside a """...""" string at position 23"#), + }, + HeaderStringTestCase { + description: "a triple quote inside, followed by more text on the line", + header: r#"description """a """ b""""#, + path: &["description"], + expected_value: Err(r#"""" inside a """...""" string at position 17"#), + }, + HeaderStringTestCase { + description: "an unclosed triple-quoted string reports where it started", + header: "description \"\"\"\n never closed\n", + path: &["description"], + expected_value: Err(r#"unclosed """ string starting at position 12"#), + }, + ]; + + for test_case in test_cases { + assert_eq!( + parsed_string(test_case.header, test_case.path), + test_case + .expected_value + .map(str::to_owned) + .map_err(str::to_owned), + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs index d116ba54d6..865264c71e 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs @@ -128,3 +128,11 @@ pub(super) struct HeaderColumnsTestCase { pub(super) expected_entries: Vec<(&'static str, [usize; 4])>, pub(super) expected_locations: Vec<(&'static str, [usize; 4])>, } + +pub(crate) struct HeaderStringTestCase { + pub(crate) description: &'static str, + pub(crate) header: &'static str, + /// Map keys, or `[index]` list items, leading to the string value. + pub(crate) path: &'static [&'static str], + pub(crate) expected_value: Result<&'static str, &'static str>, +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/_helpers/locations.rs b/crates/sqlbuild-sqltext/src/header_strings/_helpers/locations.rs new file mode 100644 index 0000000000..0731041d1d --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/_helpers/locations.rs @@ -0,0 +1,62 @@ +//! Header quote errors: which they are, and where in the file they point. + +use crate::header_strings::constants::{ + DOUBLE_QUOTE_INSIDE, DOUBLE_QUOTE_INSIDE_HELP, DOUBLE_QUOTE_INSIDE_MESSAGE, POSITION_SUFFIX, + SINGLE_QUOTE_INSIDE, SINGLE_QUOTE_INSIDE_HELP, SINGLE_QUOTE_INSIDE_MESSAGE, + TRIPLE_QUOTE_INSIDE, TRIPLE_QUOTE_INSIDE_HELP, TRIPLE_QUOTE_INSIDE_MESSAGE, + UNCLOSED_TRIPLE_QUOTE, UNCLOSED_TRIPLE_QUOTE_HELP, UNCLOSED_TRIPLE_QUOTE_MESSAGE, +}; + +/// Each quote error's kind, then the message and help it reports. +const QUOTE_ERRORS: [(&str, &str, &str); 4] = [ + ( + DOUBLE_QUOTE_INSIDE, + DOUBLE_QUOTE_INSIDE_MESSAGE, + DOUBLE_QUOTE_INSIDE_HELP, + ), + ( + SINGLE_QUOTE_INSIDE, + SINGLE_QUOTE_INSIDE_MESSAGE, + SINGLE_QUOTE_INSIDE_HELP, + ), + ( + TRIPLE_QUOTE_INSIDE, + TRIPLE_QUOTE_INSIDE_MESSAGE, + TRIPLE_QUOTE_INSIDE_HELP, + ), + ( + UNCLOSED_TRIPLE_QUOTE, + UNCLOSED_TRIPLE_QUOTE_MESSAGE, + UNCLOSED_TRIPLE_QUOTE_HELP, + ), +]; + +/// The header position, message and help of quote error `error`, if it is one. +pub(crate) fn quote_error(error: &str) -> Option<(usize, &'static str, &'static str)> { + QUOTE_ERRORS.iter().find_map(|(kind, message, help)| { + let digits: &str = error.strip_prefix(*kind)?.strip_prefix(POSITION_SUFFIX)?; + let Ok(position) = digits.parse::() else { + return None; + }; + Some((position, *message, *help)) + }) +} + +/// The one-based file line and column of character offset `position` in `header`. +pub(crate) fn location( + header: &str, + position: usize, + (header_line, header_column): (usize, usize), +) -> (usize, usize) { + let mut line: usize = header_line; + let mut column: usize = header_column; + for character in header.chars().take(position) { + if character == '\n' { + line += 1; + column = 1; + } else { + column += 1; + } + } + (line, column) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/_helpers/mod.rs b/crates/sqlbuild-sqltext/src/header_strings/_helpers/mod.rs new file mode 100644 index 0000000000..6def37b961 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/_helpers/mod.rs @@ -0,0 +1,3 @@ +pub(crate) mod locations; +pub(crate) mod prose_ends; +pub(crate) mod separators; diff --git a/crates/sqlbuild-sqltext/src/header_strings/_helpers/prose_ends.rs b/crates/sqlbuild-sqltext/src/header_strings/_helpers/prose_ends.rs new file mode 100644 index 0000000000..f8cb096515 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/_helpers/prose_ends.rs @@ -0,0 +1,64 @@ +//! Where each kind of prose header string ends, by the quoting rules of prose. + +use crate::header_strings::_helpers::separators::{run_on_after_close, separator_follows}; +use crate::header_strings::errors::{ProseError, TripleQuoteError}; +use crate::header_strings::main::closes_inside_text::closes_inside_text; +use crate::header_strings::main::triple_quoted_end::triple_quoted_end; + +/// The end of the `"""..."""` string at `start`, checked not to close inside its text. +pub(crate) fn triple_end + Into>( + text: &[T], + start: usize, +) -> Result { + let (content_end, end) = triple_quoted_end(text, start).map_err(|error| match error { + TripleQuoteError::Unclosed => ProseError::UnclosedTripleQuote(start), + TripleQuoteError::Inside(position) => ProseError::TripleQuoteInside(position), + })?; + if closes_inside_text(text, end) { + return Err(ProseError::TripleQuoteInside(content_end)); + } + Ok(end) +} + +/// The end of the `"..."` string at `start`, where a backslash is an ordinary character. +pub(crate) fn double_end + Into>( + text: &[T], + start: usize, +) -> Result { + let quote: T = T::from(b'"'); + let close: usize = (start + 1..text.len()) + .find(|index| text[*index] == quote) + .ok_or(ProseError::Unterminated(start))?; + let escaped: bool = close > start + 1 && text[close - 1] == T::from(b'\\'); + if run_on_after_close(text, close + 1) || (escaped && !separator_follows(text, close + 1)) { + return Err(ProseError::DoubleQuoteInside(close)); + } + Ok(close + 1) +} + +/// The end of the `'...'` string at `start`, where `\\'` and `''` are quotes in the text. +pub(crate) fn single_end + Into>( + text: &[T], + start: usize, +) -> Result { + let quote: T = T::from(b'\''); + let mut index: usize = start + 1; + while index < text.len() { + if text[index] == T::from(b'\\') { + index += 2; + continue; + } + if text[index] == quote { + if text.get(index + 1) == Some("e) { + index += 2; + continue; + } + if run_on_after_close(text, index + 1) { + return Err(ProseError::SingleQuoteInside(index)); + } + return Ok(index + 1); + } + index += 1; + } + Err(ProseError::Unterminated(start)) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/_helpers/separators.rs b/crates/sqlbuild-sqltext/src/header_strings/_helpers/separators.rs new file mode 100644 index 0000000000..d5fcb4ad06 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/_helpers/separators.rs @@ -0,0 +1,29 @@ +//! What may follow a header value: a separator, a comment or the end of its line, not more text. + +use crate::header_strings::constants::FIRST_NON_ASCII; + +/// Whether a separator, comment, line end or the end of `text` follows `after`, past spaces. +pub(crate) fn separator_follows>(text: &[T], after: usize) -> bool { + let is = |index: usize, byte: u8| text.get(index) == Some(&T::from(byte)); + let mut next: usize = after; + while is(next, b' ') || is(next, b'\t') || is(next, b'\r') { + next += 1; + } + next >= text.len() + || b"\n,)]}:".iter().any(|byte| is(next, *byte)) + || (is(next, b'-') && is(next + 1, b'-')) + || (is(next, b'/') && is(next + 1, b'*')) +} + +/// Whether the unit at `after`, just past a closing quote, is a word character or another quote. +pub(crate) fn run_on_after_close + Into>( + text: &[T], + after: usize, +) -> bool { + text.get(after).is_some_and(|unit| { + let code: u32 = (*unit).into(); + char::from_u32(code).is_some_and(|character| { + character.is_alphanumeric() || matches!(character, '_' | '"' | '\'') + }) || code >= FIRST_NON_ASCII + }) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/constants.rs b/crates/sqlbuild-sqltext/src/header_strings/constants.rs new file mode 100644 index 0000000000..9ee43c62ab --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/constants.rs @@ -0,0 +1,35 @@ +//! Header string delimiters, the prose keys that fold, and the quote errors and their help. + +/// The delimiter of a triple-quoted header string. +pub const TRIPLE_QUOTE: &str = "\"\"\""; +/// Characters in a triple-quote delimiter. +pub const TRIPLE_QUOTE_LENGTH: usize = 3; +/// The most double quotes that may end a triple-quoted string: two of them belong to the text. +pub const LONGEST_CLOSING_RUN: usize = 5; +/// The header key whose string values are folded prose. +pub const DESCRIPTION_KEY: &str = "description"; +/// The header key whose list items are audits, whose `description` option is prose. +pub const AUDITS_KEY: &str = "audits"; +/// What a prose value stores for a blank line: a paragraph break. +pub const PARAGRAPH_BREAK: &str = "\n\n"; + +/// Header syntax errors end with ` at position `. +pub const POSITION_SUFFIX: &str = " at position "; +pub const DOUBLE_QUOTE_INSIDE: &str = "double quote inside a \"...\" string"; +pub const SINGLE_QUOTE_INSIDE: &str = "single quote inside a '...' string"; +pub const TRIPLE_QUOTE_INSIDE: &str = "\"\"\" inside a \"\"\"...\"\"\" string"; +pub const UNCLOSED_TRIPLE_QUOTE: &str = "unclosed \"\"\" string starting"; + +pub const DOUBLE_QUOTE_INSIDE_MESSAGE: &str = "a double quote ends this \"...\" string early; use \"\"\"…\"\"\" for text containing double quotes"; +pub const DOUBLE_QUOTE_INSIDE_HELP: &str = "write the text between triple quotes, where quotes need no escaping: description \"\"\"Matches \"Miss\".\"\"\"; sqb format --fix rewrites a description that escapes its quotes as \\\" or doubles them as \"\""; +pub const SINGLE_QUOTE_INSIDE_MESSAGE: &str = + "a single quote ends this '...' string early; use \"...\" for text containing single quotes"; +pub const SINGLE_QUOTE_INSIDE_HELP: &str = "write the text between double quotes, description \"Matches 'Miss'.\", or double the quote: 'it''s'"; +pub const TRIPLE_QUOTE_INSIDE_MESSAGE: &str = + "\"\"\" inside a \"\"\"...\"\"\" string ends it early"; +pub const TRIPLE_QUOTE_INSIDE_HELP: &str = "a \"\"\"...\"\"\" string ends at the next \"\"\"; reword the text so it has no three double quotes in a row"; +pub const UNCLOSED_TRIPLE_QUOTE_MESSAGE: &str = "this \"\"\" string is never closed"; +pub const UNCLOSED_TRIPLE_QUOTE_HELP: &str = "end the text with \"\"\""; + +/// Code points from here on are not ASCII; text running on into one continues a word. +pub(crate) const FIRST_NON_ASCII: u32 = 0x80; diff --git a/crates/sqlbuild-sqltext/src/header_strings/errors.rs b/crates/sqlbuild-sqltext/src/header_strings/errors.rs new file mode 100644 index 0000000000..c02fef232b --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/errors.rs @@ -0,0 +1,25 @@ +//! Why a header string does not end where it should. + +/// Why a triple-quoted string does not end where it should. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum TripleQuoteError { + /// No closing `"""` follows. + Unclosed, + /// A `"""` at this offset ends the string inside its text. + Inside(usize), +} + +/// Why a prose header string does not end where it should. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ProseError { + /// The `"..."` or `'...'` string opening at this offset never closes. + Unterminated(usize), + /// A `"` at this offset ends a `"..."` string inside its text. + DoubleQuoteInside(usize), + /// A `'` at this offset ends a `'...'` string inside its text. + SingleQuoteInside(usize), + /// A `"""` at this offset ends a `"""..."""` string inside its text. + TripleQuoteInside(usize), + /// The `"""` string opening at this offset never closes. + UnclosedTripleQuote(usize), +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/closes_inside_text.rs b/crates/sqlbuild-sqltext/src/header_strings/main/closes_inside_text.rs new file mode 100644 index 0000000000..3589e2d7f6 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/closes_inside_text.rs @@ -0,0 +1,27 @@ +//! Whether a closing `"""` was really inside the text it seemed to end. + +use crate::header_strings::_helpers::separators::{run_on_after_close, separator_follows}; +use crate::header_strings::constants::TRIPLE_QUOTE_LENGTH; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; + +/// Whether the `"""` ending at `end` was inside its text: text, or text then `"""`, follows it. +pub fn closes_inside_text + Into>( + text: &[T], + end: usize, +) -> bool { + if run_on_after_close(text, end) { + return true; + } + if separator_follows(text, end) { + return false; + } + let is = |index: usize, byte: u8| text.get(index) == Some(&T::from(byte)); + let mut index: usize = end; + while index + TRIPLE_QUOTE_LENGTH <= text.len() && !is(index, b'\n') { + if opens_triple_quote(text, index) { + return true; + } + index += 1; + } + false +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/fold_prose.rs b/crates/sqlbuild-sqltext/src/header_strings/main/fold_prose.rs new file mode 100644 index 0000000000..a289bb2256 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/fold_prose.rs @@ -0,0 +1,38 @@ +//! Folded prose: the text a description stores, however its lines are wrapped. + +use crate::header_strings::constants::PARAGRAPH_BREAK; + +fn is_layout_space(character: char) -> bool { + matches!(character, ' ' | '\t' | '\n' | '\r') +} + +/// The line breaks in a run of layout space, counting `\r\n` once. +fn line_breaks(run: &str) -> usize { + run.matches('\n').count() + run.matches('\r').count() - run.matches("\r\n").count() +} + +/// The text `written` stores as folded prose; text on one line is kept exactly. +pub fn fold_prose(written: &str) -> String { + if !written.contains(['\n', '\r']) { + return written.to_owned(); + } + let trimmed: &str = written.trim_matches(is_layout_space); + let mut folded: String = String::with_capacity(trimmed.len()); + let mut rest: &str = trimmed; + while let Some(space_start) = rest.find(is_layout_space) { + folded.push_str(&rest[..space_start]); + let after: &str = &rest[space_start..]; + let space_end: usize = after + .find(|character: char| !is_layout_space(character)) + .unwrap_or(after.len()); + let run: &str = &after[..space_end]; + match line_breaks(run) { + 0 => folded.push_str(run), + 1 => folded.push(' '), + _ => folded.push_str(PARAGRAPH_BREAK), + } + rest = &after[space_end..]; + } + folded.push_str(rest); + folded +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/follows_description_key.rs b/crates/sqlbuild-sqltext/src/header_strings/main/follows_description_key.rs new file mode 100644 index 0000000000..64750bc975 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/follows_description_key.rs @@ -0,0 +1,29 @@ +//! Whether a header string is the value of a `description` key, which holds prose. + +use crate::header_strings::constants::DESCRIPTION_KEY; + +/// Whether `unit` can be part of a header key. +fn is_key_unit>(unit: T) -> bool { + char::from_u32(unit.into()) + .is_some_and(|character| character.is_alphanumeric() || character == '_') +} + +/// Whether the string at `start` directly follows a `description` key, in any letter case. +pub fn follows_description_key + Into>( + text: &[T], + start: usize, +) -> bool { + let is_space = |unit: T| b" \t\r\n".iter().any(|byte| unit == T::from(*byte)); + let mut key_end: usize = start; + while key_end > 0 && is_space(text[key_end - 1]) { + key_end -= 1; + } + let Some(key_start) = key_end.checked_sub(DESCRIPTION_KEY.len()) else { + return false; + }; + let matches_key: bool = DESCRIPTION_KEY.bytes().enumerate().all(|(offset, byte)| { + let unit: T = text[key_start + offset]; + unit == T::from(byte) || unit == T::from(byte.to_ascii_uppercase()) + }); + matches_key && key_end < start && (key_start == 0 || !is_key_unit(text[key_start - 1])) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/header_quoted_end.rs b/crates/sqlbuild-sqltext/src/header_strings/main/header_quoted_end.rs new file mode 100644 index 0000000000..df9c0451f2 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/header_quoted_end.rs @@ -0,0 +1,40 @@ +//! The extent of one quoted header string, for scanners that only need where it ends. + +use crate::header_strings::constants::TRIPLE_QUOTE_LENGTH; +use crate::header_strings::errors::TripleQuoteError; +use crate::header_strings::main::follows_description_key::follows_description_key; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; +use crate::header_strings::main::prose_end::prose_end; +use crate::header_strings::main::triple_quoted_end::triple_quoted_end; + +/// The offset just past the header string opening at `start`, or `None` when it never closes. +pub fn header_quoted_end + Into>( + text: &[T], + start: usize, +) -> Option { + if follows_description_key(text, start) + && let Ok(end) = prose_end(text, start) + { + return Some(end); + } + if opens_triple_quote(text, start) { + return match triple_quoted_end(text, start) { + Ok((_, end)) => Some(end), + Err(TripleQuoteError::Inside(index)) => Some(index + TRIPLE_QUOTE_LENGTH), + Err(TripleQuoteError::Unclosed) => None, + }; + } + let quote: T = text[start]; + let mut index: usize = start + 1; + while index < text.len() { + if text[index] == T::from(b'\\') { + index += 2; + continue; + } + if text[index] == quote { + return Some(index + 1); + } + index += 1; + } + None +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/mod.rs b/crates/sqlbuild-sqltext/src/header_strings/main/mod.rs new file mode 100644 index 0000000000..9bccfaa818 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/mod.rs @@ -0,0 +1,10 @@ +pub mod closes_inside_text; +pub mod fold_prose; +pub mod follows_description_key; +pub mod header_quoted_end; +pub mod opens_triple_quote; +pub mod prose_end; +pub mod prose_error_message; +pub mod prose_string; +pub mod quote_failure; +pub mod triple_quoted_end; diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/opens_triple_quote.rs b/crates/sqlbuild-sqltext/src/header_strings/main/opens_triple_quote.rs new file mode 100644 index 0000000000..7f0ab7de13 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/opens_triple_quote.rs @@ -0,0 +1,9 @@ +//! Whether a triple-quoted header string opens at an offset. + +use crate::header_strings::constants::TRIPLE_QUOTE_LENGTH; + +/// Whether `"""` starts at `index`. +pub fn opens_triple_quote>(text: &[T], index: usize) -> bool { + text.get(index..index + TRIPLE_QUOTE_LENGTH) + .is_some_and(|delimiter| delimiter.iter().all(|unit| *unit == T::from(b'"'))) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/prose_end.rs b/crates/sqlbuild-sqltext/src/header_strings/main/prose_end.rs new file mode 100644 index 0000000000..4fd5004c05 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/prose_end.rs @@ -0,0 +1,19 @@ +//! Where a prose header string ends, by the quoting rules of prose. + +use crate::header_strings::_helpers::prose_ends::{double_end, single_end, triple_end}; +use crate::header_strings::errors::ProseError; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; + +/// The offset just past the prose string opening at `start`, or why it does not end well. +pub fn prose_end + Into>( + text: &[T], + start: usize, +) -> Result { + if opens_triple_quote(text, start) { + return triple_end(text, start); + } + if text[start] == T::from(b'"') { + return double_end(text, start); + } + single_end(text, start) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/prose_error_message.rs b/crates/sqlbuild-sqltext/src/header_strings/main/prose_error_message.rs new file mode 100644 index 0000000000..34cdd1ad2f --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/prose_error_message.rs @@ -0,0 +1,29 @@ +//! The header syntax error a misquoted prose string reports. + +use crate::header_strings::constants::{ + DOUBLE_QUOTE_INSIDE, POSITION_SUFFIX, SINGLE_QUOTE_INSIDE, TRIPLE_QUOTE_INSIDE, + UNCLOSED_TRIPLE_QUOTE, +}; +use crate::header_strings::errors::ProseError; + +/// The header syntax error `error` reports, located by character offset. +pub fn prose_error_message(error: ProseError, quote: char) -> String { + match error { + ProseError::Unterminated(start) => format!( + "unterminated {}-quoted string{POSITION_SUFFIX}{start}", + if quote == '"' { "double" } else { "single" } + ), + ProseError::DoubleQuoteInside(position) => { + format!("{DOUBLE_QUOTE_INSIDE}{POSITION_SUFFIX}{position}") + } + ProseError::SingleQuoteInside(position) => { + format!("{SINGLE_QUOTE_INSIDE}{POSITION_SUFFIX}{position}") + } + ProseError::TripleQuoteInside(position) => { + format!("{TRIPLE_QUOTE_INSIDE}{POSITION_SUFFIX}{position}") + } + ProseError::UnclosedTripleQuote(start) => { + format!("{UNCLOSED_TRIPLE_QUOTE}{POSITION_SUFFIX}{start}") + } + } +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/prose_string.rs b/crates/sqlbuild-sqltext/src/header_strings/main/prose_string.rs new file mode 100644 index 0000000000..6c32b9ebf7 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/prose_string.rs @@ -0,0 +1,57 @@ +//! A prose string of a header: its delimiters checked and its text folded. + +use crate::header_strings::constants::TRIPLE_QUOTE_LENGTH; +use crate::header_strings::main::fold_prose::fold_prose; +use crate::header_strings::main::opens_triple_quote::opens_triple_quote; +use crate::header_strings::main::prose_end::prose_end; +use crate::header_strings::main::prose_error_message::prose_error_message; +use crate::header_strings::main::triple_quoted_end::triple_quoted_end; +use crate::header_strings::models::{HeaderQuote, ProseString}; + +/// The text of a `'...'` string's inside: `\x` is `x`, and `''` is `'`. +fn unescaped_single(inside: &[char]) -> String { + let mut text: String = String::with_capacity(inside.len()); + let mut index: usize = 0; + while index < inside.len() { + let escape: bool = inside[index] == '\\' + || (inside[index] == '\'' && inside.get(index + 1) == Some(&'\'')); + if escape && index + 1 < inside.len() { + index += 1; + } + text.push(inside[index]); + index += 1; + } + text +} + +/// The prose string that opens at character offset `start` of `text`, or its header syntax error. +pub fn prose_string(text: &[char], start: usize) -> Result { + let end: usize = + prose_end(text, start).map_err(|error| prose_error_message(error, text[start]))?; + let (quote, written): (HeaderQuote, String) = if opens_triple_quote(text, start) { + let content_end: usize = triple_quoted_end(text, start) + .map_or(end - TRIPLE_QUOTE_LENGTH, |(content_end, _)| content_end); + ( + HeaderQuote::Triple, + text[start + TRIPLE_QUOTE_LENGTH..content_end] + .iter() + .collect(), + ) + } else if text[start] == '"' { + ( + HeaderQuote::Double, + text[start + 1..end - 1].iter().collect(), + ) + } else { + ( + HeaderQuote::Single, + unescaped_single(&text[start + 1..end - 1]), + ) + }; + Ok(ProseString { + quote, + start, + end, + value: fold_prose(&written), + }) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/quote_failure.rs b/crates/sqlbuild-sqltext/src/header_strings/main/quote_failure.rs new file mode 100644 index 0000000000..015826825c --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/quote_failure.rs @@ -0,0 +1,21 @@ +//! A header quote error located in its file, with the message and help it reports. + +use crate::header_strings::_helpers::locations::{location, quote_error}; +use crate::header_strings::models::QuoteFailure; + +/// The failure for quote error `error` in `header`, whose text starts at file `header_start`. +pub fn quote_failure( + error: &str, + header: &str, + header_start: (usize, usize), +) -> Option { + let (position, message, help) = quote_error(error)?; + let (line, column) = location(header, position, header_start); + Some(QuoteFailure { + position, + line, + column, + message: message.to_owned(), + help: help.to_owned(), + }) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/main/triple_quoted_end.rs b/crates/sqlbuild-sqltext/src/header_strings/main/triple_quoted_end.rs new file mode 100644 index 0000000000..2ac7eb3e81 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/main/triple_quoted_end.rs @@ -0,0 +1,31 @@ +//! Where a triple-quoted header string's text and closing delimiter end. + +use crate::header_strings::constants::{LONGEST_CLOSING_RUN, TRIPLE_QUOTE_LENGTH}; +use crate::header_strings::errors::TripleQuoteError; + +/// The end of the text and of the closing `"""` of the triple-quoted string at `start`. +pub fn triple_quoted_end>( + text: &[T], + start: usize, +) -> Result<(usize, usize), TripleQuoteError> { + let quote: T = T::from(b'"'); + let mut index: usize = start + TRIPLE_QUOTE_LENGTH; + while index < text.len() { + if text[index] != quote { + index += 1; + continue; + } + let run: usize = text[index..] + .iter() + .take_while(|unit| **unit == quote) + .count(); + if run >= TRIPLE_QUOTE_LENGTH { + if run > LONGEST_CLOSING_RUN { + return Err(TripleQuoteError::Inside(index)); + } + return Ok((index + run - TRIPLE_QUOTE_LENGTH, index + run)); + } + index += run; + } + Err(TripleQuoteError::Unclosed) +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/mod.rs b/crates/sqlbuild-sqltext/src/header_strings/mod.rs new file mode 100644 index 0000000000..899237ebcb --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/mod.rs @@ -0,0 +1,8 @@ +pub(crate) mod _helpers; +pub mod constants; +pub mod errors; +pub mod main; +pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlbuild-sqltext/src/header_strings/models.rs b/crates/sqlbuild-sqltext/src/header_strings/models.rs new file mode 100644 index 0000000000..ff483dd98b --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/models.rs @@ -0,0 +1,35 @@ +//! Header string delimiters, prose values and quote failures. + +/// How a header string is delimited. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum HeaderQuote { + /// `"""..."""`: the text is taken as written, quotes included. + Triple, + /// `"..."`. + Double, + /// `'...'`. + Single, +} + +/// One prose string of a header: where it is and the folded text it stores. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProseString { + pub quote: HeaderQuote, + /// Character offsets of the opening delimiter and just past the closing one. + pub start: usize, + pub end: usize, + /// The folded text the header stores. + pub value: String, +} + +/// A header quote error located in its file, with the message and help it reports. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct QuoteFailure { + /// The character offset of the offending quote in the header text. + pub position: usize, + /// One-based file line and column of the offending quote. + pub line: usize, + pub column: usize, + pub message: String, + pub help: String, +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/tests.rs b/crates/sqlbuild-sqltext/src/header_strings/tests.rs new file mode 100644 index 0000000000..f7cd6e533b --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/tests.rs @@ -0,0 +1,6 @@ +#[path = "tests/test_header_strings.rs"] +mod header_strings; +#[path = "tests/helpers.rs"] +mod helpers; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlbuild-sqltext/src/header_strings/tests/helpers.rs b/crates/sqlbuild-sqltext/src/header_strings/tests/helpers.rs new file mode 100644 index 0000000000..7ca6df7050 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/tests/helpers.rs @@ -0,0 +1,4 @@ +/// The characters of `text`. +pub(crate) fn chars(text: &str) -> Vec { + text.chars().collect() +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/tests/test_header_strings.rs b/crates/sqlbuild-sqltext/src/header_strings/tests/test_header_strings.rs new file mode 100644 index 0000000000..f22ef69e2f --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/tests/test_header_strings.rs @@ -0,0 +1,371 @@ +use crate::header_strings::constants::{ + DOUBLE_QUOTE_INSIDE_MESSAGE, TRIPLE_QUOTE_INSIDE_MESSAGE, UNCLOSED_TRIPLE_QUOTE_MESSAGE, +}; +use crate::header_strings::errors::TripleQuoteError; +use crate::header_strings::main::closes_inside_text::closes_inside_text; +use crate::header_strings::main::fold_prose::fold_prose; +use crate::header_strings::main::header_quoted_end::header_quoted_end; +use crate::header_strings::main::prose_string::prose_string; +use crate::header_strings::main::quote_failure::quote_failure; +use crate::header_strings::main::triple_quoted_end::triple_quoted_end; +use crate::header_strings::models::HeaderQuote; +use crate::header_strings::tests::helpers::chars; +use crate::header_strings::tests::test_types::{ + ClosesInsideTestCase, DescriptionEndTestCase, FoldTestCase, ProseTestCase, + QuoteFailureTestCase, QuotedEndTestCase, TripleEndTestCase, +}; + +#[test] +fn given_written_prose_when_folding_then_line_breaks_are_spaces_and_blank_lines_paragraphs() { + let test_cases = [ + FoldTestCase { + description: "one line is kept exactly, outer and inner spaces included", + written: " Orders by day ", + expected_folded: " Orders by day ", + }, + FoldTestCase { + description: "a break and the indentation after it is one space", + written: "Orders placed\n by day.", + expected_folded: "Orders placed by day.", + }, + FoldTestCase { + description: "continuation at column zero folds the same as indented", + written: "Orders placed\nby day.", + expected_folded: "Orders placed by day.", + }, + FoldTestCase { + description: "trailing spaces before a break fold into the one space", + written: "Orders placed \n by day.", + expected_folded: "Orders placed by day.", + }, + FoldTestCase { + description: "a blank line starts a paragraph", + written: "First paragraph\ncontinues.\n \n Second.", + expected_folded: "First paragraph continues.\n\nSecond.", + }, + FoldTestCase { + description: "a block's leading and closing line breaks are dropped", + written: "\n Orders by day.\n ", + expected_folded: "Orders by day.", + }, + FoldTestCase { + description: "spaces inside a line are kept", + written: "a b\nc", + expected_folded: "a b c", + }, + FoldTestCase { + description: "Windows line endings fold like line feeds", + written: "a\r\n b\r\n\r\nc", + expected_folded: "a b\n\nc", + }, + ]; + + for test_case in test_cases { + assert_eq!( + fold_prose(test_case.written), + test_case.expected_folded, + "{}", + test_case.description + ); + assert_eq!( + fold_prose(test_case.expected_folded), + test_case.expected_folded, + "folding is stable: {}", + test_case.description + ); + } +} + +#[test] +fn given_triple_quoted_text_when_finding_its_end_then_quotes_inside_belong_to_the_text() { + let test_cases = [ + TripleEndTestCase { + description: "plain text", + text: r#""""abc""""#, + expected_end: Ok((6, 9)), + }, + TripleEndTestCase { + description: "inner quotes and apostrophes need no escaping", + text: r#""""say "hi", it's"""x"#, + expected_end: Ok((17, 20)), + }, + TripleEndTestCase { + description: "up to two quotes before the delimiter end the text", + text: r#""""say "hi"""""#, + expected_end: Ok((11, 14)), + }, + TripleEndTestCase { + description: "a run of six quotes is a delimiter inside the text", + text: r#""""a""""""b"""#, + expected_end: Err(TripleQuoteError::Inside(4)), + }, + TripleEndTestCase { + description: "never closed", + text: r#""""abc "" def"#, + expected_end: Err(TripleQuoteError::Unclosed), + }, + ]; + + for test_case in test_cases { + assert_eq!( + triple_quoted_end(&chars(test_case.text), 0), + test_case.expected_end, + "{}", + test_case.description + ); + assert_eq!( + triple_quoted_end(test_case.text.as_bytes(), 0), + test_case.expected_end, + "bytes: {}", + test_case.description + ); + } +} + +#[test] +fn given_header_string_when_finding_its_end_then_triple_and_escaped_quotes_are_skipped() { + let test_cases = [ + QuotedEndTestCase { + description: "double quotes with a backslash escape", + text: r#""a\"b" rest"#, + expected_end: Some(6), + }, + QuotedEndTestCase { + description: "an apostrophe inside double quotes", + text: r#""it's" rest"#, + expected_end: Some(6), + }, + QuotedEndTestCase { + description: "a triple-quoted string with parentheses and one quote", + text: r#""""5" pipe (x);""") rest"#, + expected_end: Some(18), + }, + QuotedEndTestCase { + description: "an unclosed triple-quoted string", + text: r#""""never"#, + expected_end: None, + }, + ]; + + for test_case in test_cases { + assert_eq!( + header_quoted_end(&chars(test_case.text), 0), + test_case.expected_end, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_description_string_when_finding_its_end_then_prose_rules_decide_it() { + let test_cases = [ + DescriptionEndTestCase { + description: "a description may end in a backslash", + text: r#"description "C:\", tags x"#, + start: 12, + expected_end: Some(17), + }, + DescriptionEndTestCase { + description: "any other string reads the backslash as an escape", + text: r#"name "C:\", tags x""#, + start: 5, + expected_end: Some(19), + }, + DescriptionEndTestCase { + description: "a misquoted description still ends where the escape reading ends", + text: r#"description "say \"hi\"", x"#, + start: 12, + expected_end: Some(24), + }, + DescriptionEndTestCase { + description: "a doubled single quote stays inside the description", + text: "DESCRIPTION 'it''s', x", + start: 12, + expected_end: Some(19), + }, + DescriptionEndTestCase { + description: "a key that only ends in description is not one", + text: r#"redescription "C:\", x""#, + start: 14, + expected_end: Some(23), + }, + ]; + + for test_case in test_cases { + assert_eq!( + header_quoted_end(&chars(test_case.text), test_case.start), + test_case.expected_end, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_closing_triple_quote_when_text_follows_then_it_was_inside_the_text() { + let test_cases = [ + ClosesInsideTestCase { + description: "more text and another triple quote follow on the line", + text: r#"a """b""" c""" d"#, + end: 9, + expected_inside: true, + }, + ClosesInsideTestCase { + description: "a separator follows", + text: "a \"\"\"b\"\"\", tags x", + end: 9, + expected_inside: false, + }, + ClosesInsideTestCase { + description: "text runs straight on", + text: "\"\"\"b\"\"\"c", + end: 7, + expected_inside: true, + }, + ]; + + for test_case in test_cases { + assert_eq!( + closes_inside_text(&chars(test_case.text), test_case.end), + test_case.expected_inside, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_prose_string_when_reading_then_text_is_folded_and_escaped_quotes_rejected() { + let test_cases = [ + ProseTestCase { + description: "a one-line double-quoted string keeps its text", + text: r#""Orders by day.""#, + expected_prose: Ok((HeaderQuote::Double, "Orders by day.")), + }, + ProseTestCase { + description: "a wrapped double-quoted string is folded", + text: "\"Orders placed\n by day.\"", + expected_prose: Ok((HeaderQuote::Double, "Orders placed by day.")), + }, + ProseTestCase { + description: "a triple-quoted string on one line keeps its quotes", + text: r#""""Matches "Miss".""""#, + expected_prose: Ok((HeaderQuote::Triple, r#"Matches "Miss"."#)), + }, + ProseTestCase { + description: "a triple-quoted block is folded", + text: "\"\"\"\n True for \"Mrs. C. O'Brien\",\n any case.\n \"\"\"", + expected_prose: Ok(( + HeaderQuote::Triple, + r#"True for "Mrs. C. O'Brien", any case."#, + )), + }, + ProseTestCase { + description: "backslashes are ordinary characters", + text: r#""C:\data\ and \\ stay""#, + expected_prose: Ok((HeaderQuote::Double, r"C:\data\ and \\ stay")), + }, + ProseTestCase { + description: "an escaped double quote is rejected at the quote", + text: r#""Matches \"Miss\".""#, + expected_prose: Err(r#"double quote inside a "..." string at position 10"#), + }, + ProseTestCase { + description: "a backslash may end a double-quoted description", + text: r#""ends in C:\""#, + expected_prose: Ok((HeaderQuote::Double, r"ends in C:\")), + }, + ProseTestCase { + description: "an escaped quote before more text is inside the text", + text: r#""C:\" more""#, + expected_prose: Err(r#"double quote inside a "..." string at position 4"#), + }, + ProseTestCase { + description: "doubled double quotes are rejected at the second quote", + text: r#""say ""hi"" now""#, + expected_prose: Err(r#"double quote inside a "..." string at position 5"#), + }, + ProseTestCase { + description: "a backslash escapes a single quote", + text: r"'it\'s here'", + expected_prose: Ok((HeaderQuote::Single, "it's here")), + }, + ProseTestCase { + description: "a doubled single quote is one quote", + text: "'it''s a long one'", + expected_prose: Ok((HeaderQuote::Single, "it's a long one")), + }, + ProseTestCase { + description: "a lone single quote ends the string inside the text", + text: "'it's fine'", + expected_prose: Err("single quote inside a '...' string at position 3"), + }, + ProseTestCase { + description: "an unclosed triple-quoted string reports where it starts", + text: r#""""never closed"#, + expected_prose: Err(r#"unclosed """ string starting at position 0"#), + }, + ]; + + for test_case in test_cases { + let actual = prose_string(&chars(test_case.text), 0); + assert_eq!( + actual + .as_ref() + .map(|prose| (prose.quote, prose.value.as_str())) + .map_err(String::as_str), + test_case.expected_prose, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_quote_error_when_locating_then_line_and_column_are_in_the_file() { + let test_cases = [ + QuoteFailureTestCase { + description: "on the header's first line, after its opening column", + error: r#"double quote inside a "..." string at position 18"#, + header: r#"description "Say \"hi\"""#, + expected_failure: Some((3, 26, DOUBLE_QUOTE_INSIDE_MESSAGE)), + }, + QuoteFailureTestCase { + description: "a message that only ends like a quote error is not one", + error: r#""""...""" string at position 0"#, + header: "x", + expected_failure: None, + }, + QuoteFailureTestCase { + description: "an unclosed triple quote on the second line", + error: r#"unclosed """ string starting at position 15"#, + header: "\n description \"\"\"never", + expected_failure: Some((4, 15, UNCLOSED_TRIPLE_QUOTE_MESSAGE)), + }, + QuoteFailureTestCase { + description: "a triple quote inside the text", + error: r#"""" inside a """...""" string at position 2"#, + header: "ab", + expected_failure: Some((3, 10, TRIPLE_QUOTE_INSIDE_MESSAGE)), + }, + QuoteFailureTestCase { + description: "any other header error is not a quote failure", + error: "expected key at position 3", + header: "abcd", + expected_failure: None, + }, + ]; + + for test_case in test_cases { + let actual = quote_failure(test_case.error, test_case.header, (3, 8)); + assert_eq!( + actual + .as_ref() + .map(|failure| (failure.line, failure.column, failure.message.as_str())), + test_case.expected_failure, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-sqltext/src/header_strings/tests/test_types.rs b/crates/sqlbuild-sqltext/src/header_strings/tests/test_types.rs new file mode 100644 index 0000000000..05d42dbe27 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/header_strings/tests/test_types.rs @@ -0,0 +1,47 @@ +use crate::header_strings::errors::TripleQuoteError; +use crate::header_strings::models::HeaderQuote; + +pub(crate) struct FoldTestCase { + pub(crate) description: &'static str, + pub(crate) written: &'static str, + pub(crate) expected_folded: &'static str, +} + +pub(crate) struct TripleEndTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) expected_end: Result<(usize, usize), TripleQuoteError>, +} + +pub(crate) struct QuotedEndTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) expected_end: Option, +} + +pub(crate) struct ProseTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) expected_prose: Result<(HeaderQuote, &'static str), &'static str>, +} + +pub(crate) struct QuoteFailureTestCase { + pub(crate) description: &'static str, + pub(crate) error: &'static str, + pub(crate) header: &'static str, + pub(crate) expected_failure: Option<(usize, usize, &'static str)>, +} + +pub(crate) struct ClosesInsideTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) end: usize, + pub(crate) expected_inside: bool, +} + +pub(crate) struct DescriptionEndTestCase { + pub(crate) description: &'static str, + pub(crate) text: &'static str, + pub(crate) start: usize, + pub(crate) expected_end: Option, +} diff --git a/crates/sqlbuild-sqltext/src/lib.rs b/crates/sqlbuild-sqltext/src/lib.rs index a1e0563af4..c8c85e9fa7 100644 --- a/crates/sqlbuild-sqltext/src/lib.rs +++ b/crates/sqlbuild-sqltext/src/lib.rs @@ -6,6 +6,7 @@ pub mod code_scanner; pub mod compiler; mod constants; pub mod ddl_effects; +pub mod header_strings; pub mod resolution; pub mod sql_references; pub mod sql_scan; diff --git a/scripts/compiler_differential/_helpers/corpus/failure_cases.py b/scripts/compiler_differential/_helpers/corpus/failure_cases.py index e1f05164bb..c9a877c914 100644 --- a/scripts/compiler_differential/_helpers/corpus/failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/failure_cases.py @@ -39,6 +39,12 @@ _MART_HEADER_START: str = 'MODEL (\n description "Order totals per customer",\n' +_DESCRIPTION_QUOTE_HELP: str = ( + 'write the text between triple quotes, where quotes need no escaping: description """Matches ' + '"Miss"."""; sqb format --fix rewrites a description that escapes its quotes as \\" ' + 'or doubles them as ""' +) + _RULES_CONFIG: str = '\n[rules]\nselect = ["{codes}"]\n' _ENGINE_ERROR_MACROS: str = "macros/python/cents.py" @@ -668,6 +674,47 @@ def engine_error_cases() -> tuple[FailureCase, ...]: 'Quote it to keep it as text ("\u0663"), or write the number with ASCII digits 0-9' ), ), + failure_case( + name="engine-error-description-escaped-quote", + expected_code="D002", + files={ + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + 'description "Order totals per customer"', + 'description "Order totals per \\"customer\\""', + 1, + ) + }, + expected_message=( + f"MODEL(...) in '/{FAILURE_MART_PATH}:2:34' contains invalid SQLBuild " + 'header syntax: a double quote ends this "..." string early; use """…""" for ' + "text containing double quotes" + ), + expected_help=_DESCRIPTION_QUOTE_HELP, + ), + failure_case( + name="engine-error-macro-description-escaped-quote", + expected_code="P015", + files={ + "macros/sql/customer_label.sql": ( + "MACRO (\n" + " description \"A customer's label,\n" + 'quoted (\\"VIP\\").",\n' + " arguments (customer_id BIGINT),\n" + " returns VARCHAR,\n" + ");\n\n" + "CAST(@customer_id AS VARCHAR)\n" + ), + **mart_body_files( + "SELECT customer_id, @sql.customer_label(customer_id) AS label\n" + 'FROM __ref("stg_orders")\n' + ), + }, + expected_message=( + 'a double quote ends this "..." string early; use """…""" for text ' + "containing double quotes\n --> macros/sql/customer_label.sql:3:10" + ), + expected_help=_DESCRIPTION_QUOTE_HELP, + ), failure_case( name="engine-error-non-ascii-digit-bare-number", expected_code="D013", diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md index 8d0fca7d69..442d2f734d 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md @@ -74,8 +74,27 @@ Format does not render templates, so a templated description that renders empty is reported by `sqb compile`. Plain `sqb format` still formats the file and exits non-zero, like any other remaining fault. -Long model and scenario descriptions are reflowed deterministically. Ordinary authored line breaks -are normalized as spaces, while blank lines preserve paragraph boundaries. +Descriptions of `MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK` and `MACRO` headers have one +canonical layout, which never changes their words, their quotes or the text they store (see +[Folded descriptions](../concepts/models/configuration.md#folded-descriptions)): + +- A description that fits on its line stays one line: `"..."`, or `"""..."""` when it contains a + double quote. A short description wrapped over several lines is joined onto one. +- A longer description, or one with paragraphs, becomes a `"""` block: its lines are indented one + level under the field and wrapped at the line width, with a blank line between paragraphs, and + the closing `"""` lines up with the field. + +```sql +MODEL ( + description """ + True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case, with or + without a dot ("MISS ANNA PARKER", "Mrs. C. O'Brien", "Ms.Robinson"). + """, +); +``` + +A one-line `'...'` description that fits is left as written. Running `sqb format` again changes +nothing. Clause layout follows a few fixed rules: @@ -166,6 +185,15 @@ reason, and the run continues. A cycle or the 128-pass limit fails without writi | SQBRSQL030 boolean CASE simplification | Refused | The `COALESCE` rewrite is not proved portable to every supported dialect. | | SQBRSQL042 unused CTE output | Applied when row-neutral | Only a column reference or deterministic scalar expression is removed, from a CTE without `DISTINCT`, `DISTINCT ON`, `GROUP BY ALL`, `ORDER BY ALL`, an ungrouped aggregate, or positional `GROUP BY`/`ORDER BY`. Other removals could change the rows and are refused with the reason. | +`--fix` also rewrites a header description that escapes double quotes as `\"` or doubles them as +`""`, which descriptions do not support, as a `"""..."""` string holding real quotes, and reports +it as an applied `description-quotes` fix. It rewrites only a description whose sole backslashes +escape quotes, and only when the new string stores exactly that text. A file that still has header +syntax errors once its descriptions are rewritten is left as written, and the fix is reported as +refused. When the project cannot compile +until the rewritten descriptions are saved, the run skips the compiler-verified Rule fixes with a +`format-fix-skipped` warning; run `sqb format --fix` again to apply them. + Edits that touch macro or intrinsic expansions are refused, and so are edits outside model SQL bodies, because recompiling cannot prove their output contract. Findings without a proposed edit are reported as unavailable. Human output lists each applied, refused, and unavailable fix with its diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md index 5ccc5999da..82417bf04e 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md @@ -69,6 +69,11 @@ ROUND(CAST(@amount_cents AS DECIMAL(18, 4)) / 100, @places) `returns` is required for expression macros and not allowed for the other forms. +The description follows the same rules as every header string: write text that contains double +quotes as `"""..."""`, and a wrapped description is stored folded into one flowing text (see +[Header strings](../models/configuration.md#header-strings)). Quotes in the macro body +never change how the header is read. + ## Call a SQL macro Every macro call says its kind: `@sql.name(...)` calls a SQL macro and `@py.name(...)` a Python diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md index 22ed33a4aa..bbf4c9afad 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md @@ -10,6 +10,7 @@ Online: https://sqlbuild.com/docs/concepts/models/configuration/ - Common fields - Descriptions +- Header strings - SQL analysis - Table fields - Incremental fields @@ -93,6 +94,77 @@ does). `sqb format --check` also fails when a `MODEL`, `SCENARIO`, `FUNCTION` or header has no description (a `MODEL` header that uses `model_schema` is left to compile), and `sqb format` still moves a leading comment into an empty model description. +## Header strings + +A quoted value in any SQLBuild header (`MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK`, `MACRO`, +`TEST`, `AUDIT`, `ENUM`, `CONSTANT`) is written as `"..."` or `'...'`. Text that contains a double +quote goes between triple quotes, on one line or across several. Inside `"""..."""`, `"` and `'` +need no escaping: + +```sql +MODEL ( + description """Matches "Miss" and "Mrs." titles, with or without a dot.""", + ... +); +``` + +A `"` inside a `"..."` string ends it, so it is an error with its line and column rather than the +start of an escape. A description has no `\"` escape and no `""` doubling: in a `"..."` +description a backslash is an ordinary character, so `"Files under C:\"` ends with a backslash and +`"Path C:\\temp"` stores both backslashes. In a `'...'` description, `\'` and `''` both stand for +one `'`, as in `'it\'s'` and `'it''s'`. `sqb format --fix` rewrites a description that escapes its +quotes as `\"` or doubles them as `""` into a `"""..."""` string holding real quotes: + +```text +error[D002]: MODEL(...) in 'models/customers.sql:2:34' contains invalid SQLBuild header syntax: a double quote ends this "..." string early; use """…""" for text containing double quotes + = help: write the text between triple quotes, where quotes need no escaping: description """Matches "Miss"."""; sqb format --fix rewrites a description that escapes its quotes as \" or doubles them as "" +``` + +A `"""..."""` string ends at the first three double quotes in a row. Up to two quotes just before +them belong to the text, so the text may end with a quote, but it cannot contain three in a row. +A `"""` inside the text and a `"""` that is never closed fail with the position of the offending +quote or of the opening `"""`. + +### Folded descriptions + +Descriptions are prose, so SQLBuild stores them folded: the same text gives the same description +whether it is written on one line or wrapped, and at any indentation. + +- Text on one line is stored exactly as written. +- Otherwise the spaces at either end are dropped, a line break and the spaces around it become one + space, and a blank line starts a new paragraph (stored as `\n\n`). + +```sql +MODEL ( + description """ + Customers with a "woman's title" flag, + one row each. + + Built from the raw seed. + """, +); +``` + +stores `Customers with a "woman's title" flag, one row each.`, a paragraph break, and `Built from +the raw seed.` + +These strings are folded prose: + +| String | Where | +|--------|-------| +| `description` | The top level of every header. `MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK` and `MACRO` headers take one; `AUDIT`, `ENUM`, `CONSTANT` and `TEST` headers reject the key | +| Column `description` | Entries of `columns (...)` in `MODEL` and `SCHEMA` headers | +| Audit `description` | The `description` option of an audit in `audits [...]` | +| Exception `reason` | `reason` of `[[rules.rule_exceptions]]`, `[[rules.graph_edge_exceptions]]`, `[[rules.rule_ignores]]`, `[[rules.select_star_allow]]` and `[[rules.threshold_overrides]]` in `sqlbuild_project.toml` | + +Every other string means exactly what it says and keeps its text as written, line breaks +included: SQL such as audit `predicate` and `where_condition` arguments or `inline_sql(...)` hooks, +regular expressions such as `name_pattern`, literal values in `meta`, `constants`, `placeholders` +and audit arguments, and names and paths. In these strings a backslash still escapes the character +after it inside `"..."` and `'...'`; a `"""..."""` string takes its text exactly as written, +backslashes included. `sqb format` lays descriptions out in one canonical form; see +[Line width and descriptions](../../cli/format.md#line-width-and-descriptions). + ## SQL analysis SQL analysis has project, invocation, and model gates: diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/findings-and-exceptions.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/findings-and-exceptions.md index 07110f3292..32d07b90d3 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/findings-and-exceptions.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/findings-and-exceptions.md @@ -37,6 +37,10 @@ reason = "This example intentionally demonstrates one sampled row." An exception refers to one exact Rule code and path. If the finding disappears, SQLBuild reports the stale exception so obsolete configuration does not accumulate silently. +A reason is prose. One written as a multi-line TOML string (`"""..."""`) is stored folded, like a +description: line breaks and the spaces around them become one space, and a blank line starts a +paragraph (see [Folded descriptions](../models/configuration.md#folded-descriptions)). + ## Relation-level exceptions A file-wide exception for a deliberate cartesian product would also hide any accidental cross join diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/__init__.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/_test_types.py new file mode 100644 index 0000000000..386cfb5d07 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/_test_types.py @@ -0,0 +1,54 @@ +"""Test case types for header string quoting and folding E2E tests.""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class HeaderQuoteErrorTestCase: + """One project file whose header string is quoted wrongly, and the error compile reports.""" + + description: str + path: str + contents: str + expected_code: str + expected_location: str + expected_message: str + + +@dataclass(frozen=True) +class FoldedDescriptionTestCase: + """Project files with prose descriptions and the folded text the manifest stores.""" + + description: str + files: dict[str, str] + expected_description: str + expected_column_descriptions: dict[str, str] + + +@dataclass(frozen=True) +class AcceptedDescriptionTestCase: + """A description quoted as earlier releases accepted it, and the text it stores.""" + + description: str + header: str + expected_description: str + + +@dataclass(frozen=True) +class ListStringTestCase: + """A `tags` list of adjacent quoted strings and the tags it holds.""" + + description: str + tags: str + expected_tags: list[str] + + +@dataclass(frozen=True) +class MacroDescriptionTestCase: + """A SQL macro description quoted as earlier releases accepted it, and compile's exit code.""" + + description: str + header: str + expected_exit_code: int diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/helpers.py new file mode 100644 index 0000000000..e6309efefd --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/helpers.py @@ -0,0 +1,28 @@ +"""Project helpers for header string quoting and folding E2E tests.""" + +from __future__ import annotations + +from pathlib import Path + +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project + +_BASE_FILES: dict[str, str] = { + "sqlbuild_project.toml": ( + 'name = "shop"\nadapter = "duckdb"\n\n[connection]\ndatabase = "shop.duckdb"\n\n' + "[scopes]\nenforce_placement = false\n" + ), + "seeds/raw_customers.csv": "customer_id,title\n1,Miss\n2,Mr\n", + "seeds/raw_customers.yml": ( + "seeds:\n - name: raw_customers\n description: Raw customers.\n" + " columns:\n - name: customer_id\n type: BIGINT\n" + " - name: title\n type: VARCHAR\n" + ), +} + + +def header_strings_project(tmp_path: Path, files: dict[str, str]) -> Path: + """Write a DuckDB customer project holding `files` and return its directory.""" + + return prepare_inline_project( + tmp_path=tmp_path, project_name="shop", repo_files={**_BASE_FILES, **files} + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/test_header_strings.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/test_header_strings.py new file mode 100644 index 0000000000..0ee70b2310 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/header_strings/test_header_strings.py @@ -0,0 +1,368 @@ +"""E2E coverage of header string quoting and folded descriptions through the real CLI.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile.header_strings._test_types import ( + AcceptedDescriptionTestCase, + FoldedDescriptionTestCase, + HeaderQuoteErrorTestCase, + ListStringTestCase, + MacroDescriptionTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.header_strings.helpers import ( + header_strings_project, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import run_sqb + +_QUOTE_MESSAGE: str = ( + 'a double quote ends this "..." string early; use """…""" for text containing double quotes' +) + +_TITLE_MACRO: str = ( + "MACRO (\n" + ' description """\n' + " True when a title is a woman's title, with or without a dot\n" + ' ("Miss", "Mrs. C. O\'Brien").\n' + ' """,\n' + " arguments (title VARCHAR),\n" + " returns BOOLEAN,\n" + ");\n\n" + "LOWER(@title) IN ('miss', 'mrs', 'ms')\n" +) + +_CUSTOMER_SCHEMA: str = ( + "SCHEMA (\n" + " name customer,\n" + ' description """One customer, as "the shop" knows them."""\n' + " ,\n" + " columns (\n" + " customer_id (type BIGINT, description \"The customer's\n" + ' key.")\n' + " ),\n" + ");\n" +) + +_CUSTOMERS_MODEL: str = ( + "MODEL (\n" + ' description """\n' + ' Customers with a "woman\'s title" flag,\n' + " one row each.\n" + "\n" + " Built from the raw seed.\n" + ' """,\n' + " model_schema customer,\n" + " columns (\n" + ' is_womans_title (type BOOLEAN, description "True for Miss,\n' + ' Mrs and Ms.")\n' + " ),\n" + ");\n\n" + "SELECT customer_id, @sql.is_womans_title(title) AS is_womans_title\n" + 'FROM __seed("raw_customers")\n' +) + + +@pytest.mark.parametrize( + "test_case", + [ + HeaderQuoteErrorTestCase( + description="model_description_escaping_a_quote", + path="models/customers.sql", + contents=( + 'MODEL (\n description "Matches a title (\\"Miss\\").",\n);\n\n' + "SELECT 1 AS customer_id\n" + ), + expected_code="D002", + expected_location="models/customers.sql:2:34", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="macro_description_escaping_a_quote", + path="macros/sql/title.sql", + contents=( + 'MACRO (\n description "Matches a title (\\"Miss\\").",\n' + " arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n@value\n" + ), + expected_code="P015", + expected_location="macros/sql/title.sql:2:34", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="wrapped_macro_description_with_apostrophes_escaping_quotes", + path="macros/sql/title.sql", + contents=( + "MACRO (\n" + " description \"True when a name starts with a woman's title: Mme, Mlle, Miss, " + "Mrs, Ms. Any case,\n" + 'with or without a dot (\\"MISS ANNA PARKER\\", \\"Mrs. C. O\'Brien\\", ' + '\\"Ms.Robinson\\"). Capital\n' + 'initials before a surname (\\"MS. Ramos\\" for M. S. Ramos) are not a title.",\n' + " arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n@value\n" + ), + expected_code="P015", + expected_location="macros/sql/title.sql:3:25", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="schema_description_escaping_a_quote", + path="schemas/customer.sql", + contents=( + 'SCHEMA (\n name customer,\n description "The \\"shop\\" customer",\n' + " columns (customer_id (type BIGINT)),\n);\n" + ), + expected_code="D013", + expected_location="schemas/customer.sql:3:21", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="model_description_doubling_a_quote", + path="models/customers.sql", + contents=('MODEL (\n description "say ""hi"" now",\n);\n\nSELECT 1 AS customer_id\n'), + expected_code="D002", + expected_location="models/customers.sql:2:20", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="macro_description_doubling_a_quote", + path="macros/sql/title.sql", + contents=( + 'MACRO (\n description "say ""hi"" now",\n' + " arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n@value\n" + ), + expected_code="P015", + expected_location="macros/sql/title.sql:2:20", + expected_message=_QUOTE_MESSAGE, + ), + HeaderQuoteErrorTestCase( + description="audit_header_scanned_past_a_triple_quoted_string", + path="audits/singular/orders_have_customers.sql", + contents=( + 'AUDIT (name "orders_have_customers", ' + 'description """Rerun "sqb audit --all" after loads""");\n\n' + 'SELECT 1 FROM __seed("raw_customers") WHERE FALSE\n' + ), + expected_code="D004", + expected_location="audits/singular/orders_have_customers.sql:1", + expected_message="has unsupported keys: description", + ), + HeaderQuoteErrorTestCase( + description="unclosed_triple_quote", + path="models/customers.sql", + contents=( + 'MODEL (\n description """Never closed,\n materialized table,\n);\n\n' + "SELECT 1 AS customer_id\n" + ), + expected_code="D002", + expected_location="models/customers.sql:2:15", + expected_message='this """ string is never closed', + ), + HeaderQuoteErrorTestCase( + description="triple_quote_inside_a_triple_quoted_string", + path="models/customers.sql", + contents=( + 'MODEL (\n description """He said """hi""" twice.""",\n);\n\n' + "SELECT 1 AS customer_id\n" + ), + expected_code="D002", + expected_location="models/customers.sql:2:26", + expected_message='""" inside a """...""" string ends it early', + ), + ], + ids=lambda case: case.description, +) +def test_given_misquoted_header_string_when_compiling_then_error_names_the_quote_and_the_fix( + test_case: HeaderQuoteErrorTestCase, tmp_path: Path +) -> None: + project_dir: Path = header_strings_project(tmp_path, {test_case.path: test_case.contents}) + + result: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile") + ) + + output: str = result.stdout + result.stderr + assert result.returncode != 0, output + assert test_case.expected_message in output, output + assert test_case.expected_location in output, output + assert test_case.expected_code in output, output + + +@pytest.mark.parametrize( + "test_case", + [ + FoldedDescriptionTestCase( + description="triple_quoted_and_wrapped_model_schema_and_macro_descriptions", + files={ + "macros/sql/is_womans_title.sql": _TITLE_MACRO, + "schemas/customer.sql": _CUSTOMER_SCHEMA, + "models/customers.sql": _CUSTOMERS_MODEL, + }, + expected_description=( + 'Customers with a "woman\'s title" flag, one row each.\n\nBuilt from the raw seed.' + ), + expected_column_descriptions={ + "is_womans_title": "True for Miss, Mrs and Ms.", + "customer_id": "The customer's key.", + }, + ), + ], + ids=lambda case: case.description, +) +def test_given_triple_quoted_and_wrapped_descriptions_when_compiling_then_folded_text_is_stored( + test_case: FoldedDescriptionTestCase, tmp_path: Path +) -> None: + project_dir: Path = header_strings_project(tmp_path, test_case.files) + + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile", "--manifest") + ) + dag: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "dag", "--json") + ) + + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + assert dag.returncode == 0, dag.stderr + manifest: dict[str, Any] = json.loads((project_dir / "target/manifest.json").read_text()) + node: dict[str, Any] = manifest["nodes"]["model.shop.customers"] + assert node["description"] == test_case.expected_description + assert { + name: node["columns"][name]["description"] + for name in test_case.expected_column_descriptions + } == test_case.expected_column_descriptions + dag_nodes: dict[str, dict[str, Any]] = { + dag_node["name"]: dag_node for dag_node in json.loads(dag.stdout)["nodes"] + } + assert dag_nodes["customers"]["description"] == test_case.expected_description + + +@pytest.mark.parametrize( + "test_case", + [ + AcceptedDescriptionTestCase( + description="escaped_quote_in_single_quotes", + header="MODEL (\n description 'it\\'s fine',\n);\n", + expected_description="it's fine", + ), + AcceptedDescriptionTestCase( + description="doubled_quote_in_single_quotes", + header="MODEL (\n description 'it''s fine',\n);\n", + expected_description="it's fine", + ), + AcceptedDescriptionTestCase( + description="double_quotes_ending_in_a_backslash", + header='MODEL (\n description "ends in C:\\",\n);\n', + expected_description="ends in C:\\", + ), + AcceptedDescriptionTestCase( + description="doubled_backslash_kept_literally", + header='MODEL (\n description "Path C:\\\\temp",\n);\n', + expected_description="Path C:\\\\temp", + ), + ], + ids=lambda case: case.description, +) +def test_given_model_description_quoted_as_before_when_compiling_then_its_text_is_stored( + test_case: AcceptedDescriptionTestCase, tmp_path: Path +) -> None: + project_dir: Path = header_strings_project( + tmp_path, {"models/customers.sql": test_case.header + "\nSELECT 1 AS customer_id\n"} + ) + + dag: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "dag", "--json") + ) + + assert dag.returncode == 0, dag.stdout + dag.stderr + descriptions: dict[str, str] = { + node["name"]: node["description"] for node in json.loads(dag.stdout)["nodes"] + } + assert descriptions["customers"] == test_case.expected_description + + +@pytest.mark.parametrize( + "test_case", + [ + MacroDescriptionTestCase( + description="escaped_quote_in_single_quotes", + header="description 'it\\'s here'", + expected_exit_code=0, + ), + MacroDescriptionTestCase( + description="doubled_quote_in_single_quotes", + header="description 'it''s here'", + expected_exit_code=0, + ), + MacroDescriptionTestCase( + description="double_quotes_ending_in_a_backslash", + header='description "ends in C:\\"', + expected_exit_code=0, + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_description_quoted_as_before_when_compiling_then_the_project_compiles( + test_case: MacroDescriptionTestCase, tmp_path: Path +) -> None: + project_dir: Path = header_strings_project( + tmp_path, + { + "macros/sql/lowered.sql": ( + f"MACRO (\n {test_case.header},\n arguments (value VARCHAR),\n" + " returns VARCHAR,\n);\n\nLOWER(@value)\n" + ), + "models/customers.sql": ( + 'MODEL (description "Customers.");\n\n' + "SELECT customer_id, @sql.lowered(title) AS title\n" + 'FROM __seed("raw_customers")\n' + ), + }, + ) + + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile") + ) + + assert compiled.returncode == test_case.expected_exit_code, compiled.stdout + compiled.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + ListStringTestCase( + description="adjacent_double_quoted_strings", + tags='["a""b"]', + expected_tags=["a", "b"], + ), + ListStringTestCase( + description="double_then_single_quoted_string", + tags="[\"x\"'y']", + expected_tags=["x", "y"], + ), + ], + ids=lambda case: case.description, +) +def test_given_adjacent_strings_outside_descriptions_when_compiling_then_they_read_as_before( + test_case: ListStringTestCase, tmp_path: Path +) -> None: + project_dir: Path = header_strings_project( + tmp_path, + { + "models/customers.sql": ( + f'MODEL (\n description "Customers.",\n tags {test_case.tags},\n);\n\n' + "SELECT 1 AS customer_id\n" + ) + }, + ) + + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile", "--manifest") + ) + + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + manifest: dict[str, Any] = json.loads((project_dir / "target/manifest.json").read_text()) + assert manifest["nodes"]["model.shop.customers"]["tags"] == test_case.expected_tags diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py index 79354ef44e..6c05df61b0 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_reference_errors.py @@ -34,6 +34,7 @@ ) _MART_HEADER: str = 'MODEL (\n description "Order totals per customer",\n);\n\n' _QUOTED_NOTE: str = "over 12\\\" boxes, by customer's orders" +_QUOTED_DESCRIPTION: str = "over 12\" boxes, by customer's orders" _NO_SQL_ANALYSIS_CONFIG: str = FAILURE_BASE_CONFIG + "\n[settings]\nsql_analysis = false\n" @@ -78,9 +79,9 @@ expected_error_type="CompileInputError", ), ReferenceScanErrorTestCase( - description="header_string_with_an_escaped_quote_and_apostrophe", + description="triple_quoted_header_string_with_a_quote_and_apostrophe", project_files={ - FAILURE_MART_PATH: f'MODEL (\n description "{_QUOTED_NOTE}",\n);\n\n' + FAILURE_MART_PATH: f'MODEL (\n description """{_QUOTED_DESCRIPTION}""",\n);\n\n' 'SELECT customer_id, 1 AS total_amount\nFROM __ref("stg_orders")\nWHERE "open = 1\n' }, expected_message=( @@ -160,7 +161,7 @@ project_files={ "sqlbuild_project.toml": _NO_SQL_ANALYSIS_CONFIG, "functions/sql/orders_for.sql": ( - f'FUNCTION (\n description "{_QUOTED_NOTE}",\n' + f'FUNCTION (\n description """{_QUOTED_DESCRIPTION}""",\n' " arguments (p_customer_id INTEGER),\n returns INTEGER\n);\n\n" 'SELECT COUNT(*) FROM __ref("stg_orders" WHERE customer_id = p_customer_id\n' ), diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py index 8213b07daf..e391fdfb92 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py @@ -127,3 +127,42 @@ class FormatCacheEndToEndTestCase: edited_model: str expected_warm_summary: str expected_edited_summary: str + + +@dataclass(frozen=True) +class DescriptionLayoutTestCase: + """One authored model header and the header `sqb format` writes for it.""" + + description: str + header: str + expected_header: str + + +@dataclass(frozen=True) +class QuoteFixTestCase: + """Project files escaping description quotes and what `sqb format --fix` makes of them.""" + + description: str + files: dict[str, str] + expected_fixes: list[tuple[str, str, int, str]] + expected_starts: dict[str, str] + expected_description: str + + +@dataclass(frozen=True) +class MacroLayoutTestCase: + """One authored SQL macro file and the file `sqb format` writes for it.""" + + description: str + macro: str + expected_macro: str + + +@dataclass(frozen=True) +class RefusedQuoteFixTestCase: + """A file whose quote fix would leave other header errors, and the refused fix reported.""" + + description: str + path: str + contents: str + expected_fix: tuple[str, str, int, str] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py index 5d6165828a..186a587394 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py @@ -1,4 +1,4 @@ -"""Generated project helpers for format performance guards.""" +"""Project helpers for format E2E tests and performance guards.""" from __future__ import annotations @@ -178,3 +178,13 @@ def model_tree(*, project_dir: Path) -> dict[str, bytes]: str(path.relative_to(project_dir)): path.read_bytes() for path in sorted((project_dir / "models").rglob("*.sql")) } + + +def stored_descriptions(project_dir: Path) -> dict[str, str]: + """The description `sqb dag` reports for each node of the project, by name.""" + + dag: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, arguments=("--no-color", "dag", "--json") + ) + assert dag.returncode == 0, dag.stderr + return {node["name"]: node["description"] for node in json.loads(dag.stdout)["nodes"]} diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/test_description_layout.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_description_layout.py new file mode 100644 index 0000000000..c8e0052e2f --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_description_layout.py @@ -0,0 +1,313 @@ +"""E2E coverage of description layout and quote repair by `sqb format`.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.format._test_types import ( + DescriptionLayoutTestCase, + MacroLayoutTestCase, + QuoteFixTestCase, + RefusedQuoteFixTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.format.helpers import stored_descriptions +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_PROJECT_FILE: str = ( + 'name = "shop"\nadapter = "duckdb"\n\n[connection]\ndatabase = "shop.duckdb"\n\n' + "[scopes]\nenforce_placement = false\n" +) +_BODY: str = "\nSELECT 1 AS customer_id\n" + + +@pytest.mark.parametrize( + "test_case", + [ + DescriptionLayoutTestCase( + description="continuation_lines_at_column_zero_become_an_indented_block", + header=( + "MODEL (\n" + " description \"True when a name starts with a woman's title: Mme, Mlle, Miss, " + "Mrs, Ms. Any case,\n" + "with or without a dot ('MISS ANNA PARKER', 'Mrs. C. O'Brien', 'Ms.Robinson').\",\n" + ");\n" + ), + expected_header=( + "MODEL (\n" + ' description """\n' + " True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any " + "case, with or\n" + " without a dot ('MISS ANNA PARKER', 'Mrs. C. O'Brien', 'Ms.Robinson').\n" + ' """,\n' + ");\n" + ), + ), + DescriptionLayoutTestCase( + description="a_short_wrapped_description_is_joined", + header='MODEL (\n description "One row per\n customer.",\n);\n', + expected_header='MODEL (\n description "One row per customer.",\n);\n', + ), + DescriptionLayoutTestCase( + description="a_short_description_with_quotes_stays_one_triple_quoted_line", + header='MODEL (\n description """Matches "Miss" titles.""",\n);\n', + expected_header='MODEL (\n description """Matches "Miss" titles.""",\n);\n', + ), + DescriptionLayoutTestCase( + description="a_long_description_with_quotes_becomes_a_block", + header=( + "MODEL (\n" + ' description """True when a name starts with a title, with or without a dot ' + '("MISS ANNA PARKER", "Mrs. C. O\'Brien", "Ms.Robinson").""",\n' + ");\n" + ), + expected_header=( + "MODEL (\n" + ' description """\n' + " True when a name starts with a title, with or without a dot " + '("MISS ANNA PARKER", "Mrs. C.\n' + ' O\'Brien", "Ms.Robinson").\n' + ' """,\n' + ");\n" + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_model_description_when_formatting_then_layout_is_canonical_and_text_unchanged( + test_case: DescriptionLayoutTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="shop", + repo_files={ + "sqlbuild_project.toml": _PROJECT_FILE, + "models/customers.sql": test_case.header + _BODY, + }, + ) + before: dict[str, str] = stored_descriptions(project_dir) + + formatted: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format") + ) + checked: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format", "--check") + ) + + assert formatted.returncode == 0, formatted.stdout + formatted.stderr + assert checked.returncode == 0, checked.stdout + checked.stderr + assert ", 0 changed)" in checked.stderr + model: str = (project_dir / "models/customers.sql").read_text() + assert model == test_case.expected_header + _BODY + assert stored_descriptions(project_dir) == before + + +@pytest.mark.parametrize( + "test_case", + [ + QuoteFixTestCase( + description="model_and_wrapped_macro_descriptions", + files={ + "models/customers.sql": ( + 'MODEL (\n description "Matches a title (\\"Miss\\").",\n);\n' + "\nSELECT @sql.title('Miss') AS title\n" + ), + "macros/sql/title.sql": ( + "MACRO (\n" + " description \"True when a name starts with a woman's title: Mme, Mlle, " + "Miss, Mrs, Ms. Any case,\n" + 'with or without a dot (\\"MISS ANNA PARKER\\", \\"Mrs. C. O\'Brien\\").",\n' + " arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n@value\n" + ), + }, + expected_fixes=[ + ("description-quotes", "customers.sql", 2, "applied"), + ("description-quotes", "title.sql", 2, "applied"), + ], + expected_starts={ + "models/customers.sql": ( + 'MODEL (\n description """Matches a title ("Miss").""",\n);\n' + ), + "macros/sql/title.sql": ( + "MACRO (\n" + ' description """\n' + " True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. " + "Any case, with or\n" + ' without a dot ("MISS ANNA PARKER", "Mrs. C. O\'Brien").\n' + ' """,\n' + ), + }, + expected_description='Matches a title ("Miss").', + ), + QuoteFixTestCase( + description="model_and_macro_descriptions_doubling_quotes", + files={ + "models/customers.sql": ( + 'MODEL (\n description "say ""hi"" now",\n);\n' + "\nSELECT @sql.title('Miss') AS title\n" + ), + "macros/sql/title.sql": ( + 'MACRO (\n description "say ""hi"" now",\n' + " arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n@value\n" + ), + }, + expected_fixes=[ + ("description-quotes", "customers.sql", 2, "applied"), + ("description-quotes", "title.sql", 2, "applied"), + ], + expected_starts={ + "models/customers.sql": 'MODEL (\n description """say "hi" now""",\n);\n', + "macros/sql/title.sql": 'MACRO (\n description """say "hi" now""",\n', + }, + expected_description='say "hi" now', + ), + ], + ids=lambda case: case.description, +) +def test_given_descriptions_escaping_quotes_when_format_fix_then_triple_quoted_real_quotes( + test_case: QuoteFixTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="shop", + repo_files={"sqlbuild_project.toml": _PROJECT_FILE, **test_case.files}, + ) + failed: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile") + ) + + fixed: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format", "--fix", "--json") + ) + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile") + ) + checked: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format", "--check") + ) + + assert failed.returncode != 0 + payload: dict[str, Any] = json.loads(fixed.stdout) + assert ( + sorted( + (fix["code"], Path(fix["file"]).name, fix["line"], fix["status"]) + for fix in payload["rule_fixes"] + ) + == test_case.expected_fixes + ) + assert any(violation["code"] == "format-fix-skipped" for violation in payload["violations"]) + assert { + path: (project_dir / path).read_text()[: len(start)] + for path, start in test_case.expected_starts.items() + } == test_case.expected_starts + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + assert checked.returncode == 0, checked.stdout + checked.stderr + assert stored_descriptions(project_dir)["customers"] == test_case.expected_description + + +@pytest.mark.parametrize( + "test_case", + [ + MacroLayoutTestCase( + description="long_single_quoted_description_with_a_doubled_quote", + macro=( + "MACRO (\n description 'it''s a " + + " ".join(["word"] * 30) + + " end',\n arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n" + "LOWER(@value)\n" + ), + expected_macro=( + 'MACRO (\n description """\n' + " it's a " + + " ".join(["word"] * 18) + + "\n " + + " ".join(["word"] * 12) + + ' end\n """,\n arguments (value VARCHAR),\n returns VARCHAR,\n);\n\n' + "LOWER(@value)\n" + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_description_when_formatting_then_its_text_survives_and_it_compiles( + test_case: MacroLayoutTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="shop", + repo_files={ + "sqlbuild_project.toml": _PROJECT_FILE, + "macros/sql/lowered.sql": test_case.macro, + "models/customers.sql": ( + "MODEL (description \"Customers.\");\n\nSELECT @sql.lowered('Miss') AS title\n" + ), + }, + ) + + formatted: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format") + ) + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile") + ) + + assert formatted.returncode == 0, formatted.stdout + formatted.stderr + assert (project_dir / "macros/sql/lowered.sql").read_text() == test_case.expected_macro + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + RefusedQuoteFixTestCase( + description="macro_with_another_header_error", + path="macros/sql/title.sql", + contents=( + 'MACRO (\n description "say \\"hi\\"",\n arguments (value VARCHAR),\n' + " returns VARCHAR,\n unknown_key x,\n);\n\n@value\n" + ), + expected_fix=("description-quotes", "title.sql", 2, "refused"), + ), + RefusedQuoteFixTestCase( + description="model_with_another_header_error", + path="models/orders.sql", + contents=( + 'MODEL (\n description "say \\"hi\\"",\n tags ["a" "b",\n);\n' + "\nSELECT 1 AS order_id\n" + ), + expected_fix=("description-quotes", "orders.sql", 2, "refused"), + ), + ], + ids=lambda case: case.description, +) +def test_given_file_still_broken_after_quote_fix_when_format_fix_then_it_is_left_and_refused( + test_case: RefusedQuoteFixTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="shop", + repo_files={ + "sqlbuild_project.toml": _PROJECT_FILE, + test_case.path: test_case.contents, + "models/customers.sql": ( + 'MODEL (\n description "Matches \\"Miss\\".",\n);\n\nSELECT 1 AS customer_id\n' + ), + }, + ) + + fixed: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format", "--fix", "--json") + ) + + payload: dict[str, Any] = json.loads(fixed.stdout) + reported: list[tuple[str, str, int, str]] = [ + (fix["code"], Path(fix["file"]).name, fix["line"], fix["status"]) + for fix in payload["rule_fixes"] + ] + assert test_case.expected_fix in reported + assert ("description-quotes", "customers.sql", 2, "applied") in reported + assert (project_dir / test_case.path).read_text() == test_case.contents diff --git a/tests/goldens/compiler/failure/engine-error-description-escaped-quote.json b/tests/goldens/compiler/failure/engine-error-description-escaped-quote.json new file mode 100644 index 0000000000..7c14153f95 --- /dev/null +++ b/tests/goldens/compiler/failure/engine-error-description-escaped-quote.json @@ -0,0 +1,19 @@ +{ + "commands": [ + { + "label": "compile", + "exit_code": 1, + "diagnostics": [ + { + "phase": "compile", + "severity": "error", + "code": "D002", + "message": "MODEL(...) in '/models/marts/customer_totals.sql:2:34' contains invalid SQLBuild header syntax: a double quote ends this \"...\" string early; use \"\"\"…\"\"\" for text containing double quotes", + "help": "write the text between triple quotes, where quotes need no escaping: description \"\"\"Matches \"Miss\".\"\"\"; sqb format --fix rewrites a description that escapes its quotes as \\\" or doubles them as \"\"" + } + ] + } + ], + "compiled": {}, + "manifest": null +} diff --git a/tests/goldens/compiler/failure/engine-error-macro-description-escaped-quote.json b/tests/goldens/compiler/failure/engine-error-macro-description-escaped-quote.json new file mode 100644 index 0000000000..732dd9a47f --- /dev/null +++ b/tests/goldens/compiler/failure/engine-error-macro-description-escaped-quote.json @@ -0,0 +1,19 @@ +{ + "commands": [ + { + "label": "compile", + "exit_code": 1, + "diagnostics": [ + { + "phase": "compile", + "severity": "error", + "code": "P015", + "message": "a double quote ends this \"...\" string early; use \"\"\"…\"\"\" for text containing double quotes\n --> macros/sql/customer_label.sql:3:10", + "help": "write the text between triple quotes, where quotes need no escaping: description \"\"\"Matches \"Miss\".\"\"\"; sqb format --fix rewrites a description that escapes its quotes as \\\" or doubles them as \"\"" + } + ] + } + ], + "compiled": {}, + "manifest": null +} 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 7be6b0d4d4..153087da93 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py @@ -394,19 +394,18 @@ def test_given_exact_contract_casts_when_formatting_then_rule_still_passes( "unmatched customers." ), expected_formatted_description=( - "Builds canonical customer records from every\n" - "available source while retaining unmatched customers." + 'description """\n' + " Builds canonical customer records from every available\n" + " source while retaining unmatched customers.\n" + ' """' ), ), DescriptionFormatIntegrationTestCase( - description="escaped description content formats and compiles without loss", + description="short wrapped description is joined and compiles", line_width=100, - authored_description=( - "Builds order summary.\\nRetains unmatched orders.\\n\\n" - "Source \\u2192 canonical orders." - ), + authored_description="Builds order summary.\n Retains unmatched orders.", expected_formatted_description=( - "Builds order summary. Retains unmatched orders.\n\nSource → canonical orders." + 'description "Builds order summary. Retains unmatched orders."' ), ), ], @@ -432,9 +431,7 @@ def test_given_description_width_when_formatting_then_wrapped_model_compiles( assert format_exit == 0 assert compile_exit == 0 - assert f'description "{test_case.expected_formatted_description}"' in model.read_text( - encoding="utf-8" - ) + assert test_case.expected_formatted_description in model.read_text(encoding="utf-8") @pytest.mark.parametrize( diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py index 8c45a1122d..8078ba585d 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py @@ -317,11 +317,11 @@ def test_given_deferred_output_locations_when_discovering_models_then_projection expected_query="SELECT 1", ), ParseModelSqlHeaderTestCase( - description="accepts double quoted strings and escapes", + description="accepts double quoted strings and triple-quoted text", contents=""" MODEL ( schema "analytics mart", - description "Bob said \\"hello\\"", + description \"\"\"Bob said "hello\".\"\"\", post_hooks [inline_sql("grant select on @@CTX:destination.qualified to role analytics")], ); @@ -329,7 +329,7 @@ def test_given_deferred_output_locations_when_discovering_models_then_projection """, expected_header_values={ "schema": "analytics mart", - "description": 'Bob said "hello"', + "description": 'Bob said "hello".', "post_hooks": [ SqlHookEntry( statement="grant select on @@CTX:destination.qualified to role analytics" @@ -599,12 +599,12 @@ def test_given_sql_model_header_variants_when_parsing_then_it_returns_expected_h ParseModelSqlErrorTestCase( description="reports an unterminated string with escaped quotes before parentheses", contents='MODEL (description "' + '\\")' * 20000 + "); SELECT 1", - expected_error_fragment="unterminated double-quoted string", + expected_error_fragment="unexpected double quote inside bare value", ), ParseModelSqlErrorTestCase( description="reports an unterminated string with many escaped quotes", contents='MODEL (description "' + '\\"' * 20000 + "); SELECT 1", - expected_error_fragment="unterminated double-quoted string", + expected_error_fragment="a double quote ends this", ), ParseModelSqlErrorTestCase( description="raises when the model header is missing", diff --git a/tests/unit/src/sqlbuild/lint/_helpers/test_native.py b/tests/unit/src/sqlbuild/lint/_helpers/test_native.py index 5fe9d28cf7..47364d79b1 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/test_native.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/test_native.py @@ -85,10 +85,10 @@ expected_codes=("description-present",), ), LintNativeTestCase( - description="escaped paragraphs fault against canonical formatted length", + description="paragraphs fault against canonical formatted length", contents=( 'MODEL (description "' - + "\\n\\n".join(f"paragraph {index}" for index in range(11)) + + "\n\n".join(f"paragraph {index}" for index in range(11)) + '");\nSELECT 1\n' ), expected_codes=("description-length",), @@ -247,8 +247,10 @@ def test_given_contents_when_formatting_then_contents_match_expected( ), line_width=60, expected_contents=( - 'MODEL (\n description "Builds canonical customer records from every\n' - 'available source while retaining unmatched customers."\n);\nSELECT 1\n' + 'MODEL (\n description """\n' + " Builds canonical customer records from every available\n" + " source while retaining unmatched customers.\n" + ' """\n);\nSELECT 1\n' ), ), FormatDescriptionTestCase( @@ -259,8 +261,10 @@ def test_given_contents_when_formatting_then_contents_match_expected( ), line_width=60, expected_contents=( - 'MODEL (\n description "Builds canonical customer records from every\n' - 'available source while retaining unmatched customers."\n);\nSELECT 1\n' + 'MODEL (\n description """\n' + " Builds canonical customer records from every available\n" + " source while retaining unmatched customers.\n" + ' """\n);\nSELECT 1\n' ), ), FormatDescriptionTestCase( @@ -271,32 +275,34 @@ def test_given_contents_when_formatting_then_contents_match_expected( ), line_width=60, expected_contents=( - 'MODEL (\n description "Builds canonical customer records from every\n' - "source.\n\nRetains unmatched customers for complete downstream\n" - 'coverage."\n);\nSELECT 1\n' + 'MODEL (\n description """\n' + " Builds canonical customer records from every source.\n\n" + " Retains unmatched customers for complete downstream\n" + " coverage.\n" + ' """\n);\nSELECT 1\n' ), ), FormatDescriptionTestCase( - description="escaped newlines and unicode retain authored meaning", + description="backslashes in a description are kept as written", contents=( 'MODEL (\n description "Builds order summary.\\nRetains unmatched orders.' '\\n\\nSource \\u2192 canonical orders."\n);\nSELECT 1\n' ), line_width=100, expected_contents=( - 'MODEL (\n description "Builds order summary. Retains unmatched orders.\n\n' - 'Source → canonical orders."\n);\nSELECT 1\n' + 'MODEL (\n description "Builds order summary.\\nRetains unmatched orders.' + '\\n\\nSource \\u2192 canonical orders."\n);\nSELECT 1\n' ), ), FormatDescriptionTestCase( - description="malformed unicode escape is retained losslessly", + description="a backslash that is no escape is retained losslessly", contents=( 'MODEL (\n description "Builds order summary.\\u-123 retains text."\n);\n' "SELECT 1\n" ), line_width=100, expected_contents=( - 'MODEL (\n description "Builds order summary.\\\\u-123 retains text."\n);\n' + 'MODEL (\n description "Builds order summary.\\u-123 retains text."\n);\n' "SELECT 1\n" ), ), @@ -316,8 +322,9 @@ def test_given_contents_when_formatting_then_contents_match_expected( line_width=60, expected_contents=( 'MODEL (\n columns (\n customer_id (type BIGINT, description "A deliberately ' - 'long column description that remains authored text.")\n ),\n description "Builds ' - 'canonical customer records from every\navailable source."\n);\nSELECT 1\n' + 'long column description that remains authored text.")\n ),\n description """\n' + " Builds canonical customer records from every available\n" + ' source.\n """\n);\nSELECT 1\n' ), ), ], diff --git a/website/src/content/docs/docs/cli/format.mdx b/website/src/content/docs/docs/cli/format.mdx index acfacb35da..0b0ad606e9 100644 --- a/website/src/content/docs/docs/cli/format.mdx +++ b/website/src/content/docs/docs/cli/format.mdx @@ -65,8 +65,27 @@ Format does not render templates, so a templated description that renders empty is reported by `sqb compile`. Plain `sqb format` still formats the file and exits non-zero, like any other remaining fault. -Long model and scenario descriptions are reflowed deterministically. Ordinary authored line breaks -are normalized as spaces, while blank lines preserve paragraph boundaries. +Descriptions of `MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK` and `MACRO` headers have one +canonical layout, which never changes their words, their quotes or the text they store (see +[Folded descriptions](/docs/concepts/models/configuration#folded-descriptions)): + +- A description that fits on its line stays one line: `"..."`, or `"""..."""` when it contains a + double quote. A short description wrapped over several lines is joined onto one. +- A longer description, or one with paragraphs, becomes a `"""` block: its lines are indented one + level under the field and wrapped at the line width, with a blank line between paragraphs, and + the closing `"""` lines up with the field. + +```sql +MODEL ( + description """ + True when a name starts with a woman's title: Mme, Mlle, Miss, Mrs, Ms. Any case, with or + without a dot ("MISS ANNA PARKER", "Mrs. C. O'Brien", "Ms.Robinson"). + """, +); +``` + +A one-line `'...'` description that fits is left as written. Running `sqb format` again changes +nothing. Clause layout follows a few fixed rules: @@ -157,6 +176,15 @@ reason, and the run continues. A cycle or the 128-pass limit fails without writi | SQBRSQL030 boolean CASE simplification | Refused | The `COALESCE` rewrite is not proved portable to every supported dialect. | | SQBRSQL042 unused CTE output | Applied when row-neutral | Only a column reference or deterministic scalar expression is removed, from a CTE without `DISTINCT`, `DISTINCT ON`, `GROUP BY ALL`, `ORDER BY ALL`, an ungrouped aggregate, or positional `GROUP BY`/`ORDER BY`. Other removals could change the rows and are refused with the reason. | +`--fix` also rewrites a header description that escapes double quotes as `\"` or doubles them as +`""`, which descriptions do not support, as a `"""..."""` string holding real quotes, and reports +it as an applied `description-quotes` fix. It rewrites only a description whose sole backslashes +escape quotes, and only when the new string stores exactly that text. A file that still has header +syntax errors once its descriptions are rewritten is left as written, and the fix is reported as +refused. When the project cannot compile +until the rewritten descriptions are saved, the run skips the compiler-verified Rule fixes with a +`format-fix-skipped` warning; run `sqb format --fix` again to apply them. + Edits that touch macro or intrinsic expansions are refused, and so are edits outside model SQL bodies, because recompiling cannot prove their output contract. Findings without a proposed edit are reported as unavailable. Human output lists each applied, refused, and unavailable fix with its diff --git a/website/src/content/docs/docs/concepts/macros/sql-macros.mdx b/website/src/content/docs/docs/concepts/macros/sql-macros.mdx index c88fbe3f48..58da2a1492 100644 --- a/website/src/content/docs/docs/concepts/macros/sql-macros.mdx +++ b/website/src/content/docs/docs/concepts/macros/sql-macros.mdx @@ -57,6 +57,11 @@ ROUND(CAST(@amount_cents AS DECIMAL(18, 4)) / 100, @places) `returns` is required for expression macros and not allowed for the other forms. +The description follows the same rules as every header string: write text that contains double +quotes as `"""..."""`, and a wrapped description is stored folded into one flowing text (see +[Header strings](/docs/concepts/models/configuration#header-strings)). Quotes in the macro body +never change how the header is read. + ## Call a SQL macro Every macro call says its kind: `@sql.name(...)` calls a SQL macro and `@py.name(...)` a Python diff --git a/website/src/content/docs/docs/concepts/models/configuration.mdx b/website/src/content/docs/docs/concepts/models/configuration.mdx index 9a66339d57..47894ebf00 100644 --- a/website/src/content/docs/docs/concepts/models/configuration.mdx +++ b/website/src/content/docs/docs/concepts/models/configuration.mdx @@ -78,6 +78,77 @@ does). `sqb format --check` also fails when a `MODEL`, `SCENARIO`, `FUNCTION` or header has no description (a `MODEL` header that uses `model_schema` is left to compile), and `sqb format` still moves a leading comment into an empty model description. +## Header strings + +A quoted value in any SQLBuild header (`MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK`, `MACRO`, +`TEST`, `AUDIT`, `ENUM`, `CONSTANT`) is written as `"..."` or `'...'`. Text that contains a double +quote goes between triple quotes, on one line or across several. Inside `"""..."""`, `"` and `'` +need no escaping: + +```sql +MODEL ( + description """Matches "Miss" and "Mrs." titles, with or without a dot.""", + ... +); +``` + +A `"` inside a `"..."` string ends it, so it is an error with its line and column rather than the +start of an escape. A description has no `\"` escape and no `""` doubling: in a `"..."` +description a backslash is an ordinary character, so `"Files under C:\"` ends with a backslash and +`"Path C:\\temp"` stores both backslashes. In a `'...'` description, `\'` and `''` both stand for +one `'`, as in `'it\'s'` and `'it''s'`. `sqb format --fix` rewrites a description that escapes its +quotes as `\"` or doubles them as `""` into a `"""..."""` string holding real quotes: + +```text +error[D002]: MODEL(...) in 'models/customers.sql:2:34' contains invalid SQLBuild header syntax: a double quote ends this "..." string early; use """…""" for text containing double quotes + = help: write the text between triple quotes, where quotes need no escaping: description """Matches "Miss"."""; sqb format --fix rewrites a description that escapes its quotes as \" or doubles them as "" +``` + +A `"""..."""` string ends at the first three double quotes in a row. Up to two quotes just before +them belong to the text, so the text may end with a quote, but it cannot contain three in a row. +A `"""` inside the text and a `"""` that is never closed fail with the position of the offending +quote or of the opening `"""`. + +### Folded descriptions + +Descriptions are prose, so SQLBuild stores them folded: the same text gives the same description +whether it is written on one line or wrapped, and at any indentation. + +- Text on one line is stored exactly as written. +- Otherwise the spaces at either end are dropped, a line break and the spaces around it become one + space, and a blank line starts a new paragraph (stored as `\n\n`). + +```sql +MODEL ( + description """ + Customers with a "woman's title" flag, + one row each. + + Built from the raw seed. + """, +); +``` + +stores `Customers with a "woman's title" flag, one row each.`, a paragraph break, and `Built from +the raw seed.` + +These strings are folded prose: + +| String | Where | +|--------|-------| +| `description` | The top level of every header. `MODEL`, `SCENARIO`, `SCHEMA`, `FUNCTION`, `HOOK` and `MACRO` headers take one; `AUDIT`, `ENUM`, `CONSTANT` and `TEST` headers reject the key | +| Column `description` | Entries of `columns (...)` in `MODEL` and `SCHEMA` headers | +| Audit `description` | The `description` option of an audit in `audits [...]` | +| Exception `reason` | `reason` of `[[rules.rule_exceptions]]`, `[[rules.graph_edge_exceptions]]`, `[[rules.rule_ignores]]`, `[[rules.select_star_allow]]` and `[[rules.threshold_overrides]]` in `sqlbuild_project.toml` | + +Every other string means exactly what it says and keeps its text as written, line breaks +included: SQL such as audit `predicate` and `where_condition` arguments or `inline_sql(...)` hooks, +regular expressions such as `name_pattern`, literal values in `meta`, `constants`, `placeholders` +and audit arguments, and names and paths. In these strings a backslash still escapes the character +after it inside `"..."` and `'...'`; a `"""..."""` string takes its text exactly as written, +backslashes included. `sqb format` lays descriptions out in one canonical form; see +[Line width and descriptions](/docs/cli/format#line-width-and-descriptions). + ## SQL analysis SQL analysis has project, invocation, and model gates: diff --git a/website/src/content/docs/docs/concepts/rules/findings-and-exceptions.mdx b/website/src/content/docs/docs/concepts/rules/findings-and-exceptions.mdx index 5ce22f0539..6088e8936f 100644 --- a/website/src/content/docs/docs/concepts/rules/findings-and-exceptions.mdx +++ b/website/src/content/docs/docs/concepts/rules/findings-and-exceptions.mdx @@ -24,6 +24,10 @@ reason = "This example intentionally demonstrates one sampled row." An exception refers to one exact Rule code and path. If the finding disappears, SQLBuild reports the stale exception so obsolete configuration does not accumulate silently. +A reason is prose. One written as a multi-line TOML string (`"""..."""`) is stored folded, like a +description: line breaks and the spaces around them become one space, and a blank line starts a +paragraph (see [Folded descriptions](/docs/concepts/models/configuration#folded-descriptions)). + ## Relation-level exceptions A file-wide exception for a deliberate cartesian product would also hide any accidental cross join