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::new(
69                        self.code(),
70                        "ORDER BY column without explicit ASC/DESC when other columns have one.",
71                        expr.span(),
72                    )
73                })
74                .collect();
75        }
76
77        vec![]
78    }
79}
80
81/// Check if an OrderByExpression has an explicit ASC or DESC.
82fn has_explicit_direction(expr: &Segment) -> bool {
83    expr.children().iter().any(|c| {
84        // Check for SortOrder node
85        if c.segment_type() == SegmentType::SortOrder {
86            return true;
87        }
88        // Check for bare ASC/DESC keyword
89        if let Segment::Token(t) = c {
90            if t.segment_type == SegmentType::Keyword
91                && (t.token.text.eq_ignore_ascii_case("ASC")
92                    || t.token.text.eq_ignore_ascii_case("DESC"))
93            {
94                return true;
95            }
96        }
97        false
98    })
99}
100
101#[cfg(test)]
102mod tests {
103    use super::*;
104    use crate::test_utils::lint_sql;
105
106    #[test]
107    fn test_am03_flags_inconsistent_direction() {
108        let violations = lint_sql("SELECT a, b FROM t ORDER BY a ASC, b", RuleAM03);
109        assert_eq!(violations.len(), 1);
110    }
111
112    #[test]
113    fn test_am03_accepts_all_explicit() {
114        let violations = lint_sql("SELECT a, b FROM t ORDER BY a ASC, b DESC", RuleAM03);
115        assert_eq!(violations.len(), 0);
116    }
117
118    #[test]
119    fn test_am03_accepts_all_implicit() {
120        let violations = lint_sql("SELECT a, b FROM t ORDER BY a, b", RuleAM03);
121        assert_eq!(violations.len(), 0);
122    }
123}