Merge #568/#569/#580/#585: shared AB status map + orphan release, confinement guard tokeniser, adapter + DefaultTransport tests

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-07 22:59:30 -04:00
parent 13a6727033 341417f193
commit 29cd25dec8
25 changed files with 430 additions and 310 deletions
@@ -1,3 +1,4 @@
+using System.Text;
 using System.Text.RegularExpressions;
 using Shouldly;
 using Xunit;
@@ -10,33 +11,230 @@ public sealed class LibplctagConfinementGuardTests
        @"\busing\s+(static\s+)?(\w+\s*=\s*)?libplctag\b|\blibplctag\.[A-Z]",
        RegexOptions.Compiled);

-    [Fact]
-    public void Only_the_libplctag_adapters_reference_libplctag()
+    // Directories whose Libplctag*.cs files may reference libplctag. LibplctagShared is the project-less home of
+    // sources compiled into BOTH drivers (#580).
+    private static readonly string[] AllowedSourceDirs =
+    [
+        "ZB.MOM.WW.OtOpcUa.Driver.AbCip",
+        "ZB.MOM.WW.OtOpcUa.Driver.AbLegacy",
+        "ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared",
+    ];
+
+    // The only projects that legitimately carry the libplctag package today (#569).
+    private static readonly string[] AllowedPackageProjects =
+    [
+        "ZB.MOM.WW.OtOpcUa.Driver.AbCip.csproj",
+        "ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.csproj",
+    ];
+
+    private static readonly Regex PackageRef = new(
+        @"<PackageReference\b[^>]*\b(Include|Update)\s*=\s*[""']libplctag(\.[\w.]*)?[""']",
+        RegexOptions.Compiled | RegexOptions.IgnoreCase);
+
+    private static DirectoryInfo RepoRoot()
    {
        var root = new DirectoryInfo(AppContext.BaseDirectory);
        while (root is not null && !File.Exists(Path.Combine(root.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) root = root.Parent;
        root.ShouldNotBeNull("could not find the repo root");
+        return root;
+    }
+
+    private static bool IsBuildOutput(string file) =>
+        file.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}") ||
+        file.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}");
+
+    /// <summary>
+    ///     Reduces C# source to its code: comments are dropped and string / char literal CONTENT is emptied
+    ///     (newlines are kept, so line numbers hold). Strings are tokenised properly rather than split on
+    ///     <c>//</c> — a URL literal used to truncate the line and hide a reference after it, and a
+    ///     <c>/* */</c> block comment was never stripped (#569).
+    /// </summary>
+    /// <param name="text">The C# source.</param>
+    /// <returns>The source with only code left.</returns>
+    internal static string StripCommentsAndLiterals(string text)
+    {
+        var sb = new StringBuilder(text.Length);
+        var i = 0;
+        while (i < text.Length)
+        {
+            var c = text[i];
+            var next = i + 1 < text.Length ? text[i + 1] : '\0';
+            if (c == '/' && next == '/')
+            {
+                while (i < text.Length && text[i] != '\n') i++;
+            }
+            else if (c == '/' && next == '*')
+            {
+                i += 2;
+                while (i < text.Length && !(text[i] == '*' && i + 1 < text.Length && text[i + 1] == '/'))
+                {
+                    if (text[i] == '\n') sb.Append('\n');
+                    i++;
+                }
+                i = Math.Min(i + 2, text.Length);
+            }
+            else if (c == '"' && next == '"' && i + 2 < text.Length && text[i + 2] == '"')
+            {
+                // Raw string literal: a run of >= 3 quotes opens it and closes on a run of at least that length.
+                var open = 0;
+                while (i + open < text.Length && text[i + open] == '"') open++;
+                i += open;
+                while (i < text.Length)
+                {
+                    if (text[i] == '"')
+                    {
+                        var run = 0;
+                        while (i + run < text.Length && text[i + run] == '"') run++;
+                        i += run;
+                        if (run >= open) break;
+                    }
+                    else
+                    {
+                        if (text[i] == '\n') sb.Append('\n');
+                        i++;
+                    }
+                }
+                sb.Append("\"\"");
+            }
+            else if (c == '"' || (c is '@' or '$' && IsStringPrefix(text, i)))
+            {
+                var verbatim = false;
+                while (text[i] != '"')
+                {
+                    if (text[i] == '@') verbatim = true;
+                    i++;
+                }
+                i++; // opening quote
+                while (i < text.Length)
+                {
+                    if (verbatim && text[i] == '"' && i + 1 < text.Length && text[i + 1] == '"') { i += 2; continue; }
+                    if (!verbatim && text[i] == '\\') { i += 2; continue; }
+                    if (text[i] == '"') { i++; break; }
+                    if (text[i] == '\n') sb.Append('\n');
+                    i++;
+                }
+                sb.Append("\"\"");
+            }
+            else if (c == '\'')
+            {
+                i++;
+                while (i < text.Length && text[i] != '\'')
+                {
+                    if (text[i] == '\\') i++;
+                    i++;
+                }
+                i++;
+                sb.Append("''");
+            }
+            else
+            {
+                sb.Append(c);
+                i++;
+            }
+        }
+        return sb.ToString();
+    }
+
+    // @"..", $"..", $@"..", @$".." — a run of prefix characters followed by a quote.
+    private static bool IsStringPrefix(string text, int i)
+    {
+        var j = i;
+        while (j < text.Length && text[j] is '@' or '$') j++;
+        return j < text.Length && text[j] == '"';
+    }
+
+    [Fact]
+    public void Only_the_libplctag_adapters_reference_libplctag()
+    {
+        var root = RepoRoot();

        var offenders = new List<string>();
        foreach (var file in Directory.EnumerateFiles(Path.Combine(root.FullName, "src"), "*.cs", SearchOption.AllDirectories))
        {
-            if (file.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}") ||
-                file.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}")) continue;
+            if (IsBuildOutput(file)) continue;
            var name = Path.GetFileName(file);
            var dir = Path.GetFileName(Path.GetDirectoryName(file)!);
-            var allowed = name.StartsWith("Libplctag", StringComparison.Ordinal)
-                && dir is "ZB.MOM.WW.OtOpcUa.Driver.AbCip" or "ZB.MOM.WW.OtOpcUa.Driver.AbLegacy";
+            var allowed = name.StartsWith("Libplctag", StringComparison.Ordinal) && AllowedSourceDirs.Contains(dir);
            if (allowed) continue;
-            var lines = File.ReadAllLines(file);
+            var raw = File.ReadAllLines(file);
+            var lines = StripCommentsAndLiterals(File.ReadAllText(file)).Split('\n');
            for (var i = 0; i < lines.Length; i++)
            {
-                var code = lines[i].Split("//")[0];
-                if (Reference.IsMatch(code)) offenders.Add($"{Path.GetRelativePath(root.FullName, file)}:{i + 1}: {lines[i].Trim()}");
+                if (Reference.IsMatch(lines[i]))
+                    offenders.Add($"{Path.GetRelativePath(root.FullName, file)}:{i + 1}: {(i < raw.Length ? raw[i].Trim() : lines[i].Trim())}");
            }
        }
        offenders.ShouldBeEmpty(string.Join(Environment.NewLine, offenders));
    }

+    /// <summary>#569: the libplctag PACKAGE may be referenced only by the two drivers that still carry the adapters.</summary>
+    [Fact]
+    public void Only_the_two_AB_driver_projects_reference_the_libplctag_package()
+    {
+        var root = RepoRoot();
+        var offenders = new List<string>();
+        foreach (var top in new[] { "src", "tests" })
+        {
+            foreach (var file in Directory.EnumerateFiles(Path.Combine(root.FullName, top), "*.csproj", SearchOption.AllDirectories))
+            {
+                if (IsBuildOutput(file)) continue;
+                if (AllowedPackageProjects.Contains(Path.GetFileName(file))) continue;
+                if (PackageRef.IsMatch(File.ReadAllText(file))) offenders.Add(Path.GetRelativePath(root.FullName, file));
+            }
+        }
+        offenders.ShouldBeEmpty("only the AbCip / AbLegacy driver projects may reference the libplctag package:" +
+                                Environment.NewLine + string.Join(Environment.NewLine, offenders));
+    }
+
+    /// <summary>The csproj regex matches the shapes a libplctag package reference can take (positive control).</summary>
+    /// <param name="xml">A package reference element.</param>
+    [Theory]
+    [InlineData("<PackageReference Include=\"libplctag\"/>")]
+    [InlineData("<PackageReference Include=\"libplctag\" Version=\"1.5.2\" />")]
+    [InlineData("<PackageReference Update='libplctag'/>")]
+    [InlineData("<PackageReference Include=\"libplctag.NativeImport\"/>")]
+    public void Csproj_package_references_trip_the_guard(string xml) => PackageRef.IsMatch(xml).ShouldBeTrue(xml);
+
+    /// <summary>An unrelated package that merely starts with the same letters does not trip it.</summary>
+    [Fact]
+    public void Unrelated_packages_do_not_trip_the_csproj_guard() =>
+        PackageRef.IsMatch("<PackageReference Include=\"libplctagger\"/>").ShouldBeFalse();
+
+    /// <summary>A reference AFTER a string literal containing <c>//</c> is still seen (#569 item 1).</summary>
+    [Fact]
+    public void A_reference_after_a_url_literal_is_not_hidden()
+    {
+        var code = StripCommentsAndLiterals("var u = \"http://example\"; var t = new libplctag.Tag();");
+        Reference.IsMatch(code).ShouldBeTrue(code);
+    }
+
+    /// <summary>A libplctag mention inside a string literal is text, not a reference.</summary>
+    [Fact]
+    public void A_reference_inside_a_string_literal_is_ignored()
+    {
+        Reference.IsMatch(StripCommentsAndLiterals("var s = \"using libplctag;\";")).ShouldBeFalse();
+        Reference.IsMatch(StripCommentsAndLiterals("var s = @\"a \"\"quoted\"\" libplctag.Tag\";")).ShouldBeFalse();
+    }
+
+    /// <summary>Block comments are stripped; code after them on the same line is kept (#569 item 2).</summary>
+    [Fact]
+    public void Block_comments_are_stripped_but_code_after_them_is_not()
+    {
+        Reference.IsMatch(StripCommentsAndLiterals("/* using libplctag; */ var x = 1;")).ShouldBeFalse();
+        Reference.IsMatch(StripCommentsAndLiterals("/* a\n using libplctag;\n b */ var x = 1;")).ShouldBeFalse();
+        Reference.IsMatch(StripCommentsAndLiterals("/* c */ var t = new libplctag.Tag();")).ShouldBeTrue();
+    }
+
+    /// <summary>Stripping keeps the line count, so offender line numbers stay accurate.</summary>
+    [Fact]
+    public void Stripping_preserves_line_numbers()
+    {
+        var src = "a\n/* x\ny */\nvar s = @\"p\nq\";\nusing libplctag;\n";
+        var stripped = StripCommentsAndLiterals(src).Split('\n');
+        stripped.Length.ShouldBe(src.Split('\n').Length);
+        Reference.IsMatch(stripped[5]).ShouldBeTrue(string.Join("|", stripped));
+    }
+
    [Fact]
    public void Prose_mentions_of_libplctag_do_not_trip_the_guard()
    {
@@ -1,3 +1,4 @@
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared;
 using System.Globalization;
 using Shouldly;
 using Xunit;
@@ -1,3 +1,4 @@
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared;
 using System.Buffers.Binary;
 using System.Globalization;
 using System.Reflection;
@@ -1,3 +1,4 @@
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared;
 using libplctag.NativeImport;
 using Shouldly;
 using Xunit;
@@ -9,27 +9,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
 [Trait("Category", "Unit")]
 public sealed class AbCipStatusMapperTests
 {
-    /// <summary>Verifies that CIP general status codes are mapped to OPC UA status codes.</summary>
-    /// <param name="status">The raw CIP general status byte.</param>
-    /// <param name="expected">The expected OPC UA status code.</param>
-    [Theory]
-    [InlineData((byte)0x00, AbCipStatusMapper.Good)]
-    [InlineData((byte)0x04, AbCipStatusMapper.BadNodeIdUnknown)]
-    [InlineData((byte)0x05, AbCipStatusMapper.BadNodeIdUnknown)]
-    [InlineData((byte)0x06, AbCipStatusMapper.GoodMoreData)]
-    [InlineData((byte)0x08, AbCipStatusMapper.BadNotSupported)]
-    [InlineData((byte)0x0A, AbCipStatusMapper.BadOutOfRange)]
-    [InlineData((byte)0x13, AbCipStatusMapper.BadOutOfRange)]
-    [InlineData((byte)0x0B, AbCipStatusMapper.Good)]
-    [InlineData((byte)0x0E, AbCipStatusMapper.BadNotWritable)]
-    [InlineData((byte)0x10, AbCipStatusMapper.BadDeviceFailure)]
-    [InlineData((byte)0x16, AbCipStatusMapper.BadNodeIdUnknown)]
-    [InlineData((byte)0xFF, AbCipStatusMapper.BadInternalError)]
-    public void MapCipGeneralStatus_maps_known_codes(byte status, uint expected)
-    {
-        AbCipStatusMapper.MapCipGeneralStatus(status).ShouldBe(expected);
-    }
-
    /// <summary>Verifies that transport status values are mapped to OPC UA status codes.</summary>
    /// <param name="status">The transport status to map.</param>
    /// <param name="expected">The expected OPC UA status code.</param>
@@ -77,6 +77,38 @@ public sealed class AbCipTypeMismatchTests
        MismatchWarnings(log).ShouldBe(1);
    }

+    /// <summary>#585: the libplctag-style width-mismatch path (status Ok, but the buffer is not the authored
+    /// type's width — a REAL authored as INT) gets the same treatment as the native TypeMismatch status.</summary>
+    [Fact]
+    public async Task A_width_mismatched_read_keeps_the_handle_leaves_health_alone_and_warns_once()
+    {
+        var creates = 0;
+        var factory = new FakeAbCipTagFactory
+        {
+            Customise = p => { Interlocked.Increment(ref creates); return new FakeAbCipTag(p) { Value = 1, Size = 4 }; },
+        };
+        var log = new CapturingLogger();
+        var drv = new AbCipDriver(new AbCipDriverOptions
+        {
+            Devices = [new AbCipDeviceOptions(Host)],
+            RawTags = AbCipRawTags.From(new AbCipTagDefinition("T", Host, "T", AbCipDataType.Int)),
+            Probe = new AbCipProbeOptions { Enabled = false },
+        }, "drv-585", factory, logger: log);
+        await drv.InitializeAsync("{}", CancellationToken.None);
+        var health = drv.GetHealth();
+
+        for (var n = 0; n < Polls; n++)
+            (await drv.ReadAsync(["T"], CancellationToken.None)).Single().StatusCode
+                .ShouldBe(AbCipStatusMapper.BadTypeMismatch);
+
+        Volatile.Read(ref creates).ShouldBe(1);
+        factory.Tags["T"].Disposed.ShouldBeFalse();
+        factory.Tags["T"].ReadCount.ShouldBe(Polls); // positive control: every poll did reach the runtime
+        drv.GetHealth().ShouldBe(health);
+        drv.GetHealth().State.ShouldNotBe(DriverState.Degraded);
+        MismatchWarnings(log).ShouldBe(1);
+    }
+
    [Fact]
    public async Task A_type_mismatched_write_keeps_the_handle_leaves_health_alone_and_warns_once()
    {
@@ -1,3 +1,4 @@
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared;
 using libplctag.NativeImport;
 using Shouldly;
 using Xunit;
@@ -39,6 +39,66 @@ public sealed class LibplctagStatusTranslationTests
        LibplctagStatusTranslation.ToAbStatus(new LibPlcTagException("gibberish")).ShouldBe(AbStatus.BadConnection);
    }

+    /// <summary>
+    ///     #568: the message-parse is pinned against the REAL exception libplctag builds from a <see cref="Status"/>
+    ///     (not a hand-typed message), for every defined status — if a libplctag bump changes the message format
+    ///     the translation would silently degrade to BadConnection, and this fails instead.
+    /// </summary>
+    [Fact]
+    public void A_real_LibPlcTagException_built_from_a_Status_round_trips_through_the_message_parse()
+    {
+        foreach (var status in Enum.GetValues<Status>())
+        {
+            if (status == Status.Ok || status == Status.Pending) continue; // libplctag throws only on errors
+            var ex = new LibPlcTagException(status);
+            LibplctagStatusTranslation.ToAbStatus(ex).ShouldBe(LibplctagStatusTranslation.ToAbStatus(status), $"{status}");
+        }
+    }
+
+    /// <summary>#568: a cancellation is not a libplctag failure — WrapSync must not translate or swallow it.</summary>
+    [Fact]
+    public void WrapSync_passes_OperationCanceledException_through()
+    {
+        Should.Throw<OperationCanceledException>(
+            () => LibplctagStatusTranslation.WrapSync<int>(() => throw new OperationCanceledException()));
+        Should.Throw<OperationCanceledException>(
+            () => LibplctagStatusTranslation.WrapSync(() => throw new OperationCanceledException()));
+    }
+
+    /// <summary>WrapSync translates a libplctag failure and returns a successful result unchanged.</summary>
+    [Fact]
+    public void WrapSync_translates_libplctag_failures_and_returns_results()
+    {
+        LibplctagStatusTranslation.WrapSync(() => 42).ShouldBe(42);
+        var ex = Should.Throw<AbTransportException>(
+            () => LibplctagStatusTranslation.WrapSync<int>(() => throw new LibPlcTagException("ErrorTimeout")));
+        ex.Status.ShouldBe(AbStatus.Timeout);
+    }
+
+    /// <summary>
+    ///     #568: the ADAPTER (not just the helper) routes decode through WrapSync. A real, never-initialised native
+    ///     <c>Tag</c> (no network I/O — nothing was created) throws a libplctag exception on a decode, and the runtime
+    ///     must surface it as an <see cref="AbTransportException"/>. Remove the WrapSync around the decode and this
+    ///     sees a raw <see cref="LibPlcTagException"/> (or <see cref="InvalidOperationException"/>) instead.
+    /// </summary>
+    [Fact]
+    public void LibplctagTagRuntime_decode_on_an_unread_tag_surfaces_AbTransportException()
+    {
+        var rt = new LibplctagTagRuntime(new AbCipTagCreateParams(
+            Gateway: "127.0.0.1", Port: 1, CipPath: "1,0", LibplctagPlcAttribute: "controllogix",
+            TagName: "NeverInitialised", Timeout: TimeSpan.FromMilliseconds(100)));
+
+        Should.Throw<AbTransportException>(() => rt.DecodeValueAt(AbCipDataType.DInt, 0, null));
+    }
+
+    /// <summary>Wrap passes a cancellation through untranslated.</summary>
+    [Fact]
+    public async Task Wrap_passes_OperationCanceledException_through()
+    {
+        await Should.ThrowAsync<OperationCanceledException>(
+            () => LibplctagStatusTranslation.Wrap(() => throw new OperationCanceledException()));
+    }
+
    /// <summary>Wrap rethrows as a transport exception keeping the original as InnerException.</summary>
    [Fact]
    public async Task Wrap_rethrows_as_AbTransportException_with_inner()
@@ -49,23 +49,6 @@ public sealed class AbLegacyHostAndStatusTests
        new AbLegacyHostAddress("10.0.0.5", 2222, "1,0").ToString().ShouldBe("ab://10.0.0.5:2222/1,0");
    }

-    /// <summary>Verifies that PCCC status codes are mapped to OPC UA status codes correctly.</summary>
-    /// <param name="sts">The PCCC status byte to map.</param>
-    /// <param name="expected">The expected OPC UA status code.</param>
-    [Theory]
-    [InlineData((byte)0x00, AbLegacyStatusMapper.Good)]
-    [InlineData((byte)0x10, AbLegacyStatusMapper.BadNotSupported)]
-    [InlineData((byte)0x20, AbLegacyStatusMapper.BadNodeIdUnknown)]
-    [InlineData((byte)0x30, AbLegacyStatusMapper.BadNotWritable)]
-    [InlineData((byte)0x40, AbLegacyStatusMapper.BadDeviceFailure)]
-    [InlineData((byte)0x50, AbLegacyStatusMapper.BadDeviceFailure)]
-    [InlineData((byte)0xF0, AbLegacyStatusMapper.BadInternalError)]
-    [InlineData((byte)0xFF, AbLegacyStatusMapper.BadCommunicationError)]
-    public void PcccStatus_maps_known_codes(byte sts, uint expected)
-    {
-        AbLegacyStatusMapper.MapPcccStatus(sts).ShouldBe(expected);
-    }
-
    /// <summary>Verifies that AbStatus members map to OPC UA status codes correctly.</summary>
    /// <param name="status">The AbStatus value to map.</param>
    /// <param name="expected">The expected OPC UA status code.</param>
@@ -92,16 +75,6 @@ public sealed class AbLegacyHostAndStatusTests
        AbLegacyStatusMapper.MapStatus((int)status).ShouldBe(expected);
    }

-    /// <summary>MapException reads the status off an AbTransportException, else falls back to comms error.</summary>
-    [Fact]
-    public void MapException_uses_transport_status()
-    {
-        AbLegacyStatusMapper.MapException(new AbTransportException(AbStatus.Timeout, "t"))
-            .ShouldBe(AbLegacyStatusMapper.BadTimeout);
-        AbLegacyStatusMapper.MapException(new InvalidOperationException("x"))
-            .ShouldBe(AbLegacyStatusMapper.BadCommunicationError);
-    }
-
    /// <summary>
    ///     The regression oracle: the OPC UA code the pre-seam <c>MapLibplctagStatus(Status)</c> returned,
    ///     copied verbatim from the deleted switch. For every libplctag member the new
@@ -1,3 +1,4 @@
+using ZB.MOM.WW.OtOpcUa.Driver.Ab.LibplctagShared;
 using libplctag.NativeImport;
 using Shouldly;
 using Xunit;