diff --git a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/formatter.rs index bf432bf0a1..ab39f208af 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/formatter.rs +++ b/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) -> Result, +) -> Result { + let authored_parenthesized: Vec = 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 = 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 = 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 { + char_slice(sql, token.span.start, token.span.end).map(|value| value.to_ascii_uppercase()) +} + fn comment_insertion( formatted: &str, formatted_tokens: &[Token], diff --git a/crates/sqlbuild-rules-native/src/sql_lint/constants.rs b/crates/sqlbuild-rules-native/src/sql_lint/constants.rs index da36d20ba1..539928f25d 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/constants.rs +++ b/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; diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests.rs index 8bf146e4e8..086e1c5203 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs +++ b/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; diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_sql_lint.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_sql_lint.rs index d8b4dddba5..71155cae67 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_sql_lint.rs +++ b/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 { diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_values_formatter.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_values_formatter.rs new file mode 100644 index 0000000000..b71fe80394 --- /dev/null +++ b/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(()) +} diff --git a/src/sqlbuild/cli/commands/_helpers/lint/selection.py b/src/sqlbuild/cli/commands/_helpers/lint/selection.py index 8c57ba6923..615f493922 100644 --- a/src/sqlbuild/cli/commands/_helpers/lint/selection.py +++ b/src/sqlbuild/cli/commands/_helpers/lint/selection.py @@ -66,6 +66,37 @@ 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, @@ -73,8 +104,7 @@ def resolve_lint_inputs( 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( diff --git a/src/sqlbuild/cli/commands/main/project/_format.py b/src/sqlbuild/cli/commands/main/project/_format.py index f42641ef3e..588fcac330 100644 --- a/src/sqlbuild/cli/commands/main/project/_format.py +++ b/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: diff --git a/src/sqlbuild/compiler/discovery/main/_parse_sql_test_file.py b/src/sqlbuild/compiler/discovery/main/_parse_sql_test_file.py new file mode 100644 index 0000000000..6c366a9231 --- /dev/null +++ b/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) diff --git a/src/sqlbuild/compiler/planner/_helpers/sql_tests/fixture_formatting.py b/src/sqlbuild/compiler/planner/_helpers/sql_tests/fixture_formatting.py index 4c72ed67ae..3f6b5e3da4 100644 --- a/src/sqlbuild/compiler/planner/_helpers/sql_tests/fixture_formatting.py +++ b/src/sqlbuild/compiler/planner/_helpers/sql_tests/fixture_formatting.py @@ -15,6 +15,7 @@ from sqlbuild.compiler.compile.models import CompileSqlTestCte from sqlbuild.compiler.compile.types import CompiledResourceType, SqlTestMode from sqlbuild.compiler.discovery.main._model_schema_columns import parse_schema_columns +from sqlbuild.compiler.discovery.main._parse_sql_test_file import parse_sql_test_file from sqlbuild.compiler.discovery.models import ( DiscoveredProjectInputs, DiscoveredSqlModelFile, @@ -56,6 +57,14 @@ re.IGNORECASE, ) _SELECT_LINE_PATTERN: re.Pattern[str] = re.compile(r"^\s*SELECT\b(?P.*)$", re.IGNORECASE) +_EMPTY_TYPED_NULL_QUERY_PATTERN: re.Pattern[str] = re.compile( + r"^\s*SELECT\s+(?P.+?)\s+WHERE\s+FALSE\s*$", + re.IGNORECASE | re.DOTALL, +) +_SQL_QUOTE_CHARACTERS: frozenset[str] = frozenset({"'", '"', "`"}) +_OPEN_PARENTHESIS_CHARACTER: str = "(" +_CLOSE_PARENTHESIS_CHARACTER: str = ")" +_COMMA_CHARACTER: str = "," def format_redundant_fixture_nulls( @@ -78,8 +87,15 @@ def format_redundant_fixture_nulls( contents: str | None = updated.get(file_path, files.get(file_path)) if contents is None: continue + try: + current_blocks: tuple[DiscoveredSqlTestBlock, ...] = parse_sql_test_file( + contents=contents, + file_path=file_path, + ) + except (OSError, UnicodeError, ValueError, SyntaxError): + continue block: DiscoveredSqlTestBlock - for block in test_file.blocks: + for block in current_blocks: fixed_block: str = _format_test_block( block=block, file_label=str(test_file.relative_path), @@ -215,6 +231,8 @@ def _remove_redundant_typed_null_lines(*, sql: str, relation: FixtureRelationMet metadata_by_name: dict[str, FixtureColumnMetadata] = { column.name.casefold(): column for column in relation.columns } + if _is_empty_typed_null_query(sql=sql, metadata_by_name=metadata_by_name): + return "SELECT * FROM __empty_fixture()" lines: list[str] = sql.splitlines() typed_null_indexes: set[int] = set() remove_indexes: set[int] = set() @@ -256,6 +274,64 @@ def _remove_redundant_typed_null_lines(*, sql: str, relation: FixtureRelationMet return candidate +def _is_empty_typed_null_query( + *, sql: str, metadata_by_name: dict[str, FixtureColumnMetadata] +) -> bool: + """Return whether an empty fixture projects only known typed-null columns.""" + + query_match: re.Match[str] | None = _EMPTY_TYPED_NULL_QUERY_PATTERN.fullmatch(sql) + if query_match is None: + return False + projections: tuple[str, ...] = _split_top_level_expressions( + value=query_match.group("projection") + ) + if not projections: + return False + for projection in projections: + typed_null_match: re.Match[str] | None = _TYPED_NULL_LINE_PATTERN.fullmatch( + projection.strip() + ) + if typed_null_match is None: + return False + alias: str = typed_null_match.group("alias").strip('"`') + column: FixtureColumnMetadata | None = metadata_by_name.get(alias.casefold()) + if column is None or column.type is None: + return False + return True + + +def _split_top_level_expressions(*, value: str) -> tuple[str, ...]: + """Split a projection list without splitting commas inside types or quoted values.""" + + expressions: list[str] = [] + start: int = 0 + depth: int = 0 + quote: str | None = None + index: int = 0 + while index < len(value): + character: str = value[index] + if quote is not None: + if character == quote: + if index + 1 < len(value) and value[index + 1] == quote: + index += 2 + continue + quote = None + index += 1 + continue + if character in _SQL_QUOTE_CHARACTERS: + quote = character + elif character == _OPEN_PARENTHESIS_CHARACTER: + depth += 1 + elif character == _CLOSE_PARENTHESIS_CHARACTER: + depth = max(0, depth - 1) + elif character == _COMMA_CHARACTER and depth == 0: + expressions.append(value[start:index]) + start = index + 1 + index += 1 + expressions.append(value[start:]) + return tuple(expression for expression in expressions if expression.strip()) + + def _projection_aliases(*, lines: list[str]) -> tuple[str, ...] | None: aliases: list[str] = [] saw_select: bool = False diff --git a/src/sqlbuild/lint/_helpers/headers.py b/src/sqlbuild/lint/_helpers/headers.py index 72fe23f732..98131f6426 100644 --- a/src/sqlbuild/lint/_helpers/headers.py +++ b/src/sqlbuild/lint/_helpers/headers.py @@ -63,12 +63,15 @@ def _scan_delimited_regions( """Return line-leading parenthesized regions with one of the requested names.""" spans: list[HeaderSpan] = [] + covered_until: int = 0 match: re.Match[str] for match in _delimited_region_pattern(kinds).finditer(contents): kind: str | None = match.group("kind") if kind is None: continue keyword_start: int = match.start("kind") + if keyword_start < covered_until: + continue span: HeaderSpan | None = _match_header_span( contents=contents, kind=kind, @@ -77,6 +80,7 @@ def _scan_delimited_regions( ) if span is not None: spans.append(span) + covered_until = span.end if first_only: break return tuple(spans) diff --git a/src/sqlbuild/lint/_helpers/native.py b/src/sqlbuild/lint/_helpers/native.py index ac9dcd97c9..553eb60398 100644 --- a/src/sqlbuild/lint/_helpers/native.py +++ b/src/sqlbuild/lint/_helpers/native.py @@ -14,12 +14,14 @@ ) from sqlbuild.compiler.discovery.exceptions import ModelSqlParseError from sqlbuild.lint.constants import ( + CARRIAGE_RETURN_LINE_FEED, CLOSING_PAREN_CHARACTER, DESCRIPTION_HEADER_KINDS, DESCRIPTION_REQUIRED_HEADER_KINDS, HEADER_KIND_FUNCTION, HEADER_KIND_MODEL, IDENTIFIER_SEPARATOR_CHARACTER, + LINE_FEED, RULE_DESCRIPTION_LENGTH, RULE_DESCRIPTION_PRESENT, RULE_HEADER_PARSE, @@ -29,6 +31,7 @@ VIOLATION_SEVERITY_WARNING, ) from sqlbuild.lint.models import HeaderSpan, LintConfig, LintViolation +from sqlbuild.lint.types import LintSeverity _QUOTE_CHARACTERS: frozenset[str] = frozenset({"'", '"'}) _ESCAPE_CHARACTER: str = "\\" @@ -59,6 +62,7 @@ def lint_native_headers( file_path: Path, headers: tuple[HeaderSpan, ...], config: LintConfig, + description_present_severity: LintSeverity = VIOLATION_SEVERITY_FAULT, ) -> tuple[LintViolation, ...]: """Run all native header rules and return their violations.""" @@ -73,6 +77,7 @@ def lint_native_headers( header=header, config=config, line_starts=line_starts, + description_present_severity=description_present_severity, ) ) violations.extend( @@ -100,6 +105,64 @@ def prepare_native_header_cache(*, files: dict[Path, str]) -> None: prepare_model_header_tokens(headers) +def safe_format_files( + *, files: dict[Path, str], config: LintConfig +) -> tuple[dict[Path, str], dict[Path, str]]: + """Return parse-safe files together with every authored newline convention.""" + + safe_files: dict[Path, str] = { + file_path: contents + for file_path, contents in files.items() + if not _header_parse_faults(file_path=file_path, contents=contents, config=config) + } + newline_by_path: dict[Path, str] = { + file_path: CARRIAGE_RETURN_LINE_FEED if CARRIAGE_RETURN_LINE_FEED in contents else LINE_FEED + for file_path, contents in files.items() + } + return safe_files, newline_by_path + + +def reject_unparseable_header_rewrites( + *, + updated: dict[Path, str], + config: LintConfig, + faults: list[LintViolation], +) -> tuple[dict[Path, str], list[LintViolation]]: + """Discard every file rewrite whose resulting header cannot be parsed.""" + + accepted: dict[Path, str] = dict(updated) + for file_path, contents in tuple(updated.items()): + parse_faults: tuple[LintViolation, ...] = _header_parse_faults( + file_path=file_path, + contents=contents, + config=config, + ) + if not parse_faults: + continue + accepted.pop(file_path, None) + faults.extend(parse_faults) + return accepted, faults + + +def _header_parse_faults( + *, file_path: Path, contents: str, config: LintConfig +) -> tuple[LintViolation, ...]: + from sqlbuild.lint._helpers.headers import scan_headers + + headers: tuple[HeaderSpan, ...] = scan_headers(contents=contents) + return tuple( + violation + for violation in lint_native_headers( + contents=contents, + file_path=file_path, + headers=headers, + config=config, + description_present_severity=VIOLATION_SEVERITY_WARNING, + ) + if violation.code == RULE_HEADER_PARSE + ) + + def format_native_headers( *, contents: str, @@ -134,6 +197,7 @@ def _lint_header_values( header: HeaderSpan, config: LintConfig, line_starts: tuple[int, ...], + description_present_severity: LintSeverity, ) -> tuple[LintViolation, ...]: header_text: str = contents[header.start : header.end] try: @@ -180,6 +244,7 @@ def _lint_header_values( code=RULE_DESCRIPTION_PRESENT, message=f"{header.kind}() header requires a description", remediation=f"Add a description to the {header.kind}() header.", + severity=description_present_severity, ) ) if header.kind in DESCRIPTION_HEADER_KINDS and isinstance(effective_description, str): @@ -598,6 +663,7 @@ def _violation_for_header_start( code: str, message: str, remediation: str, + severity: LintSeverity = VIOLATION_SEVERITY_FAULT, ) -> LintViolation: position: tuple[int, int] = _offset_to_position( offset=header.start, line_starts=_line_starts(contents) @@ -608,7 +674,7 @@ def _violation_for_header_start( column=position[1], code=code, message=message, - severity=VIOLATION_SEVERITY_FAULT, + severity=severity, engine="sqlbuild", remediation=remediation, ) diff --git a/src/sqlbuild/lint/_helpers/native_format.py b/src/sqlbuild/lint/_helpers/native_format.py index 24b16dc631..0dc110b5de 100644 --- a/src/sqlbuild/lint/_helpers/native_format.py +++ b/src/sqlbuild/lint/_helpers/native_format.py @@ -10,7 +10,10 @@ import sqlbuild._native as _native from sqlbuild.lint._helpers.headers import lint_body_ranges, scan_headers from sqlbuild.lint._helpers.sqlbuild_tokens import neutralize_interpolation, restore_interpolation -from sqlbuild.lint.constants import CARRIAGE_RETURN_LINE_FEED, LINE_FEED +from sqlbuild.lint.constants import ( + CARRIAGE_RETURN_LINE_FEED, + LINE_FEED, +) from sqlbuild.lint.exceptions import NativeLintError from sqlbuild.lint.models import HeaderSpan, InterpolationSite, LintConfig @@ -46,7 +49,7 @@ def with_newline_style(*, contents: str, newline: str) -> str: def format_native_sql_bodies( *, files: dict[Path, str], config: LintConfig, project_dir: Path ) -> dict[Path, str]: - """Format supported authored SQL bodies and preserve unsupported bodies unchanged.""" + """Format supported SQL bodies while leaving declined bodies unchanged.""" prepared_by_path: dict[Path, tuple[_PreparedBody, ...]] = {} requests_by_key: dict[tuple[str, str], dict[str, object]] = {} @@ -93,8 +96,15 @@ def format_native_sql_bodies( response: dict[str, Any] = response_cache[prepared.cache_key] raw_sql: object = response.get("sql") changed: object = response.get("changed") - if not isinstance(raw_sql, str) or not isinstance(changed, bool): + formatted: object = response.get("formatted") + if ( + not isinstance(raw_sql, str) + or not isinstance(changed, bool) + or not isinstance(formatted, bool) + ): raise NativeLintError("native formatter returned invalid SQL or changed state") + if not formatted: + continue if not changed: continue restored: str = restore_interpolation( diff --git a/src/sqlbuild/lint/main/run_format.py b/src/sqlbuild/lint/main/run_format.py index c1bca1d779..0693b86fa8 100644 --- a/src/sqlbuild/lint/main/run_format.py +++ b/src/sqlbuild/lint/main/run_format.py @@ -12,14 +12,16 @@ format_native_headers, lint_native_headers, prepare_native_header_cache, + reject_unparseable_header_rewrites, + safe_format_files, ) from sqlbuild.lint._helpers.native_format import ( format_native_sql_bodies, - newline_style, with_newline_style, ) from sqlbuild.lint._helpers.project_files import collect_project_files, sort_violations from sqlbuild.lint._helpers.suppressions import apply_suppressions +from sqlbuild.lint.constants import VIOLATION_SEVERITY_WARNING from sqlbuild.lint.models import ( FormatChange, HeaderSpan, @@ -44,11 +46,9 @@ def run_format( files: dict[Path, str] = collect_project_files( project_dir=project_dir, selected_paths=selected_paths ) - newline_by_path: dict[Path, str] = { - file_path: newline_style(contents=contents) for file_path, contents in files.items() - } - updated_contents: dict[Path, str] = _apply_fixes( - files=files, + safe_files, newline_by_path = safe_format_files(files=files, config=config) + updated_contents, format_faults = _apply_fixes( + files=safe_files, config=config, project_dir=project_dir, discovered_inputs=discovered_inputs, @@ -89,6 +89,7 @@ def run_format( value_renderer=value_renderer, ) ) + violations.extend(format_faults) final_contents: dict[Path, str] = { path: updated_contents.get(path, contents) for path, contents in files.items() } @@ -113,7 +114,7 @@ def _apply_fixes( project_dir: Path, discovered_inputs: DiscoveredProjectInputs | None, fixtures_only: bool, -) -> dict[Path, str]: +) -> tuple[dict[Path, str], list[LintViolation]]: """Return contents after native header and supported SQL body fixes.""" updated: dict[Path, str] = ( @@ -126,7 +127,7 @@ def _apply_fixes( else {} ) if fixtures_only: - return updated + return updated, [] file_path: Path contents: str current_files: dict[Path, str] = { @@ -142,18 +143,45 @@ def _apply_fixes( if native_result[0] != contents: updated[file_path] = native_result[0] if not config.native_enabled: - return updated + return reject_unparseable_header_rewrites( + updated=updated, + config=config, + faults=[], + ) current_files = { file_path: updated.get(file_path, contents) for file_path, contents in files.items() } - updated.update( - format_native_sql_bodies( - files=current_files, + native_formatted: dict[Path, str] = format_native_sql_bodies( + files=current_files, + config=config, + project_dir=project_dir, + ) + updated.update(native_formatted) + post_native_files: dict[Path, str] = { + file_path: updated.get(file_path, contents) for file_path, contents in files.items() + } + fixture_formatted: dict[Path, str] = ( + FixtureNullAutofix.apply( + files=post_native_files, + project_dir=project_dir, + discovered_inputs=discovered_inputs, + ) + if discovered_inputs is not None + else {} + ) + if fixture_formatted: + final_native: dict[Path, str] = format_native_sql_bodies( + files=fixture_formatted, config=config, project_dir=project_dir, ) + updated.update(fixture_formatted) + updated.update(final_native) + return reject_unparseable_header_rewrites( + updated=updated, + config=config, + faults=[], ) - return updated def _lint_final_contents( @@ -179,6 +207,7 @@ def _lint_final_contents( file_path=file_path, headers=headers, config=config, + description_present_severity=VIOLATION_SEVERITY_WARNING, ) ) return violations diff --git a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py index 73dec1c303..bb5568f98a 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -159,6 +159,87 @@ class DescriptionFormatIntegrationTestCase: expected_formatted_description: str +@dataclass(frozen=True) +class FormatSafetyIntegrationTestCase: + """One format safety expectation through the real CLI.""" + + description: str + authored_sql: str + expected_fault_code: str + expected_exit_code: int + + +@dataclass(frozen=True) +class FormatterDeclineIntegrationTestCase: + """One expected native formatter decline through the real CLI.""" + + description: str + authored_body: str + expected_exit_code: int + + +@dataclass(frozen=True) +class TypedNullFormatIntegrationTestCase: + """One single-pass typed-null fixture formatting expectation.""" + + description: str + fixture_projection: str + expected_literal: str + expected_exit_code: int + + +@dataclass(frozen=True) +class CanonicalFixtureFormatIntegrationTestCase: + """One post-native fixture simplification expectation.""" + + description: str + expected_retained_literal: str + expected_removed_literal: str + expected_exit_code: int + + +@dataclass(frozen=True) +class FromValuesFormatIntegrationTestCase: + """One dialect expectation for an unparenthesized values relation.""" + + description: str + adapter: str + expected_exit_code: int + expected_literal: str + + +@dataclass(frozen=True) +class MixedFromValuesFormatIntegrationTestCase: + """One positional values-relation preservation expectation.""" + + description: str + authored_query: str + expected_parenthesized_literal: str + expected_unparenthesized_literal: str + expected_exit_code: int + + +@dataclass(frozen=True) +class FormatScopeIntegrationTestCase: + """Expected CLI outcomes for the format selection contract.""" + + description: str + expected_exclude_exit: int + expected_selected_model_exit: int + expected_path_with_exclude_exit: int + expected_exclude_path_exit: int + + +@dataclass(frozen=True) +class FormatWarningIntegrationTestCase: + """One non-failing format warning expectation.""" + + description: str + expected_exit_code: int + expected_code: str + expected_severity: str + + @dataclass(frozen=True) class ContractCommandIntegrationTestCase: """One target-backed contract CLI expectation.""" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py index 2d66509057..9f080e40e5 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py @@ -28,6 +28,54 @@ def write_compile_startup_project(project_dir: Path) -> None: ) +def write_from_values_format_project(*, tmp_path: Path, adapter: str) -> tuple[Path, Path]: + """Write a format project containing an authored values relation.""" + + project_dir: Path = tmp_path / adapter + project_dir.mkdir() + (project_dir / "sqlbuild_project.toml").write_text( + f'name = "orders"\nadapter = "{adapter}"\n\n[rules]\nselect = ["SQBRSQL039"]\n', + encoding="utf-8", + ) + models: Path = project_dir / "models" + models.mkdir() + (models / "customers.sql").write_text( + 'MODEL (description "Customers.");\nSELECT 1 AS customer_key\n', encoding="utf-8" + ) + (models / "orders.sql").write_text( + 'MODEL (description "Orders.");\nSELECT customer_key, 1 AS order_count ' + 'FROM __ref("customers")\n', + encoding="utf-8", + ) + test_file: Path = project_dir / "tests" / "unit" / "test_orders.sql" + test_file.parent.mkdir(parents=True) + test_file.write_text( + 'TEST (name "orders_from_values");\n\n' + "WITH __ref__customers AS (\n" + " SELECT COLUMN1::VARCHAR AS customer_key, COLUMN2::INTEGER AS order_count\n" + " FROM VALUES\n" + " ('c1', 1),\n" + " ('c2', 2)\n" + "),\n" + "__expected__orders AS (SELECT 'c1' AS customer_key, 1 AS order_count)\n" + "SELECT 1\n", + encoding="utf-8", + ) + return project_dir, test_file + + +def write_snowflake_format_test(*, tmp_path: Path, test_sql: str) -> tuple[Path, Path]: + """Write a minimal Snowflake-dialect project for real CLI formatting.""" + + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "products"\nadapter = "snowflake"\n', encoding="utf-8" + ) + test_file: Path = tmp_path / "tests" / "unit" / "test_products.sql" + test_file.parent.mkdir(parents=True) + test_file.write_text(test_sql, encoding="utf-8") + return tmp_path, test_file + + def prepare_contract_project(tmp_path: Path, *, prod_connection_toml: str = "") -> Path: database: Path = tmp_path / "warehouse.duckdb" _ = (tmp_path / "sqlbuild_project.toml").write_text( diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py index 6f9d7b7ebf..a257ae8ca3 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_format.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format.py @@ -2,14 +2,28 @@ from __future__ import annotations +import json from pathlib import Path import pytest +from _pytest.capture import CaptureResult from sqlbuild.cli.commands.main.entrypoint.entry import main from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( + CanonicalFixtureFormatIntegrationTestCase, DescriptionFormatIntegrationTestCase, FormatCompileIntegrationTestCase, + FormatSafetyIntegrationTestCase, + FormatScopeIntegrationTestCase, + FormatterDeclineIntegrationTestCase, + FormatWarningIntegrationTestCase, + FromValuesFormatIntegrationTestCase, + MixedFromValuesFormatIntegrationTestCase, + TypedNullFormatIntegrationTestCase, +) +from tests.integration.src.sqlbuild.cli.commands.main.helpers import ( + write_from_values_format_project, + write_snowflake_format_test, ) @@ -303,3 +317,479 @@ def test_given_description_width_when_formatting_then_wrapped_model_compiles( assert f'description "{test_case.expected_formatted_description}"' in model.read_text( encoding="utf-8" ) + + +@pytest.mark.parametrize( + "test_case", + [ + FormatCompileIntegrationTestCase( + description="schema column remains valid and idempotent", + expected_literal="SCHEMA (type VARCHAR)", + ) + ], + ids=lambda case: case.description, +) +def test_given_schema_column_when_formatting_then_header_remains_valid_and_idempotent( + test_case: FormatCompileIntegrationTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "catalog"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "catalog_objects.sql" + model.parent.mkdir() + model.write_text( + "MODEL (\n" + ' description "Catalog objects.",\n' + " materialized table,\n" + " columns (\n" + " object_id (type INTEGER),\n" + " SCHEMA (type VARCHAR),\n" + " ),\n" + ");\n\n" + "select 1 as object_id, 'analytics' as \"SCHEMA\"\n", + encoding="utf-8", + ) + + first_exit: int = main(["--project-dir", str(tmp_path), "format"]) + compile_exit: int = main(["--project-dir", str(tmp_path), "compile", "--no-cache"]) + formatted_once: str = model.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(tmp_path), "format", "--check"]) + + assert first_exit == 0 + assert compile_exit == 0 + assert second_exit == 0 + assert formatted_once == model.read_text(encoding="utf-8") + assert formatted_once.count(test_case.expected_literal) == 1 + + +@pytest.mark.parametrize( + "test_case", + [ + FormatSafetyIntegrationTestCase( + description="unparseable header is untouched", + authored_sql="MODEL (description);\nselect 1 as order_id\n", + expected_fault_code="header-parse", + expected_exit_code=1, + ), + ], + ids=lambda case: case.description, +) +def test_given_unparseable_file_when_formatting_then_file_is_untouched_and_faults( + test_case: FormatSafetyIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "orders.sql" + model.parent.mkdir() + model.write_text(test_case.authored_sql, encoding="utf-8") + + exit_code: int = main(["--project-dir", str(tmp_path), "format"]) + + captured: CaptureResult[str] = capsys.readouterr() + assert exit_code == test_case.expected_exit_code + assert test_case.expected_fault_code in captured.out + assert model.read_text(encoding="utf-8") == test_case.authored_sql + + +@pytest.mark.parametrize( + "test_case", + [ + FormatterDeclineIntegrationTestCase( + description="unsupported SQL body is skipped while header formats", + authored_body="select from\n", + expected_exit_code=0, + ), + FormatterDeclineIntegrationTestCase( + description="comment attachment decline is skipped while header formats", + authored_body="select order_id order_key /* keep */ from orders\n", + expected_exit_code=0, + ), + ], + ids=lambda case: case.description, +) +def test_given_native_formatter_decline_when_formatting_then_header_formats_and_body_is_unchanged( + test_case: FormatterDeclineIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "orders.sql" + model.parent.mkdir() + model.write_text( + "MODEL (\n" + ' description "Orders.", \n' + " materialized table,\n" + ");\n\n" + f"{test_case.authored_body}", + encoding="utf-8", + ) + + first_exit: int = main(["--project-dir", str(tmp_path), "format"]) + first_output: str = capsys.readouterr().out + formatted_once: str = model.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(tmp_path), "format", "--check"]) + second_output: str = capsys.readouterr().out + + assert first_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert ' description "Orders.",\n' in formatted_once + assert test_case.authored_body in formatted_once + assert "FAULT=0 WARN=0" in first_output + assert "FAULT=0 WARN=0" in second_output + assert formatted_once == model.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + TypedNullFormatIntegrationTestCase( + description="inline colon casts reach empty fixture fixed point in one pass", + fixture_projection=("NULL::VARCHAR AS customer_key, NULL::BIGINT AS order_count"), + expected_literal="__EMPTY_FIXTURE()", + expected_exit_code=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_inline_typed_null_fixture_when_formatting_then_one_pass_is_idempotent( + test_case: TypedNullFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "customers"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "customers.sql" + model.parent.mkdir() + model.write_text( + "MODEL (\n" + ' description "Customers.",\n' + " contract enforced,\n" + " columns (\n" + " customer_key (type VARCHAR),\n" + " order_count (type BIGINT),\n" + " ),\n" + ");\n\n" + "SELECT 'c1' AS customer_key, 1 AS order_count\n", + encoding="utf-8", + ) + test_file: Path = tmp_path / "tests" / "unit" / "test_customers.sql" + test_file.parent.mkdir(parents=True) + test_file.write_text( + "TEST();\n\n" + "WITH __ref__customers AS (\n" + f" SELECT {test_case.fixture_projection}\n" + " WHERE FALSE\n" + "),\n" + "__expected__customers AS (\n" + " SELECT 'c1' AS customer_key, 1 AS order_count\n" + ")\n" + "SELECT 1\n", + encoding="utf-8", + ) + + first_exit: int = main(["--project-dir", str(tmp_path), "format"]) + formatted_once: str = test_file.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(tmp_path), "format", "--check"]) + + assert first_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert test_case.expected_literal in formatted_once + assert "CAST(NULL" not in formatted_once + assert formatted_once == test_file.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + CanonicalFixtureFormatIntegrationTestCase( + description="post-native fixture simplification reaches fixed point in one pass", + expected_retained_literal="CAST(COLUMN2 AS TEXT) AS CLUSTER_ID", + expected_removed_literal="is_archived", + expected_exit_code=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_redundant_typed_null_after_multi_projection_when_formatting_then_is_idempotent( + test_case: CanonicalFixtureFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "customer_segments"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "customer_clusters.sql" + model.parent.mkdir() + model.write_text( + "MODEL (\n" + ' description "Customer clusters.",\n' + " contract enforced,\n" + " columns (\n" + " customer_key (type VARCHAR),\n" + " cluster_id (type VARCHAR),\n" + " is_archived (type BOOLEAN),\n" + " ),\n" + ");\n\n" + "SELECT 'c1' AS customer_key, 'k1' AS cluster_id, FALSE AS is_archived\n", + encoding="utf-8", + ) + test_file: Path = tmp_path / "tests" / "unit" / "test_customer_clusters.sql" + test_file.parent.mkdir(parents=True) + test_file.write_text( + "TEST();\n\n" + "WITH __ref__customer_clusters AS (\n" + " SELECT COLUMN1::VARCHAR AS CUSTOMER_KEY, COLUMN2::VARCHAR AS CLUSTER_ID,\n" + " NULL::BOOLEAN AS is_archived\n" + " FROM VALUES\n" + " ('c1', 'k1')\n" + "),\n" + "__expected__customer_clusters AS (\n" + " SELECT 'c1' AS customer_key, 'k1' AS cluster_id, FALSE AS is_archived\n" + ")\n" + "SELECT 1\n", + encoding="utf-8", + ) + + first_exit: int = main(["--project-dir", str(tmp_path), "format"]) + formatted_once: str = test_file.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(tmp_path), "format", "--check"]) + fixture_section: str = formatted_once.split("__expected__customer_clusters", maxsplit=1)[0] + + assert first_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert test_case.expected_retained_literal in fixture_section + assert test_case.expected_removed_literal not in fixture_section + assert formatted_once == test_file.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + FromValuesFormatIntegrationTestCase( + description="snowflake preserves unparenthesized values relation", + adapter="snowflake", + expected_exit_code=0, + expected_literal="FROM VALUES", + ) + ], + ids=lambda case: case.description, +) +def test_given_snowflake_from_values_when_formatting_then_form_is_preserved( + test_case: FromValuesFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + project_dir, test_file = write_from_values_format_project( + tmp_path=tmp_path, adapter=test_case.adapter + ) + + first_exit: int = main(["--project-dir", str(project_dir), "format"]) + formatted_once: str = test_file.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(project_dir), "format", "--check"]) + + assert first_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert test_case.expected_literal in formatted_once + assert "FROM (VALUES" not in formatted_once + assert formatted_once == test_file.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + FromValuesFormatIntegrationTestCase( + description="duckdb values relation remains valid and idempotent", + adapter="duckdb", + expected_exit_code=0, + expected_literal="FROM VALUES", + ) + ], + ids=lambda case: case.description, +) +def test_given_duckdb_from_values_when_formatting_then_project_compiles( + test_case: FromValuesFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + project_dir, test_file = write_from_values_format_project( + tmp_path=tmp_path, adapter=test_case.adapter + ) + + first_exit: int = main(["--project-dir", str(project_dir), "format"]) + compile_exit: int = main(["--project-dir", str(project_dir), "compile", "--no-cache"]) + formatted_once: str = test_file.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(project_dir), "format", "--check"]) + + assert first_exit == test_case.expected_exit_code + assert compile_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert test_case.expected_literal in formatted_once + assert formatted_once == test_file.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + MixedFromValuesFormatIntegrationTestCase( + description="mixed values forms retain positional authorship through real CLI", + authored_query=( + 'TEST (name "mixed_product_values");\n\n' + "-- VALUES inside this comment is not a relation.\n" + "SELECT 'VALUES' AS label FROM (VALUES (1)) AS first_values(id)\n" + "UNION ALL\n" + "SELECT 'second' AS label FROM VALUES (2)\n" + ), + expected_parenthesized_literal="FROM (VALUES (1)) AS first_values(id)", + expected_unparenthesized_literal="FROM VALUES (2)", + expected_exit_code=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_mixed_values_forms_when_formatting_then_each_relation_preserves_authorship( + test_case: MixedFromValuesFormatIntegrationTestCase, + tmp_path: Path, +) -> None: + project_dir, test_file = write_snowflake_format_test( + tmp_path=tmp_path, + test_sql=test_case.authored_query, + ) + + first_exit: int = main(["--project-dir", str(project_dir), "format"]) + formatted_once: str = test_file.read_text(encoding="utf-8") + second_exit: int = main(["--project-dir", str(project_dir), "format", "--check"]) + + assert first_exit == test_case.expected_exit_code + assert second_exit == test_case.expected_exit_code + assert test_case.expected_parenthesized_literal in formatted_once + assert test_case.expected_unparenthesized_literal in formatted_once + assert formatted_once == test_file.read_text(encoding="utf-8") + + +@pytest.mark.parametrize( + "test_case", + [ + FormatScopeIntegrationTestCase( + description="default exclusions and path selectors share format scope", + expected_exclude_exit=1, + expected_selected_model_exit=0, + expected_path_with_exclude_exit=1, + expected_exclude_path_exit=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_format_selectors_when_scoping_then_paths_and_default_exclusions_are_consistent( + test_case: FormatScopeIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + models: Path = tmp_path / "models" + models.mkdir() + (models / "customers.sql").write_text( + "MODEL (materialized table, columns (customer_id (type INTEGER),));\n" + "SELECT\n" + " 1 AS customer_id\n", + encoding="utf-8", + ) + (models / "orders.sql").write_text( + 'MODEL (description "Orders.", materialized table, ' + "columns (order_id (type INTEGER),));\n" + "SELECT\n" + " customer_id AS order_id\n" + 'FROM __ref("customers")\n', + encoding="utf-8", + ) + test_file: Path = tmp_path / "tests" / "unit" / "test_orders.sql" + test_file.parent.mkdir(parents=True) + unformatted_test: str = ( + 'TEST (name "orders_returns_one_row");\n\n' + "WITH __ref__customers AS (\n SELECT 1 AS customer_id\n),\n" + "__expected__orders AS (\n SELECT 1 AS order_id\n)\nSELECT 1\n" + ) + test_file.write_text(unformatted_test, encoding="utf-8") + + exclude_exit: int = main( + ["--project-dir", str(tmp_path), "format", "--check", "--exclude", "customers"] + ) + exclude_output: str = capsys.readouterr().err + selected_model_exit: int = main( + ["--project-dir", str(tmp_path), "format", "--check", "--select", "orders"] + ) + selected_model_output: str = capsys.readouterr().err + path_with_exclude_exit: int = main( + [ + "--project-dir", + str(tmp_path), + "format", + "--check", + "--select", + "path:tests", + "--exclude", + "orders", + ] + ) + path_with_exclude_output: str = capsys.readouterr().err + exclude_path_exit: int = main( + ["--project-dir", str(tmp_path), "format", "--check", "--exclude", "path:tests"] + ) + exclude_path_output: str = capsys.readouterr().err + + assert exclude_exit == test_case.expected_exclude_exit + assert "2 files checked" in exclude_output + assert selected_model_exit == test_case.expected_selected_model_exit + assert "1 files checked" in selected_model_output + assert path_with_exclude_exit == test_case.expected_path_with_exclude_exit + assert "1 files checked" in path_with_exclude_output + assert exclude_path_exit == test_case.expected_exclude_path_exit + assert "2 files checked" in exclude_path_output + assert test_file.read_text(encoding="utf-8") == unformatted_test + + +@pytest.mark.parametrize( + "test_case", + [ + FormatWarningIntegrationTestCase( + description="missing model description warns without failing format", + expected_exit_code=0, + expected_code="description-present", + expected_severity="warning", + ) + ], + ids=lambda case: case.description, +) +def test_given_missing_description_when_formatting_then_warning_does_not_fail( + test_case: FormatWarningIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + model: Path = tmp_path / "models" / "orders.sql" + model.parent.mkdir() + model.write_text("MODEL (materialized table);\nSELECT 1 AS order_id\n", encoding="utf-8") + + write_exit: int = main(["--project-dir", str(tmp_path), "format"]) + _ = capsys.readouterr() + text_exit: int = main(["--project-dir", str(tmp_path), "format", "--check"]) + text_output: str = capsys.readouterr().out + json_exit: int = main(["--project-dir", str(tmp_path), "format", "--check", "--json"]) + payload: dict[str, object] = json.loads(capsys.readouterr().out) + + assert write_exit == test_case.expected_exit_code + assert text_exit == test_case.expected_exit_code + assert f"warning[{test_case.expected_code}]" in text_output + assert json_exit == test_case.expected_exit_code + assert payload["faults"] == 0 + assert payload["warnings"] == 1 + violations: object = payload["violations"] + assert isinstance(violations, list) + assert violations[0]["code"] == test_case.expected_code + assert violations[0]["severity"] == test_case.expected_severity diff --git a/tests/unit/src/sqlbuild/lint/_helpers/test_headers.py b/tests/unit/src/sqlbuild/lint/_helpers/test_headers.py index 0041b4d6d3..a2710cb009 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/test_headers.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/test_headers.py @@ -54,6 +54,14 @@ ), expected_kinds=("MODEL",), ), + ScanHeadersTestCase( + description="ignores header keyword used as a nested column name", + contents=( + "MODEL (\n columns (\n object_id (type INTEGER),\n" + " SCHEMA (type VARCHAR(100)),\n ),\n);\nSELECT 1 AS object_id\n" + ), + expected_kinds=("MODEL",), + ), ScanHeadersTestCase( description="ignores header-like text inside comments", contents=( diff --git a/tests/unit/src/sqlbuild/lint/test_run_lint.py b/tests/unit/src/sqlbuild/lint/test_run_lint.py index 95d626b25b..40838593e9 100644 --- a/tests/unit/src/sqlbuild/lint/test_run_lint.py +++ b/tests/unit/src/sqlbuild/lint/test_run_lint.py @@ -90,10 +90,10 @@ def test_given_synthetic_project_when_linting_then_results_match_expected( expected_formatted_count=1, ), FormatProjectTestCase( - description="unfixable faults remain after formatting", + description="missing description remains a non-failing format warning", files={"models/no_description.sql": NO_DESCRIPTION_MODEL}, expected_written_fragments={}, - expected_fault_codes=("description-present",), + expected_fault_codes=(), expected_formatted_count=1, ), FormatProjectTestCase(