diff --git a/Makefile b/Makefile index 677f6f4f16..1845611f69 100644 --- a/Makefile +++ b/Makefile @@ -161,6 +161,7 @@ E2E_DUCKDB_CLI_PATHS := \ tests/e2e/src/sqlbuild/cli/commands/main/databricks \ tests/e2e/src/sqlbuild/cli/commands/main/dbt \ tests/e2e/src/sqlbuild/cli/commands/main/debug \ + tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_cache.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_example_projects.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_rule_fixes.py \ tests/e2e/src/sqlbuild/cli/commands/main/format/test_token_preservation.py \ diff --git a/crates/sqlbuild-cli/src/argv/command_table.json b/crates/sqlbuild-cli/src/argv/command_table.json index fd2f44b5d8..bd81c7e6ef 100644 --- a/crates/sqlbuild-cli/src/argv/command_table.json +++ b/crates/sqlbuild-cli/src/argv/command_table.json @@ -2385,6 +2385,18 @@ "help": "only remove redundant typed-null SQL test fixture columns", "group": 1 }, + { + "kind": "store_true", + "options": [ + "--no-cache" + ], + "dest": "no_cache", + "nargs": 0, + "const": true, + "default": false, + "help": "format every file instead of skipping files a previous run found clean", + "group": 1 + }, { "kind": "store", "dest": "format_paths", diff --git a/crates/sqlbuild-cli/src/dispatch/constants.rs b/crates/sqlbuild-cli/src/dispatch/constants.rs index e1240666b5..b32def4996 100644 --- a/crates/sqlbuild-cli/src/dispatch/constants.rs +++ b/crates/sqlbuild-cli/src/dispatch/constants.rs @@ -629,7 +629,7 @@ pub(crate) const SKILLS_ROUTE: Route = routed( }, ); -const FORMAT_ARGUMENTS: [Field; 9] = [ +const FORMAT_ARGUMENTS: [Field; 10] = [ PROJECT_DIR, field("select", Source::FormatSelect), EXCLUDE, @@ -637,6 +637,7 @@ const FORMAT_ARGUMENTS: [Field; 9] = [ attr("diff", "format_diff"), attr("fixtures_only", "format_fixtures_only"), attr("fix", "format_fix"), + same("no_cache"), JSON_OUTPUT, NO_COLOR, ]; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/stage_work.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/stage_work.rs index cc89abc6c2..88d543f7fa 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/stage_work.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/stage_work.rs @@ -23,6 +23,8 @@ pub(crate) const LINEAGE_GRAPH_BUILDS: &str = "lineage_graph_builds"; pub(crate) const PROJECT_GRAPHS: &str = "build_project_graph"; pub(crate) const FORMAT_LINT_BATCHES: &str = "format_lint_batches"; pub(crate) const FORMAT_LINT_FILES: &str = "format_lint_files"; +pub(crate) const FORMAT_CACHE_HITS: &str = "format_cache_hits"; +pub(crate) const FORMAT_CACHE_MISSES: &str = "format_cache_misses"; pub(crate) const SQL_TEST_ARTIFACT_PLANS: &str = "plan_compiled_sql_test_artifacts"; pub(crate) const SQL_TEST_MODEL_CHAINS: &str = "sql_test_model_chain_names_by_key"; pub(crate) const BACKGROUND_SQL_TEST_PLANS: &str = "background_sql_test_plans"; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/cache.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/cache.rs new file mode 100644 index 0000000000..a4c6ae3c1f --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/cache.rs @@ -0,0 +1,84 @@ +//! Files already formatted and fault-free, kept in the shared native store to skip next time. + +use std::path::{Path, PathBuf}; + +use pyo3::exceptions::PyOSError; +use pyo3::prelude::{PyResult, Python}; +use sqlbuild_cache::digest::main::content_digest::content_digest; +use sqlbuild_cache::digest::main::hex_digest::hex_digest; +use sqlbuild_cache::digest::types::ContentDigest; +use sqlbuild_cache::store::models::NativeStore; + +use crate::bindings::_helpers::cache::native_store::{ + current_code_identity, open_store, save_store, +}; +use crate::bindings::_helpers::compile_command::constants::TARGET_DIRECTORY_NAME; +use crate::bindings::_helpers::frontier::constants::{ + CACHE_DIRECTORY_NAME, COMPILER_CACHE_DIRECTORY_NAME, NATIVE_CACHE_NAMESPACE_SUFFIX, +}; + +/// The native store kind holding clean format results. +const FORMAT_RESULT_STORE_KIND: &str = "format-results"; +/// The store file below the compile cache folder. +pub(crate) const FORMAT_RESULT_STORE_FILE_NAME: &str = "format-results.bin"; +/// Bumped whenever what a clean result means changes; older stores open empty. +const FORMAT_RESULT_STORE_VERSION: &str = "format-result-store-v1"; +/// The one value stored: formatting left the file unchanged and reported nothing about it. +const CLEAN: &[u8] = b"clean"; + +/// One run's view of the stored clean results, keyed by file, text and effective settings. +pub(crate) struct FormatCache { + store: NativeStore, + path: PathBuf, + settings: String, +} + +impl FormatCache { + /// The project's store for this SQLBuild build, or `None` when it cannot be read. + pub(crate) fn open( + py: Python<'_>, + project_dir: &Path, + settings: String, + ) -> PyResult> { + let path: PathBuf = project_dir + .join(TARGET_DIRECTORY_NAME) + .join(CACHE_DIRECTORY_NAME) + .join(format!( + "{COMPILER_CACHE_DIRECTORY_NAME}{NATIVE_CACHE_NAMESPACE_SUFFIX}" + )) + .join(FORMAT_RESULT_STORE_FILE_NAME); + let environment: String = hex_digest(&content_digest(&[ + FORMAT_RESULT_STORE_VERSION, + ¤t_code_identity(py)?, + ])); + match open_store(py, &path, FORMAT_RESULT_STORE_KIND, &environment) { + Ok(store) => Ok(Some(Self { + store, + path, + settings, + })), + Err(error) if error.is_instance_of::(py) => Ok(None), + Err(error) => Err(error), + } + } + + /// The key of one file: its project-relative path, its exact text and the run's settings. + pub(crate) fn key(&self, relative_path: &str, contents: &str) -> ContentDigest { + content_digest(&[relative_path, contents, &self.settings]) + } + + /// Whether a previous run found the file under `key` clean. + pub(crate) fn is_clean(&mut self, key: &ContentDigest) -> bool { + self.store.get(key) == Some(CLEAN) + } + + /// Remember that the file under `key` formatted cleanly. + pub(crate) fn record_clean(&mut self, key: ContentDigest) { + self.store.put(key, CLEAN.to_vec()); + } + + /// Save newly recorded results; a store that cannot be written is left as it was. + pub(crate) fn save(&self, py: Python<'_>) { + let _ = save_store(py, &self.store, &self.path, &[]); + } +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs index 46a9cc1923..93945309cb 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs @@ -41,6 +41,7 @@ struct FormatFlags { fix: bool, json_output: bool, use_color: bool, + no_cache: bool, } /// The adapter, selected files and discovered inputs a run formats with. @@ -227,6 +228,7 @@ fn run_command( fixtures_only: flags.fixtures_only, write: !(flags.check || flags.diff), fix: flags.fix, + cache: !(flags.no_cache || flags.fix), }; let (outcome, status, elapsed) = format_with_status(py, &request, flags.use_color)?; let root: PathBuf = project_dir.extract()?; @@ -279,6 +281,7 @@ pub(crate) fn run_format_command<'py>( "fix", "json_output", "no_color", + "no_cache", ], ), )?; @@ -296,6 +299,7 @@ pub(crate) fn run_format_command<'py>( fix: arguments.flag("fix")?, json_output: arguments.flag("json_output")?, use_color, + no_cache: arguments.flag("no_cache")?, }; let select: Vec = string_tuple(&arguments.value("select")?)?; let exclude: Vec = string_tuple(&arguments.value("exclude")?)?; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs index 0b4e817f02..19e5d730ae 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs @@ -1,4 +1,4 @@ -//! Authored SQL files as `{Path: str}` dictionaries, and writing formatted files back. +//! Writing formatted files back atomically, in the authored newline convention. use std::fs::{self, File, OpenOptions}; use std::io::Write; @@ -6,9 +6,11 @@ use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use pyo3::exceptions::PyOSError; -use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; -use pyo3::types::{PyDict, PyDictMethods}; -use pyo3::{pyfunction, wrap_pyfunction}; +use pyo3::prelude::{Bound, PyAny, PyModule, PyModuleMethods, PyResult, Python}; +use pyo3::{IntoPyObject, pyfunction, wrap_pyfunction}; +use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; + +use crate::bindings::_helpers::format_command::stages::on_format_workers; const LINE_FEED: &str = "\n"; const CARRIAGE_RETURN_LINE_FEED: &str = "\r\n"; @@ -16,36 +18,20 @@ const CARRIAGE_RETURN_LINE_FEED: &str = "\r\n"; /// One file the formatter would change: its path and its text before and after. pub(crate) struct FormatChange<'py> { pub(crate) path: Bound<'py, PyAny>, + /// The run's index of the changed file. + pub(crate) entry: usize, pub(crate) before: String, pub(crate) after: String, } -/// The entries of a `{Path: value}` dictionary, sorted as Python sorts paths. -pub(crate) fn sorted_items<'py>( - dictionary: &Bound<'py, PyDict>, -) -> PyResult, Bound<'py, PyAny>)>> { - let mut items: Vec<(PathBuf, Bound<'py, PyAny>, Bound<'py, PyAny>)> = dictionary - .iter() - .map(|(key, value)| Ok((key.extract::()?, key, value))) - .collect::>()?; - items.sort_by(|left, right| left.0.cmp(&right.0)); - Ok(items - .into_iter() - .map(|(_, key, value)| (key, value)) - .collect()) -} - -/// `{path: overrides.get(path, contents) for path, contents in files.items()}`. -pub(crate) fn overlaid<'py>( - files: &Bound<'py, PyDict>, - overrides: &Bound<'py, PyDict>, -) -> PyResult> { - let merged: Bound<'py, PyDict> = PyDict::new(files.py()); - for (path, contents) in files.iter() { - let value: Bound<'py, PyAny> = overrides.get_item(&path)?.unwrap_or(contents); - merged.set_item(path, value)?; - } - Ok(merged) +/// One file a format run would leave in its final text. +pub(crate) struct PendingWrite<'t> { + pub(crate) entry: usize, + pub(crate) path: &'t Path, + /// The text read, or empty for a file the run did not collect. + pub(crate) authored: &'t str, + pub(crate) contents: &'t str, + pub(crate) newline: &'static str, } /// Generated contents in the authored newline convention. @@ -68,7 +54,8 @@ fn temporary_path(path: &Path) -> PathBuf { path.with_file_name(format!(".{name}.{}{nanos:x}", std::process::id())) } -fn write_replacing(path: &Path, temporary: &Path, contents: &str) -> std::io::Result<()> { +/// Write and sync `contents` to a new `temporary` beside `path`, with its permissions. +fn stage_replacement(path: &Path, temporary: &Path, contents: &str) -> std::io::Result<()> { let permissions: fs::Permissions = fs::metadata(path)?.permissions(); let mut handle: File = OpenOptions::new() .write(true) @@ -76,16 +63,45 @@ fn write_replacing(path: &Path, temporary: &Path, contents: &str) -> std::io::Re .open(temporary)?; handle.set_permissions(permissions)?; handle.write_all(contents.as_bytes())?; - handle.sync_all()?; - fs::rename(temporary, path) + handle.sync_all() +} + +fn write_failure(path: &Path, error: &std::io::Error) -> String { + format!("{}: {error}", path.display()) } /// Replace a text file atomically, keeping its permissions. fn write_atomically(path: &Path, contents: &str) -> PyResult<()> { let temporary: PathBuf = temporary_path(path); - let written: std::io::Result<()> = write_replacing(path, &temporary, contents); + let written: std::io::Result<()> = + stage_replacement(path, &temporary, contents).and_then(|()| fs::rename(&temporary, path)); let _ = fs::remove_file(&temporary); - written.map_err(|error| PyOSError::new_err(format!("{}: {error}", path.display()))) + written.map_err(|error| PyOSError::new_err(write_failure(path, &error))) +} + +/// Replace `files` in order after staging all in parallel; the earliest failure stops the rest. +fn replace_in_order(files: &[(&Path, &str)]) -> Result<(), String> { + let temporaries: Vec = files.iter().map(|(path, _)| temporary_path(path)).collect(); + let staged: Vec> = on_format_workers(|| { + Ok(files + .par_iter() + .zip(temporaries.par_iter()) + .map(|((path, contents), temporary)| stage_replacement(path, temporary, contents)) + .collect()) + })?; + let mut outcome: Result<(), String> = Ok(()); + let mut renamed: usize = 0; + for (((path, _), temporary), staging) in files.iter().zip(&temporaries).zip(staged) { + if let Err(error) = staging.and_then(|()| fs::rename(temporary, path)) { + outcome = Err(write_failure(path, &error)); + break; + } + renamed += 1; + } + for temporary in &temporaries[renamed..] { + let _ = fs::remove_file(temporary); + } + outcome } /// `write_atomically(*, path, contents)`: replace a text file atomically, keeping its mode. @@ -99,45 +115,37 @@ pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(write_file_atomically, module)?) } -/// The changed files in path order, written in place when `write` is set. +/// The changed files of `writes` in order, replaced in that order when `write` is set. pub(crate) fn persist_changes<'py>( - (files, updated): (&Bound<'py, PyDict>, &Bound<'py, PyDict>), - newlines: &Bound<'py, PyDict>, + py: Python<'py>, + writes: &[PendingWrite<'_>], write: bool, ) -> PyResult>> { - let mut changes: Vec> = Vec::new(); - for (path, contents) in sorted_items(updated)? { - let newline: String = newlines - .get_item(&path)? - .map_or_else(|| Ok(LINE_FEED.to_owned()), |value| value.extract())?; - let authored: String = files - .get_item(&path)? - .map_or_else(|| Ok(String::new()), |value| value.extract())?; - let after: String = with_newline_style(&contents.extract::()?, &newline); - let before: String = with_newline_style(&authored, &newline); - if after == before { - continue; - } - if write { - write_atomically(&path.extract::()?, &after)?; - } - changes.push(FormatChange { - path, - before, - after, - }); - } - Ok(changes) -} - -/// `{Path: contents}` of the collected files, in collection order. -pub(crate) fn file_dictionary<'py>( - py: Python<'py>, - files: Vec<(PathBuf, String)>, -) -> PyResult> { - let dictionary: Bound<'py, PyDict> = PyDict::new(py); - for (path, contents) in files { - dictionary.set_item(path, contents)?; + let changed: Vec<(&PendingWrite<'_>, String, String)> = writes + .iter() + .filter_map(|pending| { + let after: String = with_newline_style(pending.contents, pending.newline); + let before: String = with_newline_style(pending.authored, pending.newline); + (after != before).then_some((pending, before, after)) + }) + .collect(); + if write { + let files: Vec<(&Path, &str)> = changed + .iter() + .map(|(pending, _, after)| (pending.path, after.as_str())) + .collect(); + py.detach(|| replace_in_order(&files)) + .map_err(PyOSError::new_err)?; } - Ok(dictionary) + changed + .into_iter() + .map(|(pending, before, after)| { + Ok(FormatChange { + path: pending.path.into_pyobject(py)?.into_any(), + entry: pending.entry, + before, + after, + }) + }) + .collect() } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/mod.rs index 7404b5ea9d..8bfb1b8f28 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod cache; pub(crate) mod command; pub(crate) mod config; pub(crate) mod files; @@ -5,3 +6,4 @@ pub(crate) mod registration; pub(crate) mod report; pub(crate) mod run; pub(crate) mod selection; +pub(crate) mod stages; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/registration.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/registration.rs index 1b00100a44..28114d91b5 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/registration.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/registration.rs @@ -1,10 +1,15 @@ //! Register the native `sqb format` command and its atomic file writer. -use pyo3::prelude::{Bound, PyModule, PyResult}; +use pyo3::prelude::{Bound, PyModule, PyModuleMethods, PyResult}; +use crate::bindings::_helpers::format_command::cache::FORMAT_RESULT_STORE_FILE_NAME; use crate::bindings::_helpers::format_command::{command, files}; pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { command::register(module)?; - files::register(module) + files::register(module)?; + module.add( + "FORMAT_RESULT_STORE_FILE_NAME", + FORMAT_RESULT_STORE_FILE_NAME, + ) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/report.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/report.rs index 037dab8908..0ea13e5f02 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/report.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/report.rs @@ -1,10 +1,9 @@ //! A format run's Python violations and Rule fix reports as plain report data. -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashMap}; use std::path::{Path, PathBuf}; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult}; -use pyo3::types::{PyDict, PyDictMethods}; use sqlbuild_core::format_command::models::{LintFinding, LintReport, RuleFixLine}; use sqlbuild_core::text::main::python_splitlines::python_splitlines; @@ -28,25 +27,24 @@ fn display_path(path: &Path, root: &Path) -> String { } fn source_line( - contents: &Bound<'_, PyDict>, + contents: &HashMap, path: &Bound<'_, PyAny>, line: i64, ) -> PyResult> { - let Some(source) = contents.get_item(path)? else { + let Some(source) = contents.get(&path.extract::()?) else { return Ok(None); }; - let source: String = source.extract()?; if line < 1 { return Ok(None); } - let lines: Vec<&str> = python_splitlines(&source); + let lines: Vec<&str> = python_splitlines(source); let index: usize = usize::try_from(line - 1).unwrap_or(usize::MAX); Ok(lines.get(index).map(|line| (*line).to_owned())) } fn finding( violation: &Bound<'_, PyAny>, - contents: &Bound<'_, PyDict>, + contents: &HashMap, root: &Path, ) -> PyResult { let path: Bound<'_, PyAny> = native_field(violation, "file_path")?; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs index 75c4fc9947..f6b68e23ce 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs @@ -1,36 +1,48 @@ //! Format every authored SQL file and collect the violations that remain. -use std::collections::BTreeSet; -use std::path::PathBuf; +use std::collections::{BTreeSet, HashMap}; +use std::fs; +use std::path::{Path, PathBuf}; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; -use pyo3::types::{PyBool, PyDict, PyDictMethods, PyList, PyListMethods}; +use pyo3::types::{PyBool, PyDict, PyDictMethods}; +use pyo3::{IntoPyObject, IntoPyObjectExt}; +use sqlbuild_cache::digest::types::ContentDigest; use sqlbuild_core::format_command::main::collect_lint_files::collect_lint_files; +use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; +use sqlbuild_rules::lint_text::main::format_sql_bodies::format_sql_bodies; +use sqlbuild_rules::lint_text::main::reject_inline_suppressions::reject_inline_suppressions; +use sqlbuild_rules::lint_text::models::{ + FormatFault, FormatFile, FormattedBodies, SuppressionFinding, +}; use crate::bindings::_helpers::boundary::panics::value_error; use crate::bindings::_helpers::boundary::python_calls::keywords; use crate::bindings::_helpers::compile_driver::stage_work::{ - FORMAT_LINT_BATCHES, FORMAT_LINT_FILES, count_stage, + FORMAT_CACHE_HITS, FORMAT_CACHE_MISSES, FORMAT_LINT_BATCHES, FORMAT_LINT_FILES, count_stage, }; use crate::bindings::_helpers::discovery::description_inputs::externally_described_models; +use crate::bindings::_helpers::format_command::cache::FormatCache; use crate::bindings::_helpers::format_command::files::{ - FormatChange, file_dictionary, overlaid, persist_changes, sorted_items, + FormatChange, PendingWrite, persist_changes, +}; +use crate::bindings::_helpers::format_command::stages::{ + each_file, format_header_text, lint_header_text, on_format_workers, parse_faults, }; use crate::bindings::_helpers::records::construct::error_class; use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::records::ids::ErrorId; -use crate::bindings::_helpers::rules::dsl_headers::format_native_headers; -use crate::bindings::_helpers::rules::dsl_headers::reject_unparseable_header_rewrites; -use crate::bindings::_helpers::rules::dsl_headers::safe_format_files; -use crate::bindings::_helpers::rules::dsl_headers::{header_violations, rules_config}; +use crate::bindings::_helpers::rules::dsl_headers::rules_config; use crate::bindings::_helpers::rules::fixture_formatting::format_redundant_fixture_nulls; -use crate::bindings::_helpers::rules::lint_pipeline::{LintViolationRow, ViolationClasses}; -use crate::bindings::_helpers::rules::lint_text::format_lint_sql_bodies; -use crate::bindings::_helpers::rules::lint_text::reject_lint_suppressions; -use sqlbuild_rules::dsl_headers::main::scan_headers::scan_headers; -use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; +use crate::bindings::_helpers::rules::lint_pipeline::{ + LintViolationRow, ViolationClasses, project_relative_path, +}; +use crate::bindings::_helpers::rules::lint_text::{relative_parts, unsafe_fault}; +use crate::bindings::types::CompilerDetach; const DESCRIPTION_PRESENT: &str = "description-present"; +const LINE_FEED: &str = "\n"; +const CARRIAGE_RETURN_LINE_FEED: &str = "\r\n"; /// What one format run formats and how. pub(crate) struct FormatRequest<'a, 'py> { @@ -42,6 +54,8 @@ pub(crate) struct FormatRequest<'a, 'py> { pub(crate) fixtures_only: bool, pub(crate) write: bool, pub(crate) fix: bool, + /// Whether files a previous run found clean are skipped, and clean files remembered. + pub(crate) cache: bool, } /// The outcome of one format run, with Python violations and Rule fix reports. @@ -52,127 +66,315 @@ pub(crate) struct FormatOutcome<'py> { pub(crate) rule_fixes: Vec>, /// Paths whose formatting was declined, so their applied fixes were not kept. pub(crate) declined: BTreeSet, - pub(crate) final_contents: Bound<'py, PyDict>, + /// The final text of every collected file a violation may quote. + pub(crate) final_contents: HashMap, } -/// Fixture-null rewrites proven safe by the contracts, when fixture inputs are known. -fn fixture_autofix<'py>( - request: &FormatRequest<'_, 'py>, - fixture_inputs: Option<&(Bound<'py, PyAny>, Bound<'py, PyAny>)>, - files: &Bound<'py, PyDict>, -) -> PyResult> { - let py: Python<'py> = files.py(); - let Some((discovered, syntax)) = fixture_inputs else { - return Ok(PyDict::new(py)); - }; - Ok(format_redundant_fixture_nulls( - &files.clone().into_any().cast_into::()?, - &request.project_dir.clone(), - &discovered.clone(), - &syntax.clone(), - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))? - .downcast_into::()?) +/// One file the run formats: where it is, the text read, and whether it was collected. +struct Entry { + path: PathBuf, + original: String, + /// False for a file only Rule fixes introduced: it is written, but neither linted nor shown. + collected: bool, } -/// SQL bodies of `files` formatted natively: changed texts and declined-body faults. -fn format_bodies<'py>( - request: &FormatRequest<'_, 'py>, - files: &Bound<'py, PyDict>, -) -> PyResult<(Bound<'py, PyDict>, Bound<'py, PyAny>)> { - let py: Python<'py> = files.py(); - let items: Vec<(Bound<'py, PyAny>, String)> = sorted_items(files)? - .into_iter() - .map(|(path, contents)| Ok((path, contents.extract::()?))) - .collect::>()?; - count_stage(py, FORMAT_LINT_BATCHES, 1)?; - count_stage(py, FORMAT_LINT_FILES, items.len() as u64)?; - let result: Bound<'py, PyAny> = format_lint_sql_bodies( - py, - items, - request.project_dir.extract::()?, - request.config, - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; - Ok(( - result.get_item(0)?.downcast_into::()?, - result.get_item(1)?, - )) +/// A violation, kept as native data until the report needs Python records. +enum Finding<'py> { + Header(usize, HeaderViolation), + Unsafe(usize, FormatFault), + Suppression(usize, SuppressionFinding), + Python(Bound<'py, PyAny>), +} + +/// Rule fix reports and the faults of fixes that could not be kept. +type RuleFixOutcome<'py> = (Vec>, Vec>); + +/// Changed texts by entry, and the findings of declined bodies. +type FormattedTexts<'py> = (Vec<(usize, String)>, Vec>); + +/// Rewritten texts by entry, in the order each entry was first rewritten. +#[derive(Default)] +struct Rewrites { + texts: HashMap, + order: Vec, } -/// Contents after header, SQL body and fixture fixes, and the faults of declined rewrites. -fn apply_fixes<'py>( - request: &FormatRequest<'_, 'py>, - files: &Bound<'py, PyDict>, - fixture_inputs: Option<&(Bound<'py, PyAny>, Bound<'py, PyAny>)>, -) -> PyResult<(Bound<'py, PyDict>, Bound<'py, PyList>)> { - let py: Python<'py> = files.py(); - let updated: Bound<'py, PyDict> = fixture_autofix(request, fixture_inputs, files)?; - let faults: Bound<'py, PyList> = PyList::empty(py); - if request.fixtures_only { - return Ok((updated, faults)); +impl Rewrites { + fn set(&mut self, index: usize, text: String) { + if self.texts.insert(index, text).is_none() { + self.order.push(index); + } + } + + fn get(&self, index: usize) -> Option<&str> { + self.texts.get(&index).map(String::as_str) } - let current: Bound<'py, PyDict> = overlaid(files, &updated)?; - for (path, contents) in sorted_items(¤t)? { +} + +/// The rewrites a run kept, the faults that declined the others, and the lint of each rewrite. +struct Applied<'py> { + rewrites: Rewrites, + rejected: BTreeSet, + faults: Vec>, + rewrite_lints: HashMap>, +} + +/// The native state of one format run. +struct Run<'a, 'r, 'py> { + request: &'r FormatRequest<'a, 'py>, + py: Python<'py>, + directory: PathBuf, + config: HeaderRulesConfig, + entries: Vec, + index: HashMap, + /// The entries headers parse in, which Rule fixes and formatting rewrite. + safe: Vec, + /// Texts Rule fixes changed. + fixed: HashMap, +} + +fn native( + py: Python<'_>, + operation: impl FnOnce() -> Result + Send, +) -> PyResult { + py.compiler_detach(|| on_format_workers(operation)) + .map_err(value_error) +} + +impl<'py> Run<'_, '_, 'py> { + /// The text formatting starts from: the Rule-fixed text, else the text read. + fn base_text(&self, index: usize) -> &str { + self.fixed + .get(&index) + .map_or(self.entries[index].original.as_str(), String::as_str) + } + + fn text<'t>(&'t self, index: usize, rewrites: &'t Rewrites) -> &'t str { + rewrites.get(index).unwrap_or_else(|| self.base_text(index)) + } + + fn sorted(&self, indices: &[usize]) -> Vec { + let mut sorted: Vec = indices.to_vec(); + sorted.sort_by(|left, right| self.entries[*left].path.cmp(&self.entries[*right].path)); + sorted + } + + /// The entry of `path`, adding one for a path the run did not collect. + fn entry_of(&mut self, path: PathBuf) -> usize { + if let Some(index) = self.index.get(&path) { + return *index; + } + let index: usize = self.entries.len(); + let _ = self.index.insert(path.clone(), index); + self.entries.push(Entry { + path, + original: String::new(), + collected: false, + }); + index + } + + /// `{Path: text}` of the safe entries, as the Python formatting passes read them. + fn safe_dictionary(&self, rewrites: &Rewrites) -> PyResult> { + let dictionary: Bound<'py, PyDict> = PyDict::new(self.py); + for index in &self.safe { + dictionary.set_item(&self.entries[*index].path, self.text(*index, rewrites))?; + } + Ok(dictionary) + } + + /// Rule fixes planned in memory and verified by compilation: reports and faults. + fn plan_rule_fixes(&mut self) -> PyResult> { + let py: Python<'py> = self.py; + let request: &FormatRequest<'_, 'py> = self.request; + let none = || py.None().into_bound(py); let result: Bound<'py, PyAny> = - format_native_headers(&contents.clone(), &path.clone(), &request.config.clone()) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; - for fault in result.get_item(1)?.try_iter()? { - faults.append(fault?)?; + crate::bindings::_helpers::rules::fix_session::plan_rule_fixes( + py, + Some(keywords( + py, + &[ + ("project_dir", request.project_dir.clone()), + ( + "files", + self.safe_dictionary(&Rewrites::default())?.into_any(), + ), + ("config", request.config.clone()), + ("adapter", request.adapter.map_or_else(none, Clone::clone)), + ( + "discovered_inputs", + request.discovered.map_or_else(none, Clone::clone), + ), + ("enabled", PyBool::new(py, true).to_owned().into_any()), + ], + )?), + )?; + let mut safe: Vec = Vec::new(); + for (path, contents) in result.get_item(0)?.cast_into::()?.iter() { + let index: usize = self.entry_of(path.extract()?); + let text: String = contents.extract()?; + if text != self.entries[index].original || !self.entries[index].collected { + let _ = self.fixed.insert(index, text); + } + safe.push(index); } - let formatted: Bound<'py, PyAny> = result.get_item(0)?; - if !formatted.eq(&contents)? { - updated.set_item(path, formatted)?; + self.safe = safe; + Ok(( + result.get_item(1)?.try_iter()?.collect::>()?, + result.get_item(2)?.try_iter()?.collect::>()?, + )) + } + + /// Fixture-null rewrites proven safe by the contracts, by entry, in the order Python made them. + fn fixture_autofix( + &mut self, + (discovered, syntax): &(Bound<'py, PyAny>, Bound<'py, PyAny>), + rewrites: &Rewrites, + ) -> PyResult> { + let updated: Bound<'py, PyAny> = format_redundant_fixture_nulls( + &self.safe_dictionary(rewrites)?, + &self.request.project_dir.clone(), + &discovered.clone(), + &syntax.clone(), + )? + .into_bound_py_any(self.py)?; + let mut texts: Vec<(usize, String)> = Vec::new(); + for (path, contents) in updated.cast_into::()?.iter() { + let index: usize = self.entry_of(path.extract()?); + texts.push((index, contents.extract()?)); } + Ok(texts) } - let (native_formatted, native_faults) = format_bodies(request, &overlaid(files, &updated)?)?; - for fault in native_faults.try_iter()? { - faults.append(fault?)?; + + /// The SQL bodies of `items` formatted natively: changed texts and declined-body faults. + fn format_bodies(&self, items: &[(usize, &str)]) -> PyResult> { + count_stage(self.py, FORMAT_LINT_BATCHES, 1)?; + count_stage(self.py, FORMAT_LINT_FILES, items.len() as u64)?; + let inputs: Vec = items + .iter() + .map(|(index, contents)| FormatFile { + relative_parts: relative_parts(&self.entries[*index].path, &self.directory), + contents: (*contents).to_owned(), + }) + .collect(); + let dialect: String = native_field(self.request.config, "dialect")?.extract()?; + let line_width: usize = native_field(self.request.config, "line_width")?.extract()?; + let result: FormattedBodies = native(self.py, || { + format_sql_bodies(&inputs, &dialect, line_width).map_err(|error| error.to_string()) + })?; + Ok(( + result + .formatted + .into_iter() + .map(|(position, text)| (items[position].0, text)) + .collect(), + result + .faults + .into_iter() + .map(|fault| Finding::Unsafe(items[fault.file].0, fault)) + .collect(), + )) } - updated.update(native_formatted.as_mapping())?; - let fixture_formatted: Bound<'py, PyDict> = - fixture_autofix(request, fixture_inputs, &overlaid(files, &updated)?)?; - if !fixture_formatted.is_empty() { - let (final_native, final_faults) = format_bodies(request, &fixture_formatted)?; - for fault in final_faults.try_iter()? { - faults.append(fault?)?; + + /// Contents after header, SQL body and fixture fixes, and the faults of declined rewrites. + fn apply_fixes( + &mut self, + fixture_inputs: Option<&(Bound<'py, PyAny>, Bound<'py, PyAny>)>, + ) -> PyResult> { + let mut applied: Applied<'py> = Applied { + rewrites: Rewrites::default(), + rejected: BTreeSet::new(), + faults: Vec::new(), + rewrite_lints: HashMap::new(), + }; + if let Some(inputs) = fixture_inputs { + for (index, text) in self.fixture_autofix(inputs, &applied.rewrites)? { + applied.rewrites.set(index, text); + } + } + if self.request.fixtures_only { + return Ok(applied); + } + let sorted: Vec = self.sorted(&self.safe); + let config: HeaderRulesConfig = self.config; + let headers: Vec<(String, Vec)> = { + let texts: Vec<&str> = sorted + .iter() + .map(|index| self.text(*index, &applied.rewrites)) + .collect(); + native(self.py, || { + each_file(&texts, |text| format_header_text(text, config)) + })? + }; + for (index, (formatted, faults)) in sorted.iter().zip(headers) { + applied.faults.extend( + faults + .into_iter() + .map(|fault| Finding::Header(*index, fault)), + ); + if formatted != self.text(*index, &applied.rewrites) { + applied.rewrites.set(*index, formatted); + } + } + let (formatted, faults) = { + let items: Vec<(usize, &str)> = sorted + .iter() + .map(|index| (*index, self.text(*index, &applied.rewrites))) + .collect(); + self.format_bodies(&items)? + }; + applied.faults.extend(faults); + for (index, text) in formatted { + applied.rewrites.set(index, text); + } + if let Some(inputs) = fixture_inputs { + let fixtures: Vec<(usize, String)> = self.fixture_autofix(inputs, &applied.rewrites)?; + if !fixtures.is_empty() { + let mut items: Vec<(usize, &str)> = fixtures + .iter() + .map(|(index, text)| (*index, text.as_str())) + .collect(); + items.sort_by(|left, right| { + self.entries[left.0].path.cmp(&self.entries[right.0].path) + }); + let (formatted, faults) = self.format_bodies(&items)?; + applied.faults.extend(faults); + for (index, text) in fixtures.into_iter().chain(formatted) { + applied.rewrites.set(index, text); + } + } } - updated.update(fixture_formatted.as_mapping())?; - updated.update(final_native.as_mapping())?; + self.reject_unparseable_rewrites(applied) } - let result: Bound<'py, PyAny> = reject_unparseable_header_rewrites( - &updated.into_any().cast_into::()?, - &request.config.clone(), - &faults.into_any().cast_into::()?, - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; - Ok(( - result.get_item(0)?.downcast_into::()?, - result.get_item(1)?.downcast_into::()?, - )) -} -/// Formatter-owned header diagnostics of the final contents, without configured Rules. -fn lint_final_contents<'py>( - request: &FormatRequest<'_, 'py>, - files: &Bound<'py, PyDict>, - updated: &Bound<'py, PyDict>, -) -> PyResult>> { - let py: Python<'py> = files.py(); - let mut violations: Vec> = Vec::new(); - let header_rules: HeaderRulesConfig = rules_config(request.config)?; - let classes: ViolationClasses<'py> = ViolationClasses::new(py)?; - for (path, contents) in sorted_items(files)? { - let final_contents: String = updated.get_item(&path)?.unwrap_or(contents).extract()?; - let text: Vec = final_contents.chars().collect(); - let found: Vec = - header_violations(&text, &scan_headers(&text, false), header_rules) - .map_err(value_error)?; - for violation in found { - violations.push(classes.violation(&path, &LintViolationRow::header(0, violation))?); + /// Decline every rewrite whose header no longer parses, adding its header-parse faults. + fn reject_unparseable_rewrites(&self, mut applied: Applied<'py>) -> PyResult> { + let config: HeaderRulesConfig = self.config; + let order: Vec = applied.rewrites.order.clone(); + let lints: Vec> = { + let texts: Vec<&str> = order + .iter() + .map(|index| applied.rewrites.get(*index).unwrap_or_default()) + .collect(); + native(self.py, || { + each_file(&texts, |text| lint_header_text(text, config)) + })? + }; + for (index, violations) in order.into_iter().zip(lints) { + let faults: Vec = parse_faults(&violations); + if faults.is_empty() { + let _ = applied.rewrite_lints.insert(index, violations); + continue; + } + let _ = applied.rejected.insert(index); + applied.faults.extend( + faults + .into_iter() + .map(|fault| Finding::Header(index, fault)), + ); } + Ok(applied) } - Ok(violations) } fn code_is(violation: &Bound<'_, PyAny>, code: &str) -> PyResult { @@ -207,44 +409,186 @@ fn drop_resolved_description_faults<'py>( Ok(kept) } -/// Rule fixes planned in memory and verified by compilation, when `--fix` is set. -fn plan_rule_fixes<'py>( - request: &FormatRequest<'_, 'py>, - files: &Bound<'py, PyDict>, -) -> PyResult<(Bound<'py, PyDict>, Bound<'py, PyAny>, Bound<'py, PyAny>)> { - let py: Python<'py> = files.py(); - let result: Bound<'py, PyAny> = crate::bindings::_helpers::rules::fix_session::plan_rule_fixes( - py, - Some(keywords( - py, - &[ - ("project_dir", request.project_dir.clone()), - ("files", files.clone().into_any()), - ("config", request.config.clone()), - ( - "adapter", - request - .adapter - .map_or_else(|| py.None().into_bound(py), Clone::clone), - ), - ( - "discovered_inputs", - request - .discovered - .map_or_else(|| py.None().into_bound(py), Clone::clone), - ), - ( - "enabled", - PyBool::new(py, request.fix).to_owned().into_any(), - ), - ], - )?), - )?; - Ok(( - result.get_item(0)?.downcast_into::()?, - result.get_item(1)?, - result.get_item(2)?, - )) +/// Python `LintViolation` records of `findings`, in order, sharing one path object per file. +fn violation_records<'py>( + py: Python<'py>, + entries: &[Entry], + findings: Vec>, +) -> PyResult>> { + let classes: ViolationClasses<'py> = ViolationClasses::new(py)?; + let mut paths: HashMap> = HashMap::new(); + let mut path = |index: usize| -> PyResult> { + if let Some(path) = paths.get(&index) { + return Ok(path.clone()); + } + let path: Bound<'py, PyAny> = entries[index].path.clone().into_pyobject(py)?.into_any(); + let _ = paths.insert(index, path.clone()); + Ok(path) + }; + findings + .into_iter() + .map(|finding| match finding { + Finding::Header(index, violation) => { + classes.violation(&path(index)?, &LintViolationRow::header(0, violation)) + } + Finding::Unsafe(index, fault) => unsafe_fault(&path(index)?, &fault), + Finding::Suppression(index, finding) => { + classes.violation(&path(index)?, &LintViolationRow::suppression(finding)) + } + Finding::Python(violation) => Ok(violation), + }) + .collect() +} + +/// The path a fault declines the formatting of. +fn fault_path(entries: &[Entry], fault: &Finding<'_>) -> PyResult { + match fault { + Finding::Header(index, _) | Finding::Unsafe(index, _) | Finding::Suppression(index, _) => { + Ok(entries[*index].path.clone()) + } + Finding::Python(violation) => native_field(violation, "file_path")?.extract(), + } +} + +/// Header violations of each collected file's final text, in path order, reusing earlier lints. +fn lint_final_contents<'py>( + run: &Run<'_, '_, 'py>, + (final_texts, original_lints): (&HashMap, &[Vec]), + applied: &Applied<'py>, +) -> PyResult>> { + let collected: Vec = (0..run.entries.len()) + .filter(|index| run.entries[*index].collected) + .collect(); + let sorted: Vec = run.sorted(&collected); + let final_text = |index: usize| -> &str { + final_texts + .get(&index) + .copied() + .unwrap_or(run.entries[index].original.as_str()) + }; + let known = |index: usize| -> Option<&Vec> { + let text: &str = final_text(index); + if text == run.entries[index].original { + return original_lints.get(index); + } + applied + .rewrite_lints + .get(&index) + .filter(|_| applied.rewrites.get(index) == Some(text)) + }; + let unknown: Vec = sorted + .iter() + .copied() + .filter(|index| known(*index).is_none()) + .collect(); + let config: HeaderRulesConfig = run.config; + let computed: Vec> = { + let texts: Vec<&str> = unknown.iter().map(|index| final_text(*index)).collect(); + native(run.py, || { + each_file(&texts, |text| lint_header_text(text, config)) + })? + }; + let computed: HashMap> = + unknown.into_iter().zip(computed).collect(); + let mut findings: Vec> = Vec::new(); + for index in sorted { + let violations: &Vec = match known(index) { + Some(violations) => violations, + None => &computed[&index], + }; + findings.extend( + violations + .iter() + .cloned() + .map(|violation| Finding::Header(index, violation)), + ); + } + Ok(findings) +} + +/// The entries `findings` are about; Python findings come only from uncached Rule fixes. +fn dirty_entries<'f>(findings: &'f [Finding<'_>]) -> impl Iterator + 'f { + findings.iter().filter_map(|finding| match finding { + Finding::Header(index, _) | Finding::Unsafe(index, _) | Finding::Suppression(index, _) => { + Some(*index) + } + Finding::Python(_) => None, + }) +} + +/// The effective settings that shape formatting, part of every cache key. +fn cache_settings(request: &FormatRequest<'_, '_>) -> PyResult { + let config: &Bound<'_, PyAny> = request.config; + let mut settings: Vec = Vec::new(); + for name in [ + "line_width", + "max_description_lines", + "max_literal_length", + "max_ranking_order_by", + ] { + settings.push(native_field(config, name)?.extract::()?.to_string()); + } + settings.push(native_field(config, "dialect")?.extract()?); + settings.push(request.fixtures_only.to_string()); + Ok(settings.join("\0")) +} + +/// SQL test files fixture-null formatting rewrites from model contracts, never skipped. +fn fixture_paths( + directory: &Path, + fixture_inputs: Option<&(Bound<'_, PyAny>, Bound<'_, PyAny>)>, +) -> PyResult> { + let Some((discovered, _)) = fixture_inputs else { + return Ok(BTreeSet::new()); + }; + let mut paths: BTreeSet = BTreeSet::new(); + for file in native_field(discovered, "test_files")?.try_iter()? { + let relative: PathBuf = native_field(&file?, "relative_path")?.extract()?; + let path: PathBuf = directory.join(relative); + let _ = paths.insert(fs::canonicalize(&path).unwrap_or_else(|_| path.clone())); + let _ = paths.insert(path); + } + Ok(paths) +} + +/// Collected files not cached as clean, each with its cache key, or none when not skippable. +fn uncached_entries( + py: Python<'_>, + (cache, uncacheable): (&mut FormatCache, &BTreeSet), + directory: &Path, + collected: Vec<(PathBuf, String)>, +) -> PyResult<(Vec, Vec>)> { + let shared: &FormatCache = cache; + let keys: Vec> = native(py, || { + each_file(&collected, |(path, contents)| { + let skippable: bool = uncacheable.is_empty() + || !(uncacheable.contains(path) + || fs::canonicalize(path) + .is_ok_and(|resolved| uncacheable.contains(&resolved))); + Ok(skippable.then(|| shared.key(&project_relative_path(path, directory), contents))) + }) + })?; + let mut entries: Vec = Vec::new(); + let mut kept: Vec> = Vec::new(); + let (mut hits, mut misses): (u64, u64) = (0, 0); + for ((path, original), key) in collected.into_iter().zip(keys) { + if let Some(key) = key.as_ref() { + if cache.is_clean(key) { + hits += 1; + continue; + } + misses += 1; + } + entries.push(Entry { + path, + original, + collected: true, + }); + kept.push(key); + } + count_stage(py, FORMAT_CACHE_HITS, hits)?; + count_stage(py, FORMAT_CACHE_MISSES, misses)?; + Ok((entries, kept)) } /// Format the project's authored SQL files and report what remains. @@ -267,53 +611,174 @@ pub(crate) fn run_format<'py>(request: &FormatRequest<'_, 'py>) -> PyResult
= collect_lint_files(&directory, request.selected) .map_err(|error| pyo3::exceptions::PyOSError::new_err(error.to_string()))?; let files_checked: usize = collected.len(); - let files: Bound<'py, PyDict> = file_dictionary(py, collected)?; - let safe: Bound<'py, PyAny> = safe_format_files( - &files.clone().into_any().cast_into::()?, - &request.config.clone(), - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; - let newlines: Bound<'py, PyDict> = safe.get_item(1)?.downcast_into::()?; - let (safe_files, rule_fixes, rule_faults) = - plan_rule_fixes(request, &safe.get_item(0)?.downcast_into::()?)?; - let (fixed, format_faults) = apply_fixes(request, &safe_files, fixture_inputs.as_ref())?; - for fault in rule_faults.try_iter()? { - format_faults.append(fault?)?; - } - let merged: Bound<'py, PyDict> = safe_files.copy()?; - merged.update(fixed.as_mapping())?; - let declined: BTreeSet = format_faults + let mut cache: Option = if request.cache { + FormatCache::open(py, &directory, cache_settings(request)?)? + } else { + None + }; + let (entries, keys): (Vec, Vec>) = match cache.as_mut() { + Some(cache) => { + let uncacheable: BTreeSet = + fixture_paths(&directory, fixture_inputs.as_ref())?; + uncached_entries(py, (cache, &uncacheable), &directory, collected)? + } + None => { + let entries: Vec = collected + .into_iter() + .map(|(path, original)| Entry { + path, + original, + collected: true, + }) + .collect(); + let keys: Vec> = vec![None; entries.len()]; + (entries, keys) + } + }; + let config: HeaderRulesConfig = rules_config(request.config)?; + let original_lints: Vec> = native(py, || { + each_file(&entries, |entry| lint_header_text(&entry.original, config)) + })?; + let mut run: Run<'_, '_, 'py> = Run { + request, + py, + directory, + config, + index: entries + .iter() + .enumerate() + .map(|(index, entry)| (entry.path.clone(), index)) + .collect(), + safe: (0..entries.len()) + .filter(|index| parse_faults(&original_lints[*index]).is_empty()) + .collect(), + entries, + fixed: HashMap::new(), + }; + let (rule_fixes, rule_faults) = if request.fix { + run.plan_rule_fixes()? + } else { + (Vec::new(), Vec::new()) + }; + let mut applied: Applied<'py> = run.apply_fixes(fixture_inputs.as_ref())?; + applied + .faults + .extend(rule_faults.into_iter().map(Finding::Python)); + let declined: BTreeSet = applied + .faults .iter() - .map(|fault| native_field(&fault, "file_path")?.extract()) + .map(|fault| fault_path(&run.entries, fault)) .collect::>()?; - let updated: Bound<'py, PyDict> = PyDict::new(py); - for (path, contents) in merged.iter() { - if !declined.contains(&path.extract::()?) { - updated.set_item(path, contents)?; - } - } - let changes: Vec> = - persist_changes((&files, &updated), &newlines, request.write)?; - let mut violations: Vec> = if request.fixtures_only { + let final_texts: HashMap = run + .safe + .iter() + .filter(|index| !declined.contains(&run.entries[**index].path)) + .map(|index| { + let text: &str = if applied.rejected.contains(index) { + run.base_text(*index) + } else { + run.text(*index, &applied.rewrites) + }; + (*index, text) + }) + .collect(); + let writes: Vec> = run + .sorted(&final_texts.keys().copied().collect::>()) + .into_iter() + .map(|index| { + let entry: &Entry = &run.entries[index]; + PendingWrite { + entry: index, + path: &entry.path, + authored: &entry.original, + contents: final_texts[&index], + newline: if entry.collected && entry.original.contains(CARRIAGE_RETURN_LINE_FEED) { + CARRIAGE_RETURN_LINE_FEED + } else { + LINE_FEED + }, + } + }) + .collect(); + let changes: Vec> = persist_changes(py, &writes, request.write)?; + let mut findings: Vec> = if request.fixtures_only { Vec::new() } else { - lint_final_contents(request, &files, &updated)? + lint_final_contents(&run, (&final_texts, &original_lints), &applied)? }; - violations.extend(format_faults.iter()); - let violations: Vec> = - drop_resolved_description_faults(request.project_dir, violations)?; - let final_contents: Bound<'py, PyDict> = overlaid(&files, &updated)?; - let violations: Vec> = reject_lint_suppressions( - py, - violations, - final_contents.clone(), - request.project_dir.extract()?, + findings.append(&mut applied.faults); + let findings_seen: BTreeSet = dirty_entries(&findings).collect(); + let python_findings: bool = findings + .iter() + .any(|finding| matches!(finding, Finding::Python(_))); + let violations: Vec> = drop_resolved_description_faults( + request.project_dir, + violation_records(py, &run.entries, findings)?, )?; + let collected: Vec = (0..run.entries.len()) + .filter(|index| run.entries[*index].collected) + .collect(); + let relative: Vec = collected + .iter() + .map(|index| project_relative_path(&run.entries[*index].path, &run.directory)) + .collect(); + let sources: Vec<(&str, &str)> = collected + .iter() + .zip(&relative) + .map(|(index, path)| { + ( + path.as_str(), + final_texts + .get(index) + .copied() + .unwrap_or(run.entries[*index].original.as_str()), + ) + }) + .collect(); + let suppressions: Vec> = native(py, || { + reject_inline_suppressions(&sources).map_err(|error| error.to_string()) + })? + .into_iter() + .map(|finding| Finding::Suppression(collected[finding.file], finding)) + .collect(); + if let Some(cache) = cache.as_mut() { + let dirty: BTreeSet = changes + .iter() + .map(|change| change.entry) + .chain(findings_seen.iter().copied()) + .chain(dirty_entries(&suppressions)) + .collect(); + for (index, key) in keys.iter().enumerate() { + if let Some(key) = key.filter(|_| !dirty.contains(&index)) { + cache.record_clean(key); + } + } + cache.save(py); + } + let reported: BTreeSet = findings_seen + .iter() + .copied() + .chain(dirty_entries(&suppressions)) + .collect(); + let mut violations: Vec> = violations; + violations.extend(violation_records(py, &run.entries, suppressions)?); + let final_contents: HashMap = collected + .iter() + .filter(|index| python_findings || reported.contains(index)) + .map(|index| { + let entry: &Entry = &run.entries[*index]; + let text: &str = final_texts + .get(index) + .copied() + .unwrap_or(entry.original.as_str()); + (entry.path.clone(), text.to_owned()) + }) + .collect(); Ok(FormatOutcome { files_checked, violations, changes, - rule_fixes: rule_fixes.try_iter()?.collect::>()?, + rule_fixes, declined, final_contents, }) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs b/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs new file mode 100644 index 0000000000..7d263fb0ce --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/format_command/stages.rs @@ -0,0 +1,72 @@ +//! The per-file native work of a format run: header lint and header formatting. + +use std::sync::OnceLock; + +use rayon::ThreadPool; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use sqlbuild_rules::dsl_headers::constants::RULE_HEADER_PARSE; +use sqlbuild_rules::dsl_headers::main::format_headers::format_headers; +use sqlbuild_rules::dsl_headers::main::scan_headers::scan_headers; +use sqlbuild_rules::dsl_headers::models::{HeaderRulesConfig, HeaderViolation}; + +use crate::bindings::_helpers::rules::dsl_headers::header_violations; +use crate::bindings::_helpers::rules::native_header_parser::NativeHeaderParser; + +/// Parsers recurse deeply on nested SQL, so format workers get the stack the main thread has. +const FORMAT_WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; + +static FORMAT_WORKERS: OnceLock> = OnceLock::new(); + +/// Run `operation` on the format worker threads, which every parallel pass inside it uses. +pub(crate) fn on_format_workers( + operation: impl FnOnce() -> Result + Send, +) -> Result { + FORMAT_WORKERS + .get_or_init(|| { + rayon::ThreadPoolBuilder::new() + .thread_name(|index| format!("sqlbuild-format-{index}")) + .stack_size(FORMAT_WORKER_STACK_BYTES) + .build() + .map_err(|error| error.to_string()) + }) + .as_ref() + .map_err(Clone::clone)? + .install(operation) +} + +/// Every header rule violation of `text`. +pub(crate) fn lint_header_text( + text: &str, + config: HeaderRulesConfig, +) -> Result, String> { + let characters: Vec = text.chars().collect(); + header_violations(&characters, &scan_headers(&characters, false), config) +} + +/// The header-parse faults among `violations`. +pub(crate) fn parse_faults(violations: &[HeaderViolation]) -> Vec { + violations + .iter() + .filter(|violation| violation.code == RULE_HEADER_PARSE) + .cloned() + .collect() +} + +/// `text` with header formatting fixes applied, and the faults relocation reported. +pub(crate) fn format_header_text( + text: &str, + config: HeaderRulesConfig, +) -> Result<(String, Vec), String> { + let characters: Vec = text.chars().collect(); + let (formatted, faults) = format_headers(&characters, config, &NativeHeaderParser)?; + Ok((formatted.into_iter().collect(), faults)) +} + +/// `operation` over every item on the worker threads; results and the first error keep item order. +pub(crate) fn each_file( + items: &[T], + operation: impl Fn(&T) -> Result + Sync + Send, +) -> Result, String> { + let results: Vec> = items.par_iter().map(operation).collect(); + results.into_iter().collect() +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/lint_text.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/lint_text.rs index 8f447e874b..a2584b361a 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/lint_text.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/lint_text.rs @@ -17,7 +17,7 @@ use sqlbuild_rules::lint_text::main::reject_inline_suppressions::reject_inline_s use sqlbuild_rules::lint_text::main::restore_interpolation::restore_interpolation; use sqlbuild_rules::lint_text::main::scan_fixture_null_candidates::scan_fixture_null_candidates; use sqlbuild_rules::lint_text::models::{ - FormatFile, FormattedBodies, InterpolationSite, Neutralized, SuppressionFinding, + FormatFault, FormatFile, FormattedBodies, InterpolationSite, Neutralized, SuppressionFinding, }; use crate::bindings::_helpers::boundary::panics::{compiler_guard, value_error}; @@ -156,7 +156,10 @@ pub(crate) fn reject_lint_suppressions<'py>( } /// The parts of `path` below `project_dir`, or `None` outside the project. -fn relative_parts(path: &std::path::Path, project_dir: &std::path::Path) -> Option> { +pub(crate) fn relative_parts( + path: &std::path::Path, + project_dir: &std::path::Path, +) -> Option> { match path.strip_prefix(project_dir) { Ok(relative) => Some( relative @@ -168,6 +171,42 @@ fn relative_parts(path: &std::path::Path, project_dir: &std::path::Path) -> Opti } } +/// The `format-unsafe` fault of a body the formatter declined in the file at `file_path`. +pub(crate) fn unsafe_fault<'py>( + file_path: &Bound<'py, PyAny>, + fault: &FormatFault, +) -> PyResult> { + let py: Python<'py> = file_path.py(); + let severity: Bound<'py, PyAny> = enum_member(py, RecordId::LintSeverity, "FAULT")?; + new_record_with( + py, + RecordId::LintViolation, + &[ + ("file_path", file_path.clone()), + ("line", fault.line.into_pyobject(py)?.into_any()), + ("column", 1_usize.into_pyobject(py)?.into_any()), + ("code", PyString::new(py, FORMAT_UNSAFE_CODE).into_any()), + ( + "message", + PyString::new( + py, + &format!( + "File left unchanged because its SQL body could not be formatted: {}", + fault.reason + ), + ) + .into_any(), + ), + ("severity", severity), + ("engine", PyString::new(py, NATIVE_ENGINE).into_any()), + ( + "remediation", + PyString::new(py, FORMAT_UNSAFE_REMEDIATION).into_any(), + ), + ], + ) +} + /// Format the SQL bodies of `files`; return changed texts by path and declined-body faults. #[pyfunction] pub(crate) fn format_lint_sql_bodies<'py>( @@ -196,36 +235,10 @@ pub(crate) fn format_lint_sql_bodies<'py>( for (index, text) in result.formatted { formatted.set_item(&files[index].0, text)?; } - let fault: Bound<'py, PyAny> = enum_member(py, RecordId::LintSeverity, "FAULT")?; let faults: Vec> = result .faults - .into_iter() - .map(|item| { - new_record_with(py, RecordId::LintViolation, &[ - ("file_path", files[item.file].0.clone()), - ("line", item.line.into_pyobject(py)?.into_any()), - ("column", 1_usize.into_pyobject(py)?.into_any()), - ("code", PyString::new(py, FORMAT_UNSAFE_CODE).into_any()), - ( - "message", - PyString::new( - py, - &format!( - "File left unchanged because its SQL body could not be formatted: {}", - item.reason - ), - ) - .into_any(), - ), - ("severity", fault.clone()), - ("engine", PyString::new(py, NATIVE_ENGINE).into_any()), - ( - "remediation", - PyString::new(py, FORMAT_UNSAFE_REMEDIATION).into_any(), - ), - ], - ) - }) + .iter() + .map(|item| unsafe_fault(&files[item.file].0, item)) .collect::>()?; Ok((formatted, faults)) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs index 8e73fc1c8a..64879da3a3 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/native_header_parser.rs @@ -7,7 +7,7 @@ use sqlbuild_rules::dsl_headers::main::inner_header_text::inner_header_text; use sqlbuild_rules::dsl_headers::models::{HeaderSpan, ParseFailure, ParseOutcome}; use sqlbuild_rules::dsl_headers::types::HeaderParser; use sqlbuild_sqltext::compiler::main::header_word_value::header_word_value; -use sqlbuild_sqltext::compiler::main::model_header_parsing::parse_batch; +use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; use sqlbuild_sqltext::compiler::models::{AuthoredValue, HeaderWord}; use crate::bindings::_helpers::boundary::panics::value_error; @@ -45,8 +45,7 @@ impl HeaderParser for NativeHeaderParser { type Error = String; fn parse(&self, kind: &str, inner: &str) -> ParseOutcome { - let headers: Vec = vec![inner.to_owned()]; - let parsed = match catch_compiler_panic(|| parse_batch(&headers)) { + let parsed = match catch_compiler_panic(|| Ok(parse_one(inner))) { Ok(parsed) => parsed, Err(message) => { return ParseOutcome::Raised( @@ -58,7 +57,7 @@ impl HeaderParser for NativeHeaderParser { ); } }; - let (value, _, error) = parsed.into_iter().next().unwrap_or((None, None, None)); + let (value, _, error) = parsed; if let Some(value) = value { return ParseOutcome::Parsed(value); } diff --git a/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs b/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs index 16c9a0567e..e87e7c25da 100644 --- a/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs +++ b/crates/sqlbuild-rules/src/dsl_headers/_helpers/scan.rs @@ -54,10 +54,14 @@ fn leading_kind<'k>(text: &[char], index: usize, kinds: &[&'k str]) -> Option<(& start += 1; } kinds.iter().find_map(|kind| { - let keyword: Vec = kind.chars().collect(); - let end: usize = start + keyword.len(); - (text[start..].starts_with(&keyword) && !text.get(end).is_some_and(|next| is_word(*next))) - .then_some((*kind, start)) + let mut end: usize = start; + for expected in kind.chars() { + if text.get(end) != Some(&expected) { + return None; + } + end += 1; + } + (!text.get(end).is_some_and(|next| is_word(*next))).then_some((*kind, start)) }) } diff --git a/crates/sqlbuild-rules/src/lint_text/main/format_sql_bodies.rs b/crates/sqlbuild-rules/src/lint_text/main/format_sql_bodies.rs index f823ee3ce7..6bf08424e0 100644 --- a/crates/sqlbuild-rules/src/lint_text/main/format_sql_bodies.rs +++ b/crates/sqlbuild-rules/src/lint_text/main/format_sql_bodies.rs @@ -1,8 +1,12 @@ //! `format_sql_bodies`: SQLBuild-owned safe formatting of every SQL body in files. -use crate::lint_text::_helpers::format_bodies::{RequestTable, prepare_file, splice_file}; +use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; + +use crate::lint_text::_helpers::format_bodies::{ + PendingBody, RequestTable, prepare_file, splice_file, +}; use crate::lint_text::errors::LintTextError; -use crate::lint_text::models::{FormatFile, FormattedBodies, PreparedBody}; +use crate::lint_text::models::{FormatFault, FormatFile, FormattedBodies, PreparedBody}; use crate::sql_lint::main::format_requests::format_requests; use crate::sql_lint::models::FormatResponse; @@ -12,20 +16,29 @@ pub fn format_sql_bodies( dialect: &str, line_width: usize, ) -> Result { + let pending: Vec, LintTextError>> = files + .par_iter() + .map(|file| prepare_file(file, dialect)) + .collect(); let mut table: RequestTable = RequestTable::default(); let mut prepared: Vec> = Vec::with_capacity(files.len()); - for file in files { + for file_bodies in pending { let mut bodies: Vec = Vec::new(); - for (mut body, sql, widths) in prepare_file(file, dialect)? { + for (mut body, sql, widths) in file_bodies? { body.request = table.insert(sql, widths, (dialect, line_width)); bodies.push(body); } prepared.push(bodies); } let responses: Vec = format_requests(table.requests); + let spliced: Vec<(Option, Vec)> = files + .par_iter() + .zip(prepared.par_iter()) + .enumerate() + .map(|(index, (file, bodies))| splice_file(index, &file.contents, bodies, &responses)) + .collect(); let mut result: FormattedBodies = FormattedBodies::default(); - for (index, (file, bodies)) in files.iter().zip(prepared).enumerate() { - let (formatted, faults) = splice_file(index, &file.contents, &bodies, &responses); + for (index, (formatted, faults)) in spliced.into_iter().enumerate() { result.formatted.extend(formatted.map(|text| (index, text))); result.faults.extend(faults); } diff --git a/crates/sqlbuild-rules/src/lint_text/main/reject_inline_suppressions.rs b/crates/sqlbuild-rules/src/lint_text/main/reject_inline_suppressions.rs index 23cf26ff3f..9cb72a4222 100644 --- a/crates/sqlbuild-rules/src/lint_text/main/reject_inline_suppressions.rs +++ b/crates/sqlbuild-rules/src/lint_text/main/reject_inline_suppressions.rs @@ -1,5 +1,7 @@ //! `reject_inline_suppressions`: an error for every `sqb: ignore` directive in a SQL comment. +use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; + use crate::lint_text::_helpers::lines::{LineSpan, line_spans}; use crate::lint_text::_helpers::suppressions::directive_findings; use crate::lint_text::errors::LintTextError; @@ -9,10 +11,17 @@ use crate::lint_text::models::SuppressionFinding; pub fn reject_inline_suppressions( files: &[(&str, &str)], ) -> Result, LintTextError> { + let per_file: Vec, LintTextError>> = files + .par_iter() + .enumerate() + .map(|(file, (path, contents))| { + let lines: Vec> = line_spans(contents); + directive_findings(file, path, (contents, &lines)) + }) + .collect(); let mut findings: Vec = Vec::new(); - for (file, (path, contents)) in files.iter().enumerate() { - let lines: Vec> = line_spans(contents); - findings.extend(directive_findings(file, path, (contents, &lines))?); + for file_findings in per_file { + findings.extend(file_findings?); } Ok(findings) } diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs index 8c41ac461f..c016eccf54 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/formatter.rs @@ -8,8 +8,8 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator}; use crate::sql_lint::_helpers::formatter_syntax::{cte_macro_indices, protect_syntax}; use crate::sql_lint::_helpers::printed_layout::{PrintedSql, WrapOptions, wrap_lines}; use crate::sql_lint::_helpers::token_layout::{ - AuthoredSql, KeywordRecase, comments_in, neutralize_comments, print_authored_tokens, - verify_token_invariant, + AuthoredSql, KeywordRecase, OracleLayout, comments_in, neutralize_comments, oracle_layout, + print_authored_tokens, tokenize, verify_token_invariant, }; use crate::sql_lint::constants::{ CAST_TYPE_SEPARATOR_KEYWORD, CLOSE_PARENTHESIS, LINT_API_VERSION, OPEN_PARENTHESIS, @@ -91,8 +91,8 @@ fn try_format_sql(request: FormatRequest) -> Result { dialect: &dialect, wrap: wrap.as_ref(), }; - let (formatted, oracle) = format_layout(&original, &layout, &parse_options)?; - if relayout(&formatted, &oracle, &layout)? != formatted { + let (formatted, formatted_tokens, oracle) = format_layout(&original, &layout, &parse_options)?; + if relayout((&formatted, &formatted_tokens), oracle.as_ref(), &layout)? != formatted { return Err(NOT_IDEMPOTENT_FAILURE.to_string()); } let changed = formatted != original; @@ -105,18 +105,21 @@ fn try_format_sql(request: FormatRequest) -> Result { }) } +/// The printed output of [`format_layout`], its tokens, and the oracle it was printed against. +type FormattedLayout = (String, Vec, Option); + /// Print the tokens of `sql` with parse-tree whitespace; return the output and its oracle. fn format_layout( sql: &str, layout: &Layout<'_>, parse_options: &ParseOptions, -) -> Result<(String, String), String> { +) -> Result { let dialect = layout.dialect; let tokens = dialect .tokenize(sql) .map_err(|error| format!("native formatter tokenization failed: {error}"))?; if tokens.is_empty() { - return Ok((sql.to_string(), sql.to_string())); + return Ok((sql.to_string(), tokens, None)); } let comments = comments_in(sql, &tokens); let neutral = neutralize_comments(sql, &comments); @@ -125,30 +128,39 @@ fn format_layout( .parse_with_options(&validation_sql.sql, parse_options) .map_err(|error| format!("native formatter could not parse SQL: {error}"))?; let semantic_tokens = without_statement_terminators(&tokens); - let oracle = layout_oracle(&neutral, &semantic_tokens, dialect, parse_options)?; + let oracle = oracle_layout( + &layout_oracle(&neutral, &semantic_tokens, dialect, parse_options)?, + dialect, + )?; let authored = AuthoredSql { sql, tokens: &tokens, comments: &comments, }; let (printed, recases) = layout.print(&authored, &oracle)?; - verify_token_invariant(sql, &printed, dialect, &recases)?; - Ok((printed, oracle)) + let printed_tokens = tokenize(&printed, dialect)?; + verify_token_invariant( + (sql, &tokens), + (&printed, &printed_tokens), + dialect, + &recases, + )?; + Ok((printed, printed_tokens, Some(oracle))) } /// Re-print formatted SQL against its oracle; unchanged tokens imply an unchanged layout. -fn relayout(formatted: &str, oracle: &str, layout: &Layout<'_>) -> Result { - let tokens = layout - .dialect - .tokenize(formatted) - .map_err(|error| format!("native formatter tokenization failed: {error}"))?; - if tokens.is_empty() { +fn relayout( + (formatted, tokens): (&str, &[Token]), + oracle: Option<&OracleLayout>, + layout: &Layout<'_>, +) -> Result { + let Some(oracle) = oracle.filter(|_| !tokens.is_empty()) else { return Ok(formatted.to_string()); - } - let comments = comments_in(formatted, &tokens); + }; + let comments = comments_in(formatted, tokens); let authored = AuthoredSql { sql: formatted, - tokens: &tokens, + tokens, comments: &comments, }; Ok(layout.print(&authored, oracle)?.0) @@ -164,7 +176,7 @@ impl Layout<'_> { fn print( &self, authored: &AuthoredSql<'_>, - oracle: &str, + oracle: &OracleLayout, ) -> Result<(String, BTreeMap), String> { let (printed, recases) = print_authored_tokens(authored, oracle, self.dialect)?; let no_widths: HashMap = HashMap::new(); diff --git a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs index eac207cbb8..80d196c96e 100644 --- a/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs +++ b/crates/sqlbuild-rules/src/sql_lint/_helpers/token_layout.rs @@ -157,10 +157,27 @@ pub(crate) struct AuthoredSql<'a> { pub(crate) comments: &'a [Comment], } +/// The lexemes and characters of a layout oracle, read once for every print against it. +pub(crate) struct OracleLayout { + generated: Vec, + characters: Vec, +} + +/// Tokenize `oracle` for printing against it. +pub(crate) fn oracle_layout(oracle: &str, dialect: &Dialect) -> Result { + let oracle_tokens = dialect + .tokenize(oracle) + .map_err(|error| error.to_string())?; + Ok(OracleLayout { + generated: lexemes(oracle, &oracle_tokens, None)?, + characters: oracle.chars().collect(), + }) +} + /// Print every authored token in order, taking only whitespace and keyword case from `oracle`. pub(crate) fn print_authored_tokens( source: &AuthoredSql<'_>, - oracle: &str, + oracle: &OracleLayout, dialect: &Dialect, ) -> Result<(String, BTreeMap), String> { let AuthoredSql { @@ -169,23 +186,21 @@ pub(crate) fn print_authored_tokens( comments, } = *source; let authored = lexemes(sql, tokens, Some(dialect))?; - let oracle_tokens = dialect - .tokenize(oracle) - .map_err(|error| error.to_string())?; - let generated = lexemes(oracle, &oracle_tokens, None)?; - let alignment = align(&authored, &generated)?; - let oracle_characters: Vec = oracle.chars().collect(); - let separators = separators(&authored, &generated, &alignment, &oracle_characters)?; + let generated = &oracle.generated; + let alignment = align(&authored, generated)?; + let separators = separators(&authored, generated, &alignment, &oracle.characters)?; let attached = attach_comments(tokens, comments); let mut output = String::with_capacity(sql.len() + sql.len() / 4); let string_breaks = string_breaks(sql, tokens); let mut recases: BTreeMap = BTreeMap::new(); for (index, lexeme) in authored.iter().enumerate() { let forced_break = string_breaks.contains(&index) && !separators[index].contains('\n'); - let separator = &if forced_break { - format!("\n{}", current_indentation(&output)) + let forced: String; + let separator: &str = if forced_break { + forced = format!("\n{}", current_indentation(&output)); + &forced } else { - separators[index].clone() + &separators[index] }; let leading = &attached[index].leading; if leading.is_empty() { @@ -246,20 +261,18 @@ pub(crate) fn print_authored_tokens( /// Refuse output that changes anything but layout, fold-set case, and recorded keyword recases. pub(crate) fn verify_token_invariant( - before: &str, - after: &str, + (before, before_tokens): (&str, &[Token]), + (after, after_tokens): (&str, &[Token]), dialect: &Dialect, recases: &BTreeMap, ) -> Result<(), String> { - let before_tokens = tokenize(before, dialect)?; - let after_tokens = tokenize(after, dialect)?; - let before_canonical = canonical_tokens(before, &before_tokens, dialect)?; - let after_canonical = canonical_tokens(after, &after_tokens, dialect)?; + let before_canonical = canonical_tokens(before, before_tokens, dialect)?; + let after_canonical = canonical_tokens(after, after_tokens, dialect)?; if before_canonical.len() != after_canonical.len() { return Err(TOKEN_PRESERVATION_FAILURE.to_string()); } - let before_texts = token_texts(before, &before_tokens)?; - let after_texts = token_texts(after, &after_tokens)?; + let before_texts = token_texts(before, before_tokens)?; + let after_texts = token_texts(after, after_tokens)?; for (index, (left, right)) in before_canonical.iter().zip(&after_canonical).enumerate() { if left != right && !recases @@ -269,10 +282,10 @@ pub(crate) fn verify_token_invariant( return Err(TOKEN_PRESERVATION_FAILURE.to_string()); } } - if comment_positions(before, &before_tokens) != comment_positions(after, &after_tokens) { + if comment_positions(before, before_tokens) != comment_positions(after, after_tokens) { return Err(COMMENT_ATTACHMENT_FAILURE.to_string()); } - if !string_breaks(before, &before_tokens).is_subset(&string_breaks(after, &after_tokens)) { + if !string_breaks(before, before_tokens).is_subset(&string_breaks(after, after_tokens)) { return Err(STRING_BREAK_FAILURE.to_string()); } Ok(()) @@ -288,7 +301,7 @@ impl KeywordRecase { } } -fn tokenize(sql: &str, dialect: &Dialect) -> Result, String> { +pub(crate) fn tokenize(sql: &str, dialect: &Dialect) -> Result, String> { dialect.tokenize(sql).map_err(|error| error.to_string()) } diff --git a/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs b/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs index 3d98f99548..af36b480a1 100644 --- a/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs +++ b/crates/sqlbuild-rules/src/sql_lint/tests/test_authored_tokens.rs @@ -2,7 +2,7 @@ use serde_json::{Value, json}; use std::collections::BTreeMap; -use crate::sql_lint::_helpers::token_layout::{KeywordRecase, verify_token_invariant}; +use crate::sql_lint::_helpers::token_layout::{KeywordRecase, tokenize, verify_token_invariant}; use crate::sql_lint::main::formatter::format_json; use crate::sql_lint::tests::test_types; @@ -394,7 +394,14 @@ fn given_formatted_candidates_when_checking_token_invariant_then_only_layout_may ) }) .collect(); - let actual = verify_token_invariant(test_case.before, test_case.after, &dialect, &recases); + let before_tokens = tokenize(test_case.before, &dialect).expect("before tokenizes"); + let after_tokens = tokenize(test_case.after, &dialect).expect("after tokenizes"); + let actual = verify_token_invariant( + (test_case.before, &before_tokens), + (test_case.after, &after_tokens), + &dialect, + &recases, + ); assert_eq!( actual.err().as_deref(), test_case.expected_error, diff --git a/fensu.toml b/fensu.toml index c401ef48db..cc5bec7318 100644 --- a/fensu.toml +++ b/fensu.toml @@ -2338,7 +2338,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/fingerprints/storage_io.rs=12", "crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs=1", - "crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs=7", + "crates/sqlbuild-python/src/bindings/_helpers/format_command/run.rs=6", "crates/sqlbuild-python/src/bindings/_helpers/format_command/selection.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/freshness/freshness_observe.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/freshness/observation.rs=3", diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md index 1662f04eb3..8d0fca7d69 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/format.md @@ -8,6 +8,7 @@ Online: https://sqlbuild.com/docs/cli/format/ ## Contents +- Skipping unchanged files - Line width and descriptions - What formatting preserves - Rule fixes @@ -25,6 +26,7 @@ sqb format [flags] | `--diff` | Print the proposed diff without writing | | `--fix` | Also apply compiler-verified, meaning-preserving Rule fixes before layout formatting | | `--json` | Print structured results | +| `--no-cache` | Format every file instead of skipping files a previous run found clean | | `--select`, `-s` | Format selected models | | `--select-file` | Read selectors from a file | | `--exclude` | Exclude models from the selected scope | @@ -38,6 +40,20 @@ sqb format --diff --select tag:marts Formatting owns presentation and source rewriting. Compiler-integrated diagnostics belong to configured [Rules](../concepts/rules.md), which never mutate source. +Files are formatted in parallel; the report, the diff and the exit code are the same as a +one-file-at-a-time run. + +## Skipping unchanged files + +`sqb format` remembers every file it found already formatted and free of faults and warnings, in +`target/cache/compiler-native-v1/format-results.bin`. A later run skips such a file while its text, +its path, the `[format]` and rule threshold settings, the adapter dialect and the SQLBuild build are +all unchanged, so `sqb format --check` on an unchanged project costs little more than reading the +files. A file that needed changes or reported anything is always formatted again, and a damaged or +unreadable cache is ignored. `--fix` never uses the cache, and SQL test files are not skipped +while typed-null fixture formatting reads their models' contracts. Pass `--no-cache` to format +every file. + ## Line width and descriptions Configure the maximum physical line width in `sqlbuild_project.toml` (the default is `100`): diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 991ee32b6f..8becb121ff 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -5344,6 +5344,7 @@ class SqlTestScanCache: def save(self) -> None: ... SQL_TEST_SCAN_STORE_FILE_NAME: str +FORMAT_RESULT_STORE_FILE_NAME: str def content_digest(parts: list[str]) -> str: ... def compiled_code_identity() -> str: ... @@ -5796,6 +5797,7 @@ def run_format_command( fix: bool = False, json_output: bool = False, no_color: bool = False, + no_cache: bool = False, ) -> int: ... def format_parameterized_test_label( *, diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py index 4cbe2169ba..8213b07daf 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/_test_types.py @@ -107,3 +107,23 @@ class TokenPreservationBuildTestCase: expected_sql: str expected_columns: list[str] expected_rows: list[tuple[object, ...]] + + +@dataclass(frozen=True) +class FormatParallelDeterminismTestCase: + """One format mode whose output must not depend on how many worker threads run it.""" + + description: str + arguments: tuple[str, ...] + expected_returncode: int + + +@dataclass(frozen=True) +class FormatCacheEndToEndTestCase: + """A generated project checked cold, warm, after an edit and after damaging the cache.""" + + description: str + model_count: int + edited_model: str + expected_warm_summary: str + expected_edited_summary: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py index c1f2e55cd6..5d6165828a 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/helpers.py @@ -9,6 +9,7 @@ import time from pathlib import Path +from scripts.format_performance._helpers.model_project import write_model_format_project from sqlbuild._native import compute_query_hash _FORMAT_ELAPSED_PATTERN: re.Pattern[str] = re.compile(r"Formatting SQL OK \(([0-9.]+)s;") @@ -131,3 +132,49 @@ def run_sqb(*, project_dir: Path, arguments: tuple[str, ...]) -> subprocess.Comp text=True, timeout=60, ) + + +FORMAT_CACHE_UNFORMATTED_ORDERS: str = ( + 'MODEL (description "Orders placed by customers.", materialized table);\n' + "select order_id, customer_id from __source('shop', 'orders')\n" +) + + +def write_mixed_format_project(*, project_dir: Path, model_count: int) -> None: + """Unformatted models plus files that fault, use CRLF line endings or carry a directive.""" + + write_model_format_project(project_dir=project_dir, model_count=model_count) + models_dir: Path = project_dir / "models" + path: Path + for path in sorted(models_dir.glob("*.sql")): + _ = path.write_text( + 'MODEL (description "Generated order model.", materialized view);\n' + + path.read_text(encoding="utf-8"), + encoding="utf-8", + ) + _ = (models_dir / "undescribed_orders.sql").write_text( + "MODEL (materialized view);\nselect 1 as order_id\n", encoding="utf-8" + ) + _ = (models_dir / "broken_header.sql").write_text( + 'MODEL (description "Broken.", materialized ((;\nselect 1 as order_id\n', + encoding="utf-8", + ) + _ = (models_dir / "unsafe_body.sql").write_text( + 'MODEL (description "Unsafe body.");\nselect (((( from where 1\n', encoding="utf-8" + ) + _ = (models_dir / "windows_orders.sql").write_bytes( + b'MODEL (description "Windows line endings.");\r\nselect 1 as order_id\r\n' + ) + _ = (models_dir / "suppressed_orders.sql").write_text( + 'MODEL (description "Suppressed.");\n-- sqb: ignore some-rule\nselect 1 as order_id\n', + encoding="utf-8", + ) + + +def model_tree(*, project_dir: Path) -> dict[str, bytes]: + """The bytes of every model file, by project-relative path.""" + + return { + str(path.relative_to(project_dir)): path.read_bytes() + for path in sorted((project_dir / "models").rglob("*.sql")) + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_cache.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_cache.py new file mode 100644 index 0000000000..e9f6a094b1 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_format_cache.py @@ -0,0 +1,127 @@ +"""End-to-end coverage for parallel format determinism and the format result cache.""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +from scripts.format_performance._helpers.model_project import write_model_format_project +from sqlbuild import _native as native_module +from tests.e2e.src.sqlbuild.cli.commands.main.format._test_types import ( + FormatCacheEndToEndTestCase, + FormatParallelDeterminismTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.format.helpers import ( + FORMAT_CACHE_UNFORMATTED_ORDERS, + model_tree, + write_mixed_format_project, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import run_sqb + +_SERIAL: dict[str, str] = {"RAYON_NUM_THREADS": "1"} +_PARALLEL: dict[str, str] = {"RAYON_NUM_THREADS": "4"} + + +@pytest.mark.parametrize( + "test_case", + [ + FormatParallelDeterminismTestCase( + description="check report", arguments=("--check",), expected_returncode=1 + ), + FormatParallelDeterminismTestCase( + description="json report", arguments=("--check", "--json"), expected_returncode=1 + ), + FormatParallelDeterminismTestCase( + description="diff", arguments=("--diff",), expected_returncode=1 + ), + FormatParallelDeterminismTestCase( + description="written files", arguments=(), expected_returncode=1 + ), + ], + ids=lambda case: case.description, +) +def test_given_mixed_project_when_formatting_serially_and_in_parallel_then_results_match( + test_case: FormatParallelDeterminismTestCase, + tmp_path: Path, +) -> None: + serial_dir: Path = tmp_path / "serial" + write_mixed_format_project(project_dir=serial_dir, model_count=60) + parallel_dir: Path = tmp_path / "parallel" + _ = shutil.copytree(serial_dir, parallel_dir) + command: tuple[str, ...] = ("--no-color", "format", "--no-cache", *test_case.arguments) + + serial: subprocess.CompletedProcess[str] = run_sqb( + project_dir=serial_dir, command=command, env=_SERIAL + ) + parallel: subprocess.CompletedProcess[str] = run_sqb( + project_dir=parallel_dir, command=command, env=_PARALLEL + ) + + assert serial.returncode == test_case.expected_returncode, serial.stderr + assert parallel.returncode == test_case.expected_returncode, parallel.stderr + assert parallel.stdout.replace(str(parallel_dir), "") == serial.stdout.replace( + str(serial_dir), "" + ) + assert model_tree(project_dir=parallel_dir) == model_tree(project_dir=serial_dir) + + +@pytest.mark.parametrize( + "test_case", + [ + FormatCacheEndToEndTestCase( + description="generated orders project", + model_count=40, + edited_model="orders_00007.sql", + expected_warm_summary="40 files checked, 0 changed", + expected_edited_summary="40 files checked, 1 changed", + ) + ], + ids=lambda case: case.description, +) +def test_given_formatted_project_when_checking_warm_edited_and_damaged_then_results_stay_exact( + test_case: FormatCacheEndToEndTestCase, + tmp_path: Path, +) -> None: + project_dir: Path = tmp_path / "project" + write_model_format_project(project_dir=project_dir, model_count=test_case.model_count) + store: Path = ( + native_module.compiler_cache_directory(project_dir) + / native_module.FORMAT_RESULT_STORE_FILE_NAME + ) + check: tuple[str, ...] = ("--no-color", "format", "--check") + + formatted: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "format") + ) + cold: subprocess.CompletedProcess[str] = run_sqb(project_dir=project_dir, command=check) + stored: bytes = store.read_bytes() + warm: subprocess.CompletedProcess[str] = run_sqb(project_dir=project_dir, command=check) + _ = (project_dir / "models" / test_case.edited_model).write_text( + FORMAT_CACHE_UNFORMATTED_ORDERS, encoding="utf-8" + ) + edited: subprocess.CompletedProcess[str] = run_sqb(project_dir=project_dir, command=check) + _ = store.write_bytes(b"not a native store") + damaged: subprocess.CompletedProcess[str] = run_sqb(project_dir=project_dir, command=check) + uncached: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=(*check, "--no-cache") + ) + + assert formatted.returncode == 0, formatted.stderr + assert cold.returncode == 0, cold.stderr + assert stored + assert warm.returncode == 0, warm.stderr + assert warm.stdout == cold.stdout + assert test_case.expected_warm_summary in warm.stderr + assert edited.returncode == 1, edited.stderr + assert test_case.edited_model in edited.stdout + assert test_case.expected_edited_summary in edited.stderr + assert damaged.returncode == 1, damaged.stderr + assert damaged.stdout == edited.stdout + assert uncached.stdout == edited.stdout + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) 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 9d10264b4c..2499250938 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -4,6 +4,8 @@ from dataclasses import dataclass, field from pathlib import Path +import pytest + from sqlbuild._native import CursorOverrides @@ -1208,3 +1210,50 @@ class TypedConfigReadsTestCase: description: str unread_keys: str expected_same_reads: bool + + +@dataclass(frozen=True) +class FormatCacheReuseTestCase: + """A second `format --check` of an unchanged project and the work its cache saves.""" + + description: str + arguments: tuple[str, ...] + expected_hits: int + expected_misses: int + expected_formatted_files: int + + +@dataclass(frozen=True) +class FormatCacheInvalidationTestCase: + """A change after a clean `format --check` and the files the next check formats again.""" + + description: str + change: Callable[[Path, pytest.MonkeyPatch], None] + expected_hits: int + expected_misses: int + expected_exit_code: int + expected_recheck_hits: int + expected_output_fragment: str = "" + + +@dataclass(frozen=True) +class FormatCacheFaultTestCase: + """A file whose remaining fault keeps it out of the format result cache.""" + + description: str + removed_text: str + expected_code: str + expected_hits: int + expected_misses: int + + +@dataclass(frozen=True) +class FormatWriteFailureTestCase: + """A write that fails part-way through a format run and which files it replaced.""" + + description: str + directories: tuple[str, ...] + locked_directory: str + expected_replaced: tuple[bool, ...] + expected_error: type[BaseException] + expected_status: str diff --git a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py index 74bc48c9e9..6ac9989863 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py @@ -32,6 +32,7 @@ from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( RepeatedJsonParseTestCase, ) +from tests.unit.src.sqlbuild.rule_engine._helpers.run.helpers import rebuild_identity STAGE_CAPTURE_SUFFIX: str = ".json" @@ -1008,3 +1009,85 @@ def typed_config_reads(*, project_dir: Path, extra_header: str) -> int: exit_code: int = main(["--project-dir", str(project_dir), "plan", "--json", "--no-cache"]) assert exit_code == 0 return native_module.compile_stage_work_info(reset=True).get("typed_config_reads", 0) + + +FORMAT_CACHE_UNFORMATTED_MODELS: dict[str, str] = { + "orders.sql": ( + 'MODEL (description "Orders placed by customers.", materialized table);\n' + "select order_id, customer_id from __source('shop', 'orders')\n" + ), + "customers.sql": ( + 'MODEL (description "Customers who placed orders.", materialized table);\n' + "select customer_id, min(order_id) as first_order_id from __ref('orders') " + "group by customer_id\n" + ), + "products.sql": ( + 'MODEL (description "Products in the catalog.", materialized view);\n' + "select 1 as product_id, 'widget' as product_name\n" + ), +} + + +def write_formatted_cache_project(*, project_dir: Path) -> None: + """Write three small models and format them once without the format result cache.""" + + (project_dir / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + models_dir: Path = project_dir / "models" + models_dir.mkdir() + name: str + sql: str + for name, sql in FORMAT_CACHE_UNFORMATTED_MODELS.items(): + (models_dir / name).write_text(sql, encoding="utf-8") + assert main(["--project-dir", str(project_dir), "format", "--no-cache"]) == 0 + + +def format_result_store_path(*, project_dir: Path) -> Path: + """The project's format result store file.""" + + return ( + native_module.compiler_cache_directory(project_dir) + / native_module.FORMAT_RESULT_STORE_FILE_NAME + ) + + +def check_format_with_work( + *, project_dir: Path, arguments: tuple[str, ...] = () +) -> tuple[int, dict[str, int]]: + """Run `format --check` in process and return its exit code and native stage work.""" + + _ = native_module.compile_stage_work_info(reset=True) + exit_code: int = main(["--project-dir", str(project_dir), "format", "--check", *arguments]) + return exit_code, native_module.compile_stage_work_info() + + +def edit_cached_orders_model(project_dir: Path, _monkeypatch: pytest.MonkeyPatch) -> None: + """Make one cached model unformatted again.""" + + (project_dir / "models" / "orders.sql").write_text( + FORMAT_CACHE_UNFORMATTED_MODELS["orders.sql"], encoding="utf-8" + ) + + +def widen_format_line_width(project_dir: Path, _monkeypatch: pytest.MonkeyPatch) -> None: + """Change the `[format]` line width, which every cache key includes.""" + + config: Path = project_dir / "sqlbuild_project.toml" + _ = config.write_text( + config.read_text(encoding="utf-8") + "\n[format]\nline_width = 120\n", encoding="utf-8" + ) + + +def damage_format_result_store(project_dir: Path, _monkeypatch: pytest.MonkeyPatch) -> None: + """Overwrite the end of the stored format results so the store fails its checksum.""" + + path: Path = format_result_store_path(project_dir=project_dir) + assert path.is_file() + _ = path.write_bytes(path.read_bytes()[:-7] + b"damaged") + + +def rebuild_formatter(_project_dir: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Pretend another SQLBuild build formats the project.""" + + rebuild_identity(monkeypatch=monkeypatch) diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_format_cache.py b/tests/integration/src/sqlbuild/cli/commands/main/test_format_cache.py new file mode 100644 index 0000000000..aedf6b9359 --- /dev/null +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format_cache.py @@ -0,0 +1,179 @@ +"""Integration coverage for the format result cache: clean files are skipped until they change.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from _pytest.capture import CaptureResult + +from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( + FormatCacheFaultTestCase, + FormatCacheInvalidationTestCase, + FormatCacheReuseTestCase, +) +from tests.integration.src.sqlbuild.cli.commands.main.helpers import ( + check_format_with_work, + damage_format_result_store, + edit_cached_orders_model, + rebuild_formatter, + widen_format_line_width, + write_formatted_cache_project, +) + + +@pytest.mark.parametrize( + "test_case", + [ + FormatCacheReuseTestCase( + description="check skips every file a previous check found clean", + arguments=(), + expected_hits=3, + expected_misses=0, + expected_formatted_files=0, + ), + FormatCacheReuseTestCase( + description="diff reuses the same clean results", + arguments=("--diff",), + expected_hits=3, + expected_misses=0, + expected_formatted_files=0, + ), + FormatCacheReuseTestCase( + description="no-cache formats every file again", + arguments=("--no-cache",), + expected_hits=0, + expected_misses=0, + expected_formatted_files=3, + ), + ], + ids=lambda case: case.description, +) +def test_given_clean_checked_project_when_checking_again_then_cached_files_are_not_formatted( + test_case: FormatCacheReuseTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_formatted_cache_project(project_dir=tmp_path) + _ = capsys.readouterr() + cold_exit, cold_work = check_format_with_work(project_dir=tmp_path) + cold: CaptureResult[str] = capsys.readouterr() + + warm_exit, warm_work = check_format_with_work( + project_dir=tmp_path, arguments=test_case.arguments + ) + warm: CaptureResult[str] = capsys.readouterr() + + assert cold_exit == 0 + assert cold_work.get("format_cache_misses", 0) == 3 + assert warm_exit == 0 + assert warm_work.get("format_cache_hits", 0) == test_case.expected_hits + assert warm_work.get("format_cache_misses", 0) == test_case.expected_misses + assert warm_work.get("format_lint_files", 0) == test_case.expected_formatted_files + assert warm.out == cold.out + assert "3 files checked, 0 changed" in warm.err + + +@pytest.mark.parametrize( + "test_case", + [ + FormatCacheInvalidationTestCase( + description="an edited file is formatted again and reported", + change=edit_cached_orders_model, + expected_hits=2, + expected_misses=1, + expected_exit_code=1, + expected_recheck_hits=2, + expected_output_fragment="orders.sql", + ), + FormatCacheInvalidationTestCase( + description="changed format settings format every file again", + change=widen_format_line_width, + expected_hits=0, + expected_misses=3, + expected_exit_code=0, + expected_recheck_hits=3, + ), + FormatCacheInvalidationTestCase( + description="another formatter build formats every file again", + change=rebuild_formatter, + expected_hits=0, + expected_misses=3, + expected_exit_code=0, + expected_recheck_hits=3, + ), + FormatCacheInvalidationTestCase( + description="a damaged store is ignored", + change=damage_format_result_store, + expected_hits=0, + expected_misses=3, + expected_exit_code=0, + expected_recheck_hits=3, + ), + ], + ids=lambda case: case.description, +) +def test_given_cached_clean_project_when_inputs_change_then_affected_files_are_formatted_again( + test_case: FormatCacheInvalidationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + write_formatted_cache_project(project_dir=tmp_path) + assert check_format_with_work(project_dir=tmp_path)[0] == 0 + _ = capsys.readouterr() + test_case.change(tmp_path, monkeypatch) + + exit_code, work = check_format_with_work(project_dir=tmp_path) + captured: CaptureResult[str] = capsys.readouterr() + rechecked_exit, rechecked_work = check_format_with_work(project_dir=tmp_path) + + assert exit_code == test_case.expected_exit_code + assert work.get("format_cache_hits", 0) == test_case.expected_hits + assert work.get("format_cache_misses", 0) == test_case.expected_misses + assert test_case.expected_output_fragment in captured.out + assert rechecked_exit == test_case.expected_exit_code + assert rechecked_work.get("format_cache_hits", 0) == test_case.expected_recheck_hits + + +@pytest.mark.parametrize( + "test_case", + [ + FormatCacheFaultTestCase( + description="a missing description is reported on every check", + removed_text='description "Products in the catalog.", ', + expected_code="description-present", + expected_hits=2, + expected_misses=1, + ) + ], + ids=lambda case: case.description, +) +def test_given_file_with_fault_when_checking_twice_then_fault_is_reported_both_times( + test_case: FormatCacheFaultTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_formatted_cache_project(project_dir=tmp_path) + products: Path = tmp_path / "models" / "products.sql" + _ = products.write_text( + products.read_text(encoding="utf-8").replace(test_case.removed_text, ""), + encoding="utf-8", + ) + _ = capsys.readouterr() + + first_exit, first_work = check_format_with_work(project_dir=tmp_path) + first: CaptureResult[str] = capsys.readouterr() + second_exit, second_work = check_format_with_work(project_dir=tmp_path) + second: CaptureResult[str] = capsys.readouterr() + + assert first_exit == second_exit == 1 + assert test_case.expected_code in first.out + assert second.out == first.out + assert first_work.get("format_cache_misses", 0) == 3 + assert second_work.get("format_cache_hits", 0) == test_case.expected_hits + assert second_work.get("format_cache_misses", 0) == test_case.expected_misses + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_format_writes.py b/tests/integration/src/sqlbuild/cli/commands/main/test_format_writes.py new file mode 100644 index 0000000000..8e9562a0be --- /dev/null +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_format_writes.py @@ -0,0 +1,82 @@ +"""Integration coverage for writing formatted files back in path order.""" + +from __future__ import annotations + +import os +import re +import stat +import sys +from pathlib import Path + +import pytest + +from sqlbuild._native import run_cli as main +from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( + FormatWriteFailureTestCase, +) + +_UNFORMATTED_SQL: str = ( + 'MODEL (description "Orders placed by customers.", materialized table);\n' + "select order_id, customer_id from __source('shop', 'orders')\n" +) + + +@pytest.mark.skipif( + sys.platform == "win32" or os.geteuid() == 0, + reason="needs a directory whose permissions the current user cannot override", +) +@pytest.mark.parametrize( + "test_case", + [ + FormatWriteFailureTestCase( + description="a failed first write leaves every later file as authored", + directories=("a_locked", "b_open"), + locked_directory="a_locked", + expected_replaced=(False, False), + expected_error=OSError, + expected_status="Formatting SQL ERROR", + ), + FormatWriteFailureTestCase( + description="files before a failed write are replaced and later ones are not", + directories=("a_open", "b_locked", "c_open", "d_open"), + locked_directory="b_locked", + expected_replaced=(True, False, False, False), + expected_error=OSError, + expected_status="Formatting SQL ERROR", + ), + ], + ids=lambda case: case.description, +) +def test_given_unwritable_file_when_formatting_then_later_files_are_left_as_authored( + test_case: FormatWriteFailureTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + files: list[Path] = [] + directory: str + for directory in test_case.directories: + (tmp_path / "models" / directory).mkdir(parents=True) + model: Path = tmp_path / "models" / directory / "orders.sql" + model.write_text(_UNFORMATTED_SQL, encoding="utf-8") + files.append(model) + locked: Path = tmp_path / "models" / test_case.locked_directory + locked.chmod(stat.S_IRUSR | stat.S_IXUSR) + try: + with pytest.raises(test_case.expected_error, match=re.escape(str(locked / "orders.sql"))): + _ = main(["--project-dir", str(tmp_path), "format", "--no-cache"]) + finally: + locked.chmod(stat.S_IRWXU) + + assert test_case.expected_status in capsys.readouterr().err + assert ( + tuple(path.read_text(encoding="utf-8") != _UNFORMATTED_SQL for path in files) + == test_case.expected_replaced + ) + assert [path.name for path in (tmp_path / "models").rglob(".*")] == [] + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/diagnostics/helpers.py b/tests/unit/src/sqlbuild/diagnostics/helpers.py index fe27b8b3d4..78a1e4a747 100644 --- a/tests/unit/src/sqlbuild/diagnostics/helpers.py +++ b/tests/unit/src/sqlbuild/diagnostics/helpers.py @@ -35,3 +35,16 @@ def spinner_threads() -> list[threading.Thread]: """The live progress spinner threads.""" return list(filter(lambda thread: thread.name == SPINNER_THREAD_NAME, threading.enumerate())) + + +def spinner_threads_started_after(earlier: list[threading.Thread]) -> list[threading.Thread]: + """The live spinner threads not in `earlier`, ignoring other tests' lingering threads.""" + + return list(filter(lambda thread: thread not in earlier, spinner_threads())) + + +def stopped_after_join(threads: list[threading.Thread], timeout_seconds: float) -> bool: + """Whether every thread in `threads` has finished within `timeout_seconds` each.""" + + list(map(lambda thread: thread.join(timeout=timeout_seconds), threads)) + return not any(map(lambda thread: thread.is_alive(), threads)) diff --git a/tests/unit/src/sqlbuild/diagnostics/test_main.py b/tests/unit/src/sqlbuild/diagnostics/test_main.py index 31f796adec..223953e34a 100644 --- a/tests/unit/src/sqlbuild/diagnostics/test_main.py +++ b/tests/unit/src/sqlbuild/diagnostics/test_main.py @@ -25,6 +25,8 @@ HostRecordingLogHandler, log_vendor_records, spinner_threads, + spinner_threads_started_after, + stopped_after_join, ) from tests.unit.src.sqlbuild.presentation.classes.helpers import RecordingStream @@ -161,18 +163,20 @@ def test_given_many_records_during_status_line_when_logging_then_one_spinner_thr stream: RecordingStream = RecordingStream(events=[], tty=True) monkeypatch.setattr(sys, "stderr", stream) reporter: TransientStatusReporter = TransientStatusReporter(stream=stream, use_color=False) + earlier: list[threading.Thread] = spinner_threads() seen: list[threading.Thread] = [] reporter.start("Preparing clone plan...") try: with configure_diagnostics(debug=True): for index in range(test_case.record_count): logging.getLogger("vendor.client").warning("retrying request %d", index) - seen.extend(spinner_threads()) + seen.extend(spinner_threads_started_after(earlier)) finally: reporter.close() - assert len({id(thread) for thread in seen}) == test_case.expected_spinner_threads - assert spinner_threads() == [] + started: list[threading.Thread] = list({id(thread): thread for thread in seen}.values()) + assert len(started) == test_case.expected_spinner_threads + assert stopped_after_join(started, timeout_seconds=5.0) assert stream.getvalue().count("retrying request") == test_case.record_count diff --git a/website/src/content/docs/docs/cli/format.mdx b/website/src/content/docs/docs/cli/format.mdx index 0cef3bee65..acfacb35da 100644 --- a/website/src/content/docs/docs/cli/format.mdx +++ b/website/src/content/docs/docs/cli/format.mdx @@ -17,6 +17,7 @@ sqb format [flags] | `--diff` | Print the proposed diff without writing | | `--fix` | Also apply compiler-verified, meaning-preserving Rule fixes before layout formatting | | `--json` | Print structured results | +| `--no-cache` | Format every file instead of skipping files a previous run found clean | | `--select`, `-s` | Format selected models | | `--select-file` | Read selectors from a file | | `--exclude` | Exclude models from the selected scope | @@ -30,6 +31,20 @@ sqb format --diff --select tag:marts Formatting owns presentation and source rewriting. Compiler-integrated diagnostics belong to configured [Rules](/docs/concepts/rules), which never mutate source. +Files are formatted in parallel; the report, the diff and the exit code are the same as a +one-file-at-a-time run. + +## Skipping unchanged files + +`sqb format` remembers every file it found already formatted and free of faults and warnings, in +`target/cache/compiler-native-v1/format-results.bin`. A later run skips such a file while its text, +its path, the `[format]` and rule threshold settings, the adapter dialect and the SQLBuild build are +all unchanged, so `sqb format --check` on an unchanged project costs little more than reading the +files. A file that needed changes or reported anything is always formatted again, and a damaged or +unreadable cache is ignored. `--fix` never uses the cache, and SQL test files are not skipped +while typed-null fixture formatting reads their models' contracts. Pass `--no-cache` to format +every file. + ## Line width and descriptions Configure the maximum physical line width in `sqlbuild_project.toml` (the default is `100`):