fix: lint SQL macro bodies as compiled and pass SQL macro expansions to Python macros intact (#1236)

* fix: lint SQL macro bodies as compiled and pass SQL macro expansions to Python macros intact

Linting a SQL macro file read a parameter followed by words and a
parenthesis, such as `@value IS NULL OR TRIM(@value) = ''`, as an
unprefixed macro call (L005), and a body opening with two parentheses
reached the native parser as a statement it rejected (rules-unevaluated).

- One rule decides an unprefixed call everywhere: `@name(` with the
  parenthesis right after the name. The Python macro call scanner, the
  SQL macro lexer, parameter references and the bare-call check share
  it, so `@name IS NULL OR f(x)` in model SQL is no longer P014 either.
  `@const`, `@enum` and `@param` stay declarations with or without a
  space before their parenthesis.
- Lint replaces a SQL macro body's parameters by sentinels with the
  compiler's own parameter scan, lints an expression body as the
  `SELECT` the compiler parses it as, and a CTE-list body with its
  ceremonial `SELECT 1` and its CTEs counted as used by the caller.
  Findings in the generated frame are dropped.
- A SQL macro expansion passed to a Python macro is spliced as a
  string literal with `"` written as `\x22`, which the call scanner
  and the argument parser read the same way, so quotes, backslashes,
  line breaks and comments reach the macro unchanged.
- An expansion ending in a line comment gets a line break only when
  text would follow it on the same line, so the caller's text stays code
  and calls already last on their line compile unchanged.
- A macro call the scan cannot read is a MacroArgumentError naming the
  call, file, line and column instead of a location-less P001.
- Rules on a caller skip comments a SQL macro expansion generated; the
  macro file is where they are judged, and compiled SQL keeps them.
  Comments from Python macro output are judged as before.

* fix: build authored lint lookups after the lint batch so SQL macro comment blanking adds no peak memory

Author: kvlonge

crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs

@@ -295,6 +295,39 @@ pub(crate) fn expand_body(
     Ok(BodyExpansion { sql, names })
 }
 
+/// `sql`, with a line break when it ends in a `--` comment and `followed` text would join that line.
+pub(crate) fn with_line_comment_closed(sql: &str, followed: bool) -> String {
+    let mut sql: String = sql.to_owned();
+    if !followed {
+        return sql;
+    }
+    let Ok(scanned) = tokens(&sql) else {
+        return sql;
+    };
+    let mut rest: &str = &sql[scanned.last().map_or(0, |last| last.end)..];
+    let mut open_line_comment: bool = false;
+    loop {
+        rest = rest.trim_start();
+        if let Some(comment) = rest.strip_prefix("--") {
+            match comment.split_once('\n') {
+                Some((_, after)) => rest = after,
+                None => {
+                    open_line_comment = true;
+                    break;
+                }
+            }
+        } else if let Some(comment) = rest.strip_prefix("/*") {
+            rest = comment.split_once("*/").map_or("", |(_, after)| after);
+        } else {
+            break;
+        }
+    }
+    if open_line_comment {
+        sql.push('\n');
+    }
+    sql
+}
+
 /// The text after its first token, the body's `WITH` or `SELECT`, trimmed.
 fn without_leading_keyword(text: &str) -> String {
     match tokens(text) {

crates/sqlbuild-analysis/src/sql_macros/_helpers/definition.rs

@@ -106,7 +106,7 @@ fn unclosed_body(name: &str, location: SqlMacroLocation) -> SqlMacroError {
 }
 
 /// The form a body's first token decides.
-fn body_form(body: &str, scanned: &[Token]) -> SqlMacroForm {
+pub(crate) fn body_form(body: &str, scanned: &[Token]) -> SqlMacroForm {
     match scanned.first() {
         Some(first) if first.is_word(body, WITH_KEYWORD) => SqlMacroForm::CteList,
         Some(first) if first.is_word(body, SELECT_KEYWORD) => SqlMacroForm::ColumnList,

crates/sqlbuild-analysis/src/sql_macros/_helpers/expansion.rs

@@ -6,7 +6,7 @@ use crate::compiler::main::function_type_validation::function_type_error;
 use crate::sql_macros::_helpers::arguments::{
     ShapeContext, argument_shape, bind_arguments, check_shape, is_atomic, split_arguments,
 };
-use crate::sql_macros::_helpers::body::expand_body;
+use crate::sql_macros::_helpers::body::{expand_body, with_line_comment_closed};
 use crate::sql_macros::_helpers::lexing::{AtConstruct, at_constructs, outer_sql_calls};
 use crate::sql_macros::_helpers::lexing::{Token, line_column, tokens};
 use crate::sql_macros::_helpers::literal_positions::{
@@ -209,7 +209,10 @@ pub(crate) fn expand_call<H: SqlMacroHost>(
     let expansion: BodyExpansion = expand_body(&definition, &body, &rendered)
         .map_err(|error| error.at(body_location(&definition)))?;
     if call.in_python {
-        return Ok((python_string(&expansion.sql), inserted));
+        return Ok((
+            python_string(&with_line_comment_closed(&expansion.sql, true)),
+            inserted,
+        ));
     }
     if matches!(context.scope, SqlMacroScope::Caller) && context.stack.is_empty() {
         let check: SqlMacroCheck = SqlMacroCheck {
@@ -229,8 +232,17 @@ pub(crate) fn expand_call<H: SqlMacroHost>(
         (call.start, call.end),
         call.replaced_end,
     );
-    placed(&definition, (&site, inserted), &expansion, context.owner)
-        .map_err(ExpansionFailure::Macro)
+    let line_continues: bool = context.text[call.replaced_end..]
+        .split('\n')
+        .next()
+        .is_some_and(|rest| !rest.trim().is_empty());
+    placed(
+        &definition,
+        (&site, inserted, line_continues),
+        &expansion,
+        context.owner,
+    )
+    .map_err(ExpansionFailure::Macro)
 }
 
 /// One bound argument checked against its parameter and rendered from `scope`.
@@ -306,14 +318,14 @@ fn rendered_argument<H: SqlMacroHost>(
     })
 }
 
-/// `sql` as a double-quoted Python string literal.
+/// `sql` as a double-quoted Python string literal holding no `"` and no line break.
 fn python_string(sql: &str) -> String {
     let mut quoted: String = String::with_capacity(sql.len() + 2);
     quoted.push('"');
     for character in sql.chars() {
         match character {
             '\\' => quoted.push_str("\\\\"),
-            '"' => quoted.push_str("\\\""),
+            '"' => quoted.push_str("\\x22"),
             '\n' => quoted.push_str("\\n"),
             '\r' => quoted.push_str("\\r"),
             '\t' => quoted.push_str("\\t"),
@@ -327,10 +339,17 @@ fn python_string(sql: &str) -> String {
 /// The expansion placed at the call, or why it cannot stand there or clashes.
 fn placed(
     definition: &SqlMacroDefinition,
-    (site, inserted): (&CallSite<'_>, InsertedNames),
+    (site, inserted, line_continues): (&CallSite<'_>, InsertedNames, bool),
     expansion: &BodyExpansion,
     owner: &str,
 ) -> Result<(String, InsertedNames), SqlMacroError> {
+    let closed = |after: &str| {
+        let followed: bool = match after.chars().next() {
+            None => line_continues,
+            Some(next) => next != '\n' && next != '\r',
+        };
+        with_line_comment_closed(&expansion.sql, followed)
+    };
     let position: CallPosition = site.position();
     let name: &str = &definition.name;
     match definition.form {
@@ -341,9 +360,9 @@ fn placed(
             }
             let comma: &str = site.expression_separator();
             if site.expression_needs_parentheses() && !is_atomic(&expansion.sql) {
-                Ok((format!("({}){comma}", expansion.sql), inserted))
+                Ok((format!("({}){comma}", closed(")")), inserted))
             } else {
-                Ok((format!("{}{comma}", expansion.sql), inserted))
+                Ok((format!("{}{comma}", closed(comma)), inserted))
             }
         }
         SqlMacroForm::CteList => {
@@ -365,7 +384,7 @@ fn placed(
                 ("CTE", owner),
             )?;
             let (before, after) = site.cte_separators();
-            Ok((format!("{before}{}{after}", expansion.sql), inserted))
+            Ok((format!("{before}{}{after}", closed(after)), inserted))
         }
         SqlMacroForm::ColumnList => {
             if position != CallPosition::SelectList {
@@ -395,7 +414,7 @@ fn placed(
                 ("column", owner),
             )?;
             let (before, after) = site.column_separators();
-            Ok((format!("{before}{}{after}", expansion.sql), inserted))
+            Ok((format!("{before}{}{after}", closed(after)), inserted))
         }
     }
 }

crates/sqlbuild-analysis/src/sql_macros/_helpers/lexing.rs

@@ -1,5 +1,7 @@
 //! 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 crate::sql_macros::constants::SQL_KIND_WORD;
 use crate::sql_macros::errors::LexError;
 
@@ -45,7 +47,7 @@ impl Token {
 
 const PUNCTUATION: &[u8] = b"()[]{},.;";
 /// The `@word(...)` declaration forms that are not macro calls.
-const DECLARATION_WORDS: [&str; 3] = ["const", "enum", "param"];
+pub(crate) const DECLARATION_WORDS: [&str; 3] = ["const", "enum", "param"];
 
 /// Every token of `sql`; text unclosed under doubled-quote escapes is reread with backslash ones.
 pub(crate) fn tokens(sql: &str) -> Result<Vec<Token>, LexError> {
@@ -341,7 +343,7 @@ pub(crate) fn at_constructs(sql: &str) -> Result<Vec<AtConstruct>, LexError> {
             });
             continue;
         }
-        if scanned.get(index + 2).is_some_and(|open| open.is(sql, "(")) {
+        if opens_bare_call(sql, word.end) {
             found.push(AtConstruct::Bare {
                 name: word_text.to_owned(),
                 start: token.start,

crates/sqlbuild-analysis/src/sql_macros/_helpers/parameters.rs

@@ -1,6 +1,8 @@
 //! `@name` parameter references in a SQL macro body, as generic audits write their parameters.
 
-use crate::sql_macros::_helpers::lexing::{Token, TokenKind};
+use sqlbuild_render::macro_calls::main::opens_bare_call::opens_bare_call;
+
+use crate::sql_macros::_helpers::lexing::{DECLARATION_WORDS, Token, TokenKind};
 
 const PLACEHOLDER_PREFIX: &str = "__sqlbuild_parameter__";
 /// Words that may follow a table reference without being its alias.
@@ -68,10 +70,12 @@ pub(crate) fn parameter_references(sql: &str, scanned: &[Token]) -> Vec<Paramete
             continue;
         };
         let after: Option<&Token> = scanned.get(index + 2);
-        if after.is_some_and(|open| open.is(sql, "(")) {
+        let name: &str = word.text(sql);
+        let declaration: bool =
+            DECLARATION_WORDS.contains(&name) && after.is_some_and(|open| open.is(sql, "("));
+        if declaration || opens_bare_call(sql, word.end) {
             continue;
         }
-        let name: &str = word.text(sql);
         if matches!(name, "sql" | "py")
             && after.is_some_and(|dot| dot.is(sql, ".") && dot.start == word.end)
             && scanned.get(index + 4).is_some_and(|open| open.is(sql, "("))

crates/sqlbuild-analysis/src/sql_macros/main/bare_macro_calls.rs

@@ -1,10 +1,11 @@
 use crate::sql_macros::_helpers::lexing::{AtConstruct, at_constructs};
 use crate::sql_macros::constants::NON_ASCII_BYTE;
+use sqlbuild_render::macro_calls::main::opens_bare_call::opens_bare_call;
 
 /// The name and byte offset of each `@name(...)` call in `text` that names no macro kind.
 #[must_use]
 pub fn bare_macro_calls(text: &str) -> Vec<(String, usize)> {
-    if !may_hold_bare_call(text.as_bytes()) {
+    if !may_hold_bare_call(text) {
         return Vec::new();
     }
     at_constructs(text)
@@ -18,7 +19,8 @@ pub fn bare_macro_calls(text: &str) -> Vec<(String, usize)> {
 }
 
 /// Whether an `@` followed by a name and `(` that is not a kind prefix or declaration occurs.
-fn may_hold_bare_call(bytes: &[u8]) -> bool {
+fn may_hold_bare_call(text: &str) -> bool {
+    let bytes: &[u8] = text.as_bytes();
     for at in memchr::memchr_iter(b'@', bytes) {
         let start: usize = at + 1;
         let mut end: usize = start;
@@ -36,7 +38,7 @@ fn may_hold_bare_call(bytes: &[u8]) -> bool {
         ) {
             continue;
         }
-        if bytes.get(end) == Some(&b'(') {
+        if opens_bare_call(text, end) {
             return true;
         }
     }

crates/sqlbuild-analysis/src/sql_macros/main/mod.rs

@@ -4,6 +4,8 @@ pub mod declaration_visible_from_macro;
 pub mod expand_sql_macro_call;
 pub mod parse_sql_macro;
 pub mod render_inline_collection;
+pub mod sql_macro_body_form;
 pub mod sql_macro_calls;
 pub mod sql_macro_cycle;
+pub mod sql_macro_parameter_spans;
 pub mod with_list_sql_macro_calls;

crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_body_form.rs

@@ -0,0 +1,14 @@
+//! The form of a SQL macro body, as its definition reads it.
+
+use crate::sql_macros::_helpers::definition::body_form;
+use crate::sql_macros::_helpers::lexing::tokens;
+use crate::sql_macros::models::SqlMacroForm;
+
+/// Whether `body` is a CTE list, a column list or an expression, from its first token.
+#[must_use]
+pub fn sql_macro_body_form(body: &str) -> SqlMacroForm {
+    match tokens(body) {
+        Ok(scanned) => body_form(body, &scanned),
+        Err(_) => SqlMacroForm::Expression,
+    }
+}

crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_parameter_spans.rs

@@ -0,0 +1,16 @@
+//! The byte spans of a SQL macro body's `@name` parameter references, as expansion reads them.
+
+use crate::sql_macros::_helpers::lexing::tokens;
+use crate::sql_macros::_helpers::parameters::parameter_references;
+
+/// The `start..end` byte span of each `@name` in `body` that expansion replaces by an argument.
+#[must_use]
+pub fn sql_macro_parameter_spans(body: &str) -> Vec<(usize, usize)> {
+    let Ok(scanned) = tokens(body) else {
+        return Vec::new();
+    };
+    parameter_references(body, &scanned)
+        .into_iter()
+        .map(|reference| (reference.start, reference.end))
+        .collect()
+}

crates/sqlbuild-analysis/src/sql_macros/tests/helpers.rs

@@ -5,7 +5,7 @@ use crate::sql_macros::errors::ExpansionFailure;
 use crate::sql_macros::errors::SqlMacroError;
 use crate::sql_macros::main::parse_sql_macro::parse_sql_macro;
 use crate::sql_macros::models::{SqlMacroDefinition, TextContext};
-use crate::sql_macros::tests::test_types::FakeHost;
+use crate::sql_macros::tests::test_types::{AsciiArguments, FakeHost};
 use crate::sql_macros::types::{ConstantShape, SqlMacroHost, SqlMacroScope};
 
 /// Constants and enum members the fake host renders, by reference text.
@@ -116,3 +116,13 @@ fn macro_error(failure: ExpansionFailure<String>) -> SqlMacroError {
 pub(super) fn owned(names: &[&str]) -> Vec<String> {
     names.iter().map(|name| (*name).to_owned()).collect()
 }
+
+impl sqlbuild_render::macro_arguments::types::ArgumentHost for AsciiArguments {
+    fn character_named(&self, _name: &str) -> Option<String> {
+        None
+    }
+
+    fn identifier(&self, _text: &str) -> Option<String> {
+        None
+    }
+}