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
{
[JsonIgnore]
private IEnumerable<CalculationFilter> _filters;
// Hereditary logical operation applied to all Filters
public LogicalOperator LogicalOperator { get; set; }
public IEnumerable<CalculationFilter> Filters
{
get => _filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
set => _filters = value;
}
public IEnumerable<CalculationFilter> Filters { get; set; }
public CalculationFilterGroup()
{
LogicalOperator = LogicalOperator.And;
_filters = new List<CalculationFilter>();
Filters = new List<CalculationFilter>();
}
[JsonConstructor]
public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
{
_filters = filters;
Filters = filters;
LogicalOperator = logicalOperator;
}
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
}
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)
{
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();
@@ -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<IHierarchicalData> AllChildData
{
get => _childDataMap.SelectMany(x => x.Value).ToList();
@@ -4,33 +4,31 @@ namespace Strata.SqlTools.SqlServer.ExpressionFactory.Query;
public class CalculationFilterGroup
{
[JsonIgnore]
private IEnumerable<CalculationFilter> _filters;
// Hereditary logical operation applied to all Filters
public LogicalOperator LogicalOperator { get; set; }
public IEnumerable<CalculationFilter> Filters
{
get => _filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
set => _filters = value;
}
public IEnumerable<CalculationFilter> Filters { get; set; }
public CalculationFilterGroup()
{
LogicalOperator = LogicalOperator.And;
_filters = new List<CalculationFilter>();
Filters = new List<CalculationFilter>();
}
[JsonConstructor]
public CalculationFilterGroup(IEnumerable<CalculationFilter> filters, LogicalOperator logicalOperator)
{
_filters = filters;
Filters = filters;
LogicalOperator = logicalOperator;
}
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
}
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)
{
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();