diff --git a/CHANGELOG.md b/CHANGELOG.md index efb1e73..185e6e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ a deprecation window (see `GOVERNANCE.md` ยง Scope discipline). ### Added +- **W014 `case-without-else`** - warns when a `CASE ... END` block has + no `ELSE` branch, so unmatched rows return `NULL`. Walks the + statement token-by-token tracking nesting, so an outer `CASE` with no + `ELSE` still fires even when an inner `CASE` does have one. Fires per + unmatched block. Suggests adding `ELSE NULL` for explicitness. + Contributed by [@hellozzm](https://github.com/hellozzm) + ([#32](https://github.com/Pawansingh3889/sql-guard/pull/32)). - **W015 `join-function-on-column`** - warns when a function wraps a column inside a `JOIN ... ON` predicate, the JOIN-side companion to W003. `JOIN customers c ON UPPER(o.email) = UPPER(c.email)` defeats every diff --git a/README.md b/README.md index 08577c6..b14eb76 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ One bad SQL query can delete production data, expose customer records, or bring | | | |---|---| -| Rules | 41 (10 errors, 26 warnings, 5 Python-source) | +| Rules | 42 (10 errors, 27 warnings, 5 Python-source) | | Tests | 152 | | Coverage | 86% | | Scan speed | 0.08s across 200 files | @@ -43,7 +43,7 @@ print(result.summary()) # "1 error, 0 warnings in 1 statement" --- -Fast, rule-based SQL linter. 41 rules (36 SQL + 5 Python), including SQL Server-focused rules for T-SQL shops. Inline disable, project config, git-changed-only mode, and SARIF output for GitHub Code Scanning. 500+ monthly downloads on PyPI. +Fast, rule-based SQL linter. 42 rules (37 SQL + 5 Python), including SQL Server-focused rules for T-SQL shops. Inline disable, project config, git-changed-only mode, and SARIF output for GitHub Code Scanning. 500+ monthly downloads on PyPI. Catches dangerous SQL before it reaches production -- DELETE without WHERE, UPDATE without WHERE, SQL injection patterns, SELECT *, and 20 more. Runs as a **CLI tool**, **pre-commit hook**, and **GitHub Action**. @@ -227,6 +227,7 @@ sql-sop list-rules # show every registered rule | W009 | `missing-semicolon` | Statement not terminated with `;` | | W010 | `commented-out-code` | `-- SELECT * FROM old_table` -- use version control | | W013 | `window-missing-partition` | `OVER ()` -- unpredictable results and unclear intent | +| W014 | `case-without-else` | `CASE WHEN ... THEN ... END` -- unmatched rows return NULL | | W015 | `join-function-on-column` | `JOIN customers c ON UPPER(o.email) = UPPER(c.email)` -- kills index seek | | W016 | `not-in-with-subquery` | `WHERE id NOT IN (SELECT ...)` -- silently returns 0 rows on NULL | W017 | `leading-wildcard-like` | `WHERE name LIKE '%smith'` -- non-SARGable, full scan | diff --git a/sql_guard/rules/__init__.py b/sql_guard/rules/__init__.py index eb32ef5..bc27180 100644 --- a/sql_guard/rules/__init__.py +++ b/sql_guard/rules/__init__.py @@ -14,6 +14,7 @@ UpdateWithoutWhere, ) from sql_guard.rules.warnings import ( + CaseWithoutElse, CommentedOutCode, CountDistinctUnbounded, FunctionOnIndexedColumn, @@ -79,6 +80,7 @@ TruncateTable(), CountDistinctUnbounded(), ScalarUdfInWhere(), + CaseWithoutElse(), # Structural (S001-S003) ImplicitCrossJoin(), DeeplyNestedSubquery(), diff --git a/sql_guard/rules/warnings.py b/sql_guard/rules/warnings.py index 1ca2d70..06c45c7 100644 --- a/sql_guard/rules/warnings.py +++ b/sql_guard/rules/warnings.py @@ -484,6 +484,61 @@ def check_statement(self, statement: str, start_line: int, file: str) -> Finding return None +class CaseWithoutElse(Rule): + """W014: CASE expression without ELSE returns NULL for unmatched rows. + + ``CASE WHEN ... THEN ... END`` without an ``ELSE`` branch returns + ``NULL`` for any row that doesn't match a ``WHEN`` condition. + Often the author assumes the conditions are exhaustive when they + aren't, or downstream code can't handle NULLs. + + Walks ``CASE`` / ``ELSE`` / ``END`` tokens with a depth-aware stack + so a nested-but-complete ``CASE`` doesn't mask an outer one that + lacks ``ELSE``. Each ``CASE`` block is judged on its own ``ELSE`` + count. Standalone ``END`` tokens (for example ``BEGIN ... END`` + blocks in T-SQL) are ignored when no matching ``CASE`` is on the + stack. + """ + + id = "W014" + name = "case-without-else" + severity = "warning" + description = "CASE without ELSE returns NULL for unmatched rows" + multiline = True + + _case_keyword = Rule._compile(r"\b(CASE|END|ELSE)\b") + + def check_statement(self, statement: str, start_line: int, file: str) -> Finding | None: + # Walk CASE/ELSE/END tokens with a depth-aware stack. Each CASE + # pushes an entry; ELSE marks the current entry; END pops and + # decides. Nested CASEs are judged independently, so an outer + # CASE with no ELSE still fires even if an inner one has ELSE. + stack: list[bool] = [] # one entry per open CASE; True if ELSE seen + for match in self._case_keyword.finditer(statement): + word = match.group(1).upper() + if word == "CASE": + stack.append(False) + elif word == "ELSE": + if stack: + stack[-1] = True + elif word == "END": + if not stack: + # END with no matching CASE -- e.g. a T-SQL BEGIN/END + # block. Skip rather than false-fire. + continue + had_else = stack.pop() + if not had_else: + return Finding( + rule_id=self.id, + severity=self.severity, + file=file, + line=start_line, + message="CASE without ELSE -- unmatched rows return NULL", + suggestion="Add an explicit ELSE clause, even if it's ELSE NULL for clarity", + ) + return None + + class WindowMissingPartition(Rule): """W013: OVER() without PARTITION BY can yield unpredictable results.""" diff --git a/tests/fixtures/warnings.sql b/tests/fixtures/warnings.sql index 26b903b..6f093be 100644 --- a/tests/fixtures/warnings.sql +++ b/tests/fixtures/warnings.sql @@ -44,3 +44,11 @@ WHERE dbo.fn_IsHighValue(total) = 1; SELECT * FROM orders o JOIN customers c ON UPPER(o.email) = UPPER(c.email); + +-- W014: CASE without ELSE +SELECT + CASE + WHEN status = 'paid' THEN 1 + WHEN status = 'pending' THEN 0 + END AS paid_flag +FROM orders; diff --git a/tests/test_rules.py b/tests/test_rules.py index 228ccb5..6d4f99f 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -18,18 +18,18 @@ class TestRuleRegistry: def test_all_rules_loaded(self) -> None: - assert len(ALL_RULES) == 36 + assert len(ALL_RULES) == 37 def test_10_errors(self) -> None: # 8 E-series + 2 T-series (T002 xp-cmdshell, T004 deprecated-outer-join). errors = [r for r in ALL_RULES if r.severity == "error"] assert len(errors) == 10 - def test_26_warnings(self) -> None: - # 20 W-series + 3 S-series + 3 T-series (T001 with-nolock, + def test_27_warnings(self) -> None: + # 21 W-series + 3 S-series + 3 T-series (T001 with-nolock, # T003 cursor-declaration, T005 create-index-without-online). warnings = [r for r in ALL_RULES if r.severity == "warning"] - assert len(warnings) == 26 + assert len(warnings) == 27 def test_unique_ids(self) -> None: ids = [r.id for r in ALL_RULES] @@ -193,6 +193,53 @@ def test_w016_not_in_value_list_ok(self, tmp_path) -> None: w016 = [f for f in result.findings if f.rule_id == "W016"] assert not w016 + def test_w014_case_without_else(self) -> None: + findings = check([str(FIXTURES / "warnings.sql")]) + w014 = [f for f in findings.findings if f.rule_id == "W014"] + assert len(w014) >= 1 + assert "CASE" in w014[0].message + + def test_w014_case_with_else_ok(self, tmp_path) -> None: + sql = tmp_path / "case_with_else.sql" + sql.write_text( + "SELECT CASE\n" + " WHEN status = 'paid' THEN 1\n" + " WHEN status = 'pending' THEN 0\n" + " ELSE NULL\n" + "END AS paid_flag\n" + "FROM orders;\n" + ) + result = check([str(sql)]) + w014 = [f for f in result.findings if f.rule_id == "W014"] + assert not w014 + + def test_w014_outer_case_without_else_fires_when_inner_has_else( + self, tmp_path + ) -> None: + # Issue #4 specifically called out the nested case: an outer + # CASE with no ELSE must still fire even when an inner CASE + # does have one. + from sql_guard.rules.warnings import CaseWithoutElse + + rule = CaseWithoutElse() + nested = ( + "SELECT CASE\n" + " WHEN x THEN CASE WHEN y THEN 1 ELSE 2 END\n" + " WHEN z THEN 3\n" + "END FROM t;" + ) + finding = rule.check_statement(nested, 1, "test.sql") + assert finding is not None + assert finding.rule_id == "W014" + + def test_w014_does_not_fire_on_begin_end_block(self) -> None: + # T-SQL BEGIN/END blocks should not trip the rule on their own. + from sql_guard.rules.warnings import CaseWithoutElse + + rule = CaseWithoutElse() + proc = "BEGIN\n SELECT 1;\nEND;" + assert rule.check_statement(proc, 1, "test.sql") is None + # --------------------------------------------------------------------------- # Clean file