fix(abcip): isArray:true without a positive arrayLength is scalar (review I-2 consistency)

This commit is contained in:
Joseph Doherty
2026-06-16 22:30:11 -04:00
parent d30fb77e31
commit 5f7a2acd27
2 changed files with 35 additions and 5 deletions
@@ -49,9 +49,11 @@ public static class AbCipEquipmentTagParser
/// <summary>
/// Resolve the 1-D array shape from an <c>isArray</c> / <c>arrayLength</c> pair (the foundation
/// contract carrier). The tag is an ARRAY ⟺ <c>isArray</c> is truthy AND <c>arrayLength</c>
/// is a number <c>&gt;= 1</c> (a 1-element array is valid). Returns
/// <c>(IsArray: false, ElementCount: 1)</c> for a scalar.
/// contract carrier). The canonical rule: the tag is an ARRAY ⟺ <c>isArray</c> is truthy AND
/// <c>arrayLength</c> is a number <c>&gt;= 1</c>. Any other combination (isArray absent/false,
/// or isArray:true with arrayLength missing/invalid/&lt;1) is a SCALAR and returns
/// <c>(IsArray: false, ElementCount: 1)</c>. This matches every other driver (Modbus, S7,
/// TwinCAT, AbLegacy) which also return scalar for that degenerate input.
/// </summary>
private static (bool IsArray, int ElementCount) ReadArrayShape(JsonElement o)
{
@@ -62,8 +64,8 @@ public static class AbCipEquipmentTagParser
&& len.TryGetInt32(out var n)
&& n >= 1)
return (true, n);
// isArray:true but arrayLength missing/invalid — treat as a 1-element array (count 1).
return (true, 1);
// isArray:true but arrayLength missing/invalid/<1 — canonical rule: scalar (matches all other drivers).
return (false, 1);
}
private static TEnum ReadEnum<TEnum>(JsonElement o, string name, TEnum fallback) where TEnum : struct, Enum
@@ -201,6 +201,34 @@ public sealed class AbCipArrayTests
def.IsArray.ShouldBeFalse();
}
/// <summary>
/// Review finding I-2 — <c>isArray:true</c> with NO <c>arrayLength</c> is a DEGENERATE input
/// that must parse as SCALAR (IsArray false, ElementCount 1), matching every other driver
/// (Modbus, S7, TwinCAT, AbLegacy). The AdminUI validator blocks authoring this combination,
/// but the parser contract must still be consistent cross-driver.
/// </summary>
[Fact]
public void Parser_isArray_true_without_arrayLength_parses_as_scalar()
{
var json = """{"tagPath":"Recipe","dataType":"DInt","isArray":true}""";
AbCipEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.IsArray.ShouldBeFalse();
def.ElementCount.ShouldBe(1);
}
/// <summary>
/// Review finding I-2 — <c>isArray:true</c> with an invalid (zero) <c>arrayLength</c> also
/// parses as SCALAR, consistent with the cross-driver rule.
/// </summary>
[Fact]
public void Parser_isArray_true_with_zero_arrayLength_parses_as_scalar()
{
var json = """{"tagPath":"Recipe","dataType":"DInt","isArray":true,"arrayLength":0}""";
AbCipEquipmentTagParser.TryParse(json, out var def).ShouldBeTrue();
def!.IsArray.ShouldBeFalse();
def.ElementCount.ShouldBe(1);
}
/// <summary>
/// Review finding I-1 — a 1-element array (<c>isArray:true, arrayLength:1</c>) is a valid
/// 1-element array, NOT a scalar: the parser sets <see cref="AbCipTagDefinition.IsArray"/>