From 922a03e2e4bba61e9ad0d4f673faba24d72cd7a3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:10:18 +0100 Subject: [PATCH 1/2] feat(native): report declaration and scope errors natively --- .../sqltext/declaration_references.rs | 27 ++- .../reference_syntax.rs | 52 +++--- .../declaration_references/scanner.rs | 84 ++++++--- .../compiler/main/declaration_references.rs | 6 +- .../sqlbuild-sqltext/src/compiler/models.rs | 18 ++ .../src/compiler/tests/helpers.rs | 64 ++++--- .../tests/test_declaration_references.rs | 33 +++- .../_helpers/corpus/scope_failure_cases.py | 74 +++++++- src/sqlbuild/_native.pyi | 2 +- .../compile/_helpers/attachment/core.py | 4 +- .../compile/_helpers/render/declarations.py | 168 +++++++++++------- src/sqlbuild/compiler/compile/exceptions.py | 4 + .../_helpers/native/declaration_files.py | 67 +------ .../macro_bridge/classes/macro_bridge.py | 8 + .../main/run_with_macro_bridge.py | 7 +- src/sqlbuild/compiler/model_loop/constants.py | 4 + .../_scan_native_declaration_references.py | 10 +- src/sqlbuild/compiler/model_loop/types.py | 1 + .../cli/commands/main/compile/_test_types.py | 10 ++ .../compile/test_native_declaration_errors.py | 138 ++++++++++++++ .../compiler/discovery/_test_types.py | 5 +- .../test_native_declaration_files.py | 26 +-- .../compiler/model_loop/_test_types.py | 4 +- .../sqlbuild/compiler/model_loop/helpers.py | 114 +++++++++--- .../test_native_declaration_references.py | 23 +-- .../compiler/macro_bridge/main/helpers.py | 23 ++- .../main/test_run_with_macro_bridge.py | 30 +++- 27 files changed, 718 insertions(+), 288 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py diff --git a/crates/sqlbuild-python/src/bindings/_helpers/sqltext/declaration_references.rs b/crates/sqlbuild-python/src/bindings/_helpers/sqltext/declaration_references.rs index 3cb89d7c60..7580cc5c9d 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/sqltext/declaration_references.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/sqltext/declaration_references.rs @@ -3,28 +3,41 @@ use pyo3::prelude::{Bound, PyModule, PyModuleMethods, PyResult, Python}; use pyo3::{pyfunction, wrap_pyfunction}; use sqlbuild_sqltext::compiler::main::declaration_references::scan_declaration_references; -use sqlbuild_sqltext::compiler::models::{DeclarationReference, DeclarationReferenceKind}; +use sqlbuild_sqltext::compiler::models::{ + DeclarationReference, DeclarationReferenceKind, DeclarationReferenceScan, + DeclarationReferenceStop, +}; use crate::bindings::_helpers::boundary::panics::compiler_error; use crate::bindings::types::CompilerDetach; /// `(kind code, name, member, start, end)`: kind 0 is an enum member, 1 a constant. type ReferenceRow = (u8, String, Option, usize, usize); +/// References and stop code: unclosed quote 0, block comment 1, invalid enum 2, constant 3. +type ScanRow = (Vec, Option); -/// Return each SQL string's references in order, or `None` where Python must expand it. +/// Return each SQL string's references and stopping error, or `None` where Python must expand it. #[pyfunction] fn scan_sql_declaration_references( py: Python<'_>, sqls: Vec, -) -> PyResult>>> { - let scanned: Vec>> = py +) -> PyResult>> { + let scanned: Vec> = py .compiler_detach(|| Ok(scan_declaration_references(&sqls))) .map_err(compiler_error)?; - Ok(scanned.into_iter().map(reference_rows).collect()) + Ok(scanned.into_iter().map(|scan| scan.map(scan_row)).collect()) } -fn reference_rows(references: Option>) -> Option> { - Some(references?.into_iter().map(reference_row).collect()) +fn scan_row(scan: DeclarationReferenceScan) -> ScanRow { + ( + scan.references.into_iter().map(reference_row).collect(), + scan.stop.map(|stop| match stop { + DeclarationReferenceStop::UnclosedQuote => 0, + DeclarationReferenceStop::UnclosedBlockComment => 1, + DeclarationReferenceStop::Malformed(DeclarationReferenceKind::Enum) => 2, + DeclarationReferenceStop::Malformed(DeclarationReferenceKind::Constant) => 3, + }), + ) } fn reference_row(reference: DeclarationReference) -> ReferenceRow { diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/reference_syntax.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/reference_syntax.rs index cf18fe4765..0f4803a502 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/reference_syntax.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/reference_syntax.rs @@ -1,5 +1,7 @@ //! Python's `@enum("name").MEMBER` and `@const("name")` patterns at one reference start. +use sqlbuild_core::text::main::is_python_space::is_python_space; + use crate::compiler::models::DeclarationReferenceKind; /// One matched reference: kind, name, member and the byte offset after it. @@ -10,12 +12,14 @@ pub(crate) struct MatchedReference<'sql> { pub(crate) end: usize, } -/// Why a reference start cannot be matched natively. +/// What Python's patterns find at one `@`. pub(crate) enum ReferenceSyntax<'sql> { /// `@enum`/`@const` followed by a word character: not a reference, keep scanning. NotReference, Matched(MatchedReference<'sql>), - /// Malformed or Unicode-dependent text: Python raises or decides. + /// A reference start whose full pattern does not match: Python raises an invalid reference. + Malformed(DeclarationReferenceKind), + /// A non-ASCII character after the keyword, whose word boundary only Python decides. Deferred, } @@ -30,14 +34,20 @@ pub(crate) fn match_reference(sql: &str, start: usize) -> ReferenceSyntax<'_> { return ReferenceSyntax::NotReference; }; match bytes.get(keyword_end) { - Some(byte) if !byte.is_ascii() => return ReferenceSyntax::Deferred, + Some(byte) + if !byte.is_ascii() + && !sql[keyword_end..] + .chars() + .next() + .is_some_and(is_python_space) => + { + return ReferenceSyntax::Deferred; + } Some(byte) if is_word(*byte) => return ReferenceSyntax::NotReference, _ => {} } - let Some(matched) = match_arguments(sql, kind, keyword_end) else { - return ReferenceSyntax::Deferred; - }; - ReferenceSyntax::Matched(matched) + match_arguments(sql, kind, keyword_end) + .map_or(ReferenceSyntax::Malformed(kind), ReferenceSyntax::Matched) } fn match_arguments( @@ -46,19 +56,19 @@ fn match_arguments( keyword_end: usize, ) -> Option> { let bytes: &[u8] = sql.as_bytes(); - let mut index: usize = consume(bytes, skip_whitespace(bytes, keyword_end)?, b'(')?; - index = skip_whitespace(bytes, index)?; + let mut index: usize = consume(bytes, skip_whitespace(sql, keyword_end), b'(')?; + index = skip_whitespace(sql, index); let quote: u8 = *bytes .get(index) .filter(|byte| matches!(byte, b'\'' | b'"'))?; let name_start: usize = index + 1; let name_end: usize = identifier_end(bytes, name_start)?; index = consume(bytes, name_end, quote)?; - index = consume(bytes, skip_whitespace(bytes, index)?, b')')?; + index = consume(bytes, skip_whitespace(sql, index), b')')?; let mut member: Option<&str> = None; if matches!(kind, DeclarationReferenceKind::Enum) { - index = consume(bytes, skip_whitespace(bytes, index)?, b'.')?; - let member_start: usize = skip_whitespace(bytes, index)?; + index = consume(bytes, skip_whitespace(sql, index), b'.')?; + let member_start: usize = skip_whitespace(sql, index); index = identifier_end(bytes, member_start)?; member = Some(&sql[member_start..index]); } @@ -70,21 +80,15 @@ fn match_arguments( }) } -/// Python's `\s*` over ASCII; a non-ASCII byte may be Unicode whitespace, so it defers. -fn skip_whitespace(bytes: &[u8], mut index: usize) -> Option { - while let Some(byte) = bytes.get(index) { - if !byte.is_ascii() { - return None; - } - if !matches!( - byte, - b' ' | b'\t' | b'\n' | b'\r' | 0x0b | 0x0c | 0x1c..=0x1f - ) { +/// Python's `\s*`: every character `str.isspace()` accepts. +fn skip_whitespace(sql: &str, mut index: usize) -> usize { + while let Some(character) = sql[index..].chars().next() { + if !is_python_space(character) { break; } - index += 1; + index += character.len_utf8(); } - Some(index) + index } fn consume(bytes: &[u8], index: usize, expected: u8) -> Option { diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/scanner.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/scanner.rs index f44a3fda6a..8fa714ab15 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/scanner.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/declaration_references/scanner.rs @@ -3,21 +3,43 @@ use crate::compiler::_helpers::declaration_references::reference_syntax::{ ReferenceSyntax, match_reference, }; -use crate::compiler::models::DeclarationReference; +use crate::compiler::models::{ + DeclarationReference, DeclarationReferenceScan, DeclarationReferenceStop, +}; const SPECIAL_BYTES: &[u8] = b"'\"`$@/-"; -/// Every `@enum`/`@const` reference in `sql` with code-point offsets, or None when Python decides. -pub(crate) fn scan_references(sql: &str) -> Option> { +/// Where the walk for the next reference start ended. +enum NextStart { + Reference(usize), + End, + Stop(DeclarationReferenceStop), + Deferred, +} + +/// Python's references in `sql` (code points) and its stopping error; None defers to Python. +pub(crate) fn scan_references(sql: &str) -> Option { let mut references: Vec = Vec::new(); let mut offsets: CharOffsets = CharOffsets::default(); let mut cursor: usize = 0; + let mut stop: Option = None; while cursor < sql.len() { - let Some(start) = next_reference_start(sql, cursor)? else { - break; + let start: usize = match next_reference_start(sql, cursor) { + NextStart::Reference(start) => start, + NextStart::End => break, + NextStart::Stop(found) => { + stop = Some(found); + break; + } + NextStart::Deferred => return None, }; - let ReferenceSyntax::Matched(matched) = match_reference(sql, start) else { - return None; + let matched = match match_reference(sql, start) { + ReferenceSyntax::Matched(matched) => matched, + ReferenceSyntax::Malformed(kind) => { + stop = Some(DeclarationReferenceStop::Malformed(kind)); + break; + } + ReferenceSyntax::NotReference | ReferenceSyntax::Deferred => return None, }; let start_char: usize = offsets.advance(sql, start); let end_char: usize = offsets.advance(sql, matched.end); @@ -30,14 +52,14 @@ pub(crate) fn scan_references(sql: &str) -> Option> { }); cursor = matched.end; } - Some(references) + Some(DeclarationReferenceScan { references, stop }) } -/// The next reference start at or after `start`; the outer None means Python decides. -fn next_reference_start(sql: &str, start: usize) -> Option> { +/// The next reference start at or after `start`, as Python's walk finds it. +fn next_reference_start(sql: &str, start: usize) -> NextStart { let rest: &str = &sql[start..]; if !rest.contains("@enum") && !rest.contains("@const") { - return Some(None); + return NextStart::End; } let bytes: &[u8] = sql.as_bytes(); let mut index: usize = start; @@ -46,27 +68,35 @@ fn next_reference_start(sql: &str, start: usize) -> Option> { .position(|byte| SPECIAL_BYTES.contains(byte)) { index += offset; - index = match bytes[index] { - b'\'' | b'"' => quoted_end(bytes, index, true)?, - b'`' => quoted_end(bytes, index, false)?, - b'$' => dollar_end(bytes, index)?.unwrap_or(index + 1), - b'-' if bytes.get(index + 1) == Some(&b'-') => bytes[index..] - .iter() - .position(|byte| *byte == b'\n') - .map_or(bytes.len(), |newline| index + newline + 1), - b'/' if bytes.get(index + 1) == Some(&b'*') => { - index + 2 + find(&bytes[index + 2..], b"*/")? + 2 - } + let next: Option = match bytes[index] { + b'\'' | b'"' => quoted_end(bytes, index, true), + b'`' => quoted_end(bytes, index, false), + b'$' => dollar_end(bytes, index).map(|end| end.unwrap_or(index + 1)), + b'-' if bytes.get(index + 1) == Some(&b'-') => Some( + bytes[index..] + .iter() + .position(|byte| *byte == b'\n') + .map_or(bytes.len(), |newline| index + newline + 1), + ), + b'/' if bytes.get(index + 1) == Some(&b'*') => match find(&bytes[index + 2..], b"*/") { + Some(close) => Some(index + 2 + close + 2), + None => return NextStart::Stop(DeclarationReferenceStop::UnclosedBlockComment), + }, b'@' => match match_reference(sql, index) { - ReferenceSyntax::NotReference => index + 1, - ReferenceSyntax::Matched(_) | ReferenceSyntax::Deferred => { - return Some(Some(index)); + ReferenceSyntax::NotReference => Some(index + 1), + ReferenceSyntax::Matched(_) | ReferenceSyntax::Malformed(_) => { + return NextStart::Reference(index); } + ReferenceSyntax::Deferred => return NextStart::Deferred, }, - _ => index + 1, + _ => Some(index + 1), }; + match next { + Some(next) => index = next, + None => return NextStart::Stop(DeclarationReferenceStop::UnclosedQuote), + } } - Some(None) + NextStart::End } /// Python's `quoted_text_end_impl` for `'`, `"` (doubled quotes escape) and backticks. diff --git a/crates/sqlbuild-sqltext/src/compiler/main/declaration_references.rs b/crates/sqlbuild-sqltext/src/compiler/main/declaration_references.rs index f6fe439602..fa87a2efdc 100644 --- a/crates/sqlbuild-sqltext/src/compiler/main/declaration_references.rs +++ b/crates/sqlbuild-sqltext/src/compiler/main/declaration_references.rs @@ -3,10 +3,10 @@ use rayon::prelude::{IntoParallelRefIterator, ParallelIterator}; use crate::compiler::_helpers::declaration_references::scanner::scan_references; -use crate::compiler::models::DeclarationReference; +use crate::compiler::models::DeclarationReferenceScan; -/// Each SQL string's declaration references in order, or None where Python must expand it. +/// Each SQL string's references and stopping error, or None where Python must expand it. #[must_use] -pub fn scan_declaration_references(sqls: &[String]) -> Vec>> { +pub fn scan_declaration_references(sqls: &[String]) -> Vec> { sqls.par_iter().map(|sql| scan_references(sql)).collect() } diff --git a/crates/sqlbuild-sqltext/src/compiler/models.rs b/crates/sqlbuild-sqltext/src/compiler/models.rs index fa15867ec1..3780be29c5 100644 --- a/crates/sqlbuild-sqltext/src/compiler/models.rs +++ b/crates/sqlbuild-sqltext/src/compiler/models.rs @@ -42,6 +42,24 @@ pub struct DeclarationReference { pub end: usize, } +/// The error at which Python's `@enum`/`@const` expansion of a string stops. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum DeclarationReferenceStop { + /// Quoted or dollar-quoted text before a later reference is never closed. + UnclosedQuote, + /// A block comment before a later reference is never closed. + UnclosedBlockComment, + /// `@enum` or `@const` starts a reference its full pattern does not match. + Malformed(DeclarationReferenceKind), +} + +/// The references Python resolves in a string, in order, and the error that ends its walk. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct DeclarationReferenceScan { + pub references: Vec, + pub stop: Option, +} + /// A header whose values nest deeper than the parser allows, located in its file. #[derive(Clone, Debug, PartialEq, Eq)] pub struct NestingFailure { diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs index 3e168ad8a3..3c9fd20066 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs @@ -7,7 +7,9 @@ use crate::compiler::_helpers::sql_interpolation::substitution::{ use crate::compiler::_helpers::sql_references::extraction::extract; use crate::compiler::main::declaration_references::scan_declaration_references; use crate::compiler::main::model_header_single_parsing::parse_one; -use crate::compiler::models::{AuthoredValue, DeclarationReference}; +use crate::compiler::models::{ + AuthoredValue, DeclarationReferenceKind, DeclarationReferenceScan, DeclarationReferenceStop, +}; use crate::compiler::tests::test_types::HeaderNestingTestCase; use std::thread; @@ -251,32 +253,52 @@ pub(crate) fn batch_sizes_bound_workers_by_contract() -> bool { true } -/// Scan one SQL string and spell its references as `kind:name[.member]@start..end, ...`. +/// Spell a scan as `kind:name[.member]@start..end, ...` plus ` | stop:` when it stops. pub(crate) fn scanned_references(sql: &str) -> Option { - let references: Vec = scan_declaration_references(&[sql.to_owned()]) + let scan: DeclarationReferenceScan = scan_declaration_references(&[sql.to_owned()]) .pop() .flatten()?; - Some( - references + let references: String = scan + .references + .iter() + .map(|reference| { + let member: String = reference + .member + .as_ref() + .map_or_else(String::new, |member| format!(".{member}")); + format!( + "{}:{}{member}@{}..{}", + reference.kind.keyword(), + reference.name, + reference.start, + reference.end + ) + }) + .collect::>() + .join(", "); + let stop: String = scan.stop.map_or_else(String::new, |stop| { + STOP_NAMES .iter() - .map(|reference| { - let member: String = reference - .member - .as_ref() - .map_or_else(String::new, |member| format!(".{member}")); - format!( - "{}:{}{member}@{}..{}", - reference.kind.keyword(), - reference.name, - reference.start, - reference.end - ) - }) - .collect::>() - .join(", "), - ) + .find(|(candidate, _)| *candidate == stop) + .map(|(_, name)| format!(" | stop:{name}")) + .expect("every stop has a name") + }); + Some(format!("{references}{stop}")) } +const STOP_NAMES: [(DeclarationReferenceStop, &str); 4] = [ + (DeclarationReferenceStop::UnclosedQuote, "quote"), + (DeclarationReferenceStop::UnclosedBlockComment, "comment"), + ( + DeclarationReferenceStop::Malformed(DeclarationReferenceKind::Enum), + "invalid enum", + ), + ( + DeclarationReferenceStop::Malformed(DeclarationReferenceKind::Constant), + "invalid const", + ), +]; + /// Rust's default spawned-thread stack: bounded nesting must parse on any ordinary thread. const ORDINARY_THREAD_STACK_BYTES: usize = 2 * 1024 * 1024; diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_declaration_references.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_declaration_references.rs index 4ab940beda..97ea5606aa 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/test_declaration_references.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_declaration_references.rs @@ -27,7 +27,7 @@ fn given_sql_when_scanning_declaration_references_then_python_offsets_are_return DeclarationReferenceScanTestCase { description: "doubled backticks close and reopen quoted text, leaving it unclosed", sql: "SELECT 'it''s @const(\"a\")', `x``@const(\"b\")", - expected_references: None, + expected_references: Some(" | stop:quote"), }, DeclarationReferenceScanTestCase { description: "SQL without references is not scanned for unclosed quotes", @@ -35,14 +35,24 @@ fn given_sql_when_scanning_declaration_references_then_python_offsets_are_return expected_references: Some(""), }, DeclarationReferenceScanTestCase { - description: "an unclosed quote before a reference defers to Python", - sql: "SELECT 'unterminated @const(\"a\")", - expected_references: None, + description: "an unclosed quote before a reference stops the walk", + sql: "SELECT @const(\"a\"), 'unterminated @const(\"b\")", + expected_references: Some("const:a@7..18 | stop:quote"), }, DeclarationReferenceScanTestCase { - description: "a malformed reference defers to Python", - sql: "SELECT @enum(\"order_status\")", - expected_references: None, + description: "a malformed enum reference stops the walk after earlier references", + sql: "SELECT @const(\"a\"), @enum(\"order_status\"), @const(\"b\")", + expected_references: Some("const:a@7..18 | stop:invalid enum"), + }, + DeclarationReferenceScanTestCase { + description: "a malformed constant reference at the end of the text stops the walk", + sql: "SELECT @const", + expected_references: Some(" | stop:invalid const"), + }, + DeclarationReferenceScanTestCase { + description: "Unicode whitespace inside and after the keyword matches Python's \\s", + sql: "SELECT @enum\u{a0}(\u{2003}'order_status'\u{1c}) .\u{3000}PLACED", + expected_references: Some("enum:order_status.PLACED@7..40"), }, DeclarationReferenceScanTestCase { description: "a non-ASCII character after the keyword defers its word boundary to Python", @@ -50,9 +60,14 @@ fn given_sql_when_scanning_declaration_references_then_python_offsets_are_return expected_references: None, }, DeclarationReferenceScanTestCase { - description: "an unclosed block comment before a reference defers to Python", + description: "an unclosed block comment before a reference stops the walk", sql: "SELECT /* open @const(\"a\")", - expected_references: None, + expected_references: Some(" | stop:comment"), + }, + DeclarationReferenceScanTestCase { + description: "an unclosed dollar literal before a reference stops the walk", + sql: "SELECT $tag$ @const(\"a\")", + expected_references: Some(" | stop:quote"), }, ]; diff --git a/scripts/compiler_differential/_helpers/corpus/scope_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/scope_failure_cases.py index 035f9531d2..82d5a3f1e0 100644 --- a/scripts/compiler_differential/_helpers/corpus/scope_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/scope_failure_cases.py @@ -1,4 +1,4 @@ -"""Minimal projects that fail on declaration scopes: the index, visibility and relationships.""" +"""Minimal projects that fail on declarations and scopes: files, references, index and grants.""" from scripts.compiler_differential.constants import ( FAILURE_BASE_FILES, @@ -16,16 +16,41 @@ _TEST_PATH: str = "tests/unit/test_customer_totals.sql" -def _case(*, name: str, expected_code: str, files: dict[str, str]) -> FailureCase: +_GLOBAL_MACRO_PATH: str = "macros/amounts.py" +_GLOBAL_MACRO: str = "def doubled(value):\n return f'{value} * 2'\n" +_MART_WITH_MACRO: str = ( + _MART_HEADER + "SELECT customer_id, SUM(@doubled('amount')) AS total_amount\n" + 'FROM __ref("stg_orders")\nGROUP BY customer_id\n' +) + + +def _case( + *, name: str, expected_code: str, files: dict[str, str], expected_message: str | None = None +) -> FailureCase: return FailureCase( - files={**FAILURE_BASE_FILES, **files}, name=name, expected_code=expected_code + files={**FAILURE_BASE_FILES, **files}, + name=name, + expected_code=expected_code, + expected_message=expected_message, ) +def _staging_after_macro(*, where: str, files: dict[str, str] | None = None) -> dict[str, str]: + return { + **(files or {}), + _GLOBAL_MACRO_PATH: _GLOBAL_MACRO, + FAILURE_MART_PATH: _MART_WITH_MACRO, + FAILURE_STAGING_PATH: _STAGING_HEADER + + "SELECT order_id, customer_id, amount, status\n" + + f'FROM __source("raw_orders")\nWHERE {where}\n', + } + + def scope_failure_cases() -> tuple[FailureCase, ...]: """Return the declaration-scope failure cases in a stable order.""" return ( + *_declaration_reference_cases(), _case( name="scope-duplicate-scoped-enum", expected_code="P001", @@ -82,3 +107,46 @@ def scope_failure_cases() -> tuple[FailureCase, ...]: }, ), ) + + +def _declaration_reference_cases() -> tuple[FailureCase, ...]: + return ( + _case( + name="declaration-unknown-enum-after-macro", + expected_code="P001", + expected_message="Unknown enum 'missing_status'", + files=_staging_after_macro(where='status = @enum("missing_status").PLACED'), + ), + _case( + name="declaration-first-of-two-reference-errors", + expected_code="P001", + expected_message="Unknown member 'HELD' for enum 'staged_status'", + files=_staging_after_macro( + where='status = @enum("staged_status").HELD OR amount > @const(cap)', + files={_STAGING_ENUM_PATH: _STAGING_ENUM}, + ), + ), + _case( + name="declaration-invalid-constant-reference", + expected_code="P001", + expected_message="Invalid constant reference", + files=_staging_after_macro(where="amount > @const(cap)"), + ), + _case( + name="declaration-unclosed-quote-before-reference", + expected_code="P001", + expected_message="Enum and constant expansion contains an unclosed quoted string", + files=_staging_after_macro(where='status = \'placed OR amount > @const("cap")'), + ), + _case( + name="declaration-inaccessible-local-constant", + expected_code="P001", + expected_message="Constant 'order_cap' is known but inaccessible", + files=_staging_after_macro( + where='amount > @const("order_cap")', + files={ + "models/marts/_constants/limits.sql": "CONSTANT (name order_cap, value 3);\n" + }, + ), + ), + ) diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 48897e6103..179ee952e4 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -225,7 +225,7 @@ def render_attached_generic_audit( ) -> tuple[str, str | None, str, str] | None: ... def scan_sql_declaration_references( sqls: list[str], -) -> list[list[tuple[int, str, str | None, int, int]] | None]: ... +) -> list[tuple[list[tuple[int, str, str | None, int, int]], int | None] | None]: ... def substitute_static_project_vars( sqls: list[str], variables: list[tuple[str, str]] ) -> list[tuple[int, str | None]]: ... diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py index 08929955a1..c9fc5c7104 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py @@ -153,7 +153,7 @@ from sqlbuild.compiler.model_loop.main._scan_native_declaration_references import ( scan_native_declaration_references, ) -from sqlbuild.compiler.model_loop.types import NativeDeclarationReference +from sqlbuild.compiler.model_loop.types import NativeDeclarationScan from sqlbuild.compiler.planner.types import MaterializationType from sqlbuild.compiler.references.types import ExternalSqlReferenceResolver from sqlbuild.compiler.scopes.models import ( @@ -284,7 +284,7 @@ class _ModelInputLoop: reusable_config_cache: _ReusableModelConfigCache declaration_cache: _VisibleModelDeclarationCache native_header_metadata: dict[Path, NativeHeaderMetadata] - native_declaration_references: dict[Path, tuple[NativeDeclarationReference, ...] | None] + native_declaration_references: dict[Path, NativeDeclarationScan | None] @dataclass(frozen=True) diff --git a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py index 444660df65..09de9598d7 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py @@ -7,9 +7,10 @@ from dataclasses import replace from pathlib import Path from types import MappingProxyType +from typing import cast from sqlbuild.compiler.compile.constants import MACRO_TOKEN, SQL_QUOTE_TOKENS -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError from sqlbuild.compiler.compile.models import ( DeclarationExpansionContext, DeclarationExpansionResult, @@ -33,11 +34,16 @@ ) from sqlbuild.compiler.frontier.main.native_stage_enabled import native_stage_enabled from sqlbuild.compiler.frontier.types import NativeStage -from sqlbuild.compiler.model_loop.constants import ENUM_REFERENCE_KIND_CODE +from sqlbuild.compiler.model_loop.constants import ( + ENUM_REFERENCE_KIND_CODE, + INVALID_CONSTANT_REFERENCE_STOP_CODE, + INVALID_ENUM_REFERENCE_STOP_CODE, + UNCLOSED_BLOCK_COMMENT_STOP_CODE, +) from sqlbuild.compiler.model_loop.main._build_native_declaration_contexts import ( build_native_declaration_contexts, ) -from sqlbuild.compiler.model_loop.types import NativeDeclarationReference +from sqlbuild.compiler.model_loop.types import NativeDeclarationScan from sqlbuild.compiler.planner.types import ContractPolicy from sqlbuild.compiler.scopes.constants import QUALIFIED_IDENTITY_SEPARATOR from sqlbuild.compiler.scopes.main._declaration_lexical_path import declaration_lexical_path @@ -88,6 +94,7 @@ _CONTEXT: str = "Enum and constant expansion" _ACCEPTED_VALUES_AUDIT: str = "accepted_values" _ENUM_REFERENCE_KIND: str = "enum" +_CONSTANT_REFERENCE_KIND: str = "const" _PATH_CACHE_PREFIX: str = "" @@ -624,12 +631,14 @@ def expand_declaration_references_result( sql, reference_start ) if start_match is None: - raise CompileInputError(f"Invalid declaration reference in '{file_path}'") + raise DeclarationReferenceError(f"Invalid declaration reference in '{file_path}'") kind: str = start_match.group("kind") if kind == _ENUM_REFERENCE_KIND: enum_match: re.Match[str] | None = _ENUM_REFERENCE_PATTERN.match(sql, reference_start) if enum_match is None: - raise CompileInputError(f"Invalid enum reference in '{file_path}'") + raise DeclarationReferenceError( + _invalid_reference_message(kind=kind, file_path=file_path) + ) replacement: str next_cursor: int replacement, next_cursor = _resolve_enum_reference( @@ -648,7 +657,9 @@ def expand_declaration_references_result( sql, reference_start ) if constant_match is None: - raise CompileInputError(f"Invalid constant reference in '{file_path}'") + raise DeclarationReferenceError( + _invalid_reference_message(kind=kind, file_path=file_path) + ) replacement, next_cursor = _resolve_constant_reference( sql=sql, reference_start=reference_start, @@ -708,38 +719,46 @@ def declaration_reference_usages( def expand_scanned_declaration_references( # noqa: PLR0913 *, sql: str, - references: tuple[NativeDeclarationReference, ...] | None, + references: NativeDeclarationScan | None, file_path: Path, declarations: DeclarationResolutionContext, value_renderer: TypedSqlValueRenderer, collection_rendering: CollectionRendering, ) -> DeclarationExpansionResult | None: - """Splice natively scanned references, or return None when Python must expand and report.""" + """Splice scanned references and raise Python's first error, or None for Python to expand.""" if references is None: return None - if not references: + scanned, stop = references + if not scanned and stop is None: return DeclarationExpansionResult(sql=sql, spans=(), usages=()) parts: list[str] = [] spans: list[ExpansionSpan] = [] usages: list[UsageRecord] = [] cursor: int = 0 output_length: int = 0 - for kind, name, member, start, end in references: - resolved: tuple[str, tuple[VisibilityRecord, ...]] | None = ( - _scanned_enum_member_text(declarations=declarations, name=name, member=member) - if kind == ENUM_REFERENCE_KIND_CODE - else _scanned_constant_text( - declarations=declarations, + for kind, name, member, start, end in scanned: + replacement: str + visibility: tuple[VisibilityRecord, ...] + if kind == ENUM_REFERENCE_KIND_CODE: + replacement = _enum_member_text( + name=name, + member_name=cast(str, member), + file_path=file_path, + enums=declarations.enums, + inaccessible_enums=declarations.inaccessible_enums, + ) + visibility = declarations.enum_visibility.get(name, ()) + else: + replacement = _constant_text( name=name, file_path=file_path, + constants=declarations.constants, + inaccessible_constants=declarations.inaccessible_constants, value_renderer=value_renderer, collection_rendering=collection_rendering, ) - ) - if resolved is None: - return None - replacement, visibility = resolved + visibility = declarations.constant_visibility.get(name, ()) parts.append(sql[cursor:start]) output_length += start - cursor usages.extend( @@ -758,50 +777,27 @@ def expand_scanned_declaration_references( # noqa: PLR0913 ) output_length += len(replacement) cursor = end + if stop is not None: + raise DeclarationReferenceError(_stop_message(stop=stop, file_path=file_path)) parts.append(sql[cursor:]) return DeclarationExpansionResult( sql="".join(parts), spans=tuple(spans), usages=tuple(dict.fromkeys(usages)) ) -def _scanned_enum_member_text( - *, declarations: DeclarationResolutionContext, name: str, member: str | None -) -> tuple[str, tuple[VisibilityRecord, ...]] | None: - declaration: EnumDeclaration | None = declarations.enums.get(name) - if declaration is None: - return None - matched: EnumMember | None = next( - (candidate for candidate in declaration.members if candidate.name == member), None - ) - if matched is None: - return None - return ( - render_enum_member_value(value=matched.value), - declarations.enum_visibility.get(name, ()), - ) +def _stop_message(*, stop: int, file_path: Path) -> str: + if stop == INVALID_ENUM_REFERENCE_STOP_CODE: + return _invalid_reference_message(kind=_ENUM_REFERENCE_KIND, file_path=file_path) + if stop == INVALID_CONSTANT_REFERENCE_STOP_CODE: + return _invalid_reference_message(kind=_CONSTANT_REFERENCE_KIND, file_path=file_path) + if stop == UNCLOSED_BLOCK_COMMENT_STOP_CODE: + return f"{_CONTEXT} contains an unclosed block comment" + return f"{_CONTEXT} contains an unclosed quoted string" -def _scanned_constant_text( - *, - declarations: DeclarationResolutionContext, - name: str, - file_path: Path, - value_renderer: TypedSqlValueRenderer, - collection_rendering: CollectionRendering, -) -> tuple[str, tuple[VisibilityRecord, ...]] | None: - declaration: ConstantDeclaration | None = declarations.constants.get(name) - if declaration is None: - return None - try: - rendered: str = render_constant_declaration( - declaration=declaration, - value_renderer=value_renderer, - collection_rendering=collection_rendering, - file_path=file_path, - ) - except CompileInputError: - return None - return rendered, declarations.constant_visibility.get(name, ()) +def _invalid_reference_message(*, kind: str, file_path: Path) -> str: + noun: str = "enum" if kind == _ENUM_REFERENCE_KIND else "constant" + return f"Invalid {noun} reference in '{file_path}'" def usage_visibility( @@ -874,36 +870,51 @@ def _resolve_enum_reference( ) -> tuple[str, int]: match: re.Match[str] | None = _ENUM_REFERENCE_PATTERN.match(sql, reference_start) if match is None: - raise CompileInputError( + raise DeclarationReferenceError( f"Invalid enum reference in '{file_path}'; use @enum(\"name\").MEMBER" ) - name: str = match.group("name") + return _enum_member_text( + name=match.group("name"), + member_name=match.group("member"), + file_path=file_path, + enums=enums, + inaccessible_enums=inaccessible_enums, + ), match.end() + + +def _enum_member_text( + *, + name: str, + member_name: str, + file_path: Path, + enums: dict[str, EnumDeclaration], + inaccessible_enums: dict[str, DeclarationRecord], +) -> str: declaration: EnumDeclaration | None = enums.get(name) if declaration is None: inaccessible: DeclarationRecord | None = inaccessible_enums.get(name) if inaccessible is not None: - raise CompileInputError( + raise DeclarationReferenceError( _inaccessible_declaration_message( kind="enum", name=name, record=inaccessible, consumer=file_path ) ) scope_help: str = " in this model" if name.startswith("_") else "" visible: str = ", ".join(sorted(enums)) or "none" - raise CompileInputError( + raise DeclarationReferenceError( f"Unknown enum '{name}'{scope_help} in '{file_path}'. Visible enums: {visible}" ) - member_name: str = match.group("member") member: EnumMember | None = next( (candidate for candidate in declaration.members if candidate.name == member_name), None, ) if member is None: available: str = ", ".join(item.name for item in declaration.members) - raise CompileInputError( + raise DeclarationReferenceError( f"Unknown member '{member_name}' for enum '{name}' in '{file_path}'. " f"Available members: {available}" ) - return render_enum_member_value(value=member.value), match.end() + return render_enum_member_value(value=member.value) def _resolve_constant_reference( @@ -918,31 +929,49 @@ def _resolve_constant_reference( ) -> tuple[str, int]: match: re.Match[str] | None = _CONSTANT_REFERENCE_PATTERN.match(sql, reference_start) if match is None: - raise CompileInputError( + raise DeclarationReferenceError( f"Invalid constant reference in '{file_path}'; use @const(\"name\")" ) - name: str = match.group("name") + return _constant_text( + name=match.group("name"), + file_path=file_path, + constants=constants, + inaccessible_constants=inaccessible_constants, + value_renderer=value_renderer, + collection_rendering=collection_rendering, + ), match.end() + + +def _constant_text( + *, + name: str, + file_path: Path, + constants: dict[str, ConstantDeclaration], + inaccessible_constants: dict[str, DeclarationRecord], + value_renderer: TypedSqlValueRenderer, + collection_rendering: CollectionRendering, +) -> str: declaration: ConstantDeclaration | None = constants.get(name) if declaration is None: inaccessible: DeclarationRecord | None = inaccessible_constants.get(name) if inaccessible is not None: - raise CompileInputError( + raise DeclarationReferenceError( _inaccessible_declaration_message( kind="constant", name=name, record=inaccessible, consumer=file_path ) ) scope_help: str = " in this model" if name.startswith("_") else "" visible: str = ", ".join(sorted(constants)) or "none" - raise CompileInputError( + raise DeclarationReferenceError( f"Unknown constant '{name}'{scope_help} in '{file_path}'. Visible constants: {visible}" ) - rendered: str = render_constant_declaration( + return render_constant_declaration( declaration=declaration, value_renderer=value_renderer, collection_rendering=collection_rendering, file_path=file_path, + error_class=DeclarationReferenceError, ) - return rendered, match.end() def render_constant_declaration( @@ -951,6 +980,7 @@ def render_constant_declaration( value_renderer: TypedSqlValueRenderer, collection_rendering: CollectionRendering, file_path: Path | None = None, + error_class: type[CompileInputError] = CompileInputError, ) -> str: """Render one validated constant with the active adapter's typed-value contract.""" @@ -978,7 +1008,7 @@ def render_constant_declaration( context=f"{declaration.relative_path} constant '{declaration.name}'", ) except (SqlValueRenderingError, SqlValueValidationError) as error: - raise CompileInputError( + raise error_class( f"{declaration.relative_path} constant '{declaration.name}' could not be rendered " f"in '{file_path or declaration.relative_path}' by adapter " f"'{value_renderer.adapter_name}' as " diff --git a/src/sqlbuild/compiler/compile/exceptions.py b/src/sqlbuild/compiler/compile/exceptions.py index 919c705d8e..f6aa4f5e98 100644 --- a/src/sqlbuild/compiler/compile/exceptions.py +++ b/src/sqlbuild/compiler/compile/exceptions.py @@ -29,6 +29,10 @@ def __init__(self, message: str, *, location: SourceLocation, help: str) -> None self.location: SourceLocation = location +class DeclarationReferenceError(CompileInputError): + """Raised for an `@enum` or `@const` reference authored SQL cannot resolve.""" + + class MacroDeclarationLookupError(CompileInputError, KeyError): """Retain authored diagnostics while honoring the Mapping lookup contract.""" diff --git a/src/sqlbuild/compiler/discovery/_helpers/native/declaration_files.py b/src/sqlbuild/compiler/discovery/_helpers/native/declaration_files.py index cc5f2d2651..3316b96fe2 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/native/declaration_files.py +++ b/src/sqlbuild/compiler/discovery/_helpers/native/declaration_files.py @@ -49,9 +49,6 @@ ModelSchemaDeclaration, ) from sqlbuild.compiler.discovery.types import NativeFileScope, NativeLocation, NativeScopeFields -from sqlbuild.compiler.frontier.constants import COMPILER_ENGINE_ENV_VAR -from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError -from sqlbuild.compiler.frontier.types import CompilerEngine, NativeStage from sqlbuild.compiler.scopes.types import DeclarationKind, ScopeKind type _NativeDeclarationFile = tuple[str, NativeFileScope | None, tuple[object, ...]] @@ -62,7 +59,6 @@ type _SchemaPayload = tuple[str, str | None, str | None, dict[str, object], list[NativeLocation]] _SESSION_MEMO_KEY: str = "native_discovery_session" -_FILE_ERRORS: tuple[type[Exception], ...] = (OSError, UnicodeError, ValueError, SyntaxError) def native_enum_files( @@ -273,68 +269,19 @@ def _record[RecordT]( build: _Build[RecordT], parse_with_python: _PythonParse[RecordT], ) -> RecordT: - """Build a parsed file; any other outcome is Python's to raise, and must match native.""" + """Build a parsed file or raise Python's error for it; Python parses the files native defers.""" scope, payload = item tag: object = payload[0] if tag == NATIVE_DEFERRED_TAG: return parse_with_python(relative_path=relative_path, scope=scope) - expected: Exception if tag == NATIVE_PARSED_TAG: - try: - return build( - project_dir=project_dir, relative_path=relative_path, scope=scope, payload=payload - ) - except _FILE_ERRORS as error: - expected = error - elif tag == NATIVE_FAILED_TAG: - expected = native_failure(payload) - else: - expected = native_read_error(payload=payload, file_path=project_dir / relative_path) - return _python_failure( - relative_path=relative_path, - scope=scope, - expected=expected, - parse_with_python=parse_with_python, - ) - - -def _python_failure[RecordT]( - *, - relative_path: Path, - scope: NativeFileScope | None, - expected: Exception, - parse_with_python: _PythonParse[RecordT], -) -> RecordT: - """Raise Python's error for a file native discovery failed, or a mismatch if they differ.""" - - try: - _ = parse_with_python(relative_path=relative_path, scope=scope) - except _FILE_ERRORS as error: - if _same_error(error=error, expected=expected): - raise - raise _mismatch( - relative_path=relative_path, expected=expected, python_outcome=f"raised {error!r}" - ) from error - raise _mismatch(relative_path=relative_path, expected=expected, python_outcome="parsed it") - - -def _same_error(*, error: Exception, expected: Exception) -> bool: - return ( - type(error) is type(expected) - and str(error) == str(expected) - and getattr(error, "help", None) == getattr(expected, "help", None) - ) - - -def _mismatch( - *, relative_path: Path, expected: Exception, python_outcome: str -) -> NativeStageMismatchError: - return NativeStageMismatchError( - f"native {NativeStage.DECLARATION_FILES} raised {expected!r} for {relative_path} but the " - f"Python compiler {python_outcome}; rerun with {COMPILER_ENGINE_ENV_VAR}=" - f"{CompilerEngine.PYTHON} and report this mismatch" - ) + return build( + project_dir=project_dir, relative_path=relative_path, scope=scope, payload=payload + ) + if tag == NATIVE_FAILED_TAG: + raise native_failure(payload) + raise native_read_error(payload=payload, file_path=project_dir / relative_path) def _raise_partial_failure(failure: object) -> None: diff --git a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py index b073556bc5..f3a41f228e 100644 --- a/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/classes/macro_bridge.py @@ -51,6 +51,7 @@ def __init__(self, *, python_version: tuple[int, int], unicode_version: str) -> self._module_digests: dict[str, str] = {} self._observed_modules: dict[str, object] = {} self._project_inputs: tuple[Path, list[str], str] = (Path(), [], "") + self._scanned: bool = False def attach_store(self, *, cache_dir: Path, project_dir: Path, model_paths: list[str]) -> None: """Reuse call results stored by earlier compiles of the same code and project files.""" @@ -133,6 +134,7 @@ def _observe_modules(self) -> bool: def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: """Return the top-level call sites of `sql`, or None when Python must expand it.""" + self._scanned = True rows: list[tuple[int, int, str, list[str], bool]] | None = _native.scan_macro_call_sites( sql, self._python_version, self._unicode_version ) @@ -149,6 +151,12 @@ def scan(self, sql: str) -> tuple[MacroCallSite, ...] | None: for start, end, name, tree_names, typed_reference_text in rows ) + @property + def scanned(self) -> bool: + """Whether any authored string has been scanned, so this compile left the Python path.""" + + return self._scanned + def call_class( self, *, diff --git a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py index 70b7ee99e6..abb1b01316 100644 --- a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py @@ -1,4 +1,4 @@ -"""Run one render stage through the native macro bridge, re-running Python on any failure.""" +"""Run one render stage through the native macro bridge, re-running Python on its failures.""" from __future__ import annotations @@ -9,13 +9,14 @@ from contextvars import Token import sqlbuild._native as _native +from sqlbuild.compiler.compile.exceptions import DeclarationReferenceError from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.constants import ACTIVE_MACRO_BRIDGE def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: - """Run `stage` with the bridge, storing calls on success; a failure re-runs Python.""" + """Run `stage` with the bridge, storing calls on success; failures it could shape re-run.""" python_version: tuple[int, int] = (sys.version_info[0], sys.version_info[1]) if not _native.native_text_supported(python_version, unicodedata.unidata_version): @@ -28,6 +29,8 @@ def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: try: result: T = stage() except Exception as error: + if not bridge.scanned or isinstance(error, DeclarationReferenceError): + raise native_error = error else: bridge.save_store() diff --git a/src/sqlbuild/compiler/model_loop/constants.py b/src/sqlbuild/compiler/model_loop/constants.py index f88fcb825f..f1e2ec1996 100644 --- a/src/sqlbuild/compiler/model_loop/constants.py +++ b/src/sqlbuild/compiler/model_loop/constants.py @@ -9,3 +9,7 @@ } NO_DECLARATION_KIND: int = 255 ENUM_REFERENCE_KIND_CODE: int = DECLARATION_KIND_CODES[DeclarationKind.ENUM] +UNCLOSED_QUOTE_STOP_CODE: int = 0 +UNCLOSED_BLOCK_COMMENT_STOP_CODE: int = 1 +INVALID_ENUM_REFERENCE_STOP_CODE: int = 2 +INVALID_CONSTANT_REFERENCE_STOP_CODE: int = 3 diff --git a/src/sqlbuild/compiler/model_loop/main/_scan_native_declaration_references.py b/src/sqlbuild/compiler/model_loop/main/_scan_native_declaration_references.py index 88b94d4683..5d9d8b81e4 100644 --- a/src/sqlbuild/compiler/model_loop/main/_scan_native_declaration_references.py +++ b/src/sqlbuild/compiler/model_loop/main/_scan_native_declaration_references.py @@ -3,15 +3,15 @@ from __future__ import annotations import sqlbuild._native as _native -from sqlbuild.compiler.model_loop.types import NativeDeclarationReference +from sqlbuild.compiler.model_loop.types import NativeDeclarationScan def scan_native_declaration_references( *, sqls: tuple[str, ...] -) -> tuple[tuple[NativeDeclarationReference, ...] | None, ...]: - """Return each string's `(kind code, name, member, start, end)` references, or None.""" +) -> tuple[NativeDeclarationScan | None, ...]: + """Return each string's references and the code of the error ending its walk, or None.""" return tuple( - None if references is None else tuple(references) - for references in _native.scan_sql_declaration_references(list(sqls)) + None if scan is None else (tuple(scan[0]), scan[1]) + for scan in _native.scan_sql_declaration_references(list(sqls)) ) diff --git a/src/sqlbuild/compiler/model_loop/types.py b/src/sqlbuild/compiler/model_loop/types.py index eb88ba5462..703716e74c 100644 --- a/src/sqlbuild/compiler/model_loop/types.py +++ b/src/sqlbuild/compiler/model_loop/types.py @@ -1,3 +1,4 @@ """Row shapes the native model loop exchanges with its Rust bindings.""" type NativeDeclarationReference = tuple[int, str, str | None, int, int] +type NativeDeclarationScan = tuple[tuple[NativeDeclarationReference, ...], int | None] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 666ca70806..704e7d558c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -918,6 +918,16 @@ class NativeModelLoopParityTestCase: expected_report_text: str +@dataclass(frozen=True) +class NativeDeclarationErrorTestCase: + """A failing project whose declaration error both engines must report the same way.""" + + description: str + project_files: dict[str, str] + expected_report_text: str + expected_macro_calls: int + + @dataclass(frozen=True) class MacroReferenceCallStoreTestCase: """Repeated compiles of models sharing a macro that returns a rejected reference call.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py new file mode 100644 index 0000000000..1b10d978c6 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py @@ -0,0 +1,138 @@ +"""The preview engine reports declaration and scope errors as Python does, without a re-run.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from scripts.compiler_differential.constants import FAILURE_BASE_FILES, FAILURE_BASE_MART +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + NativeDeclarationErrorTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + CompileReuseRun, + report_without_engine, + run_reuse_compile, + stderr_without_durations, + write_project_file, +) + +_ENGINES: tuple[str, str] = ("python", "native-preview") +_CALL_LOG: str = "macro_calls.log" +_COUNTED_MACRO: str = ( + "from pathlib import Path\n\n\n" + "def counted(expression: str) -> str:\n" + ' """Return the expression, logging each execution."""\n\n' + f" with (Path(__file__).resolve().parents[1] / {_CALL_LOG!r}).open(\n" + ' "a", encoding="utf-8"\n' + " ) as log:\n" + ' _ = log.write("call\\n")\n' + " return expression\n" +) +_COUNTED_MART: str = FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@counted('amount'))") +_STAGING_PATH: str = "models/staging/stg_orders.sql" +_STAGING_PREFIX: str = ( + 'MODEL (\n description "Staged orders",\n);\n\n' + 'SELECT order_id, customer_id, amount, status\nFROM __source("raw_orders")\nWHERE ' +) +_STATUS_ENUM: dict[str, str] = { + "enums/order_status.sql": "ENUM (name order_status, members [PLACED, SHIPPED]);\n", +} + + +@pytest.mark.parametrize( + "test_case", + [ + NativeDeclarationErrorTestCase( + description="unknown_enum_after_a_macro_ran", + project_files={ + **_STATUS_ENUM, + _STAGING_PATH: _STAGING_PREFIX + 'status = @enum("missing_status").PLACED\n', + }, + expected_report_text="Unknown enum 'missing_status'", + expected_macro_calls=1, + ), + NativeDeclarationErrorTestCase( + description="first_of_two_reference_errors_after_a_macro_ran", + project_files={ + **_STATUS_ENUM, + _STAGING_PATH: _STAGING_PREFIX + + 'status = @enum("order_status").HELD OR amount > @const(cap)\n', + }, + expected_report_text="Unknown member 'HELD' for enum 'order_status'", + expected_macro_calls=1, + ), + NativeDeclarationErrorTestCase( + description="invalid_constant_reference_after_a_macro_ran", + project_files={_STAGING_PATH: _STAGING_PREFIX + "amount > @const(cap)\n"}, + expected_report_text="Invalid constant reference", + expected_macro_calls=1, + ), + NativeDeclarationErrorTestCase( + description="inaccessible_local_constant_after_a_macro_ran", + project_files={ + "models/marts/_constants/limits.sql": "CONSTANT (name order_cap, value 3);\n", + _STAGING_PATH: _STAGING_PREFIX + 'amount > @const("order_cap")\n', + }, + expected_report_text="Constant 'order_cap' is known but inaccessible", + expected_macro_calls=1, + ), + NativeDeclarationErrorTestCase( + description="duplicate_scoped_enum_before_any_macro_runs", + project_files={ + **_STATUS_ENUM, + "models/staging/_enums/order_status.sql": ( + "ENUM (name order_status, members [HELD]);\n" + ), + }, + expected_report_text="order_status", + expected_macro_calls=0, + ), + NativeDeclarationErrorTestCase( + description="invalid_enum_declaration_file", + project_files={"enums/order_status.sql": "ENUM (name order_status, members [held]);\n"}, + expected_report_text="member identifiers must be uppercase", + expected_macro_calls=0, + ), + ], + ids=lambda case: case.description, +) +def test_given_declaration_error_when_compiling_with_preview_then_error_matches_python_once( + test_case: NativeDeclarationErrorTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "orders" + runs: list[CompileReuseRun] = [] + macro_calls: list[int] = [] + for engine in _ENGINES: + shutil.rmtree(project_dir, ignore_errors=True) + files: dict[str, str] = { + **FAILURE_BASE_FILES, + "macros/counted.py": _COUNTED_MACRO, + "models/marts/customer_totals.sql": _COUNTED_MART, + **test_case.project_files, + } + for relative_path, contents in {**files, _CALL_LOG: ""}.items(): + write_project_file(project_dir, relative_path, contents) + runs.append( + run_reuse_compile(project_dir=project_dir, global_args=("--compiler-engine", engine)) + ) + macro_calls.append(len((project_dir / _CALL_LOG).read_text(encoding="utf-8").splitlines())) + + assert ( + tuple(run.returncode for run in runs), + report_without_engine(runs[1]) == report_without_engine(runs[0]), + stderr_without_durations(stderr=runs[1].stderr) + == stderr_without_durations(stderr=runs[0].stderr), + test_case.expected_report_text in runs[0].report + runs[0].stderr, + macro_calls, + ) == ((1, 1), True, True, True, [test_case.expected_macro_calls] * 2), ( + runs[0].report, + runs[1].report, + runs[1].stderr, + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py index 43308c43f7..272473d7e4 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py @@ -190,14 +190,15 @@ class DeclarationFilesParityTestCase: @dataclass(frozen=True) -class DeclarationMismatchTestCase: - """A declaration file the Python parser is patched to accept, which native discovery rejects.""" +class NativeDeclarationFailureTestCase: + """A declaration file native discovery rejects while the Python parser is patched away.""" description: str relative_path: str contents: bytes patched_parser: str patched: Callable[..., object] + expected_failure_type: str expected_error_fragment: str diff --git a/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py b/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py index ac76077d7e..2c997d3792 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py @@ -19,10 +19,10 @@ from sqlbuild.compiler.frontier.constants import COMPILER_ENGINE_ENV_VAR from tests.integration.src.sqlbuild.compiler.discovery._test_types import ( DeclarationFilesParityTestCase, - DeclarationMismatchTestCase, DeclarationReuseTestCase, DeepDeclarationHeaderTestCase, GeneratedDeclarationFileTestCase, + NativeDeclarationFailureTestCase, NativeSessionTestCase, TolerantDeclarationFilesTestCase, ) @@ -433,27 +433,29 @@ def test_given_broken_declaration_files_when_discovering_tolerantly_then_faults_ @pytest.mark.parametrize( "test_case", [ - DeclarationMismatchTestCase( - description="Python accepting a file native rejects is a mismatch", + NativeDeclarationFailureTestCase( + description="a native parse error stands even where Python would accept the file", relative_path="enums/status.sql", contents=b"ENUM (name order_status);\n", patched_parser="parse_enum_declaration_file", patched=accept_any_declarations, - expected_error_fragment="native declaration_files raised DeclarationParseError", + expected_failure_type="DeclarationParseError", + expected_error_fragment="enums/status.sql", ), - DeclarationMismatchTestCase( - description="Python failing differently from native is a mismatch", + NativeDeclarationFailureTestCase( + description="a native parse error stands where Python would fail differently", relative_path="audits/generic/is_true.sql", contents=b"SELECT 1\n", patched_parser="parse_sql_audit_file", patched=reject_any_contents, - expected_error_fragment="but the Python compiler raised ValueError('different')", + expected_failure_type="SqlAuditParseError", + expected_error_fragment="audits/generic/is_true.sql", ), ], ids=lambda case: case.description, ) -def test_given_python_disagrees_with_a_native_failure_when_discovering_then_mismatch_raises( - test_case: DeclarationMismatchTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +def test_given_native_declaration_failure_when_discovering_then_python_is_not_rerun( + test_case: NativeDeclarationFailureTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: write_project(project_dir=tmp_path, files=((test_case.relative_path, test_case.contents),)) monkeypatch.setattr(core, test_case.patched_parser, test_case.patched) @@ -461,8 +463,10 @@ def test_given_python_disagrees_with_a_native_failure_when_discovering_then_mism project_dir=tmp_path, engine=_PREVIEW, monkeypatch=monkeypatch ) - assert outcome[1] == "NativeStageMismatchError" - assert test_case.expected_error_fragment in str(outcome[2]) + assert (outcome[1], test_case.expected_error_fragment in str(outcome[2])) == ( + test_case.expected_failure_type, + True, + ), outcome @pytest.mark.parametrize( diff --git a/tests/integration/src/sqlbuild/compiler/model_loop/_test_types.py b/tests/integration/src/sqlbuild/compiler/model_loop/_test_types.py index 65a567c33b..299ea79987 100644 --- a/tests/integration/src/sqlbuild/compiler/model_loop/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/model_loop/_test_types.py @@ -11,5 +11,5 @@ class DeclarationReferenceParityTestCase: seed: int count: int expected_minimum_native_references: int - expected_minimum_deferred: int - expected_minimum_python_errors: int + expected_maximum_deferred: int + expected_minimum_native_errors: int diff --git a/tests/integration/src/sqlbuild/compiler/model_loop/helpers.py b/tests/integration/src/sqlbuild/compiler/model_loop/helpers.py index bcfb384ed1..a6b3feb1b7 100644 --- a/tests/integration/src/sqlbuild/compiler/model_loop/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/model_loop/helpers.py @@ -20,9 +20,21 @@ from sqlbuild.compiler.model_loop.main._scan_native_declaration_references import ( scan_native_declaration_references, ) -from sqlbuild.compiler.model_loop.types import NativeDeclarationReference -from sqlbuild.compiler.scopes.models import DeclarationIdentity, ResourceIdentity, VisibilityRecord -from sqlbuild.compiler.scopes.types import DeclarationKind, ResourceKind, VisibilityReason +from sqlbuild.compiler.model_loop.types import NativeDeclarationScan +from sqlbuild.compiler.scopes.models import ( + DeclarationIdentity, + DeclarationRecord, + OwnershipRoot, + ResourceIdentity, + VisibilityRecord, +) +from sqlbuild.compiler.scopes.types import ( + DeclarationKind, + OwnershipRootKind, + ResourceKind, + ScopeKind, + VisibilityReason, +) from sqlbuild.sql_values.main.normalize import normalize_sql_value from sqlbuild.sql_values.types import CollectionRendering @@ -48,6 +60,11 @@ normalize_sql_value(raw_value=["north", "south"], context="test"), Path("constants/regions.sql"), ), + "quoted_note": ConstantDeclaration( + "quoted_note", + normalize_sql_value(raw_value="'" * 600_000, context="test"), + Path("constants/note.sql"), + ), } DECLARATIONS: DeclarationResolutionContext = DeclarationResolutionContext( enums=_ENUMS, @@ -71,9 +88,31 @@ ) for name in _CONSTANTS }, + inaccessible_enums={ + "staged_status": DeclarationRecord( + identity=DeclarationIdentity(DeclarationKind.ENUM, "staged_status"), + path="models/staging/_enums/status.sql", + line=1, + column=1, + scope=ScopeKind.LOCAL, + ownership_root=OwnershipRoot("models", OwnershipRootKind.RESOURCE, ResourceKind.MODEL), + owning_path="models/staging", + ) + }, + inaccessible_constants={ + "staged_cap": DeclarationRecord( + identity=DeclarationIdentity(DeclarationKind.CONSTANT, "staged_cap"), + path="models/staging/_constants/cap.sql", + line=1, + column=1, + scope=ScopeKind.INHERITED, + ownership_root=OwnershipRoot("models", OwnershipRootKind.RESOURCE, ResourceKind.MODEL), + owning_path="models/staging", + ) + }, consumer=_CONSUMER, ) -_WHITESPACE: tuple[str, ...] = ("", " ", "\t", "\n ", "\x1c", "\u00a0") +_WHITESPACE: tuple[str, ...] = ("", " ", "\t", "\n ", "\x1c", "\u00a0", "\u2003") _REFERENCES: tuple[str, ...] = ( '@enum({s}"order_status"{s}){s}.{s}PLACED', "@enum('order_status').SHIPPED", @@ -85,8 +124,15 @@ _INVALID_REFERENCES: tuple[str, ...] = ( '@enum("order_status").MISSING', '@const("unknown_cap")', + '@const("_private_cap")', + '@enum("_private_status").PLACED', + '@enum("staged_status").PLACED', + '@const("staged_cap")', + '@const("quoted_note")', '@enum("order_status")', "@const(sales_cap)", + "@enum", + "@const\u00a0(", ) _TEXT: tuple[str, ...] = ( "SELECT ", @@ -114,27 +160,38 @@ "$q$ open ", "@const\u00e9 ", *_INVALID_REFERENCES, + *_INVALID_REFERENCES, ) @dataclass(frozen=True) class ReferenceParity: - """Python's expansion (or error text) and the native one (or None) of one SQL string.""" + """Python's expansion (or error) and the native one (or error, or None) of one SQL string.""" sql: str - python: DeclarationExpansionResult | str - native: DeclarationExpansionResult | None + python: DeclarationExpansionResult | tuple[str, str, str | None] + native: DeclarationExpansionResult | tuple[str, str, str | None] | None native_references: int def generated_reference_sql(*, rng: random.Random) -> str: """Return SQL mixing references, quoted text, comments and Unicode in random order.""" + return _reference_sql(rng=rng, rare_parts=int(rng.random() < 0.15)) + + +def generated_failing_reference_sql(*, rng: random.Random) -> str: + """Return reference SQL that often holds several invalid references or unclosed text.""" + + return _reference_sql(rng=rng, rare_parts=rng.choice((0, 0, 0, 0, 1, 2, 3))) + + +def _reference_sql(*, rng: random.Random, rare_parts: int) -> str: parts: list[str] = [ rng.choices((_reference_part, _text_part), weights=(4, 6))[0](rng) for _ in range(rng.randint(1, 12)) ] - parts.extend(rng.choices(_RARE, k=int(rng.random() < 0.15))) + parts.extend(rng.choices(_RARE, k=rare_parts)) rng.shuffle(parts) return "".join(parts) @@ -142,22 +199,15 @@ def generated_reference_sql(*, rng: random.Random) -> str: def reference_parities(*, sqls: list[str]) -> list[ReferenceParity]: """Expand every string in Python and through the native scan with Python rendering.""" - scanned: tuple[tuple[NativeDeclarationReference, ...] | None, ...] = ( - scan_native_declaration_references(sqls=tuple(sqls)) + scanned: tuple[NativeDeclarationScan | None, ...] = scan_native_declaration_references( + sqls=tuple(sqls) ) return [ ReferenceParity( sql=sql, python=_python_outcome(sql=sql), - native=expand_scanned_declaration_references( - sql=sql, - references=references, - file_path=_FILE_PATH, - declarations=DECLARATIONS, - value_renderer=DuckDbAdapter(), - collection_rendering=CollectionRendering.VALUE_LIST, - ), - native_references=len(references or ()), + native=_native_outcome(sql=sql, references=references), + native_references=len((references or ((), None))[0]), ) for sql, references in zip(sqls, scanned, strict=True) ] @@ -177,7 +227,13 @@ def _text_part(rng: random.Random) -> str: return rng.choice(_TEXT) -def _python_outcome(*, sql: str) -> DeclarationExpansionResult | str: +def is_error(outcome: object) -> bool: + """Whether an expansion outcome is an error's code, message and help.""" + + return isinstance(outcome, tuple) + + +def _python_outcome(*, sql: str) -> DeclarationExpansionResult | tuple[str, str, str | None]: try: return expand_declaration_references_result( sql=sql, @@ -187,4 +243,20 @@ def _python_outcome(*, sql: str) -> DeclarationExpansionResult | str: collection_rendering=CollectionRendering.VALUE_LIST, ) except CompileInputError as error: - return str(error) + return (error.code, str(error), error.help) + + +def _native_outcome( + *, sql: str, references: NativeDeclarationScan | None +) -> DeclarationExpansionResult | tuple[str, str, str | None] | None: + try: + return expand_scanned_declaration_references( + sql=sql, + references=references, + file_path=_FILE_PATH, + declarations=DECLARATIONS, + value_renderer=DuckDbAdapter(), + collection_rendering=CollectionRendering.VALUE_LIST, + ) + except CompileInputError as error: + return (error.code, str(error), error.help) diff --git a/tests/integration/src/sqlbuild/compiler/model_loop/test_native_declaration_references.py b/tests/integration/src/sqlbuild/compiler/model_loop/test_native_declaration_references.py index bcdb646bfb..409f409bb7 100644 --- a/tests/integration/src/sqlbuild/compiler/model_loop/test_native_declaration_references.py +++ b/tests/integration/src/sqlbuild/compiler/model_loop/test_native_declaration_references.py @@ -1,4 +1,4 @@ -"""Natively scanned `@enum`/`@const` references expand exactly as Python expands them.""" +"""Natively scanned `@enum`/`@const` references expand, and fail, exactly as Python's do.""" from __future__ import annotations @@ -12,7 +12,8 @@ ) from tests.integration.src.sqlbuild.compiler.model_loop.helpers import ( ReferenceParity, - generated_reference_sql, + generated_failing_reference_sql, + is_error, is_native, reference_parities, ) @@ -22,21 +23,21 @@ "test_case", [ DeclarationReferenceParityTestCase( - description="seeded references among quotes, comments, dollar literals and Unicode", + description="seeded references and errors among quotes, comments and Unicode", seed=20261007, count=3000, - expected_minimum_native_references=2000, - expected_minimum_deferred=600, - expected_minimum_python_errors=400, + expected_minimum_native_references=5000, + expected_maximum_deferred=150, + expected_minimum_native_errors=1200, ), ], ids=lambda case: case.description, ) -def test_given_generated_sql_when_expanding_references_natively_then_python_output_matches( +def test_given_generated_sql_when_expanding_references_natively_then_python_outcome_matches( test_case: DeclarationReferenceParityTestCase, ) -> None: rng: random.Random = random.Random(test_case.seed) - sqls: list[str] = [generated_reference_sql(rng=rng) for _ in range(test_case.count)] + sqls: list[str] = [generated_failing_reference_sql(rng=rng) for _ in range(test_case.count)] parities: list[ReferenceParity] = reference_parities(sqls=sqls) @@ -50,9 +51,9 @@ def test_given_generated_sql_when_expanding_references_natively_then_python_outp ), sum(parity.native_references for parity in expanded) >= test_case.expected_minimum_native_references, - len(parities) - len(expanded) >= test_case.expected_minimum_deferred, - sum(isinstance(parity.python, str) for parity in parities) - >= test_case.expected_minimum_python_errors, + len(parities) - len(expanded) <= test_case.expected_maximum_deferred, + sum(is_error(parity.native) for parity in expanded) + >= test_case.expected_minimum_native_errors, ) == ([], True, True, True), test_case.description diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py index 9914dce8e2..afedeeaf05 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py @@ -3,8 +3,10 @@ from __future__ import annotations from collections.abc import Callable +from typing import cast -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError +from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge @@ -20,6 +22,25 @@ def failed() -> str: raise CompileInputError("render failed") +def failed_after_scan() -> str: + """Fail to render after the active bridge scanned a macro call.""" + + _scan_with_active_bridge() + raise CompileInputError("render failed") + + +def declaration_failed_after_scan() -> str: + """Fail on an `@enum` reference after the active bridge scanned a macro call.""" + + _scan_with_active_bridge() + raise DeclarationReferenceError("Unknown enum 'order_status' in 'models/orders.sql'") + + +def _scan_with_active_bridge() -> None: + bridge: MacroBridge = cast(MacroBridge, active_macro_bridge()) + _ = bridge.scan("SELECT @cents('amount') AS amount") + + def recording_stage( *, runs: list[bool], stages: dict[bool, Callable[[], str]] ) -> Callable[[], str]: diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py index 5b556aeb0d..1a505f7b71 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py @@ -4,7 +4,7 @@ import pytest -from sqlbuild.compiler.compile.exceptions import CompileInputError +from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge from sqlbuild.compiler.macro_bridge.main.run_with_macro_bridge import run_with_macro_bridge @@ -14,7 +14,9 @@ MacroBridgeSuccessTestCase, ) from tests.unit.src.sqlbuild.compiler.macro_bridge.main.helpers import ( + declaration_failed_after_scan, failed, + failed_after_scan, recording_stage, rendered, ) @@ -49,23 +51,37 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br "test_case", [ MacroBridgeFailureTestCase( - description="failure re-runs Python and raises its error", - stage_with_bridge=failed, + description="failure after a scan re-runs Python and raises its error", + stage_with_bridge=failed_after_scan, stage_without_bridge=failed, expected_error=CompileInputError, expected_bridged_runs=[True, False], ), MacroBridgeFailureTestCase( - description="native-only failure is reported as a divergence", - stage_with_bridge=failed, + description="native-only failure after a scan is reported as a divergence", + stage_with_bridge=failed_after_scan, stage_without_bridge=rendered, expected_error=NativeStageMismatchError, expected_bridged_runs=[True, False], ), + MacroBridgeFailureTestCase( + description="failure before the bridge scans anything is raised without a re-run", + stage_with_bridge=failed, + stage_without_bridge=rendered, + expected_error=CompileInputError, + expected_bridged_runs=[True], + ), + MacroBridgeFailureTestCase( + description="declaration reference failure after a scan is raised without a re-run", + stage_with_bridge=declaration_failed_after_scan, + stage_without_bridge=rendered, + expected_error=DeclarationReferenceError, + expected_bridged_runs=[True], + ), ], ids=lambda case: case.description, ) -def test_given_failing_stage_when_running_with_macro_bridge_then_python_reruns( +def test_given_failing_stage_when_running_with_macro_bridge_then_only_bridge_failures_rerun( test_case: MacroBridgeFailureTestCase, ) -> None: runs: list[bool] = [] @@ -86,7 +102,7 @@ def test_given_failing_stage_when_running_with_macro_bridge_then_python_reruns( [ MacroBridgeErrorContextTestCase( description="python error after a bridge failure", - stage=failed, + stage=recording_stage(runs=[], stages={True: failed_after_scan, False: failed}), expected_context=None, ), ], From 527c17582f7db00cb514f56570e815ab995588fc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:35:30 +0100 Subject: [PATCH 2/2] fix(native): mark exact declaration errors without a new exception type --- .../compile/_helpers/render/declarations.py | 74 ++++++++++++------- src/sqlbuild/compiler/compile/exceptions.py | 14 ++-- .../main/run_with_macro_bridge.py | 6 +- .../cli/commands/main/compile/_test_types.py | 9 +++ .../cli/commands/main/compile/helpers.py | 18 +++++ .../compile/test_native_declaration_errors.py | 54 ++++++++++++++ .../compiler/macro_bridge/main/helpers.py | 6 +- .../main/test_run_with_macro_bridge.py | 4 +- 8 files changed, 149 insertions(+), 36 deletions(-) diff --git a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py index 09de9598d7..87043b82d7 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py @@ -10,7 +10,7 @@ from typing import cast from sqlbuild.compiler.compile.constants import MACRO_TOKEN, SQL_QUOTE_TOKENS -from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.compile.models import ( DeclarationExpansionContext, DeclarationExpansionResult, @@ -631,13 +631,16 @@ def expand_declaration_references_result( sql, reference_start ) if start_match is None: - raise DeclarationReferenceError(f"Invalid declaration reference in '{file_path}'") + raise CompileInputError( + f"Invalid declaration reference in '{file_path}'", bridge_independent=True + ) kind: str = start_match.group("kind") if kind == _ENUM_REFERENCE_KIND: enum_match: re.Match[str] | None = _ENUM_REFERENCE_PATTERN.match(sql, reference_start) if enum_match is None: - raise DeclarationReferenceError( - _invalid_reference_message(kind=kind, file_path=file_path) + raise CompileInputError( + _invalid_reference_message(kind=kind, file_path=file_path), + bridge_independent=True, ) replacement: str next_cursor: int @@ -657,8 +660,9 @@ def expand_declaration_references_result( sql, reference_start ) if constant_match is None: - raise DeclarationReferenceError( - _invalid_reference_message(kind=kind, file_path=file_path) + raise CompileInputError( + _invalid_reference_message(kind=kind, file_path=file_path), + bridge_independent=True, ) replacement, next_cursor = _resolve_constant_reference( sql=sql, @@ -778,7 +782,9 @@ def expand_scanned_declaration_references( # noqa: PLR0913 output_length += len(replacement) cursor = end if stop is not None: - raise DeclarationReferenceError(_stop_message(stop=stop, file_path=file_path)) + raise CompileInputError( + _stop_message(stop=stop, file_path=file_path), bridge_independent=True + ) parts.append(sql[cursor:]) return DeclarationExpansionResult( sql="".join(parts), spans=tuple(spans), usages=tuple(dict.fromkeys(usages)) @@ -870,8 +876,9 @@ def _resolve_enum_reference( ) -> tuple[str, int]: match: re.Match[str] | None = _ENUM_REFERENCE_PATTERN.match(sql, reference_start) if match is None: - raise DeclarationReferenceError( - f"Invalid enum reference in '{file_path}'; use @enum(\"name\").MEMBER" + raise CompileInputError( + f"Invalid enum reference in '{file_path}'; use @enum(\"name\").MEMBER", + bridge_independent=True, ) return _enum_member_text( name=match.group("name"), @@ -894,15 +901,17 @@ def _enum_member_text( if declaration is None: inaccessible: DeclarationRecord | None = inaccessible_enums.get(name) if inaccessible is not None: - raise DeclarationReferenceError( + raise CompileInputError( _inaccessible_declaration_message( kind="enum", name=name, record=inaccessible, consumer=file_path - ) + ), + bridge_independent=True, ) scope_help: str = " in this model" if name.startswith("_") else "" visible: str = ", ".join(sorted(enums)) or "none" - raise DeclarationReferenceError( - f"Unknown enum '{name}'{scope_help} in '{file_path}'. Visible enums: {visible}" + raise CompileInputError( + f"Unknown enum '{name}'{scope_help} in '{file_path}'. Visible enums: {visible}", + bridge_independent=True, ) member: EnumMember | None = next( (candidate for candidate in declaration.members if candidate.name == member_name), @@ -910,9 +919,10 @@ def _enum_member_text( ) if member is None: available: str = ", ".join(item.name for item in declaration.members) - raise DeclarationReferenceError( + raise CompileInputError( f"Unknown member '{member_name}' for enum '{name}' in '{file_path}'. " - f"Available members: {available}" + f"Available members: {available}", + bridge_independent=True, ) return render_enum_member_value(value=member.value) @@ -929,8 +939,9 @@ def _resolve_constant_reference( ) -> tuple[str, int]: match: re.Match[str] | None = _CONSTANT_REFERENCE_PATTERN.match(sql, reference_start) if match is None: - raise DeclarationReferenceError( - f"Invalid constant reference in '{file_path}'; use @const(\"name\")" + raise CompileInputError( + f"Invalid constant reference in '{file_path}'; use @const(\"name\")", + bridge_independent=True, ) return _constant_text( name=match.group("name"), @@ -955,22 +966,24 @@ def _constant_text( if declaration is None: inaccessible: DeclarationRecord | None = inaccessible_constants.get(name) if inaccessible is not None: - raise DeclarationReferenceError( + raise CompileInputError( _inaccessible_declaration_message( kind="constant", name=name, record=inaccessible, consumer=file_path - ) + ), + bridge_independent=True, ) scope_help: str = " in this model" if name.startswith("_") else "" visible: str = ", ".join(sorted(constants)) or "none" - raise DeclarationReferenceError( - f"Unknown constant '{name}'{scope_help} in '{file_path}'. Visible constants: {visible}" + raise CompileInputError( + f"Unknown constant '{name}'{scope_help} in '{file_path}'. Visible constants: {visible}", + bridge_independent=True, ) return render_constant_declaration( declaration=declaration, value_renderer=value_renderer, collection_rendering=collection_rendering, file_path=file_path, - error_class=DeclarationReferenceError, + bridge_independent=True, ) @@ -980,7 +993,7 @@ def render_constant_declaration( value_renderer: TypedSqlValueRenderer, collection_rendering: CollectionRendering, file_path: Path | None = None, - error_class: type[CompileInputError] = CompileInputError, + bridge_independent: bool = False, ) -> str: """Render one validated constant with the active adapter's typed-value contract.""" @@ -1008,11 +1021,12 @@ def render_constant_declaration( context=f"{declaration.relative_path} constant '{declaration.name}'", ) except (SqlValueRenderingError, SqlValueValidationError) as error: - raise error_class( + raise CompileInputError( f"{declaration.relative_path} constant '{declaration.name}' could not be rendered " f"in '{file_path or declaration.relative_path}' by adapter " f"'{value_renderer.adapter_name}' as " - f"{selected_rendering.value}: {error}" + f"{selected_rendering.value}: {error}", + bridge_independent=bridge_independent, ) from error return rendered @@ -1042,6 +1056,16 @@ def render_enum_member_value(*, value: str | int) -> str: def _find_next_reference_start(*, sql: str, start: int) -> int | None: + """Python's scan for the next reference; its unclosed quote and comment errors are exact.""" + + try: + return _scan_next_reference_start(sql=sql, start=start) + except CompileInputError as error: + error.bridge_independent = True + raise + + +def _scan_next_reference_start(*, sql: str, start: int) -> int | None: if sql.find("@enum", start) < 0 and sql.find("@const", start) < 0: return None index: int = start diff --git a/src/sqlbuild/compiler/compile/exceptions.py b/src/sqlbuild/compiler/compile/exceptions.py index f6aa4f5e98..9d7946a898 100644 --- a/src/sqlbuild/compiler/compile/exceptions.py +++ b/src/sqlbuild/compiler/compile/exceptions.py @@ -10,11 +10,19 @@ class CompileInputError(ValueError): code: str = "P001" - def __init__(self, message: str, *, code: str | None = None, help: str | None = None) -> None: + def __init__( + self, + message: str, + *, + code: str | None = None, + help: str | None = None, + bridge_independent: bool = False, + ) -> None: super().__init__(message) self.message = message self.code = code if code is not None else self.code self.help = help + self.bridge_independent: bool = bridge_independent class SqlTestReferenceError(CompileInputError): @@ -29,10 +37,6 @@ def __init__(self, message: str, *, location: SourceLocation, help: str) -> None self.location: SourceLocation = location -class DeclarationReferenceError(CompileInputError): - """Raised for an `@enum` or `@const` reference authored SQL cannot resolve.""" - - class MacroDeclarationLookupError(CompileInputError, KeyError): """Retain authored diagnostics while honoring the Mapping lookup contract.""" diff --git a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py index abb1b01316..a79dc1fbd0 100644 --- a/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py +++ b/src/sqlbuild/compiler/macro_bridge/main/run_with_macro_bridge.py @@ -9,7 +9,7 @@ from contextvars import Token import sqlbuild._native as _native -from sqlbuild.compiler.compile.exceptions import DeclarationReferenceError +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.constants import ACTIVE_MACRO_BRIDGE @@ -29,7 +29,9 @@ def run_with_macro_bridge[T](*, stage: Callable[[], T]) -> T: try: result: T = stage() except Exception as error: - if not bridge.scanned or isinstance(error, DeclarationReferenceError): + if not bridge.scanned or ( + isinstance(error, CompileInputError) and error.bridge_independent + ): raise native_error = error else: diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 704e7d558c..b0402ffab7 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -928,6 +928,15 @@ class NativeDeclarationErrorTestCase: expected_macro_calls: int +@dataclass(frozen=True) +class DeclarationErrorLifecycleTestCase: + """A failing project whose invocation lifecycle every engine must report the same way.""" + + description: str + project_files: dict[str, str] + expected_error_types: tuple[str, ...] + + @dataclass(frozen=True) class MacroReferenceCallStoreTestCase: """Repeated compiles of models sharing a macro that returns a rejected reference call.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py index 309e13770b..8a1c29d7b4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py @@ -68,6 +68,7 @@ from sqlbuild.compiler.frontier.constants import COMPILER_ENGINE_ENV_VAR from sqlbuild.compiler.frontier.main.compiler_cache_directory import compiler_cache_directory from sqlbuild.compiler.macro_bridge.constants import MACRO_CALL_STORE_FILE_NAME +from sqlbuild.observability import EventDispatcher, LifecycleEvent from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( FreshProcessCompileCachePerformanceGuardTestCase, IncrementalEditStep, @@ -4832,3 +4833,20 @@ def recorded(type_sql: str, dialect: str) -> object: run: CompileReuseRun = in_process_reuse_run(project_dir=project_dir, capsys=capsys) type_normalization.normalize_type.cache_clear() return run, answered + + +def lifecycle_error_type(*, project_dir: Path, engine: str, monkeypatch: pytest.MonkeyPatch) -> str: + """Compile in this process under `engine`; return the failed invocation's error type.""" + + published: list[LifecycleEvent] = [] + with monkeypatch.context() as patch: + patch.setenv(COMPILER_ENGINE_ENV_VAR, engine) + patch.setenv(REUSE_DISABLE_ENV_VAR, "1") + patch.setattr( + EventDispatcher, + "publish_lifecycle", + lambda _dispatcher, event: published.append(event), + ) + with redirect_stdout(StringIO()): + _ = main(["--project-dir", str(project_dir), "--no-color", "compile"]) + return str(published[-1].payload.get("error_type")) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py index 1b10d978c6..90c7a90ed0 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_native_declaration_errors.py @@ -9,10 +9,12 @@ from scripts.compiler_differential.constants import FAILURE_BASE_FILES, FAILURE_BASE_MART from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + DeclarationErrorLifecycleTestCase, NativeDeclarationErrorTestCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( CompileReuseRun, + lifecycle_error_type, report_without_engine, run_reuse_compile, stderr_without_durations, @@ -79,6 +81,15 @@ expected_report_text="Constant 'order_cap' is known but inaccessible", expected_macro_calls=1, ), + NativeDeclarationErrorTestCase( + description="unclosed_quote_after_a_reference_only_python_scans", + project_files={ + **_STATUS_ENUM, + _STAGING_PATH: _STAGING_PREFIX + "status = @enum\u00e9 OR status = 'open\n", + }, + expected_report_text="Enum and constant expansion contains an unclosed quoted string", + expected_macro_calls=1, + ), NativeDeclarationErrorTestCase( description="duplicate_scoped_enum_before_any_macro_runs", project_files={ @@ -134,5 +145,48 @@ def test_given_declaration_error_when_compiling_with_preview_then_error_matches_ ) +@pytest.mark.parametrize( + "test_case", + [ + DeclarationErrorLifecycleTestCase( + description="unknown_enum_after_a_macro_ran", + project_files={ + **_STATUS_ENUM, + _STAGING_PATH: _STAGING_PREFIX + 'status = @enum("missing_status").PLACED\n', + }, + expected_error_types=("CompileInputError",) * 3, + ), + DeclarationErrorLifecycleTestCase( + description="unclosed_quote_after_a_reference_only_python_scans", + project_files={ + **_STATUS_ENUM, + _STAGING_PATH: _STAGING_PREFIX + "status = @enum\u00e9 OR status = 'open\n", + }, + expected_error_types=("CompileInputError",) * 3, + ), + ], + ids=lambda case: case.description, +) +def test_given_declaration_error_when_compiling_with_each_engine_then_lifecycle_error_type_matches( + test_case: DeclarationErrorLifecycleTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + error_types: list[str] = [] + for engine in ("python", "native", "native-preview"): + project_dir: Path = tmp_path / engine + for relative_path, contents in { + **FAILURE_BASE_FILES, + "macros/counted.py": _COUNTED_MACRO, + "models/marts/customer_totals.sql": _COUNTED_MART, + _CALL_LOG: "", + **test_case.project_files, + }.items(): + write_project_file(project_dir, relative_path, contents) + error_types.append( + lifecycle_error_type(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + + assert tuple(error_types) == test_case.expected_error_types, test_case.description + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py index afedeeaf05..b9d597ef70 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/helpers.py @@ -5,7 +5,7 @@ from collections.abc import Callable from typing import cast -from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.macro_bridge.classes.macro_bridge import MacroBridge from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge @@ -33,7 +33,9 @@ def declaration_failed_after_scan() -> str: """Fail on an `@enum` reference after the active bridge scanned a macro call.""" _scan_with_active_bridge() - raise DeclarationReferenceError("Unknown enum 'order_status' in 'models/orders.sql'") + raise CompileInputError( + "Unknown enum 'order_status' in 'models/orders.sql'", bridge_independent=True + ) def _scan_with_active_bridge() -> None: diff --git a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py index 1a505f7b71..81aa6d6c78 100644 --- a/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py +++ b/tests/unit/src/sqlbuild/compiler/macro_bridge/main/test_run_with_macro_bridge.py @@ -4,7 +4,7 @@ import pytest -from sqlbuild.compiler.compile.exceptions import CompileInputError, DeclarationReferenceError +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.macro_bridge.main.active_macro_bridge import active_macro_bridge from sqlbuild.compiler.macro_bridge.main.run_with_macro_bridge import run_with_macro_bridge @@ -75,7 +75,7 @@ def test_given_succeeding_stage_when_running_with_macro_bridge_then_runs_once_br description="declaration reference failure after a scan is raised without a re-run", stage_with_bridge=declaration_failed_after_scan, stage_without_bridge=rendered, - expected_error=DeclarationReferenceError, + expected_error=CompileInputError, expected_bridged_runs=[True], ), ],