feat!: fold header descriptions and accept triple-quoted strings (#1242)

* feat: fold prose header strings and accept triple-quoted strings

Descriptions in MODEL, SCENARIO, SCHEMA, FUNCTION, HOOK and MACRO headers,
column descriptions, audit description options and Rule exception reasons
are stored as folded prose: a line break and the spaces around it are one
space, and a blank line is a paragraph break. Strings that are meaningful
as written (SQL, regexes, literals, meta values) keep their exact text.

Any header string may be written between triple quotes, where quotes need
no escaping. A backslash-escaped double quote in a prose string, a quote
that ends a string inside its text, and an unclosed or inner triple quote
fail with their line and column. MACRO header strings no longer depend on
quotes in the macro body.

* feat: lay out header descriptions canonically in sqb format

A description that fits stays one line, written as "..." or, when it
holds a double quote, as """...""". A longer one becomes an indented
""" block wrapped at the line width, and a short wrapped one is joined
onto one line. FUNCTION, HOOK, SCHEMA and MACRO descriptions are laid out
too. Layout never changes the stored text: each rewrite is checked to fold
back to the same description.

* feat: rewrite escaped description quotes with sqb format --fix

sqb format --fix rewrites a description that escapes double quotes with \"
as a """...""" string holding real quotes, reported as an applied
description-quotes fix. The rewrite is made only when the string's sole
backslashes escape quotes and the new string stores exactly that text.
When the project cannot compile until the rewrites are saved, Rule fixes
are skipped with a warning to rerun.

* docs: document header strings, folded descriptions and their layout

* test: add escaped description quote cases to the failure corpus

* fix: count description length as wrapped to the full line width

A """ block indents its lines under the field, so counting its lines
would make a description at the line limit fail description-length after
formatting. Count the description wrapped from its field to the full width,
as before, and update tests for folded descriptions.

* test: record goldens for escaped description quote cases

* test: use neutral example names and shard the description layout E2E

* style: satisfy clippy byte string lint

* refactor: satisfy structure checks for header strings and description fixes

* fix: read description quotes by one prose rule in every header

A double-quoted description takes backslashes literally, so it may end in
one; a single-quoted description reads \' and '' as a quote. MODEL and
MACRO headers and every header scanner share this rule, and only prose
descriptions reject a quote that runs straight on into more text, so other
header strings tokenize as before.

* fix: never change a SQL macro when laying out its description

The canonical description layout and the MACRO header layout now keep a
change only when the macro parses to the same definition afterwards.

* fix: skip triple-quoted header strings in AUDIT and contract scanners

The AUDIT header scanner and the MODEL contract type scanner use the
shared header string rules instead of their own quote handling.

* feat: rewrite doubled description quotes with sqb format --fix

A description that doubles its quotes as "" is rewritten as a triple-quoted
string like one that escapes them as \", and the quote error's help names
both.

* fix: apply description quote fixes only when the whole file then parses

A file whose headers, MACRO headers included, still fail once its
descriptions are rewritten is left as written, and the fix is reported as
refused.

* docs: list every folded description and the description quoting rules

* fix: measure a description's layout by its own line, not later header fields

A one-line header such as FUNCTION (description '...', arguments (...),
returns ...) kept its short description as written before; the layout
counted the fields after it and turned it into a block. Fields that follow
on the line no longer count towards whether a description fits.

* test: write reference-error header descriptions in the supported triple-quoted form

The two cases put a description with an escaped quote and an apostrophe
before a body reference error. Descriptions have no \" escape any more, so
they now hold the same text as a triple-quoted string, and still check that
the body error is located, typed and names the file.

Author: kvlonge

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 \

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(),
     }
 }

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<Token> = 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<String, SqlMacroError> {
+        let raw: Vec<char> = 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<SqlMacroParameter, SqlMacroError> {
         let source: &str = self.source;
         let name: &Token = &tokens[0];

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<Vec<Token>, LexE
     let bytes: &[u8] = sql.as_bytes();
     let mut found: Vec<Token> = 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<Vec<Token>, 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<Vec<Token>, 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<usize, LexError> {
+    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,

crates/sqlbuild-analysis/src/sql_macros/errors.rs

@@ -88,4 +88,12 @@ impl<E> From<SqlMacroError> for ExpansionFailure<E> {
 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),
 }

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 = [

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),
+}