Skip to content

Commit cdc6a61

Browse files
authored
feat: report unsafe formatting and add join-predicate and final-CTE rules (#822)
* fix: report unsafe formatting and preserve commented implicit aliases * feat: add plain join predicate and final CTE naming rules * fix: preserve authored SQL syntax through native formatting * fix: recognize wrapped root CTEs and literal-only join predicates * fix: preserve interpolation spelling and separate macro CTE items * fix: merge identical formatter syntax branches * test: expect authored empty-fixture spelling after formatting
1 parent be1847e commit cdc6a61

23 files changed

Lines changed: 1675 additions & 166 deletions

File tree

‎README.md‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,18 +182,39 @@ physical line width in `sqlbuild_project.toml` (the default is `100`):
182182
line_width = 100
183183
```
184184

185-
Select rules in `sqlbuild_project.toml` by exact code or derived family prefix:
185+
Prefer family selection in `sqlbuild_project.toml` over hand-maintained lists of exact codes:
186186

187187
```toml
188188
[rules]
189-
select = ["SQBRSQL", "XSQBRARCH"]
189+
select = ["SQBRSQL", "SQBRMODEL", "SQBRGRAPH", "XSQBRARCH"]
190190
ignore = ["SQBRSQL004"]
191191
```
192192

193193
Built-in codes use `SQBR<FAMILY><three digits>`, such as `SQBRSQL001` and
194194
`SQBRGRAPH101`. Custom codes use `XSQBR<optional family><three digits>`, such as
195195
`XSQBRARCH001`. A family is always the code with its final three digits removed.
196196

197+
Family selectors automatically include new built-in rules on upgrade. Exact-code lists retain
198+
their current membership and must be updated manually. In particular, selecting `SQBRSQL` enables
199+
`SQBRSQL040` (plain JOIN predicates) and `SQBRSQL041` (terminal CTE naming); existing projects may
200+
need to extract computed join keys into input CTEs and rename their last CTE to `final`.
201+
See the [SQL rule conventions](docs/native-lint-rule-matrix.md#join-keys-and-final-cte-names)
202+
for their exact scope and relationship to `SQBRSQL035`.
203+
204+
`sqb format` reports a file-specific `format-unsafe` fault whenever a SQL body cannot be safely
205+
formatted, including parser, comment-attachment, interpolation-restoration, and idempotence
206+
failures. The entire original file is retained (including its headers and fixtures), the reason
207+
appears in human and JSON output, and both
208+
formatting and `sqb format --check` exit nonzero. A declined body is never counted as canonical.
209+
210+
Formatting preserves authored cast types, postfix casts, quoted literals, variant paths, typed
211+
lambda parameters, and supported SQL function spellings while applying canonical layout. It uses
212+
the compiler's trusted-SQL function-depth budget and does not impose the separate browser-oriented
213+
UNION-chain limit from Polyglot's convenience formatting API. SQLBuild calls retain their authored
214+
spelling, including zero-argument cursor and empty-fixture intrinsics. CTE-producing macros remain
215+
authored calls rather than expanded project SQL, with each call on its own CTE-list line and
216+
leading comments attached to the node they describe.
217+
197218
Custom rules are ordinary Python beneath `rules/**/*.py`. Only `@rule` functions register; helper
198219
functions, constants, dataclasses, classes, and nested packages remain ordinary Python. Typed,
199220
keyword-only parameters determine whether a rule runs once per model or once per project:

‎crates/sqlbuild-rules-native/src/sql_lint/_helpers/engine.rs‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,16 @@ const INLINE_QUERY_RELATION: LintRuleMetadata = LintRuleMetadata {
229229
remediation: "Extract the derived relation into a named top-level CTE; use explicit LATERAL only when the relation must remain correlated.",
230230
};
231231
const TRIVIAL_EQUALITY_TOKEN_COUNT: usize = 3;
232+
const JOIN_EXPRESSION: LintRuleMetadata = LintRuleMetadata {
233+
code: "SQBRSQL040",
234+
message: "JOIN ON contains a computed expression",
235+
remediation: "Compute keys, including variant path access, in input CTEs and join on plain columns. Literal-only predicates such as ON TRUE are allowed.",
236+
};
237+
const FINAL_CTE_NAME: LintRuleMetadata = LintRuleMetadata {
238+
code: "SQBRSQL041",
239+
message: "Only the last top-level CTE may be named final, and it must use that name",
240+
remediation: "Name the last top-level CTE final and rename any other final CTE; SQBRSQL035 checks the terminal SELECT.",
241+
};
232242

233243
const DEFAULT_RULES: [&str; 13] = [
234244
NULL_COMPARISON.code,
@@ -246,7 +256,7 @@ const DEFAULT_RULES: [&str; 13] = [
246256
INLINE_QUERY_RELATION.code,
247257
];
248258

249-
const ALL_RULE_METADATA: [&LintRuleMetadata; 39] = [
259+
const ALL_RULE_METADATA: [&LintRuleMetadata; 41] = [
250260
&NULL_COMPARISON,
251261
&IMPLICIT_CARTESIAN_JOIN,
252262
&JOIN_WITHOUT_CONDITION,
@@ -286,6 +296,8 @@ const ALL_RULE_METADATA: [&LintRuleMetadata; 39] = [
286296
&RECURSIVE_CTE,
287297
&CROSS_JOIN,
288298
&INLINE_QUERY_RELATION,
299+
&JOIN_EXPRESSION,
300+
&FINAL_CTE_NAME,
289301
];
290302

291303
fn is_ceremonial_cte_name(name: &str) -> bool {
@@ -395,6 +407,25 @@ pub(crate) fn lint(request: LintRequest) -> Result<LintResponse, String> {
395407
enabled: &enabled,
396408
};
397409
let mut diagnostics = diagnostics(&context);
410+
let depths = token_depths(&tokens);
411+
if enabled.contains(JOIN_EXPRESSION.code) {
412+
diagnostics.extend(diagnostics_for_spans(
413+
&JOIN_EXPRESSION,
414+
&crate::sql_lint::_helpers::join_predicates::join_expression_spans(
415+
&tokens,
416+
&depths,
417+
dialect_type,
418+
),
419+
Some("computing a join key requires an authored CTE rewrite"),
420+
));
421+
}
422+
if enabled.contains(FINAL_CTE_NAME.code) && !request.allows_ceremonial_select {
423+
diagnostics.extend(diagnostics_for_spans(
424+
&FINAL_CTE_NAME,
425+
&crate::sql_lint::_helpers::terminal_shape::final_cte_name_spans(&tokens, &depths),
426+
Some("renaming a CTE requires updating its references"),
427+
));
428+
}
398429
diagnostics.sort_by_key(|diagnostic| (diagnostic.start, diagnostic.end, diagnostic.code));
399430
Ok(LintResponse {
400431
version: LINT_API_VERSION,
@@ -508,6 +539,8 @@ fn collect_rule_migration_facts(
508539
.filter(|&index| !is_layout(&tokens[index]) && !is_comment(&tokens[index]))
509540
.collect();
510541

542+
let root_with = crate::sql_lint::_helpers::terminal_shape::root_with_indices(tokens, depths);
543+
511544
for (position, &index) in significant.iter().enumerate() {
512545
let text = tokens[index].text.as_str();
513546
if text.eq_ignore_ascii_case("cross")
@@ -517,11 +550,11 @@ fn collect_rule_migration_facts(
517550
{
518551
facts.cross_joins.push(tokens[index].span);
519552
}
520-
if text.eq_ignore_ascii_case("with") {
553+
if tokens[index].token_type == TokenType::With {
521554
let starts_hierarchy = position.checked_sub(1).is_some_and(|previous| {
522555
tokens[significant[previous]].token_type == TokenType::Start
523556
});
524-
if depths[index] > 0 && !starts_hierarchy {
557+
if !root_with.contains(&index) && !starts_hierarchy {
525558
facts.nested_ctes.push(tokens[index].span);
526559
}
527560
if !starts_hierarchy

0 commit comments

Comments
 (0)