Resolve SonarQube Tech Debt #3

2026-05-20T13:22:51-05:00

bermudalamb commented

2026-05-20 13:22:51 -05:00

📋 Description

This PR addresses technical debt and resolves outstanding SonarQube code smells introduced during the recent codebase sweep. The primary structural change eliminates massive duplication by extracting a dialect-agnostic query configuration model into a dedicated shared class library.

🔍 Root Cause of Duplication

Prior to these changes, Strata.SqlTools.Snowflake and Strata.SqlTools.SqlServer each maintained an identical 19-file tree under their respective ExpressionFactory/Query/ directories. This severe duplication was a major contributor to the project's 11.1% duplication density metric in SonarQube. A recent update to CalculationFilterGroup across both locations tipped the new-code duplication analyzer over the acceptable threshold, triggering a quality gate blocker.

🛠️ Key Changes

1. Architecture & Project Extraction

Created New Shared Project: Scaffolded src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj, a standalone, BCL-only class library targeting .NET 8.0. It contains zero internal project dependencies.
Consolidated Models: Moved the 19 byte-identical query configuration model files (including QueryConfig, Filter, Row, Value, etc.) into the new project under a single, flat namespace: Strata.SqlTools.Query.
Removed Duplication: Deleted all 38 original dialect-specific copies across the Snowflake and SQL Server projects. Both dialect projects now reference the new Strata.SqlTools.Query library via ProjectReference.
Updated Call Sites: Rewired explicit using statements in the 3 major external consumers:
- src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs
- tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs
- tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs

2. Code Smell Resolutions

Fixed IDE0028 (Collection Initialization): Consolidated and refactored the dual-instantiations of CalculationFilterGroup.GetValidFilters(). Changed the fallback empty list initialization from ?? new List<CalculationFilter>() to use the cleaner C# collection expression syntax: ?? [].
Fixed NUnit2045 (Assertion Short-Circuiting): In tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs, wrapped three independent parameter-index assertions inside an Assert.Multiple(() => { ... }) block. This ensures that failures report collectively rather than short-circuiting on the first tripped rule. (The reflection Is.Not.Null guard remains outside the block to correctly gate the subsequent Invoke statement).

🧪 Verification & Impact

Build Status: Clean build achieved via dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release.
Regression Safety: The ~819-test regression suite is completely green. Total pass counts perfectly match pre-refactor baselines.
SonarQube Metrics: * New-code period code smells dropped to 0.
- New-code duplicated lines dropped to 0.
- Project-wide duplication density (duplicated_lines_density) dropped significantly.

## 📋 Description This PR addresses technical debt and resolves outstanding SonarQube code smells introduced during the recent codebase sweep. The primary structural change eliminates massive duplication by extracting a dialect-agnostic query configuration model into a dedicated shared class library. ### 🔍 Root Cause of Duplication Prior to these changes, `Strata.SqlTools.Snowflake` and `Strata.SqlTools.SqlServer` each maintained an identical 19-file tree under their respective `ExpressionFactory/Query/` directories. This severe duplication was a major contributor to the project's **11.1% duplication density** metric in SonarQube. A recent update to `CalculationFilterGroup` across both locations tipped the new-code duplication analyzer over the acceptable threshold, triggering a quality gate blocker. --- ## 🛠️ Key Changes ### 1. Architecture & Project Extraction * **Created New Shared Project:** Scaffolded `src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj`, a standalone, BCL-only class library targeting `.NET 8.0`. It contains zero internal project dependencies. * **Consolidated Models:** Moved the 19 byte-identical query configuration model files (including `QueryConfig`, `Filter`, `Row`, `Value`, etc.) into the new project under a single, flat namespace: **`Strata.SqlTools.Query`**. * **Removed Duplication:** Deleted all 38 original dialect-specific copies across the Snowflake and SQL Server projects. Both dialect projects now reference the new `Strata.SqlTools.Query` library via `ProjectReference`. * **Updated Call Sites:** Rewired explicit `using` statements in the 3 major external consumers: * `src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs` * `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs` * `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs` ### 2. Code Smell Resolutions * **Fixed `IDE0028` (Collection Initialization):** Consolidated and refactored the dual-instantiations of `CalculationFilterGroup.GetValidFilters()`. Changed the fallback empty list initialization from `?? new List<CalculationFilter>()` to use the cleaner C# collection expression syntax: `?? []`. * **Fixed `NUnit2045` (Assertion Short-Circuiting):** In `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs`, wrapped three independent parameter-index assertions inside an `Assert.Multiple(() => { ... })` block. This ensures that failures report collectively rather than short-circuiting on the first tripped rule. *(The reflection `Is.Not.Null` guard remains outside the block to correctly gate the subsequent `Invoke` statement).* --- ## 🧪 Verification & Impact * **Build Status:** Clean build achieved via `dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release`. * **Regression Safety:** The ~819-test regression suite is completely green. Total pass counts perfectly match pre-refactor baselines. * **SonarQube Metrics:** * New-code period code smells dropped to **0**. * New-code duplicated lines dropped to **0**. * Project-wide duplication density (`duplicated_lines_density`) dropped significantly.

bermudalamb added 6 commits 2026-05-20 13:22:51 -05:00

docs: spec for Strata.SqlTools.Query shared-project extraction 55ab737888

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>

docs: implementation plan for Strata.SqlTools.Query extraction 70c9a3b3dc

Task-by-task plan to scaffold the shared project, move the 19 identical
ExpressionFactory/Query files, rewire dialect references, and clear the 3
remaining new-code SonarQube smells (IDE0028 x2, NUnit2045).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

build(query): scaffold empty Strata.SqlTools.Query shared project c5a4a4b02b

First step of extracting the duplicated ExpressionFactory/Query model
tree. Adds a BCL-only class library and registers it in the solution;
files are moved in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(query): extract shared Strata.SqlTools.Query model project 5cb32d2311

Moves the byte-identical 19-file ExpressionFactory/Query tree (duplicated
across Snowflake and SqlServer) into the new Strata.SqlTools.Query project
under the flat namespace Strata.SqlTools.Query. Both dialect projects now
reference the shared project; the Snowflake copies are deleted.

Also folds in the IDE0028 fix on CalculationFilterGroup.GetValidFilters
(collection expression []), which resolves both new-code IDE0028 smells
in one place now that there is a single copy.

Eliminates the 63 new duplicate lines flagged on the PR and removes the
largest contributor to the project's 11.1% duplication density. No
behavioral change: the moved types are identical to the originals.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(pgsql): wrap CommandVisitor asserts in Assert.Multiple 2fae7738e3

Resolves SonarQube NUnit2045. The three independent parameter-index
assertions now report together instead of short-circuiting on the first
failure. The Is.Not.Null guard in the reflection helper stays outside the
multiple block because it gates the subsequent Invoke.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Merge branch 'main' into fix/Sonarqube-Tech-Debt

SonarQube Analysis / sonarqube (pull_request) Successful in 3m49s

Details

24a9b36e36

bermudalamb merged commit 507cebd5f2 into main

2026-05-20 13:23:09 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lambda-Associates/sql-utilities#3