diff --git a/Cargo.lock b/Cargo.lock index 642536382..95340c60e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1235,6 +1235,12 @@ version = "0.15.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" +[[package]] +name = "dunce" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" + [[package]] name = "dyn-clone" version = "1.0.20" @@ -1348,7 +1354,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1400,6 +1406,38 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" +[[package]] +name = "fensu-facts" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cebbb33fa7353b9e6d12ad9c62481d9d672da2fd5e9d50429a968b4279d448b2" +dependencies = [ + "dunce", + "get-size2", + "globset", + "hex", + "rayon", + "ruff_python_ast", + "ruff_python_parser", + "ruff_text_size", + "sha2 0.10.9", + "walkdir", +] + +[[package]] +name = "fensu-native" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "385879149037e2b959b21e677c1cd8b302926f3d252b46c014d17cf88a9300cc" +dependencies = [ + "fensu-facts", + "globset", + "rayon", + "ruff_python_ast", + "serde_json", + "unicode-ident", +] + [[package]] name = "fensu-policy" version = "0.22.0" @@ -2203,7 +2241,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cde48f36530cbd91b65f050101e6c056d6eb743cea2a8f0f19beef69c6742ff8" dependencies = [ "scopeguard", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -3228,7 +3266,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3666,7 +3704,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4471,6 +4509,8 @@ name = "sqlbuild-rules" version = "0.0.0" dependencies = [ "blake3", + "fensu-facts", + "fensu-native", "fensu-policy", "globset", "rayon", @@ -4483,6 +4523,8 @@ dependencies = [ "sqlbuild-attachments", "sqlbuild-cache", "sqlbuild-core", + "sqlbuild-discovery", + "sqlbuild-pysource", "sqlbuild-scopes", "sqlbuild-sqltext", "sqlclean", @@ -5578,7 +5620,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c031931dd..870a5c559 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,10 @@ axum-server = { version = "0.8", default-features = false, features = ["tls-rust # Compresses the server's larger JSON reads and UI assets for browsers. tower-http = { version = "0.6", default-features = false, features = ["compression-gzip"] } fensu-policy = "=0.22.0" +# Fensu's Python facts and core rules without their Python bindings: the default `python` +# feature adds PyO3, rusqlite, flate2 and sha2. +fensu-facts = { version = "=0.30.0", default-features = false } +fensu-native = { version = "=0.30.0", default-features = false } futures-util = { version = "0.3", default-features = false, features = ["std"] } # `sqb` names compute log captures with random ids, as `uuid.uuid4().hex`. getrandom = "0.4" diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/audits/generic/expression_is_true.sql b/benchmarks/duckshop/edge_cases/python_rules/conforming/audits/generic/expression_is_true.sql new file mode 100644 index 000000000..075cd9181 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/audits/generic/expression_is_true.sql @@ -0,0 +1,5 @@ +AUDIT (); + +SELECT * +FROM @relation +WHERE NOT (@expression) diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/hooks/python/notify.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/hooks/python/notify.py new file mode 100644 index 000000000..de43aaf1b --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/hooks/python/notify.py @@ -0,0 +1,6 @@ +"""Note each finished build.""" + + +def note_build(ctx: object) -> None: + """Note one finished build.""" + print(ctx) diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/models/shipments.sql b/benchmarks/duckshop/edge_cases/python_rules/conforming/models/shipments.sql new file mode 100644 index 000000000..aa81ea371 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/models/shipments.sql @@ -0,0 +1,2 @@ +MODEL (description "Shipped orders.", audit_factories [shipment_quality]); +SELECT 1 AS shipment_id diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/main.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/main.py new file mode 100644 index 000000000..7ce3773fb --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/main.py @@ -0,0 +1,9 @@ +"""Write each shipped order as one carrier line.""" + +from python.utils.currency import amounts + + +def export_shipments(ctx: object) -> None: + """Write each shipped order as one carrier line.""" + label: str = "Expédition" + print(label, amounts.cents_text(1250), ctx) diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/node.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/node.py new file mode 100644 index 000000000..35836cc7a --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_export/node.py @@ -0,0 +1,10 @@ +"""Export shipped orders to the carrier.""" + +from python.fulfillment.shipment_export.main import export_shipments +from sqlbuild.tasks import task + + +@task +def shipment_export(ctx: object) -> None: + """Export shipped orders to the carrier.""" + export_shipments(ctx) diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_quality/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_quality/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_quality/factories.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_quality/factories.py new file mode 100644 index 000000000..c74d8d08f --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/fulfillment/shipment_quality/factories.py @@ -0,0 +1,15 @@ +"""Audit cases every shipment table shares.""" + +from sqlbuild.audits import AuditCase, audit_factory + + +@audit_factory +def shipment_quality() -> list[AuditCase]: + """Every shipment has a positive identifier.""" + return [ + AuditCase( + name="positive_shipment_id", + definition="expression_is_true", + arguments={"expression": "shipment_id > 0"}, + ) + ] diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/utils/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/conforming/python/utils/currency/amounts.py b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/utils/currency/amounts.py new file mode 100644 index 000000000..fe58c8af6 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/conforming/python/utils/currency/amounts.py @@ -0,0 +1,6 @@ +"""Format money for carrier files.""" + + +def cents_text(cents: int) -> str: + """Return cents as a decimal amount.""" + return f"{cents // 100}.{cents % 100:02d}" diff --git a/benchmarks/duckshop/edge_cases/python_rules/expected_findings.json b/benchmarks/duckshop/edge_cases/python_rules/expected_findings.json new file mode 100644 index 000000000..772449218 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/expected_findings.json @@ -0,0 +1,16 @@ +{ + "select": ["SQBRPYTHON", "SQBRPYTHON306"], + "findings": [ + ["SQBRPYTHON101", "hooks/python/alert.py", 4, 11], + ["SQBRPYTHON613", "python/inventory/__init__.py", 1, 1], + ["SQBRPYTHON002", "python/inventory/common/__init__.py", 1, 1], + ["SQBRPYTHON306", "python/inventory/common/stock.py", 4, 1], + ["SQBRPYTHON305", "python/inventory/main/publish.py", 1, 1], + ["SQBRPYTHON306", "python/inventory/main/publish.py", 4, 1], + ["SQBRPYTHON617", "python/inventory/restock/main.py", 1, 1], + ["SQBRPYTHON306", "python/inventory/restock/main.py", 4, 1], + ["SQBRPYTHON306", "python/inventory/restock/main.py", 9, 1], + ["SQBRPYTHON001", "python/report.py", 1, 1], + ["SQBRPYTHON614", "python/report.py", 1, 1] + ] +} diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/hooks/python/alert.py b/benchmarks/duckshop/edge_cases/python_rules/violations/hooks/python/alert.py new file mode 100644 index 000000000..7d088d5bd --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/violations/hooks/python/alert.py @@ -0,0 +1,6 @@ +"""Alert on failed builds.""" + + +def alert(ctx, reason: str) -> None: + """Alert on one failed build.""" + print(ctx, reason) diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/common/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/common/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/common/stock.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/common/stock.py new file mode 100644 index 000000000..84890dfa4 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/common/stock.py @@ -0,0 +1,6 @@ +"""Stock levels.""" + + +def stock_level(sku: str) -> int: + """Return the stock level of one product.""" + return len(sku) diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/main/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/main/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/main/publish.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/main/publish.py new file mode 100644 index 000000000..5d41c007d --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/main/publish.py @@ -0,0 +1,6 @@ +"""Publish stock levels.""" + + +def publish_levels() -> None: + """Publish stock levels.""" + print("levels") diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/restock/__init__.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/restock/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/restock/main.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/restock/main.py new file mode 100644 index 000000000..5d674d41a --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/violations/python/inventory/restock/main.py @@ -0,0 +1,11 @@ +"""Restock products.""" + + +def restock(sku: str) -> None: + """Restock one product.""" + print(sku) + + +def recount(sku: str) -> None: + """Recount one product.""" + print(sku) diff --git a/benchmarks/duckshop/edge_cases/python_rules/violations/python/report.py b/benchmarks/duckshop/edge_cases/python_rules/violations/python/report.py new file mode 100644 index 000000000..103fc50e7 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/python_rules/violations/python/report.py @@ -0,0 +1 @@ +"""A module directly under python/.""" diff --git a/crates/sqlbuild-cli/src/native_commands/_helpers/rules.rs b/crates/sqlbuild-cli/src/native_commands/_helpers/rules.rs index ef22baf41..75342e6e7 100644 --- a/crates/sqlbuild-cli/src/native_commands/_helpers/rules.rs +++ b/crates/sqlbuild-cli/src/native_commands/_helpers/rules.rs @@ -43,6 +43,13 @@ fn option_spec(option: &BuiltinOption) -> OptionSpec { OptionValue::Tuple(Vec::new()), Some(OptionKind::Other(DICT_TYPE_NAME.to_owned())), ), + BuiltinOptionKind::Integer { default, .. } => { + (OptionKind::Int, OptionValue::Int(i64::from(default)), None) + } + }; + let minimum: Option = match option.kind { + BuiltinOptionKind::Integer { minimum, .. } => Some(i64::from(minimum)), + _ => None, }; OptionSpec { name: option.name.to_owned(), @@ -50,7 +57,7 @@ fn option_spec(option: &BuiltinOption) -> OptionSpec { value_type, default, choices: Vec::new(), - minimum: None, + minimum, maximum: None, minimum_items: None, item_type, @@ -63,6 +70,7 @@ fn option_text(option: &BuiltinOption) -> OptionText { BuiltinOptionKind::Boolean => FALSE_REPR.to_owned(), BuiltinOptionKind::StringList(defaults) => string_tuple_repr(defaults), BuiltinOptionKind::TableList => EMPTY_TUPLE_REPR.to_owned(), + BuiltinOptionKind::Integer { default, .. } => default.to_string(), }; OptionText { name: option.name.to_owned(), diff --git a/crates/sqlbuild-discovery/src/python_paths/main/mod.rs b/crates/sqlbuild-discovery/src/python_paths/main/mod.rs index 0767e274b..07d5f14de 100644 --- a/crates/sqlbuild-discovery/src/python_paths/main/mod.rs +++ b/crates/sqlbuild-discovery/src/python_paths/main/mod.rs @@ -1 +1,2 @@ +pub mod supported_python_paths; pub mod unsupported_python_paths; diff --git a/crates/sqlbuild-discovery/src/python_paths/main/supported_python_paths.rs b/crates/sqlbuild-discovery/src/python_paths/main/supported_python_paths.rs new file mode 100644 index 000000000..3c4a75691 --- /dev/null +++ b/crates/sqlbuild-discovery/src/python_paths/main/supported_python_paths.rs @@ -0,0 +1,14 @@ +//! Project Python files under the documented executable roots. + +use std::path::Path; + +use crate::python_paths::_helpers::python_files::python_files; +use crate::python_paths::_helpers::supported::is_supported; + +/// Each supported Python file below `project_dir` as relative parts, sorted part by part. +pub fn supported_python_paths(project_dir: &Path) -> Vec> { + python_files(project_dir) + .into_iter() + .filter(|parts| is_supported(parts)) + .collect() +} diff --git a/crates/sqlbuild-discovery/src/python_paths/tests.rs b/crates/sqlbuild-discovery/src/python_paths/tests.rs index 66a9fa413..cf987b755 100644 --- a/crates/sqlbuild-discovery/src/python_paths/tests.rs +++ b/crates/sqlbuild-discovery/src/python_paths/tests.rs @@ -1,3 +1,5 @@ +#[path = "tests/test_supported_python_paths.rs"] +mod supported_python_paths; #[path = "tests/test_types.rs"] mod test_types; #[path = "tests/test_unsupported_python_paths.rs"] diff --git a/crates/sqlbuild-discovery/src/python_paths/tests/test_supported_python_paths.rs b/crates/sqlbuild-discovery/src/python_paths/tests/test_supported_python_paths.rs new file mode 100644 index 000000000..409d0c042 --- /dev/null +++ b/crates/sqlbuild-discovery/src/python_paths/tests/test_supported_python_paths.rs @@ -0,0 +1,52 @@ +use crate::python_paths::main::supported_python_paths::supported_python_paths; +use crate::python_paths::tests::test_types::SupportedPathsTestCase; + +#[test] +fn given_project_python_when_listing_supported_paths_then_only_documented_roots_remain() { + let test_cases = [ + SupportedPathsTestCase { + description: "documented roots sorted by parts, skipping hidden, generated and undocumented files", + files: &[ + "python/orders/main.py", + "adapter.py", + "orchestration/sensors/stock.py", + "functions/python/f.py", + "models/a/_sqlbuild/hooks/python/h.py", + "helpers.py", + "models/x.py", + ".hidden/x.py", + "target/x.py", + "python/__pycache__/main.py", + ], + expected_supported: &[ + "adapter.py", + "functions/python/f.py", + "models/a/_sqlbuild/hooks/python/h.py", + "orchestration/sensors/stock.py", + "python/orders/main.py", + ], + }, + SupportedPathsTestCase { + description: "a project without Python", + files: &[], + expected_supported: &[], + }, + ]; + for test_case in test_cases { + let directory = tempfile::tempdir().expect("temporary project"); + for file in test_case.files { + let path = directory.path().join(file); + std::fs::create_dir_all(path.parent().expect("parent")).expect("folders"); + std::fs::write(&path, "").expect("file"); + } + let supported: Vec = supported_python_paths(directory.path()) + .into_iter() + .map(|parts| parts.join("/")) + .collect(); + assert_eq!( + supported, test_case.expected_supported, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-discovery/src/python_paths/tests/test_types.rs b/crates/sqlbuild-discovery/src/python_paths/tests/test_types.rs index 65946a359..640c9b2ae 100644 --- a/crates/sqlbuild-discovery/src/python_paths/tests/test_types.rs +++ b/crates/sqlbuild-discovery/src/python_paths/tests/test_types.rs @@ -4,3 +4,10 @@ pub(super) struct PythonPathsTestCase { pub(super) files: &'static [&'static str], pub(super) expected_unsupported: &'static [&'static str], } + +/// One project layout and the Python files the Python rules read. +pub(super) struct SupportedPathsTestCase { + pub(super) description: &'static str, + pub(super) files: &'static [&'static str], + pub(super) expected_supported: &'static [&'static str], +} diff --git a/crates/sqlbuild-pysource/src/definition_locations/_helpers/decorators.rs b/crates/sqlbuild-pysource/src/definition_locations/_helpers/decorators.rs new file mode 100644 index 000000000..b75241f1b --- /dev/null +++ b/crates/sqlbuild-pysource/src/definition_locations/_helpers/decorators.rs @@ -0,0 +1,16 @@ +//! How a decorator expression is named. + +use ruff_python_ast::Expr; + +/// The decorator's last name segment, looking through a call: `udf`, `sqlbuild.udf` and `udf(...)` all name `udf`. +pub(crate) fn decorator_name(expression: &Expr) -> Option<&str> { + let target: &Expr = match expression { + Expr::Call(call) => &call.func, + _ => expression, + }; + match target { + Expr::Name(name) => Some(name.id.as_str()), + Expr::Attribute(attribute) => Some(attribute.attr.as_str()), + _ => None, + } +} diff --git a/crates/sqlbuild-pysource/src/definition_locations/_helpers/mod.rs b/crates/sqlbuild-pysource/src/definition_locations/_helpers/mod.rs new file mode 100644 index 000000000..6e708cdf4 --- /dev/null +++ b/crates/sqlbuild-pysource/src/definition_locations/_helpers/mod.rs @@ -0,0 +1 @@ +pub(crate) mod decorators; diff --git a/crates/sqlbuild-pysource/src/definition_locations/main/decorated_functions.rs b/crates/sqlbuild-pysource/src/definition_locations/main/decorated_functions.rs new file mode 100644 index 000000000..eb26415b2 --- /dev/null +++ b/crates/sqlbuild-pysource/src/definition_locations/main/decorated_functions.rs @@ -0,0 +1,38 @@ +//! The top-level functions of a module that carry one of the named decorators. + +use ruff_python_ast::{Stmt, StmtFunctionDef}; + +use crate::cpython_tree::main::parse_cpython_module::parse_cpython_module; +use crate::cpython_tree::models::ParsedSource; +use crate::definition_locations::_helpers::decorators::decorator_name; + +/// Names of top-level functions with a decorator whose last name segment is in `decorators`; none when the module does not parse. +#[must_use] +pub fn decorated_functions( + contents: &str, + decorators: &[&str], + python_version: (u8, u8), +) -> Vec { + let Some(parsed): Option = parse_cpython_module(contents, python_version) else { + return Vec::new(); + }; + parsed + .module + .body + .iter() + .filter_map(|statement| match statement { + Stmt::FunctionDef(function) => Some(function), + _ => None, + }) + .filter(|function| has_decorator(function, decorators)) + .map(|function| function.name.id.to_string()) + .collect() +} + +fn has_decorator(function: &StmtFunctionDef, decorators: &[&str]) -> bool { + function + .decorator_list + .iter() + .filter_map(|item| decorator_name(&item.expression)) + .any(|name| decorators.contains(&name)) +} diff --git a/crates/sqlbuild-pysource/src/definition_locations/main/decorator_location.rs b/crates/sqlbuild-pysource/src/definition_locations/main/decorator_location.rs index 384afa2f1..897470e6a 100644 --- a/crates/sqlbuild-pysource/src/definition_locations/main/decorator_location.rs +++ b/crates/sqlbuild-pysource/src/definition_locations/main/decorator_location.rs @@ -1,12 +1,13 @@ //! Where a named decorator of a function sits in a module source. -use ruff_python_ast::{Expr, Stmt}; +use ruff_python_ast::Stmt; use ruff_text_size::Ranged; use crate::cpython_tree::main::parse_cpython_module::parse_cpython_module; use crate::cpython_tree::main::source_position::source_position; use crate::cpython_tree::main::walk_breadth_first::walk_breadth_first; use crate::cpython_tree::models::{CpythonNode, ParsedSource}; +use crate::definition_locations::_helpers::decorators::decorator_name; use crate::definition_locations::constants::MISSING_DECORATOR_LOCATION; /// The `lineno` and byte `col_offset` of the first `@decorator` in `ast.walk` order, else `(1, 1)`. @@ -31,15 +32,3 @@ pub fn decorator_location( } MISSING_DECORATOR_LOCATION } - -fn decorator_name(expression: &Expr) -> Option<&str> { - let target: &Expr = match expression { - Expr::Call(call) => &call.func, - _ => expression, - }; - match target { - Expr::Name(name) => Some(name.id.as_str()), - Expr::Attribute(attribute) => Some(attribute.attr.as_str()), - _ => None, - } -} diff --git a/crates/sqlbuild-pysource/src/definition_locations/main/mod.rs b/crates/sqlbuild-pysource/src/definition_locations/main/mod.rs index 4cb8145dd..71dd5232e 100644 --- a/crates/sqlbuild-pysource/src/definition_locations/main/mod.rs +++ b/crates/sqlbuild-pysource/src/definition_locations/main/mod.rs @@ -1 +1,2 @@ +pub mod decorated_functions; pub mod decorator_location; diff --git a/crates/sqlbuild-pysource/src/definition_locations/mod.rs b/crates/sqlbuild-pysource/src/definition_locations/mod.rs index 9b926d1cd..37f9081f4 100644 --- a/crates/sqlbuild-pysource/src/definition_locations/mod.rs +++ b/crates/sqlbuild-pysource/src/definition_locations/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod _helpers; pub(crate) mod constants; pub mod main; diff --git a/crates/sqlbuild-pysource/src/definition_locations/tests.rs b/crates/sqlbuild-pysource/src/definition_locations/tests.rs index 6888c15fd..36ba3562c 100644 --- a/crates/sqlbuild-pysource/src/definition_locations/tests.rs +++ b/crates/sqlbuild-pysource/src/definition_locations/tests.rs @@ -1,3 +1,5 @@ +#[path = "tests/test_decorated_functions.rs"] +mod decorated_functions; #[path = "tests/test_decorator_location.rs"] mod decorator_location; #[path = "tests/test_types.rs"] diff --git a/crates/sqlbuild-pysource/src/definition_locations/tests/test_decorated_functions.rs b/crates/sqlbuild-pysource/src/definition_locations/tests/test_decorated_functions.rs new file mode 100644 index 000000000..ca64b4ae9 --- /dev/null +++ b/crates/sqlbuild-pysource/src/definition_locations/tests/test_decorated_functions.rs @@ -0,0 +1,46 @@ +use crate::definition_locations::main::decorated_functions::decorated_functions; +use crate::definition_locations::tests::test_types::DecoratedFunctionsTestCase; + +#[test] +fn given_module_when_listing_decorated_functions_then_only_top_level_functions_count() { + let test_cases = [ + DecoratedFunctionsTestCase { + description: "an attribute call decorator", + contents: "import sqlbuild\n\n@sqlbuild.loader(name=\"orders\")\ndef load():\n pass\n\n\ndef plain():\n pass\n", + expected_functions: &["load"], + }, + DecoratedFunctionsTestCase { + description: "a bare imported decorator among others", + contents: "@cache\n@task\ndef run():\n pass\n\n\n@asset\ndef build():\n pass\n", + expected_functions: &["run", "build"], + }, + DecoratedFunctionsTestCase { + description: "a decorated method is not a top-level function", + contents: "class Box:\n @task\n def run(self):\n pass\n", + expected_functions: &[], + }, + DecoratedFunctionsTestCase { + description: "a nested function is not top level", + contents: "def outer():\n @asset\n def inner():\n pass\n", + expected_functions: &[], + }, + DecoratedFunctionsTestCase { + description: "an unrelated decorator", + contents: "@udf\ndef add_one(value):\n return value + 1\n", + expected_functions: &[], + }, + DecoratedFunctionsTestCase { + description: "a syntax error", + contents: "@task\ndef run(:\n", + expected_functions: &[], + }, + ]; + for test_case in test_cases { + assert_eq!( + decorated_functions(test_case.contents, &["loader", "task", "asset"], (3, 12)), + test_case.expected_functions, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-pysource/src/definition_locations/tests/test_types.rs b/crates/sqlbuild-pysource/src/definition_locations/tests/test_types.rs index e8b055787..3e346fdbc 100644 --- a/crates/sqlbuild-pysource/src/definition_locations/tests/test_types.rs +++ b/crates/sqlbuild-pysource/src/definition_locations/tests/test_types.rs @@ -4,3 +4,9 @@ pub(super) struct DecoratorLocationTestCase { pub(super) python_version: (u8, u8), pub(super) expected_location: (usize, usize), } + +pub(super) struct DecoratedFunctionsTestCase { + pub(super) description: &'static str, + pub(super) contents: &'static str, + pub(super) expected_functions: &'static [&'static str], +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/builtin_catalogue.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/builtin_catalogue.rs index ad2a9d22e..bc17f398f 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/builtin_catalogue.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/builtin_catalogue.rs @@ -1,5 +1,6 @@ //! The built-in rule catalogue returned as the Python `Rule` models. +use pyo3::IntoPyObject; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; use pyo3::types::{PyBool, PyDict, PyDictMethods, PyString, PyTuple}; use sqlbuild_rules::models::RuleMetadata; @@ -50,18 +51,22 @@ fn option<'py>( )?; return new_record(py, RecordId::RuleOption, (), Some(&arguments)); } + BuiltinOptionKind::Integer { default, .. } => { + ("integer", default.into_pyobject(py)?.into_any()) + } }; - let arguments = keywords( - py, - vec![ - ("name", PyString::new(py, declared.name).into_any()), - ("default", default), - ( - "description", - PyString::new(py, declared.description).into_any(), - ), - ], - )?; + let mut fields: Vec<(&str, Bound<'py, PyAny>)> = vec![ + ("name", PyString::new(py, declared.name).into_any()), + ("default", default), + ( + "description", + PyString::new(py, declared.description).into_any(), + ), + ]; + if let BuiltinOptionKind::Integer { minimum, .. } = declared.kind { + fields.push(("minimum", minimum.into_pyobject(py)?.into_any())); + } + let arguments = keywords(py, fields)?; option_class.call_method(factory, (), Some(&arguments)) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs index 7edea1672..8f1c461ba 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/rows.rs @@ -202,6 +202,8 @@ fn request_header( header["scope_index"] = metadata_projection_value(&project_attribute(project, "scope_index")?)?; } + let version = py.version_info(); + header["python_version"] = json!([version.major, version.minor]); header["defer_suppressions"] = Value::Bool(true); header["custom_rules"] = plain_value(custom_payloads)?; header["rules_cache_path"] = Value::String(path_text( diff --git a/crates/sqlbuild-rules/Cargo.toml b/crates/sqlbuild-rules/Cargo.toml index eb4badd99..098bb9194 100644 --- a/crates/sqlbuild-rules/Cargo.toml +++ b/crates/sqlbuild-rules/Cargo.toml @@ -9,6 +9,8 @@ rust-version.workspace = true [dependencies] blake3.workspace = true +fensu-facts.workspace = true +fensu-native.workspace = true fensu-policy.workspace = true globset.workspace = true sqlquack-parse.workspace = true @@ -23,6 +25,8 @@ sqlbuild-analysis.workspace = true sqlbuild-attachments.workspace = true sqlbuild-cache.workspace = true sqlbuild-core.workspace = true +sqlbuild-discovery.workspace = true +sqlbuild-pysource.workspace = true sqlbuild-scopes.workspace = true sqlbuild-sqltext.workspace = true sqlclean.workspace = true diff --git a/crates/sqlbuild-rules/src/constants.rs b/crates/sqlbuild-rules/src/constants.rs index e3f4b5ad7..4a21d14f0 100644 --- a/crates/sqlbuild-rules/src/constants.rs +++ b/crates/sqlbuild-rules/src/constants.rs @@ -27,6 +27,8 @@ pub(crate) const SETTING_SNIPPET_INDENT: &str = " "; pub(crate) const NATIVE_ROWS_MEMO_VERSION: &str = "native-rules-rows-response-v2"; /// The namespace of the SQL Rules SQLClean owns, whose codes are `SQLCLEAN` and three digits. pub(crate) const SQL_RULE_PREFIX: &str = "SQLCLEAN"; +/// The Python rule family, which runs only when a selector names it or one of its codes. +pub(crate) const PYTHON_RULE_PREFIX: &str = "SQBRPYTHON"; /// Threshold key setting the fewest test cases a custom rule must ship. pub const MIN_CUSTOM_RULE_TEST_CASES_KEY: &str = "min_custom_rule_test_cases"; /// Rules whose findings name a joined relation, so an exception may cover just that relation. diff --git a/crates/sqlbuild-rules/src/engine/_helpers/cache.rs b/crates/sqlbuild-rules/src/engine/_helpers/cache.rs index 5bf79b7bc..ef0b2e665 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/cache.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/cache.rs @@ -1,12 +1,24 @@ use crate::models::Fault; +use crate::python_rules::models::PythonCacheEntry; +use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -#[derive(Clone, Debug, Default, Deserialize, Serialize)] -pub(crate) struct RuleCacheBucket { +/// Cached findings by path, valid for one ruleset fingerprint. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub(crate) struct RuleCacheBucket { pub fingerprint: String, - pub entries: BTreeMap, + pub entries: BTreeMap, +} + +impl Default for RuleCacheBucket { + fn default() -> Self { + Self { + fingerprint: String::new(), + entries: BTreeMap::new(), + } + } } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -16,6 +28,7 @@ pub(crate) struct RuleCacheEntry { } const FIXTURE_SUMMARIES_FILE: &str = "sql-test-fixtures.json"; +const PYTHON_RULES_FILE: &str = "python.json"; /// Where the SQL-test fixture summaries sit, beside the bulk native rules cache. pub(crate) fn fixture_summaries_path(project_dir: &Path, rules_cache_path: &str) -> PathBuf { @@ -61,31 +74,67 @@ impl Cache { &self, ruleset_identity: &str, bucket: RuleCacheBucket, - ) -> PendingBucketWrite { - PendingBucketWrite { - path: self.native_rules_path.clone(), - fingerprint: ruleset_identity.to_owned(), - bucket, - } + ) -> Result { + PendingBucketWrite::new(&self.native_rules_path, ruleset_identity, &bucket) + } + + /// The Python rules' findings by file, kept beside the bulk native rules cache. + pub(crate) fn python_rules_bucket( + &self, + ruleset_identity: &str, + ) -> Result, String> { + read_bucket(&self.python_rules_path(), ruleset_identity) + } + + /// The write of the Python rules' findings, held until it is published. + pub(crate) fn pending_python_rules_bucket( + &self, + ruleset_identity: &str, + bucket: &RuleCacheBucket, + ) -> Result { + PendingBucketWrite::new(&self.python_rules_path(), ruleset_identity, bucket) + } + + fn python_rules_path(&self) -> PathBuf { + self.native_rules_path.with_file_name(PYTHON_RULES_FILE) } } -/// One bulk rules cache write that has not been made yet. +/// Rules cache writes that have not been made yet, made in order when published. #[derive(Debug)] pub(crate) struct PendingBucketWrite { - path: PathBuf, - fingerprint: String, - bucket: RuleCacheBucket, + writes: Vec<(PathBuf, Vec)>, } impl PendingBucketWrite { - /// Make the write exactly as `Cache::put_native_rules_bucket` makes it. + fn new( + path: &Path, + fingerprint: &str, + bucket: &RuleCacheBucket, + ) -> Result { + Ok(Self { + writes: vec![(path.to_path_buf(), encoded_bucket(fingerprint, bucket)?)], + }) + } + + /// These writes followed by `later`'s. + pub(crate) fn then(mut self, later: Self) -> Self { + self.writes.extend(later.writes); + self + } + + /// Make the writes exactly as the immediate `put` methods make them. pub(crate) fn publish(&self) -> Result<(), String> { - write_bucket(&self.path, &self.fingerprint, &self.bucket) + self.writes + .iter() + .try_for_each(|(path, bytes)| write_bytes(path, bytes)) } } -fn read_bucket(path: &Path, fingerprint: &str) -> Result { +fn read_bucket( + path: &Path, + fingerprint: &str, +) -> Result, String> { let bytes = match std::fs::read(path) { Ok(value) => value, Err(error) if error.kind() == std::io::ErrorKind::NotFound => { @@ -96,7 +145,7 @@ fn read_bucket(path: &Path, fingerprint: &str) -> Result return Err(format!("could not read bulk rules cache: {error}")), }; - let bucket: RuleCacheBucket = match serde_json::from_slice(&bytes) { + let bucket: RuleCacheBucket = match serde_json::from_slice(&bytes) { Ok(value) => value, Err(error) if error.is_syntax() || error.is_eof() || error.is_data() => { RuleCacheBucket::default() @@ -114,16 +163,26 @@ fn read_bucket(path: &Path, fingerprint: &str) -> Result Result<(), String> { + write_bytes(path, &encoded_bucket(fingerprint, bucket)?) +} + +fn encoded_bucket( + fingerprint: &str, + bucket: &RuleCacheBucket, +) -> Result, String> { let mut output = bucket.clone(); output.fingerprint = fingerprint.to_owned(); + serde_json::to_vec(&output) + .map_err(|error| format!("could not encode bulk rules cache: {error}")) +} + +fn write_bytes(path: &Path, bytes: &[u8]) -> Result<(), String> { let parent = path .parent() .ok_or_else(|| "bulk rules cache path has no parent".to_owned())?; std::fs::create_dir_all(parent) .map_err(|error| format!("could not create bulk rules cache: {error}"))?; let temporary = path.with_extension(format!("tmp-{}", std::process::id())); - let bytes = serde_json::to_vec(&output) - .map_err(|error| format!("could not encode bulk rules cache: {error}"))?; std::fs::write(&temporary, bytes) .map_err(|error| format!("could not write bulk rules cache: {error}"))?; std::fs::rename(&temporary, path) diff --git a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs index d76c61a0c..e2ab69c28 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs @@ -1,11 +1,15 @@ use crate::configuration::main::validate as config; -use crate::constants::{API_VERSION, NATIVE_BUILD_IDENTITY, RELATION_EXCEPTION_RULES}; -use crate::engine::_helpers::cache::{Cache, PendingBucketWrite, RuleCacheEntry}; +use crate::constants::{ + API_VERSION, NATIVE_BUILD_IDENTITY, PYTHON_RULE_PREFIX, RELATION_EXCEPTION_RULES, +}; +use crate::engine::_helpers::cache::{Cache, PendingBucketWrite, RuleCacheBucket, RuleCacheEntry}; use crate::lint_text::constants::LINT_UNAVAILABLE_CODE; use crate::models::{ EvaluateRequest, EvaluateResponse, Fault, Model, RuleException, RuleMetadata, RuleScope, RulesCodeGrammar, RulesConfig, }; +use crate::python_rules::main::evaluate_python_rules::evaluate_python_rules; +use crate::python_rules::models::PythonEvaluation; use crate::rules::main::{ assemble_catalogue, evaluate as rules, evaluate_project, fingerprint, resolve_threshold_overrides, select, sql_test_coverage, @@ -101,18 +105,13 @@ fn evaluate_native_models( cache, model_rules_fingerprint, } = input; - if !context - .selected - .values() - .any(|rule| !rule.custom && !rule.code.starts_with("SQLCLEAN")) - { + if !context.selected.values().any(|rule| reads_models(rule)) { return Ok(NativeModelEvaluation::default()); } let mut models: Vec<_> = context.request.models.iter().collect(); models.sort_by(|left, right| left.relative_path.cmp(&right.relative_path)); let all_models_required = context.selected.values().any(|rule| { - !rule.custom - && !rule.code.starts_with("SQLCLEAN") + reads_models(rule) && !matches!( rule.code.as_str(), "SQBRDECLARATION201" | "SQBRDECLARATION301" | "SQBRTEST301" @@ -190,7 +189,7 @@ fn evaluate_native_models( None => cache.put_native_rules_bucket(model_rules_fingerprint, &bucket)?, Some(deferred) => { *deferred.borrow_mut() = - Some(cache.pending_native_rules_bucket(model_rules_fingerprint, bucket)); + Some(cache.pending_native_rules_bucket(model_rules_fingerprint, bucket)?); } } } @@ -284,6 +283,57 @@ fn parse_model_payloads( results.into_iter().collect() } +/// The Python rules' findings, reusing and then replacing their per-file cache when it is enabled. +fn evaluate_python_phase( + request: &EvaluateRequest, + selected: &[&RuleMetadata], + cache: Option<&Cache>, + control: &EvaluationControl<'_>, +) -> Result { + let python_rules: Vec<&RuleMetadata> = selected + .iter() + .copied() + .filter(|rule| rule.scope.evaluates_python()) + .collect(); + let fingerprint: String = format!( + "{}\0{}.{}", + fingerprint::fingerprint(&python_rules, &python_config(&request.config), "")?, + request.python_version.0, + request.python_version.1 + ); + let previous = cache + .map(|store| store.python_rules_bucket(&fingerprint)) + .transpose()? + .unwrap_or_default(); + let evaluation = native_rule_pool(request.python_sources.len().max(1))? + .install(|| evaluate_python_rules(request, selected, &previous.entries, &fingerprint))?; + let unchanged: bool = + evaluation.misses == 0 && evaluation.entries.len() == previous.entries.len(); + if let Some(store) = cache.filter(|_| !unchanged) { + let bucket = RuleCacheBucket { + fingerprint: fingerprint.clone(), + entries: evaluation.entries.clone(), + }; + let write: PendingBucketWrite = store.pending_python_rules_bucket(&fingerprint, &bucket)?; + match control.deferred_bucket { + None => write.publish()?, + Some(deferred) => { + let earlier: Option = deferred.borrow_mut().take(); + *deferred.borrow_mut() = Some(match earlier { + Some(earlier) => earlier.then(write), + None => write, + }); + } + } + } + Ok(evaluation) +} + +/// Built-in rules other than SQLClean and the Python rules, which read neither models nor the model cache. +fn reads_models(rule: &RuleMetadata) -> bool { + !rule.custom && !rule.code.starts_with("SQLCLEAN") && !rule.scope.evaluates_python() +} + fn native_rule_pool(items: usize) -> Result { rayon::ThreadPoolBuilder::new() .num_threads(NATIVE_RULE_WORKERS.min(items)) @@ -311,13 +361,20 @@ fn scoped_faults( Ok(faults) } -/// Fingerprint model-rule inputs; suppressions and graph-edge exceptions never reach model rules. +/// Fingerprint model-rule inputs; suppressions, graph-edge exceptions and Python-rule settings never reach model rules. fn model_rules_fingerprint( selected: &[&RuleMetadata], config: &RulesConfig, dialect: &str, ) -> Result { let model_config = RulesConfig { + select: config.select.as_deref().map(model_selectors), + rule_options: config + .rule_options + .iter() + .filter(|(code, _)| !code.starts_with(PYTHON_RULE_PREFIX)) + .map(|(code, options)| (code.clone(), options.clone())) + .collect(), rule_exceptions: Vec::new(), rule_ignores: Vec::new(), graph_edge_exceptions: Vec::new(), @@ -327,6 +384,28 @@ fn model_rules_fingerprint( fingerprint::fingerprint(selected, &model_config, dialect) } +/// The selectors outside the Python family, which alone can name model rules. +fn model_selectors(selectors: &[String]) -> Vec { + selectors + .iter() + .filter(|selector| !selector.starts_with(PYTHON_RULE_PREFIX)) + .cloned() + .collect() +} + +/// The only configuration the Python rules read: their own options. +fn python_config(config: &RulesConfig) -> RulesConfig { + RulesConfig { + rule_options: config + .rule_options + .iter() + .filter(|(code, _)| code.starts_with(PYTHON_RULE_PREFIX)) + .map(|(code, options)| (code.clone(), options.clone())) + .collect(), + ..RulesConfig::default() + } +} + #[cfg(test)] pub(crate) fn evaluate_request(request: EvaluateRequest) -> Result { evaluate_request_with(request, &EvaluationControl::immediate()) @@ -407,7 +486,14 @@ fn evaluated_request( let cache_enabled = request.config.cache.enabled; let model_rules_fingerprint = cache_enabled - .then(|| model_rules_fingerprint(&selected, &request.config, &request.dialect)) + .then(|| { + let model_rules: Vec<&RuleMetadata> = selected + .iter() + .copied() + .filter(|rule| !rule.scope.evaluates_python()) + .collect(); + model_rules_fingerprint(&model_rules, &request.config, &request.dialect) + }) .transpose()? .unwrap_or_default(); let cache = cache_enabled @@ -440,9 +526,22 @@ fn evaluated_request( }, control, )?; - let cache_hits = native.hits; - let cache_misses = native.misses; + let mut cache_hits = native.hits; + let mut cache_misses = native.misses; raw_faults.extend(native.faults); + if selected.iter().any(|rule| rule.scope.evaluates_python()) { + let python = evaluate_python_phase(&request, &selected, cache.as_ref(), control)?; + raw_faults.extend(scoped_faults( + &selected_by_code, + python.faults, + RuleScope::evaluates_python, + )?); + cache_hits += python.hits; + cache_misses += python.misses; + if control.cancelled() { + return Err(CANCELLED.to_owned()); + } + } let built_in_ms = built_in_started.elapsed().as_millis() as u64; validate_exception_paths(&request)?; let faults = if request.defer_suppressions { diff --git a/crates/sqlbuild-rules/src/engine/_helpers/memo.rs b/crates/sqlbuild-rules/src/engine/_helpers/memo.rs index d6e217a14..c119b34c6 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/memo.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/memo.rs @@ -12,6 +12,7 @@ use crate::engine::_helpers::evaluation::{EvaluationControl, evaluate_request_wi use crate::engine::_helpers::retained::encoded_rows; use crate::errors::RowsError; use crate::models::{EvaluateRequest, Model, RetainedRows, RowsEvaluation, RowsRequest}; +use crate::python_rules::main::python_sources::python_sources; /// The shared key prefix of fields a request serializes but evaluation derives again. const DERIVED_FIELD_PREFIX: &str = ",\"empty_input_only"; @@ -57,6 +58,7 @@ pub(crate) fn prepare(rows: RowsRequest, keyed: bool) -> Result>()?; + request.python_sources = python_sources(&request).map_err(RowsError::Rules)?; let mut request_json: Option> = None; let memo: Option<(String, PathBuf)> = match memo { Some((code_identity, path)) => { diff --git a/crates/sqlbuild-rules/src/engine/tests/helpers.rs b/crates/sqlbuild-rules/src/engine/tests/helpers.rs index b0c48450d..798aa9f1c 100644 --- a/crates/sqlbuild-rules/src/engine/tests/helpers.rs +++ b/crates/sqlbuild-rules/src/engine/tests/helpers.rs @@ -521,6 +521,12 @@ pub(crate) fn cached_paths(project_dir: &TempDir) -> Result, String> const SPECULATIVE_CODE_IDENTITY: &str = "speculative-rows-test-build"; pub(crate) const SPECULATIVE_CACHE_FILE: &str = "target/rules-cache/bulk/native.json"; +pub(crate) const SPECULATIVE_PYTHON_CACHE_FILE: &str = "target/rules-cache/bulk/python.json"; +/// A project Python file with an unannotated parameter, so the Python rules write their cache. +const SPECULATIVE_PYTHON_FILE: (&str, &str) = ( + "hooks/python/notify.py", + "def send(count) -> None:\n pass\n", +); const SPECULATIVE_MEMO_FILE: &str = "target/rules-cache/bulk/native-response.json"; pub(crate) fn speculative_rows_request( @@ -528,6 +534,11 @@ pub(crate) fn speculative_rows_request( cache_enabled: bool, models: &[Value], ) -> Result { + let (python_path, python_text) = SPECULATIVE_PYTHON_FILE; + let python_file: PathBuf = project_dir.join(python_path); + std::fs::create_dir_all(python_file.parent().unwrap_or(project_dir)) + .map_err(|error| error.to_string())?; + std::fs::write(&python_file, python_text).map_err(|error| error.to_string())?; let mut request: Value = speculative_request_json(project_dir, cache_enabled); request["models"] = Value::Array(models.to_vec()); let request: EvaluateRequest = @@ -549,7 +560,7 @@ fn speculative_request_json(project_dir: &Path, cache_enabled: bool) -> Value { "version": 1, "project_dir": project_dir, "dialect": "duckdb", - "config": {"select": ["SQBR"], "domains": ["commerce"], "cache": {"enabled": cache_enabled}}, + "config": {"select": ["SQBR", "SQBRPYTHON101"], "domains": ["commerce"], "cache": {"enabled": cache_enabled}}, "models": [], "defer_suppressions": true }) @@ -577,11 +588,15 @@ pub(crate) fn evaluation_summary( } pub(crate) fn written_rules_files(project_dir: &Path) -> Vec { - [SPECULATIVE_CACHE_FILE, SPECULATIVE_MEMO_FILE] - .iter() - .map(|file| project_dir.join(file)) - .filter(|path| path.is_file()) - .collect() + [ + SPECULATIVE_CACHE_FILE, + SPECULATIVE_PYTHON_CACHE_FILE, + SPECULATIVE_MEMO_FILE, + ] + .iter() + .map(|file| project_dir.join(file)) + .filter(|path| path.is_file()) + .collect() } /// Discard a speculation after it ran a while; it never reports stopping. diff --git a/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs b/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs index 2fa5b4290..17946eff4 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs @@ -46,6 +46,70 @@ const REVIEWED_SCOPES: &[(&str, RuleScope)] = &[ ("SQBRPROJECT202", RuleScope::Project), ("SQBRPROJECT203", RuleScope::Project), ("SQBRPROJECT204", RuleScope::Project), + ("SQBRPYTHON001", RuleScope::Python), + ("SQBRPYTHON002", RuleScope::Python), + ("SQBRPYTHON101", RuleScope::Python), + ("SQBRPYTHON102", RuleScope::Python), + ("SQBRPYTHON103", RuleScope::Python), + ("SQBRPYTHON104", RuleScope::Python), + ("SQBRPYTHON105", RuleScope::Python), + ("SQBRPYTHON201", RuleScope::Python), + ("SQBRPYTHON202", RuleScope::Python), + ("SQBRPYTHON203", RuleScope::Python), + ("SQBRPYTHON204", RuleScope::Python), + ("SQBRPYTHON205", RuleScope::Python), + ("SQBRPYTHON206", RuleScope::Python), + ("SQBRPYTHON207", RuleScope::Python), + ("SQBRPYTHON208", RuleScope::Python), + ("SQBRPYTHON301", RuleScope::Python), + ("SQBRPYTHON302", RuleScope::Python), + ("SQBRPYTHON303", RuleScope::Python), + ("SQBRPYTHON304", RuleScope::Python), + ("SQBRPYTHON305", RuleScope::Python), + ("SQBRPYTHON306", RuleScope::Python), + ("SQBRPYTHON307", RuleScope::Python), + ("SQBRPYTHON401", RuleScope::Python), + ("SQBRPYTHON402", RuleScope::Python), + ("SQBRPYTHON403", RuleScope::Python), + ("SQBRPYTHON404", RuleScope::Python), + ("SQBRPYTHON501", RuleScope::Python), + ("SQBRPYTHON502", RuleScope::Python), + ("SQBRPYTHON503", RuleScope::Python), + ("SQBRPYTHON504", RuleScope::Python), + ("SQBRPYTHON505", RuleScope::Python), + ("SQBRPYTHON506", RuleScope::Python), + ("SQBRPYTHON507", RuleScope::Python), + ("SQBRPYTHON508", RuleScope::Python), + ("SQBRPYTHON509", RuleScope::Python), + ("SQBRPYTHON510", RuleScope::Python), + ("SQBRPYTHON511", RuleScope::Python), + ("SQBRPYTHON601", RuleScope::Python), + ("SQBRPYTHON602", RuleScope::Python), + ("SQBRPYTHON603", RuleScope::Python), + ("SQBRPYTHON604", RuleScope::Python), + ("SQBRPYTHON605", RuleScope::Python), + ("SQBRPYTHON606", RuleScope::Python), + ("SQBRPYTHON607", RuleScope::Python), + ("SQBRPYTHON608", RuleScope::Python), + ("SQBRPYTHON609", RuleScope::Python), + ("SQBRPYTHON610", RuleScope::Python), + ("SQBRPYTHON611", RuleScope::Python), + ("SQBRPYTHON612", RuleScope::Python), + ("SQBRPYTHON613", RuleScope::Python), + ("SQBRPYTHON614", RuleScope::Python), + ("SQBRPYTHON615", RuleScope::Python), + ("SQBRPYTHON616", RuleScope::Python), + ("SQBRPYTHON617", RuleScope::Python), + ("SQBRPYTHON618", RuleScope::Python), + ("SQBRPYTHON619", RuleScope::Python), + ("SQBRPYTHON620", RuleScope::Python), + ("SQBRPYTHON621", RuleScope::Python), + ("SQBRPYTHON622", RuleScope::Python), + ("SQBRPYTHON623", RuleScope::Python), + ("SQBRPYTHON624", RuleScope::Python), + ("SQBRPYTHON625", RuleScope::Python), + ("SQBRPYTHON626", RuleScope::Python), + ("SQBRPYTHON627", RuleScope::Python), ("SQBRTEST101", RuleScope::Project), ("SQBRTEST102", RuleScope::Project), ("SQBRTEST103", RuleScope::Project), diff --git a/crates/sqlbuild-rules/src/engine/tests/test_speculative_rows.rs b/crates/sqlbuild-rules/src/engine/tests/test_speculative_rows.rs index f8c2c4793..9c73e1f4f 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_speculative_rows.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_speculative_rows.rs @@ -9,8 +9,9 @@ use crate::engine::main::answer_speculative_rows::answer_speculative_rows; use crate::engine::main::evaluate_rows::evaluate_rows; use crate::engine::main::start_speculative_rows::start_speculative_rows; use crate::engine::tests::helpers::{ - SPECULATIVE_CACHE_FILE, answer_after_stop, changed_orders_models, discard_after_running, - drop_at_once, evaluation_summary, orders_models, speculative_rows_request, written_rules_files, + SPECULATIVE_CACHE_FILE, SPECULATIVE_PYTHON_CACHE_FILE, answer_after_stop, + changed_orders_models, discard_after_running, drop_at_once, evaluation_summary, orders_models, + speculative_rows_request, written_rules_files, }; use crate::engine::tests::test_types::{ SpeculativeOutcomeTestCase, SpeculativeRowsTestCase, SpeculativeStopTestCase, @@ -68,7 +69,10 @@ fn given_speculative_rows_when_answering_then_result_and_writes_match_ordinary_e return Err(format!("{}: answer stopped", test_case.description)); }; let evaluation = evaluation.map_err(|_| "speculative answer failed".to_owned())?; - let cache = |dir: &Path| std::fs::read(dir.join(SPECULATIVE_CACHE_FILE)).ok(); + let cache = |dir: &Path| { + [SPECULATIVE_CACHE_FILE, SPECULATIVE_PYTHON_CACHE_FILE] + .map(|file| std::fs::read(dir.join(file)).ok()) + }; assert_eq!(used, test_case.expected_used, "{}", test_case.description); assert_eq!( evaluation_summary(&evaluation), diff --git a/crates/sqlbuild-rules/src/lib.rs b/crates/sqlbuild-rules/src/lib.rs index 7f34fa03b..c74ed8db1 100644 --- a/crates/sqlbuild-rules/src/lib.rs +++ b/crates/sqlbuild-rules/src/lib.rs @@ -13,6 +13,7 @@ pub mod fix_verification; pub mod lint_text; pub mod models; pub mod project_tree; +pub mod python_rules; pub mod query_facts; pub mod relation_keys; pub mod rule_cache; diff --git a/crates/sqlbuild-rules/src/models.rs b/crates/sqlbuild-rules/src/models.rs index 099bcd3ae..4a9f740f2 100644 --- a/crates/sqlbuild-rules/src/models.rs +++ b/crates/sqlbuild-rules/src/models.rs @@ -202,6 +202,12 @@ impl RulesConfig { .and_then(|options| options.get(option)) } + pub(crate) fn rule_option_u32(&self, code: &str, option: &str) -> Option { + self.rule_option(code, option) + .and_then(Value::as_u64) + .map(|value| u32::try_from(value).unwrap_or(u32::MAX)) + } + pub(crate) fn rule_option_enabled(&self, code: &str, option: &str) -> bool { self.rule_option(code, option) .and_then(Value::as_bool) @@ -762,6 +768,21 @@ pub struct EvaluateRequest { pub defer_suppressions: bool, pub custom_rules: Vec, pub rules_cache_path: String, + /// The interpreter's major and minor version, which decides how project Python parses. + pub python_version: (u8, u8), + /// The supported project Python files, filled from disk only when a Python rule is selected. + pub python_sources: Vec, +} + +/// One project Python file, keyed in the request by the digest of its bytes. +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct PythonSource { + pub relative_path: String, + pub digest: String, + /// The text that was digested; it never enters the request JSON or the memo. + #[serde(skip)] + pub text: String, } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -799,6 +820,8 @@ impl Default for EvaluateRequest { defer_suppressions: false, custom_rules: vec![], rules_cache_path: String::new(), + python_version: (3, 12), + python_sources: vec![], } } } @@ -839,6 +862,8 @@ pub enum RuleScope { Project, ModelAndProject, SqlLint, + /// Project Python files, evaluated per file outside the model phases. + Python, } impl RuleScope { @@ -849,6 +874,10 @@ impl RuleScope { pub(crate) fn evaluates_project(self) -> bool { matches!(self, Self::Project | Self::ModelAndProject) } + + pub(crate) fn evaluates_python(self) -> bool { + matches!(self, Self::Python) + } } #[derive(Clone, Debug, Serialize)] diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/contexts.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/contexts.rs new file mode 100644 index 000000000..23e2d3a0e --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/contexts.rs @@ -0,0 +1,143 @@ +//! The context Fensu's rules read for one file, built the way Fensu builds it for its own runs. + +use std::collections::HashMap; + +use fensu_native::rules::constants::DEAD_DEFINITION_CODE; +use fensu_native::rules::constants::{ + TOO_MANY_DISTINCT_CALLS_CODE, TOO_MANY_LOCALS_CODE, TOO_MANY_STATEMENTS_CODE, +}; +use fensu_native::{NativeDeadCodeContext, NativeRuleContext}; +use std::path::Path; + +use crate::models::{EvaluateRequest, RulesConfig}; +use crate::python_rules::constants::{ + NAMING_CONTRACTS, NAMING_FAMILY, PACKAGE_ROOT, PROJECT_CONFIG_FILE, ROOT_SCOPE, +}; +use crate::python_rules::models::{PythonRuleSpec, PythonTarget}; + +/// Fensu passes these thresholds only to main modules. +const MAIN_ONLY_THRESHOLD_CODES: [&str; 3] = [ + TOO_MANY_STATEMENTS_CODE, + TOO_MANY_DISTINCT_CALLS_CODE, + TOO_MANY_LOCALS_CODE, +]; + +/// What every file's context shares. +#[derive(Debug)] +pub(crate) struct ContextSettings { + pub repo_root: String, + /// The runtime package folder, where domains begin. + pub ownership_root: String, + pub contracts: Vec<(String, String)>, + pub dead_code: NativeDeadCodeContext, +} + +/// The shared context of a run; `entrypoints` are the definitions SQLBuild loads, for reachability. +pub(crate) fn context_settings( + request: &EvaluateRequest, + rules: &[&PythonRuleSpec], + entrypoints: Vec<(String, String)>, +) -> ContextSettings { + let contracts: Vec<(String, String)> = if rules.iter().any(|rule| rule.family == NAMING_FAMILY) + { + NAMING_CONTRACTS + .iter() + .map(|(pattern, contract)| ((*pattern).to_owned(), (*contract).to_owned())) + .collect() + } else { + Vec::new() + }; + ContextSettings { + repo_root: request.project_dir.clone(), + ownership_root: Path::new(&request.project_dir) + .join(PACKAGE_ROOT) + .to_string_lossy() + .into_owned(), + contracts, + dead_code: NativeDeadCodeContext { + enabled: rules + .iter() + .any(|rule| rule.fensu_code == Some(DEAD_DEFINITION_CODE)), + roots: Vec::new(), + entrypoints, + config_path: PROJECT_CONFIG_FILE.to_owned(), + }, + } +} + +/// The context of `target`, without thresholds, observations or the dead-code context. +pub(crate) fn rule_context(target: &PythonTarget, settings: &ContextSettings) -> NativeRuleContext { + let shared = NativeRuleContext { + scope: ROOT_SCOPE.to_owned(), + repository_path: target.repository_path.clone(), + relative_parts: target.relative_parts.clone(), + contracts: settings.contracts.clone(), + repo_root: settings.repo_root.clone(), + scope_roots: vec![(ROOT_SCOPE.to_owned(), target.root.clone())], + ..NativeRuleContext::default() + }; + if !target.package { + return NativeRuleContext { + package_name: target + .root + .rsplit('/') + .next() + .unwrap_or_default() + .to_owned(), + ownership_offset: None, + ..shared + }; + } + NativeRuleContext { + role: target.role.clone(), + is_main_module: target.is_main_module, + is_entry_module: target.is_entry_module, + package_name: PACKAGE_ROOT.to_owned(), + ownership_root: Some(settings.ownership_root.clone()), + ownership_roots: vec![settings.ownership_root.clone()], + ownership_offset: Some(0), + ..shared + } +} + +/// The file's rules split into calls whose thresholds agree, each as Fensu codes and thresholds. +pub(crate) fn threshold_calls( + rules: &[&PythonRuleSpec], + config: &RulesConfig, + is_main_module: bool, +) -> Vec<(Vec, HashMap)> { + let mut calls: Vec<(Vec, HashMap)> = Vec::new(); + for rule in rules { + let Some(fensu_code) = rule.fensu_code else { + continue; + }; + let thresholds: Vec<(String, u32)> = if !is_main_module + && MAIN_ONLY_THRESHOLD_CODES.contains(&fensu_code) + { + Vec::new() + } else { + rule.options + .iter() + .map(|(name, default, _)| { + let value: u32 = config.rule_option_u32(rule.code, name).unwrap_or(*default); + ((*name).to_owned(), value) + }) + .collect() + }; + let agrees = |current: &HashMap| { + thresholds + .iter() + .all(|(name, value)| current.get(name).is_none_or(|existing| existing == value)) + }; + let index: usize = match calls.iter().position(|(_, current)| agrees(current)) { + Some(index) => index, + None => { + calls.push((Vec::new(), HashMap::new())); + calls.len() - 1 + } + }; + calls[index].0.push(fensu_code.to_owned()); + calls[index].1.extend(thresholds); + } + calls +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/execution.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/execution.rs new file mode 100644 index 000000000..ca6e48483 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/execution.rs @@ -0,0 +1,300 @@ +//! Which file runs which rules, and each file's findings, from the cache when its answers still hold. + +use std::collections::{BTreeMap, HashSet}; +use std::path::Path; + +use fensu_facts::snapshot::models::RepositoryObservationIndex; +use fensu_native::rules::constants::DEAD_DEFINITION_CODE; +use fensu_native::{ + NativeDeadCodeContext, NativeExecutionRule, NativeExecutionTarget, NativeProjectPlane, + NativeProjectQuery, NativeRuleContext, evaluate_core_rules, plan_core_rule_queries, + plan_execution_owners, +}; +use rayon::iter::{IntoParallelIterator, ParallelIterator}; + +use crate::models::{Fault, RuleMetadata}; +use crate::python_rules::_helpers::contexts::{rule_context, threshold_calls}; +use crate::python_rules::_helpers::faults::python_fault; +use crate::python_rules::_helpers::file_cache::{answers_digest, file_identity}; +use crate::python_rules::_helpers::observations::{ + observations, relative_queries, repository_index, +}; +use crate::python_rules::constants::{PACKAGE_ROOT, PYTHON_RULES, ROOT_SCOPE}; +use crate::python_rules::models::{ + CacheSplit, CallInputs, FilePlan, PythonCacheEntry, PythonEvaluation, PythonFiles, + PythonRuleSpec, PythonTarget, RuleCall, +}; + +/// The selected rules that Fensu evaluates. +pub(crate) fn selected_rules(selected: &[&RuleMetadata]) -> Vec<&'static PythonRuleSpec> { + let codes: HashSet<&str> = selected.iter().map(|rule| rule.code.as_str()).collect(); + PYTHON_RULES + .iter() + .filter(|rule| rule.fensu_code.is_some() && codes.contains(rule.code)) + .collect() +} + +/// Each file's calls and cache identity, worked out before any file is parsed. +pub(crate) fn file_plans<'a>( + targets: &'a [PythonTarget], + rules: &[&PythonRuleSpec], + inputs: &CallInputs<'_>, +) -> Result>, String> { + Ok(targets + .iter() + .zip(file_rules(targets, rules)?) + .filter(|(_, rules)| !rules.is_empty()) + .map(|(target, rules)| { + let calls = threshold_calls(&rules, &inputs.request.config, target.is_main_module); + FilePlan { + target, + identity: file_identity(target, &calls, inputs), + calls, + } + }) + .collect()) +} + +/// Every planned file's findings: cached ones whose project answers still hold, the rest evaluated. +pub(crate) fn evaluated_files( + plans: Vec>, + inputs: &CallInputs<'_>, + plane: &NativeProjectPlane, +) -> Result { + let split: CacheSplit<'_> = cached_files(plans, inputs)?; + let hits: usize = split.fresh.len(); + let misses: usize = split.stale.len(); + let mut entries: BTreeMap = split.fresh.into_iter().collect(); + entries.extend(evaluate_files(split.stale, inputs, plane)?); + Ok(PythonEvaluation { + faults: entries + .values() + .flat_map(|entry| entry.faults.iter().cloned()) + .collect(), + entries, + hits, + misses, + }) +} + +/// Split `plans` into cached entries that still hold and files to evaluate again. +fn cached_files<'a>( + plans: Vec>, + inputs: &CallInputs<'_>, +) -> Result, String> { + let cached = |plan: &FilePlan<'_>| { + inputs + .cached + .get(&plan.target.repository_path) + .filter(|entry| entry.identity == plan.identity) + }; + let queries: Vec = plans + .iter() + .filter_map(cached) + .flat_map(|entry| entry.queries.iter().map(project_query)) + .collect(); + let index: Option = (!queries.is_empty()) + .then(|| { + let borrowed: Vec<&NativeProjectQuery> = queries.iter().collect(); + repository_index(Path::new(&inputs.request.project_dir), &borrowed) + }) + .transpose()?; + let mut split: CacheSplit<'a> = CacheSplit::default(); + for plan in plans { + match cached(&plan) { + Some(entry) if still_holds(entry, inputs, index.as_ref())? => { + split + .fresh + .push((plan.target.repository_path.clone(), entry.clone())); + } + _ => split.stale.push(plan), + } + } + Ok(split) +} + +/// Whether `entry`'s recorded project questions still have the answers it was computed from. +fn still_holds( + entry: &PythonCacheEntry, + inputs: &CallInputs<'_>, + index: Option<&RepositoryObservationIndex>, +) -> Result { + let queries: Vec = entry.queries.iter().map(project_query).collect(); + let answers: BTreeMap> = match index { + Some(index) => observations(&queries, inputs.programs, index)? + .into_iter() + .collect(), + None => BTreeMap::new(), + }; + Ok(answers_digest(&answers) == entry.answers) +} + +/// Parse and evaluate `plans`, answering every file's project questions from one walk. +fn evaluate_files( + plans: Vec>, + inputs: &CallInputs<'_>, + plane: &NativeProjectPlane, +) -> Result, String> { + if plans.is_empty() { + return Ok(Vec::new()); + } + let files: Vec<(FilePlan<'_>, Vec)> = plans + .into_par_iter() + .map(|plan| { + let calls = file_calls(&plan, inputs)?; + Ok((plan, calls)) + }) + .collect::>()?; + let mut queries: Vec<&NativeProjectQuery> = Vec::new(); + for call in files.iter().flat_map(|(_, calls)| calls) { + queries.extend(call.queries.iter()); + } + let index = repository_index(Path::new(&inputs.request.project_dir), &queries)?; + files + .into_par_iter() + .map(|file| file_entry(file, inputs, &index, plane)) + .collect() +} + +/// One file's findings and the cache entry that records what they were computed from. +fn file_entry( + (plan, calls): (FilePlan<'_>, Vec), + inputs: &CallInputs<'_>, + index: &RepositoryObservationIndex, + plane: &NativeProjectPlane, +) -> Result<(String, PythonCacheEntry), String> { + let mut answers: BTreeMap> = BTreeMap::new(); + let mut queries: Vec<(String, String, String)> = Vec::new(); + let mut faults: Vec = Vec::new(); + for call in calls { + let observed = observations(&call.queries, inputs.programs, index)?; + answers.extend( + observed + .iter() + .map(|(key, value)| (key.clone(), value.clone())), + ); + queries.extend(call.queries.iter().map(|query| { + ( + query.kind.clone(), + query.path.clone(), + query.argument.clone(), + ) + })); + let context = NativeRuleContext { + observations: observed, + ..call.context + }; + let program = inputs.programs.program(plan.target.source)?; + for row in evaluate_core_rules(program, &call.codes, &context, plane)? { + faults.extend(python_fault(row, plan.target, inputs.programs)?); + } + } + queries.sort(); + queries.dedup(); + Ok(( + plan.target.repository_path.clone(), + PythonCacheEntry { + identity: plan.identity, + queries, + answers: answers_digest(&answers), + faults, + }, + )) +} + +/// The file's calls with their contexts and project questions; parses the file. +fn file_calls<'a>(plan: &FilePlan<'a>, inputs: &CallInputs<'_>) -> Result, String> { + let program = inputs.programs.program(plan.target.source)?; + let base: NativeRuleContext = rule_context(plan.target, &inputs.settings); + Ok(plan + .calls + .iter() + .map(|(codes, thresholds)| { + let dead_code: NativeDeadCodeContext = dead_code_context(codes, inputs); + let context = NativeRuleContext { + thresholds: thresholds.clone(), + dead_code, + ..base.clone() + }; + RuleCall { + queries: relative_queries( + plan_core_rule_queries(program, codes, &context), + &inputs.request.project_dir, + ), + codes: codes.clone(), + context, + } + }) + .collect()) +} + +/// The loaded definitions for a call running the dead-code rule; other calls skip copying them per file. +fn dead_code_context(codes: &[String], inputs: &CallInputs<'_>) -> NativeDeadCodeContext { + if codes.iter().any(|code| code == DEAD_DEFINITION_CODE) { + inputs.settings.dead_code.clone() + } else { + NativeDeadCodeContext::default() + } +} + +fn project_query((kind, path, argument): &(String, String, String)) -> NativeProjectQuery { + NativeProjectQuery { + kind: kind.clone(), + path: path.clone(), + argument: argument.clone(), + } +} + +/// Each file's rules; project-, scope- and package-wide rules run once, on the file Fensu picks. +fn file_rules<'a>( + targets: &[PythonTarget], + rules: &[&'a PythonRuleSpec], +) -> Result>, String> { + let mut by_file: Vec> = vec![Vec::new(); targets.len()]; + for package in [true, false] { + let indexes: Vec = (0..targets.len()) + .filter(|index| targets[*index].package == package) + .collect(); + let applicable: Vec<&PythonRuleSpec> = rules + .iter() + .copied() + .filter(|rule| package || rule.applies == PythonFiles::All) + .collect(); + let execution_targets: Vec = indexes + .iter() + .map(|index| execution_target(&targets[*index])) + .collect(); + let execution_rules: Vec = + applicable.iter().map(|rule| execution_rule(rule)).collect(); + let plans = plan_execution_owners(&execution_targets, &execution_rules)?; + for (index, plan) in indexes.iter().zip(plans) { + let codes: HashSet<&str> = plan.codes.iter().map(String::as_str).collect(); + by_file[*index] = applicable + .iter() + .copied() + .filter(|rule| rule.fensu_code.is_some_and(|code| codes.contains(code))) + .collect(); + } + } + Ok(by_file) +} + +fn execution_rule(rule: &PythonRuleSpec) -> NativeExecutionRule { + NativeExecutionRule::new( + rule.fensu_code.unwrap_or_default().to_owned(), + rule.family.to_owned(), + rule.owner.to_owned(), + ) +} + +fn execution_target(target: &PythonTarget) -> NativeExecutionTarget { + NativeExecutionTarget { + repository_path: target.repository_path.clone(), + scope: ROOT_SCOPE.to_owned(), + root: target.root.clone(), + relative_parts: target.relative_parts.clone(), + direct: true, + ownership_root: target.package.then(|| PACKAGE_ROOT.to_owned()), + ownership_offset: target.package.then_some(0), + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/faults.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/faults.rs new file mode 100644 index 000000000..144b95f21 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/faults.rs @@ -0,0 +1,79 @@ +//! Fensu findings as `SQBRPYTHON` faults at SQLBuild positions. + +use fensu_native::NativeFaultRow; + +use crate::models::Fault; +use crate::python_rules::_helpers::programs::Programs; +use crate::python_rules::constants::{CATALOGUE_REMEDIATION_CODES, PACKAGE_ROOT, PYTHON_RULES}; +use crate::python_rules::models::{PythonFiles, PythonRuleSpec, PythonTarget}; + +/// `row` under its `SQBRPYTHON` code on its file, or none when a `python/` rule reports outside `python/`. +pub(crate) fn python_fault( + row: NativeFaultRow, + target: &PythonTarget, + programs: &Programs<'_>, +) -> Result, String> { + let rule: &PythonRuleSpec = PYTHON_RULES + .iter() + .find(|rule| rule.fensu_code == Some(row.code.as_str())) + .ok_or_else(|| { + format!( + "Fensu reported rule {} that no SQBRPYTHON rule runs", + row.code + ) + })?; + let path: String = row.path.unwrap_or_else(|| target.repository_path.clone()); + if rule.applies == PythonFiles::Package && !path.starts_with(&format!("{PACKAGE_ROOT}/")) { + return Ok(None); + } + let line: u32 = row.line.max(1); + let column: u64 = programs.index_of(&path).map_or(1, |index| { + character_column(&programs.sources[index].text, line, row.column) + }); + Ok(Some(Fault { + unevaluated: false, + code: rule.code.to_owned(), + path, + line: u64::from(line), + column, + message: row.message.unwrap_or_else(|| rule.message.to_owned()), + remediation: row + .remediation + .filter(|_| !CATALOGUE_REMEDIATION_CODES.contains(&row.code.as_str())) + .unwrap_or_else(|| rule.remediation.to_owned()), + })) +} + +/// The 1-based character column of 0-based UTF-8 byte offset `column` on 1-based `line`. +fn character_column(text: &str, line: u32, column: u32) -> u64 { + let Some(start): Option = line_start(text, line) else { + return 1; + }; + let rest: &str = &text[start..]; + let end: usize = rest.find(['\n', '\r']).unwrap_or(rest.len()); + let mut offset: usize = (column as usize).min(end); + while !rest.is_char_boundary(offset) { + offset -= 1; + } + rest[..offset].chars().count() as u64 + 1 +} + +/// The byte offset where 1-based `line` starts, ending lines at `\n`, `\r\n` or `\r` as Python does. +fn line_start(text: &str, line: u32) -> Option { + let mut remaining: u32 = line.checked_sub(1)?; + let bytes: &[u8] = text.as_bytes(); + let mut index: usize = 0; + while remaining > 0 { + let next: usize = index + + bytes[index..] + .iter() + .position(|byte| matches!(byte, b'\n' | b'\r'))?; + index = if bytes[next] == b'\r' && bytes.get(next + 1) == Some(&b'\n') { + next + 2 + } else { + next + 1 + }; + remaining -= 1; + } + Some(index) +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/file_cache.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/file_cache.rs new file mode 100644 index 000000000..8a87b0496 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/file_cache.rs @@ -0,0 +1,65 @@ +//! When a file's cached Python findings still hold: same file, rules and ruleset, same project answers. + +use std::collections::BTreeMap; + +use fensu_native::rules::constants::{DEAD_DEFINITION_CODE, PUBLIC_MAIN_ENTRY_EXTERNAL_USE_CODE}; + +use crate::models::PythonSource; +use crate::python_rules::models::{CallInputs, PythonTarget}; + +/// The digest of every Python source by path, which project-wide rules read. +pub(crate) fn project_digest(sources: &[PythonSource]) -> String { + let mut hasher = blake3::Hasher::new(); + for source in sources { + hasher.update(source.relative_path.as_bytes()); + hasher.update(b"\0"); + hasher.update(source.digest.as_bytes()); + hasher.update(b"\0"); + } + hasher.finalize().to_hex().to_string() +} + +/// The identity a file's cached findings must carry: its bytes, its calls, and for project-wide rules the project. +pub(crate) fn file_identity( + target: &PythonTarget, + calls: &[(Vec, std::collections::HashMap)], + inputs: &CallInputs<'_>, +) -> String { + let mut hasher = blake3::Hasher::new(); + hasher.update(inputs.fingerprint.as_bytes()); + hasher.update(b"\0"); + hasher.update(target.repository_path.as_bytes()); + hasher.update(b"\0"); + hasher.update(inputs.programs.sources[target.source].digest.as_bytes()); + for (codes, thresholds) in calls { + hasher.update(b"\0call\0"); + hasher.update(codes.join(",").as_bytes()); + let sorted: BTreeMap<&String, &u32> = thresholds.iter().collect(); + for (name, value) in sorted { + hasher.update(format!("\0{name}={value}").as_bytes()); + } + } + let reads_project: bool = calls + .iter() + .flat_map(|(codes, _)| codes) + .any(|code| code == PUBLIC_MAIN_ENTRY_EXTERNAL_USE_CODE || code == DEAD_DEFINITION_CODE); + if reads_project { + hasher.update(b"\0project\0"); + hasher.update(inputs.project_digest.as_bytes()); + } + hasher.finalize().to_hex().to_string() +} + +/// The digest of project answers, in key order. +pub(crate) fn answers_digest(answers: &BTreeMap>) -> String { + let mut hasher = blake3::Hasher::new(); + for (key, values) in answers { + hasher.update(key.as_bytes()); + for value in values { + hasher.update(b"\x01"); + hasher.update(value.as_bytes()); + } + hasher.update(b"\x02"); + } + hasher.finalize().to_hex().to_string() +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/layout.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/layout.rs new file mode 100644 index 000000000..f61e67846 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/layout.rs @@ -0,0 +1,108 @@ +//! SQLBuild's own layout rules for the `python/` package: what may sit at its root, and package names. + +use std::collections::BTreeSet; + +use crate::models::{Fault, PythonSource}; +use crate::python_rules::_helpers::placement::package_anchor; +use crate::python_rules::constants::{ + GENERIC_PACKAGE_NAMES, INIT_FILE, NODE_KIND_DIRECTORIES, PACKAGE_ROOT, + PROJECT_ROOT_ALLOWLIST_CODE, PYTHON_RULES, ROLE_DIRECTORIES, RUNTIME_PACKAGE_OWNER_NAMES_CODE, + SHARED_UTILITIES_PACKAGE, +}; + +/// Faults of the selected layout rules among `codes`, one per offending entry or package. +pub(crate) fn layout_faults(sources: &[PythonSource], codes: &BTreeSet<&str>) -> Vec { + let mut faults: Vec = Vec::new(); + if codes.contains(PROJECT_ROOT_ALLOWLIST_CODE) { + faults.extend(root_entry_faults(sources)); + } + if codes.contains(RUNTIME_PACKAGE_OWNER_NAMES_CODE) { + faults.extend(generic_package_faults(sources)); + } + faults +} + +/// Direct modules, node-kind folders and role folders directly under `python/`. +fn root_entry_faults(sources: &[PythonSource]) -> Vec { + let mut reported: BTreeSet = BTreeSet::new(); + let mut faults: Vec = Vec::new(); + for parts in package_parts(sources) { + let [entry, rest @ ..] = parts.as_slice() else { + continue; + }; + let folder: bool = !rest.is_empty(); + let kind: &str = if !folder { + if *entry == INIT_FILE { + continue; + } + "a module directly under python/" + } else if NODE_KIND_DIRECTORIES.contains(entry) { + "a node-kind folder" + } else if ROLE_DIRECTORIES.iter().any(|(name, _)| name == entry) { + "a role folder" + } else { + continue; + }; + let path: String = format!("{PACKAGE_ROOT}/{entry}"); + if !reported.insert(path.clone()) { + continue; + } + let (shown, reported): (String, Option<&str>) = if folder { + (format!("{path}/"), package_anchor(sources, &path)) + } else { + (path.clone(), Some(path.as_str())) + }; + let detail: String = format!("{shown} is {kind}"); + faults.extend( + reported.and_then(|anchor| layout_fault(PROJECT_ROOT_ALLOWLIST_CODE, anchor, &detail)), + ); + } + faults +} + +/// Packages under `python/` with generic names, except `utils/` directly under it. +fn generic_package_faults(sources: &[PythonSource]) -> Vec { + let mut packages: BTreeSet = BTreeSet::new(); + for parts in package_parts(sources) { + let folders: &[&str] = &parts[..parts.len().saturating_sub(1)]; + for (depth, folder) in folders.iter().enumerate() { + let allowed: bool = depth == 0 && *folder == SHARED_UTILITIES_PACKAGE; + if GENERIC_PACKAGE_NAMES.contains(folder) && !allowed { + packages.insert(format!("{PACKAGE_ROOT}/{}", folders[..=depth].join("/"))); + } + } + } + packages + .iter() + .filter_map(|package| { + let anchor: &str = package_anchor(sources, package)?; + layout_fault( + RUNTIME_PACKAGE_OWNER_NAMES_CODE, + anchor, + &format!("{package}/ has a generic name"), + ) + }) + .collect() +} + +/// Each `python/` file's path parts below `python/`. +fn package_parts(sources: &[PythonSource]) -> impl Iterator> { + sources.iter().filter_map(|source| { + let mut parts = source.relative_path.split('/'); + (parts.next() == Some(PACKAGE_ROOT)).then(|| parts.collect()) + }) +} + +/// A layout fault of rule `code` on the file at `path`. +fn layout_fault(code: &str, path: &str, detail: &str) -> Option { + let rule = PYTHON_RULES.iter().find(|rule| rule.code == code)?; + Some(Fault { + unevaluated: false, + code: code.to_owned(), + path: path.to_owned(), + line: 1, + column: 1, + message: format!("{}: {detail}", rule.message), + remediation: rule.remediation.to_owned(), + }) +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/mod.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/mod.rs new file mode 100644 index 000000000..6685ab9f6 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/mod.rs @@ -0,0 +1,9 @@ +pub(crate) mod contexts; +pub(crate) mod execution; +pub(crate) mod faults; +pub(crate) mod file_cache; +pub(crate) mod layout; +pub(crate) mod observations; +pub(crate) mod placement; +pub(crate) mod programs; +pub(crate) mod project_plane; diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/observations.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/observations.rs new file mode 100644 index 000000000..89fde7800 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/observations.rs @@ -0,0 +1,158 @@ +//! Answers to the project questions Fensu's rules ask about the files around the one they check. + +use std::collections::HashMap; +use std::path::Path; + +use fensu_facts::snapshot::models::{ + RepositoryObservationAnswer, RepositoryObservationIndex, RepositoryObservationQuery, +}; +use fensu_native::NativeProjectQuery; + +use crate::python_rules::_helpers::placement::package_anchor; +use crate::python_rules::_helpers::programs::Programs; +use crate::python_rules::constants::PROJECT_ROOT_QUERY_PATH; + +const TRUE_TEXT: &str = "true"; +const FALSE_TEXT: &str = "false"; +const MEANINGFUL_RESULT: &str = "meaningful"; +const EMPTY_RESULT: &str = "empty"; +/// Query kinds answered from one walk of the project tree. +const FILESYSTEM_KINDS: [&str; 6] = [ + "exists", + "is_file", + "is_dir", + "directory_entries", + "python_anchor", + "glob", +]; + +/// One walk of the project tree covering every filesystem query of the run. +pub(crate) fn repository_index( + repo_root: &Path, + queries: &[&NativeProjectQuery], +) -> Result { + let walked: Vec = queries + .iter() + .filter(|query| FILESYSTEM_KINDS.contains(&query.kind.as_str())) + .map(|query| filesystem_query(query)) + .collect(); + RepositoryObservationIndex::build(repo_root, &walked) + .ok_or_else(|| format!("{}: cannot read the project folder", repo_root.display())) +} + +/// The answers to `queries`, keyed as Fensu's rules look them up. +pub(crate) fn observations( + queries: &[NativeProjectQuery], + programs: &Programs<'_>, + index: &RepositoryObservationIndex, +) -> Result>, String> { + queries + .iter() + .map(|query| Ok((query.key(), answer(query, programs, index)?))) + .collect() +} + +fn answer( + query: &NativeProjectQuery, + programs: &Programs<'_>, + index: &RepositoryObservationIndex, +) -> Result, String> { + let path: &str = project_path(&query.path); + Ok(match query.kind.as_str() { + kind if FILESYSTEM_KINDS.contains(&kind) => { + match filesystem_query(query) + .observe(index) + .map(|state| state.answer) + { + Some(RepositoryObservationAnswer::Bool(value)) => vec![bool_text(value)], + Some(RepositoryObservationAnswer::Paths(paths)) => paths, + Some(RepositoryObservationAnswer::String(text)) => vec![text], + Some(RepositoryObservationAnswer::None) | None => { + if matches!(kind, "exists" | "is_file" | "is_dir") { + vec![bool_text(false)] + } else { + Vec::new() + } + } + } + } + "module_function" => match programs.module_index(&query.path) { + Some(module) => programs + .program(module)? + .project_rows() + .0 + .iter() + .find(|function| function.name == query.argument) + .map(|function| { + if function.meaningful_result { + MEANINGFUL_RESULT.to_owned() + } else { + EMPTY_RESULT.to_owned() + } + }) + .into_iter() + .collect(), + None => Vec::new(), + }, + "public_facade" => vec![bool_text(match programs.index_of(path) { + Some(module) => programs.program(module)?.is_public_facade(&query.argument), + None => false, + })], + "package_anchor" => vec![bool_text( + package_anchor(programs.sources, path).is_some_and(|anchor| anchor == query.argument), + )], + kind => { + return Err(format!( + "Python rules asked an unsupported project question: {kind}" + )); + } + }) +} + +fn filesystem_query(query: &NativeProjectQuery) -> RepositoryObservationQuery { + let (pattern, recursive) = match query.argument.split_once('\0') { + Some((pattern, recursive)) => (Some(pattern.to_owned()), recursive == TRUE_TEXT), + None => (None, false), + }; + RepositoryObservationQuery { + relative_path: project_path(&query.path).to_owned(), + kind: query.kind.clone(), + pattern, + recursive, + } +} + +/// Each of `queries` with its path relative to `project_dir`, so its answer can be read again from the project. +pub(crate) fn relative_queries( + queries: Vec, + project_dir: &str, +) -> Vec { + queries + .into_iter() + .map(|query| relative_query(query, project_dir)) + .collect() +} + +/// `query` relative to `project_dir`, both spelled with `/` as Fensu spells query paths, even on Windows. +pub(crate) fn relative_query(query: NativeProjectQuery, project_dir: &str) -> NativeProjectQuery { + let project_dir: String = project_dir.replace('\\', "/"); + let path: String = match query + .path + .replace('\\', "/") + .strip_prefix(project_dir.as_str()) + { + Some("") => PROJECT_ROOT_QUERY_PATH.to_owned(), + Some(rest) if rest.starts_with('/') => rest.trim_start_matches('/').to_owned(), + _ => query.path, + }; + NativeProjectQuery { path, ..query } +} + +/// A project-relative path; Fensu spells paths below a top-level root with a leading `/`. +fn project_path(path: &str) -> &str { + path.strip_prefix('/').unwrap_or(path) +} + +fn bool_text(value: bool) -> String { + if value { TRUE_TEXT } else { FALSE_TEXT }.to_owned() +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/placement.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/placement.rs new file mode 100644 index 000000000..4239ce8a5 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/placement.rs @@ -0,0 +1,84 @@ +//! Where each project Python file sits, in the terms Fensu's rules read. + +use crate::models::PythonSource; +use crate::python_rules::constants::{ + INIT_FILE, MAIN_ROLE, PACKAGE_ROOT, ROLE_DIRECTORIES, ROLE_FILES, UNIT_MAIN_FILE, +}; +use crate::python_rules::models::PythonTarget; + +/// Place every source: files under `python/` in the runtime package, the rest under their own folder. +pub(crate) fn python_targets(sources: &[PythonSource]) -> Vec { + sources.iter().enumerate().map(placed).collect() +} + +fn placed((source, python): (usize, &PythonSource)) -> PythonTarget { + let parts: Vec = python.relative_path.split('/').map(str::to_owned).collect(); + if parts.len() > 1 && parts[0] == PACKAGE_ROOT { + let relative_parts: Vec = parts[1..].to_vec(); + let first_role: Option<&str> = first_directory_role(&relative_parts); + let unit_main: bool = first_role.is_none() + && relative_parts.len() > 1 + && relative_parts + .last() + .is_some_and(|name| name == UNIT_MAIN_FILE); + let is_main_module: bool = first_role == Some(MAIN_ROLE) || unit_main; + return PythonTarget { + source, + repository_path: python.relative_path.clone(), + package: true, + root: PACKAGE_ROOT.to_owned(), + role: role(&relative_parts), + is_main_module, + is_entry_module: is_main_module + && relative_parts.last().is_some_and(|name| name != INIT_FILE), + relative_parts, + }; + } + let (root, file) = python + .relative_path + .rsplit_once('/') + .unwrap_or(("", python.relative_path.as_str())); + PythonTarget { + source, + repository_path: python.relative_path.clone(), + package: false, + root: root.to_owned(), + relative_parts: vec![file.to_owned()], + role: None, + is_main_module: false, + is_entry_module: false, + } +} + +/// Fensu's `role_of`: a role file name wins, else the first role directory on the path. +fn role(relative_parts: &[String]) -> Option { + let file: &str = relative_parts.last()?; + ROLE_FILES + .iter() + .find(|(name, _)| *name == file) + .map(|(_, role)| *role) + .or_else(|| first_directory_role(relative_parts)) + .map(str::to_owned) +} + +fn first_directory_role(relative_parts: &[String]) -> Option<&'static str> { + relative_parts[..relative_parts.len().saturating_sub(1)] + .iter() + .find_map(|part| directory_role(part)) +} + +fn directory_role(directory: &str) -> Option<&'static str> { + ROLE_DIRECTORIES + .iter() + .find(|(name, _)| *name == directory) + .map(|(_, role)| *role) +} + +/// Fensu's anchor of a package folder: its `__init__.py`, else its first Python file in path order. +pub(crate) fn package_anchor<'a>(sources: &'a [PythonSource], package: &str) -> Option<&'a str> { + let initializer: String = format!("{package}/{INIT_FILE}"); + let prefix: String = format!("{package}/"); + let mut paths = sources.iter().map(|source| source.relative_path.as_str()); + let first: Option<&str> = paths.clone().find(|path| path.starts_with(&prefix)); + paths.find(|path| *path == initializer).or(first) +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/programs.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/programs.rs new file mode 100644 index 000000000..be39783bd --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/programs.rs @@ -0,0 +1,68 @@ +//! The project's Python sources, each parsed at most once and only when a rule needs it. + +use std::collections::HashMap; +use std::sync::OnceLock; + +use fensu_facts::ProgramHandle; + +use crate::models::PythonSource; + +/// Parsed programs for a request's Python sources, filled on first use. +#[derive(Debug)] +pub(crate) struct Programs<'a> { + pub sources: &'a [PythonSource], + python_version: (u8, u8), + parsed: Vec>>, + by_path: HashMap<&'a str, usize>, +} + +impl<'a> Programs<'a> { + pub(crate) fn new(sources: &'a [PythonSource], python_version: (u8, u8)) -> Self { + Self { + sources, + python_version, + parsed: sources.iter().map(|_| OnceLock::new()).collect(), + by_path: sources + .iter() + .enumerate() + .map(|(index, source)| (source.relative_path.as_str(), index)) + .collect(), + } + } + + /// The parsed program of source `index`; a file the parser rejects fails the run. + pub(crate) fn program(&self, index: usize) -> Result<&ProgramHandle, String> { + self.parsed[index] + .get_or_init(|| { + let source: &PythonSource = &self.sources[index]; + ProgramHandle::parse_source( + &source.text, + self.python_version.0, + self.python_version.1, + ) + .map_err(|failure| { + format!( + "{}:{}:{}: cannot parse Python source: {}", + source.relative_path, + failure.line.max(1), + failure.column + 1, + failure.message + ) + }) + }) + .as_ref() + .map_err(Clone::clone) + } + + /// The index of the source at project-relative `path`. + pub(crate) fn index_of(&self, path: &str) -> Option { + self.by_path.get(path).copied() + } + + /// The source of dotted `module` inside the runtime package, as a module file or a package initializer. + pub(crate) fn module_index(&self, module: &str) -> Option { + let path: String = module.replace('.', "/"); + self.index_of(&format!("{path}.py")) + .or_else(|| self.index_of(&format!("{path}/__init__.py"))) + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/_helpers/project_plane.rs b/crates/sqlbuild-rules/src/python_rules/_helpers/project_plane.rs new file mode 100644 index 000000000..6c2ca577a --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/_helpers/project_plane.rs @@ -0,0 +1,140 @@ +//! Every project module at once, for the rules that judge how modules use each other. + +use std::collections::BTreeSet; +use std::path::Path; + +use fensu_facts::ProgramHandle; + +use fensu_native::rules::constants::{DEAD_DEFINITION_CODE, PUBLIC_MAIN_ENTRY_EXTERNAL_USE_CODE}; +use fensu_native::{NativeProjectModule, NativeProjectPlane}; +use sqlbuild_pysource::definition_locations::main::decorated_functions::decorated_functions; + +use crate::models::EvaluateRequest; +use crate::python_rules::_helpers::programs::Programs; +use crate::python_rules::constants::{ + ENTRYPOINT_KIND, INIT_FILE, LOADED_DECORATORS, PACKAGE_ROOT, ROOT_SCOPE, +}; +use crate::python_rules::models::{PythonRuleSpec, PythonTarget}; + +/// The project plane and the definitions SQLBuild loads, built only for the rules that read them. +pub(crate) fn loaded_project( + rules: &[&PythonRuleSpec], + targets: &[PythonTarget], + programs: &Programs<'_>, + request: &EvaluateRequest, +) -> Result<(NativeProjectPlane, Vec<(String, String)>), String> { + let reads_plane: bool = rules.iter().any(|rule| { + matches!( + rule.fensu_code, + Some(PUBLIC_MAIN_ENTRY_EXTERNAL_USE_CODE | DEAD_DEFINITION_CODE) + ) + }); + if !reads_plane { + return Ok((NativeProjectPlane::default(), Vec::new())); + } + let ownership_root: String = Path::new(&request.project_dir) + .join(PACKAGE_ROOT) + .to_string_lossy() + .into_owned(); + project_plane(targets, programs, &ownership_root, request.python_version) +} + +/// Every module, with the modules and definitions SQLBuild loads: all outside `python/`, node functions inside. +fn project_plane( + targets: &[PythonTarget], + programs: &Programs<'_>, + ownership_root: &str, + python_version: (u8, u8), +) -> Result<(NativeProjectPlane, Vec<(String, String)>), String> { + let mut modules: Vec = Vec::with_capacity(targets.len()); + let mut entry_modules: Vec = Vec::new(); + let mut entrypoints: Vec<(String, String)> = Vec::new(); + for target in targets { + let module_parts: Vec = module_parts(&target.repository_path); + let module_name: String = module_parts.join("."); + let program = programs.program(target.source)?; + let loaded: Vec = if target.package { + decorated_functions( + &programs.sources[target.source].text, + &LOADED_DECORATORS, + python_version, + ) + } else { + program + .declaration_rows() + .statements + .iter() + .filter_map(|row| row.function_name.clone().or_else(|| row.class_name.clone())) + .collect() + }; + if !target.package || !loaded.is_empty() { + entry_modules.push(module_name.clone()); + } + entrypoints.extend( + loaded + .into_iter() + .map(|name| (ENTRYPOINT_KIND.to_owned(), format!("{module_name}:{name}"))), + ); + let module = NativeProjectModule::new( + target.repository_path.clone(), + ROOT_SCOPE.to_owned(), + module_parts, + program.clone(), + ); + modules.push(if target.package { + module.with_ownership(Some(1), Some(ownership_root.to_owned())) + } else { + module.with_ownership(None, None) + }); + } + modules.extend(namespace_packages(targets, ownership_root, python_version)?); + Ok((NativeProjectPlane::new(modules, entry_modules), entrypoints)) +} + +/// An empty package module for each folder under `python/` without `__init__.py`, so its submodules resolve. +fn namespace_packages( + targets: &[PythonTarget], + ownership_root: &str, + python_version: (u8, u8), +) -> Result, String> { + let paths: BTreeSet<&str> = targets + .iter() + .map(|target| target.repository_path.as_str()) + .collect(); + let folders: BTreeSet<&str> = targets + .iter() + .filter(|target| target.package) + .flat_map(|target| ancestor_folders(&target.repository_path)) + .filter(|folder| !paths.contains(format!("{folder}/{INIT_FILE}").as_str())) + .collect(); + let empty = ProgramHandle::parse_source("", python_version.0, python_version.1) + .map_err(|failure| format!("cannot parse an empty Python module: {failure:?}"))?; + Ok(folders + .into_iter() + .map(|folder| { + NativeProjectModule::new( + format!("{folder}/{INIT_FILE}"), + ROOT_SCOPE.to_owned(), + folder.split('/').map(str::to_owned).collect(), + empty.clone(), + ) + .with_ownership(Some(1), Some(ownership_root.to_owned())) + }) + .collect()) +} + +/// Every folder above project-relative `path`, outermost first. +fn ancestor_folders(path: &str) -> impl Iterator { + path.match_indices('/').map(move |(end, _)| &path[..end]) +} + +/// The dotted-name parts of the module at project-relative `path`. +fn module_parts(path: &str) -> Vec { + let mut parts: Vec = path.split('/').map(str::to_owned).collect(); + if parts.last().is_some_and(|name| name == INIT_FILE) { + parts.pop(); + } else if let Some(last) = parts.last_mut() { + *last = last.strip_suffix(".py").unwrap_or(last).to_owned(); + } + parts +} diff --git a/crates/sqlbuild-rules/src/python_rules/constants.rs b/crates/sqlbuild-rules/src/python_rules/constants.rs new file mode 100644 index 000000000..f946cac88 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/constants.rs @@ -0,0 +1,906 @@ +//! The Python rule codes: two SQLBuild layout rules, then Fensu's core rules in bands by Fensu family. + +use fensu_native::rules::constants as fensu; + +use crate::python_rules::models::{PythonFiles, PythonRuleSpec}; + +/// Every `SQBRPYTHON` rule; numbers are never reused, and new rules append to their band. +pub(crate) const PYTHON_RULES: [PythonRuleSpec; 64] = [ + PythonRuleSpec { + code: "SQBRPYTHON001", + fensu_code: None, + slug: "project-root-allowlist", + family: "layout", + owner: "project", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "python/ may hold only domain packages, utils/ and __init__.py", + remediation: "Move direct modules into a domain package, and nest node-kind and role folders such as loaders/, tasks/, main/ and _helpers/ inside a domain.", + }, + PythonRuleSpec { + code: "SQBRPYTHON002", + fensu_code: None, + slug: "runtime-package-owner-names", + family: "layout", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "packages under python/ must be named for what they own", + remediation: "Rename generic packages such as common/, shared/ or helpers/ after the domain or capability they hold; utils/ is allowed only directly under python/.", + }, + PythonRuleSpec { + code: "SQBRPYTHON101", + fensu_code: Some(fensu::PARAMETER_ANNOTATION_CODE), + slug: "parameter-annotation", + family: "annotations", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "function parameters must define type annotations", + remediation: "Annotate every parameter with the value type accepted by the function.", + }, + PythonRuleSpec { + code: "SQBRPYTHON102", + fensu_code: Some(fensu::RETURN_ANNOTATION_CODE), + slug: "return-annotation", + family: "annotations", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "functions must define return type annotations", + remediation: "Declare the returned value type, using None when the function returns no value.", + }, + PythonRuleSpec { + code: "SQBRPYTHON103", + fensu_code: Some(fensu::MODULE_VARIABLE_ANNOTATION_CODE), + slug: "module-variable-annotation", + family: "annotations", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "module-level variables must define type annotations", + remediation: "Add an explicit annotation to the first module-level assignment.", + }, + PythonRuleSpec { + code: "SQBRPYTHON104", + fensu_code: Some(fensu::CLASS_ATTRIBUTE_ANNOTATION_CODE), + slug: "class-attribute-annotation", + family: "annotations", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "class attributes must define type annotations", + remediation: "Annotate the class attribute where it is first assigned.", + }, + PythonRuleSpec { + code: "SQBRPYTHON105", + fensu_code: Some(fensu::LOCAL_VARIABLE_ANNOTATION_CODE), + slug: "local-variable-annotation", + family: "annotations", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "local variables must define type annotations on first binding unless assigned a scalar literal", + remediation: "Annotate first bindings whose type is not evident from a number, string, bool, bytes, or f-string literal.", + }, + PythonRuleSpec { + code: "SQBRPYTHON201", + fensu_code: Some(fensu::SINGLE_LINE_DOCSTRINGS_CODE), + slug: "single-line-docstrings", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "docstrings must be a single line; move extended explanation into docs or tests", + remediation: "Keep one concise summary line and move extended rationale into documentation or tests.", + }, + PythonRuleSpec { + code: "SQBRPYTHON202", + fensu_code: Some(fensu::NO_STANDALONE_COMMENTS_CODE), + slug: "no-standalone-comments", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "standalone comments are not allowed; prefer clear names or docs/tests", + remediation: "Replace the comment with clearer names or move lasting explanation into documentation or tests. Tooling directives beginning with these prefixes are allowed: #!, # -*-, # coding:, # noqa, # type:, # pyright:, # pylint:, # pragma:.", + }, + PythonRuleSpec { + code: "SQBRPYTHON203", + fensu_code: Some(fensu::NO_RAW_BUILTIN_RAISE_CODE), + slug: "no-raw-builtin-raise", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "runtime code must raise structured errors instead of raw built-in exceptions", + remediation: "Raise a domain-specific exception from exceptions.py with a stable actionable message.", + }, + PythonRuleSpec { + code: "SQBRPYTHON204", + fensu_code: Some(fensu::NO_ASSERT_IN_RUNTIME_CODE), + slug: "no-assert-in-runtime", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "runtime code must not use assert for invariants; raise a structured error", + remediation: "Replace assert with an explicit guard that raises a domain-specific exception.", + }, + PythonRuleSpec { + code: "SQBRPYTHON205", + fensu_code: Some(fensu::NO_SWALLOWED_EXCEPTION_PROBE_CODE), + slug: "no-swallowed-exception-probe", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "runtime code must not swallow broad exceptions as existence probe answers", + remediation: "Use an explicit metadata or existence check, or catch only the expected exception and preserve failures.", + }, + PythonRuleSpec { + code: "SQBRPYTHON206", + fensu_code: Some(fensu::NO_UNNAMED_STRING_DECISIONS_CODE), + slug: "no-unnamed-string-decisions", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "string literals must not directly control comparison behavior", + remediation: "Name the decision value in constants.py or compare against an enum member so the branch expresses the concept it represents.", + }, + PythonRuleSpec { + code: "SQBRPYTHON207", + fensu_code: Some(fensu::NO_MAGIC_NUMERIC_COMPARISONS_CODE), + slug: "no-magic-numeric-comparisons", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "non-canonical numeric literals must not directly control comparisons", + remediation: "Name the threshold or sentinel in constants.py and compare against that name; only -1, 0, and 1 are self-explanatory comparison values.", + }, + PythonRuleSpec { + code: "SQBRPYTHON208", + fensu_code: Some(fensu::NO_IMPORT_TIME_SIDE_EFFECTS_CODE), + slug: "no-import-time-side-effects", + family: "hygiene", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "runtime and tooling modules must not execute standalone calls during import", + remediation: "Move the operation into an explicit function or assign a pure constructor result.", + }, + PythonRuleSpec { + code: "SQBRPYTHON301", + fensu_code: Some(fensu::ABSOLUTE_IMPORTS_ONLY_CODE), + slug: "absolute-imports-only", + family: "layers", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "use absolute imports; relative imports hide package boundaries", + remediation: "Replace relative imports with an absolute import path.", + }, + PythonRuleSpec { + code: "SQBRPYTHON302", + fensu_code: Some(fensu::NO_STAR_IMPORTS_CODE), + slug: "no-star-imports", + family: "layers", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "star imports hide names from dependency-boundary analysis", + remediation: "Import each required name explicitly.", + }, + PythonRuleSpec { + code: "SQBRPYTHON303", + fensu_code: Some(fensu::NO_INTERNAL_PUBLIC_SURFACE_IMPORTS_CODE), + slug: "no-internal-public-surface-imports", + family: "layers", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "internal code must import from the owning module, not the bare package", + remediation: "Import from the concrete owning module below the package surface.", + }, + PythonRuleSpec { + code: "SQBRPYTHON304", + fensu_code: Some(fensu::NO_CROSS_DOMAIN_PRIVATE_MAIN_IMPORTS_CODE), + slug: "no-cross-domain-private-main-imports", + family: "layers", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "domain-private main entries may only be imported within their owning domain", + remediation: "Remove the leading underscore to publish the main entry, or route the caller through a public main entry owned by the target domain.", + }, + PythonRuleSpec { + code: "SQBRPYTHON305", + fensu_code: Some(fensu::PUBLIC_MAIN_ENTRY_EXTERNAL_USE_CODE), + slug: "public-main-entry-external-use", + family: "layers", + owner: "project", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "public main entries must have an importer outside their owning domain", + remediation: "Prefix the entry module filename with '_' until another domain or tooling imports it.", + }, + PythonRuleSpec { + code: "SQBRPYTHON306", + fensu_code: Some(fensu::DEAD_DEFINITION_CODE), + slug: "unreachable-definition", + family: "layers", + owner: "project", + applies: PythonFiles::Package, + enabled_by_default: false, + options: &[], + message: "definition is unreachable from production roots", + remediation: "Check how the definition is loaded before deleting it. For code loaded in a way the Rule cannot follow, such as by name at run time, add a rule exception with a reason.", + }, + PythonRuleSpec { + code: "SQBRPYTHON307", + fensu_code: Some(fensu::NO_CROSS_FILE_HELPER_PRIVATE_CLASS_CODE), + slug: "no-cross-file-use-of-helper-private-class", + family: "layers", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "helper-private classes are file-local details; move shared classes to classes/", + remediation: "If another module needs this class, move it to the owning classes/ package.", + }, + PythonRuleSpec { + code: "SQBRPYTHON401", + fensu_code: Some(fensu::VALIDATOR_MUST_NOT_RETURN_CODE), + slug: "validator-must-not-return", + family: "naming", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "functions under no-return naming contracts must not return values", + remediation: "Remove the meaningful return and raise on invalid input, or rename a value-producing function as a query such as is_valid or get_validation_result.", + }, + PythonRuleSpec { + code: "SQBRPYTHON402", + fensu_code: Some(fensu::PREDICATE_MUST_RETURN_BOOL_CODE), + slug: "predicate-must-return-bool", + family: "naming", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "predicate names must declare an ordinary boolean result", + remediation: "Return bool (or TypeGuard/TypeIs), or rename the function to describe the value it returns, such as read_status or current_status.", + }, + PythonRuleSpec { + code: "SQBRPYTHON403", + fensu_code: Some(fensu::VALUE_NAME_MUST_RETURN_VALUE_CODE), + slug: "value-name-must-return-value", + family: "naming", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "value-producing names must not declare a no-value result", + remediation: "Return the queried or converted value, or rename the function to describe its side effect, such as initialize_cache or export_json.", + }, + PythonRuleSpec { + code: "SQBRPYTHON404", + fensu_code: Some(fensu::ITERATOR_NAME_MUST_PRODUCE_ITERATOR_CODE), + slug: "iterator-name-must-produce-iterator", + family: "naming", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "iterator names must produce an iterator or generator", + remediation: "Return an iterator, generator, async iterator, or async generator; otherwise rename an eager collection function with a name such as collect_items.", + }, + PythonRuleSpec { + code: "SQBRPYTHON501", + fensu_code: Some(fensu::TOO_MANY_STATEMENTS_CODE), + slug: "too-many-statements", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_statements", + 40, + "Most statements one main-module function may hold.", + )], + message: "main functions must stay phase-shaped and below the statement limit", + remediation: "Extract cohesive phases into helpers that return explicit result models.", + }, + PythonRuleSpec { + code: "SQBRPYTHON502", + fensu_code: Some(fensu::TOO_MANY_DISTINCT_CALLS_CODE), + slug: "too-many-distinct-calls", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_distinct_calls", + 20, + "Most distinct callees one main-module function may call.", + )], + message: "main functions must not coordinate too many distinct callees", + remediation: "Group related work into named phase helpers and keep main/ as a short ordered flow.", + }, + PythonRuleSpec { + code: "SQBRPYTHON503", + fensu_code: Some(fensu::TOO_MANY_LOCALS_CODE), + slug: "too-many-locals", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_locals", + 20, + "Most local names one main-module function may bind.", + )], + message: "main functions must not juggle too many local variables", + remediation: "Let each extracted phase own its intermediates and return one structured result.", + }, + PythonRuleSpec { + code: "SQBRPYTHON504", + fensu_code: Some(fensu::MAX_ARGUMENTS_CODE), + slug: "max-arguments", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_arguments", + 10, + "Most parameters one function may declare.", + )], + message: "functions must stay below the configured argument limit", + remediation: "Reduce the function's responsibility or group cohesive inputs into a typed model.", + }, + PythonRuleSpec { + code: "SQBRPYTHON505", + fensu_code: Some(fensu::MAX_STATEMENTS_GLOBAL_CODE), + slug: "max-statements-global", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_statements_global", + 70, + "Most statements any function may hold.", + )], + message: "functions must stay below the global statement limit", + remediation: "Split the function at a meaningful phase boundary with explicit inputs and outputs. Top-level main functions are governed by FFS001 instead.", + }, + PythonRuleSpec { + code: "SQBRPYTHON506", + fensu_code: Some(fensu::MEANINGFUL_PROJECT_RESULT_DISCARDED_CODE), + slug: "meaningful-project-result-discarded", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "main orchestrators must consume meaningful project-local call results", + remediation: "Assign, return, or explicitly discard the phase result with _ = call(...).", + }, + PythonRuleSpec { + code: "SQBRPYTHON507", + fensu_code: Some(fensu::PARAMETER_MUTATION_IN_PHASE_HELPERS_CODE), + slug: "parameter-mutation-in-phase-helpers", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: false, + options: &[], + message: "helpers must return values instead of mutating parameters", + remediation: "Return a new or updated value so dataflow remains visible to the caller.", + }, + PythonRuleSpec { + code: "SQBRPYTHON508", + fensu_code: Some(fensu::DEFAULT_MUTATION_RETURN_CODE), + slug: "default-mutation-return", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "functions that mutate parameters must return every mutated parameter", + remediation: "Return each mutated parameter explicitly, or avoid parameter mutation and return a new value.", + }, + PythonRuleSpec { + code: "SQBRPYTHON509", + fensu_code: Some(fensu::NO_OUTER_STATE_MUTATION_CODE), + slug: "no-outer-state-mutation", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "functions must not mutate module-global or closure-captured state", + remediation: "Pass state explicitly and return the updated value instead of mutating outer scope.", + }, + PythonRuleSpec { + code: "SQBRPYTHON510", + fensu_code: Some(fensu::NO_COMPLEX_COMPREHENSIONS_CODE), + slug: "no-complex-comprehensions", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "nested or multi-generator comprehensions hide control flow and data shapes", + remediation: "Extract a named helper when the transformation has a coherent purpose. For one-off local logic, use simple statements with named intermediate values instead of nested comprehension control flow.", + }, + PythonRuleSpec { + code: "SQBRPYTHON511", + fensu_code: Some(fensu::MUTABLE_RESULT_MODEL_CODE), + slug: "mutable-result-model", + family: "shape", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "dataclass result models must be frozen", + remediation: "Declare the shared result model with @dataclass(frozen=True).", + }, + PythonRuleSpec { + code: "SQBRPYTHON601", + fensu_code: Some(fensu::MODELS_ONLY_MODELS_CODE), + slug: "models-only-models", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "models role files may contain only structured runtime models", + remediation: "Keep imports and public structured models only. Accepted models are dataclasses and Pydantic BaseModel subclasses; move functions and other declarations to their owning role module.", + }, + PythonRuleSpec { + code: "SQBRPYTHON602", + fensu_code: Some(fensu::TYPES_ONLY_TYPES_CODE), + slug: "types-only-types", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "types role files may contain only type-layer declarations", + remediation: "Move runtime values and functions out of types.py into their owning runtime role.", + }, + PythonRuleSpec { + code: "SQBRPYTHON603", + fensu_code: Some(fensu::CONSTANTS_ONLY_CONSTANTS_CODE), + slug: "constants-only-constants", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "constants role files may contain only assignments and imports", + remediation: "Move functions and classes out of constants.py into their owning role module.", + }, + PythonRuleSpec { + code: "SQBRPYTHON604", + fensu_code: Some(fensu::EXCEPTIONS_ONLY_EXCEPTIONS_CODE), + slug: "exceptions-only-exceptions", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "exceptions role files may contain only custom exceptions", + remediation: "Move non-exception declarations out of exceptions.py into their owning role.", + }, + PythonRuleSpec { + code: "SQBRPYTHON605", + fensu_code: Some(fensu::CONSTANT_OUTSIDE_CONSTANTS_CODE), + slug: "constant-outside-constants", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "public uppercase constants must be defined in the constants role", + remediation: "Move the public constant into constants.py and import it from there.", + }, + PythonRuleSpec { + code: "SQBRPYTHON606", + fensu_code: Some(fensu::BANNED_GENERIC_FILENAME_CODE), + slug: "banned-generic-filename", + family: "roles", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[], + message: "generic filenames hide module ownership", + remediation: "Rename the module after the domain concept or operation it owns. Forbidden module filenames: misc.py.", + }, + PythonRuleSpec { + code: "SQBRPYTHON607", + fensu_code: Some(fensu::HELPERS_MODULE_NAME_CODE), + slug: "helpers-module-name", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "use an _helpers package instead of helpers.py", + remediation: "Replace helpers.py with an _helpers/ package of specifically named modules.", + }, + PythonRuleSpec { + code: "SQBRPYTHON608", + fensu_code: Some(fensu::CLASSES_MODULE_NAME_CODE), + slug: "classes-module-name", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "use a classes package instead of classes.py", + remediation: "Replace classes.py with a classes/ package containing one class per module.", + }, + PythonRuleSpec { + code: "SQBRPYTHON609", + fensu_code: Some(fensu::HELPERS_CLASSES_FILE_PRIVATE_CODE), + slug: "helpers-classes-file-private", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "plain classes in _helpers modules must be file-private", + remediation: "Prefix a file-local helper class with _, or move a shared class into classes/.", + }, + PythonRuleSpec { + code: "SQBRPYTHON610", + fensu_code: Some(fensu::HELPERS_PACKAGE_LAYOUT_CODE), + slug: "helpers-package-layout", + family: "roles", + owner: "package", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[ + ( + "max_helpers_container_modules", + 10, + "Most modules one _helpers/ package may hold directly.", + ), + ( + "max_role_depth", + 1, + "How many package levels may sit below a role package.", + ), + ], + message: "_helpers/ packages must use bounded flat-or-grouped containers", + remediation: "Keep _helpers/ flat or group every module into bounded shallow buckets; do not mix direct Python modules and Python-containing buckets in one container. Only direct .py files other than __init__.py count toward the module limit; empty and asset-only directories do not count as buckets. Runtime role names are not valid buckets. Container depth and entry-module shape are evaluated independently.", + }, + PythonRuleSpec { + code: "SQBRPYTHON611", + fensu_code: Some(fensu::MAIN_PACKAGE_LAYOUT_CODE), + slug: "main-package-layout", + family: "roles", + owner: "package", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[ + ( + "max_main_container_modules", + 20, + "Most modules one main/ package may hold directly.", + ), + ( + "max_role_depth", + 1, + "How many package levels may sit below a role package.", + ), + ], + message: "main/ packages must use bounded flat-or-grouped orchestration containers", + remediation: "Keep main/ flat or group every entry into bounded shallow buckets; do not mix direct Python modules and Python-containing buckets in one container. Only direct .py files other than __init__.py count toward the module limit; empty and asset-only directories do not count as buckets. Runtime role names are not valid buckets. Container depth and entry-module shape are evaluated independently.", + }, + PythonRuleSpec { + code: "SQBRPYTHON612", + fensu_code: Some(fensu::HELPERS_RESERVED_ROLE_FILENAMES_CODE), + slug: "helpers-reserved-role-filenames", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "_helpers/ packages must not contain reserved role filenames", + remediation: "Rename the helper module after its specific operation, or move role-owned declarations to the corresponding sibling models, types, constants, or exceptions role.", + }, + PythonRuleSpec { + code: "SQBRPYTHON613", + fensu_code: Some(fensu::TOP_LEVEL_DOMAIN_SHAPE_CODE), + slug: "top-level-domain-shape", + family: "roles", + owner: "scope", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "top-level domains must be either role leaves or subdomain branches", + remediation: "Keep direct role content in a leaf domain, or move it into a named subdomain when the domain contains subdomains.", + }, + PythonRuleSpec { + code: "SQBRPYTHON614", + fensu_code: Some(fensu::TOP_LEVEL_DIRECT_MODULES_CODE), + slug: "top-level-direct-modules", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "runtime roots and top-level domains must not contain ad hoc direct modules", + remediation: "Keep only package protocol modules and deterministic static-__all__ public facades at the runtime root; move implementation into an owning domain and its direct role boundary.", + }, + PythonRuleSpec { + code: "SQBRPYTHON615", + fensu_code: Some(fensu::SHARED_DOMAIN_PREFIX_CODE), + slug: "shared-domain-prefix", + family: "roles", + owner: "scope", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[( + "min_shared_domain_prefix_packages", + 2, + "Fewest sibling domains sharing a name prefix that are reported; 0 turns the rule off.", + )], + message: "sibling domains must not encode one parent domain through a shared name prefix", + remediation: "Create one parent domain from the shared prefix and move each remaining suffix beneath it as a named subdomain. Setting min_shared_domain_prefix_packages to 0 disables this rule.", + }, + PythonRuleSpec { + code: "SQBRPYTHON616", + fensu_code: Some(fensu::PUBLIC_FACADE_IMPORT_DIRECTION_CODE), + slug: "public-facade-import-direction", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "runtime modules must not import public facade modules", + remediation: "Import the facade's owning domain directly; reserve root facade imports for external consumers, tests, and tooling.", + }, + PythonRuleSpec { + code: "SQBRPYTHON617", + fensu_code: Some(fensu::ENTRY_MODULE_SHAPE_CODE), + slug: "entry-module-shape", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "main/ entry modules must expose one focused public function", + remediation: "Every non-__init__.py module whose first structural role is main is an entry, including grouped main modules. Keep only imports, one public entry function, and at most two small private glue functions; move phase logic to _helpers/. A main bucket below another role is not an entry boundary.", + }, + PythonRuleSpec { + code: "SQBRPYTHON618", + fensu_code: Some(fensu::INIT_MODULE_EMPTY_CODE), + slug: "init-module-empty", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "nested __init__.py files must be empty or docstring-only", + remediation: "Remove runtime declarations and import from the concrete owning module instead.", + }, + PythonRuleSpec { + code: "SQBRPYTHON619", + fensu_code: Some(fensu::NO_REEXPORT_SHIM_CODE), + slug: "no-reexport-shim", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "internal modules must not exist only to re-export imports", + remediation: "Import the implementation module directly or expose a deliberate API through an approved public surface.", + }, + PythonRuleSpec { + code: "SQBRPYTHON620", + fensu_code: Some(fensu::NO_INTERNAL_HELPER_EXPORTS_CODE), + slug: "no-internal-helper-exports", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "_helpers/ modules must not publish an __all__ surface", + remediation: "Keep _helpers/ internal and expose public behavior through main/, classes/, models, types, constants, or exceptions.", + }, + PythonRuleSpec { + code: "SQBRPYTHON621", + fensu_code: Some(fensu::MAIN_ENTRY_NAME_COLLISION_CODE), + slug: "main-entry-name-collision", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "main/ cannot define a module and package with the same entry name", + remediation: "Choose either the flat entry module or the same-named package and remove the competing surface.", + }, + PythonRuleSpec { + code: "SQBRPYTHON622", + fensu_code: Some(fensu::PUBLIC_SURFACE_SHAPE_CODE), + slug: "public-surface-shape", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "root package surfaces may contain only imports and one __all__ declaration", + remediation: "Move runtime behavior into an owning module and keep the root __init__.py as a deliberate import surface.", + }, + PythonRuleSpec { + code: "SQBRPYTHON623", + fensu_code: Some(fensu::CLASSES_ONE_CLASS_PER_MODULE_CODE), + slug: "classes-one-class-per-module", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "classes/ modules must define exactly one top-level class", + remediation: "Split additional classes into separately named modules under classes/.", + }, + PythonRuleSpec { + code: "SQBRPYTHON624", + fensu_code: Some(fensu::HELPERS_PACKAGE_SHAPE_CODE), + slug: "helpers-package-shape", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "_helpers/ packages must contain no main.py orchestration entrypoints", + remediation: "Move main.py orchestration into the sibling main/ role; helper depth is enforced by FFR301.", + }, + PythonRuleSpec { + code: "SQBRPYTHON625", + fensu_code: Some(fensu::PRIVATE_DEFINITION_ORDERING_CODE), + slug: "private-definition-ordering", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "private constants and dataclasses must appear before top-level functions", + remediation: "Move private module declarations above the first function so readers see module state before behavior.", + }, + PythonRuleSpec { + code: "SQBRPYTHON626", + fensu_code: Some(fensu::CLASSES_RUNTIME_REEXPORT_CODE), + slug: "classes-runtime-reexport", + family: "roles", + owner: "file", + applies: PythonFiles::Package, + enabled_by_default: true, + options: &[], + message: "classes/ modules must not publicly re-export imported runtime symbols", + remediation: "Import runtime symbols from their defining modules and keep each classes/ module's public surface owned by its class.", + }, + PythonRuleSpec { + code: "SQBRPYTHON627", + fensu_code: Some(fensu::SOURCE_FILE_LINE_COUNT_CODE), + slug: "source-file-line-count", + family: "roles", + owner: "file", + applies: PythonFiles::All, + enabled_by_default: true, + options: &[( + "max_file_lines", + 2000, + "Most lines one Python file may hold.", + )], + message: "source files must stay below the configured line limit", + remediation: "Split the file by a cohesive role or concern instead of extracting arbitrary numbered fragments.", + }, +]; + +/// The project folder that holds the runtime Python package, imported as `python`. +pub(crate) const PACKAGE_ROOT: &str = "python"; +/// Fensu's scope for runtime code; every carried family applies to it. +pub(crate) const ROOT_SCOPE: &str = "root"; +pub(crate) const INIT_FILE: &str = "__init__.py"; +/// The file holding a unit's implementation next to its node declarations. +pub(crate) const UNIT_MAIN_FILE: &str = "main.py"; +/// The project file reported for project-wide reachability findings. +pub(crate) const PROJECT_CONFIG_FILE: &str = "sqlbuild_project.toml"; +/// Fensu's fixed role file names and the role each gives its file. +pub(crate) const ROLE_FILES: [(&str, &str); 4] = [ + ("models.py", "models"), + ("types.py", "types"), + ("constants.py", "constants"), + ("exceptions.py", "exceptions"), +]; +/// Fensu's role directories and the role each gives the files below it. +pub(crate) const ROLE_DIRECTORIES: [(&str, &str); 7] = [ + ("main", "main"), + ("_helpers", "helpers"), + ("classes", "classes"), + ("models", "models"), + ("types", "types"), + ("constants", "constants"), + ("exceptions", "exceptions"), +]; +pub(crate) const MAIN_ROLE: &str = "main"; +/// Fensu's default naming contracts as prefix and contract; SQLBuild does not make them configurable. +pub(crate) const NAMING_CONTRACTS: [(&str, &str); 10] = [ + ("validate_*", "no-return"), + ("enforce_*", "no-return"), + ("is_*", "returns-bool"), + ("has_*", "returns-bool"), + ("can_*", "returns-bool"), + ("supports_*", "returns-bool"), + ("get_*", "returns-value"), + ("to_*", "returns-value"), + ("as_*", "returns-value"), + ("iter_*", "returns-iterator"), +]; +/// Decorators whose top-level functions SQLBuild loads from `python/`, making their module an entry point. +pub(crate) const LOADED_DECORATORS: [&str; 6] = [ + "loader", + "task", + "asset", + "check", + "factory", + "audit_factory", +]; +/// Fensu codes whose own remediation names settings SQLBuild does not have, so the catalogue's is used. +pub(crate) const CATALOGUE_REMEDIATION_CODES: [&str; 1] = [fensu::DEAD_DEFINITION_CODE]; +/// How Fensu's observation index names the project directory itself. +pub(crate) const PROJECT_ROOT_QUERY_PATH: &str = "."; +/// The entry-point kind recorded for modules SQLBuild loads. +pub(crate) const ENTRYPOINT_KIND: &str = "sqlbuild"; +/// Fensu's family of naming-contract rules, which read the naming contracts. +pub(crate) const NAMING_FAMILY: &str = "naming"; +/// Folders named after node kinds, which belong inside a domain rather than directly under `python/`. +pub(crate) const NODE_KIND_DIRECTORIES: [&str; 10] = [ + "loaders", + "loader", + "tasks", + "task", + "assets", + "asset", + "checks", + "check", + "factories", + "factory", +]; +/// Fensu's banned generic package names (FFR204, private in its crate), plus `general`. +pub(crate) const GENERIC_PACKAGE_NAMES: [&str; 9] = [ + "base", "common", "general", "helpers", "lib", "misc", "shared", "util", "utils", +]; +/// The one generic name allowed, and only directly under `python/`. +pub(crate) const SHARED_UTILITIES_PACKAGE: &str = "utils"; +pub(crate) const PROJECT_ROOT_ALLOWLIST_CODE: &str = "SQBRPYTHON001"; +pub(crate) const RUNTIME_PACKAGE_OWNER_NAMES_CODE: &str = "SQBRPYTHON002"; diff --git a/crates/sqlbuild-rules/src/python_rules/main/evaluate_python_rules.rs b/crates/sqlbuild-rules/src/python_rules/main/evaluate_python_rules.rs new file mode 100644 index 000000000..35d02041a --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/main/evaluate_python_rules.rs @@ -0,0 +1,53 @@ +//! Run the selected Python rules over the request's Python sources. + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::models::{EvaluateRequest, RuleMetadata}; +use crate::python_rules::_helpers::contexts::context_settings; +use crate::python_rules::_helpers::execution::{evaluated_files, file_plans, selected_rules}; +use crate::python_rules::_helpers::file_cache::project_digest; +use crate::python_rules::_helpers::layout::layout_faults; +use crate::python_rules::_helpers::placement::python_targets; +use crate::python_rules::_helpers::programs::Programs; +use crate::python_rules::_helpers::project_plane::loaded_project; +use crate::python_rules::models::{ + CallInputs, PythonCacheEntry, PythonEvaluation, PythonRuleSpec, PythonTarget, +}; + +/// Sorted findings of the selected Python rules, reusing `cached` ones made under `fingerprint` that still hold. +pub(crate) fn evaluate_python_rules( + request: &EvaluateRequest, + selected: &[&RuleMetadata], + cached: &BTreeMap, + fingerprint: &str, +) -> Result { + let codes: BTreeSet<&str> = selected.iter().map(|rule| rule.code.as_str()).collect(); + let rules: Vec<&PythonRuleSpec> = selected_rules(selected); + let mut evaluation = PythonEvaluation::default(); + if !rules.is_empty() && !request.python_sources.is_empty() { + let programs = Programs::new(&request.python_sources, request.python_version); + let targets: Vec = python_targets(&request.python_sources); + let (plane, entrypoints) = loaded_project(&rules, &targets, &programs, request)?; + let inputs = CallInputs { + request, + settings: context_settings(request, &rules, entrypoints), + programs: &programs, + cached, + fingerprint, + project_digest: project_digest(&request.python_sources), + }; + evaluation = evaluated_files(file_plans(&targets, &rules, &inputs)?, &inputs, &plane)?; + } + evaluation + .faults + .extend(layout_faults(&request.python_sources, &codes)); + evaluation.faults.sort_by(|left, right| { + (&left.path, left.line, left.column, &left.code).cmp(&( + &right.path, + right.line, + right.column, + &right.code, + )) + }); + Ok(evaluation) +} diff --git a/crates/sqlbuild-rules/src/python_rules/main/mod.rs b/crates/sqlbuild-rules/src/python_rules/main/mod.rs new file mode 100644 index 000000000..b735fb0d5 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/main/mod.rs @@ -0,0 +1,4 @@ +pub mod evaluate_python_rules; +pub mod python_catalogue; +pub mod python_rule_options; +pub mod python_sources; diff --git a/crates/sqlbuild-rules/src/python_rules/main/python_catalogue.rs b/crates/sqlbuild-rules/src/python_rules/main/python_catalogue.rs new file mode 100644 index 000000000..35229d244 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/main/python_catalogue.rs @@ -0,0 +1,28 @@ +//! The Python rules as catalogue entries. + +use crate::constants::NATIVE_BUILD_IDENTITY; +use crate::models::{RuleMetadata, RuleScope}; +use crate::python_rules::constants::PYTHON_RULES; +use crate::python_rules::models::PythonRuleSpec; + +/// Every `SQBRPYTHON` rule, reported against the project rather than a model. +#[must_use] +pub fn python_catalogue() -> Vec { + PYTHON_RULES.iter().map(metadata).collect() +} + +fn metadata(rule: &PythonRuleSpec) -> RuleMetadata { + RuleMetadata { + code: rule.code.into(), + family: "python".into(), + slug: rule.slug.into(), + message: rule.message.into(), + remediation: rule.remediation.into(), + guidance: None, + implementation_fingerprint: NATIVE_BUILD_IDENTITY.into(), + enabled_by_default: rule.enabled_by_default, + project_wide: true, + custom: false, + scope: RuleScope::Python, + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/main/python_rule_options.rs b/crates/sqlbuild-rules/src/python_rules/main/python_rule_options.rs new file mode 100644 index 000000000..468354fec --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/main/python_rule_options.rs @@ -0,0 +1,22 @@ +//! The whole-number thresholds a Python rule declares. + +use crate::python_rules::constants::PYTHON_RULES; +use crate::rules::models::{BuiltinOption, BuiltinOptionKind}; + +/// The options of the Python rule `code`, or none for any other code. +#[must_use] +pub fn python_rule_options(code: &str) -> Vec { + PYTHON_RULES + .iter() + .filter(|rule| rule.code == code) + .flat_map(|rule| rule.options.iter()) + .map(|(name, default, description)| BuiltinOption { + kind: BuiltinOptionKind::Integer { + default: *default, + minimum: 0, + }, + name, + description, + }) + .collect() +} diff --git a/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs b/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs new file mode 100644 index 000000000..f31ebf899 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs @@ -0,0 +1,40 @@ +//! The project Python files the selected Python rules read, digested so the memo sees every edit. + +use std::path::Path; + +use sqlbuild_discovery::python_paths::main::supported_python_paths::supported_python_paths; + +use crate::models::{EvaluateRequest, PythonSource}; +use crate::rules::main::assemble_catalogue::assemble_catalogue; +use crate::rules::main::select::select; + +/// Every supported project Python file when the request selects a Python rule, otherwise none. +pub(crate) fn python_sources(request: &EvaluateRequest) -> Result, String> { + let catalogue = assemble_catalogue(&request.custom_rules)?; + let selected = select( + &catalogue, + request.config.select.as_deref(), + &request.config.ignore, + )?; + if !selected.iter().any(|rule| rule.scope.evaluates_python()) { + return Ok(Vec::new()); + } + let project_dir: &Path = Path::new(&request.project_dir); + supported_python_paths(project_dir) + .into_iter() + .map(|parts| read_source(project_dir, parts.join("/"))) + .collect() +} + +fn read_source(project_dir: &Path, relative_path: String) -> Result { + let bytes: Vec = std::fs::read(project_dir.join(&relative_path)) + .map_err(|error| format!("{relative_path}: cannot read Python source: {error}"))?; + let digest: String = blake3::hash(&bytes).to_hex().to_string(); + let text: String = String::from_utf8(bytes) + .map_err(|_| format!("{relative_path}: Python source is not valid UTF-8"))?; + Ok(PythonSource { + relative_path, + digest, + text, + }) +} diff --git a/crates/sqlbuild-rules/src/python_rules/mod.rs b/crates/sqlbuild-rules/src/python_rules/mod.rs new file mode 100644 index 000000000..1665593be --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/mod.rs @@ -0,0 +1,9 @@ +//! Rules over a project's Python files: SQLBuild's layout rules and Fensu's core rules. + +pub(crate) mod _helpers; +pub(crate) mod constants; +pub mod main; +pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlbuild-rules/src/python_rules/models.rs b/crates/sqlbuild-rules/src/python_rules/models.rs new file mode 100644 index 000000000..a0227a87d --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/models.rs @@ -0,0 +1,103 @@ +//! How each Python rule maps to Fensu and which files it reads. + +/// The Python files a rule applies to. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum PythonFiles { + /// Only the runtime package under `python/`, whose layout the rule checks. + Package, + /// Every supported project Python file, including hooks, UDFs and orchestration modules. + All, +} + +/// One `SQBRPYTHON` rule and the Fensu core rule it runs, if any. +#[derive(Clone, Copy, Debug)] +pub struct PythonRuleSpec { + pub code: &'static str, + /// The Fensu code it evaluates; `None` for SQLBuild's own layout rules. + pub fensu_code: Option<&'static str>, + pub slug: &'static str, + /// Fensu's family, which with `owner` decides the file a project-wide finding is reported on. + pub family: &'static str, + pub owner: &'static str, + pub applies: PythonFiles, + pub enabled_by_default: bool, + /// Whole-number options as name, default and description, named as Fensu names its thresholds. + pub options: &'static [(&'static str, u32, &'static str)], + pub message: &'static str, + pub remediation: &'static str, +} + +/// One project Python file placed the way Fensu places a file under its scan root. +#[derive(Clone, Debug)] +pub(crate) struct PythonTarget { + /// The file's index in the request's Python sources. + pub source: usize, + pub repository_path: String, + /// Whether the file belongs to the runtime package under `python/`. + pub package: bool, + /// The directory the file is placed under: `python` for package files, else its parent. + pub root: String, + /// Path parts below `root`. + pub relative_parts: Vec, + pub role: Option, + pub is_main_module: bool, + pub is_entry_module: bool, +} + +/// One `evaluate_core_rules` call on a file: the Fensu codes it runs and the context they read. +#[derive(Debug)] +pub(crate) struct RuleCall { + pub codes: Vec, + pub context: fensu_native::NativeRuleContext, + pub queries: Vec, +} + +/// What every call of one run reads. +#[derive(Debug)] +pub(crate) struct CallInputs<'a> { + pub request: &'a crate::models::EvaluateRequest, + pub settings: crate::python_rules::_helpers::contexts::ContextSettings, + pub programs: &'a crate::python_rules::_helpers::programs::Programs<'a>, + /// Cached findings by file from the last run with the same ruleset. + pub cached: &'a std::collections::BTreeMap, + /// The digest of the ruleset and interpreter version the cached findings must match. + pub fingerprint: &'a str, + /// The digest of every Python source, for rules that read the whole project. + pub project_digest: String, +} + +/// One file's cached Python findings and the project answers they were computed from. +#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] +pub struct PythonCacheEntry { + /// The digest of the file, its rules and thresholds, and the ruleset. + pub identity: String, + /// The project questions the file's rules asked, as kind, path and argument. + pub queries: Vec<(String, String, String)>, + /// The digest of their answers; a changed answer makes the entry stale. + pub answers: String, + pub faults: Vec, +} + +/// The Python rules' findings, with the cache entries of every file that ran Fensu's rules. +#[derive(Debug, Default)] +pub(crate) struct PythonEvaluation { + pub faults: Vec, + pub entries: std::collections::BTreeMap, + pub hits: usize, + pub misses: usize, +} + +/// One file's planned calls before it is parsed: Fensu codes and thresholds per call. +#[derive(Debug)] +pub(crate) struct FilePlan<'a> { + pub target: &'a PythonTarget, + pub calls: Vec<(Vec, std::collections::HashMap)>, + pub identity: String, +} + +/// Planned files split into cached entries that still hold and files to evaluate again. +#[derive(Debug, Default)] +pub(crate) struct CacheSplit<'a> { + pub fresh: Vec<(String, PythonCacheEntry)>, + pub stale: Vec>, +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests.rs b/crates/sqlbuild-rules/src/python_rules/tests.rs new file mode 100644 index 000000000..33bff6f6d --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests.rs @@ -0,0 +1,12 @@ +#[path = "tests/test_evaluate_python_rules.rs"] +mod evaluate_python_rules; +#[path = "tests/helpers.rs"] +mod helpers; +#[path = "tests/test_python_cache.rs"] +mod python_cache; +#[path = "tests/test_python_catalogue.rs"] +mod python_catalogue; +#[path = "tests/test_python_scaling.rs"] +mod python_scaling; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlbuild-rules/src/python_rules/tests/helpers.rs b/crates/sqlbuild-rules/src/python_rules/tests/helpers.rs new file mode 100644 index 000000000..ab947c647 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/helpers.rs @@ -0,0 +1,280 @@ +use std::collections::BTreeSet; + +use fensu_facts::extension::models::ProgramHandle; +use fensu_native::{NativeProjectPlane, NativeRuleContext, evaluate_core_rules}; + +use crate::engine::_helpers::memo::{evaluate_rows, prepare}; +use crate::models::{EvaluateRequest, RowsRequest, RuleMetadata, RuleScope, RulesConfig}; +use crate::python_rules::tests::test_types::CachedRuns; +use crate::rules::_helpers::catalogue::{catalogue, select}; + +/// How many distinct `SQBRPYTHON` codes the catalogue holds. +pub(super) fn distinct_python_codes(rules: &[RuleMetadata]) -> usize { + rules + .iter() + .filter(|rule| rule.code.starts_with("SQBRPYTHON")) + .map(|rule| rule.code.as_str()) + .collect::>() + .len() +} + +/// How many catalogue rules are project-wide Python rules. +pub(super) fn project_wide_python_rules(rules: &[RuleMetadata]) -> usize { + rules + .iter() + .filter(|rule| { + rule.scope == RuleScope::Python && rule.project_wide && rule.family == "python" + }) + .count() +} + +/// Whether selecting `select` runs the rule `code`. +pub(super) fn is_selected(selectors: &[String], code: &str) -> bool { + let rules: Vec = catalogue(); + select(&rules, Some(selectors), &[]) + .expect("valid selection") + .iter() + .any(|rule| rule.code == code) +} + +/// The faults one Fensu core rule reports on a root-scope module, as code and line. +pub(super) fn core_rule_faults(source: &str, fensu_code: &str) -> Vec<(String, u32)> { + let program: ProgramHandle = ProgramHandle::parse_source(source, 3, 12).expect("source parses"); + let context = NativeRuleContext { + scope: "root".to_owned(), + ..NativeRuleContext::default() + }; + evaluate_core_rules( + &program, + &[fensu_code.to_owned()], + &context, + &NativeProjectPlane::new(Vec::new(), Vec::new()), + ) + .expect("rule evaluates") + .into_iter() + .map(|fault| (fault.code, fault.line)) + .collect() +} + +/// The speculation keys of a request selecting `select`, before and after its one Python module changes from `original` to `edited`. +pub(super) fn source_keys(select: &[&str], original: &str, edited: &str) -> (String, String) { + let directory = tempfile::tempdir().expect("temporary project"); + let module = directory.path().join("python/orders/main.py"); + std::fs::create_dir_all(module.parent().expect("parent")).expect("folders"); + std::fs::write(&module, original).expect("module"); + let before: String = request_key(directory.path(), select); + std::fs::write(&module, edited).expect("module"); + (before, request_key(directory.path(), select)) +} + +fn request_key(project_dir: &std::path::Path, select: &[&str]) -> String { + let config: RulesConfig = + serde_json::from_value(serde_json::json!({ "select": select })).expect("rules config"); + let request = EvaluateRequest { + project_dir: project_dir.to_string_lossy().into_owned(), + config, + ..EvaluateRequest::default() + }; + prepare( + RowsRequest { + request, + memo: None, + retain: false, + }, + true, + ) + .expect("request prepares") + .key + .expect("keyed") +} + +/// The Python faults of a project holding `files`, under rules config `config`, as code, path, line and column. +pub(super) fn python_rule_faults( + files: &[(&str, &str)], + config: serde_json::Value, +) -> Result, String> { + let directory = tempfile::tempdir().map_err(|error| error.to_string())?; + for (path, contents) in files { + let file = directory.path().join(path); + std::fs::create_dir_all(file.parent().expect("parent")) + .map_err(|error| error.to_string())?; + std::fs::write(&file, contents).map_err(|error| error.to_string())?; + } + let mut config = config; + config["cache"] = serde_json::json!({ "enabled": false }); + let request = EvaluateRequest { + project_dir: directory.path().to_string_lossy().into_owned(), + config: serde_json::from_value(config).map_err(|error| error.to_string())?, + ..EvaluateRequest::default() + }; + let evaluation = evaluate_rows(RowsRequest { + request, + memo: None, + retain: false, + }) + .map_err(|error| format!("{error:?}"))?; + Ok(evaluation + .faults + .into_iter() + .filter(|fault| fault.code.starts_with("SQBRPYTHON")) + .map(|fault| (fault.code, fault.path, fault.line, fault.column)) + .collect()) +} + +/// Expected faults as the owned tuples `python_rule_faults` returns. +pub(super) fn owned_faults(faults: &[(&str, &str, u64, u64)]) -> Vec<(String, String, u64, u64)> { + faults + .iter() + .map(|(code, path, line, column)| ((*code).to_owned(), (*path).to_owned(), *line, *column)) + .collect() +} + +/// Python faults of two cached runs, `files` then `changes` under `changed_config`, and the second's cache use. +pub(super) fn cached_runs( + files: &[(&str, &str)], + config: serde_json::Value, + changes: &[(&str, &str)], + changed_config: serde_json::Value, +) -> Result { + let directory = tempfile::tempdir().map_err(|error| error.to_string())?; + write_files(directory.path(), files)?; + let first = cached_run(directory.path(), config)?; + write_files(directory.path(), changes)?; + let second = cached_run(directory.path(), changed_config)?; + Ok(CachedRuns { + first_faults: python_codes(&first), + second_faults: python_codes(&second), + second_hits: second.cache_hits, + second_misses: second.cache_misses, + }) +} + +fn write_files, C: AsRef>( + root: &std::path::Path, + files: &[(P, C)], +) -> Result<(), String> { + for (path, contents) in files { + let file = root.join(path.as_ref()); + std::fs::create_dir_all(file.parent().expect("parent")) + .map_err(|error| error.to_string())?; + std::fs::write(&file, contents.as_ref()).map_err(|error| error.to_string())?; + } + Ok(()) +} + +/// A project of `units` small domain entry modules sharing one helper, plus `noisy_files` hooks of `functions` unannotated functions each. +pub(super) fn scaling_project( + units: usize, + noisy_files: usize, + functions: usize, +) -> Vec<(String, String)> { + let helper = ( + "python/utils/text/join.py".to_owned(), + "\"\"\"Join text.\"\"\"\n\n\ndef joined(value: str) -> str:\n \"\"\"Return the value.\"\"\"\n return value\n".to_owned(), + ); + let modules = (0..units).map(|unit| { + ( + format!("python/domain_{}/unit_{unit}/main.py", unit % 50), + format!( + "\"\"\"Unit {unit}.\"\"\"\n\nfrom python.utils.text.join import joined\n\n\ndef run_{unit}(value: str) -> str:\n \"\"\"Run unit {unit}.\"\"\"\n return joined(value)\n" + ), + ) + }); + let noisy = (0..noisy_files).map(|file| { + let body: String = (0..functions) + .map(|function| { + format!("def alert_{function}(ctx, reason):\n print(ctx, reason)\n\n\n") + }) + .collect(); + (format!("hooks/python/alert_{file}.py"), body) + }); + std::iter::once(helper) + .chain(modules) + .chain(noisy) + .collect() +} + +/// The codes of the Python faults `files` produce under `config` with the cache off, and how long evaluation took. +pub(super) fn timed_python_codes( + files: &[(String, String)], + config: serde_json::Value, +) -> Result<(Vec, std::time::Duration), String> { + let directory = tempfile::tempdir().map_err(|error| error.to_string())?; + write_files(directory.path(), files)?; + let mut config = config; + config["cache"] = serde_json::json!({ "enabled": false }); + let started = std::time::Instant::now(); + let evaluation = cached_run(directory.path(), config)?; + let elapsed = started.elapsed(); + let codes = python_codes(&evaluation) + .into_iter() + .map(|(code, _)| code) + .collect(); + Ok((codes, elapsed)) +} + +/// Hold while timing, so the scaling tests never measure each other's work. +pub(super) fn serial_timing() -> std::sync::MutexGuard<'static, ()> { + static TIMING: std::sync::Mutex<()> = std::sync::Mutex::new(()); + TIMING + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// The median over interleaved `pairs` of the large project's time divided by the small one's. +pub(super) fn median_growth( + small: &[(String, String)], + large: &[(String, String)], + config: &serde_json::Value, + pairs: usize, +) -> f64 { + let mut ratios: Vec = (0..pairs) + .map(|_| pair_growth(small, large, config)) + .collect(); + ratios.sort_by(f64::total_cmp); + ratios[ratios.len() / 2] +} + +fn pair_growth( + small: &[(String, String)], + large: &[(String, String)], + config: &serde_json::Value, +) -> f64 { + let (_, small_elapsed) = timed_python_codes(small, config.clone()).expect("rules run"); + let (_, large_elapsed) = timed_python_codes(large, config.clone()).expect("rules run"); + large_elapsed.as_secs_f64() / small_elapsed.as_secs_f64() +} + +fn cached_run( + project_dir: &std::path::Path, + config: serde_json::Value, +) -> Result { + let request = EvaluateRequest { + project_dir: project_dir.to_string_lossy().into_owned(), + config: serde_json::from_value(config).map_err(|error| error.to_string())?, + ..EvaluateRequest::default() + }; + evaluate_rows(RowsRequest { + request, + memo: None, + retain: false, + }) + .map_err(|error| format!("{error:?}")) +} + +fn python_codes(evaluation: &crate::models::RowsEvaluation) -> Vec<(String, String)> { + evaluation + .faults + .iter() + .filter(|fault| fault.code.starts_with("SQBRPYTHON")) + .map(|fault| (fault.code.clone(), fault.path.clone())) + .collect() +} + +/// Expected faults as owned code and path pairs. +pub(super) fn named_faults(items: &[(&str, &str)]) -> Vec<(String, String)> { + items + .iter() + .map(|(code, path)| ((*code).to_owned(), (*path).to_owned())) + .collect() +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests/test_evaluate_python_rules.rs b/crates/sqlbuild-rules/src/python_rules/tests/test_evaluate_python_rules.rs new file mode 100644 index 000000000..0a0c34f9d --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/test_evaluate_python_rules.rs @@ -0,0 +1,254 @@ +use serde_json::json; + +use crate::python_rules::tests::helpers::{owned_faults, python_rule_faults}; +use crate::python_rules::tests::test_types::{PythonParseFailureTestCase, PythonRunTestCase}; + +#[test] +fn given_project_python_when_evaluating_python_rules_then_reports_fensu_findings_in_sqlbuild_terms() +{ + let test_cases = [ + PythonRunTestCase { + description: "a file outside python/ reports at a character column after non-ASCII text", + files: &[( + "hooks/python/notify.py", + "\"\"\"Notify.\"\"\"\n\n\ndef send(é: int, count) -> None:\n pass\n", + )], + config: json!({ "select": ["SQBRPYTHON101"] }), + expected_faults: vec![("SQBRPYTHON101", "hooks/python/notify.py", 4, 18)], + }, + PythonRunTestCase { + description: "python/ holds only domains, utils/ and __init__.py; folders report on their anchor", + files: &[ + ("python/__init__.py", ""), + ("python/report.py", "\"\"\"Report.\"\"\"\n"), + ("python/loaders/orders.py", "\"\"\"Orders.\"\"\"\n"), + ("python/_helpers/Alpha.py", "\"\"\"Alpha.\"\"\"\n"), + ("python/_helpers/__init__.py", ""), + ("python/orders/daily/node.py", "\"\"\"Daily.\"\"\"\n"), + ("python/utils/currency/format.py", "\"\"\"Format.\"\"\"\n"), + ], + config: json!({ "select": ["SQBRPYTHON001"] }), + expected_faults: vec![ + ("SQBRPYTHON001", "python/_helpers/__init__.py", 1, 1), + ("SQBRPYTHON001", "python/loaders/orders.py", 1, 1), + ("SQBRPYTHON001", "python/report.py", 1, 1), + ], + }, + PythonRunTestCase { + description: "generic package names report once per package, except utils/ at the root", + files: &[ + ("python/orders/common/b.py", "\"\"\"B.\"\"\"\n"), + ("python/orders/common/a.py", "\"\"\"A.\"\"\"\n"), + ("python/utils/currency/format.py", "\"\"\"Format.\"\"\"\n"), + ("python/utils/shared/text.py", "\"\"\"Text.\"\"\"\n"), + ("python/helpers/rows.py", "\"\"\"Rows.\"\"\"\n"), + ], + config: json!({ "select": ["SQBRPYTHON002"] }), + expected_faults: vec![ + ("SQBRPYTHON002", "python/helpers/rows.py", 1, 1), + ("SQBRPYTHON002", "python/orders/common/a.py", 1, 1), + ("SQBRPYTHON002", "python/utils/shared/text.py", 1, 1), + ], + }, + PythonRunTestCase { + description: "package-only rules skip files outside python/", + files: &[ + ("python/report.py", "\"\"\"Report.\"\"\"\n"), + ("providers/report.py", "\"\"\"Report.\"\"\"\n"), + ], + config: json!({ "select": ["SQBRPYTHON614"] }), + expected_faults: vec![("SQBRPYTHON614", "python/report.py", 1, 1)], + }, + PythonRunTestCase { + description: "a unit main.py is an entry module holding one public function", + files: &[ + ( + "python/orders/daily/main.py", + "def first() -> None:\n pass\n\n\ndef second() -> None:\n pass\n", + ), + ( + "python/orders/weekly/main.py", + "def only() -> None:\n pass\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON617"] }), + expected_faults: vec![("SQBRPYTHON617", "python/orders/daily/main.py", 1, 1)], + }, + PythonRunTestCase { + description: "a main-only threshold reads its option and skips helpers", + files: &[ + ( + "python/orders/main/build.py", + "def build() -> None:\n a: int = 1\n b: int = a\n print(b)\n", + ), + ( + "python/orders/_helpers/build_rows.py", + "def build_rows() -> None:\n a: int = 1\n b: int = a\n print(b)\n", + ), + ], + config: json!({ + "select": ["SQBRPYTHON501"], + "rule_options": { "SQBRPYTHON501": { "max_statements": 2 } }, + }), + expected_faults: vec![("SQBRPYTHON501", "python/orders/main/build.py", 1, 1)], + }, + PythonRunTestCase { + description: "a package-wide rule reports once per package", + files: &[ + ( + "python/orders/_helpers/first.py", + "def first() -> None:\n pass\n", + ), + ( + "python/orders/_helpers/second.py", + "def second() -> None:\n pass\n", + ), + ], + config: json!({ + "select": ["SQBRPYTHON610"], + "rule_options": { "SQBRPYTHON610": { "max_helpers_container_modules": 1 } }, + }), + expected_faults: vec![("SQBRPYTHON610", "python/orders/_helpers/first.py", 1, 1)], + }, + PythonRunTestCase { + description: "a public main entry nobody outside its domain imports", + files: &[( + "python/orders/main/export.py", + "def export_orders() -> None:\n pass\n", + )], + config: json!({ "select": ["SQBRPYTHON305"] }), + expected_faults: vec![("SQBRPYTHON305", "python/orders/main/export.py", 1, 1)], + }, + PythonRunTestCase { + description: "a hook importing a public main entry counts as outside use", + files: &[ + ( + "python/orders/main/export.py", + "def export_orders() -> None:\n pass\n", + ), + ( + "hooks/python/notify.py", + "from python.orders.main.export import export_orders\n\n\ndef notify() -> None:\n export_orders()\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON305"] }), + expected_faults: vec![], + }, + PythonRunTestCase { + description: "naming contracts apply", + files: &[( + "providers/flags.py", + "def is_ready() -> int:\n return 1\n", + )], + config: json!({ "select": ["SQBRPYTHON402"] }), + expected_faults: vec![("SQBRPYTHON402", "providers/flags.py", 1, 1)], + }, + PythonRunTestCase { + description: "imports resolve under the python package", + files: &[ + ( + "python/orders/main/export.py", + "from python.billing.main._totals import total\n", + ), + ( + "python/billing/main/_totals.py", + "def total() -> int:\n return 1\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON304"] }), + expected_faults: vec![("SQBRPYTHON304", "python/orders/main/export.py", 1, 1)], + }, + PythonRunTestCase { + description: "a discarded result of a project function is found through the called module", + files: &[ + ( + "python/orders/main/export.py", + "from python.orders._helpers.totals import order_total\n\n\ndef export_orders() -> None:\n order_total()\n", + ), + ( + "python/orders/_helpers/totals.py", + "def order_total() -> int:\n return 1\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON506"] }), + expected_faults: vec![("SQBRPYTHON506", "python/orders/main/export.py", 5, 5)], + }, + PythonRunTestCase { + description: "node and hook functions are live, unreferenced helpers are dead, and hooks are never reported", + files: &[ + ( + "python/orders/daily/node.py", + "@loader\ndef daily() -> None:\n pass\n", + ), + ( + "python/orders/_helpers/unused.py", + "def unused() -> None:\n pass\n", + ), + ( + "python/orders/_helpers/totals.py", + "def order_total() -> int:\n return 1\n", + ), + ( + "hooks/python/notify.py", + "from python.orders._helpers.totals import order_total\n\n_UNUSED_LIMIT: int = 3\n\n\ndef notify() -> int:\n return order_total()\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON306"] }), + expected_faults: vec![("SQBRPYTHON306", "python/orders/_helpers/unused.py", 1, 1)], + }, + PythonRunTestCase { + description: "an audit factory under python/ is loaded", + files: &[( + "python/orders/quality/factories.py", + "@audit_factory\ndef order_quality() -> list:\n return []\n", + )], + config: json!({ "select": ["SQBRPYTHON306"] }), + expected_faults: vec![], + }, + PythonRunTestCase { + description: "a submodule imported from a folder without __init__.py is used", + files: &[ + ( + "python/orders/_tables.py", + "TABLES: int = 1\nUNUSED: int = 2\n", + ), + ( + "python/orders/node.py", + "from python.orders import _tables\n\n\n@task\ndef count() -> int:\n return _tables.TABLES\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON306"] }), + expected_faults: vec![("SQBRPYTHON306", "python/orders/_tables.py", 2, 1)], + }, + ]; + for test_case in test_cases { + let faults = python_rule_faults(test_case.files, test_case.config.clone()); + + assert_eq!( + faults, + Ok(owned_faults(&test_case.expected_faults)), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_python_the_parser_rejects_when_evaluating_python_rules_then_the_run_fails_at_the_file() { + let test_cases = [PythonParseFailureTestCase { + description: "a parameter list cut short", + files: &[("python/orders/main/bad.py", "def broken(:\n")], + expected_error: "python/orders/main/bad.py:1:12: cannot parse Python source", + }]; + for test_case in test_cases { + let faults = python_rule_faults(test_case.files, json!({ "select": ["SQBRPYTHON101"] })); + + assert!( + faults + .as_ref() + .is_err_and(|error| error.contains(test_case.expected_error)), + "{}: {faults:?}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests/test_python_cache.rs b/crates/sqlbuild-rules/src/python_rules/tests/test_python_cache.rs new file mode 100644 index 000000000..6f4cfdfda --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/test_python_cache.rs @@ -0,0 +1,171 @@ +use serde_json::json; + +use fensu_native::NativeProjectQuery; + +use crate::python_rules::_helpers::observations::relative_query; +use crate::python_rules::tests::helpers::{cached_runs, named_faults}; +use crate::python_rules::tests::test_types::{ + CachedRuns, PythonCacheTestCase, RelativeQueryTestCase, +}; + +const UNANNOTATED: &str = "def send(count) -> None:\n pass\n"; +const ANNOTATED: &str = "def send(count: int) -> None:\n pass\n"; + +#[test] +fn given_a_cached_python_run_when_running_again_then_only_changed_inputs_are_evaluated() { + let test_cases = [ + PythonCacheTestCase { + description: "an unchanged project is answered from the cache", + files: &[ + ("hooks/python/notify.py", UNANNOTATED), + ("providers/settings.py", ANNOTATED), + ], + config: json!({ "select": ["SQBRPYTHON101"] }), + changes: &[], + changed_config: json!({ "select": ["SQBRPYTHON101"] }), + expected_runs: CachedRuns { + first_faults: named_faults(&[("SQBRPYTHON101", "hooks/python/notify.py")]), + second_faults: named_faults(&[("SQBRPYTHON101", "hooks/python/notify.py")]), + second_hits: 2, + second_misses: 0, + }, + }, + PythonCacheTestCase { + description: "an edited file is evaluated again and its old finding goes", + files: &[ + ("hooks/python/notify.py", UNANNOTATED), + ("providers/settings.py", ANNOTATED), + ], + config: json!({ "select": ["SQBRPYTHON101"] }), + changes: &[("hooks/python/notify.py", ANNOTATED)], + changed_config: json!({ "select": ["SQBRPYTHON101"] }), + expected_runs: CachedRuns { + first_faults: named_faults(&[("SQBRPYTHON101", "hooks/python/notify.py")]), + second_faults: named_faults(&[]), + second_hits: 1, + second_misses: 1, + }, + }, + PythonCacheTestCase { + description: "a changed option evaluates every file again", + files: &[( + "python/orders/main/build.py", + "def build() -> None:\n a: int = 1\n b: int = a\n print(b)\n", + )], + config: json!({ "select": ["SQBRPYTHON501"] }), + changes: &[], + changed_config: json!({ + "select": ["SQBRPYTHON501"], + "rule_options": { "SQBRPYTHON501": { "max_statements": 2 } }, + }), + expected_runs: CachedRuns { + first_faults: named_faults(&[]), + second_faults: named_faults(&[("SQBRPYTHON501", "python/orders/main/build.py")]), + second_hits: 0, + second_misses: 1, + }, + }, + PythonCacheTestCase { + description: "an unchanged file whose import target appears is evaluated again", + files: &[( + "python/orders/main/export.py", + "from python.billing.main._totals import total\n", + )], + config: json!({ "select": ["SQBRPYTHON304"] }), + changes: &[( + "python/billing/main/_totals.py", + "def total() -> int:\n return 1\n", + )], + changed_config: json!({ "select": ["SQBRPYTHON304"] }), + expected_runs: CachedRuns { + first_faults: named_faults(&[]), + second_faults: named_faults(&[("SQBRPYTHON304", "python/orders/main/export.py")]), + second_hits: 0, + second_misses: 2, + }, + }, + PythonCacheTestCase { + description: "an unchanged domain package whose folder gains a subdomain is evaluated again", + files: &[ + ("python/inventory/__init__.py", ""), + ("python/inventory/main/__init__.py", ""), + ( + "python/inventory/main/publish.py", + "def publish() -> None:\n pass\n", + ), + ], + config: json!({ "select": ["SQBRPYTHON613"] }), + changes: &[ + ("python/inventory/restock/__init__.py", ""), + ("python/inventory/restock/main/__init__.py", ""), + ], + changed_config: json!({ "select": ["SQBRPYTHON613"] }), + expected_runs: CachedRuns { + first_faults: named_faults(&[]), + second_faults: named_faults(&[("SQBRPYTHON613", "python/inventory/__init__.py")]), + second_hits: 0, + second_misses: 1, + }, + }, + ]; + for test_case in test_cases { + let runs = cached_runs( + test_case.files, + test_case.config.clone(), + test_case.changes, + test_case.changed_config.clone(), + ); + + assert_eq!( + runs, + Ok(test_case.expected_runs), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_a_planned_project_query_when_caching_it_then_its_path_is_relative_to_the_project() { + let test_cases = [ + RelativeQueryTestCase { + description: "a POSIX path below the project", + project_dir: "/work/proj", + path: "/work/proj/python/inventory", + expected_path: "python/inventory", + }, + RelativeQueryTestCase { + description: "a Windows project directory against Fensu's forward-slash path", + project_dir: "C:\\work\\proj", + path: "C:/work/proj/python/inventory", + expected_path: "python/inventory", + }, + RelativeQueryTestCase { + description: "the project directory itself", + project_dir: "/work/proj", + path: "/work/proj", + expected_path: ".", + }, + RelativeQueryTestCase { + description: "a sibling directory sharing the project's name prefix", + project_dir: "/work/proj", + path: "/work/proj2/python", + expected_path: "/work/proj2/python", + }, + ]; + for test_case in test_cases { + let query = NativeProjectQuery { + kind: "directory_entries".to_owned(), + path: test_case.path.to_owned(), + argument: String::new(), + }; + + let relative = relative_query(query, test_case.project_dir); + + assert_eq!( + relative.path, test_case.expected_path, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests/test_python_catalogue.rs b/crates/sqlbuild-rules/src/python_rules/tests/test_python_catalogue.rs new file mode 100644 index 000000000..46b636470 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/test_python_catalogue.rs @@ -0,0 +1,213 @@ +use fensu_native::rules::constants as fensu; + +use crate::python_rules::tests::helpers::{ + core_rule_faults, distinct_python_codes, is_selected, project_wide_python_rules, source_keys, +}; +use crate::python_rules::tests::test_types::{ + CatalogueTestCase, CoreRuleTestCase, OptionsTestCase, SelectionTestCase, SourceKeyTestCase, +}; +use crate::rules::_helpers::catalogue::catalogue; +use crate::rules::main::builtin_options::builtin_options; +use crate::rules::models::BuiltinOptionKind; + +#[test] +fn given_the_catalogue_when_listing_python_rules_then_each_has_its_own_project_wide_code() { + let test_cases = [ + CatalogueTestCase { + description: "two SQLBuild layout rules and 62 Fensu core rules have distinct codes", + count: distinct_python_codes, + expected_count: 64, + }, + CatalogueTestCase { + description: "every Python rule reports against the project in the python family", + count: project_wide_python_rules, + expected_count: 64, + }, + ]; + for test_case in test_cases { + assert_eq!( + (test_case.count)(&catalogue()), + test_case.expected_count, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_python_selectors_when_selecting_then_opt_in_rules_need_their_exact_code() { + let test_cases = [ + SelectionTestCase { + description: "the family selects a default rule", + select: vec!["SQBRPYTHON".to_owned()], + code: "SQBRPYTHON101", + expected_selected: true, + }, + SelectionTestCase { + description: "the family leaves out opt-in unreachable definitions", + select: vec!["SQBRPYTHON".to_owned()], + code: "SQBRPYTHON306", + expected_selected: false, + }, + SelectionTestCase { + description: "the family leaves out opt-in parameter mutation", + select: vec!["SQBRPYTHON".to_owned()], + code: "SQBRPYTHON507", + expected_selected: false, + }, + SelectionTestCase { + description: "the exact code selects an opt-in rule", + select: vec!["SQBRPYTHON306".to_owned()], + code: "SQBRPYTHON306", + expected_selected: true, + }, + SelectionTestCase { + description: "the whole built-in namespace leaves Python rules out", + select: vec!["SQBR".to_owned()], + code: "SQBRPYTHON101", + expected_selected: false, + }, + SelectionTestCase { + description: "the built-in namespace with one Python code leaves the other Python rules out", + select: vec!["SQBR".to_owned(), "SQBRPYTHON101".to_owned()], + code: "SQBRPYTHON613", + expected_selected: false, + }, + SelectionTestCase { + description: "the built-in namespace with the Python family selects both", + select: vec!["SQBR".to_owned(), "SQBRPYTHON".to_owned()], + code: "SQBRPYTHON101", + expected_selected: true, + }, + SelectionTestCase { + description: "the default selection leaves Python rules out", + select: Vec::new(), + code: "SQBRPYTHON101", + expected_selected: false, + }, + ]; + for test_case in test_cases { + assert_eq!( + is_selected(&test_case.select, test_case.code), + test_case.expected_selected, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_python_source_when_evaluating_a_fensu_core_rule_then_reports_its_faults() { + let test_cases = [ + CoreRuleTestCase { + description: "an unannotated parameter is reported on its line", + source: "def total(amount) -> int:\n return amount\n", + fensu_code: fensu::PARAMETER_ANNOTATION_CODE, + expected_faults: vec![(fensu::PARAMETER_ANNOTATION_CODE.to_owned(), 1)], + }, + CoreRuleTestCase { + description: "an annotated parameter passes", + source: "def total(amount: int) -> int:\n return amount\n", + fensu_code: fensu::PARAMETER_ANNOTATION_CODE, + expected_faults: Vec::new(), + }, + ]; + for test_case in test_cases { + assert_eq!( + core_rule_faults(test_case.source, test_case.fensu_code), + test_case.expected_faults, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_python_rules_when_reading_their_options_then_fensu_thresholds_are_whole_numbers() { + let test_cases = [ + OptionsTestCase { + description: "a shape rule declares its one threshold with Fensu's default", + code: "SQBRPYTHON501", + expected_options: vec![( + "max_statements", + BuiltinOptionKind::Integer { + default: 40, + minimum: 0, + }, + )], + }, + OptionsTestCase { + description: "a package layout rule declares its module count and role depth", + code: "SQBRPYTHON610", + expected_options: vec![ + ( + "max_helpers_container_modules", + BuiltinOptionKind::Integer { + default: 10, + minimum: 0, + }, + ), + ( + "max_role_depth", + BuiltinOptionKind::Integer { + default: 1, + minimum: 0, + }, + ), + ], + }, + OptionsTestCase { + description: "a rule without thresholds declares no options", + code: "SQBRPYTHON101", + expected_options: Vec::new(), + }, + ]; + for test_case in test_cases { + let declared: Vec<(&str, BuiltinOptionKind)> = builtin_options(test_case.code) + .into_iter() + .map(|option| (option.name, option.kind)) + .collect(); + assert_eq!( + declared, test_case.expected_options, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_a_python_edit_when_preparing_rules_then_only_python_selections_see_it() { + const ORIGINAL: &str = "def total(amount: int) -> int:\n return amount\n"; + let test_cases = [ + SourceKeyTestCase { + description: "an edit changes the key when the Python family is selected", + select: &["SQBRPYTHON"], + edited_source: "def total(amount):\n return amount\n", + expected_same_key: false, + }, + SourceKeyTestCase { + description: "unchanged Python keeps the key", + select: &["SQBRPYTHON"], + edited_source: ORIGINAL, + expected_same_key: true, + }, + SourceKeyTestCase { + description: "an edit is not read when only SQL rules run", + select: &["SQBR"], + edited_source: "def total(amount):\n return amount\n", + expected_same_key: true, + }, + ]; + for test_case in test_cases { + assert_eq!( + { + let (before, after) = + source_keys(test_case.select, ORIGINAL, test_case.edited_source); + before == after + }, + test_case.expected_same_key, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests/test_python_scaling.rs b/crates/sqlbuild-rules/src/python_rules/tests/test_python_scaling.rs new file mode 100644 index 000000000..d959136f5 --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/test_python_scaling.rs @@ -0,0 +1,77 @@ +use std::time::Duration; + +use serde_json::json; + +use crate::python_rules::tests::helpers::{ + median_growth, scaling_project, serial_timing, timed_python_codes, +}; +use crate::python_rules::tests::test_types::{PythonGrowthTestCase, PythonScalingTestCase}; + +#[test] +fn given_many_modules_and_dense_findings_when_evaluating_python_rules_then_finishes_within_budget() +{ + let _serial = serial_timing(); + let test_cases = [PythonScalingTestCase { + description: "default rules over 4000 modules and 40 hooks of 300 findings each", + units: 4000, + noisy_files: 40, + functions: 150, + config: json!({ "select": ["SQBRPYTHON"] }), + expected_annotation_faults: 12000, + expected_max_duration: Duration::from_secs(5), + }]; + for test_case in test_cases { + let files = scaling_project(test_case.units, test_case.noisy_files, test_case.functions); + let (codes, elapsed) = timed_python_codes(&files, test_case.config).expect("rules run"); + let annotation_faults: usize = codes.iter().filter(|code| *code == "SQBRPYTHON101").count(); + assert_eq!( + annotation_faults, test_case.expected_annotation_faults, + "{}", + test_case.description + ); + assert!( + elapsed < test_case.expected_max_duration, + "{}: took {elapsed:?}", + test_case.description + ); + } +} + +#[test] +fn given_four_times_the_modules_when_finding_unreachable_definitions_then_time_grows_near_linearly() +{ + let _serial = serial_timing(); + let test_cases = [PythonGrowthTestCase { + description: "unreachable definitions walking every import, 500 against 2000 modules", + units: 500, + noisy_files: 5, + functions: 150, + config: json!({ "select": ["SQBRPYTHON", "SQBRPYTHON306"] }), + growth: 4, + pairs: 5, + expected_unreachable_definitions: 2001, + expected_max_ratio: 6.5, + }]; + for test_case in test_cases { + let small = scaling_project(test_case.units, test_case.noisy_files, test_case.functions); + let large = scaling_project( + test_case.growth * test_case.units, + test_case.growth * test_case.noisy_files, + test_case.functions, + ); + let (codes, _) = timed_python_codes(&large, test_case.config.clone()).expect("rules run"); + let unreachable: usize = codes.iter().filter(|code| *code == "SQBRPYTHON306").count(); + assert_eq!( + unreachable, test_case.expected_unreachable_definitions, + "{}", + test_case.description + ); + let ratio: f64 = median_growth(&small, &large, &test_case.config, test_case.pairs); + assert!( + ratio < test_case.expected_max_ratio, + "{}: growing the project {}x multiplied the time by {ratio:.2}", + test_case.description, + test_case.growth + ); + } +} diff --git a/crates/sqlbuild-rules/src/python_rules/tests/test_types.rs b/crates/sqlbuild-rules/src/python_rules/tests/test_types.rs new file mode 100644 index 000000000..865513b6d --- /dev/null +++ b/crates/sqlbuild-rules/src/python_rules/tests/test_types.rs @@ -0,0 +1,94 @@ +use crate::models::RuleMetadata; +use crate::rules::models::BuiltinOptionKind; + +pub(super) struct CatalogueTestCase { + pub(super) description: &'static str, + pub(super) count: fn(&[RuleMetadata]) -> usize, + pub(super) expected_count: usize, +} + +pub(super) struct SelectionTestCase { + pub(super) description: &'static str, + pub(super) select: Vec, + pub(super) code: &'static str, + pub(super) expected_selected: bool, +} + +pub(super) struct CoreRuleTestCase { + pub(super) description: &'static str, + pub(super) source: &'static str, + pub(super) fensu_code: &'static str, + pub(super) expected_faults: Vec<(String, u32)>, +} + +pub(super) struct OptionsTestCase { + pub(super) description: &'static str, + pub(super) code: &'static str, + pub(super) expected_options: Vec<(&'static str, BuiltinOptionKind)>, +} + +pub(super) struct SourceKeyTestCase { + pub(super) description: &'static str, + pub(super) select: &'static [&'static str], + pub(super) edited_source: &'static str, + pub(super) expected_same_key: bool, +} + +pub(super) struct PythonRunTestCase { + pub(super) description: &'static str, + pub(super) files: &'static [(&'static str, &'static str)], + pub(super) config: serde_json::Value, + pub(super) expected_faults: Vec<(&'static str, &'static str, u64, u64)>, +} + +pub(super) struct PythonParseFailureTestCase { + pub(super) description: &'static str, + pub(super) files: &'static [(&'static str, &'static str)], + pub(super) expected_error: &'static str, +} + +#[derive(Debug, PartialEq)] +pub(super) struct CachedRuns { + pub(super) first_faults: Vec<(String, String)>, + pub(super) second_faults: Vec<(String, String)>, + pub(super) second_hits: u64, + pub(super) second_misses: u64, +} + +pub(super) struct PythonCacheTestCase { + pub(super) description: &'static str, + pub(super) files: &'static [(&'static str, &'static str)], + pub(super) config: serde_json::Value, + pub(super) changes: &'static [(&'static str, &'static str)], + pub(super) changed_config: serde_json::Value, + pub(super) expected_runs: CachedRuns, +} + +pub(super) struct PythonScalingTestCase { + pub(super) description: &'static str, + pub(super) units: usize, + pub(super) noisy_files: usize, + pub(super) functions: usize, + pub(super) config: serde_json::Value, + pub(super) expected_annotation_faults: usize, + pub(super) expected_max_duration: std::time::Duration, +} + +pub(super) struct PythonGrowthTestCase { + pub(super) description: &'static str, + pub(super) units: usize, + pub(super) noisy_files: usize, + pub(super) functions: usize, + pub(super) config: serde_json::Value, + pub(super) growth: usize, + pub(super) pairs: usize, + pub(super) expected_unreachable_definitions: usize, + pub(super) expected_max_ratio: f64, +} + +pub(super) struct RelativeQueryTestCase { + pub(super) description: &'static str, + pub(super) project_dir: &'static str, + pub(super) path: &'static str, + pub(super) expected_path: &'static str, +} diff --git a/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs b/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs index 509a23fe0..283a00ce7 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs @@ -1,6 +1,7 @@ -use crate::constants::{API_VERSION, NATIVE_BUILD_IDENTITY, SQL_RULE_PREFIX}; +use crate::constants::{API_VERSION, NATIVE_BUILD_IDENTITY, PYTHON_RULE_PREFIX, SQL_RULE_PREFIX}; use crate::models::RulesCodeGrammar; use crate::models::{CustomRule, ResolveRulesRequest, RuleGuidance, RuleMetadata, RuleScope}; +use crate::python_rules::main::python_catalogue::python_catalogue; use fensu_policy::policy::errors::PolicyError; use fensu_policy::policy::main::resolve_policy::resolve_policy; use fensu_policy::policy::models::PolicySelectors; @@ -391,6 +392,7 @@ pub(crate) fn catalogue() -> Vec { scope: RuleScope::SqlLint, }), ); + rules.extend(python_catalogue()); rules.sort_by(|a, b| a.code.cmp(&b.code)); rules } @@ -480,9 +482,30 @@ pub(crate) fn select<'a>( warn: Vec::new(), ignore: ignored.to_vec(), }; - resolve_policy(&references, &(), &effective, &grammar) - .map(|ruleset| ruleset.blocking) - .map_err(rules_error) + let blocking: Vec<&RuleMetadata> = resolve_policy(&references, &(), &effective, &grammar) + .map_err(rules_error)? + .blocking; + Ok(without_unnamed_python(blocking, selected_or_none, &grammar)) +} + +/// Keep a Python rule only when a selector inside its family matches it; `SQBR` alone selects none. +fn without_unnamed_python<'a>( + rules: Vec<&'a RuleMetadata>, + selectors: &[String], + grammar: &RulesCodeGrammar, +) -> Vec<&'a RuleMetadata> { + let python_selectors: Vec = selectors + .iter() + .filter(|selector| selector.starts_with(PYTHON_RULE_PREFIX)) + .cloned() + .collect(); + rules + .into_iter() + .filter(|rule| { + !rule.scope.evaluates_python() + || matches_any_selector(grammar, &rule.code, &python_selectors) + }) + .collect() } /// SQLClean's selection: its default Rules without a selection, and every Rule a selector names, whatever its default. diff --git a/crates/sqlbuild-rules/src/rules/main/builtin_options.rs b/crates/sqlbuild-rules/src/rules/main/builtin_options.rs index 41c222fea..051606f1f 100644 --- a/crates/sqlbuild-rules/src/rules/main/builtin_options.rs +++ b/crates/sqlbuild-rules/src/rules/main/builtin_options.rs @@ -3,6 +3,7 @@ use crate::constants::{ ALLOWED_QUALIFIERS_OPTION, DEFAULT_PUBLISHED_LAYERS, PUBLISHED_LAYERS_OPTION, }; +use crate::python_rules::main::python_rule_options::python_rule_options; use crate::rules::models::{BuiltinOption, BuiltinOptionKind}; const fn boolean(name: &'static str, description: &'static str) -> BuiltinOption { @@ -72,6 +73,6 @@ pub fn builtin_options(code: &str) -> Vec { description: "Reviewed SQL test names that may assert only that empty inputs produce \ no rows; listed tests count toward min_tests_per_model.", }], - _ => Vec::new(), + _ => python_rule_options(code), } } diff --git a/crates/sqlbuild-rules/src/rules/models.rs b/crates/sqlbuild-rules/src/rules/models.rs index cc882b495..ef37e8989 100644 --- a/crates/sqlbuild-rules/src/rules/models.rs +++ b/crates/sqlbuild-rules/src/rules/models.rs @@ -80,6 +80,8 @@ pub enum BuiltinOptionKind { StringList(&'static [&'static str]), /// A list of tables, each checked by the rule's own configuration validation. TableList, + /// A whole number of at least `minimum`, with the given default. + Integer { default: u32, minimum: u32 }, } /// One option a built-in rule declares. diff --git a/scripts/cold_compile_performance/_helpers/dense_project.py b/scripts/cold_compile_performance/_helpers/dense_project.py index 0403ec439..608daf546 100644 --- a/scripts/cold_compile_performance/_helpers/dense_project.py +++ b/scripts/cold_compile_performance/_helpers/dense_project.py @@ -12,6 +12,7 @@ DENSE_MACRO_QUANTITY_COLUMNS, DENSE_MACRO_SHARE, DENSE_MEDIUM_CTE_PERCENTILE, + DENSE_RULE_SELECT, DENSE_UNION_MEDIUM_INDEX, DENSE_UNION_TAIL_INDEX, ) @@ -76,7 +77,7 @@ def write_dense_compile_project(*, project_dir: Path, model_count: int) -> None: (project_dir / "sqlbuild_project.toml").write_text( 'name = "dense_orders"\nadapter = "snowflake"\n' '[defaults]\ndatabase = "warehouse"\nschema = "analytics"\n' - f'[rules]\nselect = ["SQBR", "SQLCLEAN", "XSQBR"]\ndomains = [{domains}]\n', + f"[rules]\n{DENSE_RULE_SELECT}\ndomains = [{domains}]\n", encoding="utf-8", ) source_count: int = max(1, model_count * 24 // 100) diff --git a/scripts/cold_compile_performance/constants.py b/scripts/cold_compile_performance/constants.py index d964f2189..63ed73b06 100644 --- a/scripts/cold_compile_performance/constants.py +++ b/scripts/cold_compile_performance/constants.py @@ -3,6 +3,9 @@ UNLIMITED_CGROUP_MEMORY_VALUE: str = "max" COMPILE_MEMORY_LIMIT_ENV_VAR: str = "SQLBUILD_COMPILE_MEMORY_LIMIT_BYTES" COMPILE_MEASUREMENT_FIELD_COUNT: int = 6 +DENSE_RULE_SELECT: str = ( + 'select = ["SQBR", "SQBRPYTHON", "SQBRPYTHON306", "SQBRPYTHON507", "SQLCLEAN", "XSQBR"]' +) DENSE_JOIN_THRESHOLDS: tuple[int, ...] = (55, 75, 90, 99) DENSE_JOIN_COUNTS: tuple[int, ...] = (0, 1, 4, 8, 20) diff --git a/scripts/delta_compile_series/constants.py b/scripts/delta_compile_series/constants.py index 7c8a33a4f..daada218e 100644 --- a/scripts/delta_compile_series/constants.py +++ b/scripts/delta_compile_series/constants.py @@ -20,6 +20,7 @@ STALE_PROBLEMS: str = "reports problems in files the edit leaves alone" AFTER_FAILED_RULES: str = "the previous compile failed its Rules" PROJECT_CONFIG: str = "sqlbuild_project.toml" +DELTA_RULE_SELECT: str = 'select = ["SQBR", "SQLCLEAN", "XSQBR"]' RANDOM_VALUE_LIMIT: int = 10**6 FULL_COPY_SUFFIX: str = "_full_copy" LOG_RUNS_GLOB: str = "logs/*/*" diff --git a/scripts/delta_compile_series/main/write_delta_compile_project.py b/scripts/delta_compile_series/main/write_delta_compile_project.py index 5735eddd4..128fb0df4 100644 --- a/scripts/delta_compile_series/main/write_delta_compile_project.py +++ b/scripts/delta_compile_series/main/write_delta_compile_project.py @@ -5,7 +5,9 @@ from pathlib import Path from scripts.cold_compile_performance._helpers.dense_project import write_dense_compile_project +from scripts.cold_compile_performance.constants import DENSE_RULE_SELECT from scripts.delta_compile_series.constants import ( + DELTA_RULE_SELECT, PROJECT_CONFIG, SQL_GLOB, TSQL_ADAPTER, @@ -21,9 +23,9 @@ def write_delta_compile_project( write_dense_compile_project(project_dir=project_dir, model_count=model_count) config: Path = project_dir / PROJECT_CONFIG config.write_text( - config.read_text(encoding="utf-8").replace( - 'adapter = "snowflake"', f'adapter = "{adapter}"' - ), + config.read_text(encoding="utf-8") + .replace('adapter = "snowflake"', f'adapter = "{adapter}"') + .replace(DENSE_RULE_SELECT, DELTA_RULE_SELECT), encoding="utf-8", ) if adapter != TSQL_ADAPTER: diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md index 6b4a04a62..10800fcd1 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md @@ -48,6 +48,7 @@ Bundled copies of every page on the SQLBuild documentation site, matching the in - [Overview](concepts/rules.md) (`concepts/rules`) - Turn repeated SQL and project review decisions into compiler-enforced requirements. - [Configuration and selection](concepts/rules/configuration-and-selection.md) (`concepts/rules/configuration-and-selection`) - Select built-in and custom Rules by exact code or family prefix. - [Findings and exceptions](concepts/rules/findings-and-exceptions.md) (`concepts/rules/findings-and-exceptions`) - Understand Rule diagnostics and record intentional exceptions safely. +- [Python Rules](concepts/rules/python-rules.md) (`concepts/rules/python-rules`) - Check the project's Python for annotations, imports, function shape and the layout of python/. - [Execution and caching](concepts/rules/execution-and-caching.md) (`concepts/rules/execution-and-caching`) - Understand Rule enforcement order, focused runs, and dependency-aware reuse. ## Ship diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/python-nodes/overview.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/python-nodes/overview.md index c4c2db12e..235e4a2ab 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/python-nodes/overview.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/python-nodes/overview.md @@ -57,7 +57,7 @@ python/ factories/regional.py ``` -Grouping by domain (`python/orders/`, `python/customers/`) works just as well, and a single file may mix kinds. Import other project modules through the `python` package, for example `from python.tasks.orders import export_orders` or `from python._helpers import normalize_status`. +Grouping by domain (`python/orders/`, `python/customers/`) works just as well, and a single file may mix kinds. A project that selects the [Python Rules](../rules/python-rules.md#laying-out-python) commits to grouping by domain: `SQBRPYTHON001` reports node-kind folders such as `python/loaders/` and modules directly under `python/`. Import other project modules through the `python` package, for example `from python.tasks.orders import export_orders` or `from python._helpers import normalize_status`. Code that extends SQLBuild itself keeps its own folder, such as `rules/`, `materializations/`, `adapters/`, `providers/`, `sinks/`, `hooks/python/`, `functions/python/`, and `macros/`. Python outside the supported locations fails compilation with `D016`; see [Project Python](../project-configuration.md#project-python) for the full list. diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md index 67b8419a8..73274c201 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md @@ -161,7 +161,9 @@ passes its configured Rules. This keeps diagnostics and source mutation separate SQLBuild maintains native built-in Rules for reusable requirements. SQL Rule codes begin with `SQLCLEAN`, for example `SQLCLEAN004`. Rules that read the project, such as `SQBRGRAPH101`, begin -with `SQBR`. +with `SQBR`. The `SQBRPYTHON` Rules check the project's Python: annotations, imports, function +shape and the layout of `python/`. They run only when `select` names `SQBRPYTHON` or one of its +codes; see [Python Rules](rules/python-rules.md). Projects can define custom Python Rules under `rules/**/*.py`. Their codes begin with `XSQBR`, for example `XSQBRARCH001`. Custom Rules use the same selection, findings, exceptions, execution order, @@ -244,6 +246,7 @@ Rules add about 14% to a cold compile and nothing to a warm one. See Select exact Rules or families and configure project-owned options. Understand diagnostics, exact exceptions, path ignores, and stale checks. + Check the project's Python for annotations, imports, function shape and the layout of `python/`. Keep each domain's published layer free of consumer-specific variants. Learn evaluation order, focused execution, and dependency-aware reuse. List, inspect, run, and generate guidance for configured Rules. diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md index 030432523..d9a093ace 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md @@ -49,6 +49,11 @@ XSQBR001 → XSQBR Exact codes select one Rule. Families select every matching Rule, whether or not it is enabled by default. `ignore` uses the same matching semantics and takes precedence over `select`. +The Python Rules are the exception. `SQBR` does not select them: they run only when a selector names +`SQBRPYTHON` or one of its codes, as in `select = ["SQBR", "SQBRPYTHON"]`. `SQBRPYTHON` selects the +Python Rules that are enabled by default, and the others need their exact code. `ignore = ["SQBR"]` +still turns them off. See [Python Rules](python-rules.md#selecting-the-python-rules). + A selector that matches no Rule fails with `C000`. `SQBRTEST105` (scenario description) was retired when descriptions became the compile error `P010` ([Descriptions](../models/configuration.md#descriptions)); remove it from `select` and @@ -83,6 +88,9 @@ reviewed exceptions. Listed tests are not flagged and still count toward `SQBRTE optional `domain`; `qualifier` and `paths` accept globs. See [Published-layer Rules](published-layer-rules.md). +The Python Rules take whole-number limits such as `max_arguments` for `SQBRPYTHON504`; see +[Python Rules](python-rules.md#rule-options). + Unknown Rule codes, option names, and invalid option values fail configuration. ## Focused selection diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md index 1b6a0d345..c0b2fddca 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/execution-and-caching.md @@ -40,6 +40,10 @@ Model-subject Rules support fine-grained reuse. Editing one model invalidates af subjects rather than every Rule invocation. Project-subject Rules run once and are appropriate for genuinely project-wide invariants, with correspondingly broader invalidation. +Python Rules are cached per file. Editing a Python file re-checks that file and the files whose +findings read it, such as an importer checked against the module it imports. `SQBRPYTHON305` and +`SQBRPYTHON306` read every Python file, so any Python edit re-checks them. + Positive and negative project-tree observations are dependencies. If a Rule observes that a glob has no matches, adding a matching file invalidates that result. diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/python-rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/python-rules.md new file mode 100644 index 000000000..c3d55f151 --- /dev/null +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/python-rules.md @@ -0,0 +1,267 @@ + + +# Python Rules + +> Check the project's Python for annotations, imports, function shape and the layout of python/. + +Online: https://sqlbuild.com/docs/concepts/rules/python-rules/ + +## Contents + +- Selecting the Python Rules +- Which files are checked +- Laying out `python/` +- Main modules and entry modules +- Unreachable definitions +- Rule options +- Rule reference +- Details + +The `SQBRPYTHON` Rules check the Python in your project the way `SQLCLEAN` checks its SQL. Select +the family and `sqb compile` reports each finding with its file, line and column: + +```toml +[rules] +select = ["SQBR", "SQBRPYTHON"] +``` + +```text +error[SQBRPYTHON101]: function parameter 'ctx' must define a type annotation + --> hooks/python/alert.py:4:11 + = help: Annotate every parameter with the value type accepted by the function. +``` + +## Selecting the Python Rules + +The Python Rules run only when a selector names `SQBRPYTHON` or one of its codes. `SQBR` on its own +leaves the whole family out, so a project that selects `SQBR` keeps its current findings until it +adds `SQBRPYTHON`. + +| Selection | Python Rules that run | +|---|---| +| no `select`, or `select = ["SQBR"]` | none | +| `select = ["SQBRPYTHON"]` | every Rule enabled by default | +| `select = ["SQBRPYTHON", "SQBRPYTHON306"]` | the defaults and `SQBRPYTHON306` | +| `select = ["SQBRPYTHON101"]` | `SQBRPYTHON101` only | + +`SQBRPYTHON306` (unreachable definitions) and `SQBRPYTHON507` (parameter mutation) are off by +default. Select them by their exact code; the family selector does not include them. + +`ignore` matches by prefix as usual, so `ignore = ["SQBR"]` also turns the Python Rules off. + +Try the family before adding it to `select`: + +```bash +sqb rules run SQBRPYTHON +``` + +## Which files are checked + +The Python Rules read every supported project Python file: the runtime package under `python/` and +the folders that extend SQLBuild, such as `hooks/python/`, `functions/python/`, `providers/`, +`rules/` and `macros/`. See [Project Python](../project-configuration.md#project-python) +for the full list. Files under `tests/` are not checked. + +Rules marked `python/` in the [reference](#rule-reference) check only the runtime package, because +they describe how its packages are organised. The others check every file. + +## Laying out `python/` + +`SQBRPYTHON001` and `SQBRPYTHON002` keep `python/` organised by what the code is for, not by what +kind of node it holds. Directly under `python/`, keep only domain packages, `utils/` for code no +domain owns, and `__init__.py`: + +```text +python/ + __init__.py + fulfillment/ + shipment_export/ + node.py # @task shipment_export + main.py # export_shipments, the unit's one entry + utils/ + currency/ + format.py +``` + +These layouts are reported: + +| Finding | Example | Fix | +|---|---|---| +| A module directly under `python/` | `python/report.py` | Move it into the domain that owns it | +| A folder named for a node kind or a role | `python/loaders/`, `python/_helpers/` | Group the nodes by domain instead | +| A package with a generic name | `python/inventory/common/` | Name it after what it owns | + +The generic names are `base`, `common`, `general`, `helpers`, `lib`, `misc`, `shared`, `util` and +`utils`. `utils/` is allowed directly under `python/`. A finding about a folder points at its +`__init__.py`, or at its first Python file when it has none. + +## Main modules and entry modules + +Some Rules apply only to a domain's main modules: files under a `main/` folder, and a unit's +`main.py`, such as `python/fulfillment/shipment_export/main.py`. A main module that is not +`__init__.py` is an entry module. `SQBRPYTHON617` requires it to expose one public function, and +`SQBRPYTHON305` requires something outside its domain to import it. + +`SQBRPYTHON501` to `SQBRPYTHON503` (statements, distinct calls and locals) check main modules only. +The other size limits check every function. + +## Unreachable definitions + +`SQBRPYTHON306` reports functions, classes and module values that nothing SQLBuild loads can reach. +SQLBuild loads every decorated function under `python/` (`@loader`, `@task`, `@asset`, `@check`, +`@factory` and `@audit_factory`) and everything defined outside `python/`, such as hooks and +providers. A definition reached only through them is live. Imports through folders without an +`__init__.py` count too. + +Code that SQLBuild loads in a way the Rule cannot follow, such as by name at run time, needs a rule +exception with a reason: + +```toml +[[rules.rule_exceptions]] +rule = "SQBRPYTHON306" +path = "python/fulfillment/carriers/registry.py" +reason = "Carriers are looked up by name from the shipment's carrier code." +``` + +## Rule options + +Configure the size limits by exact code: + +```toml +[rules.rule_options.SQBRPYTHON504] +max_arguments = 6 +``` + +| Rule | Option | Default | Meaning | +|---|---|---|---| +| `SQBRPYTHON501` | `max_statements` | 40 | Most statements one main-module function may hold. | +| `SQBRPYTHON502` | `max_distinct_calls` | 20 | Most distinct callees one main-module function may call. | +| `SQBRPYTHON503` | `max_locals` | 20 | Most local names one main-module function may bind. | +| `SQBRPYTHON504` | `max_arguments` | 10 | Most parameters one function may declare. | +| `SQBRPYTHON505` | `max_statements_global` | 70 | Most statements any function may hold. | +| `SQBRPYTHON610` | `max_helpers_container_modules` | 10 | Most modules one `_helpers/` package may hold directly. | +| `SQBRPYTHON610` | `max_role_depth` | 1 | How many package levels may sit below a role package. | +| `SQBRPYTHON611` | `max_main_container_modules` | 20 | Most modules one `main/` package may hold directly. | +| `SQBRPYTHON611` | `max_role_depth` | 1 | How many package levels may sit below a role package. | +| `SQBRPYTHON615` | `min_shared_domain_prefix_packages` | 2 | Fewest sibling domains sharing a name prefix that are reported; 0 turns the rule off. | +| `SQBRPYTHON627` | `max_file_lines` | 2000 | Most lines one Python file may hold. | + +Each option takes a whole number of 0 or more. Unknown option names, text values and negative +numbers fail configuration with `C000`. + +## Rule reference + +`Default` says whether `SQBRPYTHON` selects the Rule. `sqb rules show ` prints the full +remediation. + +### Layout of `python/` + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON001` | project-root-allowlist | `python/` | yes | `python/` may hold only domain packages, `utils/` and `__init__.py` | +| `SQBRPYTHON002` | runtime-package-owner-names | `python/` | yes | Packages under `python/` must be named for what they own | + +### Type annotations + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON101` | parameter-annotation | All | yes | Function parameters must define type annotations | +| `SQBRPYTHON102` | return-annotation | All | yes | Functions must define return type annotations | +| `SQBRPYTHON103` | module-variable-annotation | All | yes | Module-level variables must define type annotations | +| `SQBRPYTHON104` | class-attribute-annotation | All | yes | Class attributes must define type annotations | +| `SQBRPYTHON105` | local-variable-annotation | All | yes | Local variables must define type annotations on first binding unless assigned a scalar literal | + +### Hygiene + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON201` | single-line-docstrings | All | yes | Docstrings must be a single line; move extended explanation into docs or tests | +| `SQBRPYTHON202` | no-standalone-comments | All | yes | Standalone comments are not allowed; prefer clear names or docs/tests | +| `SQBRPYTHON203` | no-raw-builtin-raise | All | yes | Runtime code must raise structured errors instead of raw built-in exceptions | +| `SQBRPYTHON204` | no-assert-in-runtime | All | yes | Runtime code must not use assert for invariants; raise a structured error | +| `SQBRPYTHON205` | no-swallowed-exception-probe | All | yes | Runtime code must not swallow broad exceptions as existence probe answers | +| `SQBRPYTHON206` | no-unnamed-string-decisions | All | yes | String literals must not directly control comparison behavior | +| `SQBRPYTHON207` | no-magic-numeric-comparisons | All | yes | Non-canonical numeric literals must not directly control comparisons | +| `SQBRPYTHON208` | no-import-time-side-effects | All | yes | Runtime and tooling modules must not execute standalone calls during import | + +### Imports and reachability + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON301` | absolute-imports-only | All | yes | Use absolute imports; relative imports hide package boundaries | +| `SQBRPYTHON302` | no-star-imports | All | yes | Star imports hide names from dependency-boundary analysis | +| `SQBRPYTHON303` | no-internal-public-surface-imports | `python/` | yes | Internal code must import from the owning module, not the bare package | +| `SQBRPYTHON304` | no-cross-domain-private-main-imports | `python/` | yes | Domain-private main entries may only be imported within their owning domain | +| `SQBRPYTHON305` | public-main-entry-external-use | `python/` | yes | Public main entries must have an importer outside their owning domain | +| `SQBRPYTHON306` | unreachable-definition | `python/` | no | Definition is unreachable from production roots | +| `SQBRPYTHON307` | no-cross-file-use-of-helper-private-class | `python/` | yes | Helper-private classes are file-local details; move shared classes to `classes/` | + +### Naming contracts + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON401` | validator-must-not-return | All | yes | Functions under no-return naming contracts must not return values | +| `SQBRPYTHON402` | predicate-must-return-bool | All | yes | Predicate names must declare an ordinary boolean result | +| `SQBRPYTHON403` | value-name-must-return-value | All | yes | Value-producing names must not declare a no-value result | +| `SQBRPYTHON404` | iterator-name-must-produce-iterator | All | yes | Iterator names must produce an iterator or generator | + +### Function shape + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON501` | too-many-statements | All | yes | Main functions must stay phase-shaped and below the statement limit | +| `SQBRPYTHON502` | too-many-distinct-calls | All | yes | Main functions must not coordinate too many distinct callees | +| `SQBRPYTHON503` | too-many-locals | All | yes | Main functions must not juggle too many local variables | +| `SQBRPYTHON504` | max-arguments | All | yes | Functions must stay below the configured argument limit | +| `SQBRPYTHON505` | max-statements-global | All | yes | Functions must stay below the global statement limit | +| `SQBRPYTHON506` | meaningful-project-result-discarded | All | yes | Main orchestrators must consume meaningful project-local call results | +| `SQBRPYTHON507` | parameter-mutation-in-phase-helpers | All | no | Helpers must return values instead of mutating parameters | +| `SQBRPYTHON508` | default-mutation-return | All | yes | Functions that mutate parameters must return every mutated parameter | +| `SQBRPYTHON509` | no-outer-state-mutation | All | yes | Functions must not mutate module-global or closure-captured state | +| `SQBRPYTHON510` | no-complex-comprehensions | All | yes | Nested or multi-generator comprehensions hide control flow and data shapes | +| `SQBRPYTHON511` | mutable-result-model | All | yes | Dataclass result models must be frozen | + +### Module roles and package layout + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON601` | models-only-models | `python/` | yes | Models role files may contain only structured runtime models | +| `SQBRPYTHON602` | types-only-types | `python/` | yes | Types role files may contain only type-layer declarations | +| `SQBRPYTHON603` | constants-only-constants | `python/` | yes | Constants role files may contain only assignments and imports | +| `SQBRPYTHON604` | exceptions-only-exceptions | `python/` | yes | Exceptions role files may contain only custom exceptions | +| `SQBRPYTHON605` | constant-outside-constants | `python/` | yes | Public uppercase constants must be defined in the constants role | +| `SQBRPYTHON606` | banned-generic-filename | All | yes | Generic filenames hide module ownership | +| `SQBRPYTHON607` | helpers-module-name | `python/` | yes | Use an _helpers package instead of `helpers.py` | +| `SQBRPYTHON608` | classes-module-name | `python/` | yes | Use a classes package instead of `classes.py` | +| `SQBRPYTHON609` | helpers-classes-file-private | `python/` | yes | Plain classes in _helpers modules must be file-private | +| `SQBRPYTHON610` | helpers-package-layout | `python/` | yes | `_helpers/` packages must use bounded flat-or-grouped containers | +| `SQBRPYTHON611` | main-package-layout | `python/` | yes | `main/` packages must use bounded flat-or-grouped orchestration containers | +| `SQBRPYTHON612` | helpers-reserved-role-filenames | `python/` | yes | `_helpers/` packages must not contain reserved role filenames | +| `SQBRPYTHON613` | top-level-domain-shape | `python/` | yes | Top-level domains must be either role leaves or subdomain branches | +| `SQBRPYTHON614` | top-level-direct-modules | `python/` | yes | Runtime roots and top-level domains must not contain ad hoc direct modules | +| `SQBRPYTHON615` | shared-domain-prefix | `python/` | yes | Sibling domains must not encode one parent domain through a shared name prefix | +| `SQBRPYTHON616` | public-facade-import-direction | `python/` | yes | Runtime modules must not import public facade modules | +| `SQBRPYTHON617` | entry-module-shape | `python/` | yes | `main/` entry modules must expose one focused public function | +| `SQBRPYTHON618` | init-module-empty | `python/` | yes | Nested `__init__.py` files must be empty or docstring-only | +| `SQBRPYTHON619` | no-reexport-shim | `python/` | yes | Internal modules must not exist only to re-export imports | +| `SQBRPYTHON620` | no-internal-helper-exports | `python/` | yes | `_helpers/` modules must not publish an `__all__` surface | +| `SQBRPYTHON621` | main-entry-name-collision | `python/` | yes | `main/` cannot define a module and package with the same entry name | +| `SQBRPYTHON622` | public-surface-shape | `python/` | yes | Root package surfaces may contain only imports and one `__all__` declaration | +| `SQBRPYTHON623` | classes-one-class-per-module | `python/` | yes | `classes/` modules must define exactly one top-level class | +| `SQBRPYTHON624` | helpers-package-shape | `python/` | yes | `_helpers/` packages must contain no main.py orchestration entrypoints | +| `SQBRPYTHON625` | private-definition-ordering | `python/` | yes | Private constants and dataclasses must appear before top-level functions | +| `SQBRPYTHON626` | classes-runtime-reexport | `python/` | yes | `classes/` modules must not publicly re-export imported runtime symbols | +| `SQBRPYTHON627` | source-file-line-count | All | yes | Source files must stay below the configured line limit | + +## Details + +- Findings, exceptions, ignores and stale checks work as for every Rule; see + [Findings and exceptions](findings-and-exceptions.md). An exception covers one + Rule in one file. +- Findings on a file or a folder point at line 1, column 1. Columns count characters, so text before + the finding in another script does not shift them. +- SQLBuild reads the files with the syntax of the Python it runs on. A file it cannot read fails the + run with `cannot parse Python source` and the file's position. +- Editing one file re-checks it and the files whose findings read it. `SQBRPYTHON305` and + `SQBRPYTHON306` read the whole project, so any Python edit re-checks them. See + [Execution and caching](execution-and-caching.md). diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py index 9d997e32e..779eff63a 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py @@ -146,7 +146,12 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ assert timings["analysis_cache_bypasses"] == test_case.model_count assert timings["analysis_entry_cache_hits"] == timings["analysis_batch_cache_hits"] == 0 assert timings["rule_cache_hits"] == 0 - assert timings["rule_cache_misses"] == 2 * test_case.model_count + 1 + # Each model misses in model rules and SQL lint, the custom rule once, and the Python rules + # once per project Python file: the custom rule module and every Python macro. + python_files: int = len(list(project_dir.glob("rules/*.py"))) + len( + list(project_dir.glob("models/**/macros/python/**/*.py")) + ) + assert timings["rule_cache_misses"] == 2 * test_case.model_count + 1 + python_files assert timings["built_in_rules_ms"] > 0 assert timings["custom_rules_ms"] > 0 assert timings["custom_rules_cpu_ms"] > 0 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 a0984474b..043b4ffbe 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -1163,6 +1163,29 @@ class SqlRuleCacheSequenceTestCase: expected_hits_and_misses: tuple[tuple[int, int], ...] +@dataclass(frozen=True) +class PythonRulesIntegrationTestCase: + """One compile of a DuckShop Python-rules fixture under a `[rules]` table.""" + + description: str + fixture: str + rules_toml: str + expected_exit_code: int + expected_findings: tuple[tuple[str, str | None, int | None, int | None], ...] + expected_text: str = "" + + +@dataclass(frozen=True) +class PythonRuleCacheSequenceTestCase: + """Compiles after each `(project toml, alert hook source)` step, with each step's cache counts + and whether a Python rules cache exists afterwards.""" + + description: str + steps: tuple[tuple[str, str], ...] + expected_hits_and_misses: tuple[tuple[int, int], ...] + expected_python_cache: tuple[bool, ...] + + @dataclass(frozen=True) class CustomRuleCacheSequenceTestCase: """Compiles after each `(rule source, model SQL)` step and the cache counts each reports.""" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py index ca3df8681..f161d036b 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py @@ -1354,3 +1354,21 @@ def _default_interrupt_handling() -> None: """Give the spawned command default Ctrl-C handling even when its parent ignores SIGINT.""" _ = signal.signal(signal.SIGINT, signal.SIG_DFL) + + +PYTHON_RULES_FIXTURES: Path = Path("benchmarks/duckshop/edge_cases/python_rules") + + +def write_python_rules_project(*, project_dir: Path, fixture: str, rules_toml: str) -> None: + """Copy one DuckShop Python-rules fixture with one model under the given `[rules]` table.""" + + shutil.copytree(PYTHON_RULES_FIXTURES / fixture, project_dir, dirs_exist_ok=True) + write_project_files( + project_dir=project_dir, + files={ + "sqlbuild_project.toml": ( + f'name = "orders"\nadapter = "duckdb"\n\n[rules]\n{rules_toml}' + ), + "models/orders.sql": 'MODEL (description "Orders");\nSELECT 1 AS order_id\n', + }, + ) diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_dense_compile_fixture.py b/tests/integration/src/sqlbuild/cli/commands/main/test_dense_compile_fixture.py index f054070a0..88edda5aa 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_dense_compile_fixture.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_dense_compile_fixture.py @@ -14,7 +14,7 @@ @pytest.mark.parametrize( "test_case", - (DenseCompileFixtureTestCase("joins unions macros and functions", 64, 129, 134),), + (DenseCompileFixtureTestCase("joins unions macros and functions", 64, 131, 134),), ids=lambda case: case.description, ) def test_given_dense_fixture_when_compiling_and_testing_then_all_rules_and_rows_pass( diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index 032c97ead..3b3cad7f4 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -36,6 +36,8 @@ ImplicitAliasRuleIntegrationTestCase, NumericRangeDecisionIntegrationTestCase, ProjectRuleScratchStateTestCase, + PythonRuleCacheSequenceTestCase, + PythonRulesIntegrationTestCase, RulePassIntegrationTestCase, RulesIntegrationTestCase, SetOperationStarRuleTestCase, @@ -44,6 +46,7 @@ ) from tests.integration.src.sqlbuild.cli.commands.main.helpers import ( EARLY_LINT_PROJECT_FILES, + PYTHON_RULES_FIXTURES, child_processes, compile_finding_keys, compile_findings, @@ -52,6 +55,7 @@ write_custom_host_project, write_project_files, write_project_scratch_rules, + write_python_rules_project, ) _CACHE_RULES_TOML: str = ( @@ -77,6 +81,18 @@ def final_directory(*, model: Model, ctx: RuleContext) -> list[Finding]: return [] if "final" in model.path.parts else [ctx.finding(subject=model)] """ _CUSTOM_CACHE_EDITED_RULE: str = _CUSTOM_CACHE_RULE.replace('"final" in', '"staging" in') +_PYTHON_RULES_EXPECTED: dict[str, Any] = json.loads( + (PYTHON_RULES_FIXTURES / "expected_findings.json").read_text(encoding="utf-8") +) +_PYTHON_RULES_SELECT: str = f"select = {json.dumps(_PYTHON_RULES_EXPECTED['select'])}\n" +_PYTHON_ALERT_PATH: str = "hooks/python/alert.py" +_PYTHON_ALERT: str = (PYTHON_RULES_FIXTURES / "violations" / _PYTHON_ALERT_PATH).read_text( + encoding="utf-8" +) +_PYTHON_ANNOTATED_ALERT: str = _PYTHON_ALERT.replace("def alert(ctx,", "def alert(ctx: object,") +_PYTHON_CACHE_TOML: str = ( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRPYTHON101"]\n' +) _CUSTOM_CACHE_MODEL_SQL: str = 'MODEL (description "Orders");\nSELECT 1 AS order_id\n' _CUSTOM_CACHE_EDITED_SQL: str = _CUSTOM_CACHE_MODEL_SQL.replace("SELECT 1", "SELECT 2") @@ -3345,3 +3361,184 @@ def test_given_star_in_set_operation_branches_when_compiling_then_each_branch_is assert exit_code == 1 assert tuple(item["code"] for item in diagnostics) == test_case.expected_codes assert len({item["line"] for item in diagnostics}) == test_case.expected_distinct_lines + + +@pytest.mark.parametrize( + "test_case", + [ + PythonRulesIntegrationTestCase( + description="every DuckShop violation is reported where it is", + fixture="violations", + rules_toml=_PYTHON_RULES_SELECT, + expected_exit_code=1, + expected_findings=tuple( + (code, path, line, column) + for code, path, line, column in _PYTHON_RULES_EXPECTED["findings"] + ), + ), + PythonRulesIntegrationTestCase( + description="the conforming DuckShop layout passes every Python rule", + fixture="conforming", + rules_toml=_PYTHON_RULES_SELECT, + expected_exit_code=0, + expected_findings=(), + ), + PythonRulesIntegrationTestCase( + description="the built-in namespace leaves the Python family out", + fixture="violations", + rules_toml=( + 'select = ["SQBR"]\n\n[[rules.rule_ignores]]\nrules = ["SQBR"]\n' + 'paths = ["models/orders.sql"]\nreason = "Only Python findings matter here."\n' + ), + expected_exit_code=0, + expected_findings=(), + ), + PythonRulesIntegrationTestCase( + description="a rule exception suppresses a Python finding", + fixture="violations", + rules_toml=( + 'select = ["SQBRPYTHON101"]\n\n[[rules.rule_exceptions]]\n' + 'rule = "SQBRPYTHON101"\npath = "hooks/python/alert.py"\n' + 'reason = "The alert hook keeps its documented signature."\n' + ), + expected_exit_code=0, + expected_findings=(), + ), + PythonRulesIntegrationTestCase( + description="a rule exception that suppresses nothing fails as stale", + fixture="violations", + rules_toml=( + 'select = ["SQBRPYTHON101"]\n\n[[rules.rule_exceptions]]\n' + 'rule = "SQBRPYTHON101"\npath = "python/report.py"\nreason = "Legacy."\n' + ), + expected_exit_code=1, + expected_findings=(("C000", None, None, None),), + expected_text=( + "stale rule exception suppresses no finding: SQBRPYTHON101 at python/report.py" + ), + ), + PythonRulesIntegrationTestCase( + description="a rule option tightens a Python threshold", + fixture="violations", + rules_toml=( + 'select = ["SQBRPYTHON504"]\n\n' + "[rules.rule_options.SQBRPYTHON504]\nmax_arguments = 1\n" + ), + expected_exit_code=1, + expected_findings=(("SQBRPYTHON504", "hooks/python/alert.py", 4, 1),), + expected_text="function has 2 parameters", + ), + ], + ids=lambda case: case.description, +) +def test_given_project_python_when_compiling_with_python_rules_then_findings_follow_configuration( + test_case: PythonRulesIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_python_rules_project( + project_dir=tmp_path, fixture=test_case.fixture, rules_toml=test_case.rules_toml + ) + + exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + python_diagnostics: list[dict[str, Any]] = json.loads(capsys.readouterr().out)["diagnostics"] + + assert exit_code == test_case.expected_exit_code + assert ( + tuple( + (item["code"], item.get("path"), item.get("line"), item.get("column")) + for item in python_diagnostics + ) + == test_case.expected_findings + ) + assert test_case.expected_text in json.dumps(python_diagnostics) + + +@pytest.mark.parametrize( + "test_case", + [RulesIntegrationTestCase("rules run reports what compile reports", 1, "SQBRPYTHON")], + ids=lambda case: case.description, +) +def test_given_project_python_when_running_python_rules_then_findings_match_compile( + test_case: RulesIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_python_rules_project( + project_dir=tmp_path, + fixture="violations", + rules_toml=f'select = ["{test_case.expected_code}"]\n', + ) + + run_exit: int = main( + ["--project-dir", str(tmp_path), "rules", "--json", "run", test_case.expected_code] + ) + run_findings: list[dict[str, Any]] = json.loads(capsys.readouterr().out)["findings"] + compiled: tuple[tuple[str, str | None, int | None, int | None], ...] = compile_findings( + project_dir=tmp_path, capsys=capsys + ) + + assert run_exit == test_case.expected_exit_code + assert run_findings + assert ( + tuple((item["code"], item["path"], item["line"], item["column"]) for item in run_findings) + == compiled + ) + + +@pytest.mark.parametrize( + "test_case", + [ + PythonRuleCacheSequenceTestCase( + description=( + "no Python cache without a Python rule; a cold run misses every file, an edit " + "misses only the edited file, a new Python selection misses every file, and " + "restoring the first model rule reuses its entry and every file" + ), + steps=( + (_PYTHON_CACHE_TOML.replace("SQBRPYTHON101", "SQLCLEAN004"), _PYTHON_ALERT), + (_PYTHON_CACHE_TOML, _PYTHON_ALERT), + (_PYTHON_CACHE_TOML, _PYTHON_ANNOTATED_ALERT), + ( + _PYTHON_CACHE_TOML.replace( + '"SQBRPYTHON101"', '"SQBRPYTHON101", "SQBRPYTHON504"' + ), + _PYTHON_ANNOTATED_ALERT, + ), + ( + _PYTHON_CACHE_TOML.replace( + '"SQBRPYTHON101"', '"SQBRPYTHON101", "SQBRPYTHON504", "SQLCLEAN004"' + ), + _PYTHON_ANNOTATED_ALERT, + ), + ), + expected_hits_and_misses=((0, 1), (0, 9), (8, 1), (0, 9), (10, 0)), + expected_python_cache=(False, True, True, True, True), + ) + ], + ids=lambda case: case.description, +) +def test_given_python_rule_cache_when_inputs_change_then_only_changed_files_rerun( + test_case: PythonRuleCacheSequenceTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_python_rules_project(project_dir=tmp_path, fixture="violations", rules_toml="") + observed: list[tuple[int, int]] = [] + cached: list[bool] = [] + for project_toml, alert_source in test_case.steps: + write_project_files( + project_dir=tmp_path, + files={"sqlbuild_project.toml": project_toml, _PYTHON_ALERT_PATH: alert_source}, + ) + _ = main(["--project-dir", str(tmp_path), "compile", "--json"]) + timings: dict[str, int] = cast( + dict[str, int], json.loads(capsys.readouterr().out)["compile_timings"] + ) + observed.append((timings["rule_cache_hits"], timings["rule_cache_misses"])) + cached.append( + rules_bulk_cache_path(project_dir=tmp_path, file_name="python.json").is_file() + ) + + assert tuple(observed) == test_case.expected_hits_and_misses + assert tuple(cached) == test_case.expected_python_cache diff --git a/tests/integration/src/sqlbuild/cli/entry/test_python_free_commands.py b/tests/integration/src/sqlbuild/cli/entry/test_python_free_commands.py index cc85d0fd8..31938730d 100644 --- a/tests/integration/src/sqlbuild/cli/entry/test_python_free_commands.py +++ b/tests/integration/src/sqlbuild/cli/entry/test_python_free_commands.py @@ -393,9 +393,9 @@ def test_given_python_free_command_when_interrupted_then_sqb_dies_by_sigint_with expected_stderr_end=TRACEBACK_LAST_LINE, ), BrokenPipeTestCase( - description="a report the failed flush drops", - args=("rules", "list"), - prepare=prepare_project, + description="output the failed flush drops", + args=("build", "--help"), + prepare=prepare_nothing, expected_exit_code=0, expected_stderr_end=b"", ), diff --git a/tests/unit/scripts/compiler_differential/_helpers/corpus/_test_types.py b/tests/unit/scripts/compiler_differential/_helpers/corpus/_test_types.py index 5f250d89b..3754b3795 100644 --- a/tests/unit/scripts/compiler_differential/_helpers/corpus/_test_types.py +++ b/tests/unit/scripts/compiler_differential/_helpers/corpus/_test_types.py @@ -44,3 +44,20 @@ class AnalysisCodesTestCase: description: str compiler_root: Path expected_codes: frozenset[str] + + +@dataclass(frozen=True) +class DiscoveryCodesTestCase: + """The discovery codes no failure-corpus case declares, which must be none.""" + + description: str + expected_undeclared_codes: frozenset[str] + + +@dataclass(frozen=True) +class FailureGoldensTestCase: + """The golden directory and the failure cases with no golden there, which must be none.""" + + description: str + golden_dir: Path + expected_missing: frozenset[str] diff --git a/tests/unit/scripts/compiler_differential/_helpers/corpus/test_discovery_codes.py b/tests/unit/scripts/compiler_differential/_helpers/corpus/test_discovery_codes.py new file mode 100644 index 000000000..39cc23872 --- /dev/null +++ b/tests/unit/scripts/compiler_differential/_helpers/corpus/test_discovery_codes.py @@ -0,0 +1,70 @@ +"""Every discovery error code has a failure-corpus case with a golden, checked without compiling the corpus.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.compiler_differential._helpers.corpus.emitted_codes import discovery_error_codes +from scripts.compiler_differential._helpers.refactor_corpus.refactor_cases import refactor_cases +from scripts.compiler_differential.constants import ( + DISCOVERY_UNREACHABLE_CODES, + GOLDEN_DIRECTORY, + GOLDEN_SUFFIX, +) +from scripts.compiler_differential.main.failure_cases import failure_cases +from tests.unit.scripts.compiler_differential._helpers.corpus._test_types import ( + DiscoveryCodesTestCase, + FailureGoldensTestCase, +) + +_REPO_ROOT: Path = Path(__file__).resolve().parents[6] + + +@pytest.mark.parametrize( + "test_case", + [DiscoveryCodesTestCase(description="failure_corpus", expected_undeclared_codes=frozenset())], + ids=lambda case: case.description, +) +def test_given_discovery_error_codes_when_reading_the_failure_corpus_then_each_has_a_case( + test_case: DiscoveryCodesTestCase, +) -> None: + declared: set[str | None] = {project.expected.error_code for project in refactor_cases()} + for case in failure_cases(): + declared.add(case.expected_code) + declared.update(case.expected_codes or ()) + + undeclared: frozenset[str] = ( + discovery_error_codes() - frozenset(DISCOVERY_UNREACHABLE_CODES) - frozenset(declared) + ) + + assert undeclared == test_case.expected_undeclared_codes, test_case.description + + +@pytest.mark.parametrize( + "test_case", + [ + FailureGoldensTestCase( + description="failure_and_refactor_cases", + golden_dir=_REPO_ROOT / GOLDEN_DIRECTORY, + expected_missing=frozenset(), + ) + ], + ids=lambda case: case.description, +) +def test_given_failure_corpus_when_reading_the_goldens_then_each_case_has_one( + test_case: FailureGoldensTestCase, +) -> None: + names: list[str] = [f"failure/{case.name}" for case in failure_cases()] + names.extend(project.name for project in refactor_cases()) + + expected: frozenset[Path] = frozenset( + test_case.golden_dir / f"{name}{GOLDEN_SUFFIX}" for name in names + ) + missing: frozenset[str] = frozenset( + path.relative_to(test_case.golden_dir).as_posix() + for path in expected - frozenset(test_case.golden_dir.rglob("*")) + ) + + assert missing == test_case.expected_missing, test_case.description diff --git a/website/src/content/docs/docs/concepts/python-nodes/overview.mdx b/website/src/content/docs/docs/concepts/python-nodes/overview.mdx index 9d215b702..31657a885 100644 --- a/website/src/content/docs/docs/concepts/python-nodes/overview.mdx +++ b/website/src/content/docs/docs/concepts/python-nodes/overview.mdx @@ -41,7 +41,7 @@ python/ factories/regional.py ``` -Grouping by domain (`python/orders/`, `python/customers/`) works just as well, and a single file may mix kinds. Import other project modules through the `python` package, for example `from python.tasks.orders import export_orders` or `from python._helpers import normalize_status`. +Grouping by domain (`python/orders/`, `python/customers/`) works just as well, and a single file may mix kinds. A project that selects the [Python Rules](/docs/concepts/rules/python-rules#laying-out-python) commits to grouping by domain: `SQBRPYTHON001` reports node-kind folders such as `python/loaders/` and modules directly under `python/`. Import other project modules through the `python` package, for example `from python.tasks.orders import export_orders` or `from python._helpers import normalize_status`. Code that extends SQLBuild itself keeps its own folder, such as `rules/`, `materializations/`, `adapters/`, `providers/`, `sinks/`, `hooks/python/`, `functions/python/`, and `macros/`. Python outside the supported locations fails compilation with `D016`; see [Project Python](/docs/concepts/project-configuration#project-python) for the full list. diff --git a/website/src/content/docs/docs/concepts/rules.mdx b/website/src/content/docs/docs/concepts/rules.mdx index 59dc5f81e..11cf81241 100644 --- a/website/src/content/docs/docs/concepts/rules.mdx +++ b/website/src/content/docs/docs/concepts/rules.mdx @@ -148,7 +148,9 @@ passes its configured Rules. This keeps diagnostics and source mutation separate SQLBuild maintains native built-in Rules for reusable requirements. SQL Rule codes begin with `SQLCLEAN`, for example `SQLCLEAN004`. Rules that read the project, such as `SQBRGRAPH101`, begin -with `SQBR`. +with `SQBR`. The `SQBRPYTHON` Rules check the project's Python: annotations, imports, function +shape and the layout of `python/`. They run only when `select` names `SQBRPYTHON` or one of its +codes; see [Python Rules](/docs/concepts/rules/python-rules). Projects can define custom Python Rules under `rules/**/*.py`. Their codes begin with `XSQBR`, for example `XSQBRARCH001`. Custom Rules use the same selection, findings, exceptions, execution order, @@ -238,6 +240,9 @@ Rules add about 14% to a cold compile and nothing to a warm one. See Understand diagnostics, exact exceptions, path ignores, and stale checks. + + Check the project's Python for annotations, imports, function shape and the layout of `python/`. + Keep each domain's published layer free of consumer-specific variants. diff --git a/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx b/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx index 0f30c8173..3fb0dc23d 100644 --- a/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx +++ b/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx @@ -39,6 +39,11 @@ XSQBR001 → XSQBR Exact codes select one Rule. Families select every matching Rule, whether or not it is enabled by default. `ignore` uses the same matching semantics and takes precedence over `select`. +The Python Rules are the exception. `SQBR` does not select them: they run only when a selector names +`SQBRPYTHON` or one of its codes, as in `select = ["SQBR", "SQBRPYTHON"]`. `SQBRPYTHON` selects the +Python Rules that are enabled by default, and the others need their exact code. `ignore = ["SQBR"]` +still turns them off. See [Python Rules](/docs/concepts/rules/python-rules#selecting-the-python-rules). + A selector that matches no Rule fails with `C000`. `SQBRTEST105` (scenario description) was retired when descriptions became the compile error `P010` ([Descriptions](/docs/concepts/models/configuration#descriptions)); remove it from `select` and @@ -73,6 +78,9 @@ reviewed exceptions. Listed tests are not flagged and still count toward `SQBRTE optional `domain`; `qualifier` and `paths` accept globs. See [Published-layer Rules](/docs/concepts/rules/published-layer-rules). +The Python Rules take whole-number limits such as `max_arguments` for `SQBRPYTHON504`; see +[Python Rules](/docs/concepts/rules/python-rules#rule-options). + Unknown Rule codes, option names, and invalid option values fail configuration. ## Focused selection diff --git a/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx b/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx index a875667f4..62b27e0d8 100644 --- a/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx +++ b/website/src/content/docs/docs/concepts/rules/execution-and-caching.mdx @@ -37,6 +37,10 @@ Model-subject Rules support fine-grained reuse. Editing one model invalidates af subjects rather than every Rule invocation. Project-subject Rules run once and are appropriate for genuinely project-wide invariants, with correspondingly broader invalidation. +Python Rules are cached per file. Editing a Python file re-checks that file and the files whose +findings read it, such as an importer checked against the module it imports. `SQBRPYTHON305` and +`SQBRPYTHON306` read every Python file, so any Python edit re-checks them. + Positive and negative project-tree observations are dependencies. If a Rule observes that a glob has no matches, adding a matching file invalidates that result. diff --git a/website/src/content/docs/docs/concepts/rules/python-rules.mdx b/website/src/content/docs/docs/concepts/rules/python-rules.mdx new file mode 100644 index 000000000..b2aa1e660 --- /dev/null +++ b/website/src/content/docs/docs/concepts/rules/python-rules.mdx @@ -0,0 +1,253 @@ +--- +title: "Python Rules" +description: "Check the project's Python for annotations, imports, function shape and the layout of python/." +--- + +The `SQBRPYTHON` Rules check the Python in your project the way `SQLCLEAN` checks its SQL. Select +the family and `sqb compile` reports each finding with its file, line and column: + +```toml +[rules] +select = ["SQBR", "SQBRPYTHON"] +``` + +```text +error[SQBRPYTHON101]: function parameter 'ctx' must define a type annotation + --> hooks/python/alert.py:4:11 + = help: Annotate every parameter with the value type accepted by the function. +``` + +## Selecting the Python Rules + +The Python Rules run only when a selector names `SQBRPYTHON` or one of its codes. `SQBR` on its own +leaves the whole family out, so a project that selects `SQBR` keeps its current findings until it +adds `SQBRPYTHON`. + +| Selection | Python Rules that run | +|---|---| +| no `select`, or `select = ["SQBR"]` | none | +| `select = ["SQBRPYTHON"]` | every Rule enabled by default | +| `select = ["SQBRPYTHON", "SQBRPYTHON306"]` | the defaults and `SQBRPYTHON306` | +| `select = ["SQBRPYTHON101"]` | `SQBRPYTHON101` only | + +`SQBRPYTHON306` (unreachable definitions) and `SQBRPYTHON507` (parameter mutation) are off by +default. Select them by their exact code; the family selector does not include them. + +`ignore` matches by prefix as usual, so `ignore = ["SQBR"]` also turns the Python Rules off. + +Try the family before adding it to `select`: + +```bash +sqb rules run SQBRPYTHON +``` + +## Which files are checked + +The Python Rules read every supported project Python file: the runtime package under `python/` and +the folders that extend SQLBuild, such as `hooks/python/`, `functions/python/`, `providers/`, +`rules/` and `macros/`. See [Project Python](/docs/concepts/project-configuration#project-python) +for the full list. Files under `tests/` are not checked. + +Rules marked `python/` in the [reference](#rule-reference) check only the runtime package, because +they describe how its packages are organised. The others check every file. + +## Laying out `python/` + +`SQBRPYTHON001` and `SQBRPYTHON002` keep `python/` organised by what the code is for, not by what +kind of node it holds. Directly under `python/`, keep only domain packages, `utils/` for code no +domain owns, and `__init__.py`: + +```text +python/ + __init__.py + fulfillment/ + shipment_export/ + node.py # @task shipment_export + main.py # export_shipments, the unit's one entry + utils/ + currency/ + format.py +``` + +These layouts are reported: + +| Finding | Example | Fix | +|---|---|---| +| A module directly under `python/` | `python/report.py` | Move it into the domain that owns it | +| A folder named for a node kind or a role | `python/loaders/`, `python/_helpers/` | Group the nodes by domain instead | +| A package with a generic name | `python/inventory/common/` | Name it after what it owns | + +The generic names are `base`, `common`, `general`, `helpers`, `lib`, `misc`, `shared`, `util` and +`utils`. `utils/` is allowed directly under `python/`. A finding about a folder points at its +`__init__.py`, or at its first Python file when it has none. + +## Main modules and entry modules + +Some Rules apply only to a domain's main modules: files under a `main/` folder, and a unit's +`main.py`, such as `python/fulfillment/shipment_export/main.py`. A main module that is not +`__init__.py` is an entry module. `SQBRPYTHON617` requires it to expose one public function, and +`SQBRPYTHON305` requires something outside its domain to import it. + +`SQBRPYTHON501` to `SQBRPYTHON503` (statements, distinct calls and locals) check main modules only. +The other size limits check every function. + +## Unreachable definitions + +`SQBRPYTHON306` reports functions, classes and module values that nothing SQLBuild loads can reach. +SQLBuild loads every decorated function under `python/` (`@loader`, `@task`, `@asset`, `@check`, +`@factory` and `@audit_factory`) and everything defined outside `python/`, such as hooks and +providers. A definition reached only through them is live. Imports through folders without an +`__init__.py` count too. + +Code that SQLBuild loads in a way the Rule cannot follow, such as by name at run time, needs a rule +exception with a reason: + +```toml +[[rules.rule_exceptions]] +rule = "SQBRPYTHON306" +path = "python/fulfillment/carriers/registry.py" +reason = "Carriers are looked up by name from the shipment's carrier code." +``` + +## Rule options + +Configure the size limits by exact code: + +```toml +[rules.rule_options.SQBRPYTHON504] +max_arguments = 6 +``` + +| Rule | Option | Default | Meaning | +|---|---|---|---| +| `SQBRPYTHON501` | `max_statements` | 40 | Most statements one main-module function may hold. | +| `SQBRPYTHON502` | `max_distinct_calls` | 20 | Most distinct callees one main-module function may call. | +| `SQBRPYTHON503` | `max_locals` | 20 | Most local names one main-module function may bind. | +| `SQBRPYTHON504` | `max_arguments` | 10 | Most parameters one function may declare. | +| `SQBRPYTHON505` | `max_statements_global` | 70 | Most statements any function may hold. | +| `SQBRPYTHON610` | `max_helpers_container_modules` | 10 | Most modules one `_helpers/` package may hold directly. | +| `SQBRPYTHON610` | `max_role_depth` | 1 | How many package levels may sit below a role package. | +| `SQBRPYTHON611` | `max_main_container_modules` | 20 | Most modules one `main/` package may hold directly. | +| `SQBRPYTHON611` | `max_role_depth` | 1 | How many package levels may sit below a role package. | +| `SQBRPYTHON615` | `min_shared_domain_prefix_packages` | 2 | Fewest sibling domains sharing a name prefix that are reported; 0 turns the rule off. | +| `SQBRPYTHON627` | `max_file_lines` | 2000 | Most lines one Python file may hold. | + +Each option takes a whole number of 0 or more. Unknown option names, text values and negative +numbers fail configuration with `C000`. + +## Rule reference + +`Default` says whether `SQBRPYTHON` selects the Rule. `sqb rules show ` prints the full +remediation. + +### Layout of `python/` + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON001` | project-root-allowlist | `python/` | yes | `python/` may hold only domain packages, `utils/` and `__init__.py` | +| `SQBRPYTHON002` | runtime-package-owner-names | `python/` | yes | Packages under `python/` must be named for what they own | + +### Type annotations + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON101` | parameter-annotation | All | yes | Function parameters must define type annotations | +| `SQBRPYTHON102` | return-annotation | All | yes | Functions must define return type annotations | +| `SQBRPYTHON103` | module-variable-annotation | All | yes | Module-level variables must define type annotations | +| `SQBRPYTHON104` | class-attribute-annotation | All | yes | Class attributes must define type annotations | +| `SQBRPYTHON105` | local-variable-annotation | All | yes | Local variables must define type annotations on first binding unless assigned a scalar literal | + +### Hygiene + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON201` | single-line-docstrings | All | yes | Docstrings must be a single line; move extended explanation into docs or tests | +| `SQBRPYTHON202` | no-standalone-comments | All | yes | Standalone comments are not allowed; prefer clear names or docs/tests | +| `SQBRPYTHON203` | no-raw-builtin-raise | All | yes | Runtime code must raise structured errors instead of raw built-in exceptions | +| `SQBRPYTHON204` | no-assert-in-runtime | All | yes | Runtime code must not use assert for invariants; raise a structured error | +| `SQBRPYTHON205` | no-swallowed-exception-probe | All | yes | Runtime code must not swallow broad exceptions as existence probe answers | +| `SQBRPYTHON206` | no-unnamed-string-decisions | All | yes | String literals must not directly control comparison behavior | +| `SQBRPYTHON207` | no-magic-numeric-comparisons | All | yes | Non-canonical numeric literals must not directly control comparisons | +| `SQBRPYTHON208` | no-import-time-side-effects | All | yes | Runtime and tooling modules must not execute standalone calls during import | + +### Imports and reachability + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON301` | absolute-imports-only | All | yes | Use absolute imports; relative imports hide package boundaries | +| `SQBRPYTHON302` | no-star-imports | All | yes | Star imports hide names from dependency-boundary analysis | +| `SQBRPYTHON303` | no-internal-public-surface-imports | `python/` | yes | Internal code must import from the owning module, not the bare package | +| `SQBRPYTHON304` | no-cross-domain-private-main-imports | `python/` | yes | Domain-private main entries may only be imported within their owning domain | +| `SQBRPYTHON305` | public-main-entry-external-use | `python/` | yes | Public main entries must have an importer outside their owning domain | +| `SQBRPYTHON306` | unreachable-definition | `python/` | no | Definition is unreachable from production roots | +| `SQBRPYTHON307` | no-cross-file-use-of-helper-private-class | `python/` | yes | Helper-private classes are file-local details; move shared classes to `classes/` | + +### Naming contracts + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON401` | validator-must-not-return | All | yes | Functions under no-return naming contracts must not return values | +| `SQBRPYTHON402` | predicate-must-return-bool | All | yes | Predicate names must declare an ordinary boolean result | +| `SQBRPYTHON403` | value-name-must-return-value | All | yes | Value-producing names must not declare a no-value result | +| `SQBRPYTHON404` | iterator-name-must-produce-iterator | All | yes | Iterator names must produce an iterator or generator | + +### Function shape + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON501` | too-many-statements | All | yes | Main functions must stay phase-shaped and below the statement limit | +| `SQBRPYTHON502` | too-many-distinct-calls | All | yes | Main functions must not coordinate too many distinct callees | +| `SQBRPYTHON503` | too-many-locals | All | yes | Main functions must not juggle too many local variables | +| `SQBRPYTHON504` | max-arguments | All | yes | Functions must stay below the configured argument limit | +| `SQBRPYTHON505` | max-statements-global | All | yes | Functions must stay below the global statement limit | +| `SQBRPYTHON506` | meaningful-project-result-discarded | All | yes | Main orchestrators must consume meaningful project-local call results | +| `SQBRPYTHON507` | parameter-mutation-in-phase-helpers | All | no | Helpers must return values instead of mutating parameters | +| `SQBRPYTHON508` | default-mutation-return | All | yes | Functions that mutate parameters must return every mutated parameter | +| `SQBRPYTHON509` | no-outer-state-mutation | All | yes | Functions must not mutate module-global or closure-captured state | +| `SQBRPYTHON510` | no-complex-comprehensions | All | yes | Nested or multi-generator comprehensions hide control flow and data shapes | +| `SQBRPYTHON511` | mutable-result-model | All | yes | Dataclass result models must be frozen | + +### Module roles and package layout + +| Code | Name | Files | Default | Checks | +|---|---|---|---|---| +| `SQBRPYTHON601` | models-only-models | `python/` | yes | Models role files may contain only structured runtime models | +| `SQBRPYTHON602` | types-only-types | `python/` | yes | Types role files may contain only type-layer declarations | +| `SQBRPYTHON603` | constants-only-constants | `python/` | yes | Constants role files may contain only assignments and imports | +| `SQBRPYTHON604` | exceptions-only-exceptions | `python/` | yes | Exceptions role files may contain only custom exceptions | +| `SQBRPYTHON605` | constant-outside-constants | `python/` | yes | Public uppercase constants must be defined in the constants role | +| `SQBRPYTHON606` | banned-generic-filename | All | yes | Generic filenames hide module ownership | +| `SQBRPYTHON607` | helpers-module-name | `python/` | yes | Use an _helpers package instead of `helpers.py` | +| `SQBRPYTHON608` | classes-module-name | `python/` | yes | Use a classes package instead of `classes.py` | +| `SQBRPYTHON609` | helpers-classes-file-private | `python/` | yes | Plain classes in _helpers modules must be file-private | +| `SQBRPYTHON610` | helpers-package-layout | `python/` | yes | `_helpers/` packages must use bounded flat-or-grouped containers | +| `SQBRPYTHON611` | main-package-layout | `python/` | yes | `main/` packages must use bounded flat-or-grouped orchestration containers | +| `SQBRPYTHON612` | helpers-reserved-role-filenames | `python/` | yes | `_helpers/` packages must not contain reserved role filenames | +| `SQBRPYTHON613` | top-level-domain-shape | `python/` | yes | Top-level domains must be either role leaves or subdomain branches | +| `SQBRPYTHON614` | top-level-direct-modules | `python/` | yes | Runtime roots and top-level domains must not contain ad hoc direct modules | +| `SQBRPYTHON615` | shared-domain-prefix | `python/` | yes | Sibling domains must not encode one parent domain through a shared name prefix | +| `SQBRPYTHON616` | public-facade-import-direction | `python/` | yes | Runtime modules must not import public facade modules | +| `SQBRPYTHON617` | entry-module-shape | `python/` | yes | `main/` entry modules must expose one focused public function | +| `SQBRPYTHON618` | init-module-empty | `python/` | yes | Nested `__init__.py` files must be empty or docstring-only | +| `SQBRPYTHON619` | no-reexport-shim | `python/` | yes | Internal modules must not exist only to re-export imports | +| `SQBRPYTHON620` | no-internal-helper-exports | `python/` | yes | `_helpers/` modules must not publish an `__all__` surface | +| `SQBRPYTHON621` | main-entry-name-collision | `python/` | yes | `main/` cannot define a module and package with the same entry name | +| `SQBRPYTHON622` | public-surface-shape | `python/` | yes | Root package surfaces may contain only imports and one `__all__` declaration | +| `SQBRPYTHON623` | classes-one-class-per-module | `python/` | yes | `classes/` modules must define exactly one top-level class | +| `SQBRPYTHON624` | helpers-package-shape | `python/` | yes | `_helpers/` packages must contain no main.py orchestration entrypoints | +| `SQBRPYTHON625` | private-definition-ordering | `python/` | yes | Private constants and dataclasses must appear before top-level functions | +| `SQBRPYTHON626` | classes-runtime-reexport | `python/` | yes | `classes/` modules must not publicly re-export imported runtime symbols | +| `SQBRPYTHON627` | source-file-line-count | All | yes | Source files must stay below the configured line limit | + +## Details + +- Findings, exceptions, ignores and stale checks work as for every Rule; see + [Findings and exceptions](/docs/concepts/rules/findings-and-exceptions). An exception covers one + Rule in one file. +- Findings on a file or a folder point at line 1, column 1. Columns count characters, so text before + the finding in another script does not shift them. +- SQLBuild reads the files with the syntax of the Python it runs on. A file it cannot read fails the + run with `cannot parse Python source` and the file's position. +- Editing one file re-checks it and the files whose findings read it. `SQBRPYTHON305` and + `SQBRPYTHON306` read the whole project, so any Python edit re-checks them. See + [Execution and caching](/docs/concepts/rules/execution-and-caching). diff --git a/website/src/sidebar.json b/website/src/sidebar.json index cee743e40..7c48b62c1 100644 --- a/website/src/sidebar.json +++ b/website/src/sidebar.json @@ -75,6 +75,7 @@ }, "docs/concepts/rules/configuration-and-selection", "docs/concepts/rules/findings-and-exceptions", + "docs/concepts/rules/python-rules", "docs/concepts/rules/execution-and-caching" ] }