refactor: address S2365 collection-copying property getters

Resolves SonarQube S2365 x3.

CalculationFilterGroup (Snowflake + SqlServer): the `Filters` getter
ran .Where(...).ToList() on every access. Convert to a plain
auto-property holding the raw collection plus a `GetValidFilters()`
method that does the filtering. Update internal IsValid() and the two
QueryConfigExtensions callers to use the new method.

HierarchicalData.AllChildData: part of the IHierarchicalData interface
and JSON-serialized; the transformation IS the property's contract.
Suppress S2365 with justification rather than refactor -- same pattern
as S3875.

Behavior change: CalculationFilterGroup JSON output now serializes the
raw filter collection rather than the pre-filtered one. Round-trip is
preserved; callers needing the filtered view must call GetValidFilters().

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thom Lamb
2026-05-19 16:55:13 -05:00
co-authored by Claude Opus 4.7
parent f96161a664
commit 6f85358f63
5 changed files with 22 additions and 24 deletions
@@ -4,33 +4,31 @@ namespace Strata.SqlTools.Snowflake.ExpressionFactory.Query;
public class CalculationFilterGroup public class CalculationFilterGroup
{ {
[JsonIgnore]
private IEnumerable<CalculationFilter> _filters;
// Hereditary logical operation applied to all Filters // Hereditary logical operation applied to all Filters
public LogicalOperator LogicalOperator { get; set; } public LogicalOperator LogicalOperator { get; set; }
public IEnumerable<CalculationFilter> Filters public IEnumerable<CalculationFilter> Filters { get; set; }
{
get => _filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
set => _filters = value;
}
public CalculationFilterGroup() public CalculationFilterGroup()
{ {
LogicalOperator = LogicalOperator.And; LogicalOperator = LogicalOperator.And;
_filters = new List<CalculationFilter>(); Filters = new List<CalculationFilter>();
} }
[JsonConstructor] [JsonConstructor]
public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator) public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
{ {
_filters = filters; Filters = filters;
LogicalOperator = logicalOperator; LogicalOperator = logicalOperator;
} }
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
}
public bool IsValid() public bool IsValid()
{ {
return Filters != null && Filters.Any(); return GetValidFilters().Any();
} }
} }
@@ -11,7 +11,7 @@ public static class QueryConfigExtensions
public static int[] GetAllColumnIds(this QueryConfig queryConfig) public static int[] GetAllColumnIds(this QueryConfig queryConfig)
{ {
return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds) 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.Rows.Select(row => row.DataColumnId))
.Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId))) .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId)))
.ToArray(); .ToArray();
@@ -1,6 +1,7 @@
using Strata.SqlTools.SqlBreakdown.Interfaces.Core; using Strata.SqlTools.SqlBreakdown.Interfaces.Core;
using System.Collections; using System.Collections;
using System.Diagnostics.CodeAnalysis;
using System.Globalization; using System.Globalization;
using System.Text.Json; using System.Text.Json;
using System.Text.Json.Serialization; using System.Text.Json.Serialization;
@@ -373,6 +374,7 @@ public class HierarchicalData : IHierarchicalData
public IFlatData Data { get; set; } 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<IHierarchicalData> AllChildData public IEnumerable<IHierarchicalData> AllChildData
{ {
get => _childDataMap.SelectMany(x => x.Value).ToList(); get => _childDataMap.SelectMany(x => x.Value).ToList();
@@ -4,33 +4,31 @@ namespace Strata.SqlTools.SqlServer.ExpressionFactory.Query;
public class CalculationFilterGroup public class CalculationFilterGroup
{ {
[JsonIgnore]
private IEnumerable<CalculationFilter> _filters;
// Hereditary logical operation applied to all Filters // Hereditary logical operation applied to all Filters
public LogicalOperator LogicalOperator { get; set; } public LogicalOperator LogicalOperator { get; set; }
public IEnumerable<CalculationFilter> Filters public IEnumerable<CalculationFilter> Filters { get; set; }
{
get => _filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
set => _filters = value;
}
public CalculationFilterGroup() public CalculationFilterGroup()
{ {
LogicalOperator = LogicalOperator.And; LogicalOperator = LogicalOperator.And;
_filters = new List<CalculationFilter>(); Filters = new List<CalculationFilter>();
} }
[JsonConstructor] [JsonConstructor]
public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator) public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
{ {
_filters = filters; Filters = filters;
LogicalOperator = logicalOperator; LogicalOperator = logicalOperator;
} }
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
}
public bool IsValid() public bool IsValid()
{ {
return Filters != null && Filters.Any(); return GetValidFilters().Any();
} }
} }
@@ -11,7 +11,7 @@ public static class QueryConfigExtensions
public static int[] GetAllColumnIds(this QueryConfig queryConfig) public static int[] GetAllColumnIds(this QueryConfig queryConfig)
{ {
return queryConfig.Values.SelectMany(value => value.CalculationDataColumnIds) 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.Rows.Select(row => row.DataColumnId))
.Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId))) .Union(queryConfig.FilterGroups.SelectMany(filterGroup => filterGroup.Filters.Select(filter => filter.DataColumnId)))
.ToArray(); .ToArray();