From 2c2a8b1193e59c2215f82000cd556ca88bf1946c Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:38:54 -0500 Subject: [PATCH 1/7] chore(sonar): bulk-fix mechanical CA/IDE analyzer warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied via `dotnet format analyzers --diagnostics IDE0028 CA1825 CA1834 CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA1846 CA1806 CA1869 CA2249 --severity info`. 19 files touched, all mechanical syntactic rewrites: - CA1847: string.Contains("x") -> string.Contains('x') - CA2249: s.IndexOf(c) == -1 -> !s.Contains(c) - CA1830: sb.Append(sb.ToString()) -> sb.Append(sb) - CA1834: StringBuilder.Append("x") -> Append('x') - CA1825, CA1860, CA1866, CA1853, IDE0028, CA1845: corresponding fixers Three rules in the batch had no batch fixer available (CA1846 ×4, CA1806 ×1, CA1869 ×1) and stay open for separate manual handling. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/LinqQueryBreakdown.cs | 2 +- .../Visitors/LinqExpressionVisitor.cs | 4 ++-- .../Expressions/ExpressionGenerator.cs | 6 +++--- .../SqlServer/QueryBreakdownGenerator.cs | 2 +- .../SqlServer/SqlStatementGenerator.cs | 2 +- .../Statements/StatementExpressionParser.cs | 4 ++-- src/Strata.SqlTools.Query/Value.cs | 2 +- .../Breakdowns/ProcedureBreakdown.cs | 4 ++-- .../Breakdowns/QueryBreakdown.cs | 8 ++++---- .../Breakdowns/QueryBreakdownCollection.cs | 6 +++--- src/Strata.SqlTools.SqlBreakdown/Utilities/GuidUtils.cs | 6 +++--- .../Utilities/SqlColumnHelpers.cs | 6 +++--- .../Utilities/SqlGuidHelpers.cs | 6 +++--- .../Utilities/SqlUtils.Filters.cs | 4 ++-- .../Breakdowns/QueryBreakdown.cs | 2 +- .../PostgreSql/QueryBreakdownTests.cs | 8 ++++---- tests/Strata.SqlTools.Rules.Tests/RuleVisitorTests.cs | 2 +- .../ExpressionTests/ExpressionFactoryFilterTests.cs | 2 +- .../SqlServer/RecursiveCTETests.cs | 2 +- 19 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs index eac22d4..df6053e 100644 --- a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs +++ b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs @@ -580,7 +580,7 @@ public class LinqQueryBreakdown : QueryBreakdown /// True if SELECT contains *; otherwise, false. public bool SelectsAllColumns() { - return SelectClause?.Clause?.Contains("*") ?? false; + return SelectClause?.Clause?.Contains('*') ?? false; } /// diff --git a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs index 26b6391..01f8bd9 100644 --- a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs +++ b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs @@ -248,13 +248,13 @@ public class LinqExpressionVisitor : ExpressionVisitor { if (_isInWhereClause) { - _whereBuilder.Append("("); + _whereBuilder.Append('('); Visit(node.Left); _whereBuilder.Append($" {GetOperator(node.NodeType)} "); Visit(node.Right); - _whereBuilder.Append(")"); + _whereBuilder.Append(')'); return node; } diff --git a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs index f9b5505..a74e8e2 100644 --- a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs @@ -366,7 +366,7 @@ public class ExpressionGenerator : IVisitor { return text; } - return text.Substring(0, maxLength) + "..."; + return string.Concat(text.AsSpan(0, maxLength), "..."); } #region IVisitor Implementation @@ -583,7 +583,7 @@ public class ExpressionGenerator : IVisitor { var sb = new StringBuilder(); sb.AppendLine($"{Indent()}Function: {function.FunctionName}"); - if (function.Arguments.Any()) + if (function.Arguments.Length != 0) { _indentLevel++; sb.AppendLine($"{Indent()}Arguments:"); @@ -602,7 +602,7 @@ public class ExpressionGenerator : IVisitor { var sb = new StringBuilder(); sb.AppendLine($"{Indent()}Aggregate Function: {aggregateFunction.FunctionName}"); - if (aggregateFunction.Arguments.Any()) + if (aggregateFunction.Arguments.Length != 0) { _indentLevel++; sb.AppendLine($"{Indent()}Arguments:"); diff --git a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs index db32359..05bb1a2 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs @@ -201,7 +201,7 @@ public class QueryBreakdownGenerator return text; } - return text.Substring(0, maxLength) + "..."; + return string.Concat(text.AsSpan(0, maxLength), "..."); } } diff --git a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs index 23cceb8..8c00dc6 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs @@ -122,7 +122,7 @@ public class SqlStatementGenerator return text; } - return text.Substring(0, maxLength) + "..."; + return string.Concat(text.AsSpan(0, maxLength), "..."); } /// diff --git a/src/Strata.SqlTools.PostgreSql/Statements/StatementExpressionParser.cs b/src/Strata.SqlTools.PostgreSql/Statements/StatementExpressionParser.cs index 130dc86..fc3cf9e 100644 --- a/src/Strata.SqlTools.PostgreSql/Statements/StatementExpressionParser.cs +++ b/src/Strata.SqlTools.PostgreSql/Statements/StatementExpressionParser.cs @@ -129,7 +129,7 @@ public class StatementExpressionParser : SqlServerStatementExpressionParser if (reader.TokenType == TokenType.String || reader.TokenType == TokenType.ColumnIdentifier) { - columnBuilder.Append(".").Append(reader.TokenValue); + columnBuilder.Append('.').Append(reader.TokenValue); reader.Read(); } else @@ -191,7 +191,7 @@ public class StatementExpressionParser : SqlServerStatementExpressionParser if (reader.TokenType == TokenType.ColumnIdentifier || reader.TokenType == TokenType.String) { - columnBuilder.Append(".").Append(reader.TokenValue); + columnBuilder.Append('.').Append(reader.TokenValue); reader.Read(); } else diff --git a/src/Strata.SqlTools.Query/Value.cs b/src/Strata.SqlTools.Query/Value.cs index c84325a..9672d29 100644 --- a/src/Strata.SqlTools.Query/Value.cs +++ b/src/Strata.SqlTools.Query/Value.cs @@ -11,7 +11,7 @@ public class Value public IEnumerable FilterGroups { get; } - public Value() : this(string.Empty, string.Empty, new int[0], new string[0], new CalculationFilterGroup[0]) + public Value() : this(string.Empty, string.Empty, Array.Empty(), Array.Empty(), Array.Empty()) { } diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs index 4e79517..9db037b 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/ProcedureBreakdown.cs @@ -55,7 +55,7 @@ public class ProcedureBreakdown : SqlServerProcedureBreakdown sb.Append("CALL "); sb.Append(ProcedureName.Clause); - sb.Append("("); + sb.Append('('); if (IsUsingParameters) { @@ -68,7 +68,7 @@ public class ProcedureBreakdown : SqlServerProcedureBreakdown sb.Append(string.Join(", ", paramList)); } - sb.Append(")"); + sb.Append(')'); return sb.ToString(); } diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs index 4ee4c10..4ab2451 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdown.cs @@ -141,7 +141,7 @@ public class QueryBreakdown : SqlServerQueryBreakdown if (parameterName.StartsWith('@')) { - return ":" + parameterName.Substring(1); + return string.Concat(":", parameterName.AsSpan(1)); } // Add : prefix @@ -159,7 +159,7 @@ public class QueryBreakdown : SqlServerQueryBreakdown if (paramName.StartsWith(':')) { // Add @param version - var atParam = "@" + paramName.Substring(1); + var atParam = string.Concat("@", paramName.AsSpan(1)); if (!Parameters.ContainsKey(atParam)) { Parameters[atParam] = Parameters[paramName]; @@ -168,7 +168,7 @@ public class QueryBreakdown : SqlServerQueryBreakdown else if (paramName.StartsWith('@')) { // Add :param version - var colonParam = ":" + paramName.Substring(1); + var colonParam = string.Concat(":", paramName.AsSpan(1)); if (!Parameters.ContainsKey(colonParam)) { Parameters[colonParam] = Parameters[paramName]; @@ -539,7 +539,7 @@ public class QueryBreakdown : SqlServerQueryBreakdown if (i > 0) { - sb.Append(","); + sb.Append(','); sb.AppendLine(); } diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs index 3fe389d..e4d469f 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs @@ -67,7 +67,7 @@ public class QueryBreakdownCollection : SqlServer.QueryBreakdownCollectionBase private static bool UsesStageReference(QueryBreakdown query) { - return query.GetSql().Contains("@") && + return query.GetSql().Contains('@') && (query.GetSql().Contains("FROM @") || query.GetSql().Contains(" @")); } diff --git a/src/Strata.SqlTools.SqlBreakdown/Utilities/GuidUtils.cs b/src/Strata.SqlTools.SqlBreakdown/Utilities/GuidUtils.cs index a869cd0..08a4e34 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Utilities/GuidUtils.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Utilities/GuidUtils.cs @@ -41,7 +41,7 @@ public static class GuidUtils return Guid.Empty; } - if (value.Contains("-")) + if (value.Contains('-')) { // This function was called on an already valid guid return new Guid(value); @@ -92,7 +92,7 @@ public static class GuidUtils var sb = new System.Text.StringBuilder(newguid); while (sb.Length < Guid.Empty.ToString().Length) { - sb.Append(sb.ToString()); + sb.Append(sb); } newguid = sb.ToString(); @@ -114,7 +114,7 @@ public static class GuidUtils for (int i = 0; i < guidChars.Length; i++) { char chr = guidChars[i]; - if (valid.IndexOf(chr) == -1 && !((chr == '-') && (i == 8 || i == 13 || i == 18 || i == 23))) + if (!valid.Contains(chr) && !((chr == '-') && (i == 8 || i == 13 || i == 18 || i == 23))) { guidChars[i] = valid[Math.Abs(StringUtils.GetHashCode32Bit(chr)) % valid.Length]; } diff --git a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlColumnHelpers.cs b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlColumnHelpers.cs index cfdf4d4..54c22a4 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlColumnHelpers.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlColumnHelpers.cs @@ -16,7 +16,7 @@ public static partial class SqlUtils public static void AppendAliasColumnWithComma(StringBuilder stringBuilder, string alias, string columnName) { stringBuilder.Append(alias); - stringBuilder.Append("."); + stringBuilder.Append('.'); stringBuilder.Append(columnName); stringBuilder.AppendLine(","); } @@ -58,7 +58,7 @@ public static partial class SqlUtils { stringBuilder.Append("CAST("); stringBuilder.Append(alias); - stringBuilder.Append("."); + stringBuilder.Append('.'); stringBuilder.Append(columnName); stringBuilder.Append(" AS "); stringBuilder.Append(dataType); @@ -94,7 +94,7 @@ public static partial class SqlUtils { if (i > 0) { - sb.Append(","); + sb.Append(','); } string trimmed = columnNames[i].Trim(); diff --git a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlGuidHelpers.cs b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlGuidHelpers.cs index 0319599..b9f50ed 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlGuidHelpers.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlGuidHelpers.cs @@ -42,10 +42,10 @@ public static partial class SqlUtils foreach (Guid g in guidList) { - sb.Append("'"); + sb.Append('\''); sb.Append(g.ToString()); - sb.Append("'"); - sb.Append(","); + sb.Append('\''); + sb.Append(','); } return sb.ToString().Trim().TrimEnd(','); diff --git a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.Filters.cs b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.Filters.cs index 0bc03bd..235836b 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.Filters.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.Filters.cs @@ -27,7 +27,7 @@ public static partial class SqlUtils double.TryParse(value, out double dblValue); // Optimize IN if only one value - if (operation == FilterOperation.In && !value.Contains(",")) + if (operation == FilterOperation.In && !value.Contains(',')) { operation = FilterOperation.Equal; } @@ -144,7 +144,7 @@ public static partial class SqlUtils } int lastCloseParen = sb.ToString().LastIndexOf(')'); - filter.SqlExpression = sb.ToString().Substring(0, lastCloseParen + 1) + ")"; + filter.SqlExpression = string.Concat(sb.ToString().AsSpan(0, lastCloseParen + 1), ")"); break; case FilterOperation.NotBetween: diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs index b816559..b1faed0 100644 --- a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs +++ b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs @@ -578,7 +578,7 @@ public class QueryBreakdown : SqlBreakdownBase, IQueryBreakdown if (i > 0) { - sb.Append(","); + sb.Append(','); sb.AppendLine(); } diff --git a/tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/QueryBreakdownTests.cs b/tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/QueryBreakdownTests.cs index fdb2d29..542d615 100644 --- a/tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/QueryBreakdownTests.cs +++ b/tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/QueryBreakdownTests.cs @@ -513,7 +513,7 @@ public class QueryBreakdownTests // Assert // Find the UserId parameter value - var userIdKey = merged.Keys.FirstOrDefault(k => k.Contains("UserId") && !k.Contains("$")); + var userIdKey = merged.Keys.FirstOrDefault(k => k.Contains("UserId") && !k.Contains('$')); Assert.That(userIdKey, Is.Not.Null); Assert.That(merged[userIdKey], Is.EqualTo(123)); // Main query value, not CTE value } @@ -559,9 +559,9 @@ public class QueryBreakdownTests var merged = mainQuery.GetMergedParameters(); // Assert - var dateKey = merged.Keys.FirstOrDefault(k => k.Contains("CreatedDate") && !k.Contains("$")); - var boolKey = merged.Keys.FirstOrDefault(k => k.Contains("IsActive") && !k.Contains("$")); - var doubleKey = merged.Keys.FirstOrDefault(k => k.Contains("Threshold") && !k.Contains("$")); + var dateKey = merged.Keys.FirstOrDefault(k => k.Contains("CreatedDate") && !k.Contains('$')); + var boolKey = merged.Keys.FirstOrDefault(k => k.Contains("IsActive") && !k.Contains('$')); + var doubleKey = merged.Keys.FirstOrDefault(k => k.Contains("Threshold") && !k.Contains('$')); Assert.That(dateKey, Is.Not.Null); Assert.That(merged[dateKey], Is.TypeOf()); diff --git a/tests/Strata.SqlTools.Rules.Tests/RuleVisitorTests.cs b/tests/Strata.SqlTools.Rules.Tests/RuleVisitorTests.cs index cc0ee1e..bd2de7c 100644 --- a/tests/Strata.SqlTools.Rules.Tests/RuleVisitorTests.cs +++ b/tests/Strata.SqlTools.Rules.Tests/RuleVisitorTests.cs @@ -133,7 +133,7 @@ public class TestRuleInput public string? ImGoingToMakeThisNull { get; set; } = null; - public TestRuleDetail[] Details { get; set; } = { }; + public TestRuleDetail[] Details { get; set; } = Array.Empty(); } public class TestRuleDetail diff --git a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs index eb5d708..dc9f9ee 100644 --- a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs +++ b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs @@ -31,7 +31,7 @@ public class ExpressionFactoryFilterTests : ExpressionTestsBase ).SetName("CalendarFilter_{m}"); yield return new TestCaseData( - new Filter(4, FilterType.Timeframe, new object[] { }, Array.Empty(), DatePart.Month, false, 1, 3), + new Filter(4, FilterType.Timeframe, Array.Empty(), Array.Empty(), DatePart.Month, false, 1, 3), $"DEPT.DISCHARGE_DATE >= '{startOfCurrentMonth:yyyy-MM-dd}' AND DEPT.DISCHARGE_DATE < '{startOfEndMonth:yyyy-MM-dd}'" ).SetName("TimeframeFilter_{m}"); } diff --git a/tests/Strata.SqlTools.SqlServer.Tests/SqlServer/RecursiveCTETests.cs b/tests/Strata.SqlTools.SqlServer.Tests/SqlServer/RecursiveCTETests.cs index 6880dc0..ba0d20e 100644 --- a/tests/Strata.SqlTools.SqlServer.Tests/SqlServer/RecursiveCTETests.cs +++ b/tests/Strata.SqlTools.SqlServer.Tests/SqlServer/RecursiveCTETests.cs @@ -174,7 +174,7 @@ public class RecursiveCTETests var merged = mainQuery.GetMergedParameters(); // Assert - var statusKey = merged.Keys.FirstOrDefault(k => k.Contains("Status") && !k.Contains("@") && !k.Contains("$")); + var statusKey = merged.Keys.FirstOrDefault(k => k.Contains("Status") && !k.Contains('@') && !k.Contains('$')); Assert.That(statusKey, Is.Not.Null); Assert.That(merged[statusKey], Is.EqualTo("active"), "Main query parameter should take precedence"); } -- 2.54.0 From 8b7fc1a327b6fa7dca5463cfcb1bfeb6653eb3a4 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:39:49 -0500 Subject: [PATCH 2/7] chore(sonar): use ArgumentNullException.ThrowIfNull (CA1510) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied via `dotnet format analyzers --diagnostics CA1510 --severity info`. Replaces `if (x == null) throw new ArgumentNullException(nameof(x));` blocks with the one-line `ArgumentNullException.ThrowIfNull(x);` — same behavior, same parameter name, much less noise. 6 files touched across SqlBreakdown, SqlServer, LinqToSql. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/LinqQueryBreakdown.cs | 30 ++++--------------- .../Converters/ReverseConverterExtensions.cs | 30 ++++--------------- .../Validators/QueryValidator.cs | 5 +--- .../Classes/SqlBreakdownCollection.cs | 15 ++-------- .../Expressions/InputPropertyExpression.cs | 5 +--- .../QueryBreakdownCollectionBase.cs | 10 ++----- 6 files changed, 19 insertions(+), 76 deletions(-) diff --git a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs index df6053e..68ad8d8 100644 --- a/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs +++ b/src/Strata.SqlTools.LinqToSql/Breakdowns/LinqQueryBreakdown.cs @@ -71,10 +71,7 @@ public class LinqQueryBreakdown : QueryBreakdown /// A LinqQueryBreakdown representing the query structure. public static LinqQueryBreakdown Analyze(IQueryable query) { - if (query == null) - { - throw new ArgumentNullException(nameof(query)); - } + ArgumentNullException.ThrowIfNull(query); var breakdown = new LinqQueryBreakdown { @@ -232,10 +229,7 @@ public class LinqQueryBreakdown : QueryBreakdown /// An InsertBreakdown representing the insert operation. public static Breakdowns.SqlServer.InsertBreakdown AnalyzeInsert(T entity) where T : class { - if (entity == null) - { - throw new ArgumentNullException(nameof(entity)); - } + ArgumentNullException.ThrowIfNull(entity); var breakdown = new Breakdowns.SqlServer.InsertBreakdown(); breakdown.TableName.Clause = typeof(T).Name; @@ -312,10 +306,7 @@ public class LinqQueryBreakdown : QueryBreakdown /// A DeleteBreakdown representing the delete operation. public static Breakdowns.SqlServer.DeleteBreakdown AnalyzeDelete(Expression> filterExpression) where T : class { - if (filterExpression == null) - { - throw new ArgumentNullException(nameof(filterExpression)); - } + ArgumentNullException.ThrowIfNull(filterExpression); var breakdown = new Breakdowns.SqlServer.DeleteBreakdown(); breakdown.FromClause.Clause = typeof(T).Name; @@ -343,14 +334,8 @@ public class LinqQueryBreakdown : QueryBreakdown Expression> filterExpression, Expression> updateExpression) where T : class { - if (filterExpression == null) - { - throw new ArgumentNullException(nameof(filterExpression)); - } - if (updateExpression == null) - { - throw new ArgumentNullException(nameof(updateExpression)); - } + ArgumentNullException.ThrowIfNull(filterExpression); + ArgumentNullException.ThrowIfNull(updateExpression); var breakdown = new Breakdowns.SqlServer.UpdateBreakdown(); breakdown.TableName.Clause = typeof(T).Name; @@ -425,10 +410,7 @@ public class LinqQueryBreakdown : QueryBreakdown /// A string representation of the trace analysis. public static string AnalyzeTrace(IQueryable query, string? executionContext = null) where T : class { - if (query == null) - { - throw new ArgumentNullException(nameof(query)); - } + ArgumentNullException.ThrowIfNull(query); var lines = new List { diff --git a/src/Strata.SqlTools.LinqToSql/Converters/ReverseConverterExtensions.cs b/src/Strata.SqlTools.LinqToSql/Converters/ReverseConverterExtensions.cs index d684c1a..9f5cc55 100644 --- a/src/Strata.SqlTools.LinqToSql/Converters/ReverseConverterExtensions.cs +++ b/src/Strata.SqlTools.LinqToSql/Converters/ReverseConverterExtensions.cs @@ -18,10 +18,7 @@ public static class ReverseConverterExtensions /// A new LinqQueryBreakdown with the same clauses. public static LinqQueryBreakdown ToLinqQueryBreakdown(this QueryBreakdown breakdown) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); var linq = new LinqQueryBreakdown( breakdown.SelectClause?.Clause ?? "*", @@ -54,10 +51,7 @@ public static class ReverseConverterExtensions /// A new LinqQueryBreakdown with the same clauses. public static LinqQueryBreakdown ToLinqQueryBreakdown(this PostgreSqlBreakdown breakdown) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); var linq = new LinqQueryBreakdown( breakdown.SelectClause?.Clause ?? "*", @@ -90,10 +84,7 @@ public static class ReverseConverterExtensions /// A new LinqQueryBreakdown with the same clauses. public static LinqQueryBreakdown ToLinqQueryBreakdown(this SnowflakeBreakdown breakdown) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); var linq = new LinqQueryBreakdown( breakdown.SelectClause?.Clause ?? "*", @@ -127,10 +118,7 @@ public static class ReverseConverterExtensions /// A new breakdown in the target dialect format. public static object ConvertToDialect(this QueryBreakdown breakdown, string targetDialect) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); return targetDialect.ToLowerInvariant() switch { @@ -150,10 +138,7 @@ public static class ReverseConverterExtensions /// A new breakdown in the target dialect format. public static object ConvertToDialect(this PostgreSqlBreakdown breakdown, string targetDialect) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); return targetDialect.ToLowerInvariant() switch { @@ -173,10 +158,7 @@ public static class ReverseConverterExtensions /// A new breakdown in the target dialect format. public static object ConvertToDialect(this SnowflakeBreakdown breakdown, string targetDialect) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); return targetDialect.ToLowerInvariant() switch { diff --git a/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs b/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs index 3085adb..6170410 100644 --- a/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs +++ b/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs @@ -71,10 +71,7 @@ public class QueryValidator /// This validator for method chaining. public QueryValidator Validate(LinqQueryBreakdown breakdown) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); _issues.Clear(); diff --git a/src/Strata.SqlTools.SqlBreakdown/Classes/SqlBreakdownCollection.cs b/src/Strata.SqlTools.SqlBreakdown/Classes/SqlBreakdownCollection.cs index ae378df..0abfdee 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Classes/SqlBreakdownCollection.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Classes/SqlBreakdownCollection.cs @@ -88,10 +88,7 @@ public class SqlBreakdownCollection : ICollection /// Thrown when breakdown is null. public void Add(ISqlBreakdown breakdown) { - if (breakdown == null) - { - throw new ArgumentNullException(nameof(breakdown)); - } + ArgumentNullException.ThrowIfNull(breakdown); _breakdowns.Add(breakdown); } @@ -103,10 +100,7 @@ public class SqlBreakdownCollection : ICollection /// Thrown when breakdowns is null. public void AddRange(IEnumerable breakdowns) { - if (breakdowns == null) - { - throw new ArgumentNullException(nameof(breakdowns)); - } + ArgumentNullException.ThrowIfNull(breakdowns); _breakdowns.AddRange(breakdowns); } @@ -142,10 +136,7 @@ public class SqlBreakdownCollection : ICollection /// Thrown when sqlBatch is null. public void ParseBatch(string sqlBatch, string? separator = null) { - if (sqlBatch == null) - { - throw new ArgumentNullException(nameof(sqlBatch)); - } + ArgumentNullException.ThrowIfNull(sqlBatch); Clear(); diff --git a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs index f984759..fd53385 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs @@ -269,10 +269,7 @@ public static class FlatDataUtils public static object GetValue(this IFlatData data, string key) { - if (data == null) - { - throw new ArgumentNullException(nameof(data)); - } + ArgumentNullException.ThrowIfNull(data); if (!data.ContainsKey(key)) { diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdownCollectionBase.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdownCollectionBase.cs index 5a5d0f9..88ad272 100644 --- a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdownCollectionBase.cs +++ b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdownCollectionBase.cs @@ -51,10 +51,7 @@ public abstract class QueryBreakdownCollectionBase : SqlBreakdownCollect /// Thrown when is null. public void Add(TQuery queryBreakdown) { - if (queryBreakdown == null) - { - throw new ArgumentNullException(nameof(queryBreakdown)); - } + ArgumentNullException.ThrowIfNull(queryBreakdown); QueryBreakdownList.Add(queryBreakdown); base.Add(queryBreakdown); @@ -67,10 +64,7 @@ public abstract class QueryBreakdownCollectionBase : SqlBreakdownCollect /// Thrown when is null. public void AddRange(IEnumerable queryBreakdowns) { - if (queryBreakdowns == null) - { - throw new ArgumentNullException(nameof(queryBreakdowns)); - } + ArgumentNullException.ThrowIfNull(queryBreakdowns); foreach (var breakdown in queryBreakdowns) { -- 2.54.0 From 84b06557e572eda357fe07d6dfff73365b8a5488 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:41:46 -0500 Subject: [PATCH 3/7] chore(sonar)!: mark instance-data-free members static (CA1822) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied via `dotnet format analyzers --diagnostics CA1822 --severity info`. 12 files touched. The fixer also updated internal callers in tests to use the type-name form (e.g. `gen.Method(x)` -> `Generator.Method(x)`); build and full test suite remain green. BREAKING CHANGE: two public methods become static and therefore can no longer be invoked through an instance reference by external consumers: - Strata.SqlTools.Markdown.LinqToSql.QueryBreakdownGenerator.GenerateMethodChainDiagram - Strata.SqlTools.Markdown.LinqToSql.SqlStatementGenerator.GenerateLinqPipelineDiagram Both are stateless utility methods on Generator classes — the static form is the correct shape; the only callers in this repo already used the type-name form. External code should change `gen.GenerateMethodChainDiagram(...)` to `QueryBreakdownGenerator.GenerateMethodChainDiagram(...)`. All other CA1822 hits in this commit are on private/protected members (no public-surface impact). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Validators/QueryValidator.cs | 6 +- .../Visitors/LinqExpressionVisitor.cs | 4 +- .../Expressions/ExpressionGenerator.cs | 2 +- .../Expressions/SimpleExpressionGenerator.cs | 2 +- .../LinqToSql/QueryBreakdownGenerator.cs | 2 +- .../LinqToSql/SqlStatementGenerator.cs | 2 +- .../SqlServer/QueryBreakdownGenerator.cs | 4 +- .../SqlServer/SqlStatementGenerator.cs | 6 +- .../LinqQueryBreakdownTests.cs | 70 +++++++++---------- .../LinqToSql/QueryBreakdownGeneratorTests.cs | 16 ++--- .../LinqToSql/SqlStatementGeneratorTests.cs | 8 +-- .../ExpressionTests/ExpressionTestsBase.cs | 2 +- 12 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs b/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs index 6170410..8e5e752 100644 --- a/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs +++ b/src/Strata.SqlTools.LinqToSql/Validators/QueryValidator.cs @@ -222,7 +222,7 @@ public class QueryValidator } } - private void ValidateWhereClause(LinqQueryBreakdown breakdown) + private static void ValidateWhereClause(LinqQueryBreakdown breakdown) { // No validation needed - WHERE is optional } @@ -242,12 +242,12 @@ public class QueryValidator } } - private void ValidateHavingClause(LinqQueryBreakdown breakdown) + private static void ValidateHavingClause(LinqQueryBreakdown breakdown) { // Validation delegated to ValidateGroupByClause } - private void ValidateOrderByClause(LinqQueryBreakdown breakdown) + private static void ValidateOrderByClause(LinqQueryBreakdown breakdown) { // No validation needed - ORDER BY is optional } diff --git a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs index 01f8bd9..4f07ad5 100644 --- a/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs +++ b/src/Strata.SqlTools.LinqToSql/Visitors/LinqExpressionVisitor.cs @@ -323,7 +323,7 @@ public class LinqExpressionVisitor : ExpressionVisitor return expression.ToString(); } - private string GetFullMemberName(MemberExpression expression) + private static string GetFullMemberName(MemberExpression expression) { var parts = new Stack(); var current = expression; @@ -354,7 +354,7 @@ public class LinqExpressionVisitor : ExpressionVisitor return string.Join(".", parts); } - private string GetOperator(ExpressionType nodeType) + private static string GetOperator(ExpressionType nodeType) { return nodeType switch { diff --git a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs index a74e8e2..706d197 100644 --- a/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/Expressions/ExpressionGenerator.cs @@ -260,7 +260,7 @@ public class ExpressionGenerator : IVisitor return sb.ToString(); } - private int GenerateMermaidNodes(Expression expression, StringBuilder sb, Dictionary nodeMap, ref int nodeCounter) + private static int GenerateMermaidNodes(Expression expression, StringBuilder sb, Dictionary nodeMap, ref int nodeCounter) { var currentNode = nodeCounter++; nodeMap[expression] = currentNode; diff --git a/src/Strata.SqlTools.Markdown/Expressions/SimpleExpressionGenerator.cs b/src/Strata.SqlTools.Markdown/Expressions/SimpleExpressionGenerator.cs index 9503e6f..afebea0 100644 --- a/src/Strata.SqlTools.Markdown/Expressions/SimpleExpressionGenerator.cs +++ b/src/Strata.SqlTools.Markdown/Expressions/SimpleExpressionGenerator.cs @@ -106,7 +106,7 @@ public class SimpleExpressionGenerator return sb.ToString(); } - private string GetExpressionDescription(Expression expression) + private static string GetExpressionDescription(Expression expression) { var typeName = expression.GetType().Name; diff --git a/src/Strata.SqlTools.Markdown/LinqToSql/QueryBreakdownGenerator.cs b/src/Strata.SqlTools.Markdown/LinqToSql/QueryBreakdownGenerator.cs index ff389b1..c55953b 100644 --- a/src/Strata.SqlTools.Markdown/LinqToSql/QueryBreakdownGenerator.cs +++ b/src/Strata.SqlTools.Markdown/LinqToSql/QueryBreakdownGenerator.cs @@ -37,7 +37,7 @@ public class QueryBreakdownGenerator /// The LINQ QueryBreakdown to visualize. /// Optional title for the diagram. /// A string containing the Mermaid flowchart showing method calls. - public string GenerateMethodChainDiagram(LinqQueryBreakdown queryBreakdown, string? title = null) + public static string GenerateMethodChainDiagram(LinqQueryBreakdown queryBreakdown, string? title = null) { var sb = new System.Text.StringBuilder(); diff --git a/src/Strata.SqlTools.Markdown/LinqToSql/SqlStatementGenerator.cs b/src/Strata.SqlTools.Markdown/LinqToSql/SqlStatementGenerator.cs index 3bb9583..b46c933 100644 --- a/src/Strata.SqlTools.Markdown/LinqToSql/SqlStatementGenerator.cs +++ b/src/Strata.SqlTools.Markdown/LinqToSql/SqlStatementGenerator.cs @@ -58,7 +58,7 @@ public class SqlStatementGenerator /// The query breakdown containing query information. /// Optional title for the diagram. /// A string containing the Mermaid diagram markdown. - public string GenerateLinqPipelineDiagram(IQueryBreakdown queryBreakdown, string? title = null) + public static string GenerateLinqPipelineDiagram(IQueryBreakdown queryBreakdown, string? title = null) { var sb = new System.Text.StringBuilder(); diff --git a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs index 05bb1a2..aa735a3 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/QueryBreakdownGenerator.cs @@ -177,7 +177,7 @@ public class QueryBreakdownGenerator /// /// Escapes text for Mermaid diagram labels to prevent syntax errors. /// - private string EscapeMermaidText(string text) + private static string EscapeMermaidText(string text) { return text .Replace("\"", """) @@ -194,7 +194,7 @@ public class QueryBreakdownGenerator /// /// Truncates text to a maximum length and adds ellipsis if needed. /// - private string TruncateText(string text, int maxLength) + private static string TruncateText(string text, int maxLength) { if (string.IsNullOrWhiteSpace(text) || text.Length <= maxLength) { diff --git a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs index 8c00dc6..30cce70 100644 --- a/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs +++ b/src/Strata.SqlTools.Markdown/SqlServer/SqlStatementGenerator.cs @@ -104,7 +104,7 @@ public class SqlStatementGenerator /// /// Escapes text for Mermaid diagram labels. /// - private string EscapeMermaidText(string text) + private static string EscapeMermaidText(string text) { return text .Replace("\"", """) @@ -115,7 +115,7 @@ public class SqlStatementGenerator /// /// Truncates text to a maximum length. /// - private string TruncateText(string text, int maxLength) + private static string TruncateText(string text, int maxLength) { if (string.IsNullOrWhiteSpace(text) || text.Length <= maxLength) { @@ -128,7 +128,7 @@ public class SqlStatementGenerator /// /// Cleans table name for use in Mermaid diagrams. /// - private string CleanTableName(string tableName) + private static string CleanTableName(string tableName) { return tableName .Replace("[", "") diff --git a/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs b/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs index b881741..e507480 100644 --- a/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs +++ b/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs @@ -18,7 +18,7 @@ public class LinqQueryBreakdownTests public void Analyze_SimpleSelectQuery_ExtractsCorrectClauses() { // Arrange - var query = from user in _context.Users + var query = from user in TestDataContext.Users select user; // Act @@ -34,7 +34,7 @@ public class LinqQueryBreakdownTests public void Analyze_WhereClause_ExtractsCondition() { // Arrange - var query = from user in _context.Users + var query = from user in TestDataContext.Users where user.Age > 21 select user; @@ -51,7 +51,7 @@ public class LinqQueryBreakdownTests public void Analyze_SelectWithProjection_ExtractsSelectedFields() { // Arrange - var query = from user in _context.Users + var query = from user in TestDataContext.Users select new { user.Id, user.Name }; // Act @@ -67,7 +67,7 @@ public class LinqQueryBreakdownTests public void Analyze_OrderByClause_ExtractsOrdering() { // Arrange - var query = from user in _context.Users + var query = from user in TestDataContext.Users orderby user.Name select user; @@ -83,7 +83,7 @@ public class LinqQueryBreakdownTests public void Analyze_MethodSyntax_ExtractsCorrectClauses() { // Arrange - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.Age > 18) .OrderBy(u => u.Name) .Select(u => new { u.Id, u.Name }); @@ -101,7 +101,7 @@ public class LinqQueryBreakdownTests public void Analyze_MethodSyntax_TracksMethodChain() { // Arrange - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.Age > 18) .OrderBy(u => u.Name) .Select(u => u.Name); @@ -120,7 +120,7 @@ public class LinqQueryBreakdownTests public void GetQuerySummary_ReturnsFormattedString() { // Arrange - var query = _context.Users.Where(u => u.Age > 21); + var query = TestDataContext.Users.Where(u => u.Age > 21); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -137,7 +137,7 @@ public class LinqQueryBreakdownTests public void GetMethodChain_ReturnsMethodSequence() { // Arrange - var query = _context.Users.Where(u => u.Age > 18).OrderBy(u => u.Name); + var query = TestDataContext.Users.Where(u => u.Age > 18).OrderBy(u => u.Name); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -153,7 +153,7 @@ public class LinqQueryBreakdownTests public void TryAnalyze_ValidQuery_ReturnsTrue() { // Arrange - var query = _context.Users.Where(u => u.Age > 21); + var query = TestDataContext.Users.Where(u => u.Age > 21); // Act var success = LinqQueryBreakdown.TryAnalyze(query, out var breakdown, out var error); @@ -182,7 +182,7 @@ public class LinqQueryBreakdownTests public void Analyze_ComplexQuery_HandlesCombinedClauses() { // Arrange - var query = from user in _context.Users + var query = from user in TestDataContext.Users where user.Age > 21 && user.IsActive orderby user.Name descending select new { user.Id, user.Name, user.Email }; @@ -205,7 +205,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndModify_AddWhereClause_GeneratesUpdatedSql() { // Arrange - Analyze an existing LINQ query - var originalQuery = _context.Users.Where(u => u.Age > 21); + var originalQuery = TestDataContext.Users.Where(u => u.Age > 21); var breakdown = LinqQueryBreakdown.Analyze(originalQuery); // Act - Add additional filter using breakdown @@ -222,7 +222,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndModify_ChangeSelectClause_GeneratesNewProjection() { // Arrange - Analyze query with projection - var originalQuery = _context.Users.Select(u => new { u.Id, u.Name }); + var originalQuery = TestDataContext.Users.Select(u => new { u.Id, u.Name }); var breakdown = LinqQueryBreakdown.Analyze(originalQuery); // Act - Modify the SELECT clause @@ -243,7 +243,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndModify_CloneAndExtend_CreatesIndependentQuery() { // Arrange - Analyze base query - var baseQuery = _context.Users.Where(u => u.Age > 18); + var baseQuery = TestDataContext.Users.Where(u => u.Age > 18); var baseBreakdown = LinqQueryBreakdown.Analyze(baseQuery); // Act - Clone and extend @@ -267,10 +267,10 @@ public class LinqQueryBreakdownTests public void AnalyzeAndCompose_MultipleQueries_CreatesUnionScenario() { // Arrange - Analyze two different queries - var activeUsersQuery = _context.Users.Where(u => u.IsActive); + var activeUsersQuery = TestDataContext.Users.Where(u => u.IsActive); var activeBreakdown = LinqQueryBreakdown.Analyze(activeUsersQuery); - var recentUsersQuery = _context.Users.Where(u => u.Age < 25); + var recentUsersQuery = TestDataContext.Users.Where(u => u.Age < 25); var recentBreakdown = LinqQueryBreakdown.Analyze(recentUsersQuery); // Act - Get SQL for both (could be used in UNION scenario) @@ -287,7 +287,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndBuildFilter_IncrementallyAddConditions_BuildsComplexFilter() { // Arrange - Start with simple query - var query = _context.Users; + var query = TestDataContext.Users; var breakdown = LinqQueryBreakdown.Analyze(query); // Act - Incrementally add filter conditions (simulating filter builder UI) @@ -311,7 +311,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndPaginate_AddOrderAndLimits_CreatesPaginatedQuery() { // Arrange - Analyze base query - var query = _context.Users.Where(u => u.IsActive); + var query = TestDataContext.Users.Where(u => u.IsActive); var breakdown = LinqQueryBreakdown.Analyze(query); // Act - Add pagination (ORDER BY required for consistent pagination) @@ -335,7 +335,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndGenerateReport_ExtractQueryMetrics_ProvidesAnalytics() { // Arrange - Complex query to analyze - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.Age > 21) .Where(u => u.IsActive) .OrderBy(u => u.Name) @@ -364,7 +364,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndOptimize_RemoveSelectStar_ImprovedProjection() { // Arrange - Analyze query with SELECT * - var query = _context.Users.Where(u => u.IsActive); + var query = TestDataContext.Users.Where(u => u.IsActive); var breakdown = LinqQueryBreakdown.Analyze(query); // Verify it initially has SELECT * @@ -390,10 +390,10 @@ public class LinqQueryBreakdownTests public void AnalyzeMultipleQueries_CompareAndMerge_CreatesCompositeQuery() { // Arrange - Analyze two related queries - var usersQuery = _context.Users.Where(u => u.Age > 21); + var usersQuery = TestDataContext.Users.Where(u => u.Age > 21); var usersBreakdown = LinqQueryBreakdown.Analyze(usersQuery); - var activeQuery = _context.Users.Where(u => u.IsActive); + var activeQuery = TestDataContext.Users.Where(u => u.IsActive); var activeBreakdown = LinqQueryBreakdown.Analyze(activeQuery); // Act - Merge conditions from both queries @@ -416,7 +416,7 @@ public class LinqQueryBreakdownTests public void AnalyzeAndDocument_GenerateQueryDocumentation_CreatesReadableOutput() { // Arrange - Analyze a business query - var query = _context.Orders + var query = TestDataContext.Orders .Where(o => o.Amount > 1000) .Where(o => o.OrderDate > DateTime.Now.AddDays(-30)) .OrderBy(o => o.OrderDate); @@ -451,8 +451,8 @@ public class LinqQueryBreakdownTests // Test data context and entities public class TestDataContext { - public IQueryable Users => new List().AsQueryable(); - public IQueryable Orders => new List().AsQueryable(); + public static IQueryable Users => new List().AsQueryable(); + public static IQueryable Orders => new List().AsQueryable(); } public class User @@ -488,7 +488,7 @@ public class GetQueryTests public void GetQuery_LinqBreakdown_ReturnsNullByDefault() { // Arrange - var query = _context.Users.Where(u => u.Age > 18); + var query = TestDataContext.Users.Where(u => u.Age > 18); var breakdown = LinqQueryBreakdown.Analyze(query); // Act - GetQuery returns null because LinqQueryBreakdown needs the original provider @@ -515,7 +515,7 @@ public class GetQueryTests public void GetQuery_MultipleBreakdownTypes_AllReturnNull() { // Arrange - var linqBreakdown = LinqQueryBreakdown.Analyze(_context.Users); + var linqBreakdown = LinqQueryBreakdown.Analyze(TestDataContext.Users); var sqlBreakdown = new QueryBreakdown("*", "Users"); // Act @@ -710,7 +710,7 @@ public class GetQueryTests public void AnalyzeTrace_WithValidQuery_ReturnsTraceString() { // Arrange - var query = _context.Users.Where(u => u.Age > 18); + var query = TestDataContext.Users.Where(u => u.Age > 18); // Act var trace = LinqQueryBreakdown.AnalyzeTrace(query); @@ -726,7 +726,7 @@ public class GetQueryTests public void AnalyzeTrace_WithExecutionContext_IncludesContextInTrace() { // Arrange - var query = _context.Users; + var query = TestDataContext.Users; const string context = "Initial Load"; // Act @@ -754,7 +754,7 @@ public class GetQueryTests public void ConvertToSqlServerBreakdown_CopiesAllClauses() { // Arrange - var query = _context.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); + var query = TestDataContext.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -772,7 +772,7 @@ public class GetQueryTests public void ConvertToPostgreSqlBreakdown_CopiesAllClauses() { // Arrange - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.IsActive) .OrderBy(u => u.Name) .Select(u => new { u.Id, u.Name }); @@ -793,7 +793,7 @@ public class GetQueryTests public void ConvertToSnowflakeBreakdown_CopiesAllClauses() { // Arrange - var query = _context.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); + var query = TestDataContext.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -827,7 +827,7 @@ public class GetQueryTests public void ConvertToPostgreSqlBreakdown_WithComplexQuery_CopieAllClauses() { // Arrange - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.IsActive && u.Age >= 18) .OrderByDescending(u => u.Age); @@ -848,7 +848,7 @@ public class GetQueryTests public void ConvertToSnowflakeBreakdown_PreservesAllClauseInformation() { // Arrange - var breakdown = LinqQueryBreakdown.Analyze(_context.Users.OrderBy(u => u.Name)); + var breakdown = LinqQueryBreakdown.Analyze(TestDataContext.Users.OrderBy(u => u.Name)); // Act var snowflakeBreakdown = breakdown.ConvertToSnowflakeBreakdown(); @@ -862,7 +862,7 @@ public class GetQueryTests public void ConvertToSqlServerBreakdown_InstancesAreIndependent() { // Arrange - var breakdown = LinqQueryBreakdown.Analyze(_context.Users.Where(u => u.Age > 21)); + var breakdown = LinqQueryBreakdown.Analyze(TestDataContext.Users.Where(u => u.Age > 21)); var sqlServerBreakdown = breakdown.ConvertToSqlServerBreakdown(); // Act - Modify the SQL Server breakdown @@ -876,7 +876,7 @@ public class GetQueryTests public void ConvertChainMultipleTimes_EachConversionIndependent() { // Arrange - var breakdown = LinqQueryBreakdown.Analyze(_context.Users.OrderBy(u => u.Name)); + var breakdown = LinqQueryBreakdown.Analyze(TestDataContext.Users.OrderBy(u => u.Name)); // Act var sqlServer1 = breakdown.ConvertToSqlServerBreakdown(); diff --git a/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/QueryBreakdownGeneratorTests.cs b/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/QueryBreakdownGeneratorTests.cs index e1603c6..d10d930 100644 --- a/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/QueryBreakdownGeneratorTests.cs +++ b/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/QueryBreakdownGeneratorTests.cs @@ -20,7 +20,7 @@ public class QueryBreakdownGeneratorTests public void GenerateMermaidDiagram_SimpleLinqQuery_GeneratesValidMermaid() { // Arrange - var query = _context.Users.Where(u => u.Age > 21); + var query = TestDataContext.Users.Where(u => u.Age > 21); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -39,14 +39,14 @@ public class QueryBreakdownGeneratorTests public void GenerateMethodChainDiagram_WithMethodCalls_ShowsChain() { // Arrange - var query = _context.Users + var query = TestDataContext.Users .Where(u => u.Age > 18) .OrderBy(u => u.Name) .Select(u => new { u.Id, u.Name }); var breakdown = LinqQueryBreakdown.Analyze(query); // Act - var result = _generator.GenerateMethodChainDiagram(breakdown, "Method Chain"); + var result = QueryBreakdownGenerator.GenerateMethodChainDiagram(breakdown, "Method Chain"); // Assert Assert.That(result, Does.Contain("```mermaid")); @@ -66,7 +66,7 @@ public class QueryBreakdownGeneratorTests var breakdown = new LinqQueryBreakdown("*", "Users"); // Act - var result = _generator.GenerateMethodChainDiagram(breakdown); + var result = QueryBreakdownGenerator.GenerateMethodChainDiagram(breakdown); // Assert Assert.That(result, Does.Contain("IQueryable")); @@ -77,7 +77,7 @@ public class QueryBreakdownGeneratorTests public void GenerateCombinedDiagram_IncludesBothDiagrams() { // Arrange - var query = _context.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); + var query = TestDataContext.Users.Where(u => u.Age > 21).OrderBy(u => u.Name); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -95,7 +95,7 @@ public class QueryBreakdownGeneratorTests public void GenerateMermaidDiagram_WithProjection_ShowsSelectedFields() { // Arrange - var query = _context.Users.Select(u => new { u.Id, u.Name, u.Email }); + var query = TestDataContext.Users.Select(u => new { u.Id, u.Name, u.Email }); var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -110,7 +110,7 @@ public class QueryBreakdownGeneratorTests public void GenerateMermaidDiagram_NullTitle_GeneratesWithoutTitle() { // Arrange - var query = _context.Users; + var query = TestDataContext.Users; var breakdown = LinqQueryBreakdown.Analyze(query); // Act @@ -125,7 +125,7 @@ public class QueryBreakdownGeneratorTests // Test data context public class TestDataContext { - public IQueryable Users => new List().AsQueryable(); + public static IQueryable Users => new List().AsQueryable(); } public class User diff --git a/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/SqlStatementGeneratorTests.cs b/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/SqlStatementGeneratorTests.cs index a91516a..5dafdfd 100644 --- a/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/SqlStatementGeneratorTests.cs +++ b/tests/Strata.SqlTools.Markdown.Tests/LinqToSql/SqlStatementGeneratorTests.cs @@ -21,7 +21,7 @@ public class SqlStatementGeneratorTests var query = new LinqQueryBreakdown("*", "Users"); // Act - var result = _generator.GenerateLinqPipelineDiagram(query, "User Query Pipeline"); + var result = SqlStatementGenerator.GenerateLinqPipelineDiagram(query, "User Query Pipeline"); // Assert Assert.That(result, Does.Contain("```mermaid")); @@ -42,7 +42,7 @@ public class SqlStatementGeneratorTests query.AddWhereClause("Age > 21"); // Act - var result = _generator.GenerateLinqPipelineDiagram(query); + var result = SqlStatementGenerator.GenerateLinqPipelineDiagram(query); // Assert Assert.That(result, Does.Contain("Where Predicate")); @@ -57,7 +57,7 @@ public class SqlStatementGeneratorTests var query = new LinqQueryBreakdown("Id, Name, Email", "Users"); // Act - var result = _generator.GenerateLinqPipelineDiagram(query); + var result = SqlStatementGenerator.GenerateLinqPipelineDiagram(query); // Assert Assert.That(result, Does.Contain("Select Projection")); @@ -72,7 +72,7 @@ public class SqlStatementGeneratorTests var query = new LinqQueryBreakdown("*", "Users"); // Act - var result = _generator.GenerateLinqPipelineDiagram(query, null); + var result = SqlStatementGenerator.GenerateLinqPipelineDiagram(query, null); // Assert Assert.That(result, Does.Not.Contain("###")); diff --git a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs index fa1efae..843eea0 100644 --- a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs +++ b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs @@ -57,7 +57,7 @@ public abstract class ExpressionTestsBase /// Executes an expression test case with arrange, act, and assert phases. /// /// The test case to execute. - protected void ExecuteExpressionTest(ExpressionTestCase testCase) + protected static void ExecuteExpressionTest(ExpressionTestCase testCase) { // Arrange var visitor = new CommandVisitor(); -- 2.54.0 From f5d539b906ebd88b5a75272369c8502a9a208792 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:45:16 -0500 Subject: [PATCH 4/7] chore(sonar): hoist constant array literals to static readonly fields (CA1861) Applied via `dotnet format analyzers --diagnostics CA1861 --severity info`, plus manual cleanup: - Renamed two cryptic fixer-generated field names: - QueryBreakdownCollection.stringArray -> SnowflakeFunctionNames (and inlined the now-redundant local alias) - ExpressionObjectTests.arg2 -> NotInValues - Deduped three identical `separator = ['\r','\n']` fields the fixer emitted in the same test class (kept the first declaration; the other two test methods now reuse it). 8 files touched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Breakdowns/QueryBreakdownCollection.cs | 21 +++++++++---------- .../Utilities/SqlUtils.cs | 3 ++- .../Statements/StatementParser.cs | 3 ++- .../LinqQueryBreakdownTests.cs | 3 ++- .../Snowflake/QueryBreakdownTests.cs | 8 ++++--- .../ExpressionFactoryFilterTests.cs | 4 +++- .../ExpressionTests/ExpressionObjectTests.cs | 4 +++- .../QueryBreakdownExtensionsTests.cs | 4 +++- 8 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs index e4d469f..7875456 100644 --- a/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs +++ b/src/Strata.SqlTools.Snowflake/Breakdowns/QueryBreakdownCollection.cs @@ -182,6 +182,15 @@ public class QueryBreakdownCollection : SqlServer.QueryBreakdownCollectionBase /// Filters queries that use Snowflake functions (PARSE_JSON, OBJECT_INSERT, ARRAY, etc.). /// @@ -191,17 +200,7 @@ public class QueryBreakdownCollection : SqlServer.QueryBreakdownCollectionBase { var sql = q.GetSql().ToUpperInvariant(); - - var snowflakeFunctions = new[] - { - "PARSE_JSON", "OBJECT_INSERT", "ARRAY_CONSTRUCT", "ARRAY_AGG", - "FLATTEN", "GET_PATH", "TRY_PARSE_JSON", "JSON_EXTRACT_PATH_TEXT", - "JSON_EXTRACT_PATH_WITH_DEFAULT", "HASHAGGREGATE", "LISTAGG", - "APPROX_COUNT_DISTINCT", "APPROX_PERCENTILE", "GREATEST", "LEAST", - "NULLIF", "ZEROIFNULL", "STRTOK", "SPLIT_PART", "PIVOT", "UNPIVOT" - }; - - return snowflakeFunctions.Any(func => sql.Contains(func)); + return SnowflakeFunctionNames.Any(func => sql.Contains(func)); }); } diff --git a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.cs b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.cs index ec6d6ee..fb3c1fd 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Utilities/SqlUtils.cs @@ -23,6 +23,7 @@ public static partial class SqlUtils public const string DATETIME_INSERT_FORMAT = "yyyyMMdd HH:mm:ss"; private const string DEFAULT_SCHEMA = "dbo"; + private static readonly char[] separator = new[] { ',' }; #region SQL String Manipulation @@ -34,7 +35,7 @@ public static partial class SqlUtils public static string StripColumnTableAlias(string sql) { // Break sql into words and remove "abc." from each column - string[] commaWords = sql.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries); + string[] commaWords = sql.Split(separator, StringSplitOptions.RemoveEmptyEntries); var newParts = new List(); foreach (string commaWord in commaWords) diff --git a/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs b/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs index 7bf7db3..79a6383 100644 --- a/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs +++ b/src/Strata.SqlTools.SqlServer/Statements/StatementParser.cs @@ -21,6 +21,7 @@ public class StatementParser public const string KeywordGroupBy = "GROUP BY"; public const string KeywordHaving = "HAVING"; public const string KeywordOrderBy = "ORDER BY"; + private static readonly char[] separator = new[] { '\r', '\n' }; #endregion @@ -50,7 +51,7 @@ public class StatementParser // Replace multiple spaces/tabs with single space, but preserve newlines for comment handling sql = Regex.Replace(sql, @"[ \t]+", " ", RegexOptions.None, RegexDefaults.MatchTimeout); // Remove leading/trailing whitespace from each line - var lines = sql.Split(new[] { '\r', '\n' }, StringSplitOptions.None); + var lines = sql.Split(separator, StringSplitOptions.None); sql = string.Join("\n", lines.Select(line => line.Trim())); return sql.Trim(); } diff --git a/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs b/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs index e507480..3464611 100644 --- a/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs +++ b/tests/Strata.SqlTools.LinqToSql.Tests/LinqQueryBreakdownTests.cs @@ -7,6 +7,7 @@ namespace Strata.SqlTools.LinqToSql.Tests; public class LinqQueryBreakdownTests { private TestDataContext _context = null!; + private static readonly string[] separator = new[] { "AND" }; [SetUp] public void Setup() @@ -303,7 +304,7 @@ public class LinqQueryBreakdownTests Assert.That(filterSql, Does.Contain("IsActive = 1")); // Should have multiple AND conditions - var andCount = filterSql.Split(new[] { "AND" }, StringSplitOptions.None).Length - 1; + var andCount = filterSql.Split(separator, StringSplitOptions.None).Length - 1; Assert.That(andCount, Is.GreaterThanOrEqualTo(2)); } diff --git a/tests/Strata.SqlTools.Snowflake.Tests/Snowflake/QueryBreakdownTests.cs b/tests/Strata.SqlTools.Snowflake.Tests/Snowflake/QueryBreakdownTests.cs index 28a97ec..58848b7 100644 --- a/tests/Strata.SqlTools.Snowflake.Tests/Snowflake/QueryBreakdownTests.cs +++ b/tests/Strata.SqlTools.Snowflake.Tests/Snowflake/QueryBreakdownTests.cs @@ -98,6 +98,8 @@ public class QueryBreakdownTests Assert.That(sql, Does.Contain("ORDER BY")); } + private static readonly char[] separator = new[] { '\r', '\n' }; + [Test] public void GetSql_WithSingleWithClause_UsesSnowflakeIndentation() { @@ -114,7 +116,7 @@ public class QueryBreakdownTests Assert.That(sql, Does.Contain("WITH")); Assert.That(sql, Does.Contain("PRODUCT_SUMMARY AS (")); // Verify 4-space Snowflake indentation - var lines = sql.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + var lines = sql.Split(separator, StringSplitOptions.RemoveEmptyEntries); var indentedLines = lines.Where(l => l.StartsWith(" ")).ToList(); Assert.That(indentedLines.Count, Is.GreaterThan(0)); } @@ -1118,7 +1120,7 @@ public class QueryBreakdownTests _ = baseQueryBreakdown.GetSql(); // Assert - Snowflake should use 4-space indent, base uses 5-space - var snowflakeLines = snowflakeSql.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + var snowflakeLines = snowflakeSql.Split(separator, StringSplitOptions.RemoveEmptyEntries); var snowflakeIndentedLines = snowflakeLines.Where(l => l.StartsWith(" ") && !l.StartsWith(" ")).ToList(); Assert.That(snowflakeIndentedLines.Count, Is.GreaterThan(0), "Snowflake should use 4-space indentation"); } @@ -1322,7 +1324,7 @@ public class QueryBreakdownTests Assert.That(snowflakeSql, Does.Contain("WHERE")); Assert.That(snowflakeSql, Does.Contain("ORDER BY")); // Verify Snowflake-style formatting (4-space indentation) - var lines = snowflakeSql.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + var lines = snowflakeSql.Split(separator, StringSplitOptions.RemoveEmptyEntries); var indentedLines = lines.Where(l => l.StartsWith(" ")).ToList(); Assert.That(indentedLines.Count, Is.GreaterThan(0), "Should have Snowflake-style indentation"); } diff --git a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs index dc9f9ee..25ce2f1 100644 --- a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs +++ b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs @@ -5,6 +5,8 @@ namespace Strata.SqlTools.SqlBreakdown.Tests.ExpressionTests; [TestFixture] public class ExpressionFactoryFilterTests : ExpressionTestsBase { + private static readonly string[] values = new[] { "FY2019", "FY2020", "FY2021", "FY2022" }; + private static IEnumerable FilterTestCases() { var dischargeDate = DateTime.Now.Date.AddMonths(1); @@ -16,7 +18,7 @@ public class ExpressionFactoryFilterTests : ExpressionTestsBase ).SetName("ListFilterContinuous_{m}"); yield return new TestCaseData( - new Filter(4, FilterType.List, new[] { "FY2019", "FY2020", "FY2021", "FY2022" }, Array.Empty(), DatePart.FiscalYear, false, 0, 0), + new Filter(4, FilterType.List, values, Array.Empty(), DatePart.FiscalYear, false, 0, 0), "(DEPT.DISCHARGE_DATE >= '2018-07-01' AND DEPT.DISCHARGE_DATE < '2019-07-01') OR \n(DEPT.DISCHARGE_DATE >= '2019-07-01' AND DEPT.DISCHARGE_DATE < '2020-07-01') OR \n(DEPT.DISCHARGE_DATE >= '2020-07-01' AND DEPT.DISCHARGE_DATE < '2021-07-01') OR \n(DEPT.DISCHARGE_DATE >= '2021-07-01' AND DEPT.DISCHARGE_DATE < '2022-07-01')" ).SetName("DateListFilterFiscalYear_{m}"); diff --git a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionObjectTests.cs b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionObjectTests.cs index 5aa05b2..ed6af05 100644 --- a/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionObjectTests.cs +++ b/tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionObjectTests.cs @@ -29,6 +29,8 @@ public class ExpressionObjectTests : ExpressionTestsBase Assert.That(paramExp.ParameterName, Is.EqualTo("MY_PARAM")); } + private static readonly string[] NotInValues = new[] { "value1", "value2", "value3" }; + private static IEnumerable ComparisonExpressionTestCases() { yield return new TestCaseData("GreaterThanOrEqual", 250, typeof(GreaterThanOrEqualToExpression)) @@ -40,7 +42,7 @@ public class ExpressionObjectTests : ExpressionTestsBase yield return new TestCaseData("Equals", "TestDept", typeof(EqualToExpression)) .SetName("Equals_{m}"); - yield return new TestCaseData("NotIn", new[] { "value1", "value2", "value3" }, typeof(NotInExpression)) + yield return new TestCaseData("NotIn", NotInValues, typeof(NotInExpression)) .SetName("NotIn_{m}"); yield return new TestCaseData("Like", "%pattern%", typeof(LikeExpression)) diff --git a/tests/Strata.SqlTools.SqlBreakdown.Tests/Extensions/QueryBreakdownExtensionsTests.cs b/tests/Strata.SqlTools.SqlBreakdown.Tests/Extensions/QueryBreakdownExtensionsTests.cs index 5061e75..8b4185e 100644 --- a/tests/Strata.SqlTools.SqlBreakdown.Tests/Extensions/QueryBreakdownExtensionsTests.cs +++ b/tests/Strata.SqlTools.SqlBreakdown.Tests/Extensions/QueryBreakdownExtensionsTests.cs @@ -243,12 +243,14 @@ public class QueryBreakdownExtensionsTests Assert.That(query.WithClauses[1].TableName, Is.EqualTo("recent_orders")); } + private static readonly string[] columns = new[] { "id", "name", "email" }; + [Test] public void WithCte_WithColumnList_AddsCtesWithColumns() { // Arrange & Act var query = new QueryBreakdown() - .WithCte("active_users", new[] { "id", "name", "email" }, cte => cte + .WithCte("active_users", columns, cte => cte .Select("user_id, user_name, user_email") .From("users") .Where("status = 'active'")) -- 2.54.0 From 81254c12f99e33ead8a013630b5f9540b4bcb47b Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:46:19 -0500 Subject: [PATCH 5/7] chore(sonar): prefer TryGetValue over ContainsKey+indexer (CA1854) Applied via `dotnet format analyzers --diagnostics CA1854 --severity info`. Eliminates the duplicate hash lookup in the `if (d.ContainsKey(k)) d[k]++ else d[k] = 1` pattern. The fixer rewrites the conditional to `if (d.TryGetValue(k, out var value)) d[k] = ++value;` which is semantically identical but does the lookup once. 4 files touched. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Analyzers/QueryCollectionAnalyzer.cs | 8 ++++---- .../Statements/StatementParser.cs | 6 ++---- .../Statements/StatementParser.cs | 4 ++-- .../Expressions/InputPropertyExpression.cs | 4 +--- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/Strata.SqlTools.LinqToSql/Analyzers/QueryCollectionAnalyzer.cs b/src/Strata.SqlTools.LinqToSql/Analyzers/QueryCollectionAnalyzer.cs index d692287..77fccb9 100644 --- a/src/Strata.SqlTools.LinqToSql/Analyzers/QueryCollectionAnalyzer.cs +++ b/src/Strata.SqlTools.LinqToSql/Analyzers/QueryCollectionAnalyzer.cs @@ -186,9 +186,9 @@ public class QueryCollectionAnalyzer var table = query.FromClause?.Clause?.Trim(); if (!string.IsNullOrWhiteSpace(table)) { - if (tableUsage.ContainsKey(table)) + if (tableUsage.TryGetValue(table, out int value)) { - tableUsage[table]++; + tableUsage[table] = ++value; } else { @@ -221,9 +221,9 @@ public class QueryCollectionAnalyzer foreach (var col in columns) { var columnName = col.Trim(); - if (columnUsage.ContainsKey(columnName)) + if (columnUsage.TryGetValue(columnName, out int value)) { - columnUsage[columnName]++; + columnUsage[columnName] = ++value; } else { diff --git a/src/Strata.SqlTools.PostgreSql/Statements/StatementParser.cs b/src/Strata.SqlTools.PostgreSql/Statements/StatementParser.cs index 8727e3f..444f2be 100644 --- a/src/Strata.SqlTools.PostgreSql/Statements/StatementParser.cs +++ b/src/Strata.SqlTools.PostgreSql/Statements/StatementParser.cs @@ -113,9 +113,8 @@ public class StatementParser : SqlServerStatementParser // PostgreSQL-specific: Append LIMIT/OFFSET to ORDER BY if present var orderByClause = clauses.OrderByClause?.Clause ?? string.Empty; - if (clausePositions.ContainsKey(KeywordLimit)) + if (clausePositions.TryGetValue(KeywordLimit, out int limitStart)) { - var limitStart = clausePositions[KeywordLimit]; var limitEnd = clausePositions.Values .Where(v => v > limitStart) .Order() @@ -127,9 +126,8 @@ public class StatementParser : SqlServerStatementParser : $"{orderByClause} {limitClause}"; } - if (clausePositions.ContainsKey(KeywordOffset)) + if (clausePositions.TryGetValue(KeywordOffset, out int offsetStart)) { - var offsetStart = clausePositions[KeywordOffset]; var offsetEnd = clausePositions.Values .Where(v => v > offsetStart) .Order() diff --git a/src/Strata.SqlTools.Snowflake/Statements/StatementParser.cs b/src/Strata.SqlTools.Snowflake/Statements/StatementParser.cs index 0feb85f..def7770 100644 --- a/src/Strata.SqlTools.Snowflake/Statements/StatementParser.cs +++ b/src/Strata.SqlTools.Snowflake/Statements/StatementParser.cs @@ -119,9 +119,9 @@ public class StatementParser : SqlServerStatementParser protected override void PostProcessClauses(SqlClauses clauses, string sql, Dictionary clausePositions) { // Snowflake-specific: Append LIMIT to ORDER BY if present - if (clausePositions.ContainsKey(KeywordLimit)) + if (clausePositions.TryGetValue(KeywordLimit, out int value)) { - var limitClause = sql.Substring(clausePositions[KeywordLimit]).Trim(); + var limitClause = sql.Substring(value).Trim(); if (clauses.OrderByClause != null) { clauses.OrderByClause.Clause = string.IsNullOrEmpty(clauses.OrderByClause.Clause) diff --git a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs index fd53385..013c9b4 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs @@ -271,15 +271,13 @@ public static class FlatDataUtils { ArgumentNullException.ThrowIfNull(data); - if (!data.ContainsKey(key)) + if (!data.TryGetValue(key, out object? rawValue)) { throw new ArgumentException( $"The specified key is not available. Requested key: [{key}] Available keys: [{string.Join(", ", data.Keys)}]", nameof(key)); } - var rawValue = data[key]; - return rawValue; } -- 2.54.0 From 6af239035baa5c129f454d0649e7c521ecda7f7f Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:49:20 -0500 Subject: [PATCH 6/7] chore(sonar): use concrete return types where binary-safe (CA1859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CA1859 has no `dotnet format` batch fixer, so applied manually after verifying each site is private/internal/test (no public-surface impact): - Markdown.TryParseLogicalOperation: Expression? -> BoolExpr? (private) - Markdown.TryParseComparison: Expression? -> Comparison? (private) - RuleSet.GetAllSingleRules(IGroup): IEnumerable -> List (private overload) - SqlExpressionClause.SplitOnComma: IEnumerable -> List (private) - QueryBreakdownRepositoryTests._repository: IQueryBreakdownRepository -> QueryBreakdownRepository (test private field) - UnitTest1.StartsWith(...,string): Expression -> MethodCallExpression (test private helper) The public `RuleSet.GetAllSingleRules()` overload still returns IEnumerable — only the private recursive helper was tightened. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Strata.SqlTools.Rules/Rule/Expression/Markdown.cs | 4 ++-- src/Strata.SqlTools.Rules/Rule/RuleSet.cs | 2 +- .../Classes/SqlExpressionClause.cs | 2 +- .../QueryBreakdownRepositoryTests.cs | 2 +- tests/Strata.SqlTools.Rules.Tests/UnitTest1.cs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Strata.SqlTools.Rules/Rule/Expression/Markdown.cs b/src/Strata.SqlTools.Rules/Rule/Expression/Markdown.cs index ca1c590..716a14f 100644 --- a/src/Strata.SqlTools.Rules/Rule/Expression/Markdown.cs +++ b/src/Strata.SqlTools.Rules/Rule/Expression/Markdown.cs @@ -115,7 +115,7 @@ public static class Markdown return ParseAtomicExpression(text); } - private static Expression? TryParseLogicalOperation(string text) + private static BoolExpr? TryParseLogicalOperation(string text) { foreach (var op in LogicalOperators.Keys) { @@ -130,7 +130,7 @@ public static class Markdown return null; } - private static Expression? TryParseComparison(string text) + private static Comparison? TryParseComparison(string text) { foreach (var op in ComparisonOperators.Keys) { diff --git a/src/Strata.SqlTools.Rules/Rule/RuleSet.cs b/src/Strata.SqlTools.Rules/Rule/RuleSet.cs index 6dbc2d0..a90d5b2 100644 --- a/src/Strata.SqlTools.Rules/Rule/RuleSet.cs +++ b/src/Strata.SqlTools.Rules/Rule/RuleSet.cs @@ -50,7 +50,7 @@ public class RuleSet : IGroup /// /// The rule group to traverse. /// An enumerable of all single rules found in the group. - private static IEnumerable GetAllSingleRules(IGroup group) + private static List GetAllSingleRules(IGroup group) { var rules = new List(); foreach (var rule in group.Rules) diff --git a/src/Strata.SqlTools.SqlBreakdown/Classes/SqlExpressionClause.cs b/src/Strata.SqlTools.SqlBreakdown/Classes/SqlExpressionClause.cs index e1bffdc..c747d40 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Classes/SqlExpressionClause.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Classes/SqlExpressionClause.cs @@ -85,7 +85,7 @@ public class SqlExpressionClause : SqlClause, ISqlExpressionClause /// /// The clause to split. /// An enumerable collection of individual items. - private static IEnumerable SplitOnComma(string clause) + private static List SplitOnComma(string clause) { var items = new List(); var current = new StringBuilder(); diff --git a/tests/Strata.SqlTools.EFCore.Tests/QueryBreakdownRepositoryTests.cs b/tests/Strata.SqlTools.EFCore.Tests/QueryBreakdownRepositoryTests.cs index 2641cde..eaad8d3 100644 --- a/tests/Strata.SqlTools.EFCore.Tests/QueryBreakdownRepositoryTests.cs +++ b/tests/Strata.SqlTools.EFCore.Tests/QueryBreakdownRepositoryTests.cs @@ -11,7 +11,7 @@ public class QueryBreakdownRepositoryTests { private DbContext _dbContext = null!; private IQueryBreakdownMapper _mapper = null!; - private IQueryBreakdownRepository _repository = null!; + private QueryBreakdownRepository _repository = null!; [SetUp] public void Setup() diff --git a/tests/Strata.SqlTools.Rules.Tests/UnitTest1.cs b/tests/Strata.SqlTools.Rules.Tests/UnitTest1.cs index 83f2a63..382ed2d 100644 --- a/tests/Strata.SqlTools.Rules.Tests/UnitTest1.cs +++ b/tests/Strata.SqlTools.Rules.Tests/UnitTest1.cs @@ -148,7 +148,7 @@ public class Tests return result; } - static System.Linq.Expressions.Expression StartsWith(System.Linq.Expressions.Expression expression, string value) + static MethodCallExpression StartsWith(System.Linq.Expressions.Expression expression, string value) { return StartsWith(expression, LinqExpression.Constant(value)); } -- 2.54.0 From c38d122d76d9bb7c0bf8268fb7c300e6cf1ea521 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 26 May 2026 15:50:25 -0500 Subject: [PATCH 7/7] docs(skill): rewrite sonarqube cleanup section around tiered bulk fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Section A (server inspection / API access) is unchanged. Section B is rewritten around the tiered model: - Tier 1 — mechanical: bulk fix multiple rule IDs in one dotnet format invocation (IDE0028, CA1825, CA1834, etc). - Tier 2 — judgment: one commit per rule, audit diff before staging. Calls out CA1822 public-static = binary break, CA1861 cryptic generated field names + duplicate-field collisions, CA1859 has no batch fixer. - Tier 3 — manual / no fixer: csharpsquid:Sxxxx, public-API design calls. Reference [[sonarqube-wontfix-rules]] memory for triage. Adds the local-Java-version gotcha: `./scan-sonar.ps1` is blocked here (system JRE is 8, scanner needs 17); CI handles the upload via actions/setup-java@v4 with temurin 17. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/sonarqube/SKILL.MD | 83 ++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 19 deletions(-) diff --git a/.claude/skills/sonarqube/SKILL.MD b/.claude/skills/sonarqube/SKILL.MD index 97ad648..8a66a17 100644 --- a/.claude/skills/sonarqube/SKILL.MD +++ b/.claude/skills/sonarqube/SKILL.MD @@ -38,29 +38,74 @@ Invoke-RestMethod -Uri "$url/api/issues/search?componentKeys=sql-utilities&resol `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 +## B — Cleanup loop (tiered) ```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 [label="Query open issues + facets=rules"]; + triage [label="Tier each rule:\nmechanical / judgment / API-impact"]; + t1 [label="Tier 1 — mechanical:\nbulk-fix in one commit"]; + t2 [label="Tier 2 — judgment:\none commit per rule, audit diff"]; + t3 [label="Tier 3 — API-impact:\nmanual, accept !breaking or skip"]; + build [label="dotnet build + dotnet test\nafter EACH commit"]; + push [label="git push + open PR"]; + ci [label="CI runs SonarScanner\non Java 17"]; + verify [label="Re-query API:\nconfirm rule counts dropped"]; - query -> pick -> edit -> build -> test -> commit -> more; - more -> pick [label="yes"]; - more -> scan [label="no"]; - scan -> verify; + query -> triage; + triage -> t1 -> build; + triage -> t2 -> build; + triage -> t3 -> build; + build -> push -> ci -> 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. +**The loop has shifted from per-rule commits to *tiered batches* once a project has more than a handful of issues left.** Use `dotnet format analyzers --diagnostics --severity info` — it runs all the Roslyn-shipped code fixers for the listed diagnostics, including `CA*`, `IDE*`, and `NUnit*`. (The `csharpsquid:Sxxxx` family from SonarAnalyzer.CSharp usually has no `dotnet format` fixer; those still need manual edits.) + +### Tier 1 — mechanical (one bulk commit) + +Rules where the fixer's rewrite is purely syntactic and can't change behavior — pile them into one `dotnet format` invocation, audit the diff for sanity, build, test, single commit: + +```powershell +dotnet format analyzers Strata.SqlTools.QueryBreakdown.sln ` + --diagnostics IDE0028 CA1825 CA1834 CA1845 CA1847 CA1860 CA1866 CA1853 CA1830 CA2249 ` + --severity info --verbosity normal +``` + +Typical safe rules: `IDE0028`, `CA1825`, `CA1834`, `CA1845`, `CA1847`, `CA1853`, `CA1860`, `CA1866`, `CA2249`, `CA1830`. Some rules in this family report "no associated code fix" — they'll need manual handling separately. + +### Tier 2 — judgment (one commit per rule) + +Rules whose fixer can produce mediocre output or surprise the reader — bulk-fix but **audit before staging**: + +- `CA1510` — `ArgumentNullException.ThrowIfNull` rollup; always safe but voluminous, deserves its own commit. +- `CA1822` — make-method-static; the fixer also rewrites internal callers to use type-name form. **Public methods becoming static are a binary-break for external NuGet consumers** — commit with `chore(sonar)!:` and a `BREAKING CHANGE:` footer naming each affected member, or revert those file diffs and apply only the private-helper changes. +- `CA1861` — hoists constant array args to `static readonly` fields. The fixer's field names are sometimes cryptic (`stringArray`, `arg2`) or it emits duplicate `separator` fields in the same class. After running, rename ugly fields and dedupe collisions before committing. +- `CA1854` — `TryGetValue`-style double-lookup elimination; trivial. +- `CA1859` — concrete return type for perf. **No batch fixer**: edit each site manually after checking visibility. Apply only to private/internal/test; on public/protected, either revert or treat as a `!breaking` change (most cases are private helpers, so this is usually fine). + +### Tier 3 — manual (no fixer) + +`CA1806`, `CA1846`, `CA1869`, the `csharpsquid:Sxxxx` family that has no `dotnet format` fixer, and individual public-API rules that need design judgment. Edit by hand, one rule at a time. If a fix would degrade clarity or break a tested contract, prefer **server-side Won't Fix with justification** — see [[sonarqube-wontfix-rules]] for the catalog already triaged that way (S1168, S3925, CS8601, S107). + +### Guardrail + +After every commit (Tier 1, 2, or 3): + +```powershell +dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo +dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo +``` + +Build succeeds with no new warnings beyond the baseline. Tests stay green. If either fails, fix or revert before continuing to the next tier — incremental builds can mask warning regressions, so re-run with `--no-incremental` if the warning count looks suspicious. + +### Scan & verify + +**Local `./scan-sonar.ps1` is blocked here** — the system JRE is 8 and the SonarScanner CLI requires Java 17 (`UnsupportedClassVersionError`). CI handles the scan: `.gitea/workflows/sonarqube.yml` runs `actions/setup-java@v4` with `temurin` 17 and uploads on every push and PR. So the path is **push → open PR → CI scans → re-query the API to confirm net-down**: + +```powershell +$h = @{ Authorization = "Basic $b64" } +Invoke-RestMethod -Uri "$url/api/issues/search?componentKeys=sql-utilities&resolved=false&facets=rules&ps=500" -Headers $h +``` + +The targeted rule IDs should each drop to 0 (or near-0 if the fixer left a few cases it couldn't batch-resolve). Commit subject style stays `chore(sonar): … (RuleId)` — bang (`!`) if the change is a deliberate breaking rename or visibility shift. -- 2.54.0