diff --git a/crates/sqlbuild-attachments/src/audits/_helpers/mod.rs b/crates/sqlbuild-attachments/src/audits/_helpers/mod.rs index 64996b43bb..78767d0537 100644 --- a/crates/sqlbuild-attachments/src/audits/_helpers/mod.rs +++ b/crates/sqlbuild-attachments/src/audits/_helpers/mod.rs @@ -1,2 +1,3 @@ pub(crate) mod parameters; pub(crate) mod policies; +pub(crate) mod rendering; diff --git a/crates/sqlbuild-attachments/src/audits/_helpers/parameters.rs b/crates/sqlbuild-attachments/src/audits/_helpers/parameters.rs index fdc1b0feb6..586f51d388 100644 --- a/crates/sqlbuild-attachments/src/audits/_helpers/parameters.rs +++ b/crates/sqlbuild-attachments/src/audits/_helpers/parameters.rs @@ -60,13 +60,13 @@ pub(crate) fn parameter_matches(sql: &str) -> Option>> { Some(matches) } -/// Python's `render_sql_argument_value` for one value. -pub(crate) fn render_value(value: &ArgumentValue, quoted: bool) -> String { - match value { +/// Python's `render_sql_argument_value` for one value; `None` where it holds an opaque value. +pub(crate) fn render_value(value: &ArgumentValue, quoted: bool) -> Option { + Some(match value { ArgumentValue::List(items) => items .iter() .map(|item| render_value(item, quoted)) - .collect::>() + .collect::>>()? .join(", "), ArgumentValue::Boolean(true) => "TRUE".to_owned(), ArgumentValue::Boolean(false) => "FALSE".to_owned(), @@ -74,7 +74,8 @@ pub(crate) fn render_value(value: &ArgumentValue, quoted: bool) -> String { ArgumentValue::Number(text) => text.clone(), ArgumentValue::Text(text) if quoted => format!("'{}'", text.replace('\'', "''")), ArgumentValue::Text(text) => text.clone(), - } + ArgumentValue::Opaque => return None, + }) } /// The end of `[A-Za-z_][A-Za-z0-9_]*` at `start`. @@ -110,30 +111,45 @@ fn opens_call(bytes: &[u8], mut index: usize) -> Option { Some(false) } -/// Python's `render_parameterized_sql`, or None where Python raises or decides. +/// Why rendering stops: Python's missing-argument or value error, or input Python must judge. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum RenderStop { + MissingArgument(String), + /// The argument whose value Python cannot render. + UnsupportedValue(String), + Deferred, +} + +/// Python's `render_parameterized_sql`; unused arguments under `reject_unused` defer. pub(crate) fn render_parameterized_sql( sql: &str, arguments: &[(String, ArgumentValue)], reject_unused: bool, -) -> Option { - let matches = parameter_matches(sql)?; +) -> Result { + let matches = parameter_matches(sql).ok_or(RenderStop::Deferred)?; if reject_unused { for (name, _) in arguments { if !matches.iter().any(|item| item.name == name.as_str()) { - return None; + return Err(RenderStop::Deferred); } } } let mut rendered: String = String::with_capacity(sql.len()); let mut previous_end: usize = 0; for item in &matches { - let (_, value) = arguments + let Some((_, value)) = arguments .iter() - .find(|(name, _)| name.as_str() == item.name)?; + .find(|(name, _)| name.as_str() == item.name) + else { + return Err(RenderStop::MissingArgument(item.name.to_owned())); + }; rendered.push_str(&sql[previous_end..item.start]); - rendered.push_str(&render_value(value, item.quoted)); + let Some(text) = render_value(value, item.quoted) else { + return Err(RenderStop::UnsupportedValue(item.name.to_owned())); + }; + rendered.push_str(&text); previous_end = item.end; } rendered.push_str(&sql[previous_end..]); - Some(rendered) + Ok(rendered) } diff --git a/crates/sqlbuild-attachments/src/audits/_helpers/policies.rs b/crates/sqlbuild-attachments/src/audits/_helpers/policies.rs index b951b8aef0..c9df6e1bce 100644 --- a/crates/sqlbuild-attachments/src/audits/_helpers/policies.rs +++ b/crates/sqlbuild-attachments/src/audits/_helpers/policies.rs @@ -1,33 +1,59 @@ -//! Python's `resolve_audit_severity` and `resolve_audit_run_scope`. +//! Python's `resolve_audit_severity` and `resolve_audit_run_scope`, with their errors. use crate::audits::models::PolicySource; const SEVERITIES: [&str; 2] = ["warn", "error"]; -const RUN_SCOPES: [&str; 2] = ["final", "delta_and_final"]; +/// Python lists run scopes sorted. +const RUN_SCOPES: [&str; 2] = ["delta_and_final", "final"]; +const SETTINGS_FILE: &str = "sqlbuild_project.toml"; -/// The severity text; unknown values defer so Python raises. +/// The severity text, or Python's error naming the unknown value. pub(crate) fn resolve_severity( instance: Option<&str>, default: Option<&str>, -) -> Option<&'static str> { + audit_label: &str, +) -> Result<&'static str, String> { match (instance, default) { - (Some(value), _) | (None, Some(value)) => known(&SEVERITIES, value), - (None, None) => Some("error"), + (Some(value), _) => known(&SEVERITIES, value).ok_or_else(|| { + format!( + "{audit_label}: unknown severity '{value}'; valid values: {}", + SEVERITIES.join(", ") + ) + }), + (None, Some(value)) => known(&SEVERITIES, value) + .ok_or_else(|| unknown_setting("default_audit_severity", value, &SEVERITIES)), + (None, None) => Ok("error"), } } -/// Where the run scope comes from; unknown values defer so Python raises. +/// Where the run scope comes from, or Python's error naming the unknown value. pub(crate) fn resolve_run_scope( instance: Option<&str>, default: Option<&str>, -) -> Option { +) -> Result { match (instance, default) { - (Some(value), _) => known(&RUN_SCOPES, value).map(|_| PolicySource::Instance), - (None, Some(value)) => known(&RUN_SCOPES, value).map(|_| PolicySource::Default), - (None, None) => Some(PolicySource::Fallback), + (Some(value), _) => known(&RUN_SCOPES, value) + .map(|_| PolicySource::Instance) + .ok_or_else(|| { + format!( + "unknown audit run_scope '{value}'; valid values: {}", + RUN_SCOPES.join(", ") + ) + }), + (None, Some(value)) => known(&RUN_SCOPES, value) + .map(|_| PolicySource::Default) + .ok_or_else(|| unknown_setting("default_audit_run_scope", value, &RUN_SCOPES)), + (None, None) => Ok(PolicySource::Fallback), } } +fn unknown_setting(setting: &str, value: &str, valid: &[&str]) -> String { + format!( + "settings.{setting} in {SETTINGS_FILE}: unknown value '{value}'; valid values: {}", + valid.join(", ") + ) +} + fn known(values: &[&'static str], value: &str) -> Option<&'static str> { values.iter().copied().find(|candidate| *candidate == value) } diff --git a/crates/sqlbuild-attachments/src/audits/_helpers/rendering.rs b/crates/sqlbuild-attachments/src/audits/_helpers/rendering.rs new file mode 100644 index 0000000000..cc794952a2 --- /dev/null +++ b/crates/sqlbuild-attachments/src/audits/_helpers/rendering.rs @@ -0,0 +1,119 @@ +//! Python's argument merge, SQL rendering and policy resolution for one attachment. + +use crate::audits::_helpers::parameters::{RenderStop, render_parameterized_sql}; +use crate::audits::_helpers::policies::{resolve_run_scope, resolve_severity}; +use crate::audits::models::{ArgumentValue, AuditAttachment, PolicySource}; + +/// Severity first, then run scope, as Python resolves them after expanding the SQL. +pub(crate) fn policies( + attachment: &AuditAttachment, +) -> Result<(&'static str, PolicySource), String> { + let severity: &'static str = if attachment.has_thresholds { + if attachment.threshold_error { + "error" + } else { + "warn" + } + } else { + resolve_severity( + attachment.instance_severity.as_deref(), + attachment.default_severity.as_deref(), + &format!( + "{} audit '{}'", + attachment.owner_label, attachment.definition_name + ), + )? + }; + let run_scope_source: PolicySource = resolve_run_scope( + attachment.instance_run_scope.as_deref(), + attachment.default_run_scope.as_deref(), + )?; + Ok((severity, run_scope_source)) +} + +/// One SQL text rendered, Python's missing-argument error, or None to defer. +pub(crate) fn rendered( + attachment: &AuditAttachment, + sql: &str, + arguments: &[(String, ArgumentValue)], +) -> Option> { + match render_parameterized_sql(sql, arguments, false) { + Ok(rendered) => Some(Ok(rendered)), + Err(RenderStop::MissingArgument(name)) => Some(Err(format!( + "{} is missing argument '{name}' for generic audit '{}'", + attachment.owner_label, attachment.definition_name + ))), + Err(RenderStop::UnsupportedValue(name)) => Some(Err(format!( + "{} generic audit '{}' argument '{name}' uses an unsupported value", + attachment.owner_label, attachment.definition_name + ))), + Err(RenderStop::Deferred) => None, + } +} + +/// Python's `merge_audit_arguments`; None where only Python's `!=` can compare the values. +pub(crate) fn merged_arguments( + attachment: &AuditAttachment, +) -> Option, String>> { + let mut merged: Vec<(String, ArgumentValue)> = attachment.implicit_arguments.clone(); + for (name, value) in &attachment.explicit_arguments { + match merged.iter_mut().find(|(existing, _)| existing == name) { + Some((_, existing)) => { + if python_unequal(existing, value)? { + return Some(Err(format!( + "{} audit '{}' must not override implicit {name} from attached context", + attachment.owner_label, attachment.definition_name + ))); + } + } + None => merged.push((name.clone(), value.clone())), + } + } + Some(Ok(merged)) +} + +/// Python's `left != right`, or None for nested opaque values and numbers differing in text. +fn python_unequal(left: &ArgumentValue, right: &ArgumentValue) -> Option { + if contains_opaque(left) || contains_opaque(right) { + return None; + } + if left == right { + return Some(false); + } + match (left, right) { + (ArgumentValue::List(left), ArgumentValue::List(right)) => { + if left.len() != right.len() { + return Some(true); + } + let mut undecided: bool = false; + for (left, right) in left.iter().zip(right) { + match python_unequal(left, right) { + Some(true) => return Some(true), + Some(false) => {} + None => undecided = true, + } + } + if undecided { None } else { Some(false) } + } + ( + ArgumentValue::Number(_) | ArgumentValue::Boolean(_), + ArgumentValue::Number(_) | ArgumentValue::Boolean(_), + ) if !matches!( + (left, right), + (ArgumentValue::Boolean(_), ArgumentValue::Boolean(_)) + ) => + { + None + } + _ => Some(true), + } +} + +/// Whether `value` holds an opaque value at any depth, which only Python can compare. +fn contains_opaque(value: &ArgumentValue) -> bool { + match value { + ArgumentValue::Opaque => true, + ArgumentValue::List(items) => items.iter().any(contains_opaque), + _ => false, + } +} diff --git a/crates/sqlbuild-attachments/src/audits/main/render_attached_audit.rs b/crates/sqlbuild-attachments/src/audits/main/render_attached_audit.rs index 8de9074e92..832440a26a 100644 --- a/crates/sqlbuild-attachments/src/audits/main/render_attached_audit.rs +++ b/crates/sqlbuild-attachments/src/audits/main/render_attached_audit.rs @@ -1,12 +1,11 @@ //! Python's attached generic audit rendering: argument merge, SQL, severity and run scope. -use crate::audits::_helpers::parameters::render_parameterized_sql; -use crate::audits::_helpers::policies::{resolve_run_scope, resolve_severity}; -use crate::audits::models::{ArgumentValue, AuditAttachment, PolicySource, RenderedAudit}; +use crate::audits::_helpers::rendering::{merged_arguments, policies, rendered}; +use crate::audits::models::{ArgumentValue, AuditAttachment, AuditRendering, RenderedAudit}; -/// Render one attachment as Python does, or None where Python raises or decides. +/// Render one attachment as Python does, with its errors, or None where Python must decide. #[must_use] -pub fn render_attached_audit(attachment: &AuditAttachment) -> Option { +pub fn render_attached_audit(attachment: &AuditAttachment) -> Option { if attachment.measurement { if attachment.instance_severity.is_some() || !attachment.has_thresholds { return None; @@ -14,45 +13,24 @@ pub fn render_attached_audit(attachment: &AuditAttachment) -> Option = merged_arguments(attachment)?; - let sql_body: String = render_parameterized_sql(&attachment.sql_body, &arguments, false)?; + let arguments: Vec<(String, ArgumentValue)> = match merged_arguments(attachment)? { + Ok(arguments) => arguments, + Err(message) => return Some(AuditRendering::Failed(message)), + }; + let sql_body: String = match rendered(attachment, &attachment.sql_body, &arguments)? { + Ok(sql) => sql, + Err(message) => return Some(AuditRendering::Failed(message)), + }; let evidence_sql: Option = match &attachment.evidence_sql { - Some(evidence) => Some(render_parameterized_sql(evidence, &arguments, false)?), + Some(evidence) => match rendered(attachment, evidence, &arguments)? { + Ok(sql) => Some(sql), + Err(message) => return Some(AuditRendering::Failed(message)), + }, None => None, }; - let severity: &'static str = if attachment.has_thresholds { - if attachment.threshold_error { - "error" - } else { - "warn" - } - } else { - resolve_severity( - attachment.instance_severity.as_deref(), - attachment.default_severity.as_deref(), - )? - }; - let run_scope_source: PolicySource = resolve_run_scope( - attachment.instance_run_scope.as_deref(), - attachment.default_run_scope.as_deref(), - )?; - Some(RenderedAudit { + Some(AuditRendering::Rendered(RenderedAudit { sql_body, evidence_sql, - severity, - run_scope_source, - }) -} - -/// Python's `merge_audit_arguments`; values that differ only as Python objects defer. -fn merged_arguments(attachment: &AuditAttachment) -> Option> { - let mut merged: Vec<(String, ArgumentValue)> = attachment.implicit_arguments.clone(); - for (name, value) in &attachment.explicit_arguments { - match merged.iter_mut().find(|(existing, _)| existing == name) { - Some((_, existing)) if existing == value => {} - Some(_) => return None, - None => merged.push((name.clone(), value.clone())), - } - } - Some(merged) + policies: policies(attachment), + })) } diff --git a/crates/sqlbuild-attachments/src/audits/models.rs b/crates/sqlbuild-attachments/src/audits/models.rs index d14d4abb39..cb99f154c1 100644 --- a/crates/sqlbuild-attachments/src/audits/models.rs +++ b/crates/sqlbuild-attachments/src/audits/models.rs @@ -8,6 +8,8 @@ pub enum ArgumentValue { Number(String), Text(String), List(Vec), + /// A value Python cannot render, such as a mapping; rendering it is Python's error. + Opaque, } /// Where a resolved run scope came from, so Python keeps its own value object. @@ -21,6 +23,9 @@ pub enum PolicySource { /// One generic audit attachment: definition SQL, arguments and authored policies. #[derive(Clone, Debug, PartialEq, Eq)] pub struct AuditAttachment { + /// Python's `str(owner_file)` and the generic audit name, which its error messages quote. + pub owner_label: String, + pub definition_name: String, pub sql_body: String, pub evidence_sql: Option, pub implicit_arguments: Vec<(String, ArgumentValue)>, @@ -35,12 +40,18 @@ pub struct AuditAttachment { pub default_run_scope: Option, } -/// The rendered SQL and the policies one attachment resolves to. +/// The rendered SQL and the policies one attachment resolves to; severity is `warn` or `error`. #[derive(Clone, Debug, PartialEq, Eq)] pub struct RenderedAudit { pub sql_body: String, pub evidence_sql: Option, - /// `warn` or `error`. - pub severity: &'static str, - pub run_scope_source: PolicySource, + /// `(severity, run scope source)`, or the error Python raises once the SQL is expanded. + pub policies: Result<(&'static str, PolicySource), String>, +} + +/// Python's rendering of one attachment: its SQL and policies, or the error rendering raises. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum AuditRendering { + Rendered(RenderedAudit), + Failed(String), } diff --git a/crates/sqlbuild-attachments/src/audits/tests/helpers.rs b/crates/sqlbuild-attachments/src/audits/tests/helpers.rs index 2e18592014..cff143ce9e 100644 --- a/crates/sqlbuild-attachments/src/audits/tests/helpers.rs +++ b/crates/sqlbuild-attachments/src/audits/tests/helpers.rs @@ -1,6 +1,8 @@ +use crate::audits::_helpers::parameters::RenderStop; use crate::audits::_helpers::parameters::render_parameterized_sql; -use crate::audits::main::render_attached_audit::render_attached_audit; -use crate::audits::models::{ArgumentValue, AuditAttachment, RenderedAudit}; +use crate::audits::models::{ + ArgumentValue, AuditAttachment, AuditRendering, PolicySource, RenderedAudit, +}; /// Arguments shared by the parameter rendering cases. pub(super) fn order_arguments() -> Vec<(String, ArgumentValue)> { @@ -21,7 +23,7 @@ pub(super) fn order_arguments() -> Vec<(String, ArgumentValue)> { ] } -pub(super) fn rendered_sql(sql: &str, reject_unused: bool) -> Option { +pub(super) fn rendered_sql(sql: &str, reject_unused: bool) -> Result { render_parameterized_sql(sql, &order_arguments(), reject_unused) } @@ -32,6 +34,8 @@ pub(super) fn attachment( run_scope: Option<&str>, ) -> AuditAttachment { AuditAttachment { + owner_label: "models/schema.yml".to_owned(), + definition_name: "accepted_values".to_owned(), sql_body: "SELECT * FROM t WHERE @column NOT IN (@'values')".to_owned(), evidence_sql: Some("SELECT @column FROM t".to_owned()), implicit_arguments: vec![( @@ -50,13 +54,18 @@ pub(super) fn attachment( } } -pub(super) fn rendering_text(attachment: &AuditAttachment) -> Option { - let rendered: RenderedAudit = render_attached_audit(attachment)?; - Some(format!( - "{} | {} | {} | {:?}", - rendered.sql_body, - rendered.evidence_sql.unwrap_or_default(), - rendered.severity, - rendered.run_scope_source - )) +/// The rendering of `attachment()` SQL: `sql_body`, the fixed evidence and these policies. +pub(super) fn rendered( + sql_body: &str, + policies: Result<(&'static str, PolicySource), &str>, +) -> Option { + Some(AuditRendering::Rendered(RenderedAudit { + sql_body: sql_body.to_owned(), + evidence_sql: Some("SELECT status FROM t".to_owned()), + policies: policies.map_err(str::to_owned), + })) +} + +pub(super) fn failed(message: &str) -> Option { + Some(AuditRendering::Failed(message.to_owned())) } diff --git a/crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs b/crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs index d4adaa5cfe..6792966c6e 100644 --- a/crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs +++ b/crates/sqlbuild-attachments/src/audits/tests/test_render_attached_audit.rs @@ -1,5 +1,6 @@ -use crate::audits::models::ArgumentValue; -use crate::audits::tests::helpers::{attachment, rendering_text}; +use crate::audits::main::render_attached_audit::render_attached_audit; +use crate::audits::models::{ArgumentValue, AuditAttachment, PolicySource}; +use crate::audits::tests::helpers::{attachment, failed, rendered}; use crate::audits::tests::test_types::AttachedAuditTestCase; #[test] @@ -12,19 +13,21 @@ fn given_attached_audits_when_rendering_then_python_rendering_is_returned() { AttachedAuditTestCase { description: "explicit arguments merge after implicit ones with default policies", attachment: attachment(vec![values.clone()], None, None), - expected_rendering: Some( - "SELECT * FROM t WHERE status NOT IN (1) | SELECT status FROM t | error | Fallback", + expected_rendering: rendered( + "SELECT * FROM t WHERE status NOT IN (1)", + Ok(("error", PolicySource::Fallback)), ), }, AttachedAuditTestCase { description: "instance severity and run scope", attachment: attachment(vec![values.clone()], Some("warn"), Some("final")), - expected_rendering: Some( - "SELECT * FROM t WHERE status NOT IN (1) | SELECT status FROM t | warn | Instance", + expected_rendering: rendered( + "SELECT * FROM t WHERE status NOT IN (1)", + Ok(("warn", PolicySource::Instance)), ), }, AttachedAuditTestCase { - description: "an explicit argument overriding an implicit one defers", + description: "an explicit argument overriding an implicit one is Python's error", attachment: attachment( vec![( "column".to_owned(), @@ -33,23 +36,104 @@ fn given_attached_audits_when_rendering_then_python_rendering_is_returned() { None, None, ), - expected_rendering: None, + expected_rendering: failed( + "models/schema.yml audit 'accepted_values' must not override \ + implicit column from attached context", + ), }, AttachedAuditTestCase { - description: "an unknown severity defers so Python raises", - attachment: attachment(vec![values.clone()], Some("fatal"), None), + description: "an override Python's equality must judge defers", + attachment: AuditAttachment { + implicit_arguments: vec![( + "values".to_owned(), + ArgumentValue::Number("1".to_owned()), + )], + ..attachment( + vec![("values".to_owned(), ArgumentValue::Number("1.0".to_owned()))], + None, + None, + ) + }, expected_rendering: None, }, AttachedAuditTestCase { - description: "an unknown run scope defers so Python raises", - attachment: attachment(vec![values], None, Some("delta")), + description: "an override holding a nested opaque value defers to Python's equality", + attachment: AuditAttachment { + implicit_arguments: vec![( + "values".to_owned(), + ArgumentValue::List(vec![ArgumentValue::Opaque]), + )], + ..attachment( + vec![( + "values".to_owned(), + ArgumentValue::List(vec![ArgumentValue::Opaque]), + )], + None, + None, + ) + }, expected_rendering: None, }, + AttachedAuditTestCase { + description: "a missing argument is Python's error before the policies", + attachment: attachment(Vec::new(), Some("fatal"), None), + expected_rendering: failed( + "models/schema.yml is missing argument 'values' for generic audit \ + 'accepted_values'", + ), + }, + AttachedAuditTestCase { + description: "an opaque value inside a referenced list is Python's value error", + attachment: attachment( + vec![( + "values".to_owned(), + ArgumentValue::List(vec![ArgumentValue::Null, ArgumentValue::Opaque]), + )], + None, + None, + ), + expected_rendering: failed( + "models/schema.yml generic audit 'accepted_values' argument \ + 'values' uses an unsupported value", + ), + }, + AttachedAuditTestCase { + description: "an unknown severity is raised after rendering, before the run scope", + attachment: attachment(vec![values.clone()], Some("fatal"), Some("delta")), + expected_rendering: rendered( + "SELECT * FROM t WHERE status NOT IN (1)", + Err("models/schema.yml audit \ + 'accepted_values': unknown severity 'fatal'; valid values: warn, error"), + ), + }, + AttachedAuditTestCase { + description: "an unknown run scope lists the sorted valid values", + attachment: attachment(vec![values.clone()], None, Some("delta")), + expected_rendering: rendered( + "SELECT * FROM t WHERE status NOT IN (1)", + Err("unknown audit run_scope \ + 'delta'; valid values: delta_and_final, final"), + ), + }, + AttachedAuditTestCase { + description: "unknown project defaults name their setting", + attachment: AuditAttachment { + default_severity: Some("warn".to_owned()), + default_run_scope: Some("always".to_owned()), + ..attachment(vec![values], None, None) + }, + expected_rendering: rendered( + "SELECT * FROM t WHERE status NOT IN (1)", + Err("\ + settings.default_audit_run_scope in sqlbuild_project.toml: unknown value \ + 'always'; valid values: delta_and_final, final"), + ), + }, ]; for test_case in test_cases { assert_eq!( - rendering_text(&test_case.attachment).as_deref(), + render_attached_audit(&test_case.attachment), test_case.expected_rendering, "{}", test_case.description diff --git a/crates/sqlbuild-attachments/src/audits/tests/test_render_parameterized_sql.rs b/crates/sqlbuild-attachments/src/audits/tests/test_render_parameterized_sql.rs index ee0822a9e4..7b19079955 100644 --- a/crates/sqlbuild-attachments/src/audits/tests/test_render_parameterized_sql.rs +++ b/crates/sqlbuild-attachments/src/audits/tests/test_render_parameterized_sql.rs @@ -1,3 +1,4 @@ +use crate::audits::_helpers::parameters::RenderStop; use crate::audits::tests::helpers::rendered_sql; use crate::audits::tests::test_types::ParameterizedSqlTestCase; @@ -8,7 +9,7 @@ fn given_parameterized_sql_when_rendering_then_python_text_is_returned() { description: "raw and quoted parameters: lists joined, only text quoted, quotes doubled", sql: "WHERE @column IN (@'values') AND x = @values", reject_unused: false, - expected_sql: Some( + expected_sql: Ok( "WHERE status IN ('it''s', 2, NULL, TRUE) AND x = it's, 2, NULL, TRUE", ), }, @@ -16,32 +17,32 @@ fn given_parameterized_sql_when_rendering_then_python_text_is_returned() { description: "macro calls, doubled @ and runtime placeholders are left alone", sql: "@column (x) @@column @@@column @column(y)", reject_unused: false, - expected_sql: Some("@column (x) @@column @@@column @column(y)"), + expected_sql: Ok("@column (x) @@column @@@column @column(y)"), }, ParameterizedSqlTestCase { - description: "a missing argument defers so Python raises", - sql: "WHERE @missing", + description: "the first missing argument stops rendering with its name", + sql: "WHERE @column = @missing AND @'other'", reject_unused: false, - expected_sql: None, + expected_sql: Err(RenderStop::MissingArgument("missing".to_owned())), }, ParameterizedSqlTestCase { description: "an unused argument defers when unused arguments are rejected", sql: "WHERE @column", reject_unused: true, - expected_sql: None, + expected_sql: Err(RenderStop::Deferred), }, ParameterizedSqlTestCase { description: "a non-ASCII character after a name defers to Python's whitespace", sql: "WHERE @column\u{a0}(", reject_unused: false, - expected_sql: None, + expected_sql: Err(RenderStop::Deferred), }, ]; for test_case in test_cases { assert_eq!( - rendered_sql(test_case.sql, test_case.reject_unused).as_deref(), - test_case.expected_sql, + rendered_sql(test_case.sql, test_case.reject_unused), + test_case.expected_sql.map(str::to_owned), "{}", test_case.description ); diff --git a/crates/sqlbuild-attachments/src/audits/tests/test_types.rs b/crates/sqlbuild-attachments/src/audits/tests/test_types.rs index fad31e8d91..b9e2aee9a9 100644 --- a/crates/sqlbuild-attachments/src/audits/tests/test_types.rs +++ b/crates/sqlbuild-attachments/src/audits/tests/test_types.rs @@ -1,15 +1,16 @@ -use crate::audits::models::AuditAttachment; +use crate::audits::_helpers::parameters::RenderStop; +use crate::audits::models::{AuditAttachment, AuditRendering}; pub(super) struct ParameterizedSqlTestCase { pub(super) description: &'static str, pub(super) sql: &'static str, pub(super) reject_unused: bool, - pub(super) expected_sql: Option<&'static str>, + pub(super) expected_sql: Result<&'static str, RenderStop>, } pub(super) struct AttachedAuditTestCase { pub(super) description: &'static str, pub(super) attachment: AuditAttachment, - /// `(sql body, evidence, severity, run scope source)` spelled as text, or None to defer. - pub(super) expected_rendering: Option<&'static str>, + /// None where Python must decide. + pub(super) expected_rendering: Option, } diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/_helpers/scan.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/_helpers/scan.rs index f38db65c8e..15d1f92248 100644 --- a/crates/sqlbuild-attachments/src/cursor_intrinsics/_helpers/scan.rs +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/_helpers/scan.rs @@ -1,44 +1,104 @@ -//! Python's cursor intrinsic scan over quotes, comments and identifier boundaries. +//! Python's cursor intrinsic scan over quotes, comments, identifier boundaries and calls. + +use sqlbuild_core::text::main::is_python_space::is_python_space; +use sqlbuild_sqltext::sql_scan::models::Unclosed; use crate::sql_lexing::main::python_non_code_end::python_non_code_end; +use crate::sql_lexing::main::unclosed_construct::unclosed_construct; use crate::sql_lexing::models::NonCode; -const INTRINSIC_NAMES: [&[u8]; 2] = [b"__cursor_start", b"__cursor_end"]; -/// The next scan offset after `index`, or `Err` where Python finds an intrinsic or raises. -pub(crate) fn step(bytes: &[u8], index: usize) -> Result { +const INTRINSIC_NAMES: [&str; 2] = ["__cursor_start", "__cursor_end"]; + +/// What Python's scan finds at one offset. +pub(crate) enum ScanStep { + Next(usize), + Unclosed(Unclosed), + /// An intrinsic name at identifier boundaries, ending at `end`. + Intrinsic { + name: &'static str, + end: usize, + }, + /// Python decides by its Unicode identifier rules. + Deferred, +} + +/// How the text after an intrinsic name reads as Python's empty call. +pub(crate) enum CallScan { + /// `name()`, with the offset of the closing parenthesis. + Called(usize), + NotCalled, + Arguments, + UnclosedParenthesis, + /// Quotes, comments or nested parentheses inside the call, which only Python matches. + Deferred, +} + +/// Python's scan at `index`. +pub(crate) fn step(bytes: &[u8], index: usize) -> ScanStep { match python_non_code_end(bytes, index) { - NonCode::End(end) => return Ok(end), - NonCode::Raises => return Err(()), + NonCode::End(end) => return ScanStep::Next(end), + NonCode::Raises => return ScanStep::Unclosed(unclosed_construct(bytes, index)), NonCode::Code => {} } for name in INTRINSIC_NAMES { - if bytes[index..].starts_with(name) { + if bytes[index..].starts_with(name.as_bytes()) { let end: usize = index + name.len(); let before: Option = index.checked_sub(1).map(|previous| bytes[previous]); - if !continues_identifier(before)? && !continues_identifier(bytes.get(end).copied())? { - return Err(()); + return match ( + continues_identifier(before), + continues_identifier(bytes.get(end).copied()), + ) { + (Some(false), Some(false)) => ScanStep::Intrinsic { name, end }, + (Some(_), Some(_)) => ScanStep::Next(end), + _ => ScanStep::Deferred, + }; + } + } + ScanStep::Next(index + 1) +} + +/// Python's whitespace skip, `(` check, parenthesis match and argument check after a name. +pub(crate) fn call_scan(sql: &str, name_end: usize) -> CallScan { + let mut open: usize = name_end; + while let Some(character) = sql[open..].chars().next() { + if !is_python_space(character) { + break; + } + open += character.len_utf8(); + } + let bytes: &[u8] = sql.as_bytes(); + if bytes.get(open) != Some(&b'(') { + return CallScan::NotCalled; + } + let mut index: usize = open + 1; + while let Some(byte) = bytes.get(index) { + match byte { + b')' => { + return if sql[open + 1..index].chars().all(is_python_space) { + CallScan::Called(index) + } else { + CallScan::Arguments + }; } - return Ok(end); + b'-' if bytes.get(index + 1) == Some(&b'-') => return CallScan::Deferred, + b'/' if bytes.get(index + 1) == Some(&b'*') => return CallScan::Deferred, + b'#' | b'\'' | b'"' | b'`' | b'$' | b'(' => return CallScan::Deferred, + _ => index += 1, } } - Ok(index + 1) + CallScan::UnclosedParenthesis } /// Python's `isalnum() or _`; non-ASCII neighbours defer to Python's Unicode rules. -fn continues_identifier(byte: Option) -> Result { +fn continues_identifier(byte: Option) -> Option { match byte { - None => Ok(false), - Some(byte) if !byte.is_ascii() => Err(()), - Some(byte) => Ok(byte.is_ascii_alphanumeric() || byte == b'_'), + None => Some(false), + Some(byte) if !byte.is_ascii() => None, + Some(byte) => Some(byte.is_ascii_alphanumeric() || byte == b'_'), } } /// Python's `any(name in sql for name in _INTRINSIC_NAMES)`. -pub(crate) fn contains_intrinsic_name(bytes: &[u8]) -> bool { - for name in INTRINSIC_NAMES { - if bytes.windows(name.len()).any(|window| window == name) { - return true; - } - } - false +pub(crate) fn contains_intrinsic_name(sql: &str) -> bool { + INTRINSIC_NAMES.iter().any(|name| sql.contains(name)) } diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/main/intrinsic_free.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/main/intrinsic_free.rs index b375e99da5..57c70b8242 100644 --- a/crates/sqlbuild-attachments/src/cursor_intrinsics/main/intrinsic_free.rs +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/main/intrinsic_free.rs @@ -1,27 +1,71 @@ -//! Python's `reject_cursor_intrinsics` for SQL that it accepts. +//! Python's `reject_cursor_intrinsics`, with the error it raises. -use crate::cursor_intrinsics::_helpers::scan::{contains_intrinsic_name, step}; +use crate::cursor_intrinsics::_helpers::scan::{ + CallScan, ScanStep, call_scan, contains_intrinsic_name, step, +}; use crate::cursor_intrinsics::models::IntrinsicCheck; +use crate::sql_lexing::main::unclosed_message::unclosed_message; +use sqlbuild_sqltext::sql_scan::models::Unclosed; -/// `Free` where Python accepts `sql`; anything Python could reject or judge by Unicode defers. +/// Python's acceptance of `sql`, its error for `context`, or `Deferred` where only Python decides. #[must_use] -pub fn intrinsic_free(sql: &str, reserved_markers: &[String]) -> IntrinsicCheck { +pub fn intrinsic_free(sql: &str, reserved_markers: &[String], context: &str) -> IntrinsicCheck { if reserved_markers .iter() .any(|marker| sql.contains(marker.as_str())) { - return IntrinsicCheck::Deferred; + return IntrinsicCheck::Rejected(format!( + "{context} contains a reserved internal cursor marker" + )); } - let bytes: &[u8] = sql.as_bytes(); - if !contains_intrinsic_name(bytes) { + if !contains_intrinsic_name(sql) { return IntrinsicCheck::Free; } + let bytes: &[u8] = sql.as_bytes(); + let mut found: bool = false; let mut index: usize = 0; while index < bytes.len() { - let Ok(next) = step(bytes, index) else { - return IntrinsicCheck::Deferred; + let (name, end): (&str, usize) = match step(bytes, index) { + ScanStep::Next(next) => { + index = next; + continue; + } + ScanStep::Unclosed(construct) => { + return IntrinsicCheck::Rejected(unclosed_message(context, construct)); + } + ScanStep::Deferred => return IntrinsicCheck::Deferred, + ScanStep::Intrinsic { name, end } => (name, end), }; - index = next; + match call_scan(sql, end) { + CallScan::Called(close) => { + found = true; + index = close + 1; + } + CallScan::NotCalled => { + return IntrinsicCheck::Rejected(format!( + "{context} intrinsic {name} must be called with ()" + )); + } + CallScan::Arguments => { + return IntrinsicCheck::Rejected(format!( + "{context} intrinsic {name} does not accept arguments" + )); + } + CallScan::UnclosedParenthesis => { + return IntrinsicCheck::Rejected(unclosed_message( + &format!("{context} cursor intrinsic"), + Unclosed::Parenthesis, + )); + } + CallScan::Deferred => return IntrinsicCheck::Deferred, + } + } + if found { + IntrinsicCheck::Rejected(format!( + "{context} uses cursor intrinsics, which are only supported in cursor-based \ + incremental model query SQL" + )) + } else { + IntrinsicCheck::Free } - IntrinsicCheck::Free } diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/models.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/models.rs index 496c2f0225..4844810821 100644 --- a/crates/sqlbuild-attachments/src/cursor_intrinsics/models.rs +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/models.rs @@ -1,8 +1,9 @@ //! Outcome of the native cursor intrinsic check. -/// Whether SQL is free of cursor intrinsics, or Python must decide (and usually raise). -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +/// Whether Python accepts SQL as intrinsic-free, rejects it with this error, or must decide. +#[derive(Clone, Debug, PartialEq, Eq)] pub enum IntrinsicCheck { Free, + Rejected(String), Deferred, } diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/tests.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests.rs index e5bc1da50a..950265ac16 100644 --- a/crates/sqlbuild-attachments/src/cursor_intrinsics/tests.rs +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests.rs @@ -1,3 +1,5 @@ +#[path = "tests/helpers.rs"] +mod helpers; #[path = "tests/test_intrinsic_free.rs"] mod intrinsic_free; #[path = "tests/test_types.rs"] diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/helpers.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/helpers.rs new file mode 100644 index 0000000000..8bf22ff204 --- /dev/null +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/helpers.rs @@ -0,0 +1,5 @@ +use crate::cursor_intrinsics::models::IntrinsicCheck; + +pub(super) fn rejected(message: &str) -> IntrinsicCheck { + IntrinsicCheck::Rejected(message.to_owned()) +} diff --git a/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/test_intrinsic_free.rs b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/test_intrinsic_free.rs index 5d398e4442..f95d1c9bd7 100644 --- a/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/test_intrinsic_free.rs +++ b/crates/sqlbuild-attachments/src/cursor_intrinsics/tests/test_intrinsic_free.rs @@ -1,9 +1,10 @@ use crate::cursor_intrinsics::main::intrinsic_free::intrinsic_free; use crate::cursor_intrinsics::models::IntrinsicCheck; +use crate::cursor_intrinsics::tests::helpers::rejected; use crate::cursor_intrinsics::tests::test_types::IntrinsicCheckTestCase; #[test] -fn given_sql_when_checking_cursor_intrinsics_then_python_acceptance_is_returned() { +fn given_sql_when_checking_cursor_intrinsics_then_python_acceptance_or_error_is_returned() { let test_cases = [ IntrinsicCheckTestCase { description: "SQL without intrinsic names is free", @@ -17,30 +18,66 @@ fn given_sql_when_checking_cursor_intrinsics_then_python_acceptance_is_returned( expected_check: IntrinsicCheck::Free, }, IntrinsicCheckTestCase { - description: "an intrinsic call defers so Python raises", - sql: "WHERE ts >= __cursor_start()", - expected_check: IntrinsicCheck::Deferred, + description: "a well-formed call is rejected once the whole SQL is scanned", + sql: "WHERE ts >= __cursor_start( ) AND ts < __cursor_end\t()", + expected_check: rejected( + "Audit 'fresh' uses cursor intrinsics, which are only supported in cursor-based \ + incremental model query SQL", + ), + }, + IntrinsicCheckTestCase { + description: "a later malformed call is raised before the use itself", + sql: "WHERE ts >= __cursor_start() AND ts < __cursor_end", + expected_check: rejected("Audit 'fresh' intrinsic __cursor_end must be called with ()"), + }, + IntrinsicCheckTestCase { + description: "call arguments are rejected", + sql: "WHERE ts >= __cursor_end(1 - 2)", + expected_check: rejected( + "Audit 'fresh' intrinsic __cursor_end does not accept arguments", + ), + }, + IntrinsicCheckTestCase { + description: "an unclosed call names the cursor intrinsic context", + sql: "WHERE ts >= __cursor_start(", + expected_check: rejected( + "Audit 'fresh' cursor intrinsic contains an unclosed parenthesis", + ), }, IntrinsicCheckTestCase { - description: "an unclosed quote after an intrinsic name defers", + description: "an unclosed quote after an intrinsic name is Python's quote error", sql: "SELECT x__cursor_start, 'open", - expected_check: IntrinsicCheck::Deferred, + expected_check: rejected("Audit 'fresh' contains an unclosed quoted string"), }, IntrinsicCheckTestCase { - description: "a non-ASCII neighbour defers to Python's Unicode identifier rules", - sql: "SELECT é__cursor_end", + description: "an unclosed block comment before a call is Python's comment error", + sql: "SELECT __cursor_end() /* open", + expected_check: rejected("Audit 'fresh' contains an unclosed block comment"), + }, + IntrinsicCheckTestCase { + description: "a reserved marker is rejected first", + sql: "SELECT __reserved_marker__, __cursor_start(", + expected_check: rejected("Audit 'fresh' contains a reserved internal cursor marker"), + }, + IntrinsicCheckTestCase { + description: "quotes inside a call defer to Python's parenthesis matching", + sql: "WHERE ts >= __cursor_start(')')", expected_check: IntrinsicCheck::Deferred, }, IntrinsicCheckTestCase { - description: "a reserved marker defers", - sql: "SELECT __reserved_marker__", + description: "a non-ASCII neighbour defers to Python's Unicode identifier rules", + sql: "SELECT é__cursor_end", expected_check: IntrinsicCheck::Deferred, }, ]; for test_case in test_cases { assert_eq!( - intrinsic_free(test_case.sql, &["__reserved_marker__".to_owned()]), + intrinsic_free( + test_case.sql, + &["__reserved_marker__".to_owned()], + "Audit 'fresh'" + ), test_case.expected_check, "{}", test_case.description diff --git a/crates/sqlbuild-attachments/src/functions/_helpers/header_parsing.rs b/crates/sqlbuild-attachments/src/functions/_helpers/header_parsing.rs new file mode 100644 index 0000000000..834d5ca2de --- /dev/null +++ b/crates/sqlbuild-attachments/src/functions/_helpers/header_parsing.rs @@ -0,0 +1,174 @@ +//! Python's header checks in the order its attachment raises them, with their messages. + +use crate::functions::_helpers::values::{ + TABLE_RETURN_KEY, description, lookup, named_types, stripped_list, stripped_text, +}; +use crate::functions::errors::NamedTypeError; +use crate::functions::models::{ + FunctionHeader, FunctionLanguage, FunctionReturns, HeaderFailure, HeaderStage, HeaderValue, + NamedType, +}; + +/// Python's error messages for one function file. +pub(crate) struct HeaderErrors<'path> { + pub(crate) language: FunctionLanguage, + pub(crate) relative_path: &'path str, +} + +impl HeaderErrors<'_> { + fn fail(&self, stage: HeaderStage, problem: &str) -> HeaderFailure { + let language: &str = match self.language { + FunctionLanguage::Sql => "SQL", + FunctionLanguage::Python => "Python", + }; + HeaderFailure { + stage, + message: format!("{language} function file {} {problem}", self.relative_path), + } + } + + fn named_type(&self, stage: HeaderStage, error: NamedTypeError, kind: &str) -> HeaderFailure { + match error { + NamedTypeError::InvalidName => self.fail(stage, &format!("has an invalid {kind} name")), + NamedTypeError::MissingType(name) => { + self.fail(stage, &format!("{kind} '{name}' must declare a type")) + } + } + } + + fn list(&self, stage: HeaderStage, key: &str, not_list: bool) -> HeaderFailure { + if not_list { + self.fail(stage, &format!("{key} must be a list")) + } else { + self.fail(stage, &format!("{key} entries must be non-empty strings")) + } + } +} + +/// The header as Python parses it, stopping at its first error. +pub(crate) fn parsed_header( + header: &[(String, HeaderValue)], + errors: &HeaderErrors<'_>, +) -> FunctionHeader { + let python: bool = errors.language == FunctionLanguage::Python; + let mut parsed = FunctionHeader { + arguments: Vec::new(), + returns: None, + tags: Vec::new(), + description: None, + runtime_version: None, + entry_point: None, + packages: Vec::new(), + failure: None, + }; + let raw_returns: Option<&HeaderValue> = lookup(header, "returns"); + let scalar_returns: Option = stripped_text(raw_returns); + if raw_returns.is_none() || (python && scalar_returns.is_none()) { + return failed( + parsed, + errors.fail(HeaderStage::Start, "must declare returns"), + ); + } + match lookup(header, "arguments") { + None => {} + Some(HeaderValue::Map(entries)) => { + let (arguments, error) = named_types(entries); + parsed.arguments = arguments; + if let Some(error) = error { + let failure = errors.named_type(HeaderStage::Arguments, error, "argument"); + return failed(parsed, failure); + } + } + Some(_) => { + let failure = errors.fail(HeaderStage::Arguments, "arguments must be a map"); + return failed(parsed, failure); + } + } + if let Some(text) = scalar_returns { + parsed.returns = Some(FunctionReturns::Type(text)); + } else { + let (columns, failure) = table_returns(raw_returns, errors); + parsed.returns = Some(FunctionReturns::Table(columns)); + if let Some(failure) = failure { + return failed(parsed, failure); + } + } + if python { + parsed.runtime_version = stripped_text(lookup(header, "runtime_version")); + if parsed.runtime_version.is_none() { + let failure = errors.fail(HeaderStage::PythonValues, "must declare runtime_version"); + return failed(parsed, failure); + } + parsed.entry_point = stripped_text(lookup(header, "entry_point")); + if parsed.entry_point.is_none() { + let failure = errors.fail(HeaderStage::PythonValues, "must declare entry_point"); + return failed(parsed, failure); + } + match stripped_list(lookup(header, "packages")) { + Ok(packages) => parsed.packages = packages, + Err(not_list) => { + let failure = errors.list(HeaderStage::PythonValues, "packages", not_list); + return failed(parsed, failure); + } + } + } + match stripped_list(lookup(header, "tags")) { + Ok(tags) => parsed.tags = tags, + Err(not_list) => { + let failure = errors.list(HeaderStage::Metadata, "tags", not_list); + return failed(parsed, failure); + } + } + match description(lookup(header, "description")) { + Some(text) => parsed.description = text, + None => { + let failure = errors.fail(HeaderStage::Metadata, "description must be a string"); + return failed(parsed, failure); + } + } + parsed +} + +fn failed(parsed: FunctionHeader, failure: HeaderFailure) -> FunctionHeader { + FunctionHeader { + failure: Some(failure), + ..parsed + } +} + +/// Python's `returns (table (...))` columns: exactly the `table` key with a non-empty column map. +fn table_returns( + raw_returns: Option<&HeaderValue>, + errors: &HeaderErrors<'_>, +) -> (Vec, Option) { + let shape_error = || { + errors.fail( + HeaderStage::Returns, + "returns must be a type string or table column declaration", + ) + }; + let Some(HeaderValue::Map(entries)) = raw_returns else { + return (Vec::new(), Some(shape_error())); + }; + let [(HeaderValue::Text(key), table)] = entries.as_slice() else { + return (Vec::new(), Some(shape_error())); + }; + if key != TABLE_RETURN_KEY { + return (Vec::new(), Some(shape_error())); + } + match table { + HeaderValue::Map(entries) if !entries.is_empty() => { + let (columns, error) = named_types(entries); + let failure = + error.map(|error| errors.named_type(HeaderStage::Returns, error, "return column")); + (columns, failure) + } + _ => ( + Vec::new(), + Some(errors.fail( + HeaderStage::Returns, + "returns table must declare at least one column", + )), + ), + } +} diff --git a/crates/sqlbuild-attachments/src/functions/_helpers/mod.rs b/crates/sqlbuild-attachments/src/functions/_helpers/mod.rs index ca0050755f..eff19e8c44 100644 --- a/crates/sqlbuild-attachments/src/functions/_helpers/mod.rs +++ b/crates/sqlbuild-attachments/src/functions/_helpers/mod.rs @@ -1,2 +1,3 @@ +pub(crate) mod header_parsing; pub(crate) mod namespace; pub(crate) mod values; diff --git a/crates/sqlbuild-attachments/src/functions/_helpers/values.rs b/crates/sqlbuild-attachments/src/functions/_helpers/values.rs index ba5079c1f3..b34c598bf6 100644 --- a/crates/sqlbuild-attachments/src/functions/_helpers/values.rs +++ b/crates/sqlbuild-attachments/src/functions/_helpers/values.rs @@ -2,6 +2,7 @@ use sqlbuild_core::text::main::python_strip::python_strip; +use crate::functions::errors::NamedTypeError; use crate::functions::models::{HeaderValue, NamedType}; /// Python's `TABLE_FUNCTION_RETURN_KEYS`. @@ -17,38 +18,43 @@ pub(crate) fn stripped_text(value: Option<&HeaderValue>) -> Option { } } -/// Named types of a map of non-blank strings; None where Python raises. -pub(crate) fn named_types(entries: &[(HeaderValue, HeaderValue)]) -> Option> { +/// Named types of a map of non-blank strings, and Python's error after the ones before it. +pub(crate) fn named_types( + entries: &[(HeaderValue, HeaderValue)], +) -> (Vec, Option) { let mut named: Vec = Vec::with_capacity(entries.len()); for (key, value) in entries { - let HeaderValue::Text(raw_name) = key else { - return None; + let (HeaderValue::Text(raw_name), Some(name)) = (key, stripped_text(Some(key))) else { + return (named, Some(NamedTypeError::InvalidName)); + }; + let Some(type_text) = stripped_text(Some(value)) else { + return (named, Some(NamedTypeError::MissingType(raw_name.clone()))); }; named.push(NamedType { raw_name: raw_name.clone(), - name: stripped_text(Some(key))?, - type_text: stripped_text(Some(value))?, + name, + type_text, }); } - Some(named) + (named, None) } -/// Stripped entries of an optional list or tuple of non-blank strings. -pub(crate) fn stripped_list(value: Option<&HeaderValue>) -> Option> { +/// Stripped list entries; `Err(true)` for a non-sequence, `Err(false)` for a blank entry. +pub(crate) fn stripped_list(value: Option<&HeaderValue>) -> Result, bool> { match value { - None => Some(Vec::new()), + None => Ok(Vec::new()), Some(HeaderValue::Sequence(items)) => { let mut entries: Vec = Vec::with_capacity(items.len()); for item in items { - entries.push(stripped_text(Some(item))?); + entries.push(stripped_text(Some(item)).ok_or(false)?); } - Some(entries) + Ok(entries) } - Some(_) => None, + Some(_) => Err(true), } } -/// An optional string description, stripped. +/// An optional string description, stripped; None when it is not a string. pub(crate) fn description(value: Option<&HeaderValue>) -> Option> { match value { None => Some(None), diff --git a/crates/sqlbuild-attachments/src/functions/errors.rs b/crates/sqlbuild-attachments/src/functions/errors.rs new file mode 100644 index 0000000000..1979ccab12 --- /dev/null +++ b/crates/sqlbuild-attachments/src/functions/errors.rs @@ -0,0 +1,9 @@ +//! Why Python rejects a function header's named type map. + +/// Which check of a named type map fails first. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum NamedTypeError { + InvalidName, + /// The authored name of the entry without a type. + MissingType(String), +} diff --git a/crates/sqlbuild-attachments/src/functions/main/parse_function_header.rs b/crates/sqlbuild-attachments/src/functions/main/parse_function_header.rs index ce366025c0..a22cdfddfd 100644 --- a/crates/sqlbuild-attachments/src/functions/main/parse_function_header.rs +++ b/crates/sqlbuild-attachments/src/functions/main/parse_function_header.rs @@ -1,61 +1,20 @@ //! Python's SQL and Python function header parsing, before template expansion. -use crate::functions::_helpers::values::{ - TABLE_RETURN_KEY, description, lookup, named_types, stripped_list, stripped_text, -}; -use crate::functions::models::{ - FunctionHeader, FunctionLanguage, FunctionReturns, HeaderValue, NamedType, -}; +use crate::functions::_helpers::header_parsing::{HeaderErrors, parsed_header}; +use crate::functions::models::{FunctionHeader, FunctionLanguage, HeaderValue}; -/// The parsed header, or None wherever Python raises for its shape. +/// The parsed header of the function file at `relative_path`, with Python's first error. #[must_use] pub fn parse_function_header( header: &[(String, HeaderValue)], language: FunctionLanguage, -) -> Option { - let returns: FunctionReturns = match (language, lookup(header, "returns")?) { - (_, HeaderValue::Text(_)) => { - FunctionReturns::Type(stripped_text(lookup(header, "returns"))?) - } - (FunctionLanguage::Sql, HeaderValue::Map(entries)) => table_returns(entries)?, - _ => return None, - }; - let arguments: Vec = match lookup(header, "arguments") { - None => Vec::new(), - Some(HeaderValue::Map(entries)) => named_types(entries)?, - Some(_) => return None, - }; - let python: bool = language == FunctionLanguage::Python; - Some(FunctionHeader { - arguments, - returns, - runtime_version: if python { - Some(stripped_text(lookup(header, "runtime_version"))?) - } else { - None + relative_path: &str, +) -> FunctionHeader { + parsed_header( + header, + &HeaderErrors { + language, + relative_path, }, - entry_point: if python { - Some(stripped_text(lookup(header, "entry_point"))?) - } else { - None - }, - packages: if python { - stripped_list(lookup(header, "packages"))? - } else { - Vec::new() - }, - tags: stripped_list(lookup(header, "tags"))?, - description: description(lookup(header, "description"))?, - }) -} - -/// Python's `returns (table (...))`: exactly the `table` key with a non-empty column map. -fn table_returns(entries: &[(HeaderValue, HeaderValue)]) -> Option { - let [(HeaderValue::Text(key), HeaderValue::Map(columns))] = entries else { - return None; - }; - if key != TABLE_RETURN_KEY || columns.is_empty() { - return None; - } - Some(FunctionReturns::Table(named_types(columns)?)) + ) } diff --git a/crates/sqlbuild-attachments/src/functions/mod.rs b/crates/sqlbuild-attachments/src/functions/mod.rs index e6e5a52eb4..a2537f3e8e 100644 --- a/crates/sqlbuild-attachments/src/functions/mod.rs +++ b/crates/sqlbuild-attachments/src/functions/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod _helpers; +pub(crate) mod errors; pub mod main; pub mod models; diff --git a/crates/sqlbuild-attachments/src/functions/models.rs b/crates/sqlbuild-attachments/src/functions/models.rs index 4c9b4a7328..6a0dc12753 100644 --- a/crates/sqlbuild-attachments/src/functions/models.rs +++ b/crates/sqlbuild-attachments/src/functions/models.rs @@ -32,17 +32,40 @@ pub enum FunctionReturns { Table(Vec), } -/// Stripped header values; argument, return and column types are still unexpanded templates. +/// Where in Python's attachment a header error is raised, relative to template expansion. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum HeaderStage { + /// Before anything is expanded: a missing `returns`. + Start, + /// After expanding the arguments before the failing one. + Arguments, + /// After expanding every argument and the return columns before the failing one. + Returns, + /// After expanding a Python function's return type: runtime, entry point and packages. + PythonValues, + /// When the attachment builds the function: tags, then the description. + Metadata, +} + +/// Python's first header error and where it raises it. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HeaderFailure { + pub stage: HeaderStage, + pub message: String, +} + +/// Stripped header values with unexpanded types; after a failure, what Python parsed before it. #[derive(Clone, Debug, PartialEq, Eq)] pub struct FunctionHeader { pub arguments: Vec, - pub returns: FunctionReturns, + pub returns: Option, pub tags: Vec, pub description: Option, /// Python functions only: `runtime_version`, `entry_point` and `packages`. pub runtime_version: Option, pub entry_point: Option, pub packages: Vec, + pub failure: Option, } /// The project and target namespace a function resolves against, already expanded. diff --git a/crates/sqlbuild-attachments/src/functions/tests/helpers.rs b/crates/sqlbuild-attachments/src/functions/tests/helpers.rs index fcae3f8eac..8ffb3778bc 100644 --- a/crates/sqlbuild-attachments/src/functions/tests/helpers.rs +++ b/crates/sqlbuild-attachments/src/functions/tests/helpers.rs @@ -1,4 +1,7 @@ -use crate::functions::models::{FunctionLanguage, HeaderValue, NamedType, NamespaceInputs}; +use crate::functions::models::{ + FunctionHeader, FunctionLanguage, HeaderFailure, HeaderStage, HeaderValue, NamedType, + NamespaceInputs, +}; pub(super) fn text(value: &str) -> HeaderValue { HeaderValue::Text(value.to_owned()) @@ -45,3 +48,24 @@ pub(super) fn named(raw_name: &str, name: &str, type_text: &str) -> NamedType { type_text: type_text.to_owned(), } } + +/// A header Python parsed nothing else from. +pub(super) fn empty_header() -> FunctionHeader { + FunctionHeader { + arguments: Vec::new(), + returns: None, + tags: Vec::new(), + description: None, + runtime_version: None, + entry_point: None, + packages: Vec::new(), + failure: None, + } +} + +pub(super) fn failure(stage: HeaderStage, message: &str) -> Option { + Some(HeaderFailure { + stage, + message: message.to_owned(), + }) +} diff --git a/crates/sqlbuild-attachments/src/functions/tests/test_parse_function_header.rs b/crates/sqlbuild-attachments/src/functions/tests/test_parse_function_header.rs index 87091ffe2a..48e2d9152d 100644 --- a/crates/sqlbuild-attachments/src/functions/tests/test_parse_function_header.rs +++ b/crates/sqlbuild-attachments/src/functions/tests/test_parse_function_header.rs @@ -1,10 +1,12 @@ use crate::functions::main::parse_function_header::parse_function_header; -use crate::functions::models::{FunctionHeader, FunctionLanguage, FunctionReturns, HeaderValue}; -use crate::functions::tests::helpers::{header, map, named, text}; +use crate::functions::models::{ + FunctionHeader, FunctionLanguage, FunctionReturns, HeaderStage, HeaderValue, +}; +use crate::functions::tests::helpers::{empty_header, failure, header, map, named, text}; use crate::functions::tests::test_types::FunctionHeaderTestCase; #[test] -fn given_function_headers_when_parsing_then_python_values_or_deferrals_are_returned() { +fn given_function_headers_when_parsing_then_python_values_and_first_error_are_returned() { let test_cases = [ FunctionHeaderTestCase { description: "a SQL table function with stripped arguments, columns and tags", @@ -18,15 +20,13 @@ fn given_function_headers_when_parsing_then_python_values_or_deferrals_are_retur ("description", text(" Order rows. ")), ]), language: FunctionLanguage::Sql, - expected_header: Some(FunctionHeader { + expected_header: FunctionHeader { arguments: vec![named(" raw_status ", "raw_status", "STRING")], - returns: FunctionReturns::Table(vec![named("id", "id", "INTEGER")]), + returns: Some(FunctionReturns::Table(vec![named("id", "id", "INTEGER")])), tags: vec!["orders".to_owned()], description: Some("Order rows.".to_owned()), - runtime_version: None, - entry_point: None, - packages: Vec::new(), - }), + ..empty_header() + }, }, FunctionHeaderTestCase { description: "a Python function with its runtime, entry point and packages", @@ -37,33 +37,97 @@ fn given_function_headers_when_parsing_then_python_values_or_deferrals_are_retur ("packages", HeaderValue::Sequence(vec![text("numpy")])), ]), language: FunctionLanguage::Python, - expected_header: Some(FunctionHeader { - arguments: Vec::new(), - returns: FunctionReturns::Type("STRING".to_owned()), - tags: Vec::new(), - description: None, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Type("STRING".to_owned())), runtime_version: Some("3.12".to_owned()), entry_point: Some("label".to_owned()), packages: vec!["numpy".to_owned()], - }), + ..empty_header() + }, }, FunctionHeaderTestCase { - description: "a missing return type defers so Python raises", - header: header(&[("arguments", map(&[]))]), + description: "a missing return type is raised before the arguments", + header: header(&[("arguments", HeaderValue::Other)]), language: FunctionLanguage::Sql, - expected_header: None, + expected_header: FunctionHeader { + failure: failure( + HeaderStage::Start, + "SQL function file functions/f.sql must declare returns", + ), + ..empty_header() + }, }, FunctionHeaderTestCase { - description: "a blank argument type defers so Python raises", + description: "a blank argument type keeps the arguments before it", header: header(&[ - ("returns", text("STRING")), - ("arguments", map(&[("value", text(" "))])), + ("returns", HeaderValue::Other), + ( + "arguments", + map(&[("id", text("INTEGER")), (" value ", text(" "))]), + ), ]), language: FunctionLanguage::Sql, - expected_header: None, + expected_header: FunctionHeader { + arguments: vec![named("id", "id", "INTEGER")], + failure: failure( + HeaderStage::Arguments, + "SQL function file functions/f.sql argument ' value ' must declare a type", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "a blank SQL return type is a shape error after the arguments", + header: header(&[("returns", text(" ")), ("tags", HeaderValue::Other)]), + language: FunctionLanguage::Sql, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Table(Vec::new())), + failure: failure( + HeaderStage::Returns, + "SQL function file functions/f.sql returns must be a type string or table \ + column declaration", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "a return column without a name keeps the columns before it", + header: header(&[( + "returns", + map(&[( + "table", + HeaderValue::Map(vec![ + (text("id"), text("INTEGER")), + (HeaderValue::Other, text("TEXT")), + ]), + )]), + )]), + language: FunctionLanguage::Sql, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Table(vec![named("id", "id", "INTEGER")])), + failure: failure( + HeaderStage::Returns, + "SQL function file functions/f.sql has an invalid return column name", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "an empty table is Python's column error", + header: header(&[("returns", map(&[("table", map(&[]))]))]), + language: FunctionLanguage::Sql, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Table(Vec::new())), + failure: failure( + HeaderStage::Returns, + "SQL function file functions/f.sql returns table must declare at least one \ + column", + ), + ..empty_header() + }, }, FunctionHeaderTestCase { - description: "a Python function returning a table defers so Python raises", + description: "a Python function returning a table must declare returns", header: header(&[ ( "returns", @@ -73,13 +137,91 @@ fn given_function_headers_when_parsing_then_python_values_or_deferrals_are_retur ("entry_point", text("label")), ]), language: FunctionLanguage::Python, - expected_header: None, + expected_header: FunctionHeader { + failure: failure( + HeaderStage::Start, + "Python function file functions/f.sql must declare returns", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "a missing entry point is raised after the runtime version", + header: header(&[ + ("returns", text("STRING")), + ("runtime_version", text("3.12")), + ("packages", HeaderValue::Other), + ]), + language: FunctionLanguage::Python, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Type("STRING".to_owned())), + runtime_version: Some("3.12".to_owned()), + failure: failure( + HeaderStage::PythonValues, + "Python function file functions/f.sql must declare entry_point", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "a blank package entry and then the tags", + header: header(&[ + ("returns", text("STRING")), + ("runtime_version", text("3.12")), + ("entry_point", text("label")), + ("packages", HeaderValue::Sequence(vec![text("")])), + ]), + language: FunctionLanguage::Python, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Type("STRING".to_owned())), + runtime_version: Some("3.12".to_owned()), + entry_point: Some("label".to_owned()), + failure: failure( + HeaderStage::PythonValues, + "Python function file functions/f.sql packages entries must be non-empty \ + strings", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "tags that are not a list are raised before the description", + header: header(&[ + ("returns", text("STRING")), + ("tags", text("orders")), + ("description", HeaderValue::Other), + ]), + language: FunctionLanguage::Sql, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Type("STRING".to_owned())), + failure: failure( + HeaderStage::Metadata, + "SQL function file functions/f.sql tags must be a list", + ), + ..empty_header() + }, + }, + FunctionHeaderTestCase { + description: "a description that is not a string", + header: header(&[ + ("returns", text("STRING")), + ("description", HeaderValue::Other), + ]), + language: FunctionLanguage::Sql, + expected_header: FunctionHeader { + returns: Some(FunctionReturns::Type("STRING".to_owned())), + failure: failure( + HeaderStage::Metadata, + "SQL function file functions/f.sql description must be a string", + ), + ..empty_header() + }, }, ]; for test_case in test_cases { assert_eq!( - parse_function_header(&test_case.header, test_case.language), + parse_function_header(&test_case.header, test_case.language, "functions/f.sql"), test_case.expected_header, "{}", test_case.description diff --git a/crates/sqlbuild-attachments/src/functions/tests/test_types.rs b/crates/sqlbuild-attachments/src/functions/tests/test_types.rs index d66213f534..8580b91dc6 100644 --- a/crates/sqlbuild-attachments/src/functions/tests/test_types.rs +++ b/crates/sqlbuild-attachments/src/functions/tests/test_types.rs @@ -6,7 +6,7 @@ pub(super) struct FunctionHeaderTestCase { pub(super) description: &'static str, pub(super) header: Vec<(String, HeaderValue)>, pub(super) language: FunctionLanguage, - pub(super) expected_header: Option, + pub(super) expected_header: FunctionHeader, } pub(super) struct FunctionNamespaceTestCase { diff --git a/crates/sqlbuild-attachments/src/sql_lexing/main/mod.rs b/crates/sqlbuild-attachments/src/sql_lexing/main/mod.rs index 1133013f95..59699c9ad7 100644 --- a/crates/sqlbuild-attachments/src/sql_lexing/main/mod.rs +++ b/crates/sqlbuild-attachments/src/sql_lexing/main/mod.rs @@ -1 +1,3 @@ pub mod python_non_code_end; +pub mod unclosed_construct; +pub mod unclosed_message; diff --git a/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_construct.rs b/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_construct.rs new file mode 100644 index 0000000000..3910ba3b39 --- /dev/null +++ b/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_construct.rs @@ -0,0 +1,13 @@ +//! Which construct Python reports unclosed where its comment and quote scan raises. + +use sqlbuild_sqltext::sql_scan::models::Unclosed; + +/// A block comment when `index` opens one, otherwise quoted text. +#[must_use] +pub fn unclosed_construct(bytes: &[u8], index: usize) -> Unclosed { + if bytes[index..].starts_with(b"/*") { + Unclosed::BlockComment + } else { + Unclosed::Quote + } +} diff --git a/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_message.rs b/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_message.rs new file mode 100644 index 0000000000..1fde963134 --- /dev/null +++ b/crates/sqlbuild-attachments/src/sql_lexing/main/unclosed_message.rs @@ -0,0 +1,14 @@ +//! Python's message for a construct its scan reaches the end of input inside. + +use sqlbuild_sqltext::sql_scan::models::Unclosed; + +/// Python's `"{context} contains an unclosed quoted string"` or block comment message. +#[must_use] +pub fn unclosed_message(context: &str, construct: Unclosed) -> String { + let construct: &str = match construct { + Unclosed::BlockComment => "block comment", + Unclosed::Quote => "quoted string", + Unclosed::Parenthesis => "parenthesis", + }; + format!("{context} contains an unclosed {construct}") +} diff --git a/crates/sqlbuild-attachments/src/test_parameters/main/parameter_references.rs b/crates/sqlbuild-attachments/src/test_parameters/main/parameter_references.rs index 250a012692..e367c9fd7c 100644 --- a/crates/sqlbuild-attachments/src/test_parameters/main/parameter_references.rs +++ b/crates/sqlbuild-attachments/src/test_parameters/main/parameter_references.rs @@ -1,17 +1,23 @@ //! Python's `expand_test_parameters` scan, leaving value rendering to the caller. use crate::sql_lexing::main::python_non_code_end::python_non_code_end; +use crate::sql_lexing::main::unclosed_construct::unclosed_construct; +use crate::sql_lexing::main::unclosed_message::unclosed_message; use crate::sql_lexing::models::NonCode; use crate::test_parameters::_helpers::reference::{parameter_token_at, reference_at}; -use crate::test_parameters::models::ParameterReference; +use crate::test_parameters::models::{ParameterReference, ParameterScan}; -/// Every active reference in order, or None where Python raises (malformed, undeclared, unclosed). +/// Every active reference in order up to Python's first error for `owner`, the SQL test case. #[must_use] -pub fn parameter_references(sql: &str, declared: &[String]) -> Option> { +pub fn parameter_references(sql: &str, declared: &[String], owner: &str) -> ParameterScan { let bytes: &[u8] = sql.as_bytes(); let mut references: Vec = Vec::new(); let mut index: usize = 0; let mut characters: usize = 0; + let stop = |references: Vec, error: String| ParameterScan { + references, + error: Some(error), + }; while index < bytes.len() { match python_non_code_end(bytes, index) { NonCode::End(end) => { @@ -19,13 +25,24 @@ pub fn parameter_references(sql: &str, declared: &[String]) -> Option return None, + NonCode::Raises => { + let construct = unclosed_construct(bytes, index); + return stop(references, unclosed_message("SQL", construct)); + } NonCode::Code => {} } if parameter_token_at(bytes, index) { - let (name, end) = reference_at(sql, index)?; + let Some((name, end)) = reference_at(sql, index) else { + return stop( + references, + format!("{owner} has malformed @param reference; expected @param(\"name\")"), + ); + }; if !declared.iter().any(|candidate| candidate == name) { - return None; + return stop( + references, + format!("{owner} references undeclared parameter '{name}'"), + ); } let length: usize = sql[index..end].chars().count(); references.push(ParameterReference { @@ -41,5 +58,8 @@ pub fn parameter_references(sql: &str, declared: &[String]) -> Option, + pub error: Option, +} diff --git a/crates/sqlbuild-attachments/src/test_parameters/tests/helpers.rs b/crates/sqlbuild-attachments/src/test_parameters/tests/helpers.rs index 9d72f6fdf3..c2ba812580 100644 --- a/crates/sqlbuild-attachments/src/test_parameters/tests/helpers.rs +++ b/crates/sqlbuild-attachments/src/test_parameters/tests/helpers.rs @@ -1,12 +1,10 @@ -use crate::test_parameters::models::ParameterReference; +use crate::test_parameters::models::ParameterScan; -/// References as `(start, end, name)` tuples. -pub(super) fn spans( - references: Option>, -) -> Option> { +/// References as `(start, end, name)` tuples, and the error that stops the scan. +pub(super) fn spans(scan: ParameterScan) -> (Vec<(usize, usize, String)>, Option) { let mut spans: Vec<(usize, usize, String)> = Vec::new(); - for reference in references? { + for reference in scan.references { spans.push((reference.start, reference.end, reference.name)); } - Some(spans) + (spans, scan.error) } diff --git a/crates/sqlbuild-attachments/src/test_parameters/tests/test_parameter_references.rs b/crates/sqlbuild-attachments/src/test_parameters/tests/test_parameter_references.rs index a084c7d79e..8103971377 100644 --- a/crates/sqlbuild-attachments/src/test_parameters/tests/test_parameter_references.rs +++ b/crates/sqlbuild-attachments/src/test_parameters/tests/test_parameter_references.rs @@ -2,46 +2,67 @@ use crate::test_parameters::main::parameter_references::parameter_references; use crate::test_parameters::tests::helpers::spans; use crate::test_parameters::tests::test_types::ParameterReferenceTestCase; +const OWNER: &str = "SQL test 'orders' case 'basic' in 'tests/orders.sql'"; + #[test] -fn given_test_sql_when_scanning_parameters_then_python_references_are_returned() { +fn given_test_sql_when_scanning_parameters_then_python_references_and_error_are_returned() { let test_cases = [ ParameterReferenceTestCase { description: "references by code point, quotes and comments skipped", sql: "SELECT 'é@param(\"x\")', @param ( \"region\" ) -- @param(\"x\")\n, @param(\"limit\")", - expected_references: Some(vec![(23, 42, "region"), (60, 75, "limit")]), + expected_references: vec![(23, 42, "region"), (60, 75, "limit")], + expected_error: None, }, ParameterReferenceTestCase { description: "a longer identifier is not a reference", sql: "SELECT @params, @param_x", - expected_references: Some(vec![]), + expected_references: vec![], + expected_error: None, + }, + ParameterReferenceTestCase { + description: "a malformed reference stops after the references before it", + sql: "SELECT @param(\"region\"), @param(region), @param(\"missing\")", + expected_references: vec![(7, 23, "region")], + expected_error: Some( + "SQL test 'orders' case 'basic' in 'tests/orders.sql' has malformed @param \ + reference; expected @param(\"name\")", + ), }, ParameterReferenceTestCase { - description: "a malformed reference defers so Python raises", - sql: "SELECT @param(region)", - expected_references: None, + description: "an undeclared parameter names itself", + sql: "SELECT @param(\"missing\"), 'open", + expected_references: vec![], + expected_error: Some( + "SQL test 'orders' case 'basic' in 'tests/orders.sql' references undeclared \ + parameter 'missing'", + ), }, ParameterReferenceTestCase { - description: "an undeclared parameter defers so Python raises", - sql: "SELECT @param(\"missing\")", - expected_references: None, + description: "an unclosed quote is Python's plain SQL quote error", + sql: "SELECT @param(\"limit\"), 'open", + expected_references: vec![(7, 22, "limit")], + expected_error: Some("SQL contains an unclosed quoted string"), }, ParameterReferenceTestCase { - description: "an unclosed quote defers so Python raises", - sql: "SELECT 'open", - expected_references: None, + description: "an unclosed block comment is Python's plain SQL comment error", + sql: "SELECT 1 /* open", + expected_references: vec![], + expected_error: Some("SQL contains an unclosed block comment"), }, ]; for test_case in test_cases { let declared: Vec = vec!["region".to_owned(), "limit".to_owned()]; assert_eq!( - spans(parameter_references(test_case.sql, &declared)), - test_case.expected_references.map(|references| { - references + spans(parameter_references(test_case.sql, &declared, OWNER)), + ( + test_case + .expected_references .into_iter() .map(|(start, end, name)| (start, end, name.to_owned())) - .collect() - }), + .collect(), + test_case.expected_error.map(str::to_owned) + ), "{}", test_case.description ); diff --git a/crates/sqlbuild-attachments/src/test_parameters/tests/test_types.rs b/crates/sqlbuild-attachments/src/test_parameters/tests/test_types.rs index c75aa925c5..68cea07fc8 100644 --- a/crates/sqlbuild-attachments/src/test_parameters/tests/test_types.rs +++ b/crates/sqlbuild-attachments/src/test_parameters/tests/test_types.rs @@ -1,6 +1,7 @@ pub(super) struct ParameterReferenceTestCase { pub(super) description: &'static str, pub(super) sql: &'static str, - /// `(start, end, name)` by code points, or None where Python raises. - pub(super) expected_references: Option>, + /// `(start, end, name)` by code points, up to Python's first error. + pub(super) expected_references: Vec<(usize, usize, &'static str)>, + pub(super) expected_error: Option<&'static str>, } diff --git a/crates/sqlbuild-model-config/src/templates/_helpers/evaluation.rs b/crates/sqlbuild-model-config/src/templates/_helpers/evaluation.rs index 0f8a42afbb..8a581660e9 100644 --- a/crates/sqlbuild-model-config/src/templates/_helpers/evaluation.rs +++ b/crates/sqlbuild-model-config/src/templates/_helpers/evaluation.rs @@ -1,5 +1,6 @@ //! The Python template evaluator: references, `if`, `eq`, `ne` and `coalesce`. +use crate::templates::errors::TemplateError; use crate::templates::models::{ ContextValue, Expression, Scalar, TemplateFailure, TemplateOptions, }; @@ -51,15 +52,19 @@ fn evaluate_reference( _ => {} } let Some((namespace, name)) = reference.split_once(':') else { - return host.variable(reference)?.ok_or(TemplateFailure::Missing); + return host.variable(reference)?.ok_or_else(|| { + TemplateFailure::Missing(TemplateError::UnknownVariable(reference.to_owned())) + }); }; match namespace { - ENVIRONMENT_NAMESPACE => host.environment(name)?.ok_or(TemplateFailure::Missing), + ENVIRONMENT_NAMESPACE => host.environment(name)?.ok_or_else(|| { + TemplateFailure::Missing(TemplateError::MissingEnvironmentVariable(name.to_owned())) + }), CONTEXT_NAMESPACE if !options.allow_context => { if options.preserve_context_tokens { host.text(&format!("${{{reference}}}")) } else { - Err(TemplateFailure::Invalid) + Err(TemplateFailure::Invalid(TemplateError::ContextNotAllowed)) } } CONTEXT_NAMESPACE => match host.context(name)? { @@ -67,9 +72,16 @@ fn evaluate_reference( ContextValue::Unknown if options.preserve_unknown_context => { host.text(&format!("${{{CONTEXT_NAMESPACE}:{name}}}")) } - ContextValue::Unknown | ContextValue::Unavailable => Err(TemplateFailure::Missing), + ContextValue::Unknown => Err(TemplateFailure::Missing( + TemplateError::UnknownContextKey(name.to_owned()), + )), + ContextValue::Unavailable => Err(TemplateFailure::Missing( + TemplateError::UnavailableContextKey(name.to_owned()), + )), }, - _ => Err(TemplateFailure::Invalid), + _ => Err(TemplateFailure::Invalid( + TemplateError::UnsupportedNamespace(namespace.to_owned()), + )), } } @@ -98,16 +110,34 @@ fn evaluate_function( for argument in arguments { match evaluate(host, argument, options) { Ok(value) if truthiness(host, &value)? => return Ok(value), - Ok(_) | Err(TemplateFailure::Missing) => {} + Ok(_) | Err(TemplateFailure::Missing(_)) => {} Err(failure) => return Err(failure), } } evaluate(host, last, options) } - _ => Err(TemplateFailure::Invalid), + (IF_FUNCTION, _) => Err(argument_count(IF_FUNCTION, 3)), + (EQ_FUNCTION | NE_FUNCTION, _) => Err(argument_count( + if name == EQ_FUNCTION { + EQ_FUNCTION + } else { + NE_FUNCTION + }, + 2, + )), + (COALESCE_FUNCTION, []) => Err(TemplateFailure::Invalid( + TemplateError::CoalesceWithoutArguments, + )), + _ => Err(TemplateFailure::Invalid( + TemplateError::UnsupportedFunction(name.to_owned()), + )), } } +fn argument_count(function: &'static str, count: usize) -> TemplateFailure { + TemplateFailure::Invalid(TemplateError::ArgumentCount(function, count)) +} + fn comparison_text(host: &H, value: &H::Value) -> Result { match host.scalar(value).ok_or(TemplateFailure::Unsupported)? { Scalar::Null => Ok(String::new()), diff --git a/crates/sqlbuild-model-config/src/templates/_helpers/expressions.rs b/crates/sqlbuild-model-config/src/templates/_helpers/expressions.rs index 2ade14256b..8847d7ba45 100644 --- a/crates/sqlbuild-model-config/src/templates/_helpers/expressions.rs +++ b/crates/sqlbuild-model-config/src/templates/_helpers/expressions.rs @@ -1,6 +1,7 @@ //! The Python template expression parser. -use crate::templates::_helpers::tokens::{Token, tokenize}; +use crate::templates::_helpers::tokens::{Token, TokenKind, tokenize}; +use crate::templates::errors::TemplateError; use crate::templates::models::{Expression, TemplateFailure}; /// Parse one `${...}` body into an expression. @@ -8,10 +9,11 @@ pub(crate) fn parse_expression(body: &str) -> Result = tokenize(body)?; let mut parser = Parser { tokens, index: 0 }; let expression = parser.expression()?; - if parser.peek() == &Token::End { + let token: &Token = parser.peek(); + if token.kind == TokenKind::End { Ok(expression) } else { - Err(TemplateFailure::Invalid) + Err(unexpected(token)) } } @@ -20,19 +22,20 @@ struct Parser { index: usize, } +fn unexpected(token: &Token) -> TemplateFailure { + TemplateFailure::Invalid(TemplateError::UnexpectedToken( + token.value(), + token.position, + )) +} + impl Parser { fn peek(&self) -> &Token { &self.tokens[self.index] } - fn advance(&mut self) -> Token { - let token = self.tokens[self.index].clone(); - self.index += 1; - token - } - fn match_symbol(&mut self, symbol: char) -> bool { - if self.peek() == &Token::Symbol(symbol) { + if self.peek().kind == TokenKind::Symbol(symbol) { self.index += 1; true } else { @@ -41,28 +44,27 @@ impl Parser { } fn expression(&mut self) -> Result { - match self.peek() { - Token::Text(_) => match self.advance() { - Token::Text(text) => Ok(Expression::Text(text)), - _ => Err(TemplateFailure::Invalid), - }, - Token::Word(_) => { - let Token::Word(word) = self.advance() else { - return Err(TemplateFailure::Invalid); - }; + let token: Token = self.peek().clone(); + match token.kind { + TokenKind::Text(text) => { + self.index += 1; + Ok(Expression::Text(text)) + } + TokenKind::Word(word) => { + self.index += 1; if self.match_symbol('(') { self.function_call(word) } else { Ok(Expression::Reference(word)) } } - Token::Symbol(_) | Token::End => Err(TemplateFailure::Invalid), + TokenKind::Symbol(_) | TokenKind::End => Err(unexpected(&token)), } } fn function_call(&mut self, name: String) -> Result { let mut arguments: Vec = Vec::new(); - while self.peek() != &Token::Symbol(')') { + while self.peek().kind != TokenKind::Symbol(')') { arguments.push(self.expression()?); if !self.match_symbol(',') { break; @@ -71,7 +73,10 @@ impl Parser { if self.match_symbol(')') { Ok(Expression::Function { name, arguments }) } else { - Err(TemplateFailure::Invalid) + Err(TemplateFailure::Invalid(TemplateError::ExpectedSymbol( + ')', + self.peek().position, + ))) } } } diff --git a/crates/sqlbuild-model-config/src/templates/_helpers/tokens.rs b/crates/sqlbuild-model-config/src/templates/_helpers/tokens.rs index 11565d6ea2..37ac76555e 100644 --- a/crates/sqlbuild-model-config/src/templates/_helpers/tokens.rs +++ b/crates/sqlbuild-model-config/src/templates/_helpers/tokens.rs @@ -1,72 +1,121 @@ //! The Python template tokenizer, deferring wherever Python's `str.isspace` would decide. +use crate::templates::errors::TemplateError; use crate::templates::models::TemplateFailure; #[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) enum Token { +pub(crate) enum TokenKind { Word(String), Text(String), Symbol(char), End, } +/// One token and the character position it starts at; tokenized text is ASCII. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Token { + pub(crate) kind: TokenKind, + pub(crate) position: usize, +} + +impl Token { + /// Python's `token.value`: the word, the unquoted text, the symbol or nothing at the end. + pub(crate) fn value(&self) -> String { + match &self.kind { + TokenKind::Word(text) | TokenKind::Text(text) => text.clone(), + TokenKind::Symbol(symbol) => symbol.to_string(), + TokenKind::End => String::new(), + } + } +} + const SYMBOLS: [char; 3] = ['(', ')', ',']; const ESCAPE: char = '\\'; -/// Split one template expression into tokens. +/// Split one template expression into tokens; positions count characters, as Python's do. pub(crate) fn tokenize(expression: &str) -> Result, TemplateFailure> { + let characters: Vec = expression.chars().collect(); let mut tokens: Vec = Vec::new(); - let mut characters = expression.chars().peekable(); - while let Some(&character) = characters.peek() { + let mut index: usize = 0; + while let Some(&character) = characters.get(index) { if !character.is_ascii() { return Err(TemplateFailure::Unsupported); } + let position: usize = index; if is_python_space(character) { - characters.next(); + index += 1; } else if SYMBOLS.contains(&character) { - tokens.push(Token::Symbol(character)); - characters.next(); - } else if is_quote(character) { - characters.next(); - tokens.push(Token::Text(quoted_text(&mut characters, character)?)); + tokens.push(Token { + kind: TokenKind::Symbol(character), + position, + }); + index += 1; + } else if let Some(quote_name) = quote_name(character) { + let (text, end) = quoted_text(&characters, index, quote_name)?; + tokens.push(Token { + kind: TokenKind::Text(text), + position, + }); + index = end; } else { let mut word = String::new(); - while let Some(&next) = characters.peek() { + while let Some(&next) = characters.get(index) { if !next.is_ascii() { return Err(TemplateFailure::Unsupported); } - if is_python_space(next) || SYMBOLS.contains(&next) || is_quote(next) { + if is_python_space(next) || SYMBOLS.contains(&next) || quote_name(next).is_some() { break; } word.push(next); - characters.next(); + index += 1; } - tokens.push(Token::Word(word)); + tokens.push(Token { + kind: TokenKind::Word(word), + position, + }); } } - tokens.push(Token::End); + tokens.push(Token { + kind: TokenKind::End, + position: characters.len(), + }); Ok(tokens) } +/// The text quoted at `start` and the index after its closing quote. fn quoted_text( - characters: &mut impl Iterator, - quote: char, -) -> Result { + characters: &[char], + start: usize, + quote_name: &'static str, +) -> Result<(String, usize), TemplateFailure> { + let quote: char = characters[start]; let mut text = String::new(); - while let Some(character) = characters.next() { + let mut index: usize = start + 1; + while let Some(&character) = characters.get(index) { if character == ESCAPE { - text.push(characters.next().ok_or(TemplateFailure::Invalid)?); + let escaped: char = *characters.get(index + 1).ok_or(TemplateFailure::Invalid( + TemplateError::UnterminatedEscape(index), + ))?; + text.push(escaped); + index += 2; } else if character == quote { - return Ok(text); + return Ok((text, index + 1)); } else { text.push(character); + index += 1; } } - Err(TemplateFailure::Invalid) + Err(TemplateFailure::Invalid(TemplateError::UnterminatedString( + quote_name, start, + ))) } -fn is_quote(character: char) -> bool { - matches!(character, '\'' | '"') +fn quote_name(character: char) -> Option<&'static str> { + match character { + '\'' => Some("single"), + '"' => Some("double"), + _ => None, + } } /// ASCII characters Python's `str.isspace` accepts, including the information separators. diff --git a/crates/sqlbuild-model-config/src/templates/errors.rs b/crates/sqlbuild-model-config/src/templates/errors.rs new file mode 100644 index 0000000000..2f96b3399c --- /dev/null +++ b/crates/sqlbuild-model-config/src/templates/errors.rs @@ -0,0 +1,68 @@ +//! The errors Python's template resolver raises. + +/// The error Python's template resolver raises. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum TemplateError { + UnknownVariable(String), + MissingEnvironmentVariable(String), + UnknownContextKey(String), + UnavailableContextKey(String), + ContextNotAllowed, + UnsupportedNamespace(String), + /// `if`, `eq` or `ne` with the wrong number of arguments, and the number it expects. + ArgumentCount(&'static str, usize), + CoalesceWithoutArguments, + UnsupportedFunction(String), + /// A token's text and character position where the parser expected another. + UnexpectedToken(String, usize), + ExpectedSymbol(char, usize), + UnterminatedEscape(usize), + /// The quote's name, `single` or `double`, and the position it opens at. + UnterminatedString(&'static str, usize), +} + +impl TemplateError { + /// Python's message for templates in `context_label`; parse errors do not name it. + #[must_use] + pub fn message(&self, context_label: &str) -> String { + match self { + Self::UnknownVariable(name) => { + format!("{context_label} references unknown variable '{name}'") + } + Self::MissingEnvironmentVariable(name) => { + format!("{context_label} references missing ENV variable '{name}'") + } + Self::UnknownContextKey(name) => { + format!("{context_label} references unknown CTX key '{name}'") + } + Self::UnavailableContextKey(name) => { + format!("{context_label} references CTX key '{name}' but no value is available") + } + Self::ContextNotAllowed => format!("{context_label} does not allow CTX templates"), + Self::UnsupportedNamespace(namespace) => { + format!("{context_label} references unsupported template namespace '{namespace}'") + } + Self::ArgumentCount(function, count) => { + format!("{context_label} {function}(...) expects {count} arguments") + } + Self::CoalesceWithoutArguments => { + format!("{context_label} coalesce(...) expects at least 1 argument") + } + Self::UnsupportedFunction(name) => { + format!("{context_label} references unsupported template function '{name}'") + } + Self::UnexpectedToken(token, position) => format!( + "template expression contains unexpected token '{token}' at position {position}" + ), + Self::ExpectedSymbol(symbol, position) => { + format!("template expression expected '{symbol}' at position {position}") + } + Self::UnterminatedEscape(position) => { + format!("template expression has unterminated escape at position {position}") + } + Self::UnterminatedString(quote, position) => format!( + "template expression has unterminated {quote}-quoted string at position {position}" + ), + } + } +} diff --git a/crates/sqlbuild-model-config/src/templates/mod.rs b/crates/sqlbuild-model-config/src/templates/mod.rs index 91fb358a0f..fbe1b3ff91 100644 --- a/crates/sqlbuild-model-config/src/templates/mod.rs +++ b/crates/sqlbuild-model-config/src/templates/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod _helpers; +pub mod errors; pub mod main; pub mod models; pub mod types; diff --git a/crates/sqlbuild-model-config/src/templates/models.rs b/crates/sqlbuild-model-config/src/templates/models.rs index 4d675be772..06c75d1aff 100644 --- a/crates/sqlbuild-model-config/src/templates/models.rs +++ b/crates/sqlbuild-model-config/src/templates/models.rs @@ -1,5 +1,7 @@ //! Template expressions, scalar values and outcomes. +use crate::templates::errors::TemplateError; + /// A parsed `${...}` expression. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Expression { @@ -29,13 +31,13 @@ pub enum ContextValue { Value(V), } -/// Why native expansion stops; Python then expands the value itself. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +/// Why native expansion stops. +#[derive(Clone, Debug, PartialEq, Eq)] pub enum TemplateFailure { /// A missing variable, environment variable or context value, which `coalesce` skips. - Missing, - /// Python raises another error for this template. - Invalid, + Missing(TemplateError), + /// Python raises this error for the template. + Invalid(TemplateError), /// Python expands this template with rules the native expansion does not reproduce. Unsupported, } diff --git a/crates/sqlbuild-model-config/src/templates/tests/test_expand_template_string.rs b/crates/sqlbuild-model-config/src/templates/tests/test_expand_template_string.rs index c50b3a8551..8aef73d97c 100644 --- a/crates/sqlbuild-model-config/src/templates/tests/test_expand_template_string.rs +++ b/crates/sqlbuild-model-config/src/templates/tests/test_expand_template_string.rs @@ -1,5 +1,6 @@ use std::cell::RefCell; +use crate::templates::errors::TemplateError; use crate::templates::main::expand_template_string::expand_template_string; use crate::templates::models::{StringExpansion, TemplateFailure, TemplateOptions}; use crate::templates::tests::helpers::{text, value}; @@ -59,19 +60,89 @@ fn given_template_strings_when_expanding_then_values_and_reads_match_python() { TemplateTestCase { description: "a context key without a value is missing", text: "${CTX:model.schema}", - expected: Err(TemplateFailure::Missing), + expected: Err(TemplateFailure::Missing( + TemplateError::UnavailableContextKey("model.schema".to_owned()), + )), expected_reads: &["CTX:model.schema"], }, TemplateTestCase { - description: "parse errors are invalid", + description: "a missing variable after a coalesce names itself", + text: "${coalesce(missing, '')}_${ENV:UNSET}", + expected: Err(TemplateFailure::Missing( + TemplateError::MissingEnvironmentVariable("UNSET".to_owned()), + )), + expected_reads: &["ENV:UNSET"], + }, + TemplateTestCase { + description: "an unexpected token reports its unquoted value and position", text: "${coalesce(a,,b)}", - expected: Err(TemplateFailure::Invalid), + expected: Err(TemplateFailure::Invalid(TemplateError::UnexpectedToken( + ",".to_owned(), + 11, + ))), + expected_reads: &[], + }, + TemplateTestCase { + description: "a trailing token after the expression", + text: "${env 'x y'}", + expected: Err(TemplateFailure::Invalid(TemplateError::UnexpectedToken( + "x y".to_owned(), + 4, + ))), + expected_reads: &[], + }, + TemplateTestCase { + description: "an unclosed call expects its parenthesis at the end", + text: "${if(flag, env", + expected: Ok(StringExpansion::Unchanged), + expected_reads: &[], + }, + TemplateTestCase { + description: "an unclosed call inside a template expects its parenthesis", + text: "${eq(flag env)}", + expected: Err(TemplateFailure::Invalid(TemplateError::ExpectedSymbol( + ')', 8, + ))), expected_reads: &[], }, TemplateTestCase { - description: "an unsupported function or namespace is invalid", + description: "an unterminated string reports where it opens", + text: "${'abc}", + expected: Err(TemplateFailure::Invalid(TemplateError::UnterminatedString( + "single", 0, + ))), + expected_reads: &[], + }, + TemplateTestCase { + description: "an escape at the end of the expression", + text: "${\"ab\\}", + expected: Err(TemplateFailure::Invalid(TemplateError::UnterminatedEscape( + 3, + ))), + expected_reads: &[], + }, + TemplateTestCase { + description: "argument counts are checked before arguments are evaluated", + text: "${ne(missing)}", + expected: Err(TemplateFailure::Invalid(TemplateError::ArgumentCount( + "ne", 2, + ))), + expected_reads: &[], + }, + TemplateTestCase { + description: "an unsupported function is invalid", text: "${upper(env)}", - expected: Err(TemplateFailure::Invalid), + expected: Err(TemplateFailure::Invalid( + TemplateError::UnsupportedFunction("upper".to_owned()), + )), + expected_reads: &[], + }, + TemplateTestCase { + description: "an unsupported namespace is invalid", + text: "${VAR:env}", + expected: Err(TemplateFailure::Invalid( + TemplateError::UnsupportedNamespace("VAR".to_owned()), + )), expected_reads: &[], }, TemplateTestCase { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/attachments/argument_values.rs b/crates/sqlbuild-python/src/bindings/_helpers/attachments/argument_values.rs index 196d0b0f10..4f685fb0cc 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/attachments/argument_values.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/attachments/argument_values.rs @@ -1,7 +1,9 @@ //! Python audit and hook argument values read as plain argument values. use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult}; -use pyo3::types::{PyBool, PyDict, PyDictMethods, PyFloat, PyInt, PyList, PyListMethods, PyString}; +use pyo3::types::{ + PyBool, PyDict, PyDictMethods, PyFloat, PyInt, PyList, PyListMethods, PyString, PyTuple, +}; use sqlbuild_attachments::audits::models::ArgumentValue; /// Read a `dict[str, object]` in insertion order, or None for values Python must judge. @@ -21,7 +23,7 @@ pub(crate) fn argument_pairs( Ok(Some(pairs)) } -/// One value: None, bool, int and finite float (by `str()`), str, or a list of those. +/// One plain or opaque value; tuples, list subclasses and non-finite floats defer to Python. fn argument_value(value: &Bound<'_, PyAny>) -> PyResult> { if value.is_none() { return Ok(Some(ArgumentValue::Null)); @@ -40,11 +42,18 @@ fn argument_value(value: &Bound<'_, PyAny>) -> PyResult> { return Ok(Some(ArgumentValue::Number(value.str()?.extract()?))); } if value.is_instance_of::() { - return Ok(Some(ArgumentValue::Text(value.extract()?))); - } - if !value.is_exact_instance_of::() { + if let Ok(text) = value.extract::() { + return Ok(Some(ArgumentValue::Text(text))); + } return Ok(None); } + if value.is_instance_of::() || value.is_instance_of::() { + if !value.is_exact_instance_of::() { + return Ok(None); + } + } else { + return Ok(Some(ArgumentValue::Opaque)); + } let list: &Bound<'_, PyList> = value.downcast_exact::()?; let mut items: Vec = Vec::with_capacity(list.len()); for item in list.iter() { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/attachments/audit_rendering.rs b/crates/sqlbuild-python/src/bindings/_helpers/attachments/audit_rendering.rs index d16db09cbf..68b16a33df 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/attachments/audit_rendering.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/attachments/audit_rendering.rs @@ -4,13 +4,20 @@ use pyo3::prelude::{Bound, PyModule, PyModuleMethods, PyResult}; use pyo3::types::PyDict; use pyo3::{FromPyObject, pyfunction, wrap_pyfunction}; use sqlbuild_attachments::audits::main::render_attached_audit::render_attached_audit; -use sqlbuild_attachments::audits::models::{AuditAttachment, PolicySource, RenderedAudit}; +use sqlbuild_attachments::audits::models::{AuditAttachment, AuditRendering, PolicySource}; use crate::bindings::_helpers::attachments::argument_values::argument_pairs; use crate::bindings::_helpers::boundary::panics::compiler_guard; -/// `(sql body, evidence sql, severity, run scope source)`. -type RenderedRow = (String, Option, &'static str, &'static str); +/// `(render error, sql body, evidence sql, severity, run scope source, policy error)`. +type RenderedRow = ( + Option, + String, + Option, + &'static str, + &'static str, + Option, +); /// Authored policies of one attachment: mode and threshold flags, severities and run scopes. #[derive(FromPyObject)] @@ -26,9 +33,10 @@ struct AuditPolicies { default_run_scope: Option, } -/// Render `(sql body, evidence)` with `(implicit, explicit)` arguments, or `None` for Python. +/// Render one `(owner, audit)` attachment with Python's errors, or `None` for Python. #[pyfunction] fn render_attached_generic_audit( + labels: (String, String), sql: (String, Option), arguments: (Bound<'_, PyDict>, Bound<'_, PyDict>), policies: AuditPolicies, @@ -40,6 +48,8 @@ fn render_attached_generic_audit( return Ok(None); }; let attachment: AuditAttachment = AuditAttachment { + owner_label: labels.0, + definition_name: labels.1, sql_body: sql.0, evidence_sql: sql.1, implicit_arguments: implicit, @@ -57,20 +67,36 @@ fn render_attached_generic_audit( }) } -fn rendered_row(rendered: RenderedAudit) -> RenderedRow { - let source: &'static str = match rendered.run_scope_source { - PolicySource::Instance => "instance", - PolicySource::Default => "default", - PolicySource::Fallback => "fallback", +fn rendered_row(rendering: AuditRendering) -> RenderedRow { + let rendered = match rendering { + AuditRendering::Failed(message) => { + return (Some(message), String::new(), None, "", "", None); + } + AuditRendering::Rendered(rendered) => rendered, }; + let (severity, source, policy_error): (&'static str, &'static str, Option) = + match rendered.policies { + Ok((severity, source)) => (severity, source_name(source), None), + Err(message) => ("", "", Some(message)), + }; ( + None, rendered.sql_body, rendered.evidence_sql, - rendered.severity, + severity, source, + policy_error, ) } +fn source_name(source: PolicySource) -> &'static str { + match source { + PolicySource::Instance => "instance", + PolicySource::Default => "default", + PolicySource::Fallback => "fallback", + } +} + pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(render_attached_generic_audit, module)?)?; Ok(()) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/attachments/cursor_intrinsics.rs b/crates/sqlbuild-python/src/bindings/_helpers/attachments/cursor_intrinsics.rs index dd7199762c..74bbf63c27 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/attachments/cursor_intrinsics.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/attachments/cursor_intrinsics.rs @@ -7,10 +7,20 @@ use sqlbuild_attachments::cursor_intrinsics::models::IntrinsicCheck; use crate::bindings::_helpers::boundary::panics::compiler_guard; -/// Return whether Python accepts `sql` as free of cursor intrinsics; `False` defers to Python. +/// `(True, None)` if Python accepts `sql`, `(False, error)` if it rejects it, else `(False, None)`. #[pyfunction] -fn sql_free_of_cursor_intrinsics(sql: &str, reserved_markers: Vec) -> PyResult { - compiler_guard(|| Ok(intrinsic_free(sql, &reserved_markers) == IntrinsicCheck::Free)) +fn sql_free_of_cursor_intrinsics( + sql: &str, + reserved_markers: Vec, + context: &str, +) -> PyResult<(bool, Option)> { + compiler_guard(|| { + Ok(match intrinsic_free(sql, &reserved_markers, context) { + IntrinsicCheck::Free => (true, None), + IntrinsicCheck::Rejected(message) => (false, Some(message)), + IntrinsicCheck::Deferred => (false, None), + }) + }) } pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/attachments/function_headers.rs b/crates/sqlbuild-python/src/bindings/_helpers/attachments/function_headers.rs index faa17d6faf..c542695ecb 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/attachments/function_headers.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/attachments/function_headers.rs @@ -8,15 +8,15 @@ use pyo3::{FromPyObject, pyfunction, wrap_pyfunction}; use sqlbuild_attachments::functions::main::parse_function_header::parse_function_header; use sqlbuild_attachments::functions::main::resolve_function_namespace::resolve_function_namespace; use sqlbuild_attachments::functions::models::{ - FunctionHeader, FunctionLanguage, FunctionNamespace, FunctionReturns, HeaderValue, NamedType, - NamespaceInputs, + FunctionHeader, FunctionLanguage, FunctionNamespace, FunctionReturns, HeaderStage, HeaderValue, + NamedType, NamespaceInputs, }; use crate::bindings::_helpers::boundary::panics::compiler_guard; /// `(authored name, name, type)` rows. type Pairs = Vec<(String, String, String)>; -/// Arguments, scalar type, table columns, tags, description, runtime, entry point, packages. +/// Parsed header fields in Python's order, then its first error as `(stage, message)`. type HeaderRow = ( Pairs, Option, @@ -26,6 +26,7 @@ type HeaderRow = ( Option, Option, Vec, + Option<(&'static str, String)>, ); type NamespaceRow = [Option; 8]; @@ -43,11 +44,12 @@ struct NamespaceInput { inherit_default_namespace: bool, } -/// Parse a function header, or return `None` where Python raises for its shape. +/// Parse one function file's header with Python's first error, or `None` for unreadable text. #[pyfunction] fn parse_function_header_values( header_values: Bound<'_, PyDict>, python: bool, + relative_path: &str, ) -> PyResult> { compiler_guard(|| { let mut header: Vec<(String, HeaderValue)> = Vec::with_capacity(header_values.len()); @@ -55,17 +57,21 @@ fn parse_function_header_values( if value.is_none() { continue; } - let Some(name) = text_value(&key) else { + let (Some(name), Some(value)) = (text_value(&key), header_value(&value)?) else { return Ok(None); }; - header.push((name, header_value(&value)?)); + header.push((name, value)); } let language: FunctionLanguage = if python { FunctionLanguage::Python } else { FunctionLanguage::Sql }; - Ok(parse_function_header(&header, language).map(header_row)) + Ok(Some(header_row(parse_function_header( + &header, + language, + relative_path, + )))) }) } @@ -100,32 +106,36 @@ fn resolve_function_namespace_values(inputs: NamespaceInput) -> PyResult) -> PyResult { +/// One header value; `None` when a string inside it is text Rust cannot hold. +fn header_value(value: &Bound<'_, PyAny>) -> PyResult> { if value.is_instance_of::() { - return Ok(text_value(value).map_or(HeaderValue::Other, HeaderValue::Text)); + return Ok(text_value(value).map(HeaderValue::Text)); } if let Ok(mapping) = value.downcast::() { let mut entries: Vec<(HeaderValue, HeaderValue)> = Vec::with_capacity(mapping.len()); for (key, item) in mapping.iter() { - entries.push((header_value(&key)?, header_value(&item)?)); - } - return Ok(HeaderValue::Map(entries)); - } - if let Ok(list) = value.downcast::() { - let mut items: Vec = Vec::with_capacity(list.len()); - for item in list.iter() { - items.push(header_value(&item)?); + let (Some(key), Some(item)) = (header_value(&key)?, header_value(&item)?) else { + return Ok(None); + }; + entries.push((key, item)); } - return Ok(HeaderValue::Sequence(items)); + return Ok(Some(HeaderValue::Map(entries))); } - if let Ok(tuple) = value.downcast::() { - let mut items: Vec = Vec::with_capacity(tuple.len()); - for item in tuple.iter() { - items.push(header_value(&item)?); - } - return Ok(HeaderValue::Sequence(items)); + let items: Vec> = if let Ok(list) = value.downcast::() { + list.iter().collect() + } else if let Ok(tuple) = value.downcast::() { + tuple.iter().collect() + } else { + return Ok(Some(HeaderValue::Other)); + }; + let mut sequence: Vec = Vec::with_capacity(items.len()); + for item in &items { + let Some(item) = header_value(item)? else { + return Ok(None); + }; + sequence.push(item); } - Ok(HeaderValue::Other) + Ok(Some(HeaderValue::Sequence(sequence))) } /// A `str` (or subclass) as text; text Rust cannot hold, such as lone surrogates, is `None`. @@ -141,8 +151,9 @@ fn text_value(value: &Bound<'_, PyAny>) -> Option { fn header_row(header: FunctionHeader) -> HeaderRow { let (scalar, table): (Option, Option) = match header.returns { - FunctionReturns::Type(text) => (Some(text), None), - FunctionReturns::Table(columns) => (None, Some(rows(columns))), + Some(FunctionReturns::Type(text)) => (Some(text), None), + Some(FunctionReturns::Table(columns)) => (None, Some(rows(columns))), + None => (None, None), }; ( rows(header.arguments), @@ -153,9 +164,22 @@ fn header_row(header: FunctionHeader) -> HeaderRow { header.runtime_version, header.entry_point, header.packages, + header + .failure + .map(|failure| (stage_name(failure.stage), failure.message)), ) } +fn stage_name(stage: HeaderStage) -> &'static str { + match stage { + HeaderStage::Start => "start", + HeaderStage::Arguments => "arguments", + HeaderStage::Returns => "returns", + HeaderStage::PythonValues => "python_values", + HeaderStage::Metadata => "metadata", + } +} + fn rows(named: Vec) -> Pairs { let mut rows: Pairs = Vec::with_capacity(named.len()); for item in named { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/attachments/test_parameters.rs b/crates/sqlbuild-python/src/bindings/_helpers/attachments/test_parameters.rs index 6ca3028e98..2d344fc816 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/attachments/test_parameters.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/attachments/test_parameters.rs @@ -6,21 +6,23 @@ use sqlbuild_attachments::test_parameters::main::parameter_references::parameter use crate::bindings::_helpers::boundary::panics::compiler_guard; -/// `(start, end, name)` code-point spans of every reference, or `None` where Python must scan. +/// `(start, end, name)` by code points. +type Span = (usize, usize, String); + +/// The references Python renders and the error it then raises for `owner`, the test case. #[pyfunction] fn scan_test_parameter_references( sql: &str, declared: Vec, -) -> PyResult>> { + owner: &str, +) -> PyResult<(Vec, Option)> { compiler_guard(|| { - let Some(references) = parameter_references(sql, &declared) else { - return Ok(None); - }; - let mut spans: Vec<(usize, usize, String)> = Vec::with_capacity(references.len()); - for reference in references { + let scan = parameter_references(sql, &declared, owner); + let mut spans: Vec = Vec::with_capacity(scan.references.len()); + for reference in scan.references { spans.push((reference.start, reference.end, reference.name)); } - Ok(Some(spans)) + Ok((spans, scan.error)) }) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/model_config/config_templates.rs b/crates/sqlbuild-python/src/bindings/_helpers/model_config/config_templates.rs index 6a6242a625..55408d370f 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/model_config/config_templates.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/model_config/config_templates.rs @@ -51,9 +51,9 @@ pub(crate) struct TemplateSources<'py>( pub(crate) Bound<'py, PyDict>, ); -/// `allow_context`, `preserve_context_tokens` and `preserve_unknown_context`. +/// The three resolution flags, then a label making rejections `("invalid", message, reads)`. #[derive(FromPyObject)] -struct TemplateFlags(bool, bool, bool); +struct TemplateFlags(bool, bool, bool, Option); /// A template failure, or a Python error raised while building the expanded containers. pub(crate) enum Stop { @@ -132,7 +132,7 @@ impl<'py> TemplateHost for PythonHost<'py> { } } -/// Return `(value, ordered reads)` as `expand_template_data` would, `invalid` or `unsupported`. +/// Return `(value, reads)` as `expand_template_data` would, a rejection, or `unsupported`. #[pyfunction] fn expand_config_templates<'py>( py: Python<'py>, @@ -140,7 +140,9 @@ fn expand_config_templates<'py>( sources: TemplateSources<'py>, flags: TemplateFlags, ) -> PyResult> { - let TemplateFlags(allow_context, preserve_context_tokens, preserve_unknown_context) = flags; + let TemplateFlags(allow_context, preserve_context_tokens, preserve_unknown_context, label) = + flags; + let context_label: Option<&str> = label.as_deref(); let options = TemplateOptions { allow_context, preserve_context_tokens, @@ -152,8 +154,18 @@ fn expand_config_templates<'py>( .into_pyobject(py)? .into_any() .unbind()), - Err(Stop::Failure(TemplateFailure::Missing | TemplateFailure::Invalid)) => { - Ok(PyString::new(py, INVALID_OUTCOME).into_any().unbind()) + Err(Stop::Failure(TemplateFailure::Missing(error) | TemplateFailure::Invalid(error))) => { + match context_label { + Some(label) => Ok(( + INVALID_OUTCOME, + error.message(label), + host.reads.into_inner(), + ) + .into_pyobject(py)? + .into_any() + .unbind()), + None => Ok(PyString::new(py, INVALID_OUTCOME).into_any().unbind()), + } } Err(Stop::Failure(TemplateFailure::Unsupported)) => { Ok(PyString::new(py, UNSUPPORTED_OUTCOME).into_any().unbind()) diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_interpolation/substitution.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_interpolation/substitution.rs index 0051e448d1..fadcd2af08 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_interpolation/substitution.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_interpolation/substitution.rs @@ -1,13 +1,32 @@ //! Conservative native fast path for compile-wide scalar SQL variables. use crate::sql_scan::main::non_code_end::non_code_end; -use crate::sql_scan::models::QuotePolicy; +use crate::sql_scan::models::{QuotePolicy, Unclosed}; use std::collections::HashMap; use std::ops::Range; pub(crate) const UNCHANGED: u8 = 0; pub(crate) const SUBSTITUTED: u8 = 1; pub(crate) const FALLBACK: u8 = 2; +/// Python raises for an unknown variable, named in the result, unless only Python holds it. +pub(crate) const UNKNOWN_VARIABLE: u8 = 3; +pub(crate) const UNCLOSED_QUOTE: u8 = 4; +pub(crate) const UNCLOSED_BLOCK_COMMENT: u8 = 5; + +/// Why substitution stops before the end of the SQL. +enum Stop { + Fallback, + UnknownVariable(String), +} + +impl Stop { + fn result(self) -> (u8, Option) { + match self { + Self::Fallback => (FALLBACK, None), + Self::UnknownVariable(name) => (UNKNOWN_VARIABLE, Some(name)), + } + } +} struct SubstitutionState { output: Option, @@ -39,30 +58,32 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option end, Ok(None) => index, - Err(_) => return (FALLBACK, None), + Err(Unclosed::BlockComment) => return (UNCLOSED_BLOCK_COMMENT, None), + Err(Unclosed::Quote) => return (UNCLOSED_QUOTE, None), + Err(Unclosed::Parenthesis) => return (FALLBACK, None), }; if text_end > index { let comment = bytes[index..].starts_with(b"--") || bytes[index..].starts_with(b"/*"); if !comment { - let Some(next_state) = - substitute_quoted_text(sql, index..text_end, variables, state) - else { - return (FALLBACK, None); - }; - state = next_state; + match substitute_quoted_text(sql, index..text_end, variables, state) { + Ok(next_state) => state = next_state, + Err(stop) => return stop.result(), + } } index = text_end; continue; } if bytes[index..].starts_with(b"@@") { - let Some((end, next_state)) = substitute_token(sql, index, variables, state) else { - return (FALLBACK, None); - }; - state = next_state; - index = end; + match substitute_token(sql, index, variables, state) { + Ok((end, next_state)) => { + state = next_state; + index = end; + } + Err(stop) => return stop.result(), + } continue; } index += 1; @@ -76,23 +97,35 @@ fn substitute_one(sql: &str, variables: &HashMap<&str, &str>) -> (u8, Option Result, Unclosed> { + if bytes[index] != b'`' { + return non_code_end(bytes, index, QuotePolicy::COMPILER); + } + bytes[index + 1..] + .iter() + .position(|byte| *byte == b'`') + .map(|offset| Some(index + 1 + offset + 1)) + .ok_or(Unclosed::Quote) +} + /// Python's `_interpolate_sql_segment`: substitute every token inside one quoted literal. fn substitute_quoted_text( sql: &str, text: Range, variables: &HashMap<&str, &str>, mut state: SubstitutionState, -) -> Option { +) -> Result { let mut index = text.start; while let Some(offset) = sql[index..text.end].find("@@") { let (token_end, next_state) = substitute_token(sql, index + offset, variables, state)?; if token_end > text.end { - return None; + return Err(Stop::Fallback); } state = next_state; index = token_end; } - Some(state) + Ok(state) } fn substitute_token( @@ -100,7 +133,7 @@ fn substitute_token( start: usize, variables: &HashMap<&str, &str>, mut state: SubstitutionState, -) -> Option<(usize, SubstitutionState)> { +) -> Result<(usize, SubstitutionState), Stop> { let bytes = sql.as_bytes(); if bytes[start..].starts_with(b"@@@") { let name_start = start + 3; @@ -109,33 +142,35 @@ fn substitute_token( .copied() .is_some_and(is_identifier_start) { - consume_ascii_identifier(bytes, name_start)? + consume_ascii_identifier(bytes, name_start).ok_or(Stop::Fallback)? } else { name_start }; - return Some((end, state)); + return Ok((end, state)); } let name_start = start + 2; if bytes[name_start..].starts_with(b"ENV:") || bytes[name_start..].starts_with(b"CTX:") { - return None; + return Err(Stop::Fallback); } if !bytes .get(name_start) .copied() .is_some_and(is_identifier_start) { - return None; + return Err(Stop::Fallback); } - let name_end = consume_ascii_identifier(bytes, name_start)?; + let name_end = consume_ascii_identifier(bytes, name_start).ok_or(Stop::Fallback)?; let name = &sql[name_start..name_end]; - let value = variables.get(name)?; + let value = variables + .get(name) + .ok_or_else(|| Stop::UnknownVariable(name.to_owned()))?; let rendered = state .output .get_or_insert_with(|| String::with_capacity(sql.len())); rendered.push_str(&sql[state.copy_start..start]); rendered.push_str(value); state.copy_start = name_end; - Some((name_end, state)) + Ok((name_end, state)) } fn consume_ascii_identifier(bytes: &[u8], start: usize) -> Option { diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs index 3c9fd20066..9a5b9624f4 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs @@ -2,7 +2,8 @@ use crate::compiler::_helpers::model_headers::tokenization::{ MAX_TOKENIZER_WORKERS, TOKENIZER_WORKER_STACK_BYTES, build_tokenizer_pool, parse_batch, }; use crate::compiler::_helpers::sql_interpolation::substitution::{ - FALLBACK, SUBSTITUTED, UNCHANGED, substitute_batch, + FALLBACK, SUBSTITUTED, UNCHANGED, UNCLOSED_BLOCK_COMMENT, UNCLOSED_QUOTE, UNKNOWN_VARIABLE, + substitute_batch, }; use crate::compiler::_helpers::sql_references::extraction::extract; use crate::compiler::main::declaration_references::scan_declaration_references; @@ -37,14 +38,22 @@ pub(crate) fn scalar_variables_preserve_lexical_boundaries() -> bool { pub(crate) fn dynamic_or_malformed_sql_requests_fallback() -> bool { let sqls = vec![ - "SELECT @@ENV:USER".to_owned(), - "SELECT @@missing".to_owned(), + "SELECT @@ENV:USER, @@missing".to_owned(), + "SELECT @@missing, @@ENV:USER".to_owned(), + "SELECT '@@missing'".to_owned(), "SELECT @@revision, 'unterminated".to_owned(), "SELECT @@revision /* unterminated".to_owned(), "SELECT @@révision".to_owned(), ]; substitute_batch(&sqls, &[("revision".to_owned(), "7".to_owned())]) - == vec![(FALLBACK, None); sqls.len()] + == vec![ + (FALLBACK, None), + (UNKNOWN_VARIABLE, Some("missing".to_owned())), + (UNKNOWN_VARIABLE, Some("missing".to_owned())), + (UNCLOSED_QUOTE, None), + (UNCLOSED_BLOCK_COMMENT, None), + (FALLBACK, None), + ] } pub(crate) fn dollar_quoted_text_is_quoted_for_substitution() -> bool { @@ -71,14 +80,14 @@ pub(crate) fn dollar_quoted_text_is_quoted_for_substitution() -> bool { ] } -pub(crate) fn unclosed_dollar_quote_requests_fallback() -> bool { +pub(crate) fn unclosed_dollar_quote_stops_as_unclosed_quote() -> bool { let sqls = vec![ "SELECT $tag$ @@region".to_owned(), "SELECT @@region, $$ open".to_owned(), "-- @@region\nSELECT $t$ closes with another tag $u$".to_owned(), ]; substitute_batch(&sqls, &[("region".to_owned(), "north".to_owned())]) - == vec![(FALLBACK, None); sqls.len()] + == vec![(UNCLOSED_QUOTE, None); sqls.len()] } pub(crate) fn simple_references_preserve_authored_order() -> bool { @@ -327,3 +336,20 @@ pub(crate) fn parse_error_on_ordinary_thread(header: String) -> Option { pub(crate) fn nesting_error_at(position: usize) -> String { format!("values nest deeper than 256 levels at position {position}") } + +pub(crate) fn doubled_backticks_close_one_segment_and_open_the_next() -> bool { + let sqls = vec![ + "SELECT `@@zz``".to_owned(), + "SELECT `@@region``@@region`, '@@region''s'".to_owned(), + "SELECT `a``b` -- @@region".to_owned(), + ]; + substitute_batch(&sqls, &[("region".to_owned(), "north".to_owned())]) + == vec![ + (UNKNOWN_VARIABLE, Some("zz".to_owned())), + ( + SUBSTITUTED, + Some("SELECT `north``north`, 'north''s'".to_owned()), + ), + (UNCHANGED, None), + ] +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_sql_interpolation.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_sql_interpolation.rs index 56c6e801f1..362e82c422 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/test_sql_interpolation.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_sql_interpolation.rs @@ -1,19 +1,26 @@ use crate::compiler::tests::helpers::{ - dollar_quoted_text_is_quoted_for_substitution, dynamic_or_malformed_sql_requests_fallback, - scalar_variables_preserve_lexical_boundaries, unclosed_dollar_quote_requests_fallback, + dollar_quoted_text_is_quoted_for_substitution, + doubled_backticks_close_one_segment_and_open_the_next, + dynamic_or_malformed_sql_requests_fallback, scalar_variables_preserve_lexical_boundaries, + unclosed_dollar_quote_stops_as_unclosed_quote, }; use crate::compiler::tests::test_types::StaticSqlOperationTestCase; #[test] fn given_sql_interpolation_cases_when_substituting_then_expected_behavior_holds() { let test_cases = [ + StaticSqlOperationTestCase { + description: "doubled backticks close one quoted segment and open the next", + run: doubled_backticks_close_one_segment_and_open_the_next, + expected_success: true, + }, StaticSqlOperationTestCase { description: "scalar variables preserve lexical boundaries", run: scalar_variables_preserve_lexical_boundaries, expected_success: true, }, StaticSqlOperationTestCase { - description: "dynamic and malformed SQL requests Python fallback", + description: "dynamic SQL falls back; unknown names and unclosed text stop", run: dynamic_or_malformed_sql_requests_fallback, expected_success: true, }, @@ -23,8 +30,8 @@ fn given_sql_interpolation_cases_when_substituting_then_expected_behavior_holds( expected_success: true, }, StaticSqlOperationTestCase { - description: "unclosed dollar quotes request Python fallback", - run: unclosed_dollar_quote_requests_fallback, + description: "unclosed dollar quotes stop as unclosed quoted text", + run: unclosed_dollar_quote_stops_as_unclosed_quote, expected_success: true, }, ]; diff --git a/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py new file mode 100644 index 0000000000..2761d8c5e6 --- /dev/null +++ b/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py @@ -0,0 +1,109 @@ +"""Minimal projects whose audits, functions or sources fail while attaching compile inputs.""" + +from scripts.compiler_differential._helpers.corpus.case_builder import config_files, failure_case +from scripts.compiler_differential.constants import ( + FAILURE_BASE_SOURCES, + FAILURE_BASE_STAGING, + FAILURE_SOURCES_PATH, + FAILURE_STAGING_PATH, +) +from scripts.compiler_differential.models import FailureCase + +_STAGING_HEADER: str = '"Staged orders",' +_FLOOR_AUDIT_PATH: str = "models/staging/_sqlbuild/_audits/generic/floor.sql" +_FLOOR_AUDIT: str = 'AUDIT ();\n\nSELECT *\nFROM __ref("@model")\nWHERE amount < @minimum\n' +_FRESH_AUDIT: str = ( + 'AUDIT ();\n\nSELECT *\nFROM __ref("@model")\nWHERE ordered_at >= __cursor_start()\n' +) +_ORDER_LABEL_PATH: str = "functions/sql/order_label.sql" + + +def _staging_audits(audits: str) -> dict[str, str]: + return { + FAILURE_STAGING_PATH: FAILURE_BASE_STAGING.replace( + _STAGING_HEADER, f"{_STAGING_HEADER}\n audits [{audits}]," + ) + } + + +def _order_label(header: str) -> dict[str, str]: + return {_ORDER_LABEL_PATH: f"FUNCTION (\n{header});\n\nUPPER(p_status)\n"} + + +def attachment_failure_cases() -> tuple[FailureCase, ...]: + """Return audit, function and source attachment failures the native stage reports.""" + + return ( + failure_case( + name="generic-audit-unknown-run-scope", + expected_code="P001", + expected_message="unknown audit run_scope 'always'", + files={ + _FLOOR_AUDIT_PATH: _FLOOR_AUDIT, + **_staging_audits('floor (minimum 0, run_scope "always")'), + }, + ), + failure_case( + name="generic-audit-overrides-implicit-model", + expected_code="P001", + expected_message="must not override implicit model from attached context", + files={ + _FLOOR_AUDIT_PATH: _FLOOR_AUDIT, + **_staging_audits('floor (minimum 0, model "customer_totals")'), + }, + ), + failure_case( + name="generic-audit-cursor-intrinsic", + expected_code="P001", + expected_message="Audit 'fresh' uses cursor intrinsics", + files={ + "models/staging/_sqlbuild/_audits/generic/fresh.sql": _FRESH_AUDIT, + **_staging_audits("fresh"), + }, + ), + failure_case( + name="audit-unknown-project-variable", + expected_code="P001", + expected_message="unknown project variable '@@floor_amount'", + files={ + **config_files('\n[vars]\nregion = "north"\n'), + _FLOOR_AUDIT_PATH: _FLOOR_AUDIT.replace("@minimum", "@@floor_amount"), + **_staging_audits("floor"), + }, + ), + failure_case( + name="audit-variable-in-doubled-backticks", + expected_code="P001", + expected_message="unknown project variable '@@floor_amount'", + files={ + **config_files('\n[vars]\nregion = "north"\n'), + _FLOOR_AUDIT_PATH: _FLOOR_AUDIT.replace("@minimum", "`@@floor_amount``"), + **_staging_audits("floor"), + }, + ), + failure_case( + name="sql-function-missing-returns", + expected_code="P001", + expected_message="functions/sql/order_label.sql must declare returns", + files=_order_label(' description "Order label",\n arguments (p_status VARCHAR),\n'), + ), + failure_case( + name="sql-function-template-namespace", + expected_code="P001", + expected_message="references unsupported template namespace 'VAR'", + files=_order_label( + ' description "Order label",\n arguments (p_status VARCHAR),\n' + ' returns "${VAR:label_type}",\n' + ), + ), + failure_case( + name="source-description-unknown-variable", + expected_code="P001", + expected_message="references unknown variable 'channel'", + files={ + FAILURE_SOURCES_PATH: FAILURE_BASE_SOURCES.replace( + "description: Orders feed.", 'description: "Orders from ${channel}"' + ) + }, + ), + ) diff --git a/scripts/compiler_differential/_helpers/corpus/failure_cases.py b/scripts/compiler_differential/_helpers/corpus/failure_cases.py index d0fb4d2c30..c22ae06ca7 100644 --- a/scripts/compiler_differential/_helpers/corpus/failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/failure_cases.py @@ -3,6 +3,9 @@ from scripts.compiler_differential._helpers.analysis_corpus.failure_cases import ( analysis_failure_cases, ) +from scripts.compiler_differential._helpers.corpus.attachment_failure_cases import ( + attachment_failure_cases, +) from scripts.compiler_differential._helpers.corpus.case_builder import ( failure_case, mart_body_files, @@ -52,6 +55,7 @@ def all_failure_cases() -> tuple[FailureCase, ...]: *config_failure_cases(), *scope_failure_cases(), *reference_failure_cases(), + *attachment_failure_cases(), *analysis_failure_cases(), ) diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 251e98b73f..dce76a705d 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -163,8 +163,8 @@ def config_contains_macro_call(value: object) -> bool | None: ... def expand_config_templates( value: object, sources: tuple[dict[str, object], object, dict[str, str | None]], - flags: tuple[bool, bool, bool], -) -> tuple[object, list[tuple[str, str]]] | str: ... + flags: tuple[bool, bool, bool, str | None], +) -> tuple[object, list[tuple[str, str]]] | tuple[str, str, list[tuple[str, str]]] | str: ... class NativeModelConfigBuilder: def __init__( @@ -232,11 +232,13 @@ class SqlTestTargetCatalog: def omitted_ceremonial_select(sql: str, syntax: dict[str, object]) -> tuple[bool, int | None]: ... def scan_test_parameter_references( - sql: str, declared: list[str] -) -> list[tuple[int, int, str]] | None: ... -def sql_free_of_cursor_intrinsics(sql: str, reserved_markers: list[str]) -> bool: ... + sql: str, declared: list[str], owner: str +) -> tuple[list[tuple[int, int, str]], str | None]: ... +def sql_free_of_cursor_intrinsics( + sql: str, reserved_markers: list[str], context: str +) -> tuple[bool, str | None]: ... def parse_function_header_values( - header_values: dict[str, object], python: bool + header_values: dict[str, object], python: bool, relative_path: str ) -> ( tuple[ list[tuple[str, str, str]], @@ -247,6 +249,7 @@ def parse_function_header_values( str | None, str | None, list[str], + tuple[str, str] | None, ] | None ): ... @@ -255,10 +258,11 @@ def pair_seed_files( declarations: list[str], stems: list[str] ) -> tuple[list[int], None] | tuple[None, int]: ... def render_attached_generic_audit( + labels: tuple[str, str], sql: tuple[str, str | None], arguments: tuple[dict[str, object], dict[str, object]], policies: dict[str, object], -) -> tuple[str, str | None, str, str] | None: ... +) -> tuple[str | None, str, str | None, str, str, str | None] | None: ... def scan_sql_declaration_references( sqls: list[str], ) -> list[tuple[list[tuple[int, str, str | None, int, int]], int | None] | None]: ... diff --git a/src/sqlbuild/compiler/attachments/constants.py b/src/sqlbuild/compiler/attachments/constants.py index 053a93caab..25136884df 100644 --- a/src/sqlbuild/compiler/attachments/constants.py +++ b/src/sqlbuild/compiler/attachments/constants.py @@ -2,3 +2,8 @@ INSTANCE_POLICY: str = "instance" DEFAULT_POLICY: str = "default" +HEADER_START_STAGE: str = "start" +HEADER_ARGUMENTS_STAGE: str = "arguments" +HEADER_RETURNS_STAGE: str = "returns" +HEADER_PYTHON_VALUES_STAGE: str = "python_values" +HEADER_METADATA_STAGE: str = "metadata" diff --git a/src/sqlbuild/compiler/attachments/main/_parse_native_function_header.py b/src/sqlbuild/compiler/attachments/main/_parse_native_function_header.py index d3f4b2403f..03cd82b5aa 100644 --- a/src/sqlbuild/compiler/attachments/main/_parse_native_function_header.py +++ b/src/sqlbuild/compiler/attachments/main/_parse_native_function_header.py @@ -2,14 +2,16 @@ from __future__ import annotations +from pathlib import Path + import sqlbuild._native as _native from sqlbuild.compiler.attachments.models import NativeFunctionHeader, NativeNamedType def parse_native_function_header( - *, header_values: dict[str, object], python: bool + *, header_values: dict[str, object], python: bool, relative_path: Path ) -> NativeFunctionHeader | None: - """Return the parsed header, or None where Python must parse it and raise its errors.""" + """Return the parsed header with Python's first error, or None where Python must parse it.""" if not isinstance(header_values, dict): return None @@ -23,12 +25,23 @@ def parse_native_function_header( str | None, str | None, list[str], + tuple[str, str] | None, ] | None - ) = _native.parse_function_header_values(header_values, python) + ) = _native.parse_function_header_values(header_values, python, str(relative_path)) if row is None: return None - arguments, returns, columns, tags, description, runtime_version, entry_point, packages = row + ( + arguments, + returns, + columns, + tags, + description, + runtime_version, + entry_point, + packages, + failure, + ) = row return NativeFunctionHeader( arguments=_named_types(arguments), returns=returns, @@ -38,6 +51,7 @@ def parse_native_function_header( runtime_version=runtime_version, entry_point=entry_point, packages=tuple(packages), + failure=failure, ) diff --git a/src/sqlbuild/compiler/attachments/main/_render_native_attached_audit.py b/src/sqlbuild/compiler/attachments/main/_render_native_attached_audit.py index e8b84975fd..10c184144f 100644 --- a/src/sqlbuild/compiler/attachments/main/_render_native_attached_audit.py +++ b/src/sqlbuild/compiler/attachments/main/_render_native_attached_audit.py @@ -10,13 +10,14 @@ def render_native_attached_audit( *, + labels: tuple[str, str], sql_body: str, evidence_sql: str | None, implicit_arguments: dict[str, object], explicit_arguments: dict[str, object], policies: NativeAuditPolicies, ) -> NativeRenderedAudit | None: - """Return the rendering, or None where Python must render the audit and raise its errors.""" + """Return the rendering with Python's errors for `(owner, audit)`, or None for Python.""" if not all( value is None or isinstance(value, str) @@ -28,7 +29,12 @@ def render_native_attached_audit( ) ) or not isinstance(explicit_arguments, dict): return None - rendered: tuple[str, str | None, str, str] | None = _native.render_attached_generic_audit( - (sql_body, evidence_sql), (implicit_arguments, explicit_arguments), asdict(policies) + rendered: tuple[str | None, str, str | None, str, str, str | None] | None = ( + _native.render_attached_generic_audit( + labels, + (sql_body, evidence_sql), + (implicit_arguments, explicit_arguments), + asdict(policies), + ) ) return None if rendered is None else NativeRenderedAudit(*rendered) diff --git a/src/sqlbuild/compiler/attachments/models.py b/src/sqlbuild/compiler/attachments/models.py index 3711e17c4f..bf740e2bbe 100644 --- a/src/sqlbuild/compiler/attachments/models.py +++ b/src/sqlbuild/compiler/attachments/models.py @@ -21,12 +21,14 @@ class NativeAuditPolicies: @dataclass(frozen=True, slots=True) class NativeRenderedAudit: - """Rendered audit SQL, the severity value and where the run scope comes from.""" + """Rendered audit SQL and policies, or Python's render error and later policy error.""" + render_error: str | None sql_body: str evidence_sql: str | None severity: str run_scope_source: str + policy_error: str | None @dataclass(frozen=True, slots=True) @@ -40,7 +42,7 @@ class NativeNamedType: @dataclass(frozen=True, slots=True) class NativeFunctionHeader: - """A function header Python accepts, with argument and return types still unexpanded.""" + """A function header with unexpanded types and Python's first `(stage, message)` error.""" arguments: tuple[NativeNamedType, ...] returns: str | None @@ -50,6 +52,7 @@ class NativeFunctionHeader: runtime_version: str | None entry_point: str | None packages: tuple[str, ...] + failure: tuple[str, str] | None @dataclass(frozen=True, slots=True) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py b/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py index 0ace8f95ed..a5c746077e 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py @@ -677,6 +677,7 @@ def build_attached_audit_input( ) native_rendering: NativeRenderedAudit | None = ( render_native_attached_audit( + labels=(str(owner_file), audit_instance.definition_name), sql_body=definition[1].sql_body, evidence_sql=definition[1].evidence_sql, implicit_arguments=implicit_arguments, @@ -686,6 +687,8 @@ def build_attached_audit_input( if native_stage_enabled(NativeStage.ATTACHMENTS) else None ) + if native_rendering is not None and native_rendering.render_error is not None: + raise CompileInputError(native_rendering.render_error, bridge_independent=True) rendered_sql_body, rendered_evidence_sql = ( (native_rendering.sql_body, native_rendering.evidence_sql) if native_rendering is not None @@ -883,6 +886,8 @@ def _native_audit_policy_values( if rendering.run_scope_source == DEFAULT_POLICY else None ) + if rendering.policy_error is not None: + raise CompileInputError(rendering.policy_error, bridge_independent=True) return AuditSeverity(rendering.severity), ( AuditRunScope.DELTA_AND_FINAL if run_scope is None else run_scope ) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py index 78564c72cb..d0ac70c771 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py @@ -1221,7 +1221,8 @@ def _native_seed_inputs( seed_entry, seed_schema_file = seed_declarations[cast(int, missing)] raise CompileInputError( f"Seed declaration '{seed_entry.name}' in {seed_schema_file.relative_path} " - "has no matching CSV file under seeds/" + "has no matching CSV file under seeds/", + bridge_independent=True, ) return tuple( CompileSeedInput( diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py b/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py index 53a07fa639..e1194d71b8 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py @@ -6,6 +6,13 @@ from pathlib import Path from typing import Any, cast +from sqlbuild.compiler.attachments.constants import ( + HEADER_ARGUMENTS_STAGE, + HEADER_METADATA_STAGE, + HEADER_PYTHON_VALUES_STAGE, + HEADER_RETURNS_STAGE, + HEADER_START_STAGE, +) from sqlbuild.compiler.attachments.main._parse_native_function_header import ( parse_native_function_header, ) @@ -254,6 +261,8 @@ def build_sql_function_inputs( known_function_names=known_function_names, known_table_function_names=known_table_function_names, ) + if native_header is not None: + _raise_header_failure(header=native_header, stage=HEADER_METADATA_STAGE) function_inputs.append( CompileSqlFunctionInput( function_file=function_file, @@ -322,23 +331,27 @@ def _sql_function_header( ]: header_values: dict[str, object] = function_file.header_values native_header: NativeFunctionHeader | None = ( - parse_native_function_header(header_values=header_values, python=False) if native else None + parse_native_function_header( + header_values=header_values, python=False, relative_path=function_file.relative_path + ) + if native + else None ) if native_header is not None: + _raise_header_failure(header=native_header, stage=HEADER_START_STAGE) arguments: tuple[FunctionArgument, ...] = _expanded_native_arguments( header=native_header, label=f"SQL function {function_file.relative_path}", effective_vars=effective_vars, ) - return ( - native_header, - arguments, - *_expanded_native_sql_returns( - header=native_header, - relative_path=function_file.relative_path, - effective_vars=effective_vars, - ), + _raise_header_failure(header=native_header, stage=HEADER_ARGUMENTS_STAGE) + native_returns: tuple[str, tuple[FunctionReturnColumn, ...]] = _expanded_native_sql_returns( + header=native_header, + relative_path=function_file.relative_path, + effective_vars=effective_vars, ) + _raise_header_failure(header=native_header, stage=HEADER_RETURNS_STAGE) + return (native_header, arguments, *native_returns) raw_returns: object | None = header_values.get("returns") if raw_returns is None: raise CompileInputError( @@ -388,6 +401,11 @@ def _sql_function_namespace( return namespace.database, namespace.schema +def _raise_header_failure(*, header: NativeFunctionHeader, stage: str) -> None: + if header.failure is not None and header.failure[0] == stage: + raise CompileInputError(header.failure[1], bridge_independent=True) + + def _expanded_native_arguments( *, header: NativeFunctionHeader, label: str, effective_vars: dict[str, object] ) -> tuple[FunctionArgument, ...]: @@ -459,7 +477,13 @@ def _build_python_function_input( header_values: dict[str, object] = python_function_file.header_values native: bool = native_stage_enabled(NativeStage.ATTACHMENTS) native_header: NativeFunctionHeader | None = ( - parse_native_function_header(header_values=header_values, python=True) if native else None + parse_native_function_header( + header_values=header_values, + python=True, + relative_path=python_function_file.relative_path, + ) + if native + else None ) arguments: tuple[FunctionArgument, ...] returns: str @@ -467,16 +491,19 @@ def _build_python_function_input( entry_point: str packages: tuple[str, ...] if native_header is not None: + _raise_header_failure(header=native_header, stage=HEADER_START_STAGE) arguments = _expanded_native_arguments( header=native_header, label=f"Python function {python_function_file.relative_path}", effective_vars=effective_vars, ) + _raise_header_failure(header=native_header, stage=HEADER_ARGUMENTS_STAGE) returns = _expand_function_header_value( raw_value=cast(str, native_header.returns), effective_vars=effective_vars, context_label=f"Python function {python_function_file.relative_path} returns", ) + _raise_header_failure(header=native_header, stage=HEADER_PYTHON_VALUES_STAGE) runtime_version = cast(str, native_header.runtime_version) entry_point = cast(str, native_header.entry_point) packages = native_header.packages @@ -551,6 +578,8 @@ def _build_python_function_input( adapter_name=context.adapter_name, context=f"Python function {python_function_file.relative_path} return type", ) + if native_header is not None: + _raise_header_failure(header=native_header, stage=HEADER_METADATA_STAGE) return CompileSqlFunctionInput( function_file=python_function_file, name=function_name, @@ -850,6 +879,7 @@ def _expand_function_header_value( preserve_context_tokens=True, preserve_unknown_context=False, native=native_stage_enabled(NativeStage.ATTACHMENTS), + exact_errors=True, ) ) @@ -949,5 +979,6 @@ def _expand_function_environment_value( preserve_context_tokens=True, preserve_unknown_context=False, native=native_stage_enabled(NativeStage.ATTACHMENTS), + exact_errors=True, ) ) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/sources.py b/src/sqlbuild/compiler/compile/_helpers/attachment/sources.py index eee929f819..d784bdbaf0 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/sources.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/sources.py @@ -329,4 +329,5 @@ def _expand_source_template_object( preserve_context_tokens=False, preserve_unknown_context=False, native=native_stage_enabled(NativeStage.ATTACHMENTS), + exact_errors=True, ) diff --git a/src/sqlbuild/compiler/compile/_helpers/render/context_templates.py b/src/sqlbuild/compiler/compile/_helpers/render/context_templates.py index a01d37e37f..f2aec618f8 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/context_templates.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/context_templates.py @@ -7,14 +7,25 @@ from uuid import uuid4 from sqlbuild.compiler.compile._helpers.render.templating import expand_template_data -from sqlbuild.compiler.compile.constants import COMPILE_INPUT_READS, PRESERVE_TARGET_VALUE +from sqlbuild.compiler.compile.constants import ( + COMPILE_INPUT_READS, + MISSING_TEMPLATE_CONTEXT_MESSAGE_PART, + MISSING_TEMPLATE_CONTEXT_VALUE_MESSAGE_PART, + MISSING_TEMPLATE_VALUE_MESSAGE_PARTS, + PRESERVE_TARGET_VALUE, +) +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.compile.types import CompileContextKey from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.model_config.constants import ENVIRONMENT_READ, INVALID_OUTCOME from sqlbuild.compiler.model_config.main._expand_native_config_templates import ( expand_native_config_templates, ) -from sqlbuild.compiler.model_config.models import NativeTemplateExpansion, TemplateResolutionFlags +from sqlbuild.compiler.model_config.models import ( + NativeTemplateExpansion, + NativeTemplateRejection, + TemplateResolutionFlags, +) from sqlbuild.spec.contracts.models import TargetConfig @@ -248,10 +259,11 @@ def expand_config_templates( preserve_context_tokens: bool, preserve_unknown_context: bool, native: bool, + exact_errors: bool = False, ) -> object: - """Expand like `expand_template_data`, natively when set; native rejections re-run Python.""" + """Expand like `expand_template_data`, natively when set; rejections re-run or raise exactly.""" - outcome: NativeTemplateExpansion | str | None = ( + outcome: NativeTemplateExpansion | NativeTemplateRejection | str | None = ( expand_native_config_templates( value=value, variables=variables, @@ -261,6 +273,11 @@ def expand_config_templates( preserve_context_tokens=preserve_context_tokens, preserve_unknown_context=preserve_unknown_context, ), + context_label=( + context_label + if exact_errors and not _names_missing_template_value(context_label) + else None + ), ) if native else None @@ -268,6 +285,9 @@ def expand_config_templates( if isinstance(outcome, NativeTemplateExpansion): record_template_reads(outcome.reads) return outcome.value + if isinstance(outcome, NativeTemplateRejection): + record_template_reads(outcome.reads) + raise CompileInputError(outcome.message, bridge_independent=True) expanded: object = expand_template_data( value=value, variables=variables, @@ -285,6 +305,19 @@ def expand_config_templates( return expanded +def _names_missing_template_value(context_label: str) -> bool: + """Whether Python's `coalesce` would read errors naming this label as missing values.""" + + return any( + part in context_label + for part in ( + *MISSING_TEMPLATE_VALUE_MESSAGE_PARTS, + MISSING_TEMPLATE_CONTEXT_MESSAGE_PART, + MISSING_TEMPLATE_CONTEXT_VALUE_MESSAGE_PART, + ) + ) + + def record_template_reads(reads: tuple[tuple[str, str], ...]) -> None: """Record native template environment and context reads for project reuse, in order.""" diff --git a/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py b/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py index ca1a597625..6b18d042b6 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py @@ -62,10 +62,14 @@ def get_validated_model_cursor_intrinsics( def reject_cursor_intrinsics(*, sql: str, context: str) -> None: """Reject cursor intrinsics in SQL that does not own an execution interval.""" - if native_stage_enabled(NativeStage.ATTACHMENTS) and _native.sql_free_of_cursor_intrinsics( - sql, [MICROBATCH_START_SENTINEL, MICROBATCH_END_SENTINEL] - ): - return + if native_stage_enabled(NativeStage.ATTACHMENTS): + free, error = _native.sql_free_of_cursor_intrinsics( + sql, [MICROBATCH_START_SENTINEL, MICROBATCH_END_SENTINEL], context + ) + if free: + return + if error is not None: + raise CompileInputError(error) _assert_no_reserved_cursor_markers(sql=sql, context=context) _, found = _transform_cursor_intrinsics( sql=sql, diff --git a/src/sqlbuild/compiler/compile/_helpers/render/parameters.py b/src/sqlbuild/compiler/compile/_helpers/render/parameters.py index 0c339f505c..c8245d7a64 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/parameters.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/parameters.py @@ -36,13 +36,11 @@ def expand_test_parameters( """Render active parameter references while leaving comments and quoted text unchanged.""" value_lookup: dict[str, SqlValue] = dict(values) - references: list[tuple[int, int, str]] | None = ( - _native.scan_test_parameter_references(sql, list(value_lookup)) - if native_stage_enabled(NativeStage.ATTACHMENTS) - else None - ) - if references is not None: - return _splice_parameter_references( + if native_stage_enabled(NativeStage.ATTACHMENTS): + references, error = _native.scan_test_parameter_references( + sql, list(value_lookup), f"SQL test '{test_name}' case '{case_name}' in '{file_path}'" + ) + expanded: tuple[str, frozenset[str]] = _splice_parameter_references( sql=sql, references=references, value_lookup=value_lookup, @@ -50,6 +48,9 @@ def expand_test_parameters( test_name=test_name, case_name=case_name, ) + if error is not None: + raise CompileInputError(error, bridge_independent=True) + return expanded used_names: set[str] = set() parts: list[str] = [] cursor: int = 0 diff --git a/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py b/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py index 4245866f05..a25848e0df 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py @@ -59,7 +59,13 @@ _NATIVE_UNCHANGED: int = 0 _NATIVE_SUBSTITUTED: int = 1 _NATIVE_FALLBACK: int = 2 +_NATIVE_UNKNOWN_VARIABLE: int = 3 +_NATIVE_UNCLOSED_QUOTE: int = 4 +_NATIVE_UNCLOSED_BLOCK_COMMENT: int = 5 _NATIVE_RESULT_LENGTH: int = 2 +_NATIVE_ERROR_STATUSES: frozenset[int] = frozenset( + {_NATIVE_UNKNOWN_VARIABLE, _NATIVE_UNCLOSED_QUOTE, _NATIVE_UNCLOSED_BLOCK_COMMENT} +) def expand_authored_sql_result( # noqa: PLR0913 @@ -79,7 +85,9 @@ def expand_authored_sql_result( # noqa: PLR0913 native: bool = native_stage_enabled(NativeStage.ATTACHMENTS) prepared_sql: str | None = ( - prepare_static_project_vars_batch(sqls=(sql,), effective_vars=effective_vars)[0] + prepare_static_project_vars_batch( + sqls=(sql,), effective_vars=effective_vars, error_file_path=file_path + )[0] if native else None ) @@ -206,8 +214,9 @@ def prepare_static_project_vars_batch( *, sqls: tuple[str, ...], effective_vars: dict[str, object], + error_file_path: Path | None = None, ) -> tuple[str | None, ...]: - """Prepare safe static substitutions, leaving dynamic fallbacks in authored order.""" + """Prepare static substitutions in order; with a file path, raise exact errors for it.""" scalar_variables: list[tuple[str, str]] = [ (name, render_project_var_text(value=value, label=f"SQL variable '@@{name}'")) @@ -233,11 +242,49 @@ def prepare_static_project_vars_batch( results.append(rendered) elif status == _NATIVE_FALLBACK and rendered is None: results.append(None) + elif status in _NATIVE_ERROR_STATUSES: + if error_file_path is not None: + _raise_native_interpolation_error( + status=status, + name=rendered, + file_path=error_file_path, + effective_vars=effective_vars, + ) + results.append(None) else: raise CompileInputError("native static SQL interpolation returned an invalid status") return tuple(results) +def _raise_native_interpolation_error( + *, status: int, name: str | None, file_path: Path, effective_vars: dict[str, object] +) -> None: + if status == _NATIVE_UNCLOSED_QUOTE: + raise CompileInputError( + f"{_CONTEXT} contains an unclosed quoted string", bridge_independent=True + ) + if status == _NATIVE_UNCLOSED_BLOCK_COMMENT: + raise CompileInputError( + f"{_CONTEXT} contains an unclosed block comment", bridge_independent=True + ) + if name is not None and name not in effective_vars: + raise CompileInputError( + _unknown_project_variable_message( + var_name=name, file_path=file_path, effective_vars=effective_vars + ), + bridge_independent=True, + ) + + +def _unknown_project_variable_message( + *, var_name: str, file_path: Path, effective_vars: dict[str, object] +) -> str: + return ( + f"unknown project variable '@@{var_name}' in '{file_path}'. " + f"Available vars: {', '.join(sorted(effective_vars)) or 'none'}" + ) + + def substitute_sql_vars_with_spans( *, sql: str, @@ -439,8 +486,9 @@ def _render_interpolation_token( var_name: str = sql[token_start:name_end] if var_name not in effective_vars: raise CompileInputError( - f"unknown project variable '@@{var_name}' in '{file_path}'. " - f"Available vars: {', '.join(sorted(effective_vars)) or 'none'}" + _unknown_project_variable_message( + var_name=var_name, file_path=file_path, effective_vars=effective_vars + ) ) try: return render_project_var_text( diff --git a/src/sqlbuild/compiler/model_config/constants.py b/src/sqlbuild/compiler/model_config/constants.py index c770c0fc2a..8d2f14051b 100644 --- a/src/sqlbuild/compiler/model_config/constants.py +++ b/src/sqlbuild/compiler/model_config/constants.py @@ -30,3 +30,4 @@ ENVIRONMENT_READ: str = "env" COLUMNS_HEADER_KEY: str = "columns" AUDITS_HEADER_KEY: str = "audits" +NATIVE_TEMPLATE_REJECTION_LENGTH: int = 3 diff --git a/src/sqlbuild/compiler/model_config/main/_expand_native_config_templates.py b/src/sqlbuild/compiler/model_config/main/_expand_native_config_templates.py index 2b657cc936..33abc06c47 100644 --- a/src/sqlbuild/compiler/model_config/main/_expand_native_config_templates.py +++ b/src/sqlbuild/compiler/model_config/main/_expand_native_config_templates.py @@ -3,11 +3,16 @@ from __future__ import annotations import os +from typing import cast import sqlbuild._native as _native -from sqlbuild.compiler.model_config.constants import UNSUPPORTED_OUTCOME +from sqlbuild.compiler.model_config.constants import ( + NATIVE_TEMPLATE_REJECTION_LENGTH, + UNSUPPORTED_OUTCOME, +) from sqlbuild.compiler.model_config.models import ( NativeTemplateExpansion, + NativeTemplateRejection, TemplateResolutionFlags, ) @@ -18,17 +23,33 @@ def expand_native_config_templates( variables: dict[str, object], context_values: dict[str, str | None], flags: TemplateResolutionFlags, -) -> NativeTemplateExpansion | str: - """Return the expansion, or `invalid` (Python raises) or `unsupported` (Python must run).""" + context_label: str | None = None, +) -> NativeTemplateExpansion | NativeTemplateRejection | str: + """Return the expansion, a rejection (exact with `context_label`), or `unsupported`.""" try: - outcome: tuple[object, list[tuple[str, str]]] | str = _native.expand_config_templates( + outcome: ( + tuple[object, list[tuple[str, str]]] | tuple[str, str, list[tuple[str, str]]] | str + ) = _native.expand_config_templates( value, (variables, os.environ, context_values), - (flags.allow_context, flags.preserve_context_tokens, flags.preserve_unknown_context), + ( + flags.allow_context, + flags.preserve_context_tokens, + flags.preserve_unknown_context, + context_label, + ), ) except TypeError: return UNSUPPORTED_OUTCOME if isinstance(outcome, str): return outcome - return NativeTemplateExpansion(value=outcome[0], reads=tuple(outcome[1])) + if len(outcome) == NATIVE_TEMPLATE_REJECTION_LENGTH: + rejection: tuple[str, str, list[tuple[str, str]]] = cast( + tuple[str, str, list[tuple[str, str]]], outcome + ) + return NativeTemplateRejection(message=rejection[1], reads=tuple(rejection[2])) + expansion: tuple[object, list[tuple[str, str]]] = cast( + tuple[object, list[tuple[str, str]]], outcome + ) + return NativeTemplateExpansion(value=expansion[0], reads=tuple(expansion[1])) diff --git a/src/sqlbuild/compiler/model_config/models.py b/src/sqlbuild/compiler/model_config/models.py index 4e53ab4450..795ca9afdc 100644 --- a/src/sqlbuild/compiler/model_config/models.py +++ b/src/sqlbuild/compiler/model_config/models.py @@ -42,6 +42,14 @@ class NativeTemplateExpansion: reads: tuple[tuple[str, str], ...] +@dataclass(frozen=True, slots=True) +class NativeTemplateRejection: + """Python's exact template error for a config value and the names read before it, in order.""" + + message: str + reads: tuple[tuple[str, str], ...] + + @dataclass(frozen=True, slots=True) class TemplateResolutionFlags: """How one template expansion treats `CTX:` references.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/__init__.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/_test_types.py new file mode 100644 index 0000000000..4b42c4ca3f --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/_test_types.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class NativeAttachmentErrorTestCase: + """One failing attachment project compiled by every engine through the CLI.""" + + description: str + case_name: str + expected_macro_calls: tuple[int, int, int] + expected_error_types: tuple[str, str, str] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py new file mode 100644 index 0000000000..a726f9e503 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/attachment_errors/test_native_attachment_errors.py @@ -0,0 +1,148 @@ +"""Every engine reports attachment errors identically; bridge-independent ones never re-run.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from scripts.compiler_differential._helpers.corpus.attachment_failure_cases import ( + attachment_failure_cases, +) +from scripts.compiler_differential.constants import FAILURE_BASE_MART, FAILURE_MART_PATH +from scripts.compiler_differential.models import FailureCase +from tests.e2e.src.sqlbuild.cli.commands.main.compile.attachment_errors._test_types import ( + NativeAttachmentErrorTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + CompileReuseRun, + lifecycle_error_type, + report_without_engine, + run_reuse_compile, + stderr_without_durations, + write_project_file, +) + +_ENGINES: tuple[str, ...] = ("python", "native", "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" +) +_ERROR_TYPES: tuple[str, str, str] = ("CompileInputError", "CompileInputError", "CompileInputError") +_CASES: dict[str, FailureCase] = {case.name: case for case in attachment_failure_cases()} + + +@pytest.mark.parametrize( + "test_case", + [ + NativeAttachmentErrorTestCase( + description="an unknown audit run scope", + case_name="generic-audit-unknown-run-scope", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="an audit argument overriding the implicit model", + case_name="generic-audit-overrides-implicit-model", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="a cursor intrinsic in a generic audit re-runs as it reads expanded SQL", + case_name="generic-audit-cursor-intrinsic", + expected_macro_calls=(1, 1, 2), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="an unknown project variable in audit SQL", + case_name="audit-unknown-project-variable", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="an unknown project variable inside doubled backticks in audit SQL", + case_name="audit-variable-in-doubled-backticks", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="a SQL function without returns", + case_name="sql-function-missing-returns", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="an unsupported template namespace in a function header", + case_name="sql-function-template-namespace", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + NativeAttachmentErrorTestCase( + description="an unknown variable in a source description", + case_name="source-description-unknown-variable", + expected_macro_calls=(1, 1, 1), + expected_error_types=_ERROR_TYPES, + ), + ], + ids=lambda case: case.description, +) +def test_given_attachment_error_when_compiling_with_each_engine_then_errors_and_types_match( + test_case: NativeAttachmentErrorTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + case: FailureCase = _CASES[test_case.case_name] + runs: list[CompileReuseRun] = [] + macro_calls: list[int] = [] + error_types: list[str] = [] + project_dir: Path = tmp_path / "orders" + files: dict[str, str] = { + **case.files, + "macros/counted.py": _COUNTED_MACRO, + FAILURE_MART_PATH: FAILURE_BASE_MART.replace("SUM(amount)", "SUM(@counted('amount'))"), + _CALL_LOG: "", + } + for engine in _ENGINES: + shutil.rmtree(project_dir, ignore_errors=True) + for relative_path, contents in files.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())) + error_types.append( + lifecycle_error_type(project_dir=project_dir, engine=engine, monkeypatch=monkeypatch) + ) + + assert ( + tuple(run.returncode for run in runs), + {report_without_engine(run) for run in runs[1:]} == {report_without_engine(runs[0])}, + {stderr_without_durations(stderr=run.stderr) for run in runs[1:]} + == {stderr_without_durations(stderr=runs[0].stderr)}, + f"[{case.expected_code}]" in runs[0].report + runs[0].stderr, + str(case.expected_message) in runs[0].report + runs[0].stderr, + tuple(macro_calls), + tuple(error_types), + ) == ( + (1, 1, 1), + True, + True, + True, + True, + test_case.expected_macro_calls, + test_case.expected_error_types, + ), ( + runs[0].report, + runs[2].report, + runs[2].stderr, + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/attachments/_test_types.py b/tests/integration/src/sqlbuild/compiler/attachments/_test_types.py index 7e47c866a5..8dd34cf57c 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/_test_types.py @@ -17,6 +17,7 @@ class AuthoredSqlParityTestCase: generate: Callable[[random.Random], str] expected_minimum_expanded: int expected_minimum_python_errors: int + expected_minimum_exact_errors: int @dataclass(frozen=True) @@ -27,6 +28,7 @@ class AttachedAuditParityTestCase: seed: int count: int expected_minimum_native: int + expected_minimum_native_errors: int expected_minimum_deferred: int expected_minimum_python_errors: int @@ -43,12 +45,13 @@ class AttachmentProjectTestCase: @dataclass(frozen=True) class CursorIntrinsicParityTestCase: - """Seeded SQL checked for cursor intrinsics natively and by Python.""" + """Seeded SQL checked for cursor intrinsics natively and by Python, with the native errors.""" description: str seed: int count: int expected_minimum_free: int + expected_minimum_native_errors: int expected_minimum_python_errors: int @@ -61,6 +64,7 @@ class ParameterParityTestCase: count: int expected_minimum_expanded: int expected_minimum_python_errors: int + expected_minimum_exact_errors: int @dataclass(frozen=True) @@ -120,6 +124,7 @@ class FunctionHeaderParityTestCase: inherit_default_namespace: bool expected_minimum_attached: int expected_minimum_python_errors: int + expected_minimum_exact_errors: int @dataclass(frozen=True) diff --git a/tests/integration/src/sqlbuild/compiler/attachments/helpers.py b/tests/integration/src/sqlbuild/compiler/attachments/helpers.py index a084961c28..0d71605bc9 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/helpers.py @@ -150,6 +150,11 @@ def generated_authored_sql(*, rng: random.Random) -> str: "@@ENV:SQB_ORDERS_REGION", '@const("sales_cap")', '@enum("order_status").PLACED', + "`", + "``", + "`@@missing``", + "`@@regions``", + "`@@CTX:run.target``", ) @@ -164,6 +169,19 @@ def generated_dollar_authored_sql(*, rng: random.Random) -> str: return "".join(parts) +class ExactErrorText(str): + """The text of an error marked bridge-independent, equal to Python's text.""" + + __slots__ = () + + +def error_text(error: CompileInputError) -> str: + """The error's text, marked when the error is bridge-independent.""" + + marker: type[str] = {True: ExactErrorText, False: str}[error.bridge_independent] + return marker(error) + + def authored_outcome( *, sql: str, engine: CompilerEngine, monkeypatch: pytest.MonkeyPatch ) -> AuthoredSqlExpansionResult | str: @@ -183,7 +201,7 @@ def authored_outcome( declarations=DECLARATIONS, ) except CompileInputError as error: - return str(error) + return error_text(error) _SQL_PIECES: tuple[str, ...] = ( @@ -285,6 +303,7 @@ def audit_parity(audit: GeneratedAudit) -> AuditParity: audit=audit, python=_python_rendering(audit), native=render_native_attached_audit( + labels=("models/orders.sql", "floor"), sql_body=audit.sql_body, evidence_sql=audit.evidence_sql, implicit_arguments=audit.implicit_arguments, @@ -309,9 +328,14 @@ def is_native(parity: AuditParity) -> bool: return parity.native is not None -def native_outcome(parity: AuditParity) -> tuple[object, ...]: - """The native rendering spelled like Python's, with the run scope value it selects.""" +def native_outcome(parity: AuditParity) -> tuple[object, ...] | str: + """The native rendering or error spelled like Python's, with the run scope it selects.""" + + rendered: NativeRenderedAudit = cast(NativeRenderedAudit, parity.native) + return rendered.render_error or rendered.policy_error or _native_rendering(parity) + +def _native_rendering(parity: AuditParity) -> tuple[object, ...]: rendered: NativeRenderedAudit = cast(NativeRenderedAudit, parity.native) run_scope: str | None = { "instance": parity.audit.instance_run_scope, @@ -338,7 +362,7 @@ def _python_rendering(audit: GeneratedAudit) -> tuple[object, ...] | str: severity: AuditSeverity = resolve_audit_severity( instance_severity=audit.instance_severity, default_severity=audit.default_severity, - audit_label="floor", + audit_label=f"{owner} audit 'floor'", ) run_scope: str = resolve_audit_run_scope( instance_run_scope=audit.instance_run_scope, @@ -566,12 +590,13 @@ def python_intrinsic_outcome(sql: str) -> str | None: return None -def native_intrinsic_free(sql: str) -> bool: - """Whether the native check accepts the SQL without Python.""" +def native_intrinsic_outcome(sql: str) -> tuple[bool, str | None]: + """Whether the native check answers without Python, and its rejection message if any.""" - return _native.sql_free_of_cursor_intrinsics( - sql, [MICROBATCH_START_SENTINEL, MICROBATCH_END_SENTINEL] + free, error = _native.sql_free_of_cursor_intrinsics( + sql, [MICROBATCH_START_SENTINEL, MICROBATCH_END_SENTINEL], "Source expression 'orders'" ) + return free or error is not None, error _PARAMETER_PIECES: tuple[str, ...] = ( @@ -624,7 +649,7 @@ def parameter_outcome( case_name="north", ) except CompileInputError as error: - return str(error) + return error_text(error) _BODY_PIECES: tuple[str, ...] = ( @@ -924,6 +949,12 @@ def target_validation_outcome( " ", "${coalesce(ENV:SQB_ATTACHMENTS_UNSET, 'DOUBLE')}", "${missing}", + "DECIMAL(${if(precision)})", + "${'open}", + "${eq(a, b, c)}", + "${upper(x)}", + "${coalesce()}", + "${CTX:model.name}", 2, ) _NAMES: tuple[object, ...] = ("raw_status", " amount ", "", " ", 1) @@ -931,6 +962,9 @@ def target_validation_outcome( _SCHEMAS: tuple[object, ...] = ( "udfs", "${coalesce(ENV:SQB_ATTACHMENTS_UNSET, 'udfs')}", + "${ENV:SQB_ATTACHMENTS_UNSET}", + "${VAR:udfs}", + "udfs_${x y}", 1, None, ) @@ -978,8 +1012,8 @@ def function_outcome( test_case: FunctionHeaderParityTestCase, engine: CompilerEngine, monkeypatch: pytest.MonkeyPatch, -) -> str: - """Attach one function under `engine`, returning its input or Python's error text.""" +) -> tuple[str, bool]: + """Attach one function under `engine`: its input or error, and whether that is bridge-free.""" monkeypatch.setenv(COMPILER_ENGINE_ENV_VAR, engine.value) sql_file: DiscoveredSqlFunctionFile = DiscoveredSqlFunctionFile( @@ -1027,9 +1061,9 @@ def function_outcome( no_sql_validation=True, python_functions_inherit_default_namespace=test_case.inherit_default_namespace, ) - ) + ), False except CompileInputError as error: - return f"error: {error}" + return f"error: {error}", error.bridge_independent _MACRO_PIECES: tuple[str, ...] = ( diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_audit_rendering.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_audit_rendering.py index c2bd59ba97..911cd23907 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_audit_rendering.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_audit_rendering.py @@ -1,4 +1,4 @@ -"""Natively rendered attached audits match Python's argument merge, SQL and policies.""" +"""Natively rendered attached audits match Python's argument merge, SQL, policies and errors.""" from __future__ import annotations @@ -26,7 +26,8 @@ description="seeded parameters, argument values, overrides and policies", seed=20261008, count=5000, - expected_minimum_native=800, + expected_minimum_native=1500, + expected_minimum_native_errors=600, expected_minimum_deferred=2000, expected_minimum_python_errors=1000, ), @@ -50,10 +51,12 @@ def test_given_generated_attachments_when_rendering_natively_then_python_renderi actual=list(map(native_outcome, answered)), ), len(answered) >= test_case.expected_minimum_native, + sum(isinstance(native_outcome(parity), str) for parity in answered) + >= test_case.expected_minimum_native_errors, len(parities) - len(answered) >= test_case.expected_minimum_deferred, sum(isinstance(parity.python, str) for parity in parities) >= test_case.expected_minimum_python_errors, - ) == ([], True, True, True), test_case.description + ) == ([], True, True, True, True), test_case.description if __name__ == "__main__": diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_authored_sql.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_authored_sql.py index cc8da56ad4..2c8a9cedac 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_authored_sql.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_authored_sql.py @@ -12,6 +12,7 @@ AuthoredSqlParityTestCase, ) from tests.integration.src.sqlbuild.compiler.attachments.helpers import ( + ExactErrorText, authored_outcome, generated_authored_sql, generated_dollar_authored_sql, @@ -29,6 +30,7 @@ generate=lambda rng: generated_authored_sql(rng=rng), expected_minimum_expanded=600, expected_minimum_python_errors=300, + expected_minimum_exact_errors=300, ), AuthoredSqlParityTestCase( description="dollar quotes around variables, enum and constant references", @@ -37,6 +39,7 @@ generate=lambda rng: generated_dollar_authored_sql(rng=rng), expected_minimum_expanded=300, expected_minimum_python_errors=300, + expected_minimum_exact_errors=600, ), ], ids=lambda case: case.description, @@ -61,7 +64,9 @@ def test_given_generated_authored_sql_when_expanding_with_preview_then_python_ou sum(isinstance(item, AuthoredSqlExpansionResult) for item in python) >= test_case.expected_minimum_expanded, sum(isinstance(item, str) for item in python) >= test_case.expected_minimum_python_errors, - ) == ([], True, True), test_case.description + sum(isinstance(item, ExactErrorText) for item in preview) + >= test_case.expected_minimum_exact_errors, + ) == ([], True, True, True), test_case.description if __name__ == "__main__": diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_cursor_intrinsics.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_cursor_intrinsics.py index 5276eb1a88..bbd9fd5eeb 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_cursor_intrinsics.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_cursor_intrinsics.py @@ -1,8 +1,9 @@ -"""The native cursor intrinsic check accepts only SQL that Python accepts.""" +"""The native cursor intrinsic check accepts and rejects SQL exactly as Python does.""" from __future__ import annotations import random +from itertools import compress import pytest @@ -11,7 +12,7 @@ ) from tests.integration.src.sqlbuild.compiler.attachments.helpers import ( generated_intrinsic_sql, - native_intrinsic_free, + native_intrinsic_outcome, python_intrinsic_outcome, ) from tests.integration.src.sqlbuild.compiler.helpers import mismatches @@ -25,29 +26,33 @@ seed=20261008, count=20000, expected_minimum_free=10000, + expected_minimum_native_errors=6000, expected_minimum_python_errors=8000, ), ], ids=lambda case: case.description, ) -def test_given_generated_sql_when_checking_natively_then_only_python_accepted_sql_is_free( +def test_given_generated_sql_when_checking_natively_then_python_outcome_matches( test_case: CursorIntrinsicParityTestCase, ) -> None: rng: random.Random = random.Random(test_case.seed) sqls: list[str] = [generated_intrinsic_sql(rng=rng) for _ in range(test_case.count)] - free: list[str] = list(filter(native_intrinsic_free, sqls)) + native: list[tuple[bool, str | None]] = list(map(native_intrinsic_outcome, sqls)) + answered: list[int] = list(compress(range(len(sqls)), [outcome[0] for outcome in native])) python: list[str | None] = list(map(python_intrinsic_outcome, sqls)) assert ( mismatches( - inputs=[*free], - expected=[None] * len(free), - actual=[*map(python_intrinsic_outcome, free)], + inputs=[sqls[index] for index in answered], + expected=[python[index] for index in answered], + actual=[native[index][1] for index in answered], ), - len(free) >= test_case.expected_minimum_free, + sum(native[index][1] is None for index in answered) >= test_case.expected_minimum_free, + sum(native[index][1] is not None for index in answered) + >= test_case.expected_minimum_native_errors, len(python) - python.count(None) >= test_case.expected_minimum_python_errors, - ) == ([], True, True), test_case.description + ) == ([], True, True, True), test_case.description if __name__ == "__main__": diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_function_headers.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_function_headers.py index 917182f63c..7917b74a0c 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_function_headers.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_function_headers.py @@ -26,8 +26,9 @@ count=1500, target_schema="prod", inherit_default_namespace=True, - expected_minimum_attached=150, + expected_minimum_attached=80, expected_minimum_python_errors=800, + expected_minimum_exact_errors=1200, ), FunctionHeaderParityTestCase( description="no target schema, Python functions keep their own namespace", @@ -35,8 +36,9 @@ count=1500, target_schema=None, inherit_default_namespace=False, - expected_minimum_attached=150, + expected_minimum_attached=80, expected_minimum_python_errors=800, + expected_minimum_exact_errors=1200, ), ], ids=lambda case: case.description, @@ -50,7 +52,7 @@ def test_given_generated_function_headers_when_attaching_with_preview_then_pytho for python in rng.choices((False, True), k=test_case.count) ] - python: list[str] = [ + python: list[tuple[str, bool]] = [ function_outcome( header_values=header, python=python_function, @@ -60,7 +62,7 @@ def test_given_generated_function_headers_when_attaching_with_preview_then_pytho ) for header, python_function in headers ] - preview: list[str] = [ + preview: list[tuple[str, bool]] = [ function_outcome( header_values=header, python=python_function, @@ -72,12 +74,17 @@ def test_given_generated_function_headers_when_attaching_with_preview_then_pytho ] assert ( - mismatches(inputs=[*headers], expected=[*python], actual=[*preview]), - sum(not item.startswith("error: ") for item in python) + mismatches( + inputs=[*headers], + expected=[text for text, _ in python], + actual=[text for text, _ in preview], + ), + sum(not text.startswith("error: ") for text, _ in python) >= test_case.expected_minimum_attached, - sum(item.startswith("error: ") for item in python) + sum(text.startswith("error: ") for text, _ in python) >= test_case.expected_minimum_python_errors, - ) == ([], True, True), test_case.description + sum(exact for _, exact in preview) >= test_case.expected_minimum_exact_errors, + ) == ([], True, True, True), test_case.description if __name__ == "__main__": diff --git a/tests/integration/src/sqlbuild/compiler/attachments/test_native_test_parameters.py b/tests/integration/src/sqlbuild/compiler/attachments/test_native_test_parameters.py index c28bf3f6c5..956e297a52 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/test_native_test_parameters.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/test_native_test_parameters.py @@ -11,6 +11,7 @@ ParameterParityTestCase, ) from tests.integration.src.sqlbuild.compiler.attachments.helpers import ( + ExactErrorText, generated_parameter_sql, parameter_outcome, ) @@ -26,6 +27,7 @@ count=3000, expected_minimum_expanded=800, expected_minimum_python_errors=800, + expected_minimum_exact_errors=1500, ), ], ids=lambda case: case.description, @@ -49,7 +51,9 @@ def test_given_generated_test_bodies_when_expanding_parameters_then_python_outpu mismatches(inputs=[*sqls], expected=python, actual=preview), sum(isinstance(item, tuple) for item in python) >= test_case.expected_minimum_expanded, sum(isinstance(item, str) for item in python) >= test_case.expected_minimum_python_errors, - ) == ([], True, True), test_case.description + sum(isinstance(item, ExactErrorText) for item in preview) + >= test_case.expected_minimum_exact_errors, + ) == ([], True, True, True), test_case.description if __name__ == "__main__": diff --git a/tests/integration/src/sqlbuild/compiler/model_config/helpers.py b/tests/integration/src/sqlbuild/compiler/model_config/helpers.py index ab41cad958..55a2a49cdc 100644 --- a/tests/integration/src/sqlbuild/compiler/model_config/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/model_config/helpers.py @@ -314,7 +314,7 @@ def _native_classification(*, value: object, flags: TemplateFlags) -> object: return _native.expand_config_templates( value, (TEMPLATE_VARIABLES, os.environ, TEMPLATE_CONTEXT), - (flags.allow_context, flags.preserve_context_tokens, flags.preserve_unknown_context), + (flags.allow_context, flags.preserve_context_tokens, flags.preserve_unknown_context, None), ) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py index 70a6f872a3..f8ed07c0ea 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py @@ -78,7 +78,9 @@ class StaticProjectVarDifferentialTestCase: seed: int sql_count: int effective_vars: dict[str, object] + fragments: tuple[str, ...] expected_minimum_dollar_quote_substitutions: int + expected_minimum_doubled_backtick_substitutions: int @dataclass(frozen=True) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/helpers.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/helpers.py index f051c38021..fc8eaeb908 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/helpers.py @@ -833,9 +833,10 @@ class StaticProjectVarDifferential: native: dict[str, str] python: dict[str, str | None] substituted_dollar_quotes: int + substituted_doubled_backticks: int -_LEXICAL_SQL_FRAGMENTS: tuple[str, ...] = ( +LEXICAL_SQL_FRAGMENTS: tuple[str, ...] = ( "$", "$$", "$t$", @@ -860,18 +861,32 @@ class StaticProjectVarDifferential: "_", "\u00e9", ) +BACKTICK_SQL_FRAGMENTS: tuple[str, ...] = ( + "`", + "``", + "'", + "''", + '"', + "@@revision", + "@@status", + "@@", + " ", + "x", + "--", + "\n", +) _LEXICAL_SQL_MAX_FRAGMENTS: int = 12 _DIFFERENTIAL_FILE_PATH: Path = Path("models/generated.sql") -def generated_lexical_sqls(*, seed: int, count: int) -> tuple[str, ...]: +def generated_lexical_sqls(*, seed: int, count: int, fragments: tuple[str, ...]) -> tuple[str, ...]: """Generate SQL-like strings mixing quotes, dollar quotes, comments and variable tokens.""" rng: random.Random = random.Random(seed) sqls: list[str] = [] for _ in range(count): fragment_count: int = rng.randint(1, _LEXICAL_SQL_MAX_FRAGMENTS) - sqls.append("".join(rng.choices(_LEXICAL_SQL_FRAGMENTS, k=fragment_count))) + sqls.append("".join(rng.choices(fragments, k=fragment_count))) return tuple(sqls) @@ -896,6 +911,9 @@ def native_static_project_var_differential( substituted_dollar_quotes=sum( sql != rendered and "$$" in sql for sql, rendered in native.items() ), + substituted_doubled_backticks=sum( + sql != rendered and "``" in sql for sql, rendered in native.items() + ), ) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_vars.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_vars.py index 59ae9b53a1..ac84cca696 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_vars.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_vars.py @@ -18,6 +18,8 @@ SubstituteSqlVarsTestCase, ) from tests.unit.src.sqlbuild.compiler.compile._helpers.helpers import ( + BACKTICK_SQL_FRAGMENTS, + LEXICAL_SQL_FRAGMENTS, StaticProjectVarDifferential, generated_lexical_sqls, native_static_project_var_differential, @@ -292,25 +294,40 @@ def test_given_static_model_sql_when_batch_preparing_then_returns_safe_results_a seed=550, sql_count=4000, effective_vars={"revision": 7, "status": "ready"}, + fragments=LEXICAL_SQL_FRAGMENTS, expected_minimum_dollar_quote_substitutions=1, - ) + expected_minimum_doubled_backtick_substitutions=0, + ), + StaticProjectVarDifferentialTestCase( + description="generated doubled backticks beside quotes, comments and tokens", + seed=1038, + sql_count=4000, + effective_vars={"revision": 7, "status": "ready"}, + fragments=BACKTICK_SQL_FRAGMENTS, + expected_minimum_dollar_quote_substitutions=0, + expected_minimum_doubled_backtick_substitutions=200, + ), ], ids=lambda case: case.description, ) def test_given_generated_lexical_sql_when_batch_preparing_then_native_matches_python_scanner( test_case: StaticProjectVarDifferentialTestCase, ) -> None: - sqls: tuple[str, ...] = generated_lexical_sqls(seed=test_case.seed, count=test_case.sql_count) + sqls: tuple[str, ...] = generated_lexical_sqls( + seed=test_case.seed, count=test_case.sql_count, fragments=test_case.fragments + ) differential: StaticProjectVarDifferential = native_static_project_var_differential( sqls=sqls, effective_vars=test_case.effective_vars ) - assert differential.native == differential.python assert ( + differential.native == differential.python, differential.substituted_dollar_quotes - >= test_case.expected_minimum_dollar_quote_substitutions - ) + >= test_case.expected_minimum_dollar_quote_substitutions, + differential.substituted_doubled_backticks + >= test_case.expected_minimum_doubled_backtick_substitutions, + ) == (True, True, True), test_case.description if __name__ == "__main__":