Skip to main content

sqruff_lib/rules/convention/
cv12.rs

1use hashbrown::HashMap;
2use sqruff_lib_core::dialects::syntax::{SyntaxKind, SyntaxSet};
3
4use crate::core::config::Value;
5use crate::core::rules::context::RuleContext;
6use crate::core::rules::crawlers::{Crawler, SegmentSeekerCrawler};
7use crate::core::rules::{Erased, ErasedRule, LintResult, Rule, RuleGroups};
8
9#[derive(Debug, Clone)]
10pub struct RuleCV12;
11
12impl Rule for RuleCV12 {
13    fn load_from_config(&self, _config: &HashMap<String, Value>) -> Result<ErasedRule, String> {
14        Ok(RuleCV12.erased())
15    }
16
17    fn name(&self) -> &'static str {
18        "convention.join_condition"
19    }
20
21    fn description(&self) -> &'static str {
22        "Join conditions should use the JOIN ... ON syntax."
23    }
24
25    fn long_description(&self) -> &'static str {
26        r#"
27**Anti-pattern**
28
29Placing join conditions in the `WHERE` clause instead of using `JOIN ... ON` mixes join logic with filtering logic, making queries harder to read.
30
31```sql
32SELECT
33    foo
34FROM bar
35JOIN baz
36WHERE bar.id = baz.id;
37```
38
39**Best practice**
40
41Use `JOIN ... ON` to specify join conditions.
42
43```sql
44SELECT
45    foo
46FROM bar
47JOIN baz ON bar.id = baz.id;
48```
49"#
50    }
51
52    fn groups(&self) -> &'static [RuleGroups] {
53        &[RuleGroups::All, RuleGroups::Convention]
54    }
55
56    fn eval(&self, context: &RuleContext) -> Vec<LintResult> {
57        assert!(context.segment.is_type(SyntaxKind::SelectStatement));
58
59        let children = context.segment.segments();
60
61        // Find the FROM clause.
62        let from_clause = children.iter().find(|s| s.is_type(SyntaxKind::FromClause));
63
64        let Some(from_clause) = from_clause else {
65            return Vec::new();
66        };
67
68        // Only flag when there's a WHERE clause — that's the signal that
69        // join conditions may have been placed there instead of ON.
70        let has_where = children.iter().any(|s| s.is_type(SyntaxKind::WhereClause));
71
72        if !has_where {
73            return Vec::new();
74        }
75
76        // Find all JoinClause descendants within the FROM clause.
77        let join_clauses = from_clause.recursive_crawl(
78            const { &SyntaxSet::single(SyntaxKind::JoinClause) },
79            true,
80            &SyntaxSet::EMPTY,
81            true,
82        );
83
84        let mut results = Vec::new();
85
86        for join in join_clauses {
87            let join_children = join.segments();
88
89            // Check if this join has an ON condition or USING clause.
90            let has_condition = join_children.iter().any(|s| {
91                s.is_type(SyntaxKind::JoinOnCondition)
92                    || s.is_type(SyntaxKind::UsingClause)
93                    || s.is_keyword("USING")
94            });
95
96            if has_condition {
97                continue;
98            }
99
100            // Skip CROSS JOIN and NATURAL JOIN — they don't need ON.
101            let is_cross_or_natural = join_children.iter().any(|s| {
102                s.is_type(SyntaxKind::Keyword)
103                    && (s.raw().eq_ignore_ascii_case("CROSS")
104                        || s.raw().eq_ignore_ascii_case("NATURAL"))
105            });
106
107            if is_cross_or_natural {
108                continue;
109            }
110
111            results.push(LintResult::new(
112                Some(join.clone()),
113                Vec::new(),
114                None,
115                Some(
116                    "Join conditions should use the JOIN ... ON syntax rather than the WHERE clause."
117                        .into(),
118                ),
119            ));
120        }
121
122        results
123    }
124
125    fn is_fix_compatible(&self) -> bool {
126        false
127    }
128
129    fn crawl_behaviour(&self) -> Crawler {
130        SegmentSeekerCrawler::new(const { SyntaxSet::new(&[SyntaxKind::SelectStatement]) }).into()
131    }
132}