2026-05-19 17:26:28 -05:00
1 changed files with 618 additions and 0 deletions
@@ -0,0 +1,618 @@
 # Critical-tier SonarQube tech-debt sweep — Implementation Plan
 > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
 **Goal:** Resolve the 8 BLOCKER/CRITICAL non-S3776 SonarQube issues in `sql-utilities` using a pragmatic mix of fixes and justified suppressions. Implements the spec at `docs/superpowers/specs/2026-05-19-sonar-tech-debt-design.md`.
 **Architecture:** Each rule is addressed in isolation on the existing branch `fix/Sonarqube-Tech-Debt`. Five distinct commits, easiest → trickiest, so an early problem doesn't block the entire sweep. Existing tests (~818 passing) are the safety net for behavioral changes.
 **Tech Stack:** C# / .NET 8 / NUnit / SonarAnalyzer.CSharp 10.4.0 (referenced via `Directory.Build.props`).
 **Spec deviation flagged here:** During plan-time exploration I found that `HierarchicalData.AllChildData` (S2365 case #3) is part of the `IHierarchicalData` interface contract, is serialized via JSON (`tests/Strata.SqlTools.Rules.Tests/Data.json`), and its transformation is the property's deliberate intent. Refactoring to a method would break the interface and the serialization shape. Treatment for this one case is changed from **fix** to **suppress with justification** — same pattern as S3875. The other two S2365 cases (Snowflake/SqlServer `CalculationFilterGroup.Filters`) still get the refactor as specified.
 **Working directory assumption:** All paths are relative to `C:\gitea\sql-utilities`. Branch `fix/Sonarqube-Tech-Debt` is already checked out; `HEAD` is the spec commit `6b46109`.
 ---
 ## File map
 **Modified:**
 - `src/Strata.SqlTools.Rules/ExpressionVisitor.cs` — interface parameter rename (Task 1)
 - `src/Strata.SqlTools.LinqToSql/Builders/LinqQueryBreakdownBuilder.cs` — delete unused field (Task 2)
 - `src/Strata.SqlTools.Rules/Rule/Expression/Expression.Operators.cs` — `[SuppressMessage]` attribute (Task 3)
 - `src/Strata.SqlTools.PostgreSql/Visitors/CommandVisitor.cs` — static → instance field (Task 4)
 - `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs` — property → method (Task 5)
 - `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs` — property → method (Task 5)
 - `src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs` — `[SuppressMessage]` on `AllChildData` (Task 5)
 - `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs` — caller update (Task 5)
 - `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs` — caller update (Task 5)
 **Created:**
 - `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs` — cross-instance isolation test for S2696 (Task 4)
 ---
 ## Task 1: S927 — Rename `IVisitor.VisitNotEquals` parameter
 **Files:**
 - Modify: `src/Strata.SqlTools.Rules/ExpressionVisitor.cs:17`
 **Background:** Sonar flags the two implementation parameters (`notEqual`) as not matching the interface declaration (`Equal`). The interface is the one that's wrong — `Equal` makes no sense as a name on a `VisitNotEquals` method. Renaming the interface fixes both implementations in one shot. Only known caller is positional (`visitor.VisitNotEquals(this)` in `NotEqual.cs:19`), so the rename is binary-safe.
 - [ ] **Step 1: Read current state**
 Run:
 ```powershell
 Select-String -Path src/Strata.SqlTools.Rules/ExpressionVisitor.cs -Pattern "VisitNotEquals" -SimpleMatch
 ```
 Expected output includes:
 ```
 17:    T VisitNotEquals(NotEqual Equal);
 44:    public virtual Expression VisitNotEquals(NotEqual notEqual) => Validate(notEqual);
 80:    public virtual string VisitNotEquals(NotEqual notEqual)
 ```
 - [ ] **Step 2: Apply rename in interface**
 Edit `src/Strata.SqlTools.Rules/ExpressionVisitor.cs` line 17 from:
 ```csharp
    T VisitNotEquals(NotEqual Equal);
 ```
 to:
 ```csharp
    T VisitNotEquals(NotEqual notEqual);
 ```
 - [ ] **Step 3: Build to confirm no callers broke**
 Run:
 ```powershell
 dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
 ```
 Expected: build succeeds (warnings OK, errors not).
 - [ ] **Step 4: Run tests**
 Run:
 ```powershell
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: all tests pass; counts match the baseline (PostgreSql 107P/1S, SqlBreakdown 148P, Snowflake 220P/1S, EFCore 15P, LinqToSql 185P, Markdown 143P/4S, plus Rules and SqlServer test projects).
 - [ ] **Step 5: Commit**
 ```powershell
 git -C C:/gitea/sql-utilities add src/Strata.SqlTools.Rules/ExpressionVisitor.cs
 git -C C:/gitea/sql-utilities commit -m "fix(rules): rename IVisitor.VisitNotEquals parameter to match impls
 Resolves SonarQube S927 x2 in ExpressionVisitor.cs.
 The interface parameter was named ``Equal`` on a ``VisitNotEquals`` method,
 which is semantically wrong and forced implementations to mismatch.
 Implementations already used ``notEqual``; rename the interface to match.
 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
 ```
 ---
 ## Task 2: S4487 — Remove unread `_breakdown` field
 **Files:**
 - Modify: `src/Strata.SqlTools.LinqToSql/Builders/LinqQueryBreakdownBuilder.cs:11, :24`
 **Background:** Field is set in the constructor but never read. Confirmed by grep — only two references, both writes (declaration + constructor assignment). Safe delete.
 - [ ] **Step 1: Confirm no readers exist**
 Run:
 ```powershell
 Select-String -Path src/Strata.SqlTools.LinqToSql -Pattern "_breakdown" -SimpleMatch -Recurse
 ```
 Expected: only `LinqQueryBreakdownBuilder.cs:11` and `LinqQueryBreakdownBuilder.cs:24`. If any other file references `_breakdown`, **stop and investigate** — those would need updates before deletion.
 - [ ] **Step 2: Delete the field declaration**
 Edit `src/Strata.SqlTools.LinqToSql/Builders/LinqQueryBreakdownBuilder.cs` to remove line 11:
 ```csharp
    private readonly LinqQueryBreakdown _breakdown;
 ```
 (Leave the surrounding lines for `_selectColumns`, `_fromTable`, etc. intact.)
 - [ ] **Step 3: Delete the constructor initialization**
 In the same file, in the constructor at line 22-25, remove the `_breakdown` assignment so the constructor body is empty:
 Before:
 ```csharp
    public LinqQueryBreakdownBuilder()
    {
        _breakdown = new LinqQueryBreakdown();
    }
 ```
 After:
 ```csharp
    public LinqQueryBreakdownBuilder()
    {
    }
 ```
 - [ ] **Step 4: Build and test**
 Run:
 ```powershell
 dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: clean build, all tests pass at baseline counts.
 - [ ] **Step 5: Commit**
 ```powershell
 git -C C:/gitea/sql-utilities add src/Strata.SqlTools.LinqToSql/Builders/LinqQueryBreakdownBuilder.cs
 git -C C:/gitea/sql-utilities commit -m "fix(linq2sql): remove unread _breakdown field from builder
 Resolves SonarQube S4487 in LinqQueryBreakdownBuilder.cs.
 The field was assigned in the constructor but never read. Grep across
 the project confirmed no external references.
 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
 ```
 ---
 ## Task 3: S3875 — Suppress `operator==` rule with justification
 **Files:**
 - Modify: `src/Strata.SqlTools.Rules/Rule/Expression/Expression.Operators.cs:7-8`
 **Background:** The `operator==` overload on `Expression` is intentional DSL syntax — it returns a `Comparison` expression, not a `bool`. The file already has `#pragma warning disable CS0660, CS0661` documenting that `Equals` / `GetHashCode` are deliberately not overridden for the same reason. Adding a `[SuppressMessage]` for S3875 makes the analyzer agree with the design.
 - [ ] **Step 1: Add `using` for `SuppressMessage` if absent**
 Read `src/Strata.SqlTools.Rules/Rule/Expression/Expression.Operators.cs` lines 1-9. If there's no `using System.Diagnostics.CodeAnalysis;` near the top, add it as the first line of the file.
 - [ ] **Step 2: Apply `[SuppressMessage]` to the partial class declaration**
 Edit lines 6-8 from:
 ```csharp
 #pragma warning disable CS0660, CS0661
 public partial class Expression
 #pragma warning restore CS0660, CS0661
 ```
 to:
 ```csharp
 #pragma warning disable CS0660, CS0661
 [SuppressMessage("Major Code Smell", "S3875:\"operator==\" should not be overloaded on reference types", Justification = "Intentional DSL syntax: `expr1 == expr2` constructs a Comparison rule expression, not a bool. The existing CS0660/CS0661 pragma documents the deliberate omission of Equals/GetHashCode for the same reason.")]
 public partial class Expression
 #pragma warning restore CS0660, CS0661
 ```
 - [ ] **Step 3: Build and test**
 Run:
 ```powershell
 dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: clean build, all tests pass at baseline. The local Sonar analyzer should no longer warn on S3875 for this class.
 - [ ] **Step 4: Commit**
 ```powershell
 git -C C:/gitea/sql-utilities add src/Strata.SqlTools.Rules/Rule/Expression/Expression.Operators.cs
 git -C C:/gitea/sql-utilities commit -m "fix(rules): suppress S3875 on intentional DSL operator==
 Resolves SonarQube S3875 (BLOCKER) in Expression.Operators.cs.
 The operator== returns a Comparison expression (DSL semantics), not a
 bool. The existing CS0660/CS0661 pragma already documents this design;
 the new SuppressMessage attribute makes the Sonar analyzer agree.
 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
 ```
 ---
 ## Task 4: S2696 — Make `CommandVisitor._parameterIndex` instance-scoped
 **Files:**
 - Modify: `src/Strata.SqlTools.PostgreSql/Visitors/CommandVisitor.cs:12, :30`
 - Create: `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs`
 **Background:** `private static int _parameterIndex = 1;` is incremented inside an instance method `FormatParameterName`. Two `CommandVisitor` instances share state and the counter never resets — almost certainly a latent bug. Fix is one keyword removal. Add a test that proves cross-instance independence so the contract sticks.
 - [ ] **Step 1: Write the failing test**
 Create `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs`:
 ```csharp
 using NUnit.Framework;
 using Strata.SqlTools.SqlBreakdown.Expressions;
 using Strata.SqlTools.Visitors.PostgreSql;
 namespace Strata.SqlTools.PostgreSql.Tests.PostgreSql;
 [TestFixture]
 public class CommandVisitorTests
 {
    [Test]
    public void TwoVisitors_HaveIndependentParameterIndices()
    {
        var visitor1 = new CommandVisitor();
        var visitor2 = new CommandVisitor();
        var param1a = InvokeFormatParameterName(visitor1, "p");
        var param1b = InvokeFormatParameterName(visitor1, "p");
        var param2a = InvokeFormatParameterName(visitor2, "p");
        Assert.That(param1a, Is.EqualTo("$1"), "first visitor's first parameter should be $1");
        Assert.That(param1b, Is.EqualTo("$2"), "first visitor's second parameter should be $2");
        Assert.That(param2a, Is.EqualTo("$1"), "second visitor must start at $1, not inherit visitor1's counter");
    }
    private static string InvokeFormatParameterName(CommandVisitor visitor, string name)
    {
        var method = typeof(CommandVisitor).GetMethod(
            "FormatParameterName",
            System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Public)
            ?? typeof(Strata.SqlTools.Visitors.SqlServer.CommandVisitor).GetMethod(
                "FormatParameterName",
                System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
        Assert.That(method, Is.Not.Null, "FormatParameterName must exist as a protected method on the visitor hierarchy");
        return (string)method!.Invoke(visitor, new object[] { name })!;
    }
 }
 ```
 - [ ] **Step 2: Run the new test to confirm it fails**
 Run:
 ```powershell
 dotnet test tests/Strata.SqlTools.PostgreSql.Tests/Strata.SqlTools.PostgreSql.Tests.csproj -c Release --filter "FullyQualifiedName~CommandVisitorTests" --nologo
 ```
 Expected: FAIL — `param2a` will be `$3` (continuing visitor1's counter), not `$1`. The failure message will read something like *"second visitor must start at $1, not inherit visitor1's counter — Expected: \"$1\" But was: \"$3\""*.
 If the test instead passes immediately, the static field has already been changed; verify by reading the source.
 - [ ] **Step 3: Change static field to instance**
 Edit `src/Strata.SqlTools.PostgreSql/Visitors/CommandVisitor.cs` line 12 from:
 ```csharp
    private static int _parameterIndex = 1;
 ```
 to:
 ```csharp
    private int _parameterIndex = 1;
 ```
 No other changes needed — `FormatParameterName` already uses `_parameterIndex++` which now operates on the instance field.
 - [ ] **Step 4: Re-run the new test to confirm it passes**
 Run:
 ```powershell
 dotnet test tests/Strata.SqlTools.PostgreSql.Tests/Strata.SqlTools.PostgreSql.Tests.csproj -c Release --filter "FullyQualifiedName~CommandVisitorTests" --nologo
 ```
 Expected: PASS — all three assertions hold.
 - [ ] **Step 5: Run the full suite for regressions**
 Run:
 ```powershell
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: all tests pass; PostgreSql test count is +1 vs. baseline.
 - [ ] **Step 6: Commit**
 ```powershell
 git -C C:/gitea/sql-utilities add src/Strata.SqlTools.PostgreSql/Visitors/CommandVisitor.cs tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs
 git -C C:/gitea/sql-utilities commit -m "fix(pgsql): make CommandVisitor parameter index instance-scoped
 Resolves SonarQube S2696 in CommandVisitor.cs.
 The static _parameterIndex field was mutated from an instance method,
 causing every new CommandVisitor to inherit the previous instance's
 counter and never reset. Parameter indices now restart at \$1 per
 visitor, which is the intended PostgreSQL behavior.
 Adds CommandVisitorTests.TwoVisitors_HaveIndependentParameterIndices to
 lock in the contract via reflection (FormatParameterName is protected).
 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
 ```
 ---
 ## Task 5: S2365 — Refactor filtering collection-property getters
 **Files:**
 - Modify: `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs`
 - Modify: `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs`
 - Modify: `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs:14, :16`
 - Modify: `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs:14, :16`
 - Modify: `src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs:376-380` (suppress only, no shape change)
 **Background:** Three S2365 issues. Two are nearly identical (`CalculationFilterGroup.Filters` in Snowflake + SqlServer): a property getter that runs `.Where(...).ToList()` on every access. Refactor: keep `Filters` as a plain auto-property holding the raw collection (JSON round-trip stays intact), add `GetValidFilters()` method that does the filtering. Update internal `IsValid()` and external callers in both `QueryConfigExtensions.cs`.
 The third (`HierarchicalData.AllChildData`) is in an interface, serialized via JSON, and its transformation IS the property's contract. Treat as `[SuppressMessage]` with justification — same pattern as S3875.
 This task produces one commit that touches both refactors and the suppression.
 ### 5a — Snowflake `CalculationFilterGroup`
 - [ ] **Step 1: Apply the refactor**
 Replace the entire body of `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs` with:
 ```csharp
 using System.Text.Json.Serialization;
 namespace Strata.SqlTools.Snowflake.ExpressionFactory.Query;
 public class CalculationFilterGroup
 {
    // Hereditary logical operation applied to all Filters
    public LogicalOperator LogicalOperator { get; set; }
    public IEnumerable<CalculationFilter> Filters { get; set; }
    public CalculationFilterGroup()
    {
        LogicalOperator = LogicalOperator.And;
        Filters = new List<CalculationFilter>();
    }
    [JsonConstructor]
    public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
    {
        Filters = filters;
        LogicalOperator = logicalOperator;
    }
    public IEnumerable<CalculationFilter> GetValidFilters()
    {
        return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
    }
    public bool IsValid()
    {
        return GetValidFilters().Any();
    }
 }
 ```
 Key changes:
 - Removed `_filters` private field and `[JsonIgnore]` attribute (no longer needed)
 - `Filters` is now a plain auto-property storing the raw collection
 - Filtering logic moved to `GetValidFilters()`
 - `IsValid()` updated to call `GetValidFilters().Any()`
 ### 5b — SqlServer `CalculationFilterGroup`
 - [ ] **Step 2: Apply the same refactor in SqlServer**
 Replace the entire body of `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs` with the same content as Step 1 but with the namespace `Strata.SqlTools.SqlServer.ExpressionFactory.Query`:
 ```csharp
 using System.Text.Json.Serialization;
 namespace Strata.SqlTools.SqlServer.ExpressionFactory.Query;
 public class CalculationFilterGroup
 {
    // Hereditary logical operation applied to all Filters
    public LogicalOperator LogicalOperator { get; set; }
    public IEnumerable<CalculationFilter> Filters { get; set; }
    public CalculationFilterGroup()
    {
        LogicalOperator = LogicalOperator.And;
        Filters = new List<CalculationFilter>();
    }
    [JsonConstructor]
    public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
    {
        Filters = filters;
        LogicalOperator = logicalOperator;
    }
    public IEnumerable<CalculationFilter> GetValidFilters()
    {
        return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
    }
    public bool IsValid()
    {
        return GetValidFilters().Any();
    }
 }
 ```
 ### 5c — Update external callers in both `QueryConfigExtensions.cs`
 - [ ] **Step 3: Update Snowflake QueryConfigExtensions**
 In `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs`, replace lines 13-17:
 Before:
 ```csharp
        return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds)
            .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.Filters.Select(f => f.DataColumnId))))
            .Union(queryConfig.Rows.Select(row => row.DataColumnId))
            .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId)))
            .ToArray();
 ```
 After:
 ```csharp
        return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds)
            .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.GetValidFilters().Select(f => f.DataColumnId))))
            .Union(queryConfig.Rows.Select(row => row.DataColumnId))
            .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.GetValidFilters().Select(filter => filter.DataColumnId)))
            .ToArray();
 ```
 Rationale: the previous code accessed `.Filters` which was already filtered. To preserve the same semantics, switch to `.GetValidFilters()`. If you wanted the raw set instead, `.Filters` would still work — but the current behavior is "only valid filters contribute to column IDs," so we keep that.
 - [ ] **Step 4: Update SqlServer QueryConfigExtensions**
 Apply the identical change to `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs` lines 13-17 (same before/after as Step 3).
 ### 5d — Suppress S2365 on `HierarchicalData.AllChildData`
 - [ ] **Step 5: Add `using` and `[SuppressMessage]`**
 In `src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs`, confirm `using System.Diagnostics.CodeAnalysis;` is present near the top. If absent, add it.
 Then locate the `AllChildData` property on the `HierarchicalData` class (around line 376):
 Before:
 ```csharp
    public IEnumerable<IHierarchicalData> AllChildData
    {
        get => _childDataMap.SelectMany(x => x.Value).ToList();
        set => _childDataMap = value.GroupBy(x => x.DataSourceGuid).ToDictionary(x => x.Key, x => x.ToList());
    }
 ```
 After:
 ```csharp
    [SuppressMessage("Major Code Smell", "S2365:Properties should not make collection or array copies", Justification = "AllChildData is part of the IHierarchicalData interface contract and is JSON-serialized (see Data.json). The flatten-on-get / group-on-set transformation is the deliberate purpose of the property — _childDataMap is the storage form, the property is the wire form.")]
    public IEnumerable<IHierarchicalData> AllChildData
    {
        get => _childDataMap.SelectMany(x => x.Value).ToList();
        set => _childDataMap = value.GroupBy(x => x.DataSourceGuid).ToDictionary(x => x.Key, x => x.ToList());
    }
 ```
 ### 5e — Verify and commit the bundle
 - [ ] **Step 6: Build**
 Run:
 ```powershell
 dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
 ```
 Expected: clean build. If `QueryConfigExtensions.cs` fails to compile, double-check that `GetValidFilters()` is spelled correctly and the call sites compile against the new method signature.
 - [ ] **Step 7: Run full suite**
 Run:
 ```powershell
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: all tests pass at baseline + the new test from Task 4. If any test that exercises `CalculationFilterGroup` serialization breaks, inspect — the JSON shape did change (raw filters now serialize, not pre-filtered). If that's an issue, switch back to the suppression approach for the two `Filters` properties as well.
 - [ ] **Step 8: Commit**
 ```powershell
 git -C C:/gitea/sql-utilities add src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs
 git -C C:/gitea/sql-utilities commit -m "refactor: address S2365 collection-copying property getters
 Resolves SonarQube S2365 x3.
 CalculationFilterGroup (Snowflake + SqlServer): the ``Filters`` getter
 ran .Where(...).ToList() on every access. Convert to a plain
 auto-property holding the raw collection plus a ``GetValidFilters()``
 method that does the filtering. Update internal IsValid() and the two
 QueryConfigExtensions callers to use the new method.
 HierarchicalData.AllChildData: part of the IHierarchicalData interface
 and JSON-serialized; the transformation IS the property's contract.
 Suppress S2365 with justification rather than refactor — same pattern
 as S3875.
 Behavior change: CalculationFilterGroup JSON output now serializes the
 raw filter collection rather than the pre-filtered one. Round-trip is
 preserved; callers needing the filtered view must call GetValidFilters().
 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
 ```
 ---
 ## Final verification
 - [ ] **Step 1: Confirm clean tree and 6 commits ahead of main**
 Run:
 ```powershell
 git -C C:/gitea/sql-utilities status --short
 git -C C:/gitea/sql-utilities log --oneline main..HEAD
 ```
 Expected: empty working tree; six commits on `fix/Sonarqube-Tech-Debt`:
 1. `docs: spec for critical-tier SonarQube tech-debt sweep`
 2. `fix(rules): rename IVisitor.VisitNotEquals parameter to match impls`
 3. `fix(linq2sql): remove unread _breakdown field from builder`
 4. `fix(rules): suppress S3875 on intentional DSL operator==`
 5. `fix(pgsql): make CommandVisitor parameter index instance-scoped`
 6. `refactor: address S2365 collection-copying property getters`
 - [ ] **Step 2: Final full build + test pass**
 Run:
 ```powershell
 dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
 dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
 ```
 Expected: clean build, full suite green, test count = baseline + 1 (for the new `CommandVisitorTests`).
 - [ ] **Step 3: Push SonarQube analysis (optional but recommended)**
 Run (requires `$env:SONAR_TOKEN` set):
 ```powershell
 ./scan-sonar.ps1
 ```
 Expected: scanner uploads results to `https://snrqbe.bermudalamb.synology.me`. Open the dashboard and confirm:
 - S3875 issue shows as **suppressed** (Won't Fix or Resolved-Won't-Fix)
 - S2365 issue on `HierarchicalData.AllChildData` shows as **suppressed**
 - The other 6 issues (S927 x2, S4487, S2696, S2365 x2) show as **resolved**
 - [ ] **Step 4: Merge to main**
 ```powershell
 git -C C:/gitea/sql-utilities checkout main
 git -C C:/gitea/sql-utilities merge --ff-only fix/Sonarqube-Tech-Debt
 ```
 Expected: fast-forward merge succeeds. If it can't fast-forward (i.e. main moved during the sweep), pause and let the user decide between rebase, merge commit, or holding.
 ---
 ## Out of scope (do not do in this plan)
 - Refactor any S3776 cognitive-complexity hotspot
 - Touch any NUnit2045/2046 test-modernization issue
 - Change `.gitea/workflows/sonarqube.yml` (coverage reporting is being worked separately)
 - Push the branch to remote (Gitea) — user has been working locally