fix: close correctness gaps in Rules, SQL scanners and sqb query

crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs

@@ -67,13 +67,22 @@ pub(crate) fn comments_and_quoted_text_hide_references() -> bool {
         == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)])
 }
 
+pub(crate) fn dollar_quoted_text_hides_references() -> bool {
+    extract(concat!(
+        "SELECT $$Customer's order -- __ref(\"ignored\") $5$$ AS label, ",
+        "$tag$ $$ __seed(\"also_ignored\") $tag$ AS note, price$1$ ",
+        "FROM __ref(orders)"
+    )) == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)])
+}
+
 pub(crate) fn complex_or_malformed_sql_requests_fallback() -> bool {
     [
         "SELECT * FROM __table_fn(\"orders\")(1)",
         "SELECT * FROM __ref(concat('ord', 'ers'))",
         "SELECT * FROM __ref(\"orders\"",
         "SELECT * FROM __ref(\"orders\") /* unterminated",
         "SELECT * FROM __ref(\"orders\") WHERE note = 'unterminated",
+        "SELECT * FROM __ref(\"orders\") WHERE note = $$unterminated",
         "SELECT * FROM __ref(örders)",
     ]
     .into_iter()

crates/sqlbuild-rules-native/src/compiler/tests/test_sql_references.rs

@@ -1,6 +1,6 @@
 use crate::compiler::tests::helpers::{
     comments_and_quoted_text_hide_references, complex_or_malformed_sql_requests_fallback,
-    simple_references_preserve_authored_order,
+    dollar_quoted_text_hides_references, simple_references_preserve_authored_order,
 };
 use crate::compiler::tests::test_types::StaticSqlOperationTestCase;
 
@@ -17,6 +17,11 @@ fn given_sql_reference_cases_when_extracting_then_expected_behavior_holds() {
             run: comments_and_quoted_text_hide_references,
             expected_success: true,
         },
+        StaticSqlOperationTestCase {
+            description: "dollar-quoted text hides embedded references",
+            run: dollar_quoted_text_hides_references,
+            expected_success: true,
+        },
         StaticSqlOperationTestCase {
             description: "complex and malformed SQL requests Python fallback",
             run: complex_or_malformed_sql_requests_fallback,

crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs

@@ -954,6 +954,60 @@ fn given_numeric_comparisons_when_evaluating_named_decisions_then_only_authored_
             authored_sql: ("SELECT CASE WHEN amount > 30 THEN 'large' END AS amount_30 FROM orders"),
             expected_fault_count: 1,
         },
+        test_types::NumericDecisionTestCase {
+            description: "authored between bounds are a named decision finding",
+            query_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61",
+            authored_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61",
+            expected_fault_count: 1,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "authored not between bounds are a named decision finding",
+            query_sql: "SELECT code FROM orders WHERE amount NOT BETWEEN 5 AND 10",
+            authored_sql: "SELECT code FROM orders WHERE amount NOT BETWEEN 5 AND 10",
+            expected_fault_count: 1,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "canonical between bounds pass",
+            query_sql: "SELECT code FROM orders WHERE amount BETWEEN 0 AND 1",
+            authored_sql: "SELECT code FROM orders WHERE amount BETWEEN 0 AND 1",
+            expected_fault_count: 0,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "expanded between constants are not authored literal findings",
+            query_sql: "SELECT code FROM orders WHERE amount BETWEEN 30 AND 61",
+            authored_sql: "SELECT code FROM orders WHERE amount BETWEEN @const(\"_low\") AND @const(\"_high\")",
+            expected_fault_count: 0,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "authored numeric in list is a named decision finding",
+            query_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) IN (30, 31, 45)",
+            authored_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) IN (30, 31, 45)",
+            expected_fault_count: 1,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "authored numeric not in list is a named decision finding",
+            query_sql: "SELECT code FROM orders WHERE amount NOT IN (30, 31)",
+            authored_sql: "SELECT code FROM orders WHERE amount NOT IN (30, 31)",
+            expected_fault_count: 1,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "canonical numeric in list passes",
+            query_sql: "SELECT code FROM orders WHERE amount IN (-1, 0, 1)",
+            authored_sql: "SELECT code FROM orders WHERE amount IN (-1, 0, 1)",
+            expected_fault_count: 0,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "expanded list constant is not an authored literal finding",
+            query_sql: "SELECT code FROM orders WHERE amount IN (30, 31, 45)",
+            authored_sql: "SELECT code FROM orders WHERE amount IN @const(\"_codes\")",
+            expected_fault_count: 0,
+        },
+        test_types::NumericDecisionTestCase {
+            description: "between in projected case is a named decision finding",
+            query_sql: "SELECT CASE WHEN amount BETWEEN 30 AND 61 THEN 'mid' END AS band FROM orders",
+            authored_sql: "SELECT CASE WHEN amount BETWEEN 30 AND 61 THEN 'mid' END AS band FROM orders",
+            expected_fault_count: 1,
+        },
         test_types::NumericDecisionTestCase {
             description: "nested comparison reports only its leaf decision",
             query_sql: ("SELECT CASE WHEN amount > 5 THEN status END = 'ready' AS selected FROM orders"),

crates/sqlbuild-rules-native/src/engine/tests/test_sql_scanning.rs

@@ -170,7 +170,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann
         SqlScannerTestCase {
             description: "dollar quote",
             fragment: "$$ ) $$",
-            expected_compiler_paren: Ok(4),
+            expected_compiler_paren: Ok(10),
             expected_table_function: "SELECT * FROM __table_fn(\"o\", $$ ) $$ x)(1)",
             expected_table_function_token: "SELECT $$ ) $$ __table_fn(\"o\", 1)",
             expected_snowflake_exclude: "SELECT *               $$ x) , a FROM t",

crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs

@@ -30,6 +30,7 @@ use std::ops::ControlFlow;
 
 const MIXED_STAR_REMEDIATION: &str = "SELECT *, a, b mixes a passthrough star with derived columns. The * exemption permits a lone SELECT * only. Move a and b into an earlier CTE so they are computed upstream, leaving the final select a pure SELECT *. If you need those columns in the output, drop the * exemption and enumerate every column explicitly.";
 const RULES_PARSER_RECURSION_LIMIT: usize = 128;
+const NUMERIC_VALUE_LIST_REMEDIATION: &str = "Declare the values as a list or set CONSTANT and compare through [NOT] IN @const(\"<name>\"); only -1, 0, and 1 are self-explanatory.";
 
 #[derive(Clone)]
 struct Position {
@@ -42,6 +43,8 @@ struct ComparisonFact {
     position: Option<Position>,
     sql: String,
     equality: bool,
+    value_list: bool,
+    binary: bool,
     output_name: Option<String>,
     columns: BTreeSet<String>,
     string_literals: Vec<String>,
@@ -312,6 +315,7 @@ pub(crate) fn rules_quote_policy(dialect_name: &str) -> QuotePolicy {
         single_quote_backslash_escapes: backslash_escapes,
         double_quote_backslash_escapes: backslash_escapes
             && !dialect.is_delimited_identifier_start('"'),
+        dollar_quotes: false,
     }
 }
 
@@ -1226,7 +1230,9 @@ fn evaluate_literal_rules(
 ) {
     let facts = select_facts(&parsed.query);
     for comparison in facts.into_iter().flat_map(|select| select.comparisons) {
-        if let Some(rule) = selected.get("SQBRDECLARATION101") {
+        if comparison.binary
+            && let Some(rule) = selected.get("SQBRDECLARATION101")
+        {
             let enum_column = comparison
                 .columns
                 .iter()
@@ -1265,7 +1271,9 @@ fn evaluate_literal_rules(
                     rule,
                     comparison.position.as_ref(),
                     format!("non-canonical numeric comparison: {}", comparison.sql),
-                    None,
+                    comparison
+                        .value_list
+                        .then(|| NUMERIC_VALUE_LIST_REMEDIATION.to_owned()),
                 ));
             }
         }
@@ -1669,17 +1677,20 @@ fn comparison_facts(root: &Expr, source_context: &SourceContext, output: &mut Ve
     impl Visitor for Comparisons<'_> {
         type Break = ();
         fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow<Self::Break> {
-            if let Expr::BinaryOp { op, .. } = expression
-                && matches!(
+            let is_comparison = match expression {
+                Expr::BinaryOp { op, .. } => matches!(
                     op,
                     BinaryOperator::Eq
                         | BinaryOperator::NotEq
                         | BinaryOperator::Gt
                         | BinaryOperator::GtEq
                         | BinaryOperator::Lt
                         | BinaryOperator::LtEq
-                )
-            {
+                ),
+                Expr::Between { .. } | Expr::InList { .. } => true,
+                _ => false,
+            };
+            if is_comparison {
                 self.output
                     .push(comparison_fact(expression, self.source_context));
             }
@@ -1776,29 +1787,40 @@ fn comparison_fact(root: &Expr, source_context: &SourceContext) -> ComparisonFac
                 Expr::BinaryOp {
                     op: BinaryOperator::Eq,
                     ..
-                }
+                } | Expr::InList { negated: false, .. }
             ),
+            value_list: matches!(root, Expr::InList { .. }),
+            binary: matches!(root, Expr::BinaryOp { .. }),
             source_context: source_context.clone(),
             ..ComparisonFact::default()
         },
     };
     let _ = root.visit(&mut visitor);
-    if let Expr::BinaryOp { left, right, .. } = root {
-        for operand in [left.as_ref(), right.as_ref()] {
-            if let Some(number) = numeric_value(operand) {
-                visitor.fact.numeric_literals.push(number);
-            }
-            if direct_column(operand).is_none() {
-                visitor.fact.modified_columns.extend(column_facts(operand));
-            }
-            if direct_string_literal(operand).is_none() && contains_string_literal(operand) {
-                visitor.fact.modified_string_literal = true;
-            }
+    for operand in comparison_operands(root) {
+        if let Some(number) = numeric_value(operand) {
+            visitor.fact.numeric_literals.push(number);
+        }
+        if direct_column(operand).is_none() {
+            visitor.fact.modified_columns.extend(column_facts(operand));
+        }
+        if direct_string_literal(operand).is_none() && contains_string_literal(operand) {
+            visitor.fact.modified_string_literal = true;
         }
     }
     visitor.fact
 }
 
+fn comparison_operands(root: &Expr) -> Vec<&Expr> {
+    match root {
+        Expr::BinaryOp { left, right, .. } => vec![left.as_ref(), right.as_ref()],
+        Expr::Between {
+            expr, low, high, ..
+        } => vec![expr.as_ref(), low.as_ref(), high.as_ref()],
+        Expr::InList { expr, list, .. } => std::iter::once(expr.as_ref()).chain(list).collect(),
+        _ => Vec::new(),
+    }
+}
+
 fn direct_column(expression: &Expr) -> Option<ColumnFact> {
     match unwrap_nested(expression) {
         Expr::Identifier(value) => Some(ColumnFact {

crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs

@@ -10,6 +10,7 @@ use crate::sql_lint::_helpers::engine::{
     direct_indices, is_comment, is_layout, is_query_from, query_end, significant_after,
     significant_before, token_depths,
 };
+use crate::sql_lint::constants::RELATION_MODIFIER_KEYWORDS;
 use crate::sql_lint::models::{AdditionalFactOptions, AdditionalQueryFacts};
 
 const COUNT_ONE_LITERAL: &str = "1";
@@ -1242,34 +1243,92 @@ fn unused_alias_spans(query: &QuerySlice<'_>) -> Vec<Span> {
     let query_end = *query_end;
     let depth = *depth;
     let mut spans: Vec<Span> = Vec::new();
-    for window in relation_clause.windows(2) {
-        if tokens[window[0]].token_type != TokenType::As || !is_identifier(&tokens[window[1]]) {
-            continue;
-        }
-        if significant_after(tokens, window[1])
+    for (span_start, alias_index) in relation_alias_positions(tokens, relation_clause) {
+        if significant_after(tokens, alias_index)
             .is_some_and(|index| tokens[index].token_type == TokenType::LParen)
         {
             continue;
         }
-        let alias = &tokens[window[1]].text;
+        let alias = &tokens[alias_index].text;
+        let relation_name = significant_before(tokens, span_start);
         let used = (query_start..query_end).any(|index| {
-            depths[index] >= depth
+            index != alias_index
+                && Some(index) != relation_name
+                && depths[index] >= depth
+                && is_identifier(&tokens[index])
                 && tokens[index].text.eq_ignore_ascii_case(alias)
+                && significant_before(tokens, index).is_none_or(|previous| {
+                    !matches!(tokens[previous].token_type, TokenType::Dot | TokenType::As)
+                })
                 && significant_after(tokens, index)
-                    .is_some_and(|next| tokens[next].token_type == TokenType::Dot)
+                    .is_none_or(|next| tokens[next].token_type != TokenType::LParen)
         });
         if !used {
             spans.push(Span {
-                start: tokens[window[0]].span.start,
-                end: tokens[window[1]].span.end,
-                line: tokens[window[0]].span.line,
-                column: tokens[window[0]].span.column,
+                start: tokens[span_start].span.start,
+                end: tokens[alias_index].span.end,
+                line: tokens[span_start].span.line,
+                column: tokens[span_start].span.column,
             });
         }
     }
     spans
 }
 
+fn relation_alias_positions(tokens: &[Token], relation_clause: &[usize]) -> Vec<(usize, usize)> {
+    let mut aliases: Vec<(usize, usize)> = Vec::new();
+    let mut segment_start = 0_usize;
+    let mut in_condition = false;
+    for (position, &index) in relation_clause.iter().enumerate() {
+        if matches!(tokens[index].token_type, TokenType::Join | TokenType::Comma) {
+            segment_start = position + 1;
+            in_condition = false;
+            continue;
+        }
+        if in_condition || is_relation_condition_start(&tokens[index]) {
+            in_condition = true;
+            continue;
+        }
+        if tokens[index].token_type == TokenType::As {
+            if let Some(&alias) = relation_clause.get(position + 1)
+                && is_identifier(&tokens[alias])
+            {
+                aliases.push((index, alias));
+            }
+            continue;
+        }
+        if is_implicit_relation_alias(
+            tokens,
+            &relation_clause[segment_start..],
+            position - segment_start,
+        ) {
+            aliases.push((index, index));
+        }
+    }
+    aliases
+}
+
+fn is_implicit_relation_alias(tokens: &[Token], segment: &[usize], position: usize) -> bool {
+    let candidate = &tokens[segment[position]];
+    if position == 0
+        || !is_identifier(candidate)
+        || candidate.token_type != TokenType::QuotedIdentifier
+            && RELATION_MODIFIER_KEYWORDS
+                .iter()
+                .any(|keyword| candidate.text.eq_ignore_ascii_case(keyword))
+    {
+        return false;
+    }
+    let previous = &tokens[segment[position - 1]];
+    if previous.token_type == TokenType::LParen {
+        return true;
+    }
+    is_identifier(previous)
+        && position
+            .checked_sub(QUALIFIER_PAIR_LENGTH)
+            .is_none_or(|before| tokens[segment[before]].token_type == TokenType::Dot)
+}
+
 fn null_rejected_left_join_spans(
     tokens: &[Token],
     direct: &[usize],

crates/sqlbuild-rules-native/src/sql_lint/constants.rs

@@ -6,3 +6,48 @@ pub(crate) const VALUES_RELATION_PREFIX_TOKEN_COUNT: usize = 2;
 pub(crate) const POSITION_ARGUMENT_SEPARATOR: &str = ",";
 pub(crate) const CTE_MACRO_PLACEHOLDER_LITERAL: &str = "1";
 pub(crate) const POSTFIX_CAST_PREFIX_TOKEN_COUNT: usize = 2;
+pub(crate) const RELATION_MODIFIER_KEYWORDS: [&str; 43] = [
+    "ANTI",
+    "APPLY",
+    "ASOF",
+    "AT",
+    "BEFORE",
+    "CHANGES",
+    "CONNECT",
+    "CROSS",
+    "EXCEPT",
+    "FETCH",
+    "FINAL",
+    "FOR",
+    "FULL",
+    "GROUP",
+    "HAVING",
+    "INNER",
+    "INTERSECT",
+    "JOIN",
+    "LATERAL",
+    "LEFT",
+    "LIMIT",
+    "MATCH_RECOGNIZE",
+    "MINUS",
+    "NATURAL",
+    "OFFSET",
+    "ON",
+    "ORDER",
+    "OUTER",
+    "PASTE",
+    "PIVOT",
+    "POSITIONAL",
+    "QUALIFY",
+    "RETURNING",
+    "RIGHT",
+    "SAMPLE",
+    "SEMI",
+    "START",
+    "TABLESAMPLE",
+    "UNION",
+    "UNPIVOT",
+    "USING",
+    "WHERE",
+    "WINDOW",
+];