Skip to content

Commit 14f613c

Browse files
authored
docs: simplify introductory custom Python Rule examples (#34)
Use public function facts for a concise report-only IFNULL convention across the README, homepage and custom Rule overview. Exercise the documented snippets and synchronize the bundled documentation reference.
1 parent cf79656 commit 14f613c

8 files changed

Lines changed: 161 additions & 143 deletions

File tree

‎README.md‎

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -85,56 +85,45 @@ SQLClean in other configurations, on the same files:
8585
## Custom Rules in Python
8686

8787
A custom Rule is a Python function over each statement, already parsed and located by the Rust
88-
engine. This one enforces a team convention, not a rule of SQL: the columns the team lists in
89-
`EMAIL_COLUMNS` are compared case-insensitively. An `=` or `<>` that reads one of them, bare or as
90-
`LOWER(email)`, passes only when both sides are `LOWER(...)` or a lower-case string literal. It
91-
reads comparisons from the statement's syntax tree, so comments and string literals never match,
92-
and it only reports, because changing a comparison is the author's call.
88+
engine. This team uses `COALESCE` instead of `IFNULL` to keep queries portable. The Rule reads
89+
function facts, not SQL text, so comments and string literals don't match. It only reports;
90+
the author chooses the replacement.
9391

9492
```python
9593
# sql_rules/team.py
96-
import polyglot_sql as ast # pip install "sqlclean[ast]"
97-
9894
from sqlclean import Finding, RuleContext, Statement, rule
9995

100-
EMAIL_COLUMNS = {"email", "billing_email"} # the columns this team treats as email addresses
101-
102-
103-
def email_column(operand: ast.Expression) -> ast.Column | None:
104-
"""The designated email column an operand reads, written bare or as LOWER(column)."""
105-
column = operand.this if isinstance(operand, ast.Lower) else operand
106-
is_email = isinstance(column, ast.Column) and column.name.lower() in EMAIL_COLUMNS
107-
return column if is_email else None
108-
109-
110-
def lowered(operand: ast.Expression) -> bool:
111-
"""Whether an operand is LOWER(...) or a string literal with no upper-case letters."""
112-
literal = isinstance(operand, ast.Literal) and operand.is_string
113-
return isinstance(operand, ast.Lower) or (literal and operand.name == operand.name.lower())
114-
11596

11697
@rule(
11798
code="XSQLCLEANTEAM001",
118-
message="Email comparisons must be case-insensitive",
119-
remediation="Compare LOWER(email) with LOWER(...) instead.",
99+
message="Use COALESCE instead of IFNULL",
100+
remediation="Write COALESCE(value, fallback).",
120101
enabled_by_default=True,
121102
)
122-
def case_insensitive_email(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
123-
starts = {
124-
column.to_dict()["column"]["span"]["start"]
125-
for comparison in statement.polyglot_ast().find_all(ast.Eq, ast.Neq)
126-
for column in map(email_column, (comparison.this, comparison.expression))
127-
if column is not None and not all(map(lowered, (comparison.this, comparison.expression)))
128-
}
103+
def prefer_coalesce(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
129104
return [
130-
ctx.finding(subject=statement, node=column)
131-
for column in statement.columns
132-
if column.start - statement.start in starts
105+
ctx.finding(subject=statement, node=function)
106+
for function in statement.functions
107+
if function.name.upper() == "IFNULL"
133108
]
134109
```
135110

136-
Point `[rules] custom = ["sql_rules"]` in `sqlclean.toml` at it. The syntax tree needs the `ast`
137-
extra: `pip install "sqlclean[ast]"`. See
111+
Point `[rules] custom = ["sql_rules"]` in `sqlclean.toml` at it, then run `sqlclean check`:
112+
113+
```sql
114+
SELECT IFNULL(discount, 0) FROM orders;
115+
```
116+
117+
```text
118+
error[XSQLCLEANTEAM001]: Use COALESCE instead of IFNULL
119+
--> orders.sql:1:8
120+
|
121+
1 | SELECT IFNULL(discount, 0) FROM orders;
122+
| ^^^^^^
123+
= help: Write COALESCE(value, fallback).
124+
```
125+
126+
No extra dependencies are needed. See
138127
[custom Rules](https://sqlclean.io/docs/custom-rules/overview/),
139128
[statement facts](https://sqlclean.io/docs/custom-rules/statement-facts/) and
140129
[testing custom Rules](https://sqlclean.io/docs/custom-rules/testing/).

‎crates/sqlclean-cli/skill/references/docs/custom-rules/overview.md‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,25 @@ A custom Rule is a Python function decorated with `@rule`. It receives a stateme
2121
whole repository, and returns findings. Custom Rules use the same selection, exceptions, fixes,
2222
output and cache as the built-in Rules.
2323

24+
This team uses `COALESCE` instead of `IFNULL` to keep queries portable. The Rule reads function
25+
facts, so comments and string literals don't match. It only reports, leaving the replacement to
26+
the author. No extra dependencies are needed.
27+
2428
```python
25-
# sql_rules/naming.py
29+
# sql_rules/team.py
2630
from sqlclean import Finding, RuleContext, Statement, rule
2731

2832
@rule(
29-
code="XSQLCLEANNAME001",
30-
message="Query reads a temporary table",
31-
remediation="Read from the published table instead.",
33+
code="XSQLCLEANTEAM001",
34+
message="Use COALESCE instead of IFNULL",
35+
remediation="Write COALESCE(value, fallback).",
3236
enabled_by_default=True,
3337
)
34-
def temporary_table(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
38+
def prefer_coalesce(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
3539
return [
36-
ctx.finding(subject=statement, node=table)
37-
for table in statement.tables
38-
if table.name.startswith("tmp_")
40+
ctx.finding(subject=statement, node=function)
41+
for function in statement.functions
42+
if function.name.upper() == "IFNULL"
3943
]
4044
```
4145

@@ -46,15 +50,23 @@ Point `[rules] custom` at the file or its directory:
4650
custom = ["sql_rules"]
4751
```
4852

53+
Run `sqlclean check` on `orders.sql`:
54+
55+
```sql
56+
SELECT IFNULL(discount, 0) FROM orders;
57+
```
58+
4959
```text
50-
error[XSQLCLEANNAME001]: Query reads a temporary table
51-
--> models/orders.sql:2:6
60+
error[XSQLCLEANTEAM001]: Use COALESCE instead of IFNULL
61+
--> orders.sql:1:8
5262
|
53-
2 | FROM tmp_orders
54-
| ^^^^^^^^^^
55-
= help: Read from the published table instead.
63+
1 | SELECT IFNULL(discount, 0) FROM orders;
64+
| ^^^^^^
65+
= help: Write COALESCE(value, fallback).
5666
```
5767

68+
Write `SELECT COALESCE(discount, 0) FROM orders;` to satisfy this convention.
69+
5870
## Where Rules live
5971

6072
`[rules] custom` lists Python files or directories, relative to the configuration root. SQLClean
@@ -63,8 +75,8 @@ functions, constants and classes stay ordinary Python.
6375

6476
- Files named `test_*.py`, `conftest.py` and `__init__.py` are not loaded as Rules, so tests can
6577
live next to the Rules they test.
66-
- Modules are imported by their path from the configuration root: `sql_rules/naming.py` is
67-
`sql_rules.naming`. A Rule module can import a helper the same way, for example
78+
- Modules are imported by their path from the configuration root: `sql_rules/team.py` is
79+
`sql_rules.team`. A Rule module can import a helper the same way, for example
6880
`from sql_rules.helpers import table_prefixes`.
6981
- Two Rules with the same code are an error: `custom Rule code XSQLCLEANNAME001 is defined twice`.
7082

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ packages = ["src/sqlclean"]
4242
dev = [
4343
"commitizen>=4.9.1",
4444
"fensu>=0.28.0",
45-
"polyglot-sql-chio==0.13.10", # the ast extra, for the README custom Rule test
45+
"polyglot-sql-chio==0.13.10", # optional AST API
4646
"pre-commit>=4.3.0",
4747
"pytest>=8.4.2",
4848
"pytest-xdist>=3.8.0",

‎tests/integration/src/sqlclean/rule_testing/_test_types.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class HarnessFailureTestCase:
3030

3131
@dataclass(frozen=True)
3232
class ReadmeRuleTestCase:
33-
"""SQL for the README custom Rule and the text of every column it must report."""
33+
"""SQL for the introductory custom Rule and the text of every function it must report."""
3434

3535
description: str
3636
source: str

‎tests/integration/src/sqlclean/rule_testing/helpers.py‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@
33
from __future__ import annotations
44

55
import importlib.util
6+
import re
7+
from html import unescape
68
from importlib.machinery import ModuleSpec
79
from pathlib import Path
810
from types import ModuleType
911

12+
import pytest
13+
1014
from sqlclean.rules.types import RuleCheck
1115

1216
SAMPLE_RULES: str = """
@@ -106,19 +110,32 @@ def sample_rule(directory: Path, name: str) -> RuleCheck:
106110

107111

108112
README: Path = Path(__file__).resolve().parents[5] / "README.md"
109-
BLOCK_START: str = "```python\n# sql_rules/team.py\n"
113+
INTRODUCTIONS: tuple[Path, ...] = (
114+
README,
115+
README.parent / "website/public/index.html",
116+
README.parent / "website/src/content/docs/docs/custom-rules/overview.mdx",
117+
)
118+
BLOCK_START: str = "# sql_rules/team.py\n"
119+
120+
121+
@pytest.fixture(params=INTRODUCTIONS, ids=lambda path: path.name)
122+
def document(request: pytest.FixtureRequest) -> Path:
123+
"""Run each case against every published introductory snippet."""
124+
125+
path: Path = request.param
126+
return path
110127

111128

112-
def readme_rule(directory: Path) -> RuleCheck:
113-
"""Return the README's case_insensitive_email Rule, imported from its code block."""
129+
def readme_rule(directory: Path, document: Path = README) -> RuleCheck:
130+
"""Import prefer_coalesce from an introductory example, removing HTML markup if needed."""
114131

115-
text: str = README.read_text(encoding="utf-8")
132+
text: str = unescape(re.sub(r"</?span\b[^>]*>", "", document.read_text(encoding="utf-8")))
116133
start: int = text.index(BLOCK_START) + len(BLOCK_START)
117134
path: Path = directory / "readme_team.py"
118-
path.write_text(text[start : text.index("```", start)], encoding="utf-8")
135+
path.write_text(text[start:].split("```")[0].split("# sqlclean.toml:")[0], encoding="utf-8")
119136
spec: ModuleSpec | None = importlib.util.spec_from_file_location("readme_team", path)
120137
assert spec is not None and spec.loader is not None
121138
module: ModuleType = importlib.util.module_from_spec(spec)
122139
spec.loader.exec_module(module)
123-
check: RuleCheck = module.case_insensitive_email
140+
check: RuleCheck = module.prefer_coalesce
124141
return check
Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""The custom Rule example in README.md, run as written through the Rule harness."""
1+
"""The introductory custom Rule examples, run as written through the Rule harness."""
22

33
from __future__ import annotations
44

@@ -9,66 +9,48 @@
99
from sqlclean.rules.types import RuleCheck
1010
from sqlclean.testing import RuleCase, RuleResult, evaluate_rule
1111
from tests.integration.src.sqlclean.rule_testing._test_types import ReadmeRuleTestCase
12+
from tests.integration.src.sqlclean.rule_testing.helpers import document as document
1213
from tests.integration.src.sqlclean.rule_testing.helpers import readme_rule
1314

1415

1516
@pytest.mark.parametrize(
1617
"test_case",
1718
[
1819
ReadmeRuleTestCase(
19-
description="a bare email column compared with a mixed-case literal",
20-
source="SELECT id FROM customers c WHERE c.email = 'Alice@example.com'",
21-
expected_texts=("c.email",),
20+
description="the documented SQL reports the function name",
21+
source="SELECT IFNULL(discount, 0) FROM orders;",
22+
expected_texts=("IFNULL",),
2223
),
2324
ReadmeRuleTestCase(
24-
description="LOWER on one side only still compares with a mixed-case literal",
25-
source="SELECT id FROM customers WHERE LOWER(email) = 'Alice@Example.com'",
26-
expected_texts=("email",),
25+
description="mixed-case calls nested in a CTE and subquery are reported",
26+
source="WITH totals AS (SELECT ifnull(discount, 0) AS discount FROM orders) "
27+
"SELECT (SELECT IfNull(MAX(discount), 0) FROM totals)",
28+
expected_texts=("ifnull", "IfNull"),
2729
),
2830
ReadmeRuleTestCase(
29-
description="a designated column on the right of <> in upper case",
30-
source="SELECT id FROM customers WHERE 'Carol@example.com' <> BILLING_EMAIL",
31-
expected_texts=("BILLING_EMAIL",),
31+
description="a later statement after a multibyte literal uses file character offsets",
32+
source="SELECT 'é';\nSELECT IFNULL(discount, 0) FROM orders;",
33+
expected_texts=("IFNULL",),
3234
),
3335
ReadmeRuleTestCase(
34-
description="UPPER does not normalize the other side to lower case",
35-
source="SELECT id FROM customers WHERE LOWER(email) = UPPER('x')",
36-
expected_texts=("email",),
37-
),
38-
ReadmeRuleTestCase(
39-
description="a bare column against a lower-case literal is still not normalized",
40-
source="SELECT id FROM customers WHERE email = 'carol@example.com'",
41-
expected_texts=("email",),
42-
),
43-
ReadmeRuleTestCase(
44-
description="a comparison inside a CTE after a multibyte literal",
45-
source="WITH u AS (SELECT 'é' AS m, email FROM users WHERE email = 'X@y.com') "
46-
"SELECT * FROM u",
47-
expected_texts=("email",),
48-
),
49-
ReadmeRuleTestCase(
50-
description="LOWER on both sides passes",
51-
source="SELECT * FROM u JOIN a ON LOWER(u.email) = LOWER(a.email)",
52-
expected_texts=(),
53-
),
54-
ReadmeRuleTestCase(
55-
description="LOWER against a lower-case literal passes",
56-
source="SELECT id FROM customers WHERE LOWER(email) = 'bob@example.com'",
36+
description="the recommended COALESCE replacement passes",
37+
source="SELECT COALESCE(discount, 0) FROM orders;",
5738
expected_texts=(),
5839
),
5940
ReadmeRuleTestCase(
60-
description="comments, strings, other columns, LIKE and projections never match",
61-
source="-- email = 'A@b.c'\nSELECT email, 'email = x' AS note FROM customers "
62-
"WHERE email_verified = TRUE AND email LIKE '%dave%'",
41+
description="comments, strings, identifiers and longer function names never match",
42+
source="-- IFNULL(discount, 0)\n"
43+
"SELECT 'IFNULL(discount, 0)' AS note, \"IFNULL\", "
44+
"my_ifnull(discount, 0) FROM orders /* IFNULL(discount, 0) */",
6345
expected_texts=(),
6446
),
6547
],
6648
ids=lambda case: case.description,
6749
)
68-
def test_given_readme_email_rule_when_evaluating_then_unnormalized_comparisons_are_reported(
69-
tmp_path: Path, test_case: ReadmeRuleTestCase
50+
def test_given_introductory_rule_when_evaluating_then_only_ifnull_calls_are_reported(
51+
tmp_path: Path, document: Path, test_case: ReadmeRuleTestCase
7052
) -> None:
71-
rule: RuleCheck = readme_rule(tmp_path)
53+
rule: RuleCheck = readme_rule(tmp_path, document)
7254

7355
result: RuleResult = evaluate_rule(
7456
rule=rule,
@@ -84,4 +66,5 @@ def test_given_readme_email_rule_when_evaluating_then_unnormalized_comparisons_a
8466
)
8567

8668
assert texts == test_case.expected_texts
69+
assert all(finding.edits == () for finding in result.findings)
8770
assert result.fixed_source == test_case.source

0 commit comments

Comments
 (0)