LinqExpressionVisitor._tableName was only assigned and read inside
VisitConstant immediately before assigning FromClause. Drop the field
entirely and assign FromClause directly from entityType.Name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rules/Rule/Groups/With.GetExpressions only called base.GetExpressions(). The
comment 'do some ordering here??' indicates the override is a TODO stub.
Drop the override and preserve the intent as an inline TODO on the class.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SqlServer/Breakdowns/QueryBreakdown.cs:26 — _clausesCacheDirty was both
initialized inline (= true) and re-assigned in the constructor at line 56.
Drop the inline initializer; the ctor remains authoritative.
The four nearby S3604 false-positives on clause backing fields stay
suppressed via #pragma — they are write-through targets of cache-invalidating
property setters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- WithClause.RecursiveQuery and WithClause.ColumnList had get/set bodies that
only forwarded to private backing fields. Convert both to auto-properties
and remove the now-orphaned _recursiveQuery / _columnList fields.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `[Serializable]` attribute and corresponding `[OnDeserialized]` methods have been removed from various breakdown classes. This eliminates reliance on `BinaryFormatter`, which is a deprecated and insecure serialization mechanism in modern .NET.
This change also resolves SonarQube rule S5766 warnings by removing the context in which they apply, leading to cleaner and more secure code.
Introduce a default regex match timeout across the library to prevent potential ReDoS attacks (SonarQube rule S6444).
Implement `[OnDeserialized]` methods to re-establish object invariants and validate state after deserialization, addressing SonarQube rule S5766.
Refactors the `CalculationFilterGroup` class to utilize C# primary constructors and property initializers, improving conciseness and readability.
The `IsValid()` method's logic is updated. Previously, it returned true if *any* filter in the group was valid. Now, it returns true only if *all* filters are valid, and an empty filter group is considered valid. This adjustment clarifies the group's validity criteria, aligning with common interpretations for `All` operations and addressing related technical debt.
Resolves SonarQube NUnit2045. The three independent parameter-index
assertions now report together instead of short-circuiting on the first
failure. The Is.Not.Null guard in the reflection helper stays outside the
multiple block because it gates the subsequent Invoke.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves the byte-identical 19-file ExpressionFactory/Query tree (duplicated
across Snowflake and SqlServer) into the new Strata.SqlTools.Query project
under the flat namespace Strata.SqlTools.Query. Both dialect projects now
reference the shared project; the Snowflake copies are deleted.
Also folds in the IDE0028 fix on CalculationFilterGroup.GetValidFilters
(collection expression []), which resolves both new-code IDE0028 smells
in one place now that there is a single copy.
Eliminates the 63 new duplicate lines flagged on the PR and removes the
largest contributor to the project's 11.1% duplication density. No
behavioral change: the moved types are identical to the originals.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First step of extracting the duplicated ExpressionFactory/Query model
tree. Adds a BCL-only class library and registers it in the solution;
files are moved in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task-by-task plan to scaffold the shared project, move the 19 identical
ExpressionFactory/Query files, rewire dialect references, and clear the 3
remaining new-code SonarQube smells (IDE0028 x2, NUnit2045).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Designs the extraction of the byte-identical 19-file ExpressionFactory/Query
tree (duplicated across Snowflake + SqlServer) into a new Strata.SqlTools.Query
project under a clean-break namespace, and folds in the 3 remaining new-code
SonarQube smells (IDE0028 x2 on CalculationFilterGroup, NUnit2045 on
CommandVisitorTests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Establishes tooling to systematically analyze and address technical debt.
This includes:
- `scan-sonar.ps1`: An orchestration script for local SonarQube scans with coverage.
- `Directory.Build.props`: Integrates SonarAnalyzer.CSharp for static analysis during build.
- `coverlet.runsettings`: Configures code coverage collection using Coverlet.
- `.claude/settings.local.json`: Adds permissions for AI to query SonarQube and local dev status.
Code review follow-up on 6f85358:
- CalculationFilterGroup.IsValid() previously called GetValidFilters().Any(),
which materialized a full filtered List<CalculationFilter> just to check
existence. Reimplement directly as Filters.Any(x => x.IsValid()) so
IsValid() recovers its pre-refactor O(1) early-exit behavior without
allocations. GetValidFilters() remains for callers that need the full
materialized list.
- QueryConfigExtensions.GetAllColumnIds line 16 uses FilterGroup (not
CalculationFilterGroup), which pre-filters in its JsonConstructor and
has no GetValidFilters() method. Add a comment to document the
intentional asymmetry between line 14 and line 16.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
Code review follow-up on acfe29e:
- Switch namespace to Strata.SqlTools.SqlBreakdown.Tests.PostgreSql to
match the dominant convention in the sibling test folder.
- Remove the unreachable SqlServer.CommandVisitor fallback in the
reflection helper. The Assert.That guard alone is enough to surface
a future regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves SonarQube S2696 in CommandVisitor.cs.
The static _parameterIndex field was mutated from an instance method,
causing every new CommandVisitor to inherit the previous instance's
counter and never reset. Parameter indices now restart at $1 per
visitor, which is the intended PostgreSQL behavior.
Adds CommandVisitorTests.TwoVisitors_HaveIndependentParameterIndices to
lock in the contract via reflection (FormatParameterName is protected).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves SonarQube S3875 (BLOCKER) in Expression.Operators.cs.
The operator== returns a Comparison expression (DSL semantics), not a
bool. The existing CS0660/CS0661 pragma already documents this design;
the new SuppressMessage attribute makes the Sonar analyzer agree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves SonarQube S4487 in LinqQueryBreakdownBuilder.cs.
The field was assigned in the constructor but never read. Grep across
the project confirmed no external references.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves SonarQube S927 x2 in ExpressionVisitor.cs.
The interface parameter was named ``Equal`` on a ``VisitNotEquals`` method,
which is semantically wrong and forced implementations to mismatch.
Implementations already used ``notEqual``; rename the interface to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five-task plan implementing the 2026-05-19 spec: per-rule commits on the
existing fix/Sonarqube-Tech-Debt branch with build+test verification
between each. Notes one spec deviation discovered during plan-time
exploration (HierarchicalData.AllChildData is suppressed rather than
refactored — it's an interface contract member tied to JSON shape).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design doc covering the 8 BLOCKER/CRITICAL non-S3776 issues across six
projects. Approach is a pragmatic mix: fix where the rule reflects a
real defect, suppress with justification where the rule conflicts with
deliberate design (DSL operator==).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bermudalamb