From 31a22082f38cd8570787c5a087164a0cb1391d5e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 8 Jul 2026 13:00:48 +0200 Subject: [PATCH 1/3] Fix Whitaker lint findings ahead of suite adoption Prepare the crate for the Whitaker Dylint suite: - Move the `cte` unit tests into a sibling `src/cte_tests.rs` submodule via `#[path]`, bringing `src/cte.rs` under the 400-line `module_max_lines` ceiling. - Add `//!` inner doc comments to the `tests` modules in `builders`, `columns`, `connection_ext`, and `macros` to satisfy `module_must_have_inner_docs`. No behavioural change; test coverage is unchanged. --- src/builders.rs | 2 + src/columns.rs | 2 + src/connection_ext.rs | 2 + src/cte.rs | 122 +----------------------------------------- src/cte_tests.rs | 110 +++++++++++++++++++++++++++++++++++++ src/macros.rs | 2 + 6 files changed, 120 insertions(+), 120 deletions(-) create mode 100644 src/cte_tests.rs diff --git a/src/builders.rs b/src/builders.rs index bdf352f..ea39aa5 100644 --- a/src/builders.rs +++ b/src/builders.rs @@ -110,6 +110,8 @@ where #[cfg(test)] mod tests { + //! Unit tests for the CTE builder functions. + use super::*; use crate::test_support::normalise_debug_sql; use diesel::{ diff --git a/src/columns.rs b/src/columns.rs index b98c60f..44de4f2 100644 --- a/src/columns.rs +++ b/src/columns.rs @@ -130,6 +130,8 @@ macro_rules! table_columns { #[cfg(test)] mod tests { + //! Unit tests for CTE column list handling. + use super::*; diesel::table! { diff --git a/src/connection_ext.rs b/src/connection_ext.rs index bc01e65..1c5241d 100644 --- a/src/connection_ext.rs +++ b/src/connection_ext.rs @@ -106,6 +106,8 @@ impl RecursiveCTEExt for SyncConnectionWrapper, Body); impl_cte_traits!(WithRecursive, Body); #[cfg(test)] -mod tests { - use super::*; - use crate::{ - builders::{self, RecursiveParts}, - test_support::normalise_debug_sql, - }; - use diesel::{ - debug_query, dsl::sql, expression::SqlLiteral, sql_types::Integer, sqlite::Sqlite, - }; - use rstest::{fixture, rstest}; - - enum Builder { - All, - Distinct, - } - - #[fixture] - fn sample_parts() - -> RecursiveParts, SqlLiteral, SqlLiteral> { - RecursiveParts::new( - sql::("SELECT 1"), - sql::("SELECT n + 1 FROM nums WHERE n < 2"), - sql::("SELECT n FROM nums"), - ) - } - - #[test] - fn duplicate_column_names_are_rejected() { - let names = &["id", "id"]; - match ensure_unique_columns(names) { - Err(err) => { - assert!(matches!(err, Error::QueryBuilderError(_))); - assert!(err.to_string().contains("duplicate column name")); - } - Ok(()) => panic!("expected duplicate column error"), - } - } - - #[rstest] - #[case::all(Builder::All, "UNION ALL")] - #[case::distinct(Builder::Distinct, "UNION")] - fn with_recursive_renders_expected_sql( - sample_parts: RecursiveParts, SqlLiteral, SqlLiteral>, - #[case] builder: Builder, - #[case] union_op: &str, - ) { - let query = match builder { - Builder::All => { - builders::with_recursive::("nums", &["n"], sample_parts) - } - Builder::Distinct => builders::with_recursive_not_all::( - "nums", - &["n"], - sample_parts, - ), - }; - - let sql = normalise_debug_sql(&debug_query::(&query).to_string()); - assert_eq!( - sql, - format!( - "WITH RECURSIVE \"nums\" (\"n\") AS (SELECT 1 {union_op} SELECT n + 1 FROM nums WHERE n < 2) SELECT n FROM nums" - ) - ); - } - - #[test] - fn with_cte_renders_expected_sql() { - let query = builders::with_cte::( - "seed", - &["value"], - builders::CteParts::new( - sql::("SELECT 42"), - sql::("SELECT value FROM seed"), - ), - ); - let sql = normalise_debug_sql(&debug_query::(&query).to_string()); - assert_eq!( - sql, - "WITH \"seed\" (\"value\") AS (SELECT 42) SELECT value FROM seed" - ); - } - - #[test] - fn with_recursive_skips_identifier_list_when_empty() { - let query = builders::with_recursive::( - "nums", - &[] as &[&str], - RecursiveParts::new( - sql::("SELECT 1"), - sql::("SELECT n + 1 FROM nums WHERE n < 2"), - sql::("SELECT n FROM nums"), - ), - ); - let sql = normalise_debug_sql(&debug_query::(&query).to_string()); - assert_eq!( - sql, - "WITH RECURSIVE \"nums\" AS (SELECT 1 UNION ALL SELECT n + 1 FROM nums WHERE n < 2) SELECT n FROM nums" - ); - } - - #[test] - fn query_id_reflects_runtime_union_choice() { - type RecursiveQuery = WithRecursive< - Sqlite, - (), - SqlLiteral, - SqlLiteral, - SqlLiteral, - >; - type CteQuery = WithCte, SqlLiteral>; - - let recursive_has_static = - std::hint::black_box(::HAS_STATIC_QUERY_ID); - let cte_has_static = std::hint::black_box(::HAS_STATIC_QUERY_ID); - - assert!(!recursive_has_static); - assert!(cte_has_static); - } -} +#[path = "cte_tests.rs"] +mod tests; diff --git a/src/cte_tests.rs b/src/cte_tests.rs new file mode 100644 index 0000000..891f8fc --- /dev/null +++ b/src/cte_tests.rs @@ -0,0 +1,110 @@ +//! Unit tests for the CTE query types in [`crate::cte`]. + +use super::*; +use crate::{ + builders::{self, RecursiveParts}, + test_support::normalise_debug_sql, +}; +use diesel::{debug_query, dsl::sql, expression::SqlLiteral, sql_types::Integer, sqlite::Sqlite}; +use rstest::{fixture, rstest}; + +enum Builder { + All, + Distinct, +} + +#[fixture] +fn sample_parts() -> RecursiveParts, SqlLiteral, SqlLiteral> { + RecursiveParts::new( + sql::("SELECT 1"), + sql::("SELECT n + 1 FROM nums WHERE n < 2"), + sql::("SELECT n FROM nums"), + ) +} + +#[test] +fn duplicate_column_names_are_rejected() { + let names = &["id", "id"]; + match ensure_unique_columns(names) { + Err(err) => { + assert!(matches!(err, Error::QueryBuilderError(_))); + assert!(err.to_string().contains("duplicate column name")); + } + Ok(()) => panic!("expected duplicate column error"), + } +} + +#[rstest] +#[case::all(Builder::All, "UNION ALL")] +#[case::distinct(Builder::Distinct, "UNION")] +fn with_recursive_renders_expected_sql( + sample_parts: RecursiveParts, SqlLiteral, SqlLiteral>, + #[case] builder: Builder, + #[case] union_op: &str, +) { + let query = match builder { + Builder::All => { + builders::with_recursive::("nums", &["n"], sample_parts) + } + Builder::Distinct => { + builders::with_recursive_not_all::("nums", &["n"], sample_parts) + } + }; + + let sql = normalise_debug_sql(&debug_query::(&query).to_string()); + assert_eq!( + sql, + format!( + "WITH RECURSIVE \"nums\" (\"n\") AS (SELECT 1 {union_op} SELECT n + 1 FROM nums WHERE n < 2) SELECT n FROM nums" + ) + ); +} + +#[test] +fn with_cte_renders_expected_sql() { + let query = builders::with_cte::( + "seed", + &["value"], + builders::CteParts::new( + sql::("SELECT 42"), + sql::("SELECT value FROM seed"), + ), + ); + let sql = normalise_debug_sql(&debug_query::(&query).to_string()); + assert_eq!( + sql, + "WITH \"seed\" (\"value\") AS (SELECT 42) SELECT value FROM seed" + ); +} + +#[test] +fn with_recursive_skips_identifier_list_when_empty() { + let query = builders::with_recursive::( + "nums", + &[] as &[&str], + RecursiveParts::new( + sql::("SELECT 1"), + sql::("SELECT n + 1 FROM nums WHERE n < 2"), + sql::("SELECT n FROM nums"), + ), + ); + let sql = normalise_debug_sql(&debug_query::(&query).to_string()); + assert_eq!( + sql, + "WITH RECURSIVE \"nums\" AS (SELECT 1 UNION ALL SELECT n + 1 FROM nums WHERE n < 2) SELECT n FROM nums" + ); +} + +#[test] +fn query_id_reflects_runtime_union_choice() { + type RecursiveQuery = + WithRecursive, SqlLiteral, SqlLiteral>; + type CteQuery = WithCte, SqlLiteral>; + + let recursive_has_static = + std::hint::black_box(::HAS_STATIC_QUERY_ID); + let cte_has_static = std::hint::black_box(::HAS_STATIC_QUERY_ID); + + assert!(!recursive_has_static); + assert!(cte_has_static); +} diff --git a/src/macros.rs b/src/macros.rs index fb63188..c6ed5f4 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -86,6 +86,8 @@ macro_rules! step_query { #[cfg(test)] mod tests { + //! Unit tests for the query-part macros. + use super::QueryPart; use crate::test_support::normalise_debug_sql; use diesel::{debug_query, dsl::sql, sql_types::Integer, sqlite::Sqlite}; From 1f49915318bd8a6ac000ac6028e1cb26e14d2c5e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 8 Jul 2026 13:00:58 +0200 Subject: [PATCH 2/3] Adopt the Whitaker Dylint suite in the lint gate and CI Extend the `lint` Makefile target to run the Whitaker Dylint suite after Clippy, denying warnings via the existing `RUST_FLAGS` convention, and introduce a `WHITAKER ?= whitaker` tool variable alongside the other tool overrides. In CI, install the suite before the lint step: pin `WHITAKER_INSTALLER_VERSION` as job env, cache the installer binary and the cargo-binstall cache keyed on OS, architecture, and version, and install via `cargo binstall` with a build-from-source fallback for runners without binstall. This mirrors the estate-wide rollout established in leynos/netsuke#410. --- .github/workflows/ci.yml | 19 +++++++++++++++++++ Makefile | 4 +++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29aabe0..a30daa5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,7 @@ jobs: env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug + WHITAKER_INSTALLER_VERSION: '0.2.5' steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 - name: Setup Rust @@ -26,6 +27,24 @@ jobs: **/*.md !**/target/** !**/dist/** + - name: Cache whitaker-installer + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: | + ~/.cargo/bin/whitaker-installer + ~/.cache/cargo-binstall + key: whitaker-installer-${{ runner.os }}-${{ runner.arch }}-${{ env.WHITAKER_INSTALLER_VERSION }} + - name: Install the Whitaker Dylint suite + run: | + if ! command -v whitaker-installer >/dev/null 2>&1; then + if cargo binstall --version >/dev/null 2>&1; then + cargo binstall --no-confirm whitaker-installer@${{ env.WHITAKER_INSTALLER_VERSION }} + else + echo "cargo-binstall unavailable; building whitaker-installer from crates.io" + cargo install --locked whitaker-installer --version ${{ env.WHITAKER_INSTALLER_VERSION }} + fi + fi + whitaker-installer - name: Lint run: make lint - name: Test pg_worker preparation diff --git a/Makefile b/Makefile index 9379bb9..dda7528 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,7 @@ CLIPPY_FLAGS ?= $(CARGO_FLAGS) -- $(RUST_FLAGS) TEST_FLAGS ?= $(CARGO_FLAGS) MDLINT ?= markdownlint-cli2 NIXIE ?= nixie +WHITAKER ?= whitaker PG_WORKER_PATH ?= $(CURDIR)/target/pg_worker PG_WORKER_PROFILE ?= dev PG_WORKER_DEBUG_PROFILES := dev test @@ -56,9 +57,10 @@ test-prepare-pg-worker: ## Test pg_worker profile mapping and fail-fast setup target/%/$(TARGET): ## Build binary in debug or release mode $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) -lint: ## Run Clippy with warnings denied +lint: ## Run Clippy and the Whitaker Dylint suite with warnings denied RUSTDOCFLAGS="$(RUSTDOC_FLAGS)" $(CARGO) doc --no-deps $(CARGO) clippy $(CLIPPY_FLAGS) + RUSTFLAGS="$(RUST_FLAGS)" $(WHITAKER) --all -- $(CARGO_FLAGS) fmt: ## Format Rust and Markdown sources $(CARGO) fmt --all From 9130d486c1f1acb5ced7b1dd1c43f6e0a60e78dd Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 8 Jul 2026 13:26:27 +0200 Subject: [PATCH 3/3] Harden the Whitaker install step Replace inline `${{ env.WHITAKER_INSTALLER_VERSION }}` interpolation in the run block with the plain shell variable `"${WHITAKER_INSTALLER_VERSION}"`; the job-level `env:` already exports it, and zizmor flags run-block template interpolation as a template-injection hazard. Add `--locked` to the cargo-binstall invocation so that binstall's compile fallback resolves dependencies from the published lockfile, keeping fallback builds reproducible. --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a30daa5..b6931ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,10 +38,10 @@ jobs: run: | if ! command -v whitaker-installer >/dev/null 2>&1; then if cargo binstall --version >/dev/null 2>&1; then - cargo binstall --no-confirm whitaker-installer@${{ env.WHITAKER_INSTALLER_VERSION }} + cargo binstall --no-confirm --locked "whitaker-installer@${WHITAKER_INSTALLER_VERSION}" else echo "cargo-binstall unavailable; building whitaker-installer from crates.io" - cargo install --locked whitaker-installer --version ${{ env.WHITAKER_INSTALLER_VERSION }} + cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}" fi fi whitaker-installer