From 6f85358f63f4c744645d1d79bac7601963c24da6 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 19 May 2026 16:55:13 -0500 Subject: [PATCH] refactor: address S2365 collection-copying property getters Resolves SonarQube S2365 x3. CalculationFilterGroup (Snowflake + SqlServer): the `Filters` getter ran .Where(...).ToList() on every access. Convert to a plain auto-property holding the raw collection plus a `GetValidFilters()` method that does the filtering. Update internal IsValid() and the two QueryConfigExtensions callers to use the new method. HierarchicalData.AllChildData: part of the IHierarchicalData interface and JSON-serialized; the transformation IS the property's contract. Suppress S2365 with justification rather than refactor -- same pattern as S3875. Behavior change: CalculationFilterGroup JSON output now serializes the raw filter collection rather than the pre-filtered one. Round-trip is preserved; callers needing the filtered view must call GetValidFilters(). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Query/CalculationFilterGroup.cs | 20 +++++++++---------- .../Query/QueryConfigExtensions.cs | 2 +- .../Expressions/InputPropertyExpression.cs | 2 ++ .../Query/CalculationFilterGroup.cs | 20 +++++++++---------- .../Query/QueryConfigExtensions.cs | 2 +- 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs b/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs index fb48dc8..4580779 100644 --- a/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs +++ b/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/CalculationFilterGroup.cs @@ -4,33 +4,31 @@ namespace Strata.SqlTools.Snowflake.ExpressionFactory.Query; public class CalculationFilterGroup { - [JsonIgnore] - private IEnumerable _filters; - // Hereditary logical operation applied to all Filters public LogicalOperator LogicalOperator { get; set; } - public IEnumerable Filters - { - get => _filters?.Where(x => x.IsValid()).ToList() ?? new List(); - set => _filters = value; - } + public IEnumerable Filters { get; set; } public CalculationFilterGroup() { LogicalOperator = LogicalOperator.And; - _filters = new List(); + Filters = new List(); } [JsonConstructor] public CalculationFilterGroup(IEnumerable filters, LogicalOperator logicalOperator) { - _filters = filters; + Filters = filters; LogicalOperator = logicalOperator; } + public IEnumerable GetValidFilters() + { + return Filters?.Where(x => x.IsValid()).ToList() ?? new List(); + } + public bool IsValid() { - return Filters != null && Filters.Any(); + return GetValidFilters().Any(); } } diff --git a/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs b/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs index fd0c34c..88f72c2 100644 --- a/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs +++ b/src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/QueryConfigExtensions.cs @@ -11,7 +11,7 @@ public static class QueryConfigExtensions public static int[] GetAllColumnIds(this QueryConfig queryConfig) { return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds) - .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.Filters.Select(f => f.DataColumnId)))) + .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.GetValidFilters().Select(f => f.DataColumnId)))) .Union(queryConfig.Rows.Select(row => row.DataColumnId)) .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId))) .ToArray(); diff --git a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs index 70ae99e..f984759 100644 --- a/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs +++ b/src/Strata.SqlTools.SqlBreakdown/Expressions/InputPropertyExpression.cs @@ -1,6 +1,7 @@ using Strata.SqlTools.SqlBreakdown.Interfaces.Core; using System.Collections; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text.Json; using System.Text.Json.Serialization; @@ -373,6 +374,7 @@ public class HierarchicalData : IHierarchicalData public IFlatData Data { get; set; } + [SuppressMessage("Major Code Smell", "S2365:Properties should not make collection or array copies", Justification = "AllChildData is part of the IHierarchicalData interface contract and is JSON-serialized (see Data.json). The flatten-on-get / group-on-set transformation is the deliberate purpose of the property — _childDataMap is the storage form, the property is the wire form.")] public IEnumerable AllChildData { get => _childDataMap.SelectMany(x => x.Value).ToList(); diff --git a/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs b/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs index ca56ae4..5fb4e0d 100644 --- a/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs +++ b/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/CalculationFilterGroup.cs @@ -4,33 +4,31 @@ namespace Strata.SqlTools.SqlServer.ExpressionFactory.Query; public class CalculationFilterGroup { - [JsonIgnore] - private IEnumerable _filters; - // Hereditary logical operation applied to all Filters public LogicalOperator LogicalOperator { get; set; } - public IEnumerable Filters - { - get => _filters?.Where(x => x.IsValid()).ToList() ?? new List(); - set => _filters = value; - } + public IEnumerable Filters { get; set; } public CalculationFilterGroup() { LogicalOperator = LogicalOperator.And; - _filters = new List(); + Filters = new List(); } [JsonConstructor] public CalculationFilterGroup(IEnumerable filters, LogicalOperator logicalOperator) { - _filters = filters; + Filters = filters; LogicalOperator = logicalOperator; } + public IEnumerable GetValidFilters() + { + return Filters?.Where(x => x.IsValid()).ToList() ?? new List(); + } + public bool IsValid() { - return Filters != null && Filters.Any(); + return GetValidFilters().Any(); } } diff --git a/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs b/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs index 7cafd14..692f4a5 100644 --- a/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs +++ b/src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/QueryConfigExtensions.cs @@ -11,7 +11,7 @@ public static class QueryConfigExtensions public static int[] GetAllColumnIds(this QueryConfig queryConfig) { return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds) - .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.Filters.Select(f => f.DataColumnId)))) + .Union(queryConfig.Values.SelectMany(x => x.FilterGroups.SelectMany(y => y.GetValidFilters().Select(f => f.DataColumnId)))) .Union(queryConfig.Rows.Select(row => row.DataColumnId)) .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId))) .ToArray();