From c5dee3a73835b50f874fac99568da8426045e554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Mon, 17 Aug 2026 12:59:35 +0200 Subject: [PATCH 01/20] fix: Except answered the whole domain for the one pair no arm covered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RangeSet.Infinite.Except(Int32Range.Infinite) returned {(,)} where X \ (-∞,+∞) is the empty set for every X, and Complement() on the infinite set was wrong through the same path. Except(TRange) guarded an empty operand and not an infinite one, so ∞ \ ∞ reached ExceptEngine as a shape pair no arm covered and the fallback answered ∞. The set-minus-set overload has always guarded its infinite operand and the single-range overload answers it through its Contains guard, so the three overloads were answering one question two ways — the same disagreement the empty-range containment bug had in 7.0.0. Fourth instance of the trap, which is the argument for the rest of this commit. Documenting the rule in CLAUDE.md after the first instance (6.2.1) did not stop the second, third or fourth: a rule in a doc is a reminder, not a constraint. The three engines now decide on the shape pair. Each had three entry points typed by the receiver's shape, each switching over the operand's shape with a discard that rebuilt the receiver or returned Empty — the structure all four bugs shared, and the reason a missing pair was invisible. They are now one entry point per engine taking IRange on both sides and switching over (left, right) with one arm per accepted pair, so an unhandled pair is a missing *line*. The receiver switches at the call sites — RangeExtensions.Except, RangeSet.SubtractOne, ComplementOfSingle — go with them, along with ExceptEngine.InfinityExcept, whose receiver was Infinity by construction. C# cannot prove a switch over interface patterns exhaustive, so the discard arm cannot be removed. What it can be is fatal: every one of the four bugs was a fallback returning something well-formed — a wrong boolean, or a range of the right shape carrying the wrong values, which looks correct in a debugger and disagrees only with the database. The discards now throw ShapePair.Unreachable naming the pair. RangeBoundHelpers.RecreateAs gets the same treatment; its IEmptyRange arm was being served by the fallback and is now written out. EngineDispatchConventionTests parses the shipping sources and enforces both halves: a switch dispatching on range shape must throw from its discard, and an engine entry point must take IRange on both sides. RangeSetHelpers already complied, so the rule needed no exemption list. Both were verified by seeding the defect they claim to catch — a discard returning TRange.Empty, and an added IFiniteRange-typed overload — and each names the file, line and offending expression. Verified by reverting: stripping the Except(TRange) guard fails both new tests, now with "ExceptEngine has no arm for the shape pair (Infinity, Infinity)" instead of a well-formed wrong answer. Full suite green including the 73 live PostgreSQL tests, so the refactor is behaviour-preserving everywhere the shape matrix reaches. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 3 +- .../Internals/ExceptEngine.cs | 91 +++++------ .../Internals/IntersectEngine.cs | 43 ++--- .../Internals/MergeEngine.cs | 44 +++--- .../Internals/RangeBoundHelpers.cs | 7 +- .../Internals/ShapePair.cs | 39 +++++ src/CodoMetis.ValueRanges/RangeExtensions.cs | 10 +- src/CodoMetis.ValueRanges/RangeSet.cs | 24 +-- ...Metis.ValueRanges.Conventions.Tests.csproj | 1 + .../EngineDispatchConventionTests.cs | 148 ++++++++++++++++++ .../RangeExceptTests.cs | 33 ++++ 11 files changed, 313 insertions(+), 130 deletions(-) create mode 100644 src/CodoMetis.ValueRanges/Internals/ShapePair.cs create mode 100644 test/CodoMetis.ValueRanges.Conventions.Tests/EngineDispatchConventionTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 4cc6d5d..8ff346a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,8 @@ repo-wide. - **NEVER** create external subtypes of range base records — the private constructor enforces exhaustive pattern matching; breaking this removes compiler guarantees - **ALWAYS** preserve RangeSet's invariant (sorted, disjoint, non-adjacent, no empties) on every code path that constructs or mutates a set - **Do NOT** add new range types without verifying the generic engines cover them — a new type must implement `IRange` + `IRangeFactory` with the five sealed variants; the engines in `Internals/` dispatch per shape through the structural interfaces -- **Ranges — NEVER decide a binary predicate by switching on the receiver's shape.** Read the bounds the relation actually compares and decide on those. Switching on the receiver and handling the operand's shapes in an inner switch has now produced the same bug three times — `IsAdjacentTo` (fixed 6.2.1), `IsStrictlyLeftOf` and `Except` (both fixed 7.0.0). The tell is an inner switch with a `_` fallback: it answers `false`, or returns the receiver unchanged, for exactly the operand shapes nobody wrote an arm for. **This applies to value-producing operations, not just predicates** — `Except` returned a well-formed range holding the wrong values, which is the harder one to notice. It hides well because the EF translation is correct, so the disagreement is between memory and the database rather than inside either. `ShapeMatrixParityTests` asks PostgreSQL for every ordered shape pair and is the check that catches it +- **Ranges — NEVER decide a binary operation by switching on the receiver's shape.** A binary relation is a function of the *pair* of shapes: read the bounds it actually compares, or switch on `(left, right)`. Switching on the receiver and handling the operand's shapes in an inner switch has now produced the same bug four times — `IsAdjacentTo` (fixed 6.2.1), `IsStrictlyLeftOf` and `Except` (both 7.0.0), and `RangeSet.Except(TRange)` with an infinity operand (7.0.1). The tell is an inner switch with a `_` fallback: it answers `false`, or returns the receiver unchanged, for exactly the operand shapes nobody wrote an arm for. **This applies to value-producing operations, not just predicates** — `Except` returned a well-formed range holding the wrong values, which is the harder one to notice. It hides well because the EF translation is correct, so the disagreement is between memory and the database rather than inside either. `ShapeMatrixParityTests` asks PostgreSQL for every ordered shape pair and is the check that catches it +- **Ranges — a shape dispatch's `_` arm MUST throw, never produce a value.** C# cannot prove a switch over interface patterns exhaustive, so the discard arm cannot be removed — the point is that it stops answering. Since 7.0.1 the three engines have one entry point each taking `IRange` on both sides, switch over `(left, right)` with one arm per accepted pair, and throw `ShapePair.Unreachable` naming the pair for the rest, so an unhandled pair is a missing *line* and a loud failure rather than a plausible value. `EngineDispatchConventionTests` parses `src/**/Internals/` and enforces both halves — throwing discards, and entry points that are not typed by one operand's shape - **EF Core**: new range types are wired exclusively through `RangeTypeRegistry.Register` (satellites call it from their options-builder extension); never bypass the registry - **EF Core — a constant element operand must be typed.** The range operators are polymorphic (`anyrange @> anyelement`) and PostgreSQL resolves those without implicit coercions, so a bare numeric literal (which it types as `integer`) does not match `int8range`. Assert the whole operand in translation tests, never the `@> ` prefix, and execute at least one constant-operand query per element type against a live server - **Value sets — ALWAYS** preserve canonical form (deduplicated, sorted by the family's canonical comparer, no nulls) on every construction path, including reads. String-backed families sort **ordinal** — never culture, never the element's own `IComparable` diff --git a/src/CodoMetis.ValueRanges/Internals/ExceptEngine.cs b/src/CodoMetis.ValueRanges/Internals/ExceptEngine.cs index 196e5fe..099fabb 100644 --- a/src/CodoMetis.ValueRanges/Internals/ExceptEngine.cs +++ b/src/CodoMetis.ValueRanges/Internals/ExceptEngine.cs @@ -6,65 +6,44 @@ namespace CodoMetis.ValueRanges.Internals; internal static class ExceptEngine { - // Called when range is IInfinityRange — removes a bounded region from the entire domain. - internal static (TRange Left, TRange? Right) InfinityExcept(IRange other) + // Subtraction is a function of the pair of shapes, so the dispatch is one switch over the + // pair. Callers guarantee both operands are non-empty and the operand is not Infinity + // (Except filters an empty operand through its Overlaps guard and a containing one through + // its Contains guard), which is why those pairs have no arm and throw rather than falling + // back — see ShapePair. + internal static (TRange Left, TRange? Right) Execute(IRange left, IRange right) where TRange : IRangeFactory, IRange where T : struct, IComparable, IEquatable - => other switch + => (left, right) switch { - IFiniteRange o => (TRange.CreateUnboundedStart(o.Start, !o.StartInclusive), - (TRange?)TRange.CreateUnboundedEnd(o.End, !o.EndInclusive)), - IUnboundedStartRange s => (TRange.CreateUnboundedEnd(s.End, !s.EndInclusive), default), - IUnboundedEndRange e => (TRange.CreateUnboundedStart(e.Start, !e.StartInclusive), default), - _ => (TRange.Infinite, default) - }; - - internal static (TRange Left, TRange? Right) Execute(IFiniteRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IFiniteRange o => FiniteExceptFinite(left, o), - IUnboundedStartRange s => (TRange.CreateFinite(s.End, left.End, !s.EndInclusive, left.EndInclusive), default), - IUnboundedEndRange e => (TRange.CreateFinite(left.Start, e.Start, left.StartInclusive, !e.StartInclusive), default), - _ => (TRange.CreateFinite(left.Start, left.End, left.StartInclusive, left.EndInclusive), default) - }; - - internal static (TRange Left, TRange? Right) Execute(IUnboundedStartRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IFiniteRange o => OpenStartExceptFinite(left, o), - IUnboundedStartRange o => (TRange.CreateFinite(o.End, left.End, !o.EndInclusive, left.EndInclusive), default), - - // (-∞, left.End] minus [e.Start, +∞): the operand runs to +∞, so it removes - // everything from its own start upwards and what survives is (-∞, e.Start). - // Callers guarantee the two overlap, so e.Start is at or below left.End and this - // bound is the binding one. - IUnboundedEndRange e => (TRange.CreateUnboundedStart(e.Start, !e.StartInclusive), default), - - // Unreachable: RangeExtensions.Except and the RangeSet merge-join both short-circuit - // an empty operand (no overlap) and an infinity operand (the operand contains the - // receiver) before dispatching here. Returning the receiver unchanged is the safe - // identity for those two, and was silently the answer for the arm above until 7.0.0. - _ => (TRange.CreateUnboundedStart(left.End, left.EndInclusive), default) - }; - - internal static (TRange Left, TRange? Right) Execute(IUnboundedEndRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IFiniteRange o => OpenEndExceptFinite(left, o), - IUnboundedEndRange o => (TRange.CreateFinite(left.Start, o.Start, left.StartInclusive, !o.StartInclusive), default), - - // The mirror of the case above: [left.Start, +∞) minus (-∞, s.End] leaves - // (s.End, +∞). - IUnboundedStartRange s => (TRange.CreateUnboundedEnd(s.End, !s.EndInclusive), default), - - // Unreachable, as above. - _ => (TRange.CreateUnboundedEnd(left.Start, left.StartInclusive), default) + // Infinity receiver — removes a bounded region from the entire domain. + (IInfinityRange, IFiniteRange o) => (TRange.CreateUnboundedStart(o.Start, !o.StartInclusive), + (TRange?)TRange.CreateUnboundedEnd(o.End, !o.EndInclusive)), + (IInfinityRange, IUnboundedStartRange o) => (TRange.CreateUnboundedEnd(o.End, !o.EndInclusive), default), + (IInfinityRange, IUnboundedEndRange o) => (TRange.CreateUnboundedStart(o.Start, !o.StartInclusive), default), + + // Finite receiver. + (IFiniteRange l, IFiniteRange o) => FiniteExceptFinite(l, o), + (IFiniteRange l, IUnboundedStartRange o) => (TRange.CreateFinite(o.End, l.End, !o.EndInclusive, l.EndInclusive), default), + (IFiniteRange l, IUnboundedEndRange o) => (TRange.CreateFinite(l.Start, o.Start, l.StartInclusive, !o.StartInclusive), default), + + // UnboundedStart receiver. + (IUnboundedStartRange l, IFiniteRange o) => OpenStartExceptFinite(l, o), + (IUnboundedStartRange l, IUnboundedStartRange o) => (TRange.CreateFinite(o.End, l.End, !o.EndInclusive, l.EndInclusive), default), + + // (-∞, l.End] minus [o.Start, +∞): the operand runs to +∞, so it removes everything + // from its own start upwards and what survives is (-∞, o.Start). Callers guarantee + // the two overlap, so o.Start is at or below l.End and this bound is the binding one. + (IUnboundedStartRange _, IUnboundedEndRange o) => (TRange.CreateUnboundedStart(o.Start, !o.StartInclusive), default), + + // UnboundedEnd receiver. + (IUnboundedEndRange l, IFiniteRange o) => OpenEndExceptFinite(l, o), + (IUnboundedEndRange l, IUnboundedEndRange o) => (TRange.CreateFinite(l.Start, o.Start, l.StartInclusive, !o.StartInclusive), default), + + // The mirror of the case above: [l.Start, +∞) minus (-∞, o.End] leaves (o.End, +∞). + (IUnboundedEndRange _, IUnboundedStartRange o) => (TRange.CreateUnboundedEnd(o.End, !o.EndInclusive), default), + + _ => throw ShapePair.Unreachable(nameof(ExceptEngine), left, right) }; // Three cases: o sits strictly inside b (split), o covers b's start (left-trim), o covers b's end (right-trim). diff --git a/src/CodoMetis.ValueRanges/Internals/IntersectEngine.cs b/src/CodoMetis.ValueRanges/Internals/IntersectEngine.cs index 67df7cd..de90ffd 100644 --- a/src/CodoMetis.ValueRanges/Internals/IntersectEngine.cs +++ b/src/CodoMetis.ValueRanges/Internals/IntersectEngine.cs @@ -6,37 +6,28 @@ namespace CodoMetis.ValueRanges.Internals; internal static class IntersectEngine { - internal static TRange Execute(IFiniteRange left, IRange right) + // One switch over the shape pair. Callers guarantee the operands overlap (so neither is + // Empty) and that an Infinity operand was answered before dispatch — RangeExtensions.Intersect + // returns the other side for it, and IInfinityRange.IntersectWith re-expresses. Those pairs + // therefore have no arm and throw rather than falling back to Empty; see ShapePair. + internal static TRange Execute(IRange left, IRange right) where TRange : IRangeFactory, IRange where T : struct, IComparable, IEquatable - => right switch + => (left, right) switch { - IFiniteRange o => FiniteWithFinite(left, o), - IUnboundedStartRange s => FiniteWithOpenStart(left, s), - IUnboundedEndRange e => FiniteWithOpenEnd(left, e), - _ => TRange.Empty - }; + (IFiniteRange l, IFiniteRange o) => FiniteWithFinite(l, o), + (IFiniteRange l, IUnboundedStartRange o) => FiniteWithOpenStart(l, o), + (IFiniteRange l, IUnboundedEndRange o) => FiniteWithOpenEnd(l, o), - internal static TRange Execute(IUnboundedStartRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IUnboundedStartRange o => OpenStartWithOpenStart(left, o), - IFiniteRange f => FiniteWithOpenStart(f, left), - IUnboundedEndRange e => OpenStartWithOpenEnd(left, e), - _ => TRange.Empty - }; + (IUnboundedStartRange l, IFiniteRange o) => FiniteWithOpenStart(o, l), + (IUnboundedStartRange l, IUnboundedStartRange o) => OpenStartWithOpenStart(l, o), + (IUnboundedStartRange l, IUnboundedEndRange o) => OpenStartWithOpenEnd(l, o), - internal static TRange Execute(IUnboundedEndRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IUnboundedEndRange o => OpenEndWithOpenEnd(left, o), - IFiniteRange f => FiniteWithOpenEnd(f, left), - IUnboundedStartRange s => OpenStartWithOpenEnd(s, left), - _ => TRange.Empty + (IUnboundedEndRange l, IFiniteRange o) => FiniteWithOpenEnd(o, l), + (IUnboundedEndRange l, IUnboundedStartRange o) => OpenStartWithOpenEnd(o, l), + (IUnboundedEndRange l, IUnboundedEndRange o) => OpenEndWithOpenEnd(l, o), + + _ => throw ShapePair.Unreachable(nameof(IntersectEngine), left, right) }; private static TRange FiniteWithFinite(IFiniteRange b, IFiniteRange o) diff --git a/src/CodoMetis.ValueRanges/Internals/MergeEngine.cs b/src/CodoMetis.ValueRanges/Internals/MergeEngine.cs index de92745..9e574db 100644 --- a/src/CodoMetis.ValueRanges/Internals/MergeEngine.cs +++ b/src/CodoMetis.ValueRanges/Internals/MergeEngine.cs @@ -6,37 +6,29 @@ namespace CodoMetis.ValueRanges.Internals; internal static class MergeEngine { - internal static TRange Execute(IFiniteRange left, IRange right) + // One switch over the shape pair. The only callers are RangeSet's greedy merges, which reach + // this behind `current.Overlaps(next) || current.IsAdjacentTo(next)` — both false for an empty + // operand — and behind Normalize, which collapses any Infinity input to the Infinite singleton + // before an element ever reaches here. Those pairs therefore have no arm and throw rather than + // falling back to Empty, which would have been the wrong answer for both; see ShapePair. + internal static TRange Execute(IRange left, IRange right) where TRange : IRangeFactory, IRange where T : struct, IComparable, IEquatable - => right switch + => (left, right) switch { - IFiniteRange o => FiniteWithFinite(left, o), - IUnboundedStartRange s => OpenStartWithFinite(s, left), - IUnboundedEndRange e => OpenEndWithFinite(e, left), - _ => TRange.Empty - }; + (IFiniteRange l, IFiniteRange o) => FiniteWithFinite(l, o), + (IFiniteRange l, IUnboundedStartRange o) => OpenStartWithFinite(o, l), + (IFiniteRange l, IUnboundedEndRange o) => OpenEndWithFinite(o, l), - internal static TRange Execute(IUnboundedStartRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IFiniteRange f => OpenStartWithFinite(left, f), - IUnboundedStartRange o => OpenStartWithOpenStart(left, o), - IUnboundedEndRange => TRange.Infinite, - _ => TRange.Empty - }; + (IUnboundedStartRange l, IFiniteRange o) => OpenStartWithFinite(l, o), + (IUnboundedStartRange l, IUnboundedStartRange o) => OpenStartWithOpenStart(l, o), + (IUnboundedStartRange, IUnboundedEndRange) => TRange.Infinite, - internal static TRange Execute(IUnboundedEndRange left, IRange right) - where TRange : IRangeFactory, IRange - where T : struct, IComparable, IEquatable - => right switch - { - IFiniteRange f => OpenEndWithFinite(left, f), - IUnboundedStartRange => TRange.Infinite, - IUnboundedEndRange o => OpenEndWithOpenEnd(left, o), - _ => TRange.Empty + (IUnboundedEndRange l, IFiniteRange o) => OpenEndWithFinite(l, o), + (IUnboundedEndRange, IUnboundedStartRange) => TRange.Infinite, + (IUnboundedEndRange l, IUnboundedEndRange o) => OpenEndWithOpenEnd(l, o), + + _ => throw ShapePair.Unreachable(nameof(MergeEngine), left, right) }; private static TRange FiniteWithFinite(IFiniteRange b, IFiniteRange o) diff --git a/src/CodoMetis.ValueRanges/Internals/RangeBoundHelpers.cs b/src/CodoMetis.ValueRanges/Internals/RangeBoundHelpers.cs index 5e765ef..737c8cf 100644 --- a/src/CodoMetis.ValueRanges/Internals/RangeBoundHelpers.cs +++ b/src/CodoMetis.ValueRanges/Internals/RangeBoundHelpers.cs @@ -103,10 +103,15 @@ internal static TRange RecreateAs(IRange source) where T : struct, IComparable, IEquatable => source switch { + IEmptyRange => TRange.Empty, IInfinityRange => TRange.Infinite, IFiniteRange f => TRange.CreateFinite(f.Start, f.End, f.StartInclusive, f.EndInclusive), IUnboundedStartRange s => TRange.CreateUnboundedStart(s.End, s.EndInclusive), IUnboundedEndRange e => TRange.CreateUnboundedEnd(e.Start, e.StartInclusive), - _ => TRange.Empty + + // Every shape is named above, so this is only reachable through an external + // implementation of IRange — which the sealed-variant rule forbids. Empty was the + // fallback until 7.0.1 and silently swallowed such a range. + _ => throw ShapePair.Unreachable(nameof(RecreateAs), source, source) }; } \ No newline at end of file diff --git a/src/CodoMetis.ValueRanges/Internals/ShapePair.cs b/src/CodoMetis.ValueRanges/Internals/ShapePair.cs new file mode 100644 index 0000000..9ad9ed8 --- /dev/null +++ b/src/CodoMetis.ValueRanges/Internals/ShapePair.cs @@ -0,0 +1,39 @@ +using System.Diagnostics; +using CodoMetis.ValueRanges.Core; + +namespace CodoMetis.ValueRanges.Internals; + +/// Diagnostics for the engines' shape dispatch. +/// +/// A binary range operation is a function of the *pair* of shapes, so the engines switch on +/// `(left, right)` with one arm per pair they accept. C# cannot prove a switch over interface +/// patterns exhaustive, so an arm for the discard pattern is mandatory — but that arm must +/// never produce a value. Returning something plausible from it is what shipped four bugs: +/// IsAdjacentTo (6.2.1), IsStrictlyLeftOf and Except (7.0.0), and the Infinity-operand +/// subtraction below it. In each case the arm nobody wrote was answered by a fallback that +/// looked right in a debugger and disagreed with PostgreSQL. +/// +/// The engines therefore throw from the discard, naming the pair. `EngineDispatchConventionTests` +/// keeps it that way. +internal static class ShapePair +{ + internal static UnreachableException Unreachable(string engine, IRange left, IRange right) + where T : struct, IComparable, IEquatable => + new($"{engine} has no arm for the shape pair ({Shape(left)}, {Shape(right)}). " + + "Either the caller's guards no longer hold, or the pair is genuinely reachable and " + + "needs its own arm — never a fallback."); + + // Deliberately an if-chain rather than a switch expression: this is the one place that has to + // cope with a shape it does not recognise, and a switch would need a value-returning discard + // of exactly the kind the engines are not allowed to have. + private static string Shape(IRange range) + where T : struct, IComparable, IEquatable + { + if (range is IEmptyRange) return "Empty"; + if (range is IInfinityRange) return "Infinity"; + if (range is IFiniteRange) return "Finite"; + if (range is IUnboundedStartRange) return "UnboundedStart"; + if (range is IUnboundedEndRange) return "UnboundedEnd"; + return range.GetType().Name; + } +} diff --git a/src/CodoMetis.ValueRanges/RangeExtensions.cs b/src/CodoMetis.ValueRanges/RangeExtensions.cs index 6470bc3..6bba451 100644 --- a/src/CodoMetis.ValueRanges/RangeExtensions.cs +++ b/src/CodoMetis.ValueRanges/RangeExtensions.cs @@ -643,15 +643,7 @@ public RangeSet Except(IRange other) { if (!range.Overlaps(other)) return RangeSet.From([range]); if (other.Contains(range)) return RangeSet.Empty; - var (left, right) = range is IInfinityRange - ? ExceptEngine.InfinityExcept(other) - : range switch - { - IFiniteRange b => ExceptEngine.Execute(b, other), - IUnboundedStartRange s => ExceptEngine.Execute(s, other), - IUnboundedEndRange e => ExceptEngine.Execute(e, other), - _ => (range, default) - }; + var (left, right) = ExceptEngine.Execute(range, other); return right is null ? RangeSet.From([left]) : RangeSet.From([left, right]); diff --git a/src/CodoMetis.ValueRanges/RangeSet.cs b/src/CodoMetis.ValueRanges/RangeSet.cs index ece9f85..d52237e 100644 --- a/src/CodoMetis.ValueRanges/RangeSet.cs +++ b/src/CodoMetis.ValueRanges/RangeSet.cs @@ -780,6 +780,13 @@ public RangeSet Intersect(RangeSet other) public RangeSet Except(TRange other) { if (other is IEmptyRange) return this; + + // X \ (-∞, +∞) is the empty set for every X. Without this the infinite-set branch below + // asked the engine to shape ∞ \ ∞, which no arm covers: until 7.0.1 the fallback there + // answered ∞, so RangeSet.Infinite.Except(TRange.Infinite) returned the whole domain. + // The RangeSet overload has always guarded its own infinite operand this way. + if (other is IInfinityRange) return Empty; + if (IsInfiniteSet) return ComplementOfSingle(other); var results = new List(); @@ -872,26 +879,21 @@ private RangeSet MergeExcept(RangeSet other) return From(results); } - // Dispatches a single subtraction (current \ o) to ExceptEngine by current's shape. + // Dispatches a single subtraction (current \ o) to ExceptEngine. // Callers guarantee `current.Overlaps(o)` and `!o.Contains(current)`, so the engine // always returns a non-empty Left and an optional non-empty Right (one-piece trim or // two-piece split). `current` is never IInfinityRange here (the Infinite case is // handled by ComplementOfSet/ComplementOfSingle before this is reached). private static (TRange Left, TRange? Right) SubtractOne(TRange current, IRange o) - => current switch - { - IFiniteRange b => ExceptEngine.Execute(b, o), - IUnboundedStartRange s => ExceptEngine.Execute(s, o), - IUnboundedEndRange e => ExceptEngine.Execute(e, o), - _ => (current, default) - }; + => ExceptEngine.Execute(current, o); // Complement of a single range within the entire domain: (-∞, +∞) \ r. - // Delegates to the existing ExceptEngine.InfinityExcept, which already returns the - // correct one- or two-piece result for every shape of r. + // Callers guarantee r is neither empty nor Infinity — Except(TRange) short-circuits both + // before reaching here, since ∞ \ ∅ is ∞ and ∞ \ ∞ is the empty set, and neither is a + // subtraction the engine should be asked to shape. private RangeSet ComplementOfSingle(TRange r) { - var (left, right) = ExceptEngine.InfinityExcept(r); + var (left, right) = ExceptEngine.Execute(TRange.Infinite, r); return right is null ? RangeSet.From([left]) : RangeSet.From([left, right]); diff --git a/test/CodoMetis.ValueRanges.Conventions.Tests/CodoMetis.ValueRanges.Conventions.Tests.csproj b/test/CodoMetis.ValueRanges.Conventions.Tests/CodoMetis.ValueRanges.Conventions.Tests.csproj index 5bf0d2e..78a68b7 100644 --- a/test/CodoMetis.ValueRanges.Conventions.Tests/CodoMetis.ValueRanges.Conventions.Tests.csproj +++ b/test/CodoMetis.ValueRanges.Conventions.Tests/CodoMetis.ValueRanges.Conventions.Tests.csproj @@ -9,6 +9,7 @@ + diff --git a/test/CodoMetis.ValueRanges.Conventions.Tests/EngineDispatchConventionTests.cs b/test/CodoMetis.ValueRanges.Conventions.Tests/EngineDispatchConventionTests.cs new file mode 100644 index 0000000..13b753c --- /dev/null +++ b/test/CodoMetis.ValueRanges.Conventions.Tests/EngineDispatchConventionTests.cs @@ -0,0 +1,148 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace CodoMetis.ValueRanges.Conventions.Tests; + +/// +/// Guards the two halves of the rule that four bugs came from: a binary range operation is a +/// function of the *pair* of shapes, and a shape the author did not think about must never be +/// answered with a plausible value. +/// +/// +/// +/// IsAdjacentTo (6.2.1), IsStrictlyLeftOf and Except (7.0.0) and the Infinity-operand subtraction +/// (7.0.1) were all the same defect: dispatch switched on the receiver's shape, the inner switch +/// covered some operand shapes, and the discard arm returned something well-formed for the rest. +/// Each was silently wrong in memory while the EF translation stayed correct, so the disagreement +/// was between the two sides of the wire rather than inside either. +/// +/// +/// C# cannot prove a switch over interface patterns exhaustive, so the discard arm itself cannot +/// be removed. What it can be is fatal: a missing pair then names itself at the first test that +/// reaches it instead of returning a value that looks right in a debugger. +/// +/// +/// Everything is discovered — engines by globbing src/, switches by parsing — so adding an +/// engine or an operation needs no edit here. +/// +/// +[TestClass] +public class EngineDispatchConventionTests +{ + private static readonly string[] ShapeInterfaces = + [ + "IFiniteRange", "IUnboundedStartRange", "IUnboundedEndRange", "IInfinityRange", "IEmptyRange" + ]; + + /// + /// A switch that dispatches on range shape must throw from its discard arm rather than + /// produce a value. Applies to every Internals/ file in every shipping project; + /// switches over anything else (a pair of bools, a comparison result) are untouched. + /// + [TestMethod] + public void ShapeDispatchDiscardArmsThrow() + { + var violations = new List(); + int inspected = 0; + + foreach (var file in InternalsFiles()) + { + var root = CSharpSyntaxTree.ParseText(File.ReadAllText(file.FullName)).GetRoot(); + + foreach (var expression in root.DescendantNodes().OfType()) + { + if (!expression.Arms.Any(arm => DispatchesOnShape(arm.Pattern))) continue; + + inspected++; + foreach (var arm in expression.Arms.Where(arm => arm.Pattern is DiscardPatternSyntax) + .Where(arm => arm.Expression is not ThrowExpressionSyntax)) + { + violations.Add($"{Relative(file)}:{LineOf(arm)} — discard arm returns " + + $"`{arm.Expression}` instead of throwing"); + } + } + + foreach (var statement in root.DescendantNodes().OfType()) + { + if (!statement.Sections.SelectMany(section => section.Labels) + .OfType() + .Any(label => DispatchesOnShape(label.Pattern))) continue; + + inspected++; + foreach (var section in statement.Sections + .Where(section => section.Labels.Any(label => label is DefaultSwitchLabelSyntax)) + .Where(section => !section.Statements.Any(ThrowsOutright))) + { + violations.Add($"{Relative(file)}:{LineOf(section)} — default section returns instead of throwing"); + } + } + } + + // A rule that inspects nothing passes for the wrong reason: if the shape interfaces are + // renamed or Internals/ moves, this is what says so instead of quietly going green. + Assert.IsTrue(inspected >= 3, + $"Expected to inspect at least the three engines' shape switches, found {inspected}. " + + "Have the shape interfaces been renamed, or has Internals/ moved?"); + + Assert.AreEqual(0, violations.Count, + "A shape a binary operation was not written for must be fatal, never a value:" + + Environment.NewLine + string.Join(Environment.NewLine, violations)); + } + + /// + /// An engine's entry points take IRange<T> on both sides. A parameter typed as one + /// specific shape *is* receiver-shaped dispatch — it pushes the pair apart into an outer + /// overload and an inner switch, which is the structure every one of the four bugs had. + /// Private helpers below the dispatch are exempt: by then the pair is already decided. + /// + [TestMethod] + public void EngineEntryPointsDispatchOnThePair() + { + var violations = new List(); + var engines = InternalsFiles().Where(file => file.Name.EndsWith("Engine.cs", StringComparison.Ordinal)).ToList(); + + Assert.IsTrue(engines.Count >= 3, + $"Expected at least three *Engine.cs files under src/, found {engines.Count}."); + + foreach (var file in engines) + { + var root = CSharpSyntaxTree.ParseText(File.ReadAllText(file.FullName)).GetRoot(); + + foreach (var method in root.DescendantNodes().OfType() + .Where(method => !method.Modifiers.Any(SyntaxKind.PrivateKeyword))) + { + foreach (var parameter in method.ParameterList.Parameters + .Where(parameter => parameter.Type is not null) + .Where(parameter => ShapeInterfaces.Any(shape => parameter.Type!.ToString().StartsWith(shape, StringComparison.Ordinal)))) + { + violations.Add($"{Relative(file)}:{LineOf(parameter)} — {method.Identifier} takes " + + $"`{parameter.Type}` for `{parameter.Identifier}`; entry points take IRange on both sides"); + } + } + } + + Assert.AreEqual(0, violations.Count, + "Engine entry points must decide on the shape pair, not on one operand's shape:" + + Environment.NewLine + string.Join(Environment.NewLine, violations)); + } + + private static bool DispatchesOnShape(PatternSyntax pattern) => + ShapeInterfaces.Any(shape => pattern.ToString().Contains(shape, StringComparison.Ordinal)); + + private static bool ThrowsOutright(StatementSyntax statement) => + statement is ThrowStatementSyntax || statement.DescendantNodes().OfType().Any(); + + private static IEnumerable InternalsFiles() => + RepoLayout.PackableProjects + .Select(project => new DirectoryInfo(Path.Combine(project.Directory.FullName, "Internals"))) + .Where(directory => directory.Exists) + .SelectMany(directory => directory.EnumerateFiles("*.cs", SearchOption.AllDirectories)) + .OrderBy(file => file.FullName, StringComparer.Ordinal); + + private static int LineOf(SyntaxNode node) => + node.GetLocation().GetLineSpan().StartLinePosition.Line + 1; + + private static string Relative(FileInfo file) => + Path.GetRelativePath(RepoLayout.Root.FullName, file.FullName); +} diff --git a/test/CodoMetis.ValueRanges.Tests/RangeExceptTests.cs b/test/CodoMetis.ValueRanges.Tests/RangeExceptTests.cs index 015e9c2..ccbf34b 100644 --- a/test/CodoMetis.ValueRanges.Tests/RangeExceptTests.cs +++ b/test/CodoMetis.ValueRanges.Tests/RangeExceptTests.cs @@ -353,4 +353,37 @@ public void Except_Set_OpposingUnboundedElements_TrimsCorrectly() Assert.AreEqual("{(,0]}", upTo5.Except(from1).ToString()); Assert.AreEqual("{[6,)}", from1.Except(upTo5).ToString()); } + + /// + /// X \ (-∞, +∞) is the empty set for every X, the infinite set included. The single-range + /// overload has always answered this through its Contains guard and the set-minus-set overload + /// through its own infinite-operand guard; had + /// neither, so ∞ \ ∞ reached the engine as a pair no arm covered and the fallback answered ∞. + /// + [TestMethod] + public void Except_InfinityOperand_LeavesNothing() + { + Assert.IsTrue(RangeSet.Infinite.Except(Int32Range.Infinite).IsEmpty()); + Assert.IsTrue(RangeSet.Infinite.Except(DecimalRange.Infinite).IsEmpty()); + + // The same operand against a bounded set, which reached the engine by a different path. + var bounded = RangeSet.From([Int32Range.CreateFinite(1, 5, true, true)]); + Assert.IsTrue(bounded.Except(Int32Range.Infinite).IsEmpty()); + + // And the single-range and set-minus-set overloads it now agrees with. + Assert.IsTrue(Int32Range.Infinite.Except(Int32Range.Infinite).IsEmpty()); + Assert.IsTrue(RangeSet.Infinite.Except(RangeSet.Infinite).IsEmpty()); + } + + /// + /// Complement is defined as Infinite.Except(this), so the infinite set's complement + /// runs through the same path and must be empty rather than the whole domain. + /// + [TestMethod] + public void Complement_OfTheInfiniteSet_IsEmpty() + { + Assert.IsTrue(RangeSet.Infinite.Complement().IsEmpty()); + Assert.AreEqual(RangeSet.Infinite, + RangeSet.Infinite.Complement().Complement()); + } } From 2fad18a613aaa39313fac01dabcf52f854be735a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Mon, 17 Aug 2026 12:59:35 +0200 Subject: [PATCH 02/20] build: release these corrections as 7.0.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch: one wrong answer corrected, no public signature touched — dotnet pack passes ApiCompat against the 7.0.0 baseline, which moves up from 6.3.0 with the bump. SECURITY.md already covers the 7.0.x line. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 45 +++++++++++++++++++ Directory.Build.props | 4 +- .../CHANGELOG.md | 5 +++ .../CHANGELOG.md | 7 +++ .../CHANGELOG.md | 6 +++ src/CodoMetis.ValueRanges/CHANGELOG.md | 26 +++++++++++ 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 166676d..61dec7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,51 @@ filtered to the entries that affect it. Versions follow [Semantic Versioning](https://semver.org/). Entries are newest-first. +## [7.0.1] — 2026-08-17 + +A fourth instance of the trap 7.0.0 documented, and the structural change that makes a fifth fail +loudly instead of silently. + +7.0.0 recorded that three bugs had come from one shape: dispatch on the *receiver's* shape, an inner +switch over the operand's, and a discard arm that answers the pairs nobody wrote. Auditing the +engines for that shape turned up one more — `RangeSet.Except(TRange)` with an infinity operand — and +made the case that documenting the rule was not going to be enough on its own. The engines now +decide on the shape *pair*, and a pair with no arm throws instead of returning something plausible. + +### Fixed + +- **`RangeSet.Except(TRange)` returned the whole domain when subtracting an infinity range.** + `RangeSet.Infinite.Except(Int32Range.Infinite)` answered `{(,)}` where `X \ (-∞, + +∞)` is the empty set for every `X`, and `Complement()` on the infinite set was wrong through the + same path. The set-minus-set overload has always guarded its infinite operand and the single-range + overload answers it through its `Contains` guard, so — as with the empty-range containment bug in + 7.0.0 — the three overloads were answering the same question two different ways. The engine's + discard arm supplied `∞` for the one pair it was never given: `(Infinity, Infinity)`. + +### Changed + +- **The `Intersect`, `Merge` and `Except` engines dispatch on the shape pair.** Each had three + entry points typed by the receiver's shape, and each of those switched over the operand's shape + with a discard that rebuilt the receiver or returned `Empty` — the structure all four bugs shared. + They are now one entry point per engine taking `IRange` on both sides, switching over + `(left, right)` with one arm per accepted pair, so a pair nobody handled is a missing *line* + rather than something a fallback absorbs. No public signature changed and no behaviour changed + beyond the fix above; the 3,300-comparison shape matrix agrees with PostgreSQL as before. +- **A shape dispatch with no arm for its operands throws `UnreachableException` naming the pair.** + C# cannot prove a switch over interface patterns exhaustive, so the discard arm cannot be removed + — but it can stop producing values. Every one of the four bugs was a fallback returning something + well-formed: a wrong boolean, or a range of the right shape carrying the wrong values, which looks + correct in a debugger and disagrees only with the database. These paths are unreachable behind the + callers' existing guards; if a future change breaks one, the first test to reach it now says which + pair is missing. + +### Added + +- **`EngineDispatchConventionTests`**, which parses the shipping sources and enforces both halves of + the rule: a switch that dispatches on range shape must throw from its discard arm, and an engine's + entry points must take `IRange` on both sides rather than one operand's shape. Both are + discovered by globbing `src/`, and both were verified by seeding the defect they claim to catch. + ## [7.0.0] — 2026-08-17 Two workstreams land together: the validated-wrapper arities now exist for every value set family diff --git a/Directory.Build.props b/Directory.Build.props index 0535d60..04e04f0 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,6 +1,6 @@ - 7.0.0 + 7.0.1 CaffeinatedCoder MIT true @@ -60,7 +60,7 @@ --> true - 6.3.0 + 7.0.0