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 1/4] 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 2/4] 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 3/4] 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 4/4] 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: