Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 25 additions & 36 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,56 +85,45 @@ SQLClean in other configurations, on the same files:
## Custom Rules in Python

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

```python
# sql_rules/team.py
import polyglot_sql as ast # pip install "sqlclean[ast]"

from sqlclean import Finding, RuleContext, Statement, rule

EMAIL_COLUMNS = {"email", "billing_email"} # the columns this team treats as email addresses


def email_column(operand: ast.Expression) -> ast.Column | None:
"""The designated email column an operand reads, written bare or as LOWER(column)."""
column = operand.this if isinstance(operand, ast.Lower) else operand
is_email = isinstance(column, ast.Column) and column.name.lower() in EMAIL_COLUMNS
return column if is_email else None


def lowered(operand: ast.Expression) -> bool:
"""Whether an operand is LOWER(...) or a string literal with no upper-case letters."""
literal = isinstance(operand, ast.Literal) and operand.is_string
return isinstance(operand, ast.Lower) or (literal and operand.name == operand.name.lower())


@rule(
code="XSQLCLEANTEAM001",
message="Email comparisons must be case-insensitive",
remediation="Compare LOWER(email) with LOWER(...) instead.",
message="Use COALESCE instead of IFNULL",
remediation="Write COALESCE(value, fallback).",
enabled_by_default=True,
)
def case_insensitive_email(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
starts = {
column.to_dict()["column"]["span"]["start"]
for comparison in statement.polyglot_ast().find_all(ast.Eq, ast.Neq)
for column in map(email_column, (comparison.this, comparison.expression))
if column is not None and not all(map(lowered, (comparison.this, comparison.expression)))
}
def prefer_coalesce(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
return [
ctx.finding(subject=statement, node=column)
for column in statement.columns
if column.start - statement.start in starts
ctx.finding(subject=statement, node=function)
for function in statement.functions
if function.name.upper() == "IFNULL"
]
```

Point `[rules] custom = ["sql_rules"]` in `sqlclean.toml` at it. The syntax tree needs the `ast`
extra: `pip install "sqlclean[ast]"`. See
Point `[rules] custom = ["sql_rules"]` in `sqlclean.toml` at it, then run `sqlclean check`:

```sql
SELECT IFNULL(discount, 0) FROM orders;
```

```text
error[XSQLCLEANTEAM001]: Use COALESCE instead of IFNULL
--> orders.sql:1:8
|
1 | SELECT IFNULL(discount, 0) FROM orders;
| ^^^^^^
= help: Write COALESCE(value, fallback).
```

No extra dependencies are needed. See
[custom Rules](https://sqlclean.io/docs/custom-rules/overview/),
[statement facts](https://sqlclean.io/docs/custom-rules/statement-facts/) and
[testing custom Rules](https://sqlclean.io/docs/custom-rules/testing/).
Expand Down
42 changes: 27 additions & 15 deletions crates/sqlclean-cli/skill/references/docs/custom-rules/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,25 @@ A custom Rule is a Python function decorated with `@rule`. It receives a stateme
whole repository, and returns findings. Custom Rules use the same selection, exceptions, fixes,
output and cache as the built-in Rules.

This team uses `COALESCE` instead of `IFNULL` to keep queries portable. The Rule reads function
facts, so comments and string literals don't match. It only reports, leaving the replacement to
the author. No extra dependencies are needed.

```python
# sql_rules/naming.py
# sql_rules/team.py
from sqlclean import Finding, RuleContext, Statement, rule

@rule(
code="XSQLCLEANNAME001",
message="Query reads a temporary table",
remediation="Read from the published table instead.",
code="XSQLCLEANTEAM001",
message="Use COALESCE instead of IFNULL",
remediation="Write COALESCE(value, fallback).",
enabled_by_default=True,
)
def temporary_table(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
def prefer_coalesce(*, statement: Statement, ctx: RuleContext) -> list[Finding]:
return [
ctx.finding(subject=statement, node=table)
for table in statement.tables
if table.name.startswith("tmp_")
ctx.finding(subject=statement, node=function)
for function in statement.functions
if function.name.upper() == "IFNULL"
]
```

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

Run `sqlclean check` on `orders.sql`:

```sql
SELECT IFNULL(discount, 0) FROM orders;
```

```text
error[XSQLCLEANNAME001]: Query reads a temporary table
--> models/orders.sql:2:6
error[XSQLCLEANTEAM001]: Use COALESCE instead of IFNULL
--> orders.sql:1:8
|
2 | FROM tmp_orders
| ^^^^^^^^^^
= help: Read from the published table instead.
1 | SELECT IFNULL(discount, 0) FROM orders;
| ^^^^^^
= help: Write COALESCE(value, fallback).
```

Write `SELECT COALESCE(discount, 0) FROM orders;` to satisfy this convention.

## Where Rules live

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

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

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ packages = ["src/sqlclean"]
dev = [
"commitizen>=4.9.1",
"fensu>=0.28.0",
"polyglot-sql-chio==0.13.10", # the ast extra, for the README custom Rule test
"polyglot-sql-chio==0.13.10", # optional AST API
"pre-commit>=4.3.0",
"pytest>=8.4.2",
"pytest-xdist>=3.8.0",
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/src/sqlclean/rule_testing/_test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class HarnessFailureTestCase:

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

description: str
source: str
Expand Down
29 changes: 23 additions & 6 deletions tests/integration/src/sqlclean/rule_testing/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
from __future__ import annotations

import importlib.util
import re
from html import unescape
from importlib.machinery import ModuleSpec
from pathlib import Path
from types import ModuleType

import pytest

from sqlclean.rules.types import RuleCheck

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


README: Path = Path(__file__).resolve().parents[5] / "README.md"
BLOCK_START: str = "```python\n# sql_rules/team.py\n"
INTRODUCTIONS: tuple[Path, ...] = (
README,
README.parent / "website/public/index.html",
README.parent / "website/src/content/docs/docs/custom-rules/overview.mdx",
)
BLOCK_START: str = "# sql_rules/team.py\n"


@pytest.fixture(params=INTRODUCTIONS, ids=lambda path: path.name)
def document(request: pytest.FixtureRequest) -> Path:
"""Run each case against every published introductory snippet."""

path: Path = request.param
return path


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

text: str = README.read_text(encoding="utf-8")
text: str = unescape(re.sub(r"</?span\b[^>]*>", "", document.read_text(encoding="utf-8")))
start: int = text.index(BLOCK_START) + len(BLOCK_START)
path: Path = directory / "readme_team.py"
path.write_text(text[start : text.index("```", start)], encoding="utf-8")
path.write_text(text[start:].split("```")[0].split("# sqlclean.toml:")[0], encoding="utf-8")
spec: ModuleSpec | None = importlib.util.spec_from_file_location("readme_team", path)
assert spec is not None and spec.loader is not None
module: ModuleType = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
check: RuleCheck = module.case_insensitive_email
check: RuleCheck = module.prefer_coalesce
return check
61 changes: 22 additions & 39 deletions tests/integration/src/sqlclean/rule_testing/test_readme_rule.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""The custom Rule example in README.md, run as written through the Rule harness."""
"""The introductory custom Rule examples, run as written through the Rule harness."""

from __future__ import annotations

Expand All @@ -9,66 +9,48 @@
from sqlclean.rules.types import RuleCheck
from sqlclean.testing import RuleCase, RuleResult, evaluate_rule
from tests.integration.src.sqlclean.rule_testing._test_types import ReadmeRuleTestCase
from tests.integration.src.sqlclean.rule_testing.helpers import document as document
from tests.integration.src.sqlclean.rule_testing.helpers import readme_rule


@pytest.mark.parametrize(
"test_case",
[
ReadmeRuleTestCase(
description="a bare email column compared with a mixed-case literal",
source="SELECT id FROM customers c WHERE c.email = 'Alice@example.com'",
expected_texts=("c.email",),
description="the documented SQL reports the function name",
source="SELECT IFNULL(discount, 0) FROM orders;",
expected_texts=("IFNULL",),
),
ReadmeRuleTestCase(
description="LOWER on one side only still compares with a mixed-case literal",
source="SELECT id FROM customers WHERE LOWER(email) = 'Alice@Example.com'",
expected_texts=("email",),
description="mixed-case calls nested in a CTE and subquery are reported",
source="WITH totals AS (SELECT ifnull(discount, 0) AS discount FROM orders) "
"SELECT (SELECT IfNull(MAX(discount), 0) FROM totals)",
expected_texts=("ifnull", "IfNull"),
),
ReadmeRuleTestCase(
description="a designated column on the right of <> in upper case",
source="SELECT id FROM customers WHERE 'Carol@example.com' <> BILLING_EMAIL",
expected_texts=("BILLING_EMAIL",),
description="a later statement after a multibyte literal uses file character offsets",
source="SELECT 'é';\nSELECT IFNULL(discount, 0) FROM orders;",
expected_texts=("IFNULL",),
),
ReadmeRuleTestCase(
description="UPPER does not normalize the other side to lower case",
source="SELECT id FROM customers WHERE LOWER(email) = UPPER('x')",
expected_texts=("email",),
),
ReadmeRuleTestCase(
description="a bare column against a lower-case literal is still not normalized",
source="SELECT id FROM customers WHERE email = 'carol@example.com'",
expected_texts=("email",),
),
ReadmeRuleTestCase(
description="a comparison inside a CTE after a multibyte literal",
source="WITH u AS (SELECT 'é' AS m, email FROM users WHERE email = 'X@y.com') "
"SELECT * FROM u",
expected_texts=("email",),
),
ReadmeRuleTestCase(
description="LOWER on both sides passes",
source="SELECT * FROM u JOIN a ON LOWER(u.email) = LOWER(a.email)",
expected_texts=(),
),
ReadmeRuleTestCase(
description="LOWER against a lower-case literal passes",
source="SELECT id FROM customers WHERE LOWER(email) = 'bob@example.com'",
description="the recommended COALESCE replacement passes",
source="SELECT COALESCE(discount, 0) FROM orders;",
expected_texts=(),
),
ReadmeRuleTestCase(
description="comments, strings, other columns, LIKE and projections never match",
source="-- email = 'A@b.c'\nSELECT email, 'email = x' AS note FROM customers "
"WHERE email_verified = TRUE AND email LIKE '%dave%'",
description="comments, strings, identifiers and longer function names never match",
source="-- IFNULL(discount, 0)\n"
"SELECT 'IFNULL(discount, 0)' AS note, \"IFNULL\", "
"my_ifnull(discount, 0) FROM orders /* IFNULL(discount, 0) */",
expected_texts=(),
),
],
ids=lambda case: case.description,
)
def test_given_readme_email_rule_when_evaluating_then_unnormalized_comparisons_are_reported(
tmp_path: Path, test_case: ReadmeRuleTestCase
def test_given_introductory_rule_when_evaluating_then_only_ifnull_calls_are_reported(
tmp_path: Path, document: Path, test_case: ReadmeRuleTestCase
) -> None:
rule: RuleCheck = readme_rule(tmp_path)
rule: RuleCheck = readme_rule(tmp_path, document)

result: RuleResult = evaluate_rule(
rule=rule,
Expand All @@ -84,4 +66,5 @@ def test_given_readme_email_rule_when_evaluating_then_unnormalized_comparisons_a
)

assert texts == test_case.expected_texts
assert all(finding.edits == () for finding in result.findings)
assert result.fixed_source == test_case.source
Loading
Loading