fix(driver-ablegacy): resolve Medium code-review finding (Driver.AbLegacy-003)
TryParse now rejects three classes of malformed PCCC address: - Sub-element + bit-index together (e.g. T4:0.ACC/2) — never valid in PCCC - File number on I/O/S system files (e.g. I3:0, S2:1) — single-letter only - Sub-element on non-T/C/R files (e.g. B3:0.DN, N7:0.FOO) — only Timer, Counter, and Control files carry structured elements New helper predicates IsNoFileNumberLetter / IsSubElementFileLetter keep the parser's intent clear. Regression tests added in AbLegacyAddressTests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -102,6 +102,19 @@ public sealed record AbLegacyAddress(
|
|||||||
if (maxBit < 0 || b > maxBit) return null;
|
if (maxBit < 0 || b > maxBit) return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// I/O/S are single-letter system files — they carry no file number in the PCCC spec.
|
||||||
|
// Accepting I3:0 or S2:1 would pass a malformed address straight to libplctag; reject early.
|
||||||
|
if (fileNumber is not null && IsNoFileNumberLetter(letter)) return null;
|
||||||
|
|
||||||
|
// A PCCC address cannot have both a sub-element and a bit index: the word is either
|
||||||
|
// structured (T4:0.ACC) or bit-addressed (N7:0/3), never both.
|
||||||
|
if (subElement is not null && bitIndex is not null) return null;
|
||||||
|
|
||||||
|
// Sub-elements are only meaningful on Timer (T), Counter (C), and Control (R) files —
|
||||||
|
// those are the only structured-element file types in the PCCC spec. Accepting B3:0.DN
|
||||||
|
// or N7:0.FOO would produce an address libplctag silently misinterprets.
|
||||||
|
if (subElement is not null && !IsSubElementFileLetter(letter)) return null;
|
||||||
|
|
||||||
return new AbLegacyAddress(letter, fileNumber, word, bitIndex, subElement);
|
return new AbLegacyAddress(letter, fileNumber, word, bitIndex, subElement);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -122,4 +135,18 @@ public sealed record AbLegacyAddress(
|
|||||||
"N" or "F" or "B" or "L" or "ST" or "T" or "C" or "R" or "I" or "O" or "S" or "A" => true,
|
"N" or "F" or "B" or "L" or "ST" or "T" or "C" or "R" or "I" or "O" or "S" or "A" => true,
|
||||||
_ => false,
|
_ => false,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Returns <see langword="true"/> for file letters that carry no explicit file number in the
|
||||||
|
/// PCCC spec. <c>I</c> (input), <c>O</c> (output), and <c>S</c> (status) are single-letter
|
||||||
|
/// system files; a digit after the letter (e.g. <c>I3</c>) is a malformed address.
|
||||||
|
/// </summary>
|
||||||
|
private static bool IsNoFileNumberLetter(string letter) => letter is "I" or "O" or "S";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Returns <see langword="true"/> for file letters that may carry a sub-element suffix
|
||||||
|
/// (<c>.ACC</c>, <c>.PRE</c>, etc.). Only Timer (<c>T</c>), Counter (<c>C</c>), and
|
||||||
|
/// Control (<c>R</c>) files have structured elements in the PCCC spec.
|
||||||
|
/// </summary>
|
||||||
|
private static bool IsSubElementFileLetter(string letter) => letter is "T" or "C" or "R";
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -65,4 +65,41 @@ public sealed class AbLegacyAddressTests
|
|||||||
a.ShouldNotBeNull();
|
a.ShouldNotBeNull();
|
||||||
a.ToLibplctagName().ShouldBe(input);
|
a.ToLibplctagName().ShouldBe(input);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- Driver.AbLegacy-003: Parser tightening ----
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("T4:0.ACC/2")] // sub-element + bit index — never valid in PCCC
|
||||||
|
[InlineData("C5:0.PRE/3")]
|
||||||
|
public void TryParse_rejects_subelement_plus_bitindex(string input) =>
|
||||||
|
AbLegacyAddress.TryParse(input).ShouldBeNull();
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("I3:0")] // I is a system file — no file number allowed
|
||||||
|
[InlineData("O2:1")]
|
||||||
|
[InlineData("S2:1")]
|
||||||
|
public void TryParse_rejects_file_number_on_IOS_files(string input) =>
|
||||||
|
AbLegacyAddress.TryParse(input).ShouldBeNull();
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("B3:0.DN")] // B (bit) file has no structured elements
|
||||||
|
[InlineData("N7:0.FOO")] // N (integer) file has no structured elements
|
||||||
|
[InlineData("F8:0.ACC")] // F (float) file has no structured elements
|
||||||
|
[InlineData("L9:0.PRE")] // L (long) file has no structured elements
|
||||||
|
public void TryParse_rejects_subelement_on_non_structured_file(string input) =>
|
||||||
|
AbLegacyAddress.TryParse(input).ShouldBeNull();
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("T4:0.ACC")] // T, C, R are the only structured-element files
|
||||||
|
[InlineData("C5:0.PRE")]
|
||||||
|
[InlineData("R6:0.LEN")]
|
||||||
|
public void TryParse_accepts_subelement_only_on_TCR_files(string input) =>
|
||||||
|
AbLegacyAddress.TryParse(input).ShouldNotBeNull();
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("I:0/0")] // I/O/S without file number are valid
|
||||||
|
[InlineData("O:1/2")]
|
||||||
|
[InlineData("S:1")]
|
||||||
|
public void TryParse_accepts_IOS_without_file_number(string input) =>
|
||||||
|
AbLegacyAddress.TryParse(input).ShouldNotBeNull();
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user