From bc86f064ed2c012bdf0b95f3b54b43447777cc40 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:16:43 +0000 Subject: [PATCH 01/14] feat(lint): warn when SQL lint is unavailable for a dialect A native lint response can report that the engine does not parse the project's dialect yet. Each such file now gets one L007 warning carrying the engine's reason, instead of looking lint-clean. A non-string reason is rejected as an invalid response. --- .../src/lint_text/_helpers/sql_diagnostics.rs | 79 +++++++++++++++++-- .../sqlbuild-rules/src/lint_text/constants.rs | 6 ++ .../src/lint_text/main/lint_sql_bodies.rs | 35 +++----- .../src/lint_text/tests/test_lint_text.rs | 72 ++++++++++++++++- .../src/lint_text/tests/test_types.rs | 10 +++ 5 files changed, 171 insertions(+), 31 deletions(-) diff --git a/crates/sqlbuild-rules/src/lint_text/_helpers/sql_diagnostics.rs b/crates/sqlbuild-rules/src/lint_text/_helpers/sql_diagnostics.rs index d7ddf43c3c..a2d51c33d3 100644 --- a/crates/sqlbuild-rules/src/lint_text/_helpers/sql_diagnostics.rs +++ b/crates/sqlbuild-rules/src/lint_text/_helpers/sql_diagnostics.rs @@ -1,6 +1,7 @@ //! Native SQL lint responses mapped back onto authored source through expansion passes. use std::borrow::Cow; +use std::collections::HashSet; use std::sync::LazyLock; use regex::Regex; @@ -10,7 +11,8 @@ use crate::lint_text::_helpers::sites::backtick_identifiers; use crate::lint_text::_helpers::sites::{byte_offset, compiled, lint_policy, site_end}; use crate::lint_text::constants::{ GENERATED_FIX, GENERATED_SQL_SUFFIX, HARNESS_CTE_PREFIXES, INVALID_DIAGNOSTIC, - INVALID_FIX_EDIT, INVALID_FIX_OBJECT, INVALID_FIX_REASON, LINT_API_VERSION, LONG_LITERAL_CODE, + INVALID_FIX_EDIT, INVALID_FIX_OBJECT, INVALID_FIX_REASON, INVALID_UNAVAILABLE, + LINT_API_VERSION, LINT_UNAVAILABLE_CODE, LINT_UNAVAILABLE_REMEDIATION, LONG_LITERAL_CODE, MACRO_MIXED_QUALIFICATION_REMEDIATION, MISSING_DIAGNOSTICS, MIXED_QUALIFICATION_CODE, NATIVE_LINT_ERROR_CODE, NON_CONTIGUOUS_FIX, NON_OBJECT_RESPONSE, NOT_AN_OBJECT, PARSE_ERROR_POSITION_PATTERN, PARSE_FAILURE_PREFIX, PARSE_FAILURE_REMEDIATION, @@ -109,8 +111,15 @@ pub(crate) fn map_offset(offset: i64, passes: &[Vec]) -> (i64, bo (current, generated) } -/// The diagnostics list of one engine response, or the error it reports. -pub(crate) fn response_diagnostics(response: &Value) -> Result, String> { +/// The diagnostics list of one engine response, or why its dialect could not be linted. +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum LintResult { + Diagnostics(Vec), + Unavailable(String), +} + +/// The result of one engine response, or the error it reports. +pub(crate) fn response_diagnostics(response: &Value) -> Result { let Some(payload) = response.as_object() else { return Err(NON_OBJECT_RESPONSE.to_string()); }; @@ -123,9 +132,67 @@ pub(crate) fn response_diagnostics(response: &Value) -> Result, Strin if integer(result.get("version")) != Some(LINT_API_VERSION) { return Err(UNSUPPORTED_VERSION.to_string()); } - match result.get("diagnostics") { - Some(Value::Array(diagnostics)) => Ok(diagnostics.clone()), - _ => Err(MISSING_DIAGNOSTICS.to_string()), + match (result.get("unavailable"), result.get("diagnostics")) { + (Some(Value::String(reason)), _) => Ok(LintResult::Unavailable(reason.clone())), + (Some(_), _) => Err(INVALID_UNAVAILABLE.to_string()), + (None, Some(Value::Array(diagnostics))) => Ok(LintResult::Diagnostics(diagnostics.clone())), + (None, _) => Err(MISSING_DIAGNOSTICS.to_string()), + } +} + +/// One body's authored diagnostics from its engine response, or the warning that it was not linted. +pub(crate) fn body_diagnostics( + response: &Result, + (body, omitted): (&LintSqlBody, Option), + file: &AuthoredFile<'_>, +) -> Vec { + let raw: Vec = match response { + Ok(LintResult::Diagnostics(raw)) => raw.clone(), + Ok(LintResult::Unavailable(reason)) => { + return vec![unavailable_warning(reason, body, file)]; + } + Err(message) => return vec![parse_failure(message, body, file)], + }; + without_generated_frame(raw, body, omitted) + .iter() + .filter_map(|item| match authored_diagnostic(item, body, file) { + Ok(diagnostic) => diagnostic, + Err(message) => Some(parse_failure(&message, body, file)), + }) + .collect() +} + +/// The diagnostics with only each file's first lint-unavailable warning. +pub(crate) fn unavailable_once_per_file( + diagnostics: Vec, +) -> Vec { + let mut warned: HashSet = HashSet::new(); + diagnostics + .into_iter() + .filter(|diagnostic| { + diagnostic.code != LINT_UNAVAILABLE_CODE || warned.insert(diagnostic.file) + }) + .collect() +} + +/// The warning that a body's SQL was not linted because the engine cannot parse its dialect. +fn unavailable_warning( + reason: &str, + body: &LintSqlBody, + file: &AuthoredFile<'_>, +) -> AuthoredDiagnostic { + let (line, column): (i64, i64) = file.position(body.body_start); + AuthoredDiagnostic { + file: body.file, + line, + column, + code: LINT_UNAVAILABLE_CODE.to_string(), + message: reason.to_string(), + fault: false, + end: None, + remediation: LINT_UNAVAILABLE_REMEDIATION.to_string(), + fix: None, + fix_unavailable_reason: None, } } diff --git a/crates/sqlbuild-rules/src/lint_text/constants.rs b/crates/sqlbuild-rules/src/lint_text/constants.rs index ea4e21a0bf..adc92a97cb 100644 --- a/crates/sqlbuild-rules/src/lint_text/constants.rs +++ b/crates/sqlbuild-rules/src/lint_text/constants.rs @@ -106,6 +106,10 @@ pub(crate) const CTE_ARGUMENT: &str = "__cte"; /// The help for mixed qualification in a SQL macro body, where slot reads are qualified through an alias. pub(crate) const MACRO_MIXED_QUALIFICATION_REMEDIATION: &str = "Use one consistent qualification style in this query scope. A plain slot read such as `@rel.column` counts as unqualified; to qualify it, read through an alias: `FROM @rel AS s`, then `s.@rel.column`."; pub(crate) const NATIVE_LINT_ERROR_CODE: &str = "L003"; +/// The warning code for SQL the native lint engine cannot parse in the project's dialect. +pub(crate) const LINT_UNAVAILABLE_CODE: &str = "L007"; +pub(crate) const LINT_UNAVAILABLE_REMEDIATION: &str = + "This SQL was not linted. Review it by hand until SQL lint supports this dialect."; pub(crate) const PARSE_ERROR_POSITION_PATTERN: &str = r"^Parse error at line (?P\d+), column (?P\d+):"; pub(crate) const HARNESS_CTE_PREFIXES: [&str; 13] = [ @@ -152,6 +156,8 @@ pub(crate) const NOT_AN_OBJECT: &str = "native lint diagnostic must be an object pub(crate) const NON_OBJECT_RESPONSE: &str = "native lint engine returned a non-object response"; pub(crate) const UNSUPPORTED_VERSION: &str = "native lint engine returned an unsupported response version"; +pub(crate) const INVALID_UNAVAILABLE: &str = + "native lint response has a non-string unavailable reason"; pub(crate) const MISSING_DIAGNOSTICS: &str = "native lint response is missing a diagnostics list"; pub(crate) const INVALID_BATCH: &str = "native lint engine returned an invalid batch response"; pub(crate) const NON_CONTIGUOUS_FIX: &str = diff --git a/crates/sqlbuild-rules/src/lint_text/main/lint_sql_bodies.rs b/crates/sqlbuild-rules/src/lint_text/main/lint_sql_bodies.rs index be43f45778..5eefd1e8f0 100644 --- a/crates/sqlbuild-rules/src/lint_text/main/lint_sql_bodies.rs +++ b/crates/sqlbuild-rules/src/lint_text/main/lint_sql_bodies.rs @@ -5,12 +5,12 @@ use std::collections::HashMap; use serde_json::Value; use crate::lint_text::_helpers::sql_diagnostics::{ - AuthoredFile, authored_diagnostic, parse_failure, response_diagnostics, without_generated_frame, + AuthoredFile, LintResult, body_diagnostics, response_diagnostics, unavailable_once_per_file, }; use crate::lint_text::_helpers::sql_requests::{lint_payload, omitted_select, run_batch}; use crate::lint_text::models::{AuthoredDiagnostic, LintSqlBody, LintSqlOptions}; -/// Lint every body once per distinct request and map each diagnostic onto its authored file. +/// Lint each distinct request once and map its diagnostics, or one unavailable warning per file, onto source. pub fn lint_sql_bodies( bodies: &[LintSqlBody], files: &[&str], @@ -30,31 +30,20 @@ pub fn lint_sql_bodies( } request_of_body.push(index); } - let responses: Vec, String>> = run_batch(&payloads) + let responses: Vec> = run_batch(&payloads) .map(|responses| responses.iter().map(response_diagnostics).collect()) .unwrap_or_else(|error| vec![Err(error); payloads.len()]); let authored: Vec> = files .iter() .map(|text| AuthoredFile::new(text, &options.dialect)) .collect(); - let mut diagnostics: Vec = Vec::new(); - for ((body, offset), request) in bodies.iter().zip(omitted).zip(request_of_body) { - let file: &AuthoredFile<'_> = &authored[body.file]; - let raw: Vec = match &responses[request] { - Ok(raw) => raw.clone(), - Err(message) => { - diagnostics.push(parse_failure(message, body, file)); - continue; - } - }; - let raw: Vec = without_generated_frame(raw, body, offset); - for item in &raw { - match authored_diagnostic(item, body, file) { - Ok(Some(diagnostic)) => diagnostics.push(diagnostic), - Ok(None) => {} - Err(message) => diagnostics.push(parse_failure(&message, body, file)), - } - } - } - diagnostics + let diagnostics: Vec = bodies + .iter() + .zip(omitted) + .zip(request_of_body) + .flat_map(|((body, offset), request)| { + body_diagnostics(&responses[request], (body, offset), &authored[body.file]) + }) + .collect(); + unavailable_once_per_file(diagnostics) } diff --git a/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs b/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs index 454ce07874..e6793bebb5 100644 --- a/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs +++ b/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs @@ -1,6 +1,9 @@ use crate::lint_text::_helpers::format_bodies::unwrapped_expression; use crate::lint_text::_helpers::sites::site_pattern; -use crate::lint_text::_helpers::sql_diagnostics::interpolation_text_at; +use crate::lint_text::_helpers::sql_diagnostics::{ + AuthoredFile, body_diagnostics, interpolation_text_at, response_diagnostics, + unavailable_once_per_file, +}; use crate::lint_text::constants::SITE_START_BYTES; use crate::lint_text::errors::LintTextError; use crate::lint_text::main::format_sql_bodies::format_sql_bodies; @@ -18,7 +21,7 @@ use crate::lint_text::tests::helpers::directive_finding; use crate::lint_text::tests::test_types::{ Finding, FormatBodiesTestCase, LintBodiesTestCase, NeutralizeTestCase, ParameterTestCase, RestoreTestCase, SitePatternTestCase, SlotReadTestCase, SuppressionTestCase, TokenAtTestCase, - UnwrappedExpressionTestCase, + UnavailableFinding, UnavailableLintTestCase, UnwrappedExpressionTestCase, }; use sqlbuild_analysis::sql_macros::main::parse_sql_macro::parse_sql_macro; @@ -666,3 +669,68 @@ fn given_sql_macro_bodies_when_linting_then_frames_and_generated_comments_are_no ); } } + +#[test] +fn given_engine_response_when_dialect_is_unavailable_then_each_file_gets_one_warning() { + const REASON: &str = + "SQL lint is unavailable for the tsql dialect: SQLQuack does not parse it yet"; + let test_cases = [ + UnavailableLintTestCase { + description: "an unavailable reason warns once at the file's first body", + response: serde_json::json!({"Ok": {"version": 1, "diagnostics": [], "unavailable": REASON}}), + bodies_in_file: 2, + expected_findings: vec![("L007", 2, 1, false, REASON)], + }, + UnavailableLintTestCase { + description: "a linted response without the key keeps its empty diagnostics", + response: serde_json::json!({"Ok": {"version": 1, "diagnostics": []}}), + bodies_in_file: 2, + expected_findings: Vec::new(), + }, + UnavailableLintTestCase { + description: "a non-string reason is an invalid response, not a clean one", + response: serde_json::json!({"Ok": {"version": 1, "diagnostics": [], "unavailable": null}}), + bodies_in_file: 1, + expected_findings: vec![( + "L003", + 2, + 1, + true, + "Native SQL parser could not analyze this body: native lint response has a non-string unavailable reason", + )], + }, + ]; + let file = AuthoredFile::new("MODEL ();\nselect a from t\n", "tsql"); + let body = LintSqlBody { + body_start: 10, + lint_text: "select a from t".to_string(), + ..LintSqlBody::default() + }; + + for test_case in test_cases { + let response = response_diagnostics(&test_case.response); + let diagnostics = unavailable_once_per_file( + (0..test_case.bodies_in_file) + .flat_map(|_| body_diagnostics(&response, (&body, None), &file)) + .collect(), + ); + let findings: Vec> = diagnostics + .iter() + .map(|finding| { + ( + finding.code.as_str(), + finding.line, + finding.column, + finding.fault, + finding.message.as_str(), + ) + }) + .collect(); + + assert_eq!( + findings, test_case.expected_findings, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs b/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs index 60c28b778c..da78f48bc7 100644 --- a/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs @@ -56,6 +56,16 @@ pub(crate) struct LintBodiesTestCase { pub(crate) expected_findings: Vec>, } +pub(crate) struct UnavailableLintTestCase { + pub(crate) description: &'static str, + pub(crate) response: serde_json::Value, + pub(crate) bodies_in_file: usize, + pub(crate) expected_findings: Vec>, +} + +/// One authored finding: code, line, column, whether it is a fault, and its message. +pub(crate) type UnavailableFinding<'a> = (&'a str, i64, i64, bool, &'a str); + /// One mapped finding: code, line, column and optional end position. pub(crate) type Finding<'a> = (&'a str, i64, i64, Option<(i64, i64)>); From 7edf70df8b270b874363bb3ef9635e193ccd7303 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 18:34:13 +0000 Subject: [PATCH 02/14] feat(lint): keep L007 out of every exit decision and show it in compile and rules run compile and rules run kept only selected Rule codes, so an unavailable dialect looked clean there. L007 now passes through as a warning-severity Rule diagnostic in compile and is left out of rules run's failing-finding count. counts_toward_exit owns the exemption. --- .../_helpers/compile_driver/analysis.rs | 12 +++++-- .../bindings/_helpers/plan_command/rules.rs | 8 ++++- .../src/bindings/_helpers/rules/sql_rules.rs | 5 ++- .../src/lint_text/main/counts_toward_exit.rs | 8 +++++ .../sqlbuild-rules/src/lint_text/main/mod.rs | 1 + .../src/lint_text/tests/test_lint_text.rs | 36 +++++++++++++++++-- .../src/lint_text/tests/test_types.rs | 6 ++++ .../references/docs/concepts/rules.md | 5 +++ .../src/content/docs/docs/concepts/rules.mdx | 5 +++ 9 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 crates/sqlbuild-rules/src/lint_text/main/counts_toward_exit.rs diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs index eaabced465..e7fd6c7603 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs @@ -6,6 +6,7 @@ use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyRes use pyo3::types::{PyBool, PyDict, PyDictMethods, PyString, PyTuple, PyTupleMethods}; use pyo3::{IntoPyObject, pyfunction, wrap_pyfunction}; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_rules::lint_text::main::counts_toward_exit::counts_toward_exit; use crate::bindings::_helpers::boundary::python_calls::{ Status, call_with, imported, keywords, within, @@ -546,7 +547,7 @@ impl<'py> Driver<'_, 'py> { } } -/// The core diagnostics followed by one rule-phase error per rule finding. +/// The core diagnostics followed by one rule-phase error per rule finding; lint-unavailable ones only warn. fn with_rule_diagnostics<'py>( py: Python<'py>, core: &Bound<'py, PyTuple>, @@ -554,6 +555,7 @@ fn with_rule_diagnostics<'py>( ) -> PyResult> { let phase: Bound<'py, PyAny> = enum_member(py, RecordId::DiagnosticPhase, "RULE")?; let severity: Bound<'py, PyAny> = enum_member(py, RecordId::DiagnosticSeverity, "ERROR")?; + let unavailable: Bound<'py, PyAny> = enum_member(py, RecordId::DiagnosticSeverity, "WARNING")?; let fix_note: Bound<'py, PyAny> = PyString::new(py, RULE_FIX_AVAILABLE_NOTE).into_any(); let mut diagnostics: Vec> = core.iter().collect(); for fault in native_field(rules_result, "findings")?.try_iter()? { @@ -568,7 +570,13 @@ fn with_rule_diagnostics<'py>( RecordId::CompilerDiagnostic, &[ ("phase", phase.clone()), - ("severity", severity.clone()), + ( + "severity", + match counts_toward_exit(&native_field(&fault, "code")?.extract::()?) { + true => severity.clone(), + false => unavailable.clone(), + }, + ), ("code", native_field(&fault, "code")?), ("message", native_field(&fault, "message")?), ("path", native_field(&fault, "path")?), diff --git a/crates/sqlbuild-python/src/bindings/_helpers/plan_command/rules.rs b/crates/sqlbuild-python/src/bindings/_helpers/plan_command/rules.rs index 19fc6051ec..36081f97c2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/plan_command/rules.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/plan_command/rules.rs @@ -5,6 +5,7 @@ use pyo3::prelude::{ }; use pyo3::types::{PyDict, PyDictMethods, PyTuple}; use pyo3::wrap_pyfunction; +use sqlbuild_rules::lint_text::main::counts_toward_exit::counts_toward_exit; use sqlbuild_rules::rule_catalogue::main::render_rule_list::render_rule_list; use sqlbuild_rules::rule_catalogue::models::RuleListing; @@ -246,7 +247,12 @@ fn run_rule_selection( native_field(request, "json_output")?.is_truthy()?, )?; print_line(py, &text, false)?; - let findings: usize = native_field(&result, "findings")?.len()?; + let mut findings: usize = 0; + for finding in native_field(&result, "findings")?.try_iter()? { + if counts_toward_exit(&native_field(&finding?, "code")?.extract::()?) { + findings += 1; + } + } if findings > 0 { print_line( py, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_rules.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_rules.rs index e211ebcefa..a404881bf2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_rules.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_rules.rs @@ -13,6 +13,7 @@ use pyo3::prelude::{ use pyo3::types::{PyBytes, PyDict, PyDictMethods, PyString, PyTuple}; use pyo3::{IntoPyObjectExt, pyfunction, wrap_pyfunction}; use sqlbuild_core::text::main::python_splitlines::python_splitlines; +use sqlbuild_rules::lint_text::main::counts_toward_exit::counts_toward_exit; use sqlbuild_rules::rule_cache::main::file_rule_key::file_rule_key; use sqlbuild_rules::rule_results::main::duplicate_literal_hints::duplicate_literal_hints; use sqlbuild_rules::rule_results::models::RuleFinding; @@ -670,7 +671,7 @@ fn lint_expansion_context( }) } -/// The findings of the selected SQL rules' violations; an engine failure leaves each unevaluated. +/// The selected SQL rules' findings and lint-unavailable warnings; an engine failure leaves each rule unevaluated. fn violation_findings( run: &SqlRun<'_, '_>, violations: &[(PathBuf, LintViolationRow)], @@ -686,6 +687,8 @@ fn violation_findings( let code: &str = &violation.code; if code == SQL_RULE_SUPPRESSION_CODE || run.codes.iter().any(|known| known == code) { evaluated.push(lint_finding((path, violation), &resolved_dir, model_paths)); + } else if !counts_toward_exit(code) { + evaluated.push(lint_finding((path, violation), &resolved_dir, &no_paths)); } else if code == engine_error { let finding: RuleFinding = lint_finding((path, violation), &resolved_dir, &no_paths); for rule_code in run.codes { diff --git a/crates/sqlbuild-rules/src/lint_text/main/counts_toward_exit.rs b/crates/sqlbuild-rules/src/lint_text/main/counts_toward_exit.rs new file mode 100644 index 0000000000..6fcb69d909 --- /dev/null +++ b/crates/sqlbuild-rules/src/lint_text/main/counts_toward_exit.rs @@ -0,0 +1,8 @@ +//! `counts_toward_exit`: whether a lint finding can make a command fail. + +use crate::lint_text::constants::LINT_UNAVAILABLE_CODE; + +/// Whether a finding with `code` counts toward a command's exit status; SQL lint being unavailable for a dialect never does. +pub fn counts_toward_exit(code: &str) -> bool { + code != LINT_UNAVAILABLE_CODE +} diff --git a/crates/sqlbuild-rules/src/lint_text/main/mod.rs b/crates/sqlbuild-rules/src/lint_text/main/mod.rs index 3343ef6e90..b3640027a2 100644 --- a/crates/sqlbuild-rules/src/lint_text/main/mod.rs +++ b/crates/sqlbuild-rules/src/lint_text/main/mod.rs @@ -1,3 +1,4 @@ +pub mod counts_toward_exit; pub mod defined_cte_names; pub mod dependency_sites; pub mod first_header_only; diff --git a/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs b/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs index e6793bebb5..0142637b30 100644 --- a/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs +++ b/crates/sqlbuild-rules/src/lint_text/tests/test_lint_text.rs @@ -6,6 +6,7 @@ use crate::lint_text::_helpers::sql_diagnostics::{ }; use crate::lint_text::constants::SITE_START_BYTES; use crate::lint_text::errors::LintTextError; +use crate::lint_text::main::counts_toward_exit::counts_toward_exit; use crate::lint_text::main::format_sql_bodies::format_sql_bodies; use crate::lint_text::main::frame_sql_macro_expression::frame_sql_macro_expression; use crate::lint_text::main::lint_sql_bodies::lint_sql_bodies; @@ -19,9 +20,9 @@ use crate::lint_text::main::restore_interpolation::restore_interpolation; use crate::lint_text::models::{ExpansionSpan, FormatFile, LintSqlBody, LintSqlOptions}; use crate::lint_text::tests::helpers::directive_finding; use crate::lint_text::tests::test_types::{ - Finding, FormatBodiesTestCase, LintBodiesTestCase, NeutralizeTestCase, ParameterTestCase, - RestoreTestCase, SitePatternTestCase, SlotReadTestCase, SuppressionTestCase, TokenAtTestCase, - UnavailableFinding, UnavailableLintTestCase, UnwrappedExpressionTestCase, + ExitCountTestCase, Finding, FormatBodiesTestCase, LintBodiesTestCase, NeutralizeTestCase, + ParameterTestCase, RestoreTestCase, SitePatternTestCase, SlotReadTestCase, SuppressionTestCase, + TokenAtTestCase, UnavailableFinding, UnavailableLintTestCase, UnwrappedExpressionTestCase, }; use sqlbuild_analysis::sql_macros::main::parse_sql_macro::parse_sql_macro; @@ -734,3 +735,32 @@ fn given_engine_response_when_dialect_is_unavailable_then_each_file_gets_one_war ); } } + +#[test] +fn given_findings_when_deciding_exit_then_only_lint_unavailable_never_fails_a_command() { + let test_cases = [ + ExitCountTestCase { + description: "an unavailable-dialect warning reports our gap, not the user's SQL", + code: "L007", + expected_counts: false, + }, + ExitCountTestCase { + description: "a SQL lint rule finding still fails", + code: "SQLCLEAN005", + expected_counts: true, + }, + ExitCountTestCase { + description: "an engine fault still fails", + code: "L003", + expected_counts: true, + }, + ]; + for test_case in test_cases { + assert_eq!( + counts_toward_exit(test_case.code), + test_case.expected_counts, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs b/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs index da78f48bc7..94f883b554 100644 --- a/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/lint_text/tests/test_types.rs @@ -56,6 +56,12 @@ pub(crate) struct LintBodiesTestCase { pub(crate) expected_findings: Vec>, } +pub(crate) struct ExitCountTestCase { + pub(crate) description: &'static str, + pub(crate) code: &'static str, + pub(crate) expected_counts: bool, +} + pub(crate) struct UnavailableLintTestCase { pub(crate) description: &'static str, pub(crate) response: serde_json::Value, diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md index 09d9a86da7..aa7890c26d 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md @@ -144,6 +144,11 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. +If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile`, +`sqb rules run` and `sqb format` show one `L007` warning per file that names the dialect. `L007` +never changes an exit code: it reports a gap in SQLBuild's dialect support, not a problem in your +SQL, so the file's SQL Rules simply did not run. + Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: diff --git a/website/src/content/docs/docs/concepts/rules.mdx b/website/src/content/docs/docs/concepts/rules.mdx index 8f761a9ee0..0ea8a21f5c 100644 --- a/website/src/content/docs/docs/concepts/rules.mdx +++ b/website/src/content/docs/docs/concepts/rules.mdx @@ -132,6 +132,11 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. +If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile`, +`sqb rules run` and `sqb format` show one `L007` warning per file that names the dialect. `L007` +never changes an exit code: it reports a gap in SQLBuild's dialect support, not a problem in your +SQL, so the file's SQL Rules simply did not run. + Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: From 2fb45477f311e8defc7da3e134fee9f277f89e63 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 18:55:08 +0000 Subject: [PATCH 03/14] fix(lint): keep L007 out of rule exceptions and the plan rules gate The plan and build rules gate failed on any finding, so an L007-only project could not plan. Rule exceptions also validate every finding's code as a rule code and rejected L007 with an invalid-lifecycle error. The gate now counts only findings that count toward exit, and L007 passes through rule exceptions unsuppressed. An end-to-end test replays a cached finding as L007 and as a rule code through compile, rules run, plan and build. --- .../_helpers/compile_driver/plan_pipeline.rs | 14 ++++- .../src/engine/_helpers/evaluation.rs | 6 ++ .../cli/commands/main/rules/_test_types.py | 7 +++ .../main/rules/test_sql_quality_rules.py | 63 +++++++++++++++++++ 4 files changed, 87 insertions(+), 3 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/plan_pipeline.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/plan_pipeline.rs index 59da81f65f..f8340fe4d9 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/plan_pipeline.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/plan_pipeline.rs @@ -1,9 +1,10 @@ //! The compile-and-plan pipeline of execution commands: compile, rules gate, then planning. use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; -use pyo3::types::PyDict; +use pyo3::types::{PyDict, PyTuple}; use pyo3::{IntoPyObject, PyErr, pyfunction, wrap_pyfunction}; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_rules::lint_text::main::counts_toward_exit::counts_toward_exit; use sqlbuild_rules::rule_results::main::default_rules_notice::default_rules_notice; use crate::bindings::_helpers::boundary::python_calls::{KeywordArguments, keywords}; @@ -135,10 +136,17 @@ fn run_rules_gate<'py>( let message: String = rules_progress(elapsed, &result, true)?; on_progress.call1((message,))?; } - let findings: Bound<'py, PyAny> = native_field(&result, "findings")?; - if !findings.is_truthy()? { + let mut failing: Vec> = Vec::new(); + for finding in native_field(&result, "findings")?.try_iter()? { + let finding: Bound<'py, PyAny> = finding?; + if counts_toward_exit(&native_field(&finding, "code")?.extract::()?) { + failing.push(finding); + } + } + if failing.is_empty() { return Ok(()); } + let findings: Bound<'py, PyAny> = PyTuple::new(py, failing)?.into_any(); let summary: Bound<'py, PyAny> = new_record_with( py, RecordId::RulesResult, diff --git a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs index 64d11d4ead..d76c61a0cb 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs @@ -1,6 +1,7 @@ use crate::configuration::main::validate as config; use crate::constants::{API_VERSION, NATIVE_BUILD_IDENTITY, RELATION_EXCEPTION_RULES}; use crate::engine::_helpers::cache::{Cache, PendingBucketWrite, RuleCacheEntry}; +use crate::lint_text::constants::LINT_UNAVAILABLE_CODE; use crate::models::{ EvaluateRequest, EvaluateResponse, Fault, Model, RuleException, RuleMetadata, RuleScope, RulesCodeGrammar, RulesConfig, @@ -580,11 +581,15 @@ pub(crate) fn finalize_findings( apply_fault_policy(&request, evaluated_codes, findings) } +/// Apply rule exceptions to `faults`; lint-unavailable warnings report our gap, not a rule, so they pass through unsuppressed. fn apply_fault_policy( request: &EvaluateRequest, evaluated_codes: &[String], faults: Vec, ) -> Result, String> { + let (unavailable, faults): (Vec, Vec) = faults + .into_iter() + .partition(|fault| fault.code == LINT_UNAVAILABLE_CODE); let unevaluated: BTreeSet<_> = faults .iter() .filter(|fault| fault.unevaluated) @@ -641,6 +646,7 @@ fn apply_fault_policy( )); fault }) + .chain(unavailable) .collect()) } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py index 6e710ee77e..977b4079b0 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py @@ -179,6 +179,13 @@ class RankingSortDefaultCase: expected_codes: tuple[str, ...] +@dataclass(frozen=True) +class ReplayedFindingExitCase: + description: str + code: str + expected_exit: int + + @dataclass(frozen=True) class FixableReportingCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py index 09961d554d..a22ba9759c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py @@ -9,12 +9,14 @@ import pytest +from sqlbuild._native import rules_bulk_cache_path from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( ConstantLiteralCase, DuplicateLiteralCase, FixableReportingCase, RankingKeyProofCase, RankingSortDefaultCase, + ReplayedFindingExitCase, SqlQualityRuleCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( @@ -557,5 +559,66 @@ def test_given_repeated_long_literal_when_running_rules_then_help_names_the_shar ), result.stdout +@pytest.mark.parametrize( + "test_case", + ( + ReplayedFindingExitCase( + description="a SQL rule finding fails every command", + code="SQLCLEAN043", + expected_exit=1, + ), + ReplayedFindingExitCase( + description="a lint-unavailable warning fails no command", + code="L007", + expected_exit=0, + ), + ), + ids=lambda case: case.description, +) +def test_given_cached_sql_finding_when_running_commands_then_only_rule_findings_fail( + test_case: ReplayedFindingExitCase, tmp_path: Path +) -> None: + write_quality_project( + project_dir=tmp_path, + toml=( + 'name = "orders"\nadapter = "duckdb"\ndefault_target = "dev"\n\n' + '[connection]\ndatabase = "plan.duckdb"\n\n[targets.dev]\nschema = "analytics"\n\n' + '[rules]\nselect = ["SQLCLEAN043"]\n' + ), + upstream=_RAW_HEADER + _UPSTREAM_SQL, + summary=( + _SUMMARY_HEADER + "SELECT o.order_id, ROW_NUMBER() OVER (ORDER BY o.amount," + " o.status, o.note, o.customer_id, o.amount + 1, o.order_id) AS rn\n" + 'FROM __ref("raw_orders") AS o\n' + ), + ) + seeded: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "compile", "--json" + ) + assert diagnostic_codes(seeded) == ("SQLCLEAN043",), seeded.stdout + seeded.stderr + # Replay the cached finding under the case's code: the engine cannot emit L007 yet. + cache: Path = rules_bulk_cache_path(project_dir=tmp_path, file_name="sql.json") + stored: dict[str, Any] = json.loads(cache.read_text(encoding="utf-8")) + for entry in stored["entries"].values(): + for finding in entry["findings"]: + finding["code"] = test_case.code + _ = cache.write_text(json.dumps(stored), encoding="utf-8") + + compiled: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "compile", "--json" + ) + exits: tuple[int, ...] = ( + compiled.returncode, + run_unevaluated_rules_cli(tmp_path, "rules", "run", "SQLCLEAN043").returncode, + run_unevaluated_rules_cli(tmp_path, "plan").returncode, + run_unevaluated_rules_cli(tmp_path, "build").returncode, + ) + + assert (diagnostic_codes(compiled), exits) == ( + (test_case.code,), + (test_case.expected_exit,) * len(exits), + ), compiled.stdout + compiled.stderr + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) From 35724086051ecc7ad02691887c4bfdf99c11b3f8 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 19:11:26 +0000 Subject: [PATCH 04/14] docs(rules): name plan and build, not format, in the L007 exit note --- .../skills/sqlbuild/references/docs/concepts/rules.md | 8 ++++---- website/src/content/docs/docs/concepts/rules.mdx | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md index aa7890c26d..0b2aac11f9 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md @@ -144,10 +144,10 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. -If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile`, -`sqb rules run` and `sqb format` show one `L007` warning per file that names the dialect. `L007` -never changes an exit code: it reports a gap in SQLBuild's dialect support, not a problem in your -SQL, so the file's SQL Rules simply did not run. +If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile` +and `sqb rules run` show one `L007` warning per file that names the dialect. `L007` never changes +an exit code, including for `sqb plan` and `sqb build`: it reports a gap in SQLBuild's dialect +support, not a problem in your SQL, so the file's SQL Rules simply did not run. Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: diff --git a/website/src/content/docs/docs/concepts/rules.mdx b/website/src/content/docs/docs/concepts/rules.mdx index 0ea8a21f5c..884993b4a5 100644 --- a/website/src/content/docs/docs/concepts/rules.mdx +++ b/website/src/content/docs/docs/concepts/rules.mdx @@ -132,10 +132,10 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. -If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile`, -`sqb rules run` and `sqb format` show one `L007` warning per file that names the dialect. `L007` -never changes an exit code: it reports a gap in SQLBuild's dialect support, not a problem in your -SQL, so the file's SQL Rules simply did not run. +If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile` +and `sqb rules run` show one `L007` warning per file that names the dialect. `L007` never changes +an exit code, including for `sqb plan` and `sqb build`: it reports a gap in SQLBuild's dialect +support, not a problem in your SQL, so the file's SQL Rules simply did not run. Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: From 630b2d8a6ff192819bbb0bb2d808429b847a7ae7 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:21:51 +0000 Subject: [PATCH 05/14] wip: read rule facts with SQLQuack --- Cargo.lock | 2 +- crates/sqlbuild-rules/Cargo.toml | 2 +- .../src/query_facts/_helpers/facts.rs | 90 ++++++++++--------- .../src/query_facts/_helpers/tokens.rs | 66 ++++++-------- .../src/query_facts/constants.rs | 8 ++ .../sqlbuild-rules/src/query_facts/models.rs | 12 ++- .../src/rules/_helpers/sql_test_rules.rs | 5 +- 7 files changed, 99 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9f5f63d09..9e33888683 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4431,7 +4431,6 @@ dependencies = [ "blake3", "fensu-policy", "globset", - "polyglot-sql", "rayon", "regex", "rustc-hash", @@ -4445,6 +4444,7 @@ dependencies = [ "sqlbuild-scopes", "sqlbuild-sqltext", "sqlclean", + "sqlquack-syntax", "stacker", "tempfile", "toml", diff --git a/crates/sqlbuild-rules/Cargo.toml b/crates/sqlbuild-rules/Cargo.toml index 535ee3687a..1070397264 100644 --- a/crates/sqlbuild-rules/Cargo.toml +++ b/crates/sqlbuild-rules/Cargo.toml @@ -11,7 +11,7 @@ rust-version.workspace = true blake3.workspace = true fensu-policy.workspace = true globset.workspace = true -polyglot-sql.workspace = true +sqlquack-syntax.workspace = true rayon.workspace = true regex.workspace = true rustc-hash.workspace = true diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 5af9afca09..e2518efce7 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,33 +1,47 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - MAX_FUNCTION_CALL_DEPTH, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, + DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, + TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, ModelShape, ParsedFixture, QueryFacts, RelationFacts, SelectFacts, SetFacts, SourceText, }; -use polyglot_sql::dialects::Dialect; -use polyglot_sql::expressions::{Identifier, Literal, Select, TableRef, With}; -use polyglot_sql::{ComplexityGuardOptions, Expression, ExpressionWalk, ParseOptions}; -use sqlbuild_analysis::lineage::main::parser_dialect::parser_dialect; +use sqlquack_syntax::expressions::names::models::{Identifier, Literal, TableRef}; +use sqlquack_syntax::expressions::queries::models::{Select, With}; +use sqlquack_syntax::{CallMode, ComplexityGuardOptions, Dialect, Expression, ExpressionWalk, ParseOptions}; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; use std::ops::Range; +/// The SQLQuack grammar that reads `dialect_name` for built-in rules, or `None` when none does. +pub(crate) fn facts_dialect(dialect_name: &str) -> Option { + if DUCKDB_DIALECTS.contains(&dialect_name) { + return Some(Dialect::DuckDB); + } + if POSTGRES_DIALECTS.contains(&dialect_name) { + return Some(Dialect::PostgreSQL); + } + match dialect_name { + SNOWFLAKE_DIALECT => Some(Dialect::Snowflake), + SQLITE_DIALECT => Some(Dialect::SQLite), + _ => None, + } +} + fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), String> { - let dialect_type = parser_dialect(Some(dialect_name)) - .map_err(|unknown| format!("unknown dialect {unknown}"))?; - let dialect = Dialect::get(dialect_type); + let dialect = facts_dialect(dialect_name) + .ok_or_else(|| format!("no SQL parser for dialect {dialect_name}"))?; let options = ParseOptions { complexity_guard: Some(ComplexityGuardOptions { max_function_call_depth: Some(MAX_FUNCTION_CALL_DEPTH), ..Default::default() }), }; - let mut statements = dialect - .parse_with_options(sql, &options) - .map_err(|error| error.to_string())?; + let mut statements = + sqlquack_syntax::parse_statements_with(sql, dialect, &options, CallMode::FaithfulLocated) + .map_err(|error| error.to_string())?; if statements.len() != 1 { return Err("expected one statement".to_owned()); } @@ -38,16 +52,18 @@ fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), S Ok((statement, dialect)) } +fn code_tokens(sql: &str, dialect: Dialect) -> Result, String> { + let tokens = sqlquack_syntax::tokenize(sql, dialect).map_err(|error| error.to_string())?; + Ok(QueryTokens::new(tokens, sql)) +} + /// Parse a stubbed SQL-test fixture as one query. pub(crate) fn parse_fixture( shape: ModelShape, dialect_name: &str, ) -> Result { let (statement, dialect) = parse_query(&shape.sql, dialect_name)?; - let tokens = dialect - .tokenize(&shape.sql) - .map_err(|error| error.to_string())?; - let tokens = QueryTokens::new(tokens, &shape.sql); + let tokens = code_tokens(&shape.sql, dialect)?; let selects = tokens.selects(); let order = tokens.top_level_word("ORDER").unwrap_or(tokens.len()); let order_end = ["LIMIT", "OFFSET", "FETCH"] @@ -72,15 +88,13 @@ pub(crate) fn parse_fixture( }) } -/// Parse normalized model SQL with Polyglot and read the facts built-in rules evaluate. +/// Parse normalized model SQL with SQLQuack and read the facts built-in rules evaluate. pub(crate) fn query_facts(sql: &str, dialect_name: &str) -> Result { let (statement, dialect) = parse_query(sql, dialect_name)?; - let tokens = dialect.tokenize(sql).map_err(|error| error.to_string())?; - let tokens = QueryTokens::new(tokens, sql); + let tokens = code_tokens(sql, dialect)?; let builder = Builder { selects: tokens.selects(), tokens, - dialect, spans: RefCell::default(), cursor: Cell::default(), claimed: RefCell::default(), @@ -104,7 +118,6 @@ fn is_query(expression: &Expression) -> bool { struct Builder<'a> { tokens: QueryTokens<'a>, selects: Vec, - dialect: Dialect, spans: RefCell>>, cursor: Cell, claimed: RefCell>, @@ -303,18 +316,9 @@ impl Builder<'_> { let range = item.map(|item| self.tokens.expression_range(item, alias.is_some())); let unwrapped = unwrap_paren(value); let cast_type = match unwrapped { - Expression::Cast(cast) | Expression::TryCast(cast) | Expression::SafeCast(cast) => { - Some( - range - .as_ref() - .and_then(|range| self.tokens.cast_type(range)) - .unwrap_or_else(|| { - self.dialect - .generate(&Expression::DataType(cast.to.clone())) - .unwrap_or_default() - }), - ) - } + Expression::Cast(_) | Expression::TryCast(_) | Expression::SafeCast(_) => range + .as_ref() + .and_then(|range| self.tokens.cast_type(range)), _ => None, }; ItemFacts::Expression(ExpressionItem { @@ -361,15 +365,13 @@ impl Builder<'_> { if let Some(span) = self.spans.borrow().get(&key) { return *span; } - let anchors = anchors(parts); - let generated = if anchors == (None, None) { - self.dialect.generate(node).unwrap_or_default() - } else { - String::new() + let anchors = match anchors(parts) { + (None, None) => (node.source_span().map(|span| span.start), None), + anchors => anchors, }; let span = self .tokens - .comparison_range(parts.operator, anchors, parts.negated, &generated); + .comparison_range(parts.operator, anchors, parts.negated); self.spans.borrow_mut().insert(key, span); span } @@ -404,7 +406,10 @@ impl Builder<'_> { let mut fact = ComparisonFacts { span, generated_sql: if span.is_none() { - self.dialect.generate(node).unwrap_or_default() + node.source_span() + .and_then(|span| self.tokens.slice(span.start, span.end)) + .unwrap_or_default() + .to_owned() } else { String::new() }, @@ -432,7 +437,8 @@ impl Builder<'_> { if let Some(generated) = string_literal(literal) { let position = string_tokens .iter() - .position(|(_, _, text)| *text == literal.value_str()); + .position(|(_, _, text)| *text == generated) + .or_else(|| (!string_tokens.is_empty()).then_some(0)); let range = position.map(|index| { let (start, end, _) = string_tokens.remove(index); (start, end) @@ -668,7 +674,7 @@ fn own_span(expression: &Expression) -> Option { Expression::Column(column) => column.span.or(column.name.span), Expression::Identifier(identifier) => identifier.span, Expression::Star(star) => star.table.as_ref().and_then(|table| table.span), - Expression::Function(function) => function.span, + Expression::Function(function) => function.span.0, Expression::Table(table) => table .catalog .as_ref() @@ -692,7 +698,7 @@ fn span_bounds(expression: &Expression) -> Option<(usize, usize)> { Expression::Column(column) => column.span.or(column.name.span), Expression::Identifier(identifier) => identifier.span, Expression::Star(star) => star.table.as_ref().and_then(|table| table.span), - Expression::Function(function) => function.span, + Expression::Function(function) => function.span.0, Expression::Count(count) => count.span, Expression::Table(table) => table.name.span, _ => None, diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index 9e6863c110..55dbf894df 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -1,6 +1,8 @@ use crate::query_facts::constants::MIN_CAST_TOKENS; -use crate::query_facts::models::ComparisonSpan; -use polyglot_sql::tokens::{Token, TokenType}; +use crate::query_facts::models::{CodeToken, ComparisonSpan}; +use sqlquack_syntax::LosslessToken; +use sqlquack_syntax::TokenKind; +use sqlquack_syntax::tokens::models::TokenType; use std::cell::RefCell; use std::collections::HashSet; use std::ops::Range; @@ -150,32 +152,35 @@ pub(crate) struct SelectTokens { /// The code tokens of a normalized query, without comments, over its text. pub(crate) struct QueryTokens<'a> { - tokens: Vec, + tokens: Vec>, sql: &'a str, bytes: Vec, used: RefCell>, } impl<'a> QueryTokens<'a> { - pub(crate) fn new(tokens: Vec, sql: &'a str) -> Self { + /// The code tokens of `tokens`, a lossless tokenization of `sql`, at character offsets. + pub(crate) fn new(tokens: Vec, sql: &'a str) -> Self { let mut bytes: Vec = sql.char_indices().map(|(byte, _)| byte).collect(); bytes.push(sql.len()); + let character = |byte: usize| bytes.partition_point(|offset| *offset < byte); + let tokens = tokens + .into_iter() + .filter_map(|token| match token.kind { + TokenKind::Significant(token_type) => Some(CodeToken { + token_type, + text: &sql[token.start..token.end], + start: character(token.start), + end: character(token.end), + }), + TokenKind::Whitespace | TokenKind::LineComment | TokenKind::BlockComment => None, + }) + .collect(); Self { sql, bytes, used: RefCell::default(), - tokens: tokens - .into_iter() - .filter(|token| { - !matches!( - token.token_type, - TokenType::BlockComment - | TokenType::LineComment - | TokenType::Space - | TokenType::Break - ) - }) - .collect(), + tokens, } } @@ -185,11 +190,11 @@ impl<'a> QueryTokens<'a> { } pub(crate) fn start(&self, index: usize) -> usize { - self.tokens[index].span.start + self.tokens[index].start } pub(crate) fn end(&self, index: usize) -> usize { - self.tokens[index].span.end + self.tokens[index].end } fn kind(&self, index: usize) -> Option { @@ -369,14 +374,14 @@ impl<'a> QueryTokens<'a> { parameterized && (cast || conversion) } - /// The string-literal tokens within a normalized range, as character range and parsed text. + /// The string-literal tokens within a normalized range, as character range and source text. pub(crate) fn string_tokens(&self, start: usize, end: usize) -> Vec<(usize, usize, &str)> { self.tokens .iter() .filter(|token| { - token.span.start >= start && token.span.end <= end && string_token(token.token_type) + token.start >= start && token.end <= end && string_token(token.token_type) }) - .map(|token| (token.span.start, token.span.end, token.text.as_str())) + .map(|token| (token.start, token.end, token.text)) .collect() } @@ -595,7 +600,6 @@ impl<'a> QueryTokens<'a> { operator: ComparisonOperator, (left_end, right_start): (Option, Option), negated: bool, - generated: &str, ) -> Option { let anchor = match (left_end, right_start) { (Some(end), _) => (0..self.tokens.len()).find(|index| { @@ -608,16 +612,7 @@ impl<'a> QueryTokens<'a> { && self.operator(*index, operator) && !self.used.borrow().contains(index) }), - (None, None) => { - let wanted = compact(generated); - (0..self.tokens.len()).find(|index| { - self.operator(*index, operator) && !self.used.borrow().contains(index) && { - let (first, last) = self.extent(*index, operator, negated); - self.slice(self.start(first), self.end(last)) - .is_some_and(|text| compact(text) == wanted) - } - }) - } + (None, None) => None, }?; self.used.borrow_mut().insert(anchor); let (first, last) = self.extent(anchor, operator, negated); @@ -685,13 +680,6 @@ impl<'a> QueryTokens<'a> { } } -fn compact(text: &str) -> String { - text.chars() - .filter(|character| !character.is_whitespace()) - .flat_map(char::to_lowercase) - .collect() -} - fn string_token(token_type: TokenType) -> bool { matches!( token_type, diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index 564143160d..db572236e9 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -10,3 +10,11 @@ pub(crate) const UDF_CALL: &str = "__udf"; pub(crate) const TABLE_FUNCTION_CALL: &str = "__table_fn"; pub(crate) const MIN_CAST_TOKENS: usize = 2; pub(crate) const TRIPLE_QUOTE_LENGTH: usize = 3; +/// Dialect names read with the shared DuckDB grammar; `generic` matches the fingerprint tokenizer. +pub(crate) const DUCKDB_DIALECTS: [&str; 3] = ["duckdb", "motherduck", "generic"]; +/// Dialect names read with the shared PostgreSQL grammar. +pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"]; +/// The Snowflake dialect name. +pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; +/// The SQLite dialect name. +pub(crate) const SQLITE_DIALECT: &str = "sqlite"; diff --git a/crates/sqlbuild-rules/src/query_facts/models.rs b/crates/sqlbuild-rules/src/query_facts/models.rs index 35dd961830..c36b32e080 100644 --- a/crates/sqlbuild-rules/src/query_facts/models.rs +++ b/crates/sqlbuild-rules/src/query_facts/models.rs @@ -1,4 +1,5 @@ -use polyglot_sql::Expression; +use sqlquack_syntax::Expression; +use sqlquack_syntax::tokens::models::TokenType; use std::collections::BTreeSet; use crate::lint_text::main::map_expanded_offset::map_expanded_offset; @@ -366,3 +367,12 @@ impl<'a> ModelSource<'a> { self.sql.get(from..to.max(from)).unwrap_or_default() } } + +/// One code token of a normalized query: its type, source text and character range. +#[derive(Debug)] +pub(crate) struct CodeToken<'a> { + pub(crate) token_type: TokenType, + pub(crate) text: &'a str, + pub(crate) start: usize, + pub(crate) end: usize, +} diff --git a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs index ae4c1d8d29..b4ac7cc0ac 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs @@ -8,8 +8,9 @@ use crate::query_facts::main::stub_call::stub_call; use crate::query_facts::models::{CallKind, ParsedFixture}; use crate::rules::_helpers::evaluation::path_fault; use crate::rules::models::ProjectEvaluationRequest; -use polyglot_sql::Expression; -use polyglot_sql::expressions::{Literal, Select}; +use sqlquack_syntax::Expression; +use sqlquack_syntax::expressions::names::models::Literal; +use sqlquack_syntax::expressions::queries::models::Select; use serde::{Deserialize, Serialize}; use sqlbuild_analysis::compiler::main::sql_test_fixture_facts::fixture_facts; use std::cell::RefCell; From d9e86d0d49855b42f2e094646f93ff5957840c02 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:37:35 +0000 Subject: [PATCH 06/14] wip: locate cast types and Snowflake date parts without a generator --- .../src/query_facts/_helpers/facts.rs | 45 ++++++++++++++++--- .../src/query_facts/_helpers/tokens.rs | 7 +++ .../src/query_facts/constants.rs | 4 ++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index e2518efce7..6f5a147e31 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,7 +1,7 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, + DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ @@ -95,6 +95,7 @@ pub(crate) fn query_facts(sql: &str, dialect_name: &str) -> Result bool { struct Builder<'a> { tokens: QueryTokens<'a>, selects: Vec, + dialect: Dialect, spans: RefCell>>, cursor: Cell, claimed: RefCell>, @@ -316,9 +318,14 @@ impl Builder<'_> { let range = item.map(|item| self.tokens.expression_range(item, alias.is_some())); let unwrapped = unwrap_paren(value); let cast_type = match unwrapped { - Expression::Cast(_) | Expression::TryCast(_) | Expression::SafeCast(_) => range + Expression::Cast(cast) | Expression::TryCast(cast) | Expression::SafeCast(cast) => range .as_ref() - .and_then(|range| self.tokens.cast_type(range)), + .and_then(|range| self.tokens.cast_type(range)) + .or_else(|| { + let span = cast.span.0?; + self.tokens + .cast_type(&self.tokens.covering(span.start, span.end)) + }), _ => None, }; ItemFacts::Expression(ExpressionItem { @@ -425,6 +432,7 @@ impl Builder<'_> { let mut string_tokens: Vec<(usize, usize, &str)> = span.map_or_else(Vec::new, |span| { self.tokens.string_tokens(span.start, span.end) }); + let units = date_part_arguments(node, self.dialect); for value in node.dfs() { match value { Expression::Column(column) => { @@ -433,7 +441,7 @@ impl Builder<'_> { Expression::Identifier(identifier) => { fact.columns.insert(identifier.name.clone()); } - Expression::Literal(literal) => { + Expression::Literal(literal) if !units.contains(&std::ptr::from_ref(value)) => { if let Some(generated) = string_literal(literal) { let position = string_tokens .iter() @@ -458,9 +466,10 @@ impl Builder<'_> { .extend(operand.dfs().filter_map(direct_column)); } if direct_string_literal(operand).is_none() - && operand - .dfs() - .any(|value| direct_string_literal(value).is_some()) + && operand.dfs().any(|value| { + !units.contains(&std::ptr::from_ref(value)) + && direct_string_literal(value).is_some() + }) { fact.modified_string_literal = true; } @@ -469,6 +478,28 @@ impl Builder<'_> { } } +/// The quoted first arguments of Snowflake's three-argument `DATEDIFF` calls, which are date parts. +fn date_part_arguments(root: &Expression, dialect: Dialect) -> HashSet<*const Expression> { + if dialect != Dialect::Snowflake { + return HashSet::new(); + } + root.dfs() + .filter_map(|node| match node { + Expression::Function(function) + if function.args.len() == DATE_DIFF_ARGUMENTS + && DATE_DIFF_CALLS.contains(&function.name.to_ascii_uppercase().as_str()) => + { + function + .args + .first() + .filter(|argument| direct_string_literal(argument).is_some()) + .map(std::ptr::from_ref) + } + _ => None, + }) + .collect() +} + fn comparison_parts(node: &Expression) -> Option> { let (operator, operands, negated) = match node { Expression::Eq(op) => (ComparisonOperator::Eq, vec![&op.left, &op.right], false), diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index 55dbf894df..18a1b33bb7 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -374,6 +374,13 @@ impl<'a> QueryTokens<'a> { parameterized && (cast || conversion) } + /// The tokens within characters `start` to `end`, as a token range. + pub(crate) fn covering(&self, start: usize, end: usize) -> Range { + let first = self.tokens.partition_point(|token| token.start < start); + let last = self.tokens.partition_point(|token| token.end <= end); + first..last.max(first) + } + /// The string-literal tokens within a normalized range, as character range and source text. pub(crate) fn string_tokens(&self, start: usize, end: usize) -> Vec<(usize, usize, &str)> { self.tokens diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index db572236e9..a87219f95e 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -18,3 +18,7 @@ pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"]; pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; /// The SQLite dialect name. pub(crate) const SQLITE_DIALECT: &str = "sqlite"; +/// Snowflake calls whose quoted first argument is a date part when they take three arguments. +pub(crate) const DATE_DIFF_CALLS: [&str; 4] = ["DATEDIFF", "DATE_DIFF", "TIMEDIFF", "TIMESTAMPDIFF"]; +/// The argument count at which a `DATE_DIFF_CALLS` call reads its first argument as a date part. +pub(crate) const DATE_DIFF_ARGUMENTS: usize = 3; From a2909a3e102d00a4acfa63e563ad6ecad66e26b1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:42:54 +0000 Subject: [PATCH 07/14] wip: report built-in rules unavailable for unparsed dialects --- crates/sqlbuild-rules/src/constants.rs | 3 ++ .../engine/tests/test_empty_input_tests.rs | 19 +++---- .../src/engine/tests/test_evaluation.rs | 14 ++--- .../src/engine/tests/test_query_facts.rs | 54 ++++++++++++------- .../src/query_facts/_helpers/facts.rs | 26 +++++---- .../src/query_facts/constants.rs | 3 +- .../src/query_facts/main/facts_available.rs | 6 +++ .../src/query_facts/main/mod.rs | 1 + .../src/query_facts/main/query_facts.rs | 2 +- .../sqlbuild-rules/src/query_facts/models.rs | 2 +- .../src/rules/_helpers/evaluation.rs | 34 ++++++++++++ .../src/rules/_helpers/sql_test_rules.rs | 54 ++++++++++++++----- 12 files changed, 155 insertions(+), 63 deletions(-) create mode 100644 crates/sqlbuild-rules/src/query_facts/main/facts_available.rs diff --git a/crates/sqlbuild-rules/src/constants.rs b/crates/sqlbuild-rules/src/constants.rs index 37b4e3f953..e3f4b5ad79 100644 --- a/crates/sqlbuild-rules/src/constants.rs +++ b/crates/sqlbuild-rules/src/constants.rs @@ -55,3 +55,6 @@ pub(crate) const ALLOWED_QUALIFIERS_OPTION: &str = "allowed_qualifiers"; pub(crate) const ALLOWED_PATHS_KEY: &str = "paths"; /// The published layers when a project does not configure them. pub(crate) const DEFAULT_PUBLISHED_LAYERS: [&str; 2] = ["mart", "mart_v"]; +/// What to do when SQLBuild does not parse a project's dialect for built-in rules. +pub(crate) const RULES_UNAVAILABLE_REMEDIATION: &str = + "These rules did not run. Review this file by hand until built-in rules support this dialect."; diff --git a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs index 3c9f34d22b..92657d96c2 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs @@ -14,6 +14,7 @@ const NO_ROWS_ASSERTION: (&str, &str) = ( "__assert__empty_inputs_produce_no_rows", "SELECT 1 AS unexpected_row FROM __ref(\"orders\")", ); +const UNAVAILABLE_MESSAGE: &str = "L007 tests/unit/test_orders__empty_inputs_produce_no_rows.sql: SQLBuild cannot parse mssql SQL for built-in rules yet, so these rules were not evaluated: SQBRTEST203"; const FLAGGED_MESSAGE: &str = "SQBRTEST203 tests/unit/test_orders__empty_inputs_produce_no_rows.sql: unit test block 1 (\"orders__empty_inputs_produce_no_rows\") mocks only empty inputs and asserts only that no rows are produced"; #[test] @@ -419,7 +420,7 @@ fn given_cached_minimum_tests_when_filler_becomes_real_test_then_model_cache_is_ } #[test] -fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_arguments_are_not_calls() +fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_reports_the_dialect_unavailable() -> Result<(), String> { let project_dir = TempDir::new().map_err(|error| error.to_string())?; let empty_orders = ( @@ -428,7 +429,7 @@ fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_argum ); let test_cases = [ test_types::EmptyInputTestRuleTestCase { - description: "a CONVERT to a parameterized type under a false predicate is an empty mock", + description: "a CONVERT to a parameterized type under a false predicate is an empty mock, reported unavailable until SQLBuild parses the dialect", test: helpers::empty_input_test_fact( "orders__empty_inputs_produce_no_rows", &[( @@ -439,10 +440,10 @@ fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_argum &[NO_ROWS_ASSERTION], ), allowed_tests: json!([]), - expected_messages: &[FLAGGED_MESSAGE], + expected_messages: &[UNAVAILABLE_MESSAGE], }, test_types::EmptyInputTestRuleTestCase { - description: "a CONVERT with a style argument is an empty mock", + description: "a CONVERT with a style argument is an empty mock, reported unavailable until SQLBuild parses the dialect", test: helpers::empty_input_test_fact( "orders__empty_inputs_produce_no_rows", &[( @@ -453,10 +454,10 @@ fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_argum &[NO_ROWS_ASSERTION], ), allowed_tests: json!([]), - expected_messages: &[FLAGGED_MESSAGE], + expected_messages: &[UNAVAILABLE_MESSAGE], }, test_types::EmptyInputTestRuleTestCase { - description: "a TRY_CONVERT to a parameterized type keeps a bare existence check", + description: "a TRY_CONVERT to a parameterized type keeps a bare existence check, reported unavailable until SQLBuild parses the dialect", test: helpers::empty_input_test_fact( "orders__empty_inputs_produce_no_rows", &[empty_orders], @@ -467,10 +468,10 @@ fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_argum )], ), allowed_tests: json!([]), - expected_messages: &[FLAGGED_MESSAGE], + expected_messages: &[UNAVAILABLE_MESSAGE], }, test_types::EmptyInputTestRuleTestCase { - description: "a function call in a CONVERT value argument still counts", + description: "a function call in a CONVERT value argument still counts, reported unavailable until SQLBuild parses the dialect", test: helpers::empty_input_test_fact( "orders__empty_inputs_produce_no_rows", &[( @@ -481,7 +482,7 @@ fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_argum &[NO_ROWS_ASSERTION], ), allowed_tests: json!([]), - expected_messages: &[], + expected_messages: &[UNAVAILABLE_MESSAGE], }, ]; diff --git a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs index 26cef0a1e1..7b77c64f26 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs @@ -27,11 +27,13 @@ fn given_dependency_tokens_when_classifying_imports_then_preserves_dialect_and_o expected_messages: &[], }, test_types::DependencyTokenTestCase { - description: "backtick quoting and comment-separated source arguments", + description: "backtick quoting and comment-separated source arguments, reported unavailable until SQLBuild parses the dialect", expected_columns: &[], dialect: "bigquery", query_sql: "WITH imported AS (SELECT * FROM __source /* ignored */ (`Orders`)) SELECT order_id FROM imported", - expected_messages: &[], + expected_messages: &[ + "SQLBuild cannot parse bigquery SQL for built-in rules yet, so these rules were not evaluated: SQBRMODEL101", + ], }, test_types::DependencyTokenTestCase { description: "dollar strings do not contribute spurious calls", @@ -552,22 +554,22 @@ fn given_dialect_quoting_around_table_functions_when_evaluating_rules_then_model expected_code: "SQBRCONTRACT101", }, test_types::DialectEvaluationTestCase { - description: "BigQuery backslash-escaped quote inside table function arguments", + description: "BigQuery backslash-escaped quote inside table function arguments, reported unavailable until SQLBuild parses the dialect", dialect: "bigquery", query_sql: concat!( "SELECT item_id ", "FROM __table_fn(\"inventory_items\", 'a\\')')(42)" ), - expected_code: "SQBRCONTRACT101", + expected_code: "L007", }, test_types::DialectEvaluationTestCase { - description: "BigQuery backtick identifier containing a parenthesis in arguments", + description: "BigQuery backtick identifier containing a parenthesis in arguments, reported unavailable until SQLBuild parses the dialect", dialect: "bigquery", query_sql: concat!( "SELECT item_id ", "FROM __table_fn(\"inventory_items\", `batch)size`)(42)" ), - expected_code: "SQBRCONTRACT101", + expected_code: "L007", }, ]; diff --git a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs index b03abed424..fa81d833c5 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs @@ -32,13 +32,13 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s expected_faults: &[], }, QueryFactsTestCase { - description: "an apostrophe in a BigQuery # comment opens no string", + description: "an apostrophe in a BigQuery # comment opens no string, reported unavailable until SQLBuild parses the dialect", dialect: "bigquery", query_sql: "WITH imported AS (\n # don't read the archive\n SELECT * FROM __ref(\"orders\")\n)\nSELECT order_id FROM imported", select: IMPORT_RULES, enum_columns: &[], columns: json!([]), - expected_faults: &[], + expected_faults: &[("L007", 1, 1, "cannot parse bigquery SQL for built-in rules")], }, QueryFactsTestCase { description: "a DuckDB FROM-first import CTE star is exempt without a SELECT keyword", @@ -115,6 +115,30 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s columns: json!([]), expected_faults: &[("SQBRDECLARATION102", 1, 35, "3 < 7")], }, + QueryFactsTestCase { + description: "DuckDB literal comparisons on separate lines each keep their own position", + dialect: "duckdb", + query_sql: "SELECT order_id FROM orders\nWHERE 3 < 7\n AND 5 > 2", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[ + ("SQBRDECLARATION102", 2, 7, "3 < 7"), + ("SQBRDECLARATION102", 3, 7, "5 > 2"), + ], + }, + QueryFactsTestCase { + description: "Snowflake literal comparisons on separate lines each keep their own position", + dialect: "snowflake", + query_sql: "SELECT order_id FROM orders\nWHERE 3 < 7\n AND 5 > 2", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[ + ("SQBRDECLARATION102", 2, 7, "3 < 7"), + ("SQBRDECLARATION102", 3, 7, "5 > 2"), + ], + }, QueryFactsTestCase { description: "unaliased later union branches keep their cast types", dialect: "duckdb", @@ -181,22 +205,22 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s )], }, QueryFactsTestCase { - description: "a BigQuery double-quoted string compared with an enum column", + description: "a BigQuery double-quoted string compared with an enum column, reported unavailable until SQLBuild parses the dialect", dialect: "bigquery", query_sql: "SELECT order_id FROM orders WHERE status = \"active\"", select: ENUM_RULE, enum_columns: STATUS, columns: json!([]), - expected_faults: &[("SQBRDECLARATION101", 1, 35, "")], + expected_faults: &[("L007", 1, 1, "cannot parse bigquery SQL for built-in rules")], }, QueryFactsTestCase { - description: "a BigQuery triple-quoted string compared with an enum column", + description: "a BigQuery triple-quoted string compared with an enum column, reported unavailable until SQLBuild parses the dialect", dialect: "bigquery", query_sql: "SELECT order_id FROM orders WHERE status = '''active'''", select: ENUM_RULE, enum_columns: STATUS, columns: json!([]), - expected_faults: &[("SQBRDECLARATION101", 1, 35, "")], + expected_faults: &[("L007", 1, 1, "cannot parse bigquery SQL for built-in rules")], }, QueryFactsTestCase { description: "a Postgres escape string compared with an enum column", @@ -226,32 +250,22 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s expected_faults: &[("SQBRDECLARATION102", 1, 35, "amount == 42")], }, QueryFactsTestCase { - description: "a parenthesized T-SQL TOP argument is not projection", + description: "a parenthesized T-SQL TOP argument is not projection, reported unavailable until SQLBuild parses the dialect", dialect: "mssql", query_sql: "SELECT TOP (5) amount + 1 AS amount FROM orders", select: OUTPUT_TYPE_RULE, enum_columns: &[], columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), - expected_faults: &[( - "SQBRCONTRACT105", - 1, - 16, - "calculated without an outer explicit cast", - )], + expected_faults: &[("L007", 1, 1, "cannot parse mssql SQL for built-in rules")], }, QueryFactsTestCase { - description: "a T-SQL TOP PERCENT clause is not projection", + description: "a T-SQL TOP PERCENT clause is not projection, reported unavailable until SQLBuild parses the dialect", dialect: "mssql", query_sql: "SELECT TOP 5 PERCENT amount + 1 AS amount FROM orders", select: OUTPUT_TYPE_RULE, enum_columns: &[], columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), - expected_faults: &[( - "SQBRCONTRACT105", - 1, - 22, - "calculated without an outer explicit cast", - )], + expected_faults: &[("L007", 1, 1, "cannot parse mssql SQL for built-in rules")], }, QueryFactsTestCase { description: "a placeholder in relation position still parses", diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 6f5a147e31..4cc9c14753 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,8 +1,8 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, - TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, + DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, + POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, @@ -10,7 +10,9 @@ use crate::query_facts::models::{ }; use sqlquack_syntax::expressions::names::models::{Identifier, Literal, TableRef}; use sqlquack_syntax::expressions::queries::models::{Select, With}; -use sqlquack_syntax::{CallMode, ComplexityGuardOptions, Dialect, Expression, ExpressionWalk, ParseOptions}; +use sqlquack_syntax::{ + CallMode, ComplexityGuardOptions, Dialect, Expression, ExpressionWalk, ParseOptions, +}; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; use std::ops::Range; @@ -318,14 +320,16 @@ impl Builder<'_> { let range = item.map(|item| self.tokens.expression_range(item, alias.is_some())); let unwrapped = unwrap_paren(value); let cast_type = match unwrapped { - Expression::Cast(cast) | Expression::TryCast(cast) | Expression::SafeCast(cast) => range - .as_ref() - .and_then(|range| self.tokens.cast_type(range)) - .or_else(|| { - let span = cast.span.0?; - self.tokens - .cast_type(&self.tokens.covering(span.start, span.end)) - }), + Expression::Cast(cast) | Expression::TryCast(cast) | Expression::SafeCast(cast) => { + range + .as_ref() + .and_then(|range| self.tokens.cast_type(range)) + .or_else(|| { + let span = cast.span.0?; + self.tokens + .cast_type(&self.tokens.covering(span.start, span.end)) + }) + } _ => None, }; ItemFacts::Expression(ExpressionItem { diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index a87219f95e..c350e969ae 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -19,6 +19,7 @@ pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; /// The SQLite dialect name. pub(crate) const SQLITE_DIALECT: &str = "sqlite"; /// Snowflake calls whose quoted first argument is a date part when they take three arguments. -pub(crate) const DATE_DIFF_CALLS: [&str; 4] = ["DATEDIFF", "DATE_DIFF", "TIMEDIFF", "TIMESTAMPDIFF"]; +pub(crate) const DATE_DIFF_CALLS: [&str; 4] = + ["DATEDIFF", "DATE_DIFF", "TIMEDIFF", "TIMESTAMPDIFF"]; /// The argument count at which a `DATE_DIFF_CALLS` call reads its first argument as a date part. pub(crate) const DATE_DIFF_ARGUMENTS: usize = 3; diff --git a/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs b/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs new file mode 100644 index 0000000000..3fc8193c5f --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs @@ -0,0 +1,6 @@ +//! `facts_available`: whether SQLBuild parses a dialect's SQL for built-in rules. + +/// Whether SQLBuild parses `dialect`'s SQL for the built-in rules that read query facts. +pub(crate) fn facts_available(dialect: &str) -> bool { + crate::query_facts::_helpers::facts::facts_dialect(dialect).is_some() +} diff --git a/crates/sqlbuild-rules/src/query_facts/main/mod.rs b/crates/sqlbuild-rules/src/query_facts/main/mod.rs index 12999bd6c5..a4b8b0f0c7 100644 --- a/crates/sqlbuild-rules/src/query_facts/main/mod.rs +++ b/crates/sqlbuild-rules/src/query_facts/main/mod.rs @@ -1,4 +1,5 @@ pub mod dependency_texts; +pub mod facts_available; pub mod model_shape; pub mod parse_fixture; pub mod query_facts; diff --git a/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs b/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs index dba714d2c9..e246caab16 100644 --- a/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs @@ -1,6 +1,6 @@ use crate::query_facts::models::QueryFacts; -/// Parse normalized model SQL with Polyglot and read the facts built-in rules evaluate. +/// Parse normalized model SQL with SQLQuack and read the facts built-in rules evaluate. pub(crate) fn query_facts(sql: &str, dialect: &str) -> Result { crate::query_facts::_helpers::facts::query_facts(sql, dialect) } diff --git a/crates/sqlbuild-rules/src/query_facts/models.rs b/crates/sqlbuild-rules/src/query_facts/models.rs index c36b32e080..a5688f1e06 100644 --- a/crates/sqlbuild-rules/src/query_facts/models.rs +++ b/crates/sqlbuild-rules/src/query_facts/models.rs @@ -193,7 +193,7 @@ impl QueryFacts { } } -/// An SQL-test fixture query parsed with Polyglot, with the function-call facts its rules read. +/// An SQL-test fixture query parsed with SQLQuack, with the function-call facts its rules read. #[derive(Debug)] pub(crate) struct ParsedFixture { pub(crate) statement: Expression, diff --git a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs index 733cabe15d..4e3256ff5e 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs @@ -6,9 +6,12 @@ use crate::constants::{ }; use crate::constants::{ MIN_CUSTOM_RULE_TEST_CASES_KEY, MODEL_NAME_LAYERS, PUBLISHED_READ_CODE, PUBLISHED_SHAPE_CODE, + RULES_UNAVAILABLE_REMEDIATION, }; +use crate::lint_text::constants::LINT_UNAVAILABLE_CODE; use crate::models::{Declaration, EvaluateRequest, Fault, Model, RuleMetadata, RulesConfig}; use crate::query_facts::main::dependency_texts::dependency_texts; +use crate::query_facts::main::facts_available::facts_available; use crate::query_facts::main::model_shape::model_shape; use crate::query_facts::main::query_facts::query_facts; use crate::query_facts::models::{ @@ -134,6 +137,17 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result) -> faults.into_inner() } +/// One non-failing warning that `dialect` SQL is not parsed, so the built-in rules `codes` did not run on `path`. +pub(crate) fn rules_unavailable<'a>( + path: &str, + codes: impl Iterator, + dialect: &str, +) -> Fault { + Fault { + unevaluated: false, + code: LINT_UNAVAILABLE_CODE.to_owned(), + path: path.to_owned(), + line: 1, + column: 1, + message: format!( + "SQLBuild cannot parse {dialect} SQL for built-in rules yet, so these rules were not evaluated: {}", + codes.collect::>().join(", ") + ), + remediation: RULES_UNAVAILABLE_REMEDIATION.to_owned(), + } +} + fn requires_parsed_model(selected: &BTreeMap) -> bool { selected.values().any(|rule| requires_model_query(rule)) } diff --git a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs index b4ac7cc0ac..9eb7006c5d 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs @@ -2,19 +2,20 @@ use crate::models::{ DeclarationKind, EvaluateRequest, Fault, Model, ResourceKind, RuleMetadata, RulesConfig, ScopeResource, SqlScenarioFact, SqlTestCteFact, SqlTestFact, SqlTestMode, }; +use crate::query_facts::main::facts_available::facts_available; use crate::query_facts::main::model_shape::model_shape; use crate::query_facts::main::parse_fixture::parse_fixture; use crate::query_facts::main::stub_call::stub_call; use crate::query_facts::models::{CallKind, ParsedFixture}; -use crate::rules::_helpers::evaluation::path_fault; +use crate::rules::_helpers::evaluation::{path_fault, rules_unavailable}; use crate::rules::models::ProjectEvaluationRequest; +use serde::{Deserialize, Serialize}; +use sqlbuild_analysis::compiler::main::sql_test_fixture_facts::fixture_facts; use sqlquack_syntax::Expression; use sqlquack_syntax::expressions::names::models::Literal; use sqlquack_syntax::expressions::queries::models::Select; -use serde::{Deserialize, Serialize}; -use sqlbuild_analysis::compiler::main::sql_test_fixture_facts::fixture_facts; use std::cell::RefCell; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use std::path::Path; use std::rc::Rc; @@ -43,17 +44,35 @@ fn evaluated_project(evaluation: ProjectEvaluationRequest<'_>) -> Result> = HashMap::new(); - for rule in evaluation.selected.values().filter(|rule| { - matches!( - rule.code.as_str(), - "SQBRTEST201" | "SQBRTEST202" | EMPTY_INPUT_RULE_CODE - ) - }) { + let fixture_rules: Vec<&RuleMetadata> = evaluation + .selected + .values() + .copied() + .filter(|rule| { + matches!( + rule.code.as_str(), + "SQBRTEST201" | "SQBRTEST202" | EMPTY_INPUT_RULE_CODE + ) + }) + .collect(); + let dialect: &str = &evaluation.request.dialect; + if !fixture_rules.is_empty() && !facts_available(dialect) { + let paths: BTreeSet<&str> = tests + .iter() + .filter(|test| reads_fixtures(test)) + .map(|test| test.source_path.as_str()) + .collect(); + faults.extend(paths.into_iter().map(|path| { + rules_unavailable( + path, + fixture_rules.iter().map(|rule| rule.code.as_str()), + dialect, + ) + })); + } + for rule in fixture_rules.iter().filter(|_| facts_available(dialect)) { for test in &tests { - if !matches!(test.mode, SqlTestMode::Model) - || test.has_macro_mocks - || test.has_model_query_overrides - { + if !reads_fixtures(test) { continue; } for cte in test @@ -115,6 +134,13 @@ fn evaluated_project(evaluation: ProjectEvaluationRequest<'_>) -> Result bool { + matches!(test.mode, SqlTestMode::Model) + && !test.has_macro_mocks + && !test.has_model_query_overrides +} + fn canonical_roots( rule: &RuleMetadata, tests: &[&SqlTestFact], From 7a85582c3a919895d00cfee3d08c4806422d716a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:45:46 +0000 Subject: [PATCH 08/14] wip: e2e for built-in rules on unparsed dialects --- .../cli/commands/main/rules/_test_types.py | 9 +++ .../main/rules/test_sql_quality_rules.py | 62 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py index 977b4079b0..61a0546429 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py @@ -179,6 +179,15 @@ class RankingSortDefaultCase: expected_codes: tuple[str, ...] +@dataclass(frozen=True) +class UnparsedDialectRulesCase: + description: str + adapter_toml: str + expected_codes: tuple[str, ...] + expected_message: str + expected_exit: int + + @dataclass(frozen=True) class ReplayedFindingExitCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py index a22ba9759c..f7c51df4c7 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py @@ -18,6 +18,7 @@ RankingSortDefaultCase, ReplayedFindingExitCase, SqlQualityRuleCase, + UnparsedDialectRulesCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( diagnostic_codes, @@ -622,3 +623,64 @@ def test_given_cached_sql_finding_when_running_commands_then_only_rule_findings_ if __name__ == "__main__": pytest.main([__file__, "-vv"]) + + +@pytest.mark.parametrize( + "test_case", + ( + UnparsedDialectRulesCase( + description="a parsed dialect reports the built-in rule finding", + adapter_toml='adapter = "duckdb"\n', + expected_codes=("SQBRMODEL102",), + expected_message="SELECT * is allowed only", + expected_exit=1, + ), + UnparsedDialectRulesCase( + description="an unparsed dialect warns once per model and fails nothing", + adapter_toml=( + 'adapter = "bigquery"\ndefault_target = "dev"\n\n' + '[targets.dev]\ndatabase = "warehouse"\nschema = "analytics"\n' + ), + expected_codes=("L007", "L007"), + expected_message=( + "SQLBuild cannot parse bigquery SQL for built-in rules yet," + " so these rules were not evaluated: SQBRMODEL102" + ), + expected_exit=0, + ), + ), + ids=lambda case: case.description, +) +def test_given_project_dialect_when_running_built_in_rules_then_unparsed_dialects_only_warn( + test_case: UnparsedDialectRulesCase, tmp_path: Path +) -> None: + write_quality_project( + project_dir=tmp_path, + toml=( + 'name = "orders"\n' + + test_case.adapter_toml + + '\n[rules]\nselect = ["SQBRMODEL102"]\n' + ), + upstream=_RAW_HEADER + _UPSTREAM_SQL, + summary=_SUMMARY_HEADER + 'SELECT * FROM __ref("raw_orders")\n', + ) + + compiled: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "compile", "--json" + ) + ran: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "rules", "run", "SQBRMODEL102" + ) + + messages: list[str] = [ + str(item["message"]) for item in json.loads(compiled.stdout)["diagnostics"] + ] + assert ( + diagnostic_codes(compiled), + all(test_case.expected_message in message for message in messages), + (compiled.returncode, ran.returncode), + ) == ( + test_case.expected_codes, + True, + (test_case.expected_exit, test_case.expected_exit), + ), compiled.stdout + compiled.stderr + ran.stdout + ran.stderr From adc798472febfcb5d53120778dd20a0b7d2397ee Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:57:11 +0000 Subject: [PATCH 09/14] wip: docs and tests for built-in rules on unparsed dialects --- .../references/docs/concepts/rules.md | 26 +++++++++++++++---- .../main/rules/test_clickhouse_comments.py | 8 +++--- .../sqlbuild/cli/commands/main/test_rules.py | 13 +++++++--- .../src/content/docs/docs/concepts/rules.mdx | 25 ++++++++++++++---- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md index 0b2aac11f9..2f7792a7a3 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md @@ -12,6 +12,7 @@ Online: https://sqlbuild.com/docs/concepts/rules/ - How Rules work - SQL Rules - Built-in and custom Rules +- Dialect support - One definition of project validity - Rules compared with tests, audits, and contracts - Findings and intentional exceptions @@ -144,11 +145,6 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. -If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile` -and `sqb rules run` show one `L007` warning per file that names the dialect. `L007` never changes -an exit code, including for `sqb plan` and `sqb build`: it reports a gap in SQLBuild's dialect -support, not a problem in your SQL, so the file's SQL Rules simply did not run. - Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: @@ -183,6 +179,26 @@ select = [ Define typed model or project checks over compiler-owned facts with `@rule`, `RuleContext`, and `Finding`. +## Dialect support + +SQL Rules, and the built-in Rules that read SQL, such as `SQBRMODEL102` and the SQL-test Rules +`SQBRTEST201` to `SQBRTEST203`, run only where SQLBuild parses the project's dialect. Built-in Rules +that read SQL support these adapters: + +| Adapter | Built-in Rules that read SQL | +|---|---| +| DuckDB, MotherDuck | Supported | +| PostgreSQL | Supported | +| Snowflake | Supported | +| SQLite | Supported | +| BigQuery, ClickHouse, Databricks, SQL Server | Not yet | + +Where SQLBuild does not parse the dialect yet, `sqb compile` and `sqb rules run` show `L007` +warnings that name it: one per file for SQL Rules, and one per model or SQL test listing the +built-in Rules that did not run. `L007` never changes an exit code, including for `sqb plan` and +`sqb build`: it reports a gap in SQLBuild's dialect support, not a problem in your SQL. A syntax +error in a supported dialect still fails the Rules that read it. + ## One definition of project validity `sqb compile` is authoritative. SQLBuild evaluates a project in this order: diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_clickhouse_comments.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_clickhouse_comments.py index 0cc24a9f8b..ba1ee05735 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_clickhouse_comments.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_clickhouse_comments.py @@ -23,14 +23,14 @@ "test_case", [ ClickHouseCommentE2ETestCase( - description="a hash comment with an apostrophe is skipped", + description="a hash comment with an apostrophe is skipped, reported unavailable until SQLBuild parses the dialect", comment="# don't keep the star", - expected_location="models/orders.sql:4:8 [SQBRMODEL102]", + expected_location="models/orders.sql:1:1 [L007]", ), ClickHouseCommentE2ETestCase( - description="a double-slash comment with an apostrophe is skipped", + description="a double-slash comment with an apostrophe is skipped, reported unavailable until SQLBuild parses the dialect", comment="// don't keep the star", - expected_location="models/orders.sql:4:8 [SQBRMODEL102]", + expected_location="models/orders.sql:1:1 [L007]", ), ], ids=lambda case: case.description, diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index 66c39436dc..e10b1b007d 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -2142,10 +2142,14 @@ def test_given_commented_dependency_call_when_running_import_rule_then_comment_i @pytest.mark.parametrize( "test_case", - [RulesIntegrationTestCase("quoted comment marker preserves real call", 1, "SQBRMODEL101")], + [ + RulesIntegrationTestCase( + "quoted comment marker" + ", reported unavailable until SQLBuild parses the dialect", 0, "SQBRMODEL101" + ) + ], ids=lambda case: case.description, ) -def test_given_bigquery_quoted_comment_marker_when_running_import_rule_then_real_call_is_detected( +def test_given_bigquery_quoted_comment_marker_when_running_import_rule_then_reports_dialect_unavailable( test_case: RulesIntegrationTestCase, tmp_path: Path, capsys: pytest.CaptureFixture[str], @@ -2194,8 +2198,9 @@ def test_given_bigquery_quoted_comment_marker_when_running_import_rule_then_real payload: dict[str, object] = json.loads(capsys.readouterr().out) findings: list[dict[str, object]] = cast(list[dict[str, object]], payload["findings"]) assert [finding["message"] for finding in findings] == [ - 'import CTE "imported_orders" contains transformation logic' - ] + "SQLBuild cannot parse bigquery SQL for built-in rules yet," + " so these rules were not evaluated: SQBRMODEL101" + ] * 2 @pytest.mark.parametrize( diff --git a/website/src/content/docs/docs/concepts/rules.mdx b/website/src/content/docs/docs/concepts/rules.mdx index 884993b4a5..a1e0eb1845 100644 --- a/website/src/content/docs/docs/concepts/rules.mdx +++ b/website/src/content/docs/docs/concepts/rules.mdx @@ -132,11 +132,6 @@ These checks run through ordinary compilation. There is intentionally no separat lifecycle. If an SQL requirement affects whether the project is acceptable, the compiler enforces it with the rest of the configured Rules. -If SQL Rules cannot run because SQLBuild does not parse the project's dialect yet, `sqb compile` -and `sqb rules run` show one `L007` warning per file that names the dialect. `L007` never changes -an exit code, including for `sqb plan` and `sqb build`: it reports a gap in SQLBuild's dialect -support, not a problem in your SQL, so the file's SQL Rules simply did not run. - Not every traditional lint concern should become a diagnostic. SQLBuild separates three kinds of SQL responsibility: @@ -173,6 +168,26 @@ select = [ `Finding`. +## Dialect support + +SQL Rules, and the built-in Rules that read SQL, such as `SQBRMODEL102` and the SQL-test Rules +`SQBRTEST201` to `SQBRTEST203`, run only where SQLBuild parses the project's dialect. Built-in Rules +that read SQL support these adapters: + +| Adapter | Built-in Rules that read SQL | +|---|---| +| DuckDB, MotherDuck | Supported | +| PostgreSQL | Supported | +| Snowflake | Supported | +| SQLite | Supported | +| BigQuery, ClickHouse, Databricks, SQL Server | Not yet | + +Where SQLBuild does not parse the dialect yet, `sqb compile` and `sqb rules run` show `L007` +warnings that name it: one per file for SQL Rules, and one per model or SQL test listing the +built-in Rules that did not run. `L007` never changes an exit code, including for `sqb plan` and +`sqb build`: it reports a gap in SQLBuild's dialect support, not a problem in your SQL. A syntax +error in a supported dialect still fails the Rules that read it. + ## One definition of project validity `sqb compile` is authoritative. SQLBuild evaluates a project in this order: From 088bd97557a7cdb681db950952b18b8ec7bc12f8 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:00:19 +0000 Subject: [PATCH 10/14] wip: format tests --- .../cli/commands/main/rules/test_sql_quality_rules.py | 4 +--- .../integration/src/sqlbuild/cli/commands/main/test_rules.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py index f7c51df4c7..9199ee3e4b 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_sql_quality_rules.py @@ -657,9 +657,7 @@ def test_given_project_dialect_when_running_built_in_rules_then_unparsed_dialect write_quality_project( project_dir=tmp_path, toml=( - 'name = "orders"\n' - + test_case.adapter_toml - + '\n[rules]\nselect = ["SQBRMODEL102"]\n' + 'name = "orders"\n' + test_case.adapter_toml + '\n[rules]\nselect = ["SQBRMODEL102"]\n' ), upstream=_RAW_HEADER + _UPSTREAM_SQL, summary=_SUMMARY_HEADER + 'SELECT * FROM __ref("raw_orders")\n', diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index e10b1b007d..1ad17c825b 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -2144,7 +2144,9 @@ def test_given_commented_dependency_call_when_running_import_rule_then_comment_i "test_case", [ RulesIntegrationTestCase( - "quoted comment marker" + ", reported unavailable until SQLBuild parses the dialect", 0, "SQBRMODEL101" + "quoted comment marker" + ", reported unavailable until SQLBuild parses the dialect", + 0, + "SQBRMODEL101", ) ], ids=lambda case: case.description, From 8b2a9d3ec5eab3880b0ff079e4cee780580dc20b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:14:04 +0000 Subject: [PATCH 11/14] fix(rules): refuse rule facts for statements deeper than 512 levels --- .../src/engine/tests/test_evaluation.rs | 45 +++++++++++++------ .../src/engine/tests/test_types.rs | 2 +- .../src/query_facts/_helpers/facts.rs | 20 ++++++++- .../src/query_facts/constants.rs | 2 + 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs index 7b77c64f26..0dbcbddc3e 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs @@ -518,7 +518,8 @@ fn given_snowflake_expression_when_evaluating_rules_then_uses_project_dialect() "CAST(amounts AS ARRAY(NUMBER(38, 10))) AS amounts, ", "OBJECT_CONSTRUCT_KEEP_NULL('group_id', group_id) AS details, ", "FROM __table_fn(\"inventory_items\")(42)" - ), + ) + .to_owned(), expected_code: "SQBRCONTRACT101", }]; @@ -550,7 +551,8 @@ fn given_dialect_quoting_around_table_functions_when_evaluating_rules_then_model query_sql: concat!( "SELECT 'it\\'s (' AS label, item_id ", "FROM __table_fn(\"inventory_items\")(42)" - ), + ) + .to_owned(), expected_code: "SQBRCONTRACT101", }, test_types::DialectEvaluationTestCase { @@ -559,7 +561,8 @@ fn given_dialect_quoting_around_table_functions_when_evaluating_rules_then_model query_sql: concat!( "SELECT item_id ", "FROM __table_fn(\"inventory_items\", 'a\\')')(42)" - ), + ) + .to_owned(), expected_code: "L007", }, test_types::DialectEvaluationTestCase { @@ -568,7 +571,8 @@ fn given_dialect_quoting_around_table_functions_when_evaluating_rules_then_model query_sql: concat!( "SELECT item_id ", "FROM __table_fn(\"inventory_items\", `batch)size`)(42)" - ), + ) + .to_owned(), expected_code: "L007", }, ]; @@ -593,12 +597,27 @@ fn given_dialect_quoting_around_table_functions_when_evaluating_rules_then_model #[test] fn given_unparseable_model_when_evaluating_then_retains_named_rule_fault() -> Result<(), String> { let project_dir = TempDir::new().map_err(|error| error.to_string())?; - let test_cases = [test_types::DialectEvaluationTestCase { - description: "invalid query is missing evidence rather than a passing contract", - dialect: "duckdb", - query_sql: "SELECT (", - expected_code: "SQBRCONTRACT101", - }]; + let deep_sql = format!("SELECT {}1 AS order_id FROM orders", "1 + ".repeat(50_000)); + let test_cases = [ + test_types::DialectEvaluationTestCase { + description: "invalid query is missing evidence rather than a passing contract", + dialect: "duckdb", + query_sql: "SELECT (".to_owned(), + expected_code: "SQBRCONTRACT101", + }, + test_types::DialectEvaluationTestCase { + description: "a DuckDB expression deeper than the AST limit is refused, not a crash", + dialect: "duckdb", + query_sql: deep_sql.clone(), + expected_code: "SQBRCONTRACT101", + }, + test_types::DialectEvaluationTestCase { + description: "a Snowflake expression deeper than the AST limit is refused, not a crash", + dialect: "snowflake", + query_sql: deep_sql, + expected_code: "SQBRCONTRACT101", + }, + ]; for test_case in test_cases { let config = json!({"select": ["SQBRCONTRACT101"], "cache": {"enabled": false}}); let mut request: Value = serde_json::from_str(&helpers::request(&project_dir, &config)) @@ -641,19 +660,19 @@ fn given_comment_apostrophes_when_generic_fallback_normalizes_rules_sql_then_mod test_types::DialectEvaluationTestCase { description: "line comment apostrophe before a colon path", dialect: "postgres", - query_sql: "-- don't drop the customer path\nSELECT payload:customer_id AS customer_id FROM orders", + query_sql: "-- don't drop the customer path\nSELECT payload:customer_id AS customer_id FROM orders".to_owned(), expected_code: "SQBRCONTRACT101", }, test_types::DialectEvaluationTestCase { description: "block comment apostrophe before a colon path", dialect: "postgres", - query_sql: "/* customer's path */ SELECT payload:customer_id AS customer_id FROM orders", + query_sql: "/* customer's path */ SELECT payload:customer_id AS customer_id FROM orders".to_owned(), expected_code: "SQBRCONTRACT101", }, test_types::DialectEvaluationTestCase { description: "colon inside a line comment stays commented", dialect: "postgres", - query_sql: "SELECT payload:customer_id AS customer_id -- note: it's nested\nFROM orders", + query_sql: "SELECT payload:customer_id AS customer_id -- note: it's nested\nFROM orders".to_owned(), expected_code: "SQBRCONTRACT101", }, ]; diff --git a/crates/sqlbuild-rules/src/engine/tests/test_types.rs b/crates/sqlbuild-rules/src/engine/tests/test_types.rs index fb3fd5bbea..4ac63e0f05 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_types.rs @@ -35,7 +35,7 @@ pub(crate) struct DeclarationScopeTestCase { pub(crate) struct DialectEvaluationTestCase { pub(crate) description: &'static str, pub(crate) dialect: &'static str, - pub(crate) query_sql: &'static str, + pub(crate) query_sql: String, pub(crate) expected_code: &'static str, } diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 4cc9c14753..98719dae46 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,7 +1,7 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_FUNCTION_CALL_DEPTH, + DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_AST_DEPTH, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ @@ -48,12 +48,30 @@ fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), S return Err("expected one statement".to_owned()); } let statement = statements.remove(0); + if deeper_than(&statement, MAX_AST_DEPTH) { + return Err(format!( + "E_GUARD_AST_DEPTH_EXCEEDED: value {} exceeds configured limit {MAX_AST_DEPTH}", + MAX_AST_DEPTH + 1 + )); + } if !is_query(&statement) { return Err("expected a query".to_owned()); } Ok((statement, dialect)) } +/// Whether `statement` nests deeper than `limit` levels, measured without recursion. +fn deeper_than(statement: &Expression, limit: usize) -> bool { + let mut stack: Vec<(&Expression, usize)> = vec![(statement, 1)]; + while let Some((node, depth)) = stack.pop() { + if depth > limit { + return true; + } + stack.extend(node.children().into_iter().map(|child| (child, depth + 1))); + } + false +} + fn code_tokens(sql: &str, dialect: Dialect) -> Result, String> { let tokens = sqlquack_syntax::tokenize(sql, dialect).map_err(|error| error.to_string())?; Ok(QueryTokens::new(tokens, sql)) diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index c350e969ae..70b6cd8b84 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -6,6 +6,8 @@ pub(crate) const UDF_STUB: &str = "__sqlbuild_udf_"; pub(crate) const PLACEHOLDER_PREFIX: &[u8] = b"@@@"; pub(crate) const PLACEHOLDER_VALUE: &str = "NULL"; pub(crate) const MAX_FUNCTION_CALL_DEPTH: usize = 128; +/// The deepest statement built-in rules read, Polyglot's former AST-depth limit. +pub(crate) const MAX_AST_DEPTH: usize = 512; pub(crate) const UDF_CALL: &str = "__udf"; pub(crate) const TABLE_FUNCTION_CALL: &str = "__table_fn"; pub(crate) const MIN_CAST_TOKENS: usize = 2; From d9fa004b5fef26bfbf43f7ed213efb06c40b8f6d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:41:04 +0000 Subject: [PATCH 12/14] fix(rules): anchor nested comparisons and Snowflake LAST_DAY date parts --- .../src/engine/tests/test_query_facts.rs | 18 ++++++++++ .../src/query_facts/_helpers/facts.rs | 36 +++++++++++++------ .../src/query_facts/constants.rs | 13 ++++--- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs index fa81d833c5..fca1058f78 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs @@ -115,6 +115,24 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s columns: json!([]), expected_faults: &[("SQBRDECLARATION102", 1, 35, "3 < 7")], }, + QueryFactsTestCase { + description: "an anchorless comparison nested in another keeps its own position", + dialect: "snowflake", + query_sql: "SELECT order_id FROM orders\nWHERE CASE WHEN 3 = 2 THEN 10 ELSE 20 END = 1", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRDECLARATION102", 2, 17, "3 = 2")], + }, + QueryFactsTestCase { + description: "a Snowflake LAST_DAY date part compared with an enum column is no enum literal", + dialect: "snowflake", + query_sql: "SELECT order_id FROM orders WHERE LAST_DAY(placed_at, 'month') = status", + select: ENUM_RULE, + enum_columns: STATUS, + columns: json!([]), + expected_faults: &[], + }, QueryFactsTestCase { description: "DuckDB literal comparisons on separate lines each keep their own position", dialect: "duckdb", diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 98719dae46..b1ee3aa54f 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,8 +1,9 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - DATE_DIFF_ARGUMENTS, DATE_DIFF_CALLS, DUCKDB_DIALECTS, MAX_AST_DEPTH, MAX_FUNCTION_CALL_DEPTH, - POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, + DUCKDB_DIALECTS, MAX_AST_DEPTH, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, + SNOWFLAKE_DATE_PART_ARGUMENTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, + TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, @@ -395,7 +396,13 @@ impl Builder<'_> { return *span; } let anchors = match anchors(parts) { - (None, None) => (node.source_span().map(|span| span.start), None), + (None, None) => ( + parts.operands[0] + .source_span() + .map(|span| span.end) + .or_else(|| node.source_span().map(|span| span.start)), + None, + ), anchors => anchors, }; let span = self @@ -463,7 +470,15 @@ impl Builder<'_> { Expression::Identifier(identifier) => { fact.columns.insert(identifier.name.clone()); } - Expression::Literal(literal) if !units.contains(&std::ptr::from_ref(value)) => { + Expression::Literal(literal) if units.contains(&std::ptr::from_ref(value)) => { + if let Some(unit) = string_literal(literal) + && let Some(index) = + string_tokens.iter().position(|(_, _, text)| *text == unit) + { + string_tokens.remove(index); + } + } + Expression::Literal(literal) => { if let Some(generated) = string_literal(literal) { let position = string_tokens .iter() @@ -500,20 +515,21 @@ impl Builder<'_> { } } -/// The quoted first arguments of Snowflake's three-argument `DATEDIFF` calls, which are date parts. +/// The quoted date-part arguments of Snowflake calls such as `DATEDIFF('day', a, b)`. fn date_part_arguments(root: &Expression, dialect: Dialect) -> HashSet<*const Expression> { if dialect != Dialect::Snowflake { return HashSet::new(); } root.dfs() .filter_map(|node| match node { - Expression::Function(function) - if function.args.len() == DATE_DIFF_ARGUMENTS - && DATE_DIFF_CALLS.contains(&function.name.to_ascii_uppercase().as_str()) => - { + Expression::Function(function) => { + let name = function.name.to_ascii_uppercase(); + let (_, _, index) = SNOWFLAKE_DATE_PART_ARGUMENTS + .iter() + .find(|(call, count, _)| *call == name && *count == function.args.len())?; function .args - .first() + .get(*index) .filter(|argument| direct_string_literal(argument).is_some()) .map(std::ptr::from_ref) } diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index 70b6cd8b84..a473ddfef1 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -20,8 +20,11 @@ pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"]; pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; /// The SQLite dialect name. pub(crate) const SQLITE_DIALECT: &str = "sqlite"; -/// Snowflake calls whose quoted first argument is a date part when they take three arguments. -pub(crate) const DATE_DIFF_CALLS: [&str; 4] = - ["DATEDIFF", "DATE_DIFF", "TIMEDIFF", "TIMESTAMPDIFF"]; -/// The argument count at which a `DATE_DIFF_CALLS` call reads its first argument as a date part. -pub(crate) const DATE_DIFF_ARGUMENTS: usize = 3; +/// Snowflake calls with a date-part argument, as (name, argument count, date-part index). +pub(crate) const SNOWFLAKE_DATE_PART_ARGUMENTS: [(&str, usize, usize); 5] = [ + ("DATEDIFF", 3, 0), + ("DATE_DIFF", 3, 0), + ("TIMEDIFF", 3, 0), + ("TIMESTAMPDIFF", 3, 0), + ("LAST_DAY", 2, 1), +]; From 0352b445946cc59551ed5703b412d48a630a81c8 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 06:29:37 +0000 Subject: [PATCH 13/14] refactor(rules): name the date-part and unavailable-rule loops --- .../src/query_facts/_helpers/facts.rs | 34 ++++++++++--------- .../src/query_facts/_helpers/tokens.rs | 2 +- .../src/rules/_helpers/sql_test_rules.rs | 14 ++++---- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index b1ee3aa54f..959cf65122 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -520,22 +520,24 @@ fn date_part_arguments(root: &Expression, dialect: Dialect) -> HashSet<*const Ex if dialect != Dialect::Snowflake { return HashSet::new(); } - root.dfs() - .filter_map(|node| match node { - Expression::Function(function) => { - let name = function.name.to_ascii_uppercase(); - let (_, _, index) = SNOWFLAKE_DATE_PART_ARGUMENTS - .iter() - .find(|(call, count, _)| *call == name && *count == function.args.len())?; - function - .args - .get(*index) - .filter(|argument| direct_string_literal(argument).is_some()) - .map(std::ptr::from_ref) - } - _ => None, - }) - .collect() + root.dfs().filter_map(date_part_argument).collect() +} + +/// The quoted date-part argument of `node` when it is a Snowflake date-part call. +fn date_part_argument(node: &Expression) -> Option<*const Expression> { + let Expression::Function(function) = node else { + return None; + }; + let name = function.name.to_ascii_uppercase(); + for (call, count, index) in SNOWFLAKE_DATE_PART_ARGUMENTS { + if call == name && count == function.args.len() { + let argument = function.args.get(index)?; + return direct_string_literal(argument) + .is_some() + .then(|| std::ptr::from_ref(argument)); + } + } + None } fn comparison_parts(node: &Expression) -> Option> { diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index 18a1b33bb7..b847237997 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -164,7 +164,7 @@ impl<'a> QueryTokens<'a> { let mut bytes: Vec = sql.char_indices().map(|(byte, _)| byte).collect(); bytes.push(sql.len()); let character = |byte: usize| bytes.partition_point(|offset| *offset < byte); - let tokens = tokens + let tokens: Vec> = tokens .into_iter() .filter_map(|token| match token.kind { TokenKind::Significant(token_type) => Some(CodeToken { diff --git a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs index 9eb7006c5d..f53e86786c 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs @@ -62,13 +62,13 @@ fn evaluated_project(evaluation: ProjectEvaluationRequest<'_>) -> Result = fixture_rules + .iter() + .map(|rule| rule.code.as_str()) + .collect(); + for path in paths { + faults.push(rules_unavailable(path, codes.iter().copied(), dialect)); + } } for rule in fixture_rules.iter().filter(|_| facts_available(dialect)) { for test in &tests { From e0638cd48ef2917d0d1788cf7f4d22f5ba29f562 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 08:27:38 +0000 Subject: [PATCH 14/14] refactor(rules): read the shared-parser dialect from one lookup --- .../sql_tree/_helpers/rules_tree_parser.rs | 16 +++----------- .../src/sql_tree/main/mod.rs | 1 + .../src/sql_tree/main/shared_dialect.rs | 22 +++++++++++++++++++ .../src/query_facts/_helpers/facts.rs | 21 +++--------------- .../src/query_facts/constants.rs | 8 ------- .../src/query_facts/main/facts_available.rs | 4 +++- 6 files changed, 32 insertions(+), 40 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/sql_tree/main/shared_dialect.rs diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs index b330c0a39c..db8eb603d9 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs @@ -1,10 +1,7 @@ //! `rules_tree_parser`: the parser that reads a dialect's SQL for rules. -use sqlquack_syntax::Dialect; - -use crate::sql_tree::constants::{ - DUCKDB_DIALECTS, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, -}; +use crate::sql_tree::constants::POSTGRES_DIALECTS; +use crate::sql_tree::main::shared_dialect::shared_dialect; use crate::sql_tree::models::TreeParser; /// The parser for `dialect`, or `None`; add a dialect (one line) only once its parser passes the depth probe with an empty allowlist. @@ -12,12 +9,5 @@ pub(crate) fn rules_tree_parser(dialect: &str) -> Option { if POSTGRES_DIALECTS.contains(&dialect) { return Some(TreeParser::Postgres); } - if DUCKDB_DIALECTS.contains(&dialect) { - return Some(TreeParser::Shared(Dialect::DuckDB)); - } - match dialect { - SNOWFLAKE_DIALECT => Some(TreeParser::Shared(Dialect::Snowflake)), - SQLITE_DIALECT => Some(TreeParser::Shared(Dialect::SQLite)), - _ => None, - } + shared_dialect(dialect).map(TreeParser::Shared) } diff --git a/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs b/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs index 0c71a2f7d5..924d435a79 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs @@ -1,3 +1,4 @@ //! Entry points. +pub mod shared_dialect; pub mod sql_tree_nodes; diff --git a/crates/sqlbuild-analysis/src/sql_tree/main/shared_dialect.rs b/crates/sqlbuild-analysis/src/sql_tree/main/shared_dialect.rs new file mode 100644 index 0000000000..5a7e95518e --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/main/shared_dialect.rs @@ -0,0 +1,22 @@ +//! `shared_dialect`: the grammar of SQLQuack's shared parser that reads a dialect's SQL. + +use sqlquack_syntax::Dialect; + +use crate::sql_tree::constants::{ + DUCKDB_DIALECTS, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, +}; + +/// The shared-parser grammar for `dialect`, or `None` when the shared parser reads none of it. +pub fn shared_dialect(dialect: &str) -> Option { + if DUCKDB_DIALECTS.contains(&dialect) { + return Some(Dialect::DuckDB); + } + if POSTGRES_DIALECTS.contains(&dialect) { + return Some(Dialect::PostgreSQL); + } + match dialect { + SNOWFLAKE_DIALECT => Some(Dialect::Snowflake), + SQLITE_DIALECT => Some(Dialect::SQLite), + _ => None, + } +} diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 959cf65122..3b468b18cb 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,14 +1,14 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; use crate::query_facts::constants::{ - DUCKDB_DIALECTS, MAX_AST_DEPTH, MAX_FUNCTION_CALL_DEPTH, POSTGRES_DIALECTS, - SNOWFLAKE_DATE_PART_ARGUMENTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, TABLE_FUNCTION_STUB, + MAX_AST_DEPTH, MAX_FUNCTION_CALL_DEPTH, SNOWFLAKE_DATE_PART_ARGUMENTS, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, }; use crate::query_facts::models::{ CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, ModelShape, ParsedFixture, QueryFacts, RelationFacts, SelectFacts, SetFacts, SourceText, }; +use sqlbuild_analysis::sql_tree::main::shared_dialect::shared_dialect; use sqlquack_syntax::expressions::names::models::{Identifier, Literal, TableRef}; use sqlquack_syntax::expressions::queries::models::{Select, With}; use sqlquack_syntax::{ @@ -18,23 +18,8 @@ use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; use std::ops::Range; -/// The SQLQuack grammar that reads `dialect_name` for built-in rules, or `None` when none does. -pub(crate) fn facts_dialect(dialect_name: &str) -> Option { - if DUCKDB_DIALECTS.contains(&dialect_name) { - return Some(Dialect::DuckDB); - } - if POSTGRES_DIALECTS.contains(&dialect_name) { - return Some(Dialect::PostgreSQL); - } - match dialect_name { - SNOWFLAKE_DIALECT => Some(Dialect::Snowflake), - SQLITE_DIALECT => Some(Dialect::SQLite), - _ => None, - } -} - fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), String> { - let dialect = facts_dialect(dialect_name) + let dialect = shared_dialect(dialect_name) .ok_or_else(|| format!("no SQL parser for dialect {dialect_name}"))?; let options = ParseOptions { complexity_guard: Some(ComplexityGuardOptions { diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index a473ddfef1..2ff3bfcad4 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -12,14 +12,6 @@ pub(crate) const UDF_CALL: &str = "__udf"; pub(crate) const TABLE_FUNCTION_CALL: &str = "__table_fn"; pub(crate) const MIN_CAST_TOKENS: usize = 2; pub(crate) const TRIPLE_QUOTE_LENGTH: usize = 3; -/// Dialect names read with the shared DuckDB grammar; `generic` matches the fingerprint tokenizer. -pub(crate) const DUCKDB_DIALECTS: [&str; 3] = ["duckdb", "motherduck", "generic"]; -/// Dialect names read with the shared PostgreSQL grammar. -pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"]; -/// The Snowflake dialect name. -pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; -/// The SQLite dialect name. -pub(crate) const SQLITE_DIALECT: &str = "sqlite"; /// Snowflake calls with a date-part argument, as (name, argument count, date-part index). pub(crate) const SNOWFLAKE_DATE_PART_ARGUMENTS: [(&str, usize, usize); 5] = [ ("DATEDIFF", 3, 0), diff --git a/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs b/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs index 3fc8193c5f..d4e29aac88 100644 --- a/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs +++ b/crates/sqlbuild-rules/src/query_facts/main/facts_available.rs @@ -1,6 +1,8 @@ //! `facts_available`: whether SQLBuild parses a dialect's SQL for built-in rules. +use sqlbuild_analysis::sql_tree::main::shared_dialect::shared_dialect; + /// Whether SQLBuild parses `dialect`'s SQL for the built-in rules that read query facts. pub(crate) fn facts_available(dialect: &str) -> bool { - crate::query_facts::_helpers::facts::facts_dialect(dialect).is_some() + shared_dialect(dialect).is_some() }