Skip to main content

rigsql_rules/ambiguous/
am03.rs

1use rigsql_core::{Segment, SegmentType};
2
3use crate::rule::{CrawlType, Rule, RuleContext, RuleGroup};
4use crate::violation::LintViolation;
5
6/// AM03: ORDER BY column with ambiguous direction.
7///
8/// When some ORDER BY expressions have explicit ASC/DESC and others don't,
9/// the inconsistency is confusing. Either specify direction for all or none.
10#[derive(Debug, Default)]
11pub struct RuleAM03;
12
13impl Rule for RuleAM03 {
14    fn code(&self) -> &'static str {
15        "AM03"
16    }
17    fn name(&self) -> &'static str {
18        "ambiguous.order_by"
19    }
20    fn description(&self) -> &'static str {
21        "Inconsistent ORDER BY direction."
22    }
23    fn explanation(&self) -> &'static str {
24        "When an ORDER BY clause has multiple columns, mixing explicit (ASC/DESC) and \
25         implicit sort directions is confusing. If some columns have an explicit direction, \
26         all columns should have one for consistency and clarity."
27    }
28    fn groups(&self) -> &[RuleGroup] {
29        &[RuleGroup::Ambiguous]
30    }
31    fn is_fixable(&self) -> bool {
32        false
33    }
34
35    fn crawl_type(&self) -> CrawlType {
36        CrawlType::Segment(vec![SegmentType::OrderByClause])
37    }
38
39    fn eval(&self, ctx: &RuleContext) -> Vec<LintViolation> {
40        let children = ctx.segment.children();
41
42        // Collect OrderByExpression children
43        let order_exprs: Vec<_> = children
44            .iter()
45            .filter(|c| c.segment_type() == SegmentType::OrderByExpression)
46            .collect();
47
48        if order_exprs.len() < 2 {
49            return vec![];
50        }
51
52        // Check which ones have explicit sort order (ASC/DESC keyword or SortOrder node)
53        let has_direction: Vec<bool> = order_exprs
54            .iter()
55            .map(|expr| has_explicit_direction(expr))
56            .collect();
57
58        let any_explicit = has_direction.iter().any(|&d| d);
59        let all_explicit = has_direction.iter().all(|&d| d);
60
61        // If some have and some don't, flag the ones without
62        if any_explicit && !all_explicit {
63            return order_exprs
64                .iter()
65                .zip(has_direction.iter())
66                .filter(|(_, &has)| !has)
67                .map(|(expr, _)| {
68                    LintViolation::with_msg_key(
69                        self.code(),
70                        "ORDER BY column without explicit ASC/DESC when other columns have one.",
71                        expr.span(),
72                        "rules.AM03.msg",
73                        vec![],
74                    )
75                })
76                .collect();
77        }
78
79        vec![]
80    }
81}
82
83/// Check if an OrderByExpression has an explicit ASC or DESC.
84fn has_explicit_direction(expr: &Segment) -> bool {
85    expr.children().iter().any(|c| {
86        // Check for SortOrder node
87        if c.segment_type() == SegmentType::SortOrder {
88            return true;
89        }
90        // Check for bare ASC/DESC keyword
91        if let Segment::Token(t) = c {
92            if t.segment_type == SegmentType::Keyword
93                && (t.token.text.eq_ignore_ascii_case("ASC")
94                    || t.token.text.eq_ignore_ascii_case("DESC"))
95            {
96                return true;
97            }
98        }
99        false
100    })
101}
102
103#[cfg(test)]
104mod tests {
105    use super::*;
106    use crate::test_utils::lint_sql;
107
108    #[test]
109    fn test_am03_flags_inconsistent_direction() {
110        let violations = lint_sql("SELECT a, b FROM t ORDER BY a ASC, b", RuleAM03);
111        assert_eq!(violations.len(), 1);
112    }
113
114    #[test]
115    fn test_am03_accepts_all_explicit() {
116        let violations = lint_sql("SELECT a, b FROM t ORDER BY a ASC, b DESC", RuleAM03);
117        assert_eq!(violations.len(), 0);
118    }
119
120    #[test]
121    fn test_am03_accepts_all_implicit() {
122        let violations = lint_sql("SELECT a, b FROM t ORDER BY a, b", RuleAM03);
123        assert_eq!(violations.len(), 0);
124    }
125}