From 1124e91141cd9ea2fca7a9ccd5741554d72984c5 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:25:34 -0500 Subject: [PATCH 01/10] chore(sonar): remove three unused local variables (S1481) - LinqQueryBreakdown.cs:211 - drop unused `expr` pattern binding (type test remains) - LinqQueryBreakdown.cs:279 - drop unused `firstEntity` (empty check already above) - LinqExpressionVisitor.cs:315 - drop unused `param` pattern binding Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/sonarqube/SKILL.MD | 0 .../Breakdowns/LinqQueryBreakdown.cs | 5 ++--- .../Visitors/LinqExpressionVisitor.cs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) create mode 100644 .claude/skills/sonarqube/SKILL.MD diff --git a/.claude/skills/sonarqube/SKILL.MD b/.claude/skills/sonarqube/SKILL.MD new file mode 100644 index 0000000..e69de29 diff --git a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs index d7074cd..eac22d4 100644 --- a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs +++ b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs @@ -208,7 +208,7 @@ public class LinqQueryBreakdown : QueryBreakdown { // If the original expression can be converted to IQueryable, 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 // 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(); diff --git a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs index fc1e324..9e52b0d 100644 --- a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs +++ b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs @@ -312,7 +312,7 @@ public class LinqExpressionVisitor : ExpressionVisitor return GetFullMemberName(member); } - if (expression is ParameterExpression param) + if (expression is ParameterExpression) { return "*"; } From e1cdcd77a4236040fe83d3154fccbd5a9fdd8979 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:26:35 -0500 Subject: [PATCH 02/10] chore(sonar): collapse trivial backing-field properties to auto-properties (S2292) - WithClause.RecursiveQuery and WithClause.ColumnList had get/set bodies that only forwarded to private backing fields. Convert both to auto-properties and remove the now-orphaned _recursiveQuery / _columnList fields. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Classes/WithClause.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/Strata.SqlTools.SqlBreakdown/Classes/WithClause.cs b/src/Strata.SqlTools.SqlBreakdown/Classes/WithClause.cs index 5a10672..829c833 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Classes/WithClause.cs +++ b/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? _columnList; /// /// 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. /// /// - public IQueryBreakdown? RecursiveQuery - { - get => _recursiveQuery; - set => _recursiveQuery = value; - } + public IQueryBreakdown? RecursiveQuery { get; set; } /// /// Gets or sets the explicit column list for the CTE. @@ -135,11 +129,7 @@ public class WithClause : SqlClause, IWithClause /// /// The number of column names must match the number of columns in the SELECT clause. /// - public List? ColumnList - { - get => _columnList; - set => _columnList = value; - } + public List? ColumnList { get; set; } /// /// Initializes a new instance of the class. From 06e826a13c9c2221d46c1452c8d83173af0039ff Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:27:15 -0500 Subject: [PATCH 03/10] chore(sonar): drop redundant inline init now set in ctor (S3604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SqlServer/Breakdowns/QueryBreakdown.cs:26 — _clausesCacheDirty was both initialized inline (= true) and re-assigned in the constructor at line 56. Drop the inline initializer; the ctor remains authoritative. The four nearby S3604 false-positives on clause backing fields stay suppressed via #pragma — they are write-through targets of cache-invalidating property setters. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs index 494c6e5..b816559 100644 --- a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs +++ b/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 From c06ab2ea2929e9d5708329a45aa90e141f7605e9 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:27:53 -0500 Subject: [PATCH 04/10] chore(sonar): remove pass-through override that just calls base (S1185) Rules/Rule/Groups/With.GetExpressions only called base.GetExpressions(). The comment 'do some ordering here??' indicates the override is a TODO stub. Drop the override and preserve the intent as an inline TODO on the class. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Strata.SqlTools.Rules/Rule/Groups/With.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Strata.SqlTools.Rules/Rule/Groups/With.cs b/src/Strata.SqlTools.Rules/Rule/Groups/With.cs index 56fa42c..a7899b3 100644 --- a/src/Strata.SqlTools.Rules/Rule/Groups/With.cs +++ b/src/Strata.SqlTools.Rules/Rule/Groups/With.cs @@ -7,15 +7,8 @@ namespace Strata.SqlTools.Rules.Rule.Groups; /// public class With : Base { - /// - /// Gets the expressions from all rules, potentially with ordering applied. - /// - /// An enumerable of BoolExpr rule expressions. - protected override IEnumerable 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. /// /// Merges two BoolExpr expressions using WITH semantics. From b6ebb8c7cdf2fcfc159db9e77037f61aaf1f5935 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:29:13 -0500 Subject: [PATCH 05/10] chore(sonar): demote single-use field to local (S1450) LinqExpressionVisitor._tableName was only assigned and read inside VisitConstant immediately before assigning FromClause. Drop the field entirely and assign FromClause directly from entityType.Name. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Visitors/LinqExpressionVisitor.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs index 9e52b0d..26b6391 100644 --- a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs +++ b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs @@ -13,7 +13,6 @@ public class LinqExpressionVisitor : ExpressionVisitor private readonly StringBuilder _orderByBuilder = new(); private readonly List _methodCalls = new(); private bool _isInWhereClause; - private string? _tableName; /// /// 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; } } } From 8211047d6b0d6b949a060b0b7218d045c2f99387 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:30:58 -0500 Subject: [PATCH 06/10] chore(sonar): use LINQ Select for parameter match projection (S3267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snowflake/Breakdowns/ProcedureBreakdown.ParseCallParameters — replace the foreach over a MatchCollection with a .Cast().Select(m => m.Groups) projection. The loop now iterates GroupCollection values directly, indexing groups[1] and groups[2] for name/value without rebinding the Match. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/ProcedureBreakdown.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs index 8d6d27a..4e79517 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs +++ b/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().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) From 71cdf8a7663b43cb039752e884b980f263e168db Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:32:02 -0500 Subject: [PATCH 07/10] chore(sonar)!: pluralize unused [Flags] enums (S2342) S2342 requires [Flags] enums to use plural names. Both enums have zero references anywhere in the repo today. - ConstraintType -> ConstraintTypes - TriggerType -> TriggerTypes BREAKING CHANGE: external NuGet consumers (if any) referencing these singular-named types will need to update to the plural names. Internal codebase has no references. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Enums/SQL/{ConstraintType.cs => ConstraintTypes.cs} | 3 +-- .../Enums/SQL/{TriggerType.cs => TriggerTypes.cs} | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) rename src/Strata.SqlTools.SqlBreakdown/Enums/SQL/{ConstraintType.cs => ConstraintTypes.cs} (95%) rename src/Strata.SqlTools.SqlBreakdown/Enums/SQL/{TriggerType.cs => TriggerTypes.cs} (94%) diff --git a/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintType.cs b/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintTypes.cs similarity index 95% rename from src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintType.cs rename to src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintTypes.cs index 8f3bdd8..6707981 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintType.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/ConstraintTypes.cs @@ -4,7 +4,7 @@ namespace Strata.SqlTools.SqlBreakdown.Enums.SQL; /// Specifies the type of SQL constraint. /// [Flags] -public enum ConstraintType +public enum ConstraintTypes { /// /// Default value constraint. @@ -36,4 +36,3 @@ public enum ConstraintType /// All = 31 } - diff --git a/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerType.cs b/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerTypes.cs similarity index 94% rename from src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerType.cs rename to src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerTypes.cs index 06b9e69..85357ce 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerType.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Enums/SQL/TriggerTypes.cs @@ -4,7 +4,7 @@ namespace Strata.SqlTools.SqlBreakdown.Enums.SQL; /// Specifies the type of SQL trigger. /// [Flags] -public enum TriggerType +public enum TriggerTypes { /// /// Trigger fires after the action. @@ -21,4 +21,3 @@ public enum TriggerType /// Both = 3 } - From d92996bd2e85ee922cfe441772c1f39c058c505d Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 09:34:44 -0500 Subject: [PATCH 08/10] docs(skill): add sonarqube cleanup + inspection skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codifies (A) how to query the bermudalamb SonarQube 9.9 server's web API from PowerShell — Basic auth with token-as-username, the SONARQUBE_URL trailing-slash gotcha, the useful endpoints for triage and Won't Fix transitions; and (B) the per-rule-group cleanup loop (query → pick → edit → build → test → commit → scan → verify). References memory sonarqube-wontfix-rules rather than duplicating the catalog. Frontmatter follows superpowers:writing-skills (description is triggering conditions only, no workflow summary). Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/sonarqube/SKILL.MD | 66 +++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/.claude/skills/sonarqube/SKILL.MD b/.claude/skills/sonarqube/SKILL.MD index e69de29..97ad648 100644 --- a/.claude/skills/sonarqube/SKILL.MD +++ b/.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): (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. From 3c8148a6b1cbc85d1a28532d79827ea42412df39 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 10:03:34 -0500 Subject: [PATCH 09/10] chore(sonar): suppress CS8601 in QueryBreakdownMapper with justification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CS8601 is reported via external_roslyn on the SonarQube server, which does not expose transitions for external-analyzer issues — so a server-side Won't Fix is not available. Silence locally with a narrow pragma so the issue stops appearing in subsequent scans. Justification: Parameters is Dictionary (non-nullable value annotation), but a SQL parameter value can legitimately be null. The proper fix is to widen the public dictionary value type to object?, which ripples through every consumer of QueryBreakdown.Parameters — deferred to a separate change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Strata.SqlTools.EFCore/Services/QueryBreakdownMapper.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Strata.SqlTools.EFCore/Services/QueryBreakdownMapper.cs b/src/Strata.SqlTools.EFCore/Services/QueryBreakdownMapper.cs index f68abe0..f84ea88 100644 --- a/src/Strata.SqlTools.EFCore/Services/QueryBreakdownMapper.cs +++ b/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 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 } } From b3020a0a20bb37967e59558aa1b0f9c88e260dbe Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 10:07:19 -0500 Subject: [PATCH 10/10] chore: gitignore SonarScanner local artifacts Add .sonarqube/ (created by dotnet sonarscanner begin) and scan.log (from local scan-sonar.ps1 diagnostics) so they don't accidentally get committed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index ed6d1d2..86af55d 100644 --- a/.gitignore +++ b/.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