fix(format): make sqb format safe and idempotent

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

@@ -6,6 +6,7 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator};
 
 use crate::sql_lint::constants::{
     CAST_TYPE_SEPARATOR_KEYWORD, CLOSE_PARENTHESIS, LINT_API_VERSION, OPEN_PARENTHESIS,
+    VALUES_RELATION_PREFIX_TOKEN_COUNT,
 };
 use crate::sql_lint::models::{FormatRequest, FormatResponse};
 
@@ -119,6 +120,7 @@ fn format_once(neutral_sql: &str, context: &FormatOnceContext<'_>) -> Result<Str
     let mut formatted = format_by_name(neutral_sql, context.dialect_name)
         .map_err(|error| error.to_string())?
         .join(";\n");
+    formatted = restore_unparenthesized_from_values(formatted, context)?;
     formatted = restore_string_literals(formatted, context)?;
     formatted = restore_cast_type_spellings(formatted, context)?;
     if context.comments.is_empty() {
@@ -153,6 +155,90 @@ fn format_once(neutral_sql: &str, context: &FormatOnceContext<'_>) -> Result<Str
     Ok(formatted)
 }
 
+fn restore_unparenthesized_from_values(
+    mut formatted: String,
+    context: &FormatOnceContext<'_>,
+) -> Result<String, String> {
+    let authored_parenthesized: Vec<bool> = context
+        .original_tokens
+        .iter()
+        .enumerate()
+        .filter(|(_, token)| token_text(context.original_sql, token).as_deref() == Some("VALUES"))
+        .map(|(index, _)| {
+            index > 0
+                && token_text(context.original_sql, &context.original_tokens[index - 1]).as_deref()
+                    == Some(OPEN_PARENTHESIS)
+        })
+        .collect();
+    if authored_parenthesized.is_empty() {
+        return Ok(formatted);
+    }
+    let formatted_tokens = context
+        .dialect
+        .tokenize(&formatted)
+        .map_err(|error| error.to_string())?;
+    let formatted_values: Vec<usize> = formatted_tokens
+        .iter()
+        .enumerate()
+        .filter_map(|(index, token)| {
+            (token_text(&formatted, token).as_deref() == Some("VALUES")).then_some(index)
+        })
+        .collect();
+    if authored_parenthesized.len() != formatted_values.len() {
+        return Ok(formatted);
+    }
+    let mut removals: Vec<(usize, usize)> = Vec::new();
+    for (was_parenthesized, values_index) in
+        authored_parenthesized.into_iter().zip(formatted_values)
+    {
+        if was_parenthesized || values_index < VALUES_RELATION_PREFIX_TOKEN_COUNT {
+            continue;
+        }
+        let wrapper_index = values_index - 1;
+        let relation_index = values_index - 2;
+        let relation_keyword = token_text(&formatted, &formatted_tokens[relation_index]);
+        if token_text(&formatted, &formatted_tokens[wrapper_index]).as_deref()
+            != Some(OPEN_PARENTHESIS)
+            || !matches!(relation_keyword.as_deref(), Some("FROM" | "JOIN"))
+        {
+            continue;
+        }
+        let mut depth = 0_i32;
+        let mut close_index: Option<usize> = None;
+        for (index, token) in formatted_tokens.iter().enumerate().skip(wrapper_index) {
+            match token_text(&formatted, token).as_deref() {
+                Some(OPEN_PARENTHESIS) => depth += 1,
+                Some(CLOSE_PARENTHESIS) => {
+                    depth -= 1;
+                    if depth == 0 {
+                        close_index = Some(index);
+                        break;
+                    }
+                }
+                _ => {}
+            }
+        }
+        let Some(close_index) = close_index else {
+            return Ok(formatted);
+        };
+        let open = &formatted_tokens[wrapper_index];
+        let close = &formatted_tokens[close_index];
+        removals.push((open.span.start, open.span.end));
+        removals.push((close.span.start, close.span.end));
+    }
+    removals.sort_unstable();
+    for (start, end) in removals.into_iter().rev() {
+        let start = char_to_byte(&formatted, start)?;
+        let end = char_to_byte(&formatted, end)?;
+        formatted.replace_range(start..end, "");
+    }
+    Ok(formatted)
+}
+
+fn token_text(sql: &str, token: &Token) -> Option<String> {
+    char_slice(sql, token.span.start, token.span.end).map(|value| value.to_ascii_uppercase())
+}
+
 fn comment_insertion(
     formatted: &str,
     formatted_tokens: &[Token],

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

@@ -2,3 +2,4 @@ pub(crate) const CAST_TYPE_SEPARATOR_KEYWORD: &str = "AS";
 pub(crate) const CLOSE_PARENTHESIS: &str = ")";
 pub(crate) const LINT_API_VERSION: u32 = 1;
 pub(crate) const OPEN_PARENTHESIS: &str = "(";
+pub(crate) const VALUES_RELATION_PREFIX_TOKEN_COUNT: usize = 2;

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

@@ -8,3 +8,5 @@ mod helpers;
 mod sql_lint;
 #[path = "tests/test_types.rs"]
 mod test_types;
+#[path = "tests/test_values_formatter.rs"]
+mod values_formatter;

crates/sqlbuild-rules-native/src/sql_lint/tests/test_sql_lint.rs

@@ -1893,6 +1893,12 @@ fn given_format_cases_when_formatting_then_output_matches() -> Result<(), String
             expected_sql: "SELECT\n  a\nFROM items -- retained\n",
             expected_changed: true,
         },
+        test_types::FormatTestCase {
+            description: "unparenthesized values relation remains unparenthesized",
+            sql: "select column1::varchar as customer_key, column2::integer as order_count from values ('c1', 1), ('c2', 2)",
+            expected_sql: "SELECT\n  CAST(column1 AS VARCHAR) AS customer_key,\n  CAST(column2 AS INT) AS order_count\nFROM VALUES ('c1', 1), ('c2', 2)",
+            expected_changed: true,
+        },
     ];
 
     for test_case in &test_cases {

crates/sqlbuild-rules-native/src/sql_lint/tests/test_values_formatter.rs

@@ -0,0 +1,53 @@
+use serde_json::{Value, json};
+
+use crate::sql_lint::main::formatter::format_json;
+use crate::sql_lint::tests::test_types;
+
+#[test]
+fn given_mixed_values_relations_when_formatting_then_each_authored_form_is_preserved()
+-> Result<(), String> {
+    let test_cases = [
+        test_types::FormatTestCase {
+            description: "mixed values relations preserve parenthesized relation first",
+            sql: "SELECT * FROM (VALUES (1, 2)) AS first_values(a, b) UNION ALL SELECT * FROM VALUES (3, 4)",
+            expected_sql: "SELECT\n  *\nFROM (VALUES (1, 2)) AS first_values(a, b)\nUNION ALL\nSELECT\n  *\nFROM VALUES (3, 4)",
+            expected_changed: true,
+        },
+        test_types::FormatTestCase {
+            description: "mixed values relations preserve unparenthesized relation first",
+            sql: "SELECT * FROM VALUES (1, 2) UNION ALL SELECT * FROM (VALUES (3, 4)) AS second_values(a, b)",
+            expected_sql: "SELECT\n  *\nFROM VALUES (1, 2)\nUNION ALL\nSELECT\n  *\nFROM (VALUES (3, 4)) AS second_values(a, b)",
+            expected_changed: true,
+        },
+        test_types::FormatTestCase {
+            description: "mixed values relations inside joined subquery preserve authored forms",
+            sql: "SELECT base.id FROM items AS base JOIN (SELECT * FROM (VALUES (1)) AS first_values(id) UNION ALL SELECT * FROM VALUES (2)) AS mixed ON base.id = mixed.id",
+            expected_sql: "SELECT\n  base.id\nFROM items AS base\nJOIN (\n  SELECT\n    *\n  FROM (VALUES (1)) AS first_values(id)\n  UNION ALL\n  SELECT\n    *\n  FROM VALUES (2)\n) AS mixed\n  ON base.id = mixed.id",
+            expected_changed: true,
+        },
+        test_types::FormatTestCase {
+            description: "values text in string and comment is ignored",
+            sql: "-- VALUES and FROM VALUES are examples\nSELECT 'VALUES' AS label FROM (VALUES (1)) AS first_values(id) UNION ALL SELECT * FROM VALUES (2)",
+            expected_sql: "-- VALUES and FROM VALUES are examples\nSELECT\n  'VALUES' AS label\nFROM (VALUES (1)) AS first_values(id)\nUNION ALL\nSELECT\n  *\nFROM VALUES (2)",
+            expected_changed: true,
+        },
+    ];
+
+    for test_case in &test_cases {
+        let response = format_json(
+            &json!({"version": 1, "sql": test_case.sql, "dialect": "snowflake"}).to_string(),
+        )?;
+        let payload: Value = serde_json::from_str(&response).map_err(|error| error.to_string())?;
+        assert_eq!(
+            payload["sql"], test_case.expected_sql,
+            "{}",
+            test_case.description
+        );
+        assert_eq!(
+            payload["changed"], test_case.expected_changed,
+            "{}",
+            test_case.description
+        );
+    }
+    Ok(())
+}

src/sqlbuild/cli/commands/_helpers/lint/selection.py

@@ -66,15 +66,45 @@ def resolve_lint_inputs(
     )
     if not select and not exclude:
         return adapter, None, discovered
+    combined_paths: frozenset[Path] | None = _resolve_combined_lint_paths(
+        project_dir=project_dir,
+        discovered=discovered,
+        adapter=adapter,
+        select=select,
+        exclude=exclude,
+    )
+    if combined_paths is not None:
+        _validate_selected_paths(paths=combined_paths)
+        return adapter, combined_paths, discovered
+    paths: frozenset[Path] = _resolve_model_paths(
+        project_dir=project_dir,
+        discovered=discovered,
+        adapter=adapter,
+        select=select,
+        exclude=exclude,
+    )
+    _validate_selected_paths(paths=paths)
+    return adapter, paths, discovered
+
+
+def _resolve_model_paths(
+    *,
+    project_dir: Path,
+    discovered: DiscoveredProjectInputs,
+    adapter: BaseAdapter,
+    select: tuple[str, ...],
+    exclude: tuple[str, ...],
+) -> frozenset[Path]:
+    """Resolve model selectors to authored model paths."""
+
     exact_paths: frozenset[Path] | None = _resolve_exact_model_paths(
         project_dir=project_dir,
         discovered=discovered,
         select=select,
         exclude=exclude,
     )
     if exact_paths is not None:
-        _validate_selected_paths(paths=exact_paths)
-        return adapter, exact_paths, discovered
+        return exact_paths
     graph: ProjectGraph = build_project_graph(discovered_inputs=discovered, adapter=adapter)
     selected_keys: frozenset[CompiledObjectKey] = resolve_project_selectors(
         select=select,
@@ -88,13 +118,84 @@ def resolve_lint_inputs(
     selected_names: frozenset[str] = frozenset(
         key.name for key in selected_keys if key.resource_type == CompiledResourceType.MODEL
     )
-    paths: frozenset[Path] = frozenset(
+    return frozenset(
         (project_dir / model.relative_path).resolve()
         for model in graph.project.models
         if model.name in selected_names
     )
-    _validate_selected_paths(paths=paths)
-    return adapter, paths, discovered
+
+
+def _resolve_combined_lint_paths(
+    *,
+    project_dir: Path,
+    discovered: DiscoveredProjectInputs,
+    adapter: BaseAdapter,
+    select: tuple[str, ...],
+    exclude: tuple[str, ...],
+) -> frozenset[Path] | None:
+    """Resolve default or path-based format scopes before graph selector expansion."""
+
+    if any(not selector.strip() for selector in (*select, *exclude)):
+        return None
+    selected_path_tokens, selected_model_tokens = _partition_lint_selectors(select)
+    excluded_path_tokens, excluded_model_tokens = _partition_lint_selectors(exclude)
+    has_path_selector: bool = bool(selected_path_tokens or excluded_path_tokens)
+    if not has_path_selector and select:
+        return None
+    all_paths: frozenset[Path] = frozenset(
+        _paths_for_prefixes(project_dir=project_dir, prefixes=())
+    )
+    if select:
+        selected: set[Path] = set(
+            _paths_for_prefixes(project_dir=project_dir, prefixes=selected_path_tokens)
+            if selected_path_tokens
+            else ()
+        )
+        if selected_model_tokens:
+            selected.update(
+                _resolve_model_paths(
+                    project_dir=project_dir,
+                    discovered=discovered,
+                    adapter=adapter,
+                    select=selected_model_tokens,
+                    exclude=(),
+                )
+            )
+    else:
+        selected = set(all_paths)
+    excluded: set[Path] = set(
+        _paths_for_prefixes(project_dir=project_dir, prefixes=excluded_path_tokens)
+        if excluded_path_tokens
+        else ()
+    )
+    if excluded_model_tokens:
+        excluded.update(
+            _resolve_model_paths(
+                project_dir=project_dir,
+                discovered=discovered,
+                adapter=adapter,
+                select=excluded_model_tokens,
+                exclude=(),
+            )
+        )
+    return frozenset(selected - excluded)
+
+
+def _partition_lint_selectors(
+    raw_selectors: tuple[str, ...],
+) -> tuple[tuple[str, ...], tuple[str, ...]]:
+    """Separate valid formatter-root paths from model selectors."""
+
+    path_tokens: list[str] = []
+    model_tokens: list[str] = []
+    for raw_selector in raw_selectors:
+        for token in raw_selector.split():
+            prefixes: tuple[str, ...] | None = _lint_path_prefixes(raw_selectors=(token,))
+            if prefixes is not None:
+                path_tokens.extend(prefixes)
+            else:
+                model_tokens.append(token)
+    return tuple(path_tokens), tuple(model_tokens)
 
 
 def _resolve_discovered_adapter(

src/sqlbuild/cli/commands/main/project/_format.py

@@ -114,7 +114,7 @@ def run_format_command(
             show_formatted=True,
             formatted_heading="Would format files:" if check or diff else "Formatted files:",
         )
-    return 1 if result.violations or (check and result.formatted_files) else 0
+    return 1 if result.faults or (check and result.formatted_files) else 0
 
 
 def _render_format_diff(*, result: LintRunResult) -> None:

src/sqlbuild/compiler/discovery/main/_parse_sql_test_file.py

@@ -0,0 +1,14 @@
+"""Public discovery entry point for parsing in-memory SQL test contents."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from sqlbuild.compiler.discovery._helpers.sql.tests import parse_sql_test_file as _parse
+from sqlbuild.compiler.discovery.models import DiscoveredSqlTestBlock
+
+
+def parse_sql_test_file(*, contents: str, file_path: Path) -> tuple[DiscoveredSqlTestBlock, ...]:
+    """Parse current SQL test contents into ordered test blocks."""
+
+    return _parse(contents=contents, file_path=file_path)