fix(test): resolve mocked and expected-row references consistently in SQL tests (#1023)

* fix(test): resolve mocked and expected-row references consistently in SQL tests

A mocked name now always means the mock in every SQL-test CTE: an assertion
calling __ref() on a mocked model reads the mock instead of running the real
model, and mocked models no longer count as test targets. References written
directly in __expected__ CTEs are resolved like assertions and helpers.

Compile reports unmocked __source(), __seed() and __dbt_ref() calls and unknown
models in expected rows with P013. The native planner computes the models
helpers and expected rows read itself and rejects any reference left raw.

* fix(test): reject SQL tests whose checks read only mocks

A model-mode SQL test whose expected rows and assertions read only mocked names
runs no model and used to be skipped silently. Compile now reports it with a
located P013 diagnostic showing how to call __ref() on the model under test.
Macro, UDF and table-function tests and scenarios keep their own targets.

* fix(test): plan SQL-test helpers from the compiler's reads and clarify mock-only errors

The native planner now takes the helpers a test reads and the models its
expected rows and helpers reference from the compiler, fails when a request
omits them, and leaves unread helpers that call references out of the test
query, so a helper named like a column no longer runs or fails a test. A
reference the planner still cannot resolve is a located P013 instead of an
unlocated P001. The error for a test that mocks the model it tests now
explains how to mock the model's inputs instead.

* fix(test): satisfy the manual_contains lint in mocked reference checks

Author: kvlonge

crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs

@@ -15,6 +15,7 @@ use crate::compiler::_helpers::sql_tests::planning::{
     DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TABLE_FUNCTION_PREFIX,
     TestFixtures, compile_error,
 };
+use crate::compiler::_helpers::sql_tests::reader_references::calls_reference;
 use crate::constants::{
     QUOTED_IDENTIFIER_DELIMITER_BYTES, SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX,
     SQL_TEST_EXPECTED_CTE,
@@ -80,6 +81,8 @@ pub(crate) struct ScopeRequest<'a> {
     pub(crate) rename_dialect: Option<&'a Dialect>,
     pub(crate) flat_dialect: bool,
     pub(crate) slice_dialect: SliceDialect,
+    /// Helpers the compiler found the test reads; an unread helper calling a reference is left out.
+    pub(crate) read_helper_names: &'a [String],
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash)]
@@ -160,13 +163,17 @@ impl ScopeGraph {
                 .mocks_by_generated_name
                 .insert(mock.generated_name.clone(), index);
         }
-        if fixtures.helpers.is_empty() {
-            return Ok(graph);
-        }
-        for (index, cte) in fixtures.helpers.iter().enumerate() {
-            graph
-                .helper_indexes
-                .insert(cte.name.to_ascii_lowercase(), index);
+        let read: HashSet<String> = request
+            .read_helper_names
+            .iter()
+            .map(|name| name.to_ascii_lowercase())
+            .collect();
+        for cte in &fixtures.helpers {
+            let key = cte.name.to_ascii_lowercase();
+            if !read.contains(&key) && calls_reference(&cte.sql_body, patterns) {
+                continue;
+            }
+            graph.helper_indexes.insert(key, graph.helpers.len());
             graph.helpers.push(ScopeHelper {
                 name: cte.name.clone(),
                 original: cte.sql_body.clone(),
@@ -176,6 +183,9 @@ impl ScopeGraph {
                 resolved: None,
             });
         }
+        if graph.helpers.is_empty() {
+            return Ok(graph);
+        }
         let readers = test_readers(fixtures);
         graph.reject_shadowed_helpers(&readers, request)?;
         if let Some(dialect) = request.rename_dialect
@@ -294,36 +304,12 @@ impl ScopeGraph {
         order.ordered
     }
 
-    /// Helpers the test's expected rows and assertions read, except those a mock it uses reads.
-    pub(crate) fn read_helpers(
-        &self,
-        readers: &[&str],
-        used_mocks: &HashSet<String>,
-        patterns: &SqlTestPatterns,
-    ) -> HashSet<usize> {
-        let helpers = |nodes: Vec<ScopeNode>| -> HashSet<usize> {
-            nodes
-                .into_iter()
-                .filter_map(|node| match node {
-                    ScopeNode::Helper(index) => Some(index),
-                    ScopeNode::Mock(_) => None,
-                })
-                .collect()
-        };
-        let mut read: HashSet<usize> = HashSet::new();
-        for sql in readers {
-            read.extend(helpers(
-                self.closure(&identifier_tokens(sql, patterns), true),
-            ));
-        }
-        for mock in &self.mocks {
-            if used_mocks.contains(&mock.mock_name) {
-                for index in helpers(self.closure(&mock.tokens, true)) {
-                    read.remove(&index);
-                }
-            }
-        }
-        read
+    /// Indexes of the helpers the compiler found the test's expected rows and assertions read.
+    pub(crate) fn read_helper_indexes(&self, names: &[String]) -> HashSet<usize> {
+        names
+            .iter()
+            .filter_map(|name| self.helper_indexes.get(&name.to_ascii_lowercase()).copied())
+            .collect()
     }
 
     /// Read helpers whose SQL, as placed in the test query, calls a reference such as `__ref()`.
@@ -344,6 +330,11 @@ impl ScopeGraph {
             .collect()
     }
 
+    /// The authored name of one helper.
+    pub(crate) fn helper_name(&self, index: usize) -> &str {
+        &self.helpers[index].name
+    }
+
     /// Replace a helper's references with the CTEs a reader must define ahead of it.
     pub(crate) fn resolve_helper(&mut self, index: usize, resolved: ResolvedHelper) {
         let placement = self.placement;
@@ -549,35 +540,21 @@ pub(crate) fn helper_scope_ctes(
     Ok(graph.collect_scope(nodes))
 }
 
-/// Merge scoped CTEs into a step's CTEs; relations resolved helpers read come first and win.
+/// Append scoped CTEs after a step's CTEs; a same-named mock or model CTE keeps the step's copy.
 pub(crate) fn merged_scoped_ctes(
     lifted: Vec<(String, String)>,
     scoped: HelperScope,
     file_label: &str,
 ) -> Result<Vec<(String, String)>, String> {
-    type Ctes = Vec<(String, String)>;
-    let (generated, authored): (Ctes, Ctes) = scoped
-        .ctes
-        .into_iter()
-        .partition(|(name, _)| scoped.generated.contains(&name.to_ascii_lowercase()));
     let mut merged: Vec<(String, String)> = lifted;
-    if !generated.is_empty() {
-        let own = std::mem::replace(&mut merged, generated);
-        for (name, sql) in own {
-            if !merged
-                .iter()
-                .any(|(existing, _)| existing.eq_ignore_ascii_case(&name))
-            {
-                merged.push((name, sql));
-            }
-        }
-    }
-    for (name, sql) in authored {
+    for (name, sql) in scoped.ctes {
         match merged
             .iter()
             .find(|(existing, _)| existing.eq_ignore_ascii_case(&name))
         {
-            Some((_, existing_sql)) if *existing_sql == sql => {}
+            Some((_, existing_sql))
+                if *existing_sql == sql
+                    || scoped.generated.contains(&name.to_ascii_lowercase()) => {}
             Some(_) => {
                 return Err(compile_error(&format!(
                     "SQL test '{file_label}' defines CTE '{name}', which conflicts with the generated CTE"

crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs

@@ -7,6 +7,7 @@ pub mod helper_names;
 pub mod helper_scope;
 pub mod markers;
 pub mod planning;
+pub mod reader_references;
 pub mod relation_markers;
 pub mod rendering;
 pub mod scenario_extraction;