Merge pull request 'Fix/sonarqube critical major debt' (#11) from fix/sonarqube-critical-major-debt into main

Reviewed-on: #11

.claude/skills/sonarqube/SKILL.MD

@@ -0,0 +1,66 @@
+---
+name: sonarqube
+description: Use when working with SonarQube tech debt in this repo — querying the bermudalamb 9.9 server's web API from PowerShell, triaging open issues for the sql-utilities project, running scan-sonar.ps1 and verifying net-down after a cleanup commit.
+---
+
+# Sonarqube (sql-utilities)
+
+## Overview
+
+Two halves: **(A)** how to inspect the server from a PowerShell prompt, and **(B)** the per-rule-group cleanup loop that keeps tech-debt fixes from introducing new warnings.
+
+The repo has `SonarAnalyzer.CSharp` referenced in `Directory.Build.props`, so every `dotnet build` surfaces the same rule set the server reports — in the Error List / build output, not the SonarLint pane. Local build is the fast feedback loop; `scan-sonar.ps1` is what publishes results to the server.
+
+## A — Inspecting the server
+
+Server: `https://snrqbe.bermudalamb.synology.me` (9.9 LTS). Project key: `sql-utilities`.
+
+Auth is **HTTP Basic with the token as the username and an empty password** — **not** Bearer (Bearer was added in 10.0 and returns 401 here).
+
+```powershell
+$url = $env:SONARQUBE_URL.TrimEnd('/')   # the env var is stored with a trailing /
+$tok = $env:SONARQUBE_TOKEN              # squ_ user token, Windows user-scope
+$b64 = [Convert]::ToBase64String([Text.Encoding]::ASCII.GetBytes("${tok}:"))
+$h   = @{ Authorization = "Basic $b64" }
+
+Invoke-RestMethod -Uri "$url/api/issues/search?componentKeys=sql-utilities&resolved=false&ps=500&facets=severities,types,rules" -Headers $h
+```
+
+**Gotcha:** `SONARQUBE_URL` is stored with a trailing `/`. Without `TrimEnd('/')` you build `//api/...` and the server answers with the SPA HTML shell — `Invoke-RestMethod` happily returns it and downstream `.issues`/`.facets` access yields blank output silently.
+
+**Useful endpoints:**
+- `/api/issues/search` — `?severities=MAJOR,MINOR`, `?rules=csharpsquid:S1168`, `?resolutions=WONTFIX`, `&facets=rules,severities,types` for triage views.
+- `/api/issues/add_comment` (POST: `issue`, `text`) — justification before a Won't Fix transition.
+- `/api/issues/do_transition` (POST: `issue`, `transition=wontfix`) — mark a known false-positive without changing code.
+- `/api/qualitygates/project_status?projectKey=sql-utilities` — current gate status.
+- `/api/measures/component?component=sql-utilities&metricKeys=code_smells,coverage,duplicated_lines_density,ncloc,sqale_index` — headline numbers.
+- `/api/ce/component?component=sql-utilities` — most-recent analysis task status (queued / in-progress / failed).
+
+`SONARQUBE_URL` / `SONARQUBE_TOKEN` are read here; `scan-sonar.ps1` reads the separate `SONAR_TOKEN` / `SONAR_HOST_URL` for the *upload* path — do not conflate them.
+
+## B — Cleanup loop
+
+```dot
+digraph cleanup {
+    query [label="Query open issues + facets"];
+    pick [label="Pick one rule group"];
+    edit [label="Edit code"];
+    build [label="dotnet build -c Release"];
+    test  [label="dotnet test -c Release --no-build"];
+    commit [label="git commit\nchore(sonar): … (Sxxxx)"];
+    more [label="More groups?" shape=diamond];
+    scan [label="scan-sonar.ps1"];
+    verify [label="Query API: confirm net-down"];
+
+    query -> pick -> edit -> build -> test -> commit -> more;
+    more -> pick [label="yes"];
+    more -> scan [label="no"];
+    scan -> verify;
+}
+```
+
+1. **Query.** Start with `severities=MAJOR,MINOR` + `facets=rules` to see what's worth fixing.
+2. **Pick a rule group.** One Sxxxx (or one tightly-related cluster) per commit, easiest first so a late blocker doesn't strand the others.
+3. **Edit → build → test.** Build must succeed; the warning count for the touched rule must drop. Tests must stay green. If a fix would degrade clarity or break a tested contract, prefer **Won't Fix on the server with a justification** over a forced code change — see [[sonarqube-wontfix-rules]] for the catalog of rules already triaged that way (S1168, S3925, CS8601, S107).
+4. **Commit.** `chore(sonar): <one-line> (Sxxxx)` — bang (`!`) if it's a breaking rename.
+5. **After all groups:** `./scan-sonar.ps1` (needs `$env:SONAR_TOKEN`). Wait ~30-60s for the CE task to finish, then re-query the issues endpoint and confirm the open MAJOR/MINOR count fell by the expected number.

.gitignore

@@ -45,6 +45,10 @@ Generated\ Files/
 [Tt]est[Rr]esult*/
 [Bb]uild[Ll]og.*
 
+# SonarScanner artifacts
+.sonarqube/
+scan.log
+
 # NUnit
 *.VisualState.xml
 TestResult.xml

src/Strata.SqlTools.EFCore/Services/QueryBreakdownMapper.cs

@@ -166,7 +166,9 @@ public class QueryBreakdownMapper : IQueryBreakdownMapper
                     ? paramEntity.ParameterName
                     : $"@{paramEntity.ParameterName}";
 
+#pragma warning disable CS8601 // Parameters is Dictionary<string, object> but SQL parameter values can legitimately be null; widening the public dict value type is a broad ripple, deferred.
                 queryBreakdown.Parameters[key] = DeserializeParameterValue(paramEntity);
+#pragma warning restore CS8601
             }
         }
 

src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs

@@ -208,7 +208,7 @@ public class LinqQueryBreakdown : QueryBreakdown
         {
             // If the original expression can be converted to IQueryable<T>, use it
             // Otherwise, we cannot safely reconstruct without the original query provider
-            if (OriginalExpression is Expression expr && EntityType == typeof(T))
+            if (OriginalExpression is Expression && EntityType == typeof(T))
             {
                 // We have the expression, but we don't have the provider to create IQueryable<T>
                 // The breakdown analysis is one-way; reconstruction requires the original provider
@@ -275,8 +275,7 @@ public class LinqQueryBreakdown : QueryBreakdown
         var breakdown = new Breakdowns.SqlServer.InsertBreakdown();
         breakdown.TableName.Clause = typeof(T).Name;
 
-        // Use first entity to get column names
-        var firstEntity = entitiesList.First();
+        // Use the entity type to get column names
         var properties = typeof(T).GetProperties();
         var columnNames = new List<string>();
 

src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs

@@ -13,7 +13,6 @@ public class LinqExpressionVisitor : ExpressionVisitor
     private readonly StringBuilder _orderByBuilder = new();
     private readonly List<string> _methodCalls = new();
     private bool _isInWhereClause;
-    private string? _tableName;
 
     /// <summary>
     /// Gets the SELECT clause extracted from the expression.
@@ -119,8 +118,7 @@ public class LinqExpressionVisitor : ExpressionVisitor
                 var entityType = node.Type.GetGenericArguments().FirstOrDefault();
                 if (entityType != null)
                 {
-                    _tableName = entityType.Name;
-                    FromClause = _tableName;
+                    FromClause = entityType.Name;
                 }
             }
         }
@@ -312,7 +310,7 @@ public class LinqExpressionVisitor : ExpressionVisitor
             return GetFullMemberName(member);
         }
 
-        if (expression is ParameterExpression param)
+        if (expression is ParameterExpression)
         {
             return "*";
         }

src/Strata.SqlTools.Rules/Rule/Groups/With.cs

@@ -7,15 +7,8 @@ namespace Strata.SqlTools.Rules.Rule.Groups;
 /// </summary>
 public class With : Base
 {
-    /// <summary>
-    /// Gets the expressions from all rules, potentially with ordering applied.
-    /// </summary>
-    /// <returns>An enumerable of BoolExpr rule expressions.</returns>
-    protected override IEnumerable<BoolExpr> GetExpressions()
-    {
-        // do some ordering here??
-        return base.GetExpressions();
-    }
+    // TODO: revisit whether ordering should be applied here before delegating
+    // to the base GetExpressions(); inherit base behavior for now.
 
     /// <summary>
     /// Merges two BoolExpr expressions using WITH semantics.

src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs

@@ -235,12 +235,10 @@ public class ProcedureBreakdown : SqlServerProcedureBreakdown
             @"(\w+)\s*=>\s*([^,]+)(?:,|$)",
             System.Text.RegularExpressions.RegexOptions.IgnoreCase, Strata.SqlTools.SqlBreakdown.Utilities.RegexDefaults.MatchTimeout);
 
-        foreach (System.Text.RegularExpressions.Match paramMatch in paramMatches)
+        foreach (var groups in paramMatches.Cast<System.Text.RegularExpressions.Match>().Select(paramMatch => paramMatch.Groups))
         {
-            var paramName = paramMatch.Groups[1].Value.Trim();
-            var paramValue = paramMatch.Groups[2].Value.Trim();
             // Store with @ prefix for consistency with SQL Server
-            parameters["@" + paramName] = paramValue;
+            parameters["@" + groups[1].Value.Trim()] = groups[2].Value.Trim();
         }
 
         // If no named parameters found, try positional parameters (just values)

src/Strata.SqlTools.SqlBreakdown/Classes/WithClause.cs

@@ -29,8 +29,6 @@ public class WithClause : SqlClause, IWithClause
 {
     private SqlClauses? _sql;
     private IQueryBreakdown? _query;
-    private IQueryBreakdown? _recursiveQuery;
-    private List<string>? _columnList;
 
     /// <summary>
     /// Gets or sets the table name for the CTE.
@@ -115,11 +113,7 @@ public class WithClause : SqlClause, IWithClause
     /// Example recursive scenario: traversing an organizational hierarchy where employees reference their managers.
     /// </para>
     /// </remarks>
-    public IQueryBreakdown? RecursiveQuery
-    {
-        get => _recursiveQuery;
-        set => _recursiveQuery = value;
-    }
+    public IQueryBreakdown? RecursiveQuery { get; set; }
 
     /// <summary>
     /// Gets or sets the explicit column list for the CTE.
@@ -135,11 +129,7 @@ public class WithClause : SqlClause, IWithClause
     /// </list>
     /// The number of column names must match the number of columns in the SELECT clause.
     /// </remarks>
-    public List<string>? ColumnList
-    {
-        get => _columnList;
-        set => _columnList = value;
-    }
+    public List<string>? ColumnList { get; set; }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="WithClause"/> class.

src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintTypes.cs

@@ -4,7 +4,7 @@ namespace Strata.SqlTools.SqlBreakdown.Enums.SQL;
 /// Specifies the type of SQL constraint.
 /// </summary>
 [Flags]
-public enum ConstraintType
+public enum ConstraintTypes
 {
     /// <summary>
     /// Default value constraint.
@@ -36,4 +36,3 @@ public enum ConstraintType
     /// </summary>
     All = 31
 }
-

src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerTypes.cs

@@ -4,7 +4,7 @@ namespace Strata.SqlTools.SqlBreakdown.Enums.SQL;
 /// Specifies the type of SQL trigger.
 /// </summary>
 [Flags]
-public enum TriggerType
+public enum TriggerTypes
 {
     /// <summary>
     /// Trigger fires after the action.
@@ -21,4 +21,3 @@ public enum TriggerType
     /// </summary>
     Both = 3
 }
-

src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs

@@ -23,7 +23,7 @@ public class QueryBreakdown : SqlBreakdownBase, IQueryBreakdown
 
     // Caching fields for GetClauses() performance optimization
     private SqlClauses? _cachedClauses;
-    private bool _clausesCacheDirty = true;
+    private bool _clausesCacheDirty;
 
     // Backing fields for clause properties to support cache invalidation
 #pragma warning disable S3604 // "Fields should not be write-only" - False positive: These fields are used as backing fields for properties that manage cache invalidation