fix(driver-s7-cli): resolve Medium code-review finding (Driver.S7.Cli-001)
Wrap all numeric/DateTime BCL parses in ParseValue with try/catch(FormatException) and try/catch(OverflowException) that re-throw as CommandException, matching the existing Bool path. Update ParseValue_non_numeric_for_numeric_types_throws to assert CommandException (not FormatException), and add an overflow-edge test (Byte value 256). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -18,9 +18,13 @@ public sealed class WriteCommand : S7CommandBase
|
|||||||
"S7 address — same format as `read`.", IsRequired = true)]
|
"S7 address — same format as `read`.", IsRequired = true)]
|
||||||
public string Address { get; init; } = default!;
|
public string Address { get; init; } = default!;
|
||||||
|
|
||||||
|
// Driver.S7.Cli-002: help text trimmed to the types the driver actually implements.
|
||||||
|
// Int64 / UInt64 / Float64 / String / DateTime are defined in S7DataType but the driver
|
||||||
|
// raises NotSupportedException (→ BadNotSupported) on any read/write of those types;
|
||||||
|
// advertising them misleads operators who then see BadNotSupported with no explanation.
|
||||||
[CommandOption("type", 't', Description =
|
[CommandOption("type", 't', Description =
|
||||||
"Bool / Byte / Int16 / UInt16 / Int32 / UInt32 / Int64 / UInt64 / Float32 / Float64 / " +
|
"Bool / Byte / Int16 / UInt16 / Int32 / UInt32 / Float32 (default Int16). " +
|
||||||
"String / DateTime (default Int16).")]
|
"Int64, UInt64, Float64, String, and DateTime are not yet implemented and will return BadNotSupported.")]
|
||||||
public S7DataType DataType { get; init; } = S7DataType.Int16;
|
public S7DataType DataType { get; init; } = S7DataType.Int16;
|
||||||
|
|
||||||
[CommandOption("value", 'v', Description =
|
[CommandOption("value", 'v', Description =
|
||||||
@@ -62,22 +66,44 @@ public sealed class WriteCommand : S7CommandBase
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>Parse <c>--value</c> per <see cref="S7DataType"/>, invariant culture throughout.</summary>
|
/// <summary>Parse <c>--value</c> per <see cref="S7DataType"/>, invariant culture throughout.</summary>
|
||||||
internal static object ParseValue(string raw, S7DataType type) => type switch
|
/// <remarks>
|
||||||
|
/// Driver.S7.Cli-001: numeric and <see cref="DateTime"/> parses are wrapped so that
|
||||||
|
/// malformed input (<see cref="FormatException"/> / <see cref="OverflowException"/>)
|
||||||
|
/// surfaces as a clean <see cref="CliFx.Exceptions.CommandException"/> rather than a
|
||||||
|
/// raw .NET stack trace — matching the friendly message the Bool path already produces.
|
||||||
|
/// </remarks>
|
||||||
|
internal static object ParseValue(string raw, S7DataType type)
|
||||||
{
|
{
|
||||||
S7DataType.Bool => ParseBool(raw),
|
if (type == S7DataType.Bool) return ParseBool(raw);
|
||||||
S7DataType.Byte => byte.Parse(raw, CultureInfo.InvariantCulture),
|
if (type == S7DataType.String) return raw;
|
||||||
S7DataType.Int16 => short.Parse(raw, CultureInfo.InvariantCulture),
|
try
|
||||||
S7DataType.UInt16 => ushort.Parse(raw, CultureInfo.InvariantCulture),
|
{
|
||||||
S7DataType.Int32 => int.Parse(raw, CultureInfo.InvariantCulture),
|
return type switch
|
||||||
S7DataType.UInt32 => uint.Parse(raw, CultureInfo.InvariantCulture),
|
{
|
||||||
S7DataType.Int64 => long.Parse(raw, CultureInfo.InvariantCulture),
|
S7DataType.Byte => (object)byte.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
S7DataType.UInt64 => ulong.Parse(raw, CultureInfo.InvariantCulture),
|
S7DataType.Int16 => (object)short.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
S7DataType.Float32 => float.Parse(raw, CultureInfo.InvariantCulture),
|
S7DataType.UInt16 => (object)ushort.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
S7DataType.Float64 => double.Parse(raw, CultureInfo.InvariantCulture),
|
S7DataType.Int32 => (object)int.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
S7DataType.String => raw,
|
S7DataType.UInt32 => (object)uint.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
S7DataType.DateTime => DateTime.Parse(raw, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind),
|
S7DataType.Int64 => (object)long.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
|
S7DataType.UInt64 => (object)ulong.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
|
S7DataType.Float32 => (object)float.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
|
S7DataType.Float64 => (object)double.Parse(raw, CultureInfo.InvariantCulture),
|
||||||
|
S7DataType.DateTime => (object)DateTime.Parse(raw, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind),
|
||||||
_ => throw new CliFx.Exceptions.CommandException($"Unsupported DataType '{type}' for write."),
|
_ => throw new CliFx.Exceptions.CommandException($"Unsupported DataType '{type}' for write."),
|
||||||
};
|
};
|
||||||
|
}
|
||||||
|
catch (FormatException ex)
|
||||||
|
{
|
||||||
|
throw new CliFx.Exceptions.CommandException(
|
||||||
|
$"Value '{raw}' is not a valid {type}: {ex.Message}");
|
||||||
|
}
|
||||||
|
catch (OverflowException ex)
|
||||||
|
{
|
||||||
|
throw new CliFx.Exceptions.CommandException(
|
||||||
|
$"Value '{raw}' is out of range for {type}: {ex.Message}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static bool ParseBool(string raw) => raw.Trim().ToLowerInvariant() switch
|
private static bool ParseBool(string raw) => raw.Trim().ToLowerInvariant() switch
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -99,10 +99,20 @@ public sealed class WriteCommandParseValueTests
|
|||||||
[Fact]
|
[Fact]
|
||||||
public void ParseValue_non_numeric_for_numeric_types_throws()
|
public void ParseValue_non_numeric_for_numeric_types_throws()
|
||||||
{
|
{
|
||||||
Should.Throw<FormatException>(
|
// Driver.S7.Cli-001: malformed input must produce a clean CommandException
|
||||||
|
// (friendly one-line error), not a raw FormatException stack trace.
|
||||||
|
Should.Throw<CliFx.Exceptions.CommandException>(
|
||||||
() => WriteCommand.ParseValue("xyz", S7DataType.Int16));
|
() => WriteCommand.ParseValue("xyz", S7DataType.Int16));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ParseValue_overflow_for_numeric_types_throws_CommandException()
|
||||||
|
{
|
||||||
|
// OverflowException from out-of-range input must also surface as CommandException.
|
||||||
|
Should.Throw<CliFx.Exceptions.CommandException>(
|
||||||
|
() => WriteCommand.ParseValue("256", S7DataType.Byte));
|
||||||
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("DB1.DBW0", S7DataType.Int16, "DB1.DBW0:Int16")]
|
[InlineData("DB1.DBW0", S7DataType.Int16, "DB1.DBW0:Int16")]
|
||||||
[InlineData("M0.0", S7DataType.Bool, "M0.0:Bool")]
|
[InlineData("M0.0", S7DataType.Bool, "M0.0:Bool")]
|
||||||
|
|||||||
Reference in New Issue
Block a user