diff --git a/crates/sqlclean-cli/skill/references/docs/embedding.md b/crates/sqlclean-cli/skill/references/docs/embedding.md index 231da35..4a600a2 100644 --- a/crates/sqlclean-cli/skill/references/docs/embedding.md +++ b/crates/sqlclean-cli/skill/references/docs/embedding.md @@ -91,6 +91,7 @@ the dialect. Offsets in requests, findings, edits and facts count Unicode charac | `max_literal_length` | `100` | The `SQLCLEAN044` limit | | `max_ranking_order_by` | `5` | The `SQLCLEAN043` limit | | `config_file` | `sqlclean.toml` | The configuration file name that remediation text refers to | +| `long_literal_guidance` | SQLClean's advice | The advice `SQLCLEAN044` gives before its `max_literal_length` setting, so a host can point at its own way to share a value | | `externally_referenced_ctes` | `[]` | CTE names that something outside the SQL reads, so `SQLCLEAN005` does not report them | | `relation_keys` | `{}` | Unique keys by table name as the SQL writes it, compared without case; an empty key means the relation holds at most one row, so `SQLCLEAN038` does not report a cross join with it and `SQLCLEAN018` can prove a ranking order | | `relation_names` | `{}` | The relation each placeholder table name stands for, so `SQLCLEAN038` messages and exceptions name it | diff --git a/crates/sqlclean/src/sql_lint/_helpers/engine.rs b/crates/sqlclean/src/sql_lint/_helpers/engine.rs index 5ad2889..4d28192 100644 --- a/crates/sqlclean/src/sql_lint/_helpers/engine.rs +++ b/crates/sqlclean/src/sql_lint/_helpers/engine.rs @@ -450,6 +450,10 @@ pub(crate) fn lint(request: LintRequest) -> Result { .config_file .as_deref() .unwrap_or(crate::constants::DEFAULT_CONFIG_FILE), + long_literal_guidance: request + .long_literal_guidance + .as_deref() + .unwrap_or(crate::sql_quality::constants::LONG_LITERAL_GUIDANCE), }, )?); diagnostics.sort_by_key(|diagnostic| (diagnostic.start, diagnostic.end, diagnostic.code)); diff --git a/crates/sqlclean/src/sql_lint/models.rs b/crates/sqlclean/src/sql_lint/models.rs index 52cf220..4d5cdb0 100644 --- a/crates/sqlclean/src/sql_lint/models.rs +++ b/crates/sqlclean/src/sql_lint/models.rs @@ -96,6 +96,9 @@ pub struct LintRequest { /// The configuration file remediation text names; SQLClean's own when absent. #[serde(default)] pub config_file: Option, + /// The advice `SQLCLEAN044` gives before its limit setting; SQLClean's own when absent. + #[serde(default)] + pub long_literal_guidance: Option, } /// The generated wrapper around a test or macro body: its terminal SELECT is ceremonial, and these names are generated. diff --git a/crates/sqlclean/src/sql_quality/_helpers/literals.rs b/crates/sqlclean/src/sql_quality/_helpers/literals.rs index cb250d0..90a7999 100644 --- a/crates/sqlclean/src/sql_quality/_helpers/literals.rs +++ b/crates/sqlclean/src/sql_quality/_helpers/literals.rs @@ -6,7 +6,7 @@ use polyglot_sql::tokens::TokenType; use crate::sql_lint::models::LintDiagnostic; use crate::sql_quality::constants::{ - LONG_LITERAL, LONG_LITERAL_GUIDANCE, MAX_LITERAL_LENGTH_KEY, RULES_THRESHOLDS_SECTION, + LONG_LITERAL, MAX_LITERAL_LENGTH_KEY, RULES_THRESHOLDS_SECTION, }; use crate::sql_quality::models::QualityRequest; use crate::sql_quality::setting_help::{Setting, setting_help}; @@ -24,11 +24,7 @@ pub(crate) fn diagnostics(request: &QualityRequest<'_>) -> Vec { (length > request.max_literal_length).then(|| LintDiagnostic { code: LONG_LITERAL.code, message: Cow::Borrowed(LONG_LITERAL.message), - remediation: Cow::Owned(remediation( - length, - request.max_literal_length, - request.config_file, - )), + remediation: Cow::Owned(remediation(length, request)), start: token.span.start, end: token.span.end, fix: None, @@ -38,14 +34,16 @@ pub(crate) fn diagnostics(request: &QualityRequest<'_>) -> Vec { .collect() } -fn remediation(length: usize, limit: usize, config_file: &str) -> String { +fn remediation(length: usize, request: &QualityRequest<'_>) -> String { format!( - "{LONG_LITERAL_GUIDANCE} {}", + "{} {}", + request.long_literal_guidance, setting_help(&Setting { purpose: &format!( - "To allow literals of {length} characters (the current value is {limit})" + "To allow literals of {length} characters (the current value is {})", + request.max_literal_length ), - config_file, + config_file: request.config_file, section: RULES_THRESHOLDS_SECTION, key: MAX_LITERAL_LENGTH_KEY, value: &length.to_string(), diff --git a/crates/sqlclean/src/sql_quality/models.rs b/crates/sqlclean/src/sql_quality/models.rs index 24892b2..0e49301 100644 --- a/crates/sqlclean/src/sql_quality/models.rs +++ b/crates/sqlclean/src/sql_quality/models.rs @@ -17,4 +17,6 @@ pub(crate) struct QualityRequest<'a> { pub(crate) fixture_body: bool, /// The configuration file remediation text tells the user to edit. pub(crate) config_file: &'a str, + /// The advice a long literal finding gives before its limit setting. + pub(crate) long_literal_guidance: &'a str, } diff --git a/crates/sqlclean/src/sql_quality/tests/helpers.rs b/crates/sqlclean/src/sql_quality/tests/helpers.rs index 3b8a43d..5b62492 100644 --- a/crates/sqlclean/src/sql_quality/tests/helpers.rs +++ b/crates/sqlclean/src/sql_quality/tests/helpers.rs @@ -4,9 +4,19 @@ use crate::sql_lint::main::engine::lint_json; use crate::sql_quality::tests::test_types::QualityRuleTestCase; pub(crate) fn lint(test_case: &QualityRuleTestCase) -> Result, String> { + lint_with_host_fields(test_case, "{}") +} + +/// Lint `test_case` with `host_fields`, a JSON object, merged into the request. +pub(crate) fn lint_with_host_fields( + test_case: &QualityRuleTestCase, + host_fields: &str, +) -> Result, String> { let relation_keys: Value = serde_json::from_str(test_case.relation_keys).map_err(|error| error.to_string())?; - let request: Value = json!({ + let host_fields: Value = + serde_json::from_str(host_fields).map_err(|error| error.to_string())?; + let mut request: Value = json!({ "version": 1, "sql": test_case.sql, "dialect": "duckdb", @@ -15,6 +25,15 @@ pub(crate) fn lint(test_case: &QualityRuleTestCase) -> Result, String "max_ranking_order_by": 2, "relation_keys": relation_keys, }); + request + .as_object_mut() + .ok_or("the request is not an object")? + .extend( + host_fields + .as_object() + .cloned() + .ok_or("host fields are not an object")?, + ); let response: Value = serde_json::from_str(&lint_json(&request.to_string())?) .map_err(|error| error.to_string())?; Ok(response["diagnostics"] diff --git a/crates/sqlclean/src/sql_quality/tests/test_quality_rules.rs b/crates/sqlclean/src/sql_quality/tests/test_quality_rules.rs index 132d61b..966f575 100644 --- a/crates/sqlclean/src/sql_quality/tests/test_quality_rules.rs +++ b/crates/sqlclean/src/sql_quality/tests/test_quality_rules.rs @@ -1,6 +1,6 @@ use serde_json::Value; -use crate::sql_quality::tests::helpers::{anchors, fixed, lint}; +use crate::sql_quality::tests::helpers::{anchors, fixed, lint, lint_with_host_fields}; use crate::sql_quality::tests::test_types::{QualityRemediationTestCase, QualityRuleTestCase}; #[test] @@ -345,44 +345,64 @@ fn given_threshold_findings_when_linting_then_remediation_shows_the_exact_settin description: "long literal points at shortening it and the length setting", sql: "SELECT 'pending orders awaiting shipment' AS label", rule: "SQLCLEAN044", + host_fields: "{}", expected_fragments: &[ "Shorten the literal", "set this in sqlclean.toml:", "the current value is 24", "[rules.thresholds]\n max_literal_length = 32", ], + absent_fragments: &[], + }, + QualityRemediationTestCase { + description: "a host's long literal guidance replaces SQLClean's and names its file", + sql: "SELECT 'pending orders awaiting shipment' AS label", + rule: "SQLCLEAN044", + host_fields: r#"{"long_literal_guidance": "Declare it once as a constant.", "config_file": "host.toml"}"#, + expected_fragments: &[ + "Declare it once as a constant. To allow literals of 32 characters", + "set this in host.toml:", + ], + absent_fragments: &["Shorten the literal", "sqlclean.toml"], }, QualityRemediationTestCase { description: "long ranking sort points at a row key and the sort setting", sql: "SELECT ROW_NUMBER() OVER (PARTITION BY o.customer_id ORDER BY o.a, o.b, o.order_id) AS rn FROM orders AS o", rule: "SQLCLEAN043", + host_fields: "{}", expected_fragments: &[ "unique row key as the final tie-breaker", "HASH(source_partition, source_offset, line_index) AS row_key", "the current value is 2", "[rules.thresholds]\n max_ranking_order_by = 3", ], + absent_fragments: &[], }, QualityRemediationTestCase { description: "unproven ranking points at a unique tie-breaker and a built row key", sql: "SELECT ROW_NUMBER() OVER (PARTITION BY o.customer_id ORDER BY o.a) AS rn FROM orders AS o", rule: "SQLCLEAN018", + host_fields: "{}", expected_fragments: &[ "cover a unique key of the ranked rows", "a unique, non-null tie-breaker column", "HASH(source_partition, source_offset, line_index) AS row_key", ], + absent_fragments: &[], }, ]; for test_case in &test_cases { - let diagnostics: Vec = lint(&QualityRuleTestCase { - description: test_case.description, - sql: test_case.sql, - rule: test_case.rule, - relation_keys: "{}", - expected_anchors: &[], - expected_fixed_sql: None, - })?; + let diagnostics: Vec = lint_with_host_fields( + &QualityRuleTestCase { + description: test_case.description, + sql: test_case.sql, + rule: test_case.rule, + relation_keys: "{}", + expected_anchors: &[], + expected_fixed_sql: None, + }, + test_case.host_fields, + )?; let remediation: &str = diagnostics .first() .and_then(|diagnostic| diagnostic["remediation"].as_str()) @@ -398,6 +418,18 @@ fn given_threshold_findings_when_linting_then_remediation_shows_the_exact_settin "{}: {remediation}", test_case.description ); + let leaked: Vec<&str> = test_case + .absent_fragments + .iter() + .copied() + .filter(|fragment| remediation.contains(fragment)) + .collect(); + assert_eq!( + leaked, + Vec::<&str>::new(), + "{}: {remediation}", + test_case.description + ); } Ok(()) } diff --git a/crates/sqlclean/src/sql_quality/tests/test_types.rs b/crates/sqlclean/src/sql_quality/tests/test_types.rs index 6d79a36..6c02eab 100644 --- a/crates/sqlclean/src/sql_quality/tests/test_types.rs +++ b/crates/sqlclean/src/sql_quality/tests/test_types.rs @@ -19,7 +19,10 @@ pub(crate) struct QualityRemediationTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, pub(crate) rule: &'static str, + /// Extra request fields as a JSON object, as a host sends them. + pub(crate) host_fields: &'static str, pub(crate) expected_fragments: &'static [&'static str], + pub(crate) absent_fragments: &'static [&'static str], } pub(crate) struct JsonParseTestCase { diff --git a/website/src/content/docs/docs/embedding.mdx b/website/src/content/docs/docs/embedding.mdx index 6129294..bbe6d23 100644 --- a/website/src/content/docs/docs/embedding.mdx +++ b/website/src/content/docs/docs/embedding.mdx @@ -77,6 +77,7 @@ the dialect. Offsets in requests, findings, edits and facts count Unicode charac | `max_literal_length` | `100` | The `SQLCLEAN044` limit | | `max_ranking_order_by` | `5` | The `SQLCLEAN043` limit | | `config_file` | `sqlclean.toml` | The configuration file name that remediation text refers to | +| `long_literal_guidance` | SQLClean's advice | The advice `SQLCLEAN044` gives before its `max_literal_length` setting, so a host can point at its own way to share a value | | `externally_referenced_ctes` | `[]` | CTE names that something outside the SQL reads, so `SQLCLEAN005` does not report them | | `relation_keys` | `{}` | Unique keys by table name as the SQL writes it, compared without case; an empty key means the relation holds at most one row, so `SQLCLEAN038` does not report a cross join with it and `SQLCLEAN018` can prove a ranking order | | `relation_names` | `{}` | The relation each placeholder table name stands for, so `SQLCLEAN038` messages and exceptions name it |