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) <noreply@anthropic.com>
This commit is contained in:
Thom Lamb
2026-05-20 10:45:21 -05:00
co-authored by Claude Opus 4.7
parent 30f451d80c
commit 55ab737888
@@ -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<CalculationFilter>()` to use the collection expression `?? []`. One edit resolves both original issues.
2. **NUnit2045.** In `CommandVisitorTests.cs`, wrap the three independent `Assert.That` assertions (lines 1820) 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.