From 55ab73788851c2885673428fe709a9cedd4a83da Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Wed, 20 May 2026 10:45:21 -0500 Subject: [PATCH] docs: spec for Strata.SqlTools.Query shared-project extraction Designs the extraction of the byte-identical 19-file ExpressionFactory/Query tree (duplicated across Snowflake + SqlServer) into a new Strata.SqlTools.Query project under a clean-break namespace, and folds in the 3 remaining new-code SonarQube smells (IDE0028 x2 on CalculationFilterGroup, NUnit2045 on CommandVisitorTests). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-shared-query-project-design.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-20-shared-query-project-design.md diff --git a/docs/superpowers/specs/2026-05-20-shared-query-project-design.md b/docs/superpowers/specs/2026-05-20-shared-query-project-design.md new file mode 100644 index 0000000..a702c3d --- /dev/null +++ b/docs/superpowers/specs/2026-05-20-shared-query-project-design.md @@ -0,0 +1,81 @@ +# Extract `Strata.SqlTools.Query` shared project + clear 3 new-code smells — Design + +**Date:** 2026-05-20 +**Branch:** `fix/Sonarqube-Tech-Debt` +**Author:** Thom Lamb (with Claude) + +## Goal + +Resolve the SonarQube tech debt introduced by yesterday's sweep: + +1. **3 new-code code smells** (the only issues in SonarQube's new-code period): + - `IDE0028` — `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs:27` + - `IDE0028` — `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs:27` + - `NUnit2045` — `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs:18` +2. **63 new duplicate lines** flagged on the PR — caused by the `CalculationFilterGroup` rewrite landing in two byte-identical copies (Snowflake + SqlServer). + +## Root cause of the duplication + +Snowflake and SqlServer each carry a **byte-identical 19-file `ExpressionFactory/Query/` tree** (verified: every file diffs to zero non-namespace lines). Project-wide duplication density is 11.1% (3,147 duplicated lines / 99 blocks); this tree is a major contributor. The `CalculationFilterGroup` change merely tipped the new-code duplication over threshold. + +The two flagged files cannot be extracted in isolation: their transitive type closure pulls in 14 of the 19 types (`QueryConfigExtensions → QueryConfig → Value → CalculationFilterGroup → CalculationFilter → Filter → …`). The remaining 4 (`AggregationType`, `ColumnQueryConfig`, `SqlResponse`, `ValueFilter`) are also identical duplicates. Therefore the coherent unit of extraction is the **whole 19-file tree**. + +## Approach + +Create a new shared class library and move the entire identical tree into it under a single new namespace. + +### New project + +- `src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj` + - `net8.0` (matches all existing projects) + - `PackageId = Strata.SqlTools.Query`, `GeneratePackageOnBuild = false` (matches dialect packages) + - No project references (the 19 types depend only on BCL: `System.Text.Json.Serialization`, `System.ComponentModel.DataAnnotations`) +- Added to `Strata.SqlTools.QueryBreakdown.sln`. + +### Namespace (clean break) + +- All 19 types move from `Strata.SqlTools.{Snowflake,SqlServer}.ExpressionFactory.Query` to the single flat namespace **`Strata.SqlTools.Query`**. +- File names are unchanged. One physical copy of each type. +- Rationale for clean break: both dialect packages have `GeneratePackageOnBuild = false` (not currently published), so the breaking namespace change has no live external consumers. If backward compat is later needed, `[TypeForwardedTo]` can be added without touching call sites. + +### Wiring + +- `Strata.SqlTools.Snowflake` and `Strata.SqlTools.SqlServer` both add a `ProjectReference` to `Strata.SqlTools.Query`. +- Delete all 38 dialect copies (19 in Snowflake, 19 in SqlServer). +- EFCore / LinqToSql / Markdown obtain the types transitively (they already reference the dialect projects). + +### Consumer migration + +Consumers reference these types either via a `using` directive or via relative resolution through the dialect's ancestor namespace. After the move they need `using Strata.SqlTools.Query;`. Known explicit-`using` sites to update: + +- `src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs` +- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs` +- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs` + +Any remaining relative references break the build; the compiler enumerates them precisely. The fix is mechanical: add the `using` (or a per-project `global using Strata.SqlTools.Query;`). + +**Collision safety:** No single source file references *both* dialect Query namespaces (verified via set intersection), so merging the two type families into one shared type cannot create ambiguous-overload or duplicate-signature errors. + +## Code smell fixes (folded into the same branch) + +1. **IDE0028 ×2 → ×1, then fixed.** After extraction there is one `CalculationFilterGroup.cs`. Change line 27 `Filters?.Where(x => x.IsValid()).ToList() ?? new List()` to use the collection expression `?? []`. One edit resolves both original issues. +2. **NUnit2045.** In `CommandVisitorTests.cs`, wrap the three independent `Assert.That` assertions (lines 18–20) in `Assert.Multiple(() => { … });` so all three report together. (The `Assert.That(method, Is.Not.Null, …)` guard in the helper stays as-is — it gates a subsequent `Invoke`, so it is correctly *not* inside a multiple block.) + +## Out of scope + +- The other 798 project-wide code smells (S3776 cognitive complexity, CA/IDE info-level, etc.). +- Extracting non-identical dialect code (visitors, breakdowns, statement parsers). +- Publishing/packaging changes beyond the new project's csproj. +- Pushing to the Gitea remote (user works locally; merge decision deferred). + +## Verification + +1. `dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release` — clean (errors fail; pre-existing warnings OK). +2. `dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build` — all tests green at baseline (~819, including the Task 4 `CommandVisitorTests`). +3. Optional: `./scan-sonar.ps1` then confirm on the dashboard that the 3 new-code smells are gone and new-code duplicated lines drop to 0. + +## Success criteria + +- New-code period shows **0** code smells and **0** duplicated lines. +- Project-wide `duplicated_lines_density` drops measurably (the identical tree no longer double-counts). +- Build clean, full suite green, no behavioral change to query-config semantics.