flowscope_core/linter/rules/
cv_008.rs

1//! LINT_CV_008: Prefer LEFT JOIN over RIGHT JOIN.
2//!
3//! RIGHT JOIN is functionally valid but harder to read and reason about in many
4//! codebases. Prefer LEFT JOIN for consistent join direction.
5
6use crate::linter::rule::{LintContext, LintRule};
7use crate::types::{issue_codes, Issue};
8use sqlparser::ast::*;
9
10pub struct LeftJoinOverRightJoin;
11
12impl LintRule for LeftJoinOverRightJoin {
13    fn code(&self) -> &'static str {
14        issue_codes::LINT_CV_008
15    }
16
17    fn name(&self) -> &'static str {
18        "LEFT JOIN convention"
19    }
20
21    fn description(&self) -> &'static str {
22        "Use 'LEFT JOIN' instead of 'RIGHT JOIN'."
23    }
24
25    fn check(&self, stmt: &Statement, ctx: &LintContext) -> Vec<Issue> {
26        let mut issues = Vec::new();
27        check_statement(stmt, ctx, &mut issues);
28        issues
29    }
30}
31
32fn check_statement(stmt: &Statement, ctx: &LintContext, issues: &mut Vec<Issue>) {
33    match stmt {
34        Statement::Query(q) => check_query(q, ctx, issues),
35        Statement::Insert(ins) => {
36            if let Some(ref source) = ins.source {
37                check_query(source, ctx, issues);
38            }
39        }
40        Statement::CreateView { query, .. } => check_query(query, ctx, issues),
41        Statement::CreateTable(create) => {
42            if let Some(ref q) = create.query {
43                check_query(q, ctx, issues);
44            }
45        }
46        _ => {}
47    }
48}
49
50fn check_query(query: &Query, ctx: &LintContext, issues: &mut Vec<Issue>) {
51    if let Some(ref with) = query.with {
52        for cte in &with.cte_tables {
53            check_query(&cte.query, ctx, issues);
54        }
55    }
56    check_set_expr(&query.body, ctx, issues);
57}
58
59fn check_set_expr(body: &SetExpr, ctx: &LintContext, issues: &mut Vec<Issue>) {
60    match body {
61        SetExpr::Select(select) => {
62            for from_item in &select.from {
63                check_table_factor(&from_item.relation, ctx, issues);
64                for join in &from_item.joins {
65                    if is_right_join(&join.join_operator) {
66                        issues.push(
67                            Issue::info(
68                                issue_codes::LINT_CV_008,
69                                "Prefer LEFT JOIN over RIGHT JOIN for readability.",
70                            )
71                            .with_statement(ctx.statement_index),
72                        );
73                    }
74                    check_table_factor(&join.relation, ctx, issues);
75                }
76            }
77        }
78        SetExpr::Query(q) => check_query(q, ctx, issues),
79        SetExpr::SetOperation { left, right, .. } => {
80            check_set_expr(left, ctx, issues);
81            check_set_expr(right, ctx, issues);
82        }
83        _ => {}
84    }
85}
86
87fn check_table_factor(relation: &TableFactor, ctx: &LintContext, issues: &mut Vec<Issue>) {
88    match relation {
89        TableFactor::Derived { subquery, .. } => check_query(subquery, ctx, issues),
90        TableFactor::NestedJoin {
91            table_with_joins, ..
92        } => {
93            check_table_factor(&table_with_joins.relation, ctx, issues);
94            for join in &table_with_joins.joins {
95                if is_right_join(&join.join_operator) {
96                    issues.push(
97                        Issue::info(
98                            issue_codes::LINT_CV_008,
99                            "Prefer LEFT JOIN over RIGHT JOIN for readability.",
100                        )
101                        .with_statement(ctx.statement_index),
102                    );
103                }
104                check_table_factor(&join.relation, ctx, issues);
105            }
106        }
107        _ => {}
108    }
109}
110
111fn is_right_join(op: &JoinOperator) -> bool {
112    matches!(
113        op,
114        JoinOperator::Right(_)
115            | JoinOperator::RightOuter(_)
116            | JoinOperator::RightSemi(_)
117            | JoinOperator::RightAnti(_)
118    )
119}
120
121#[cfg(test)]
122mod tests {
123    use super::*;
124    use crate::parser::parse_sql;
125
126    fn check_sql(sql: &str) -> Vec<Issue> {
127        let stmts = parse_sql(sql).unwrap();
128        let rule = LeftJoinOverRightJoin;
129        let ctx = LintContext {
130            sql,
131            statement_range: 0..sql.len(),
132            statement_index: 0,
133        };
134        let mut issues = Vec::new();
135        for stmt in &stmts {
136            issues.extend(rule.check(stmt, &ctx));
137        }
138        issues
139    }
140
141    #[test]
142    fn test_right_join_detected() {
143        let issues = check_sql("SELECT * FROM a RIGHT JOIN b ON a.id = b.id");
144        assert_eq!(issues.len(), 1);
145        assert_eq!(issues[0].code, "LINT_CV_008");
146    }
147
148    #[test]
149    fn test_left_join_ok() {
150        let issues = check_sql("SELECT * FROM a LEFT JOIN b ON a.id = b.id");
151        assert!(issues.is_empty());
152    }
153
154    #[test]
155    fn test_inner_join_ok() {
156        let issues = check_sql("SELECT * FROM a JOIN b ON a.id = b.id");
157        assert!(issues.is_empty());
158    }
159}
flowscope_core/linter/rules/cv_008.rs

flowscope_core/linter/rules/
cv_008.rs
