Add W024 select-distinct-suspicious (DISTINCT + JOIN band-aid heuristic) (#49)

* Add W024 select-distinct-suspicious rule

Fires when a statement contains both top-level SELECT DISTINCT and any
JOIN keyword. The pattern is a frequent band-aid for a missing join
condition or a missing GROUP BY: developers reach for DISTINCT to
swallow row duplication caused by an over-wide JOIN cardinality
rather than fixing the join.

The regex anchors on SELECT directly preceding DISTINCT so aggregate-
DISTINCT forms like COUNT(DISTINCT col) and SUM(DISTINCT amount) are
not flagged. String literals and comments are stripped first via
strip_strings_and_comments so a mention of SELECT DISTINCT inside a
literal or a comment cannot trigger the rule. multiline = True so
the DISTINCT and JOIN keywords can sit on different lines of the
same statement.

Resolves #40.

* Register W024 in rules registry; bump test counts to 42 / 31 warnings

* Add W024 select-distinct-suspicious tests

Covers: INNER JOIN, LEFT JOIN, multi-line, case-insensitive, single-table
DISTINCT (passes), COUNT(DISTINCT) with JOIN (passes), SUM(DISTINCT)
with JOIN (passes), JOIN without DISTINCT (passes), DISTINCT inside a
string literal (passes), DISTINCT inside a -- comment (passes), and a
message-content assertion.

* Document W024 in CHANGELOG under [Unreleased] Added

* Add W024 row to README rule table

* style: pre-commit auto-fixes

[pre-commit.ci] auto-applied fixes from configured hooks

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: Pawansingh3889

CHANGELOG.md

@@ -46,6 +46,17 @@ a deprecation window (see `GOVERNANCE.md` § Scope discipline).
   the explicit-column variant (`SELECT col1, col2 INTO target FROM
   source`) is allowed through here and covered by the contracts pack
   at C001/C003 if a column type drifts. Resolves #43.
+- **W024 `select-distinct-suspicious`** (warning) - fires when a
+  statement contains both top-level `SELECT DISTINCT` and any `JOIN`
+  keyword. The pattern is a common band-aid for a missing join
+  condition or a missing `GROUP BY`: developers reach for `DISTINCT`
+  to swallow row duplication caused by an over-wide JOIN cardinality
+  rather than fixing the join. Standalone `SELECT DISTINCT col FROM t`
+  (no join) is left alone, and aggregate-DISTINCT forms like
+  `COUNT(DISTINCT col)` are not flagged because the regex anchors on
+  `SELECT` directly preceding `DISTINCT`. String literals and comments
+  are stripped first so a mention of "SELECT DISTINCT" inside a string
+  or comment cannot fire the rule. Resolves #40.
 
 ### Fixed
 

README.md

@@ -238,6 +238,7 @@ sql-sop list-rules                       # show every registered rule
 | W020 | `truncate-table` | `TRUNCATE TABLE staging;` -- bypasses triggers, resets identity |
 | W022 | `cross-join-explicit` | `FROM products CROSS JOIN regions` -- Cartesian product, confirm intent |
 | W023 | `scalar-udf-in-where` | `WHERE dbo.fn_X(col) = 1` -- row-by-row predicate evaluation |
+| W024 | `select-distinct-suspicious` | `SELECT DISTINCT a, b FROM x JOIN y ON ...` -- DISTINCT often masks a missing join condition or GROUP BY |
 
 
 ### Structural (v0.3.0+, sqlparse-based)

sql_guard/rules/__init__.py

@@ -33,6 +33,7 @@
     OrAcrossColumns,
     OrderByWithoutLimit,
     ScalarUdfInWhere,
+    SelectDistinctSuspicious,
     SelectStar,
     SubqueryCouldBeJoin,
     TruncateTable,
@@ -106,6 +107,7 @@
     LeadingWildcardLike(),
     OrAcrossColumns(),
     CountDistinctUnbounded(),
+    SelectDistinctSuspicious(),
     TruncateTable(),
     HavingWithoutGroupBy(),
     CrossJoinExplicit(),

sql_guard/rules/warnings.py

@@ -4,7 +4,7 @@
 
 import re
 
-from sql_guard.rules.base import Finding, Rule
+from sql_guard.rules.base import Finding, Rule, strip_strings_and_comments
 
 
 class SelectStar(Rule):
@@ -510,6 +510,56 @@ def check_line(self, line: str, line_number: int, file: str) -> Finding | None:
         return None
 
 
+class SelectDistinctSuspicious(Rule):
+    """W024: ``SELECT DISTINCT`` paired with ``JOIN`` is often a band-aid.
+
+    Developers reach for ``SELECT DISTINCT`` to deduplicate rows when a
+    JOIN cardinality is wider than they expected, often because a join
+    condition is missing or because what they actually want is a
+    ``GROUP BY``. The result reads more rows than necessary and hides
+    the underlying join-condition bug.
+
+    Fires when a statement contains both top-level ``SELECT DISTINCT``
+    and any ``JOIN`` keyword. Standalone ``SELECT DISTINCT col FROM t``
+    (no join) is fine -- the smell is the cross-join blow-up pattern,
+    not legitimate single-table uniqueness.
+
+    ``COUNT(DISTINCT col)`` and other aggregate-DISTINCT forms are not
+    flagged: the regex anchors on ``SELECT`` directly preceding
+    ``DISTINCT``, so ``SELECT COUNT(DISTINCT x) FROM t JOIN u`` does
+    not match.
+    """
+
+    id = "W024"
+    name = "select-distinct-suspicious"
+    severity = "warning"
+    description = "SELECT DISTINCT combined with JOIN often masks a missing join condition"
+    multiline = True
+
+    _select_distinct = Rule._compile(r"\bSELECT\s+DISTINCT\b")
+    _join = Rule._compile(r"\bJOIN\b")
+
+    def check_statement(self, statement: str, start_line: int, file: str) -> Finding | None:
+        # Strip strings and comments so DISTINCT or JOIN inside literal
+        # text or comments cannot trigger a false positive.
+        stripped = strip_strings_and_comments(statement)
+        if self._select_distinct.search(stripped) and self._join.search(stripped):
+            return Finding(
+                rule_id=self.id,
+                severity=self.severity,
+                file=file,
+                line=start_line,
+                message=(
+                    "SELECT DISTINCT with JOIN often masks a missing join condition or grouping"
+                ),
+                suggestion=(
+                    "Verify the JOIN condition is correct and DISTINCT is genuinely needed; "
+                    "consider GROUP BY if you are hiding row duplication"
+                ),
+            )
+        return None
+
+
 class CountDistinctUnbounded(Rule):
     """W019: ``COUNT(DISTINCT col)`` on an unfiltered table.
 

tests/test_new_rules.py

@@ -10,6 +10,7 @@
     LeadingWildcardLike,
     OrAcrossColumns,
     ScalarUdfInWhere,
+    SelectDistinctSuspicious,
     TruncateTable,
 )
 
@@ -387,3 +388,85 @@ def test_w022_does_not_flag_cross_join_inside_trailing_comment():
         )
         is None
     )
+
+
+# W024 select-distinct-suspicious ---------------------------------------------
+
+
+def test_w024_flags_distinct_with_inner_join():
+    rule = SelectDistinctSuspicious()
+    finding = _stmt(
+        rule,
+        "SELECT DISTINCT c.id, c.name FROM customers c JOIN orders o ON c.id = o.customer_id;",
+    )
+    assert finding is not None
+    assert finding.rule_id == "W024"
+    assert finding.severity == "warning"
+
+
+def test_w024_flags_distinct_with_left_join():
+    rule = SelectDistinctSuspicious()
+    sql = "SELECT DISTINCT a.id FROM a LEFT JOIN b ON a.id = b.a_id;"
+    assert _stmt(rule, sql) is not None
+
+
+def test_w024_flags_distinct_with_join_multiline():
+    rule = SelectDistinctSuspicious()
+    sql = (
+        "SELECT DISTINCT\n  c.id, c.name\nFROM customers c\nJOIN orders o ON c.id = o.customer_id;"
+    )
+    assert _stmt(rule, sql) is not None
+
+
+def test_w024_case_insensitive():
+    rule = SelectDistinctSuspicious()
+    assert _stmt(rule, "select distinct a from x join y on x.id = y.id;") is not None
+
+
+def test_w024_does_not_flag_distinct_alone():
+    # Single-table DISTINCT is fine -- no JOIN cardinality blow-up to mask.
+    rule = SelectDistinctSuspicious()
+    assert _stmt(rule, "SELECT DISTINCT country FROM customers;") is None
+
+
+def test_w024_does_not_flag_count_distinct_with_join():
+    # Aggregate-DISTINCT is a different pattern; the regex anchors on
+    # "SELECT DISTINCT" directly, not "COUNT(DISTINCT ...)".
+    rule = SelectDistinctSuspicious()
+    sql = "SELECT COUNT(DISTINCT c.id) FROM customers c JOIN orders o ON c.id = o.customer_id;"
+    assert _stmt(rule, sql) is None
+
+
+def test_w024_does_not_flag_sum_distinct_with_join():
+    rule = SelectDistinctSuspicious()
+    sql = "SELECT SUM(DISTINCT amount) FROM payments p JOIN customers c ON p.cust_id = c.id;"
+    assert _stmt(rule, sql) is None
+
+
+def test_w024_does_not_flag_join_without_distinct():
+    rule = SelectDistinctSuspicious()
+    assert _stmt(rule, "SELECT a.id FROM a JOIN b ON a.id = b.a_id;") is None
+
+
+def test_w024_does_not_flag_distinct_inside_string_literal():
+    rule = SelectDistinctSuspicious()
+    sql = "INSERT INTO log(msg) SELECT 'SELECT DISTINCT x JOIN y' FROM t JOIN u ON t.id = u.id;"
+    # Outer SELECT does not have DISTINCT; the literal mentions it.
+    assert _stmt(rule, sql) is None
+
+
+def test_w024_does_not_flag_distinct_inside_comment():
+    rule = SelectDistinctSuspicious()
+    sql = "-- SELECT DISTINCT x FROM y JOIN z\nSELECT id FROM t;"
+    assert _stmt(rule, sql) is None
+
+
+def test_w024_message_mentions_join_or_grouping():
+    rule = SelectDistinctSuspicious()
+    finding = _stmt(
+        rule,
+        "SELECT DISTINCT a FROM x JOIN y ON x.id = y.id;",
+    )
+    assert finding is not None
+    msg = finding.message.lower()
+    assert "join" in msg or "grouping" in msg

tests/test_rules.py

@@ -18,19 +18,19 @@
 
 class TestRuleRegistry:
     def test_all_rules_loaded(self) -> None:
-        assert len(ALL_RULES) == 41
+        assert len(ALL_RULES) == 42
 
     def test_11_errors(self) -> None:
         # 9 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) == 11
 
-    def test_30_warnings(self) -> None:
-        # 23 W-series + 3 S-series + 4 T-series (T001 with-nolock,
+    def test_31_warnings(self) -> None:
+        # 24 W-series + 3 S-series + 4 T-series (T001 with-nolock,
         # T003 cursor-declaration, T005 create-index-without-online,
         # T006 select-into-without-typed-fields).
         warnings = [r for r in ALL_RULES if r.severity == "warning"]
-        assert len(warnings) == 30
+        assert len(warnings) == 31
 
     def test_unique_ids(self) -> None:
         ids = [r.id for r in ALL_RULES]