diff --git a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py index d6f0534ceb..76982efdca 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py @@ -155,6 +155,8 @@ def _analyze_compile_project( _ = complete_compile_phase( status=status, message=f"Compiled project graph. ({graph_ms / 1000:.2f}s)" ) + graph_failed: bool = any(diagnostic.is_error for diagnostic in graph.project.diagnostics) + early_lint_wait_ms: int = early_lint.stop() if graph_failed else 0 lineage_start: float = time.monotonic() _ = start_compile_phase(status=status, message="Analyzing column lineage...") lineage: ProjectColumnLineage | None = build_compile_lineage( @@ -197,7 +199,7 @@ def _analyze_compile_project( *graph.project.diagnostics, *contract_result.diagnostics, ) - if not any(diagnostic.is_error for diagnostic in graph.project.diagnostics): + if not graph_failed: rules_config: RulesConfig = load_rules_config(project_dir=project_dir) if ( prepared_artifacts is not None @@ -224,6 +226,7 @@ def _analyze_compile_project( elapsed_seconds=time.monotonic() - rules_start, result=rules_result ), ) + early_lint_wait_ms = early_lint.stop() rule_diagnostics: tuple[CompilerDiagnostic, ...] = tuple( CompilerDiagnostic( phase=DiagnosticPhase.RULE, @@ -252,6 +255,7 @@ def _analyze_compile_project( contract_ms=contract_ms, built_in_rules_ms=rules_result.built_in_ms, custom_rules_ms=rules_result.custom_ms, + early_lint_wait_ms=early_lint_wait_ms, rule_cache_hits=rules_result.cache_hits, rule_cache_misses=rules_result.cache_misses, skipped_type_proof_rules=rules_result.skipped_type_proof_rules, diff --git a/src/sqlbuild/cli/commands/main/project/_compile.py b/src/sqlbuild/cli/commands/main/project/_compile.py index 404cc1c35a..08bfa4e7c0 100644 --- a/src/sqlbuild/cli/commands/main/project/_compile.py +++ b/src/sqlbuild/cli/commands/main/project/_compile.py @@ -124,6 +124,7 @@ def _run_compile_with_status( "contracts_ms": analysis.contract_ms, "built_in_rules_ms": analysis.built_in_rules_ms, "custom_rules_ms": analysis.custom_rules_ms, + "early_lint_wait_ms": analysis.early_lint_wait_ms, "rule_cache_hits": analysis.rule_cache_hits, "rule_cache_misses": analysis.rule_cache_misses, "write_ms": write_result.write_ms, diff --git a/src/sqlbuild/cli/compile/models.py b/src/sqlbuild/cli/compile/models.py index 7a177ea7db..a29dfa2f69 100644 --- a/src/sqlbuild/cli/compile/models.py +++ b/src/sqlbuild/cli/compile/models.py @@ -60,6 +60,7 @@ class CompileAnalysis: contract_ms: int built_in_rules_ms: int = 0 custom_rules_ms: int = 0 + early_lint_wait_ms: int = 0 rule_cache_hits: int = 0 rule_cache_misses: int = 0 skipped_type_proof_rules: tuple[str, ...] = () diff --git a/src/sqlbuild/lint/_helpers/native_sql.py b/src/sqlbuild/lint/_helpers/native_sql.py index 1ec25f0171..6f27d2eb41 100644 --- a/src/sqlbuild/lint/_helpers/native_sql.py +++ b/src/sqlbuild/lint/_helpers/native_sql.py @@ -5,6 +5,7 @@ import re from bisect import bisect_right from pathlib import Path +from threading import Event from typing import Any, cast import orjson @@ -28,6 +29,7 @@ from sqlbuild.compiler.compile.main.map_expanded_offset import map_expanded_offset from sqlbuild.compiler.compile.models import MappedOffset from sqlbuild.lint._helpers.sqlbuild_tokens import interpolation_text_at +from sqlbuild.lint.classes.stop_context import LintStopContext from sqlbuild.lint.constants import ( GENERATED_SQL_MESSAGE_SUFFIX, GENERATED_SQL_MESSAGE_TEMPLATE, @@ -35,7 +37,7 @@ VIOLATION_SEVERITY_FAULT, VIOLATION_SEVERITY_WARNING, ) -from sqlbuild.lint.exceptions import NativeLintError +from sqlbuild.lint.exceptions import LintStoppedError, NativeLintError from sqlbuild.lint.models import LintBody, LintConfig, LintEdit, LintViolation _NATIVE_LINT_API_VERSION: int = 1 @@ -87,6 +89,7 @@ def run_native_sql_lint( """Lint expanded bodies in Rust and map diagnostics to authored source.""" violations_by_file: dict[Path, list[LintViolation]] = {} + line_starts_by_path: dict[Path, tuple[int, ...]] = {} requests: dict[_NativeCacheKey, dict[str, object]] = {} for body in bodies: cache_key: _NativeCacheKey = _native_cache_key(body=body, config=config) @@ -118,15 +121,20 @@ def run_native_sql_lint( if cache_key[-1]: payload["relation_keys"] = dict(cache_key[-1]) requests[cache_key] = payload + check_lint_not_stopped() response_cache: dict[_NativeCacheKey, _NativeResult] = _native_responses(requests=requests) for body in bodies: + check_lint_not_stopped() response: _NativeResult = response_cache[_native_cache_key(body=body, config=config)] + contents: str = contents_by_path[body.file_path] + line_starts: tuple[int, ...] | None = line_starts_by_path.get(body.file_path) + if line_starts is None: + line_starts = _line_starts(contents) + line_starts_by_path[body.file_path] = line_starts if isinstance(response, NativeLintError): parse_violation: LintViolation | None = _parse_failure_violation( - error=response, - body=body, - contents=contents_by_path[body.file_path], + error=response, body=body, line_starts=line_starts ) if parse_violation is None: raise response @@ -140,18 +148,27 @@ def run_native_sql_lint( violation: LintViolation | None = _authored_violation( raw_diagnostic=raw_diagnostic, body=body, - contents=contents_by_path[body.file_path], + contents=contents, + line_starts=line_starts, dialect=config.dialect, ) except NativeLintError as error: violation = _parse_failure_violation( - error=error, body=body, contents=contents_by_path[body.file_path] + error=error, body=body, line_starts=line_starts ) if violation is not None: violations_by_file.setdefault(body.file_path, []).append(violation) return {path: tuple(entries) for path, entries in violations_by_file.items()} +def check_lint_not_stopped() -> None: + """Abandon a speculative lint run on this thread once its owner no longer needs it.""" + + stop: Event | None = LintStopContext.active.get() + if stop is not None and stop.is_set(): + raise LintStoppedError + + def _native_cache_key(*, body: LintBody, config: LintConfig) -> _NativeCacheKey: return ( body.lint_text, @@ -223,7 +240,7 @@ def _native_result(*, response: object) -> _NativeResult: def _parse_failure_violation( - *, error: NativeLintError, body: LintBody, contents: str + *, error: NativeLintError, body: LintBody, line_starts: tuple[int, ...] ) -> LintViolation: """Convert a per-body native parse rejection into an authored project fault.""" @@ -237,7 +254,7 @@ def _parse_failure_violation( expanded_offset += min(expanded_column - 1, len(expanded_lines[expanded_line - 1])) mapped: MappedOffset = map_expanded_offset(offset=expanded_offset, passes=body.passes) absolute_offset: int = body.body_start + mapped.offset - line, column = _offset_position(offset=absolute_offset, line_starts=_line_starts(contents)) + line, column = _offset_position(offset=absolute_offset, line_starts=line_starts) return LintViolation( file_path=body.file_path, line=line, @@ -251,7 +268,12 @@ def _parse_failure_violation( def _authored_violation( - *, raw_diagnostic: object, body: LintBody, contents: str, dialect: str + *, + raw_diagnostic: object, + body: LintBody, + contents: str, + line_starts: tuple[int, ...], + dialect: str, ) -> LintViolation | None: if not isinstance(raw_diagnostic, dict): raise NativeLintError("native lint diagnostic must be an object") @@ -286,8 +308,7 @@ def _authored_violation( _SQLBUILD_HARNESS_CTE_PREFIXES, absolute_offset ): return None - starts: tuple[int, ...] = _line_starts(contents) - line, column = _offset_position(offset=absolute_offset, line_starts=starts) + line, column = _offset_position(offset=absolute_offset, line_starts=line_starts) end_position: tuple[int, int] | None = _authored_end_position( start=start, end=end, @@ -295,7 +316,7 @@ def _authored_violation( absolute_start=absolute_offset, body=body, contents=contents, - line_starts=starts, + line_starts=line_starts, dialect=dialect, ) fix: LintEdit | None = _authored_fix( diff --git a/src/sqlbuild/lint/classes/__init__.py b/src/sqlbuild/lint/classes/__init__.py new file mode 100644 index 0000000000..29801d9b58 --- /dev/null +++ b/src/sqlbuild/lint/classes/__init__.py @@ -0,0 +1 @@ +"""Runtime classes for the lint and format layer.""" diff --git a/src/sqlbuild/lint/classes/stop_context.py b/src/sqlbuild/lint/classes/stop_context.py new file mode 100644 index 0000000000..11d2c0b9c0 --- /dev/null +++ b/src/sqlbuild/lint/classes/stop_context.py @@ -0,0 +1,11 @@ +"""Thread-local stop signal for speculative lint runs.""" + +from contextvars import ContextVar +from threading import Event +from typing import ClassVar + + +class LintStopContext: + """Own the stop signal that lets a speculative lint run be abandoned between files.""" + + active: ClassVar[ContextVar[Event | None]] = ContextVar("active_lint_stop", default=None) diff --git a/src/sqlbuild/lint/exceptions.py b/src/sqlbuild/lint/exceptions.py index 80e650904b..bc3dd9fd03 100644 --- a/src/sqlbuild/lint/exceptions.py +++ b/src/sqlbuild/lint/exceptions.py @@ -31,3 +31,7 @@ class ProjectCompileError(LintError): """The project could not be compiled, so its SQL cannot be linted.""" code: str = "L005" + + +class LintStoppedError(Exception): + """A speculative lint run was stopped because its result is no longer needed.""" diff --git a/src/sqlbuild/lint/main/run_lint.py b/src/sqlbuild/lint/main/run_lint.py index e768fa0bf5..f746cd5975 100644 --- a/src/sqlbuild/lint/main/run_lint.py +++ b/src/sqlbuild/lint/main/run_lint.py @@ -17,7 +17,7 @@ from sqlbuild.lint._helpers.expansion import build_lint_expansion_context, prepare_lint_body from sqlbuild.lint._helpers.headers import lint_body_ranges, lint_file_role, scan_headers from sqlbuild.lint._helpers.native import external_identifiers_for_headers, lint_native_headers -from sqlbuild.lint._helpers.native_sql import run_native_sql_lint +from sqlbuild.lint._helpers.native_sql import check_lint_not_stopped, run_native_sql_lint 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 HEADER_KIND_SCENARIO, HEADER_KIND_TEST @@ -154,6 +154,7 @@ def _prepared_bodies( ) -> tuple[LintBody, ...] | str: """Prepared bodies of one file, or the reason its SQL cannot be expanded when skipping.""" + check_lint_not_stopped() bodies: list[LintBody] = [] role: LintFileRole = lint_file_role( file_path=file_path, project_dir=project_dir, relative_path=relative_path diff --git a/src/sqlbuild/rule_engine/_helpers/run/rules.py b/src/sqlbuild/rule_engine/_helpers/run/rules.py index 929e79dcca..a9aa96dd97 100644 --- a/src/sqlbuild/rule_engine/_helpers/run/rules.py +++ b/src/sqlbuild/rule_engine/_helpers/run/rules.py @@ -7,8 +7,10 @@ import os import time from concurrent.futures import Executor, Future, ThreadPoolExecutor +from contextvars import Token from dataclasses import asdict, dataclass, replace from pathlib import Path +from threading import Event from typing import Any import sqlbuild._native as _native @@ -25,6 +27,7 @@ from sqlbuild.compiler.compile.types import CompiledResourceType from sqlbuild.compiler.discovery.models import DiscoveredProjectInputs from sqlbuild.compiler.pipeline.models import ProjectGraph +from sqlbuild.lint.classes.stop_context import LintStopContext from sqlbuild.lint.constants import ( DEFAULT_MAX_LITERAL_LENGTH, DEFAULT_MAX_RANKING_ORDER_BY, @@ -244,7 +247,7 @@ def _config_references_custom_rules(config: RulesConfig) -> bool: def prepare_sql_rules( - *, inputs: CompileProjectInputs, executor: Executor, dialect: str + *, inputs: CompileProjectInputs, executor: Executor, dialect: str, stop: Event ) -> PreparedSqlLint | None: project_dir: Path | None = inputs.discovered_inputs.project_dir if project_dir is None or any(diagnostic.is_error for diagnostic in inputs.diagnostics): @@ -302,6 +305,7 @@ def prepare_sql_rules( for model in inputs.model_inputs if model.sql_expansion is not None }, + stop=stop, ), ) @@ -313,19 +317,24 @@ def _prepare_sql_lint( discovered_inputs: DiscoveredProjectInputs, static_declaration_scope: DeclarationScopeBuild | None, compiled_expansions: dict[Path, CompiledSqlExpansion], + stop: Event, ) -> PreparedSqlLintResult: - context: SqlExpansionContext = build_expansion_context( - project_dir=project_dir, - discovered_inputs=discovered_inputs, - static_declaration_scope=static_declaration_scope, - ) - result: LintRunResult = run_lint( - project_dir=project_dir, - config=config, - discovered_inputs=discovered_inputs, - compiled_expansions=compiled_expansions, - expansion_context=context, - ) + stop_token: Token[Event | None] = LintStopContext.active.set(stop) + try: + context: SqlExpansionContext = build_expansion_context( + project_dir=project_dir, + discovered_inputs=discovered_inputs, + static_declaration_scope=static_declaration_scope, + ) + result: LintRunResult = run_lint( + project_dir=project_dir, + config=config, + discovered_inputs=discovered_inputs, + compiled_expansions=compiled_expansions, + expansion_context=context, + ) + finally: + LintStopContext.active.reset(stop_token) return PreparedSqlLintResult(result=result, context=context) diff --git a/src/sqlbuild/rule_engine/classes/early_sql_lint.py b/src/sqlbuild/rule_engine/classes/early_sql_lint.py index e30eb4e87f..2da7272ec8 100644 --- a/src/sqlbuild/rule_engine/classes/early_sql_lint.py +++ b/src/sqlbuild/rule_engine/classes/early_sql_lint.py @@ -2,7 +2,9 @@ from __future__ import annotations +import time from concurrent.futures import ThreadPoolExecutor +from threading import Event from types import TracebackType from sqlbuild.compiler.compile.models import CompileProjectInputs @@ -15,6 +17,7 @@ def __init__(self, *, enabled: bool) -> None: self.enabled: bool = enabled self.preparation: PreparedSqlLint | None = None self.expansion_reuse: SqlExpansionReuse | None = None + self.stop_requested: Event = Event() self.executor: ThreadPoolExecutor = ThreadPoolExecutor( max_workers=1, thread_name_prefix="sqlbuild-lint" ) @@ -28,12 +31,15 @@ def __exit__( exception: BaseException | None, traceback: TracebackType | None, ) -> None: - self.executor.shutdown(wait=True, cancel_futures=True) + _ = self.stop() def start(self, *, inputs: CompileProjectInputs, dialect: str) -> None: if self.enabled: self.preparation = prepare_sql_rules( - inputs=inputs, executor=self.executor, dialect=dialect + inputs=inputs, + executor=self.executor, + dialect=dialect, + stop=self.stop_requested, ) self.expansion_reuse = ( None @@ -43,3 +49,11 @@ def start(self, *, inputs: CompileProjectInputs, dialect: str) -> None: declaration_scope=inputs.declaration_scope, ) ) + + def stop(self) -> int: + """Abandon unconsumed speculative work and return the milliseconds spent joining it.""" + + stop_start: float = time.monotonic() + self.stop_requested.set() + self.executor.shutdown(wait=True, cancel_futures=True) + return int((time.monotonic() - stop_start) * 1000) diff --git a/src/sqlbuild/rule_engine/main/_prepare_sql_rules.py b/src/sqlbuild/rule_engine/main/_prepare_sql_rules.py index ebdebfc73a..bac5280adc 100644 --- a/src/sqlbuild/rule_engine/main/_prepare_sql_rules.py +++ b/src/sqlbuild/rule_engine/main/_prepare_sql_rules.py @@ -3,6 +3,7 @@ from __future__ import annotations from concurrent.futures import Executor +from threading import Event from sqlbuild.compiler.compile.models import CompileProjectInputs from sqlbuild.rule_engine._helpers.run.rules import prepare_sql_rules as prepare @@ -10,8 +11,8 @@ def prepare_sql_rules( - *, inputs: CompileProjectInputs, executor: Executor, dialect: str + *, inputs: CompileProjectInputs, executor: Executor, dialect: str, stop: Event ) -> PreparedSqlLint | None: """Prepare configured SQL checks without publishing findings before compilation finishes.""" - return prepare(inputs=inputs, executor=executor, dialect=dialect) + return prepare(inputs=inputs, executor=executor, dialect=dialect, stop=stop) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py index a55b5a8880..a79d284bc6 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/build/test_compile_json_behavior.py @@ -70,6 +70,7 @@ def test_given_waffle_shop_when_running_compile_json_then_it_reports_offline_que "attachment_ms", "built_in_rules_ms", "custom_rules_ms", + "early_lint_wait_ms", "model_analysis_ms", "analysis_preparation_ms", "analysis_native_ms", diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 98a0dbe6ae..d6166840c1 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -539,3 +539,13 @@ class FormatDescriptionResolutionE2ECase: extra_files: tuple[tuple[str, str], ...] expected_returncode: int expected_faults: tuple[tuple[str, str], ...] + + +@dataclass(frozen=True) +class EarlyLintCompileTestCase: + """A rules-enabled project and the diagnostics compile must report for it.""" + + description: str + order_totals_sql: str + expected_exit_code: int + expected_diagnostics: tuple[tuple[str, str, int, int], ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py index d3e3b8b57c..40ad9bd6cd 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py @@ -2888,3 +2888,23 @@ def compile_inline_files( }, ) return run_sqb(project_dir=project_dir, command=_REQUIRED_DESCRIPTIONS_COMPILE) + + +def compile_json_payload(*, project_dir: Path) -> tuple[int, dict[str, Any]]: + """Run one JSON compile through the CLI and return its exit code and payload.""" + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile", "--json"), project_dir=project_dir + ) + return result.returncode, json.loads(result.stdout) + + +def diagnostic_location_keys(payload: dict[str, Any]) -> tuple[tuple[str, str, int, int], ...]: + """Sorted code, path, line, and column of every diagnostic in a compile payload.""" + + return tuple( + sorted( + (diagnostic["code"], diagnostic["path"], diagnostic["line"], diagnostic["column"]) + for diagnostic in payload["diagnostics"] + ) + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_early_lint.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_early_lint.py new file mode 100644 index 0000000000..e350906c26 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_early_lint.py @@ -0,0 +1,89 @@ +"""E2E tests for the speculative SQL lint started during compilation.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + EarlyLintCompileTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + compile_json_payload, + diagnostic_location_keys, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project + +_PROJECT_TOML: str = 'name = "commerce"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRSQL005"]\n' +_ORDERS_SQL: str = 'MODEL (description "Orders");\nSELECT 1 AS order_id, 2 AS amount\n' + + +@pytest.mark.parametrize( + "test_case", + ( + EarlyLintCompileTestCase( + description="graph error discards the speculative lint", + order_totals_sql=( + 'MODEL (description "Order totals");\n' + "WITH\n" + " unused_orders AS (SELECT 1 AS order_id),\n" + ' totals AS (SELECT order_id, missing_column FROM __ref("orders"))\n' + "SELECT order_id FROM totals\n" + ), + expected_exit_code=1, + expected_diagnostics=(("B002", "models/order_totals.sql", 4, 31),), + ), + EarlyLintCompileTestCase( + description="clean graph reports every lint finding at its authored line", + order_totals_sql=( + 'MODEL (description "Order totals");\n' + "WITH\n" + " unused_orders AS (SELECT 1 AS order_id),\n" + ' totals AS (SELECT order_id, amount FROM __ref("orders")),\n' + "\n" + " unused_amounts AS (\n" + " SELECT amount FROM totals\n" + " ),\n" + " unused_counts AS (SELECT COUNT(*) AS order_count FROM totals)\n" + "SELECT order_id, amount FROM totals\n" + ), + expected_exit_code=1, + expected_diagnostics=( + ("SQBRSQL005", "models/order_totals.sql", 3, 3), + ("SQBRSQL005", "models/order_totals.sql", 6, 3), + ("SQBRSQL005", "models/order_totals.sql", 9, 3), + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_rules_enabled_project_when_compiling_cold_and_warm_then_lint_wait_is_attributed( + test_case: EarlyLintCompileTestCase, + tmp_path: Path, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="commerce", + repo_files={ + "sqlbuild_project.toml": _PROJECT_TOML, + "models/orders.sql": _ORDERS_SQL, + "models/order_totals.sql": test_case.order_totals_sql, + }, + ) + + cold_exit: int + cold: dict[str, Any] + cold_exit, cold = compile_json_payload(project_dir=project_dir) + warm_exit: int + warm: dict[str, Any] + warm_exit, warm = compile_json_payload(project_dir=project_dir) + + assert (cold_exit, warm_exit) == (test_case.expected_exit_code, test_case.expected_exit_code) + assert diagnostic_location_keys(cold) == test_case.expected_diagnostics + assert diagnostic_location_keys(warm) == test_case.expected_diagnostics + for payload in (cold, warm): + wait_ms: object = payload["compile_timings"]["early_lint_wait_ms"] + assert isinstance(wait_ms, int) + assert wait_ms >= 0 diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index 80909ee47d..0ec88e2af8 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -5,7 +5,8 @@ import json from collections.abc import Callable from pathlib import Path -from typing import cast +from threading import Event +from typing import Any, cast import pytest from _pytest.capture import CaptureResult @@ -19,6 +20,8 @@ from sqlbuild.compiler.compile.models import DeclarationScopeBuild, LoadedMacro from sqlbuild.compiler.discovery.models import DiscoveredMacroFile, DiscoveredProjectInputs from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax +from sqlbuild.lint.classes.stop_context import LintStopContext +from sqlbuild.lint.models import LintRunResult from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( DynamicPivotRulesIntegrationTestCase, ExplicitContractOutputRuleIntegrationTestCase, @@ -30,6 +33,8 @@ ) from tests.integration.src.sqlbuild.cli.commands.main.helpers import compile_finding_keys +_EARLY_LINT_STOP_TIMEOUT_S: float = 30.0 + @pytest.mark.parametrize( "test_case", @@ -919,6 +924,56 @@ def counting_collect( assert collections == [None] +@pytest.mark.parametrize( + "test_case", + [RulePassIntegrationTestCase("graph error stops the unused early lint", 1, ("B002",))], + ids=lambda case: case.description, +) +def test_given_graph_error_when_compiling_then_early_lint_is_stopped_instead_of_awaited( + test_case: RulePassIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRSQL005"]\n', + encoding="utf-8", + ) + models_dir: Path = tmp_path / "models" + models_dir.mkdir() + (models_dir / "orders.sql").write_text( + 'MODEL (description "Orders");\nSELECT 1 AS order_id\n', encoding="utf-8" + ) + (models_dir / "order_totals.sql").write_text( + 'MODEL (description "Order totals");\n' + "WITH unused_orders AS (SELECT 1 AS order_id)\n" + 'SELECT order_id, missing_column FROM __ref("orders")\n', + encoding="utf-8", + ) + lint_started: Event = Event() + stop_observed: list[bool] = [] + original_run_lint: Callable[..., LintRunResult] = rules_module.run_lint + + def stop_awaiting_run_lint(**kwargs: Any) -> LintRunResult: + lint_started.set() + stop: Event | None = LintStopContext.active.get() + stop_observed.append(stop is not None and stop.wait(timeout=_EARLY_LINT_STOP_TIMEOUT_S)) + return original_run_lint(**kwargs) + + monkeypatch.setattr(rules_module, "run_lint", stop_awaiting_run_lint) + + exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json"]) + + payload: dict[str, object] = json.loads(capsys.readouterr().out) + diagnostics: list[dict[str, object]] = cast(list[dict[str, object]], payload["diagnostics"]) + assert exit_code == test_case.expected_exit_code + assert tuple(diagnostic["code"] for diagnostic in diagnostics) == ( + test_case.expected_diagnostics + ) + assert lint_started.is_set() + assert stop_observed == [True] + + @pytest.mark.parametrize( "test_case", [RulesIntegrationTestCase("custom host runs once and is skipped when warm", 1, "XSQBRARCH001")], diff --git a/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py b/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py index 584ab15bb9..3b4427709d 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py @@ -204,3 +204,14 @@ class HeaderAuditWrapTestCase: line_width: int header: str expected_header: str + + +@dataclass(frozen=True) +class ManyViolationLocationTestCase: + """Multi-line files with many native findings at independently known positions.""" + + description: str + prefix: str + unused_cte_count: int + file_count: int + expected_code: str diff --git a/tests/unit/src/sqlbuild/lint/_helpers/helpers.py b/tests/unit/src/sqlbuild/lint/_helpers/helpers.py index 4553ef7278..9863d1f4fb 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/helpers.py @@ -26,3 +26,41 @@ def lint_bodies_for(*, file_path: Path, contents: str) -> tuple[LintBody, ...]: ) ) return tuple(bodies) + + +def unused_cte_orders_file( + *, tmp_path: Path, prefix: str, file_index: int, unused_cte_count: int +) -> tuple[Path, str, LintBody]: + """Build one multi-line file, offset by its index, whose body declares many unused CTEs.""" + + target: Path = tmp_path / f"orders_{file_index}.sql" + sql: str = ( + "WITH\n" + + "".join( + f" unused_{index:03d} AS (\n SELECT {index} AS order_id\n ),\n" + for index in range(unused_cte_count) + ) + + " final AS (SELECT 1 AS order_id)\nSELECT order_id\nFROM final\n" + ) + header: str = prefix + "-- order history\n" * file_index + contents: str = header + sql + body: LintBody = LintBody( + file_path=target, + body_start=len(header), + body_end=len(contents), + lint_text=sql, + passes=(), + ) + return target, contents, body + + +def authored_positions(*, contents: str, needles: tuple[str, ...]) -> list[tuple[int, int]]: + """One-based line and column of each needle's first occurrence.""" + + positions: list[tuple[int, int]] = [] + for needle in needles: + offset: int = contents.index(needle) + positions.append( + (contents.count("\n", 0, offset) + 1, offset - contents.rfind("\n", 0, offset)) + ) + return positions diff --git a/tests/unit/src/sqlbuild/lint/_helpers/test_native_sql.py b/tests/unit/src/sqlbuild/lint/_helpers/test_native_sql.py index 431f98746f..0b46b6c6b4 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/test_native_sql.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/test_native_sql.py @@ -14,6 +14,7 @@ from tests.unit.src.sqlbuild.lint._helpers._test_types import ( GeneratedRangeFallbackTestCase, InvalidNativeSqlResponseTestCase, + ManyViolationLocationTestCase, NativeParseIsolationTestCase, NativeSqlDependencyCacheTestCase, NativeSqlFindingTestCase, @@ -21,6 +22,10 @@ NativeSqlReuseTestCase, ReservedCteLintTestCase, ) +from tests.unit.src.sqlbuild.lint._helpers.helpers import ( + authored_positions, + unused_cte_orders_file, +) @pytest.mark.parametrize( @@ -494,3 +499,49 @@ def test_given_diagnostic_crossing_generated_sql_when_mapping_then_range_falls_b violation: LintViolation = result[target][0] assert (violation.line, violation.column) == test_case.expected_position assert (violation.end_line, violation.end_column) == (None, None) + + +@pytest.mark.parametrize( + "test_case", + [ + ManyViolationLocationTestCase( + description="many unused CTEs across several multi-line files", + prefix="-- orders header\n-- second header line\n\n", + unused_cte_count=40, + file_count=3, + expected_code="SQBRSQL005", + ), + ], + ids=lambda case: case.description, +) +def test_given_many_violations_per_file_when_linting_then_locations_match_authored_lines( + test_case: ManyViolationLocationTestCase, + tmp_path: Path, +) -> None: + files: tuple[tuple[Path, str, LintBody], ...] = tuple( + unused_cte_orders_file( + tmp_path=tmp_path, + prefix=test_case.prefix, + file_index=file_index, + unused_cte_count=test_case.unused_cte_count, + ) + for file_index in range(test_case.file_count) + ) + contents_by_path: dict[Path, str] = {target: contents for target, contents, _ in files} + bodies: tuple[LintBody, ...] = tuple(body for _, _, body in files) + config: LintConfig = LintConfig( + dialect="duckdb", enabled_native_rules=(test_case.expected_code,) + ) + + result: dict[Path, tuple[LintViolation, ...]] = native_sql.run_native_sql_lint( + bodies=bodies, contents_by_path=contents_by_path, config=config + ) + + for target, contents in contents_by_path.items(): + assert {violation.code for violation in result[target]} == {test_case.expected_code} + assert sorted((violation.line, violation.column) for violation in result[target]) == ( + authored_positions( + contents=contents, + needles=tuple(f"unused_{index:03d}" for index in range(test_case.unused_cte_count)), + ) + )