From 9e6aca048a9ac6412c1f3fe36836423e399a1e5f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:25:06 +0100 Subject: [PATCH] perf(compile): stop the unused early SQL lint and map lint lines once per file Signal the speculative lint thread to stop as soon as the compile knows its result will not be consumed (graph errors or end of analysis), checking the stop between files and bodies instead of joining a full lint run. Report any remaining join as compile_timings.early_lint_wait_ms. Build each file's newline table once per native lint pass instead of once per violation. --- .../cli/commands/_helpers/compile/pipeline.py | 6 +- .../cli/commands/main/project/_compile.py | 1 + src/sqlbuild/cli/compile/models.py | 1 + src/sqlbuild/lint/_helpers/native_sql.py | 45 +++++++--- src/sqlbuild/lint/classes/__init__.py | 1 + src/sqlbuild/lint/classes/stop_context.py | 11 +++ src/sqlbuild/lint/exceptions.py | 4 + src/sqlbuild/lint/main/run_lint.py | 3 +- .../rule_engine/_helpers/run/rules.py | 35 +++++--- .../rule_engine/classes/early_sql_lint.py | 18 +++- .../rule_engine/main/_prepare_sql_rules.py | 5 +- .../main/build/test_compile_json_behavior.py | 1 + .../cli/commands/main/compile/_test_types.py | 10 +++ .../cli/commands/main/compile/helpers.py | 20 +++++ .../main/compile/test_compile_early_lint.py | 89 +++++++++++++++++++ .../sqlbuild/cli/commands/main/test_rules.py | 57 +++++++++++- .../src/sqlbuild/lint/_helpers/_test_types.py | 11 +++ .../src/sqlbuild/lint/_helpers/helpers.py | 38 ++++++++ .../sqlbuild/lint/_helpers/test_native_sql.py | 51 +++++++++++ 19 files changed, 375 insertions(+), 32 deletions(-) create mode 100644 src/sqlbuild/lint/classes/__init__.py create mode 100644 src/sqlbuild/lint/classes/stop_context.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_early_lint.py 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)), + ) + )