Skip to content

Commit fab423d

Browse files
dmealingclaude
andcommitted
refactor(csharp-render): FR-010 recover engine — pre-merge review + simplify
Pre-merge gates (code-review + code-simplifier) on the C# recover engine: Review fixes (RecoverMap, Java `instanceof Number` / `String.valueOf` parity): - numeric helpers (AsInt/AsLong/AsDouble) gate on actual numeric types and never throw — a non-numeric string or boolean returns null instead of throwing FormatException / coercing, preserving the never-throw contract once Phase-3 recover() codegen makes these helpers reachable. - AsString/AsStringList format numbers with InvariantCulture (locale- independent canonical value), matching Java String.valueOf. - 4 regression tests (never-throw, truncate-toward-zero, de-DE invariance). Simplifier: clarity-only refinements (switch expressions, pattern matching, collapsed redundant branches) — behavior unchanged. KNOWN_GAPS.md: nested-object deferral (parity with Java/Kotlin) + the blessed numeric-suffix and Unicode-whitespace cross-port divergences. Suite: 202 passed (was 198), incl. all 10 recover-conformance corpus cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bebb55b commit fab423d

7 files changed

Lines changed: 162 additions & 31 deletions

File tree

server/csharp/MetaObjects.Render.Tests/Recover/RecoverMapTests.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Globalization;
12
using MetaObjects.Render.Recover;
23
using Xunit;
34

@@ -70,4 +71,65 @@ public void AsStringListCoercesElementsToString()
7071
var result = RecoverMap.AsStringList(m, "xs");
7172
Assert.Equal(new[] { "1", "2" }, result);
7273
}
74+
75+
// --- Java-`instanceof Number` parity: numeric helpers gate on numbers, never throw (FR-010 review #1) ---
76+
77+
[Fact]
78+
public void NumericHelpersReturnNullForNonNumberValuesAndNeverThrow()
79+
{
80+
// A non-numeric string must yield null (Java `instanceof Number` is false), NOT a thrown
81+
// FormatException — recover() and its helpers must never throw.
82+
var m = new Dictionary<string, object?> { ["s"] = "abc", ["b"] = true };
83+
84+
Assert.Null(RecoverMap.AsInt(m, "s"));
85+
Assert.Null(RecoverMap.AsLong(m, "s"));
86+
Assert.Null(RecoverMap.AsDouble(m, "s"));
87+
88+
// A boolean is not a Number → null (Java parity), not coerced to 1.
89+
Assert.Null(RecoverMap.AsInt(m, "b"));
90+
Assert.Null(RecoverMap.AsLong(m, "b"));
91+
Assert.Null(RecoverMap.AsDouble(m, "b"));
92+
}
93+
94+
[Fact]
95+
public void AsIntTruncatesFloatingTowardZeroLikeJavaIntValue()
96+
{
97+
var m = new Dictionary<string, object?> { ["d"] = 42.9 };
98+
Assert.Equal(42, RecoverMap.AsInt(m, "d")); // truncate, not round-to-44
99+
Assert.Equal(42L, RecoverMap.AsLong(m, "d"));
100+
}
101+
102+
// --- Locale-independence: numeric→string is invariant, matching Java `String.valueOf` (FR-010 review #2) ---
103+
104+
[Fact]
105+
public void AsStringFormatsNumbersInvariantOfCulture()
106+
{
107+
var prior = CultureInfo.CurrentCulture;
108+
try
109+
{
110+
CultureInfo.CurrentCulture = new CultureInfo("de-DE"); // comma decimal separator
111+
var m = new Dictionary<string, object?> { ["d"] = 1234.5 };
112+
Assert.Equal("1234.5", RecoverMap.AsString(m, "d")); // dot, not "1234,5"
113+
}
114+
finally
115+
{
116+
CultureInfo.CurrentCulture = prior;
117+
}
118+
}
119+
120+
[Fact]
121+
public void AsStringListFormatsNumbersInvariantOfCulture()
122+
{
123+
var prior = CultureInfo.CurrentCulture;
124+
try
125+
{
126+
CultureInfo.CurrentCulture = new CultureInfo("de-DE");
127+
var m = new Dictionary<string, object?> { ["xs"] = new List<object?> { 1234.5, 6.25 } };
128+
Assert.Equal(new[] { "1234.5", "6.25" }, RecoverMap.AsStringList(m, "xs"));
129+
}
130+
finally
131+
{
132+
CultureInfo.CurrentCulture = prior;
133+
}
134+
}
73135
}

server/csharp/MetaObjects.Render/Recover/Coerce.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,8 @@ public static class Coerce
4747
}
4848

4949
// Per-field runtime normalizer: keyed by field path, then by simple name.
50-
Func<string, object?>? norm = null;
51-
opts.Normalizers.TryGetValue(fieldPath, out norm);
52-
if (norm == null) opts.Normalizers.TryGetValue(spec.Name, out norm);
50+
if (!opts.Normalizers.TryGetValue(fieldPath, out var norm))
51+
opts.Normalizers.TryGetValue(spec.Name, out norm);
5352
if (norm != null)
5453
{
5554
object? normalized = norm(raw);

server/csharp/MetaObjects.Render/Recover/JsonForgivingReader.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public sealed class JsonForgivingReader
3838
char c = _s[_i];
3939
if (c == '{') return ReadObject();
4040
if (c == '[') return ReadArray();
41-
if (c == '"' || c == '\'') return ReadString(c);
41+
if (c is '"' or '\'') return ReadString(c);
4242
return ReadBareScalar();
4343
}
4444

@@ -96,8 +96,7 @@ public sealed class JsonForgivingReader
9696
Ws();
9797
if (_i < _s.Length && _s[_i] == ',') _i++;
9898
else if (_i < _s.Length && _s[_i] == ']') { _i++; return xs; }
99-
else if (_i >= _s.Length) return xs;
100-
else return xs; // any other non-separator char → stop
99+
else return xs; // EOF or any other non-separator char → stop
101100
}
102101
}
103102

@@ -106,7 +105,7 @@ public sealed class JsonForgivingReader
106105
Ws();
107106
if (_i >= _s.Length) return null;
108107
char c = _s[_i];
109-
if (c == '"' || c == '\'') return ReadString(c);
108+
if (c is '"' or '\'') return ReadString(c);
110109
int start = _i;
111110
while (_i < _s.Length && (char.IsLetterOrDigit(_s[_i]) || _s[_i] == '_')) _i++;
112111
return _i > start ? _s[start.._i] : null;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# FR-010 C# recover engine — known gaps & intentional cross-port divergences
2+
3+
Scope: the tolerant `Recover` pipeline (`MetaObjects.Render/Recover/`). The Java engine
4+
(`server/java/render/.../recover/`) is the reference; `fixtures/recover-conformance/` is the
5+
cross-port oracle. All 10 corpus cases pass.
6+
7+
## Bounded deferrals (parity with Java/Kotlin)
8+
9+
- **Nested-object recover.** Only scalar / enum / scalar-array fields are recovered. A
10+
`field.object` (nested record) is not descended into. Same deferral as the Java and Kotlin
11+
ports (tracked there as Plan 2.1/3.1). Phase 3 codegen will reject / skip nested fields
12+
consistently rather than emit a partial mapping.
13+
14+
## Intentional, documented divergences (NOT bugs)
15+
16+
These differ from Java on inputs the corpus does not exercise. The cross-port contract pins
17+
*classification + canonical value* (numeric within ±1e-9), **not** byte-identical native
18+
formatting — see the FR-010 design spec and `fr-010-plan-decisions` memory.
19+
20+
- **Numeric suffix / exotic literals.** Java's `Double.parseDouble` accepts `"42f"`, `"42d"`,
21+
and hex-float literals (→ RECOVERED). The C# parse (`double.TryParse`, invariant culture)
22+
rejects them → **MALFORMED**. The load-bearing behavior — finite-only acceptance and
23+
`NaN`/`±Infinity` → MALFORMED — is identical across ports. (See the cross-port note in
24+
`Coerce.cs`.)
25+
26+
- **Unicode whitespace.** `JsonForgivingReader` uses `char.IsWhiteSpace`; Java uses
27+
`Character.isWhitespace`. They disagree on NBSP (U+00A0), U+2007, U+202F. Only reachable when
28+
a value/key is padded with a non-ASCII space — outside corpus coverage.
29+
30+
## Resolved in this port (review-driven, before Phase 1 merge)
31+
32+
- `RecoverMap` numeric helpers gate on actual numeric types (mirroring Java `instanceof Number`):
33+
a non-numeric string or a boolean returns `null` instead of throwing / coercing. Preserves the
34+
never-throw contract once Phase-3 `recover()` codegen makes these helpers reachable.
35+
- `RecoverMap.AsString` / `AsStringList` format numbers with `CultureInfo.InvariantCulture`,
36+
matching Java `String.valueOf` (locale-independent canonical value).

server/csharp/MetaObjects.Render/Recover/Recover.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,11 @@ public static RecoverOutcome Run(string? text, RecoverSchema schema, RecoverOpti
2626

2727
Dictionary<string, object?> raw;
2828
if (span == null)
29-
{
3029
raw = new Dictionary<string, object?>();
31-
}
3230
else if (schema.Format == Format.Json)
33-
{
3431
raw = new JsonForgivingReader().Read(span);
35-
}
3632
else
37-
{
3833
raw = new XmlForgivingReader().Read(span, ci);
39-
}
4034

4135
if (raw.Count == 0 && (stripped.Length == 0 || span == null))
4236
{

server/csharp/MetaObjects.Render/Recover/RecoverMap.cs

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using System.Globalization;
2+
13
namespace MetaObjects.Render.Recover;
24

35
/// <summary>
@@ -7,43 +9,88 @@ namespace MetaObjects.Render.Recover;
79
public static class RecoverMap
810
{
911
/// <summary>
10-
/// Returns the string value for key <paramref name="k"/>, coercing non-strings via
11-
/// <see cref="Convert.ToString(object)"/>. Returns <c>null</c> when the key is absent.
12+
/// Returns the string value for key <paramref name="k"/>, coercing non-strings via invariant-culture
13+
/// <see cref="Convert.ToString(object, IFormatProvider)"/>. Returns <c>null</c> when the key is absent.
1214
/// </summary>
15+
/// <remarks>
16+
/// Mirrors Java <c>String.valueOf</c>: locale-independent. The generated path only calls this on
17+
/// string-typed fields (value already a string), so the numeric fallback is defensive.
18+
/// </remarks>
1319
public static string? AsString(IReadOnlyDictionary<string, object?> d, string k)
1420
{
1521
if (!d.TryGetValue(k, out object? v)) return null;
16-
return v == null ? null : (v is string s ? s : Convert.ToString(v));
22+
return v switch
23+
{
24+
null => null,
25+
string s => s,
26+
_ => Convert.ToString(v, CultureInfo.InvariantCulture),
27+
};
1728
}
1829

1930
/// <summary>
2031
/// Returns the value narrowed to <see cref="int"/> for key <paramref name="k"/>.
21-
/// Returns <c>null</c> when the key is absent or the value is not a <see cref="IConvertible"/> number.
32+
/// Returns <c>null</c> when the key is absent or the value is not a number.
2233
/// </summary>
34+
/// <remarks>
35+
/// Mirrors Java <c>Number.intValue()</c>: gates on actual numeric types (never throws on a string/bool,
36+
/// unlike <see cref="Convert"/>) and truncates floating values toward zero.
37+
/// </remarks>
2338
public static int? AsInt(IReadOnlyDictionary<string, object?> d, string k)
2439
{
2540
if (!d.TryGetValue(k, out object? v)) return null;
26-
return v is IConvertible c ? (int?)Convert.ToInt32(c) : null;
41+
return v switch
42+
{
43+
long l => (int)l,
44+
int i => i,
45+
double db => (int)db,
46+
float f => (int)f,
47+
decimal m => (int)m,
48+
short s => s,
49+
byte b => b,
50+
_ => null,
51+
};
2752
}
2853

2954
/// <summary>
3055
/// Returns the value as <see cref="long"/> for key <paramref name="k"/>.
31-
/// Returns <c>null</c> when the key is absent or the value is not a <see cref="IConvertible"/> number.
56+
/// Returns <c>null</c> when the key is absent or the value is not a number.
3257
/// </summary>
58+
/// <remarks>Mirrors Java <c>Number.longValue()</c>: numeric-typed, never throws, truncates toward zero.</remarks>
3359
public static long? AsLong(IReadOnlyDictionary<string, object?> d, string k)
3460
{
3561
if (!d.TryGetValue(k, out object? v)) return null;
36-
return v is IConvertible c ? (long?)Convert.ToInt64(c) : null;
62+
return v switch
63+
{
64+
long l => l,
65+
int i => i,
66+
double db => (long)db,
67+
float f => (long)f,
68+
decimal m => (long)m,
69+
short s => s,
70+
byte b => b,
71+
_ => null,
72+
};
3773
}
3874

3975
/// <summary>
4076
/// Returns the value as <see cref="double"/> for key <paramref name="k"/>.
41-
/// Returns <c>null</c> when the key is absent or the value is not a <see cref="IConvertible"/> number.
77+
/// Returns <c>null</c> when the key is absent or the value is not a number.
4278
/// </summary>
79+
/// <remarks>Mirrors Java <c>Number.doubleValue()</c>: numeric-typed, never throws.</remarks>
4380
public static double? AsDouble(IReadOnlyDictionary<string, object?> d, string k)
4481
{
4582
if (!d.TryGetValue(k, out object? v)) return null;
46-
return v is IConvertible c ? (double?)Convert.ToDouble(c) : null;
83+
return v switch
84+
{
85+
long l => l,
86+
int i => i,
87+
double db => db,
88+
float f => f,
89+
decimal m => (double)m,
90+
short s => s,
91+
byte b => b,
92+
_ => null,
93+
};
4794
}
4895

4996
/// <summary>
@@ -67,7 +114,7 @@ public static class RecoverMap
67114
if (v is not List<object?> list) return null;
68115
var out_ = new List<string?>(list.Count);
69116
foreach (object? e in list)
70-
out_.Add(e == null ? null : Convert.ToString(e));
117+
out_.Add(e == null ? null : Convert.ToString(e, CultureInfo.InvariantCulture));
71118
return out_.AsReadOnly();
72119
}
73120
}

server/csharp/MetaObjects.Render/Recover/XmlForgivingReader.cs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,9 @@ private static void Accumulate(Dictionary<string, object?> out_, string key, obj
9999
out_[key] = value;
100100
return;
101101
}
102-
object? existing = out_[key];
103-
if (existing is List<object?> list)
104-
{
102+
if (out_[key] is List<object?> list)
105103
list.Add(value);
106-
}
107104
else
108-
{
109-
var newList = new List<object?> { existing, value };
110-
out_[key] = newList;
111-
}
105+
out_[key] = new List<object?> { out_[key], value };
112106
}
113107
}

0 commit comments

Comments
 (0)