From 2c571001ca9a0897f4a74e86bd6c354e191fe27d Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 22 May 2026 09:23:12 -0400
Subject: [PATCH] fix(scripting): resolve Medium code-review finding
 (Core.Scripting-004)

DependencyExtractor.VisitInvocationExpression now additionally checks
that the member-access receiver is the identifier "ctx" before treating
a GetTag / SetVirtualTag call as a ScriptContext dependency. This
prevents spurious dependencies when a script defines a local helper type
with a matching method name and calls it as other.GetTag("X"). Test
Ignores_member_access_GetTag_on_non_ctx_receiver added.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../DependencyExtractor.cs                    | 27 ++++++++++++-------
 .../DependencyExtractorTests.cs               | 20 +++++++++++++-
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/DependencyExtractor.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/DependencyExtractor.cs
index 34dde7f..b764c5d 100644
--- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/DependencyExtractor.cs
+++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/DependencyExtractor.cs
@@ -21,11 +21,15 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Scripting;
 ///         token.
 ///     </para>
 ///     <para>
-///         Identifier matching is by spelling: the extractor looks for
-///         <c>ctx.GetTag(...)</c> / <c>ctx.SetVirtualTag(...)</c> literally. A deliberately
-///         misspelled method call (<c>ctx.GetTagz</c>) is not picked up but will also fail
-///         to compile against <see cref="ScriptContext"/>, so there's no way to smuggle a
-///         dependency past the extractor while still having a working script.
+///         Matching is by spelling: the extractor looks for member-access invocations
+///         whose receiver identifier is literally <c>ctx</c> and whose method name is
+///         <c>GetTag</c> or <c>SetVirtualTag</c>. A deliberately misspelled method call
+///         (<c>ctx.GetTagz</c>) is not picked up but will also fail to compile against
+///         <see cref="ScriptContext"/>, so there is no way to smuggle a dependency past the
+///         extractor while still having a working script. Calls with the same method name on
+///         a different receiver (<c>other.GetTag("X")</c>) are explicitly ignored so that
+///         scripts defining local helper types with matching names do not produce spurious
+///         dependencies. (Core.Scripting-004.)
 ///     </para>
 /// </remarks>
 public static class DependencyExtractor
@@ -67,10 +71,15 @@ public static class DependencyExtractor
 
         public override void VisitInvocationExpression(InvocationExpressionSyntax node)
         {
-            // Only interested in member-access form: ctx.GetTag(...) / ctx.SetVirtualTag(...).
-            // Anything else (free functions, chained calls, static calls) is ignored — but
-            // still visit children in case a ctx.GetTag call is nested inside.
-            if (node.Expression is MemberAccessExpressionSyntax member)
+            // Only interested in ctx.GetTag(...) / ctx.SetVirtualTag(...) — member-access
+            // form where the receiver is the identifier "ctx" (the ScriptGlobals<T>.ctx
+            // field). Calls with the same method name on a different receiver (e.g.
+            // someHelper.GetTag("X")) are ignored — not picking them up avoids spurious
+            // dependencies when scripts define local types with matching method names.
+            // (Core.Scripting-004.)
+            if (node.Expression is MemberAccessExpressionSyntax member
+                && member.Expression is IdentifierNameSyntax receiver
+                && receiver.Identifier.ValueText == "ctx")
             {
                 var methodName = member.Name.Identifier.ValueText;
                 if (methodName is nameof(ScriptContext.GetTag) or nameof(ScriptContext.SetVirtualTag))
diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/DependencyExtractorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/DependencyExtractorTests.cs
index 4387dc6..aaa6751 100644
--- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/DependencyExtractorTests.cs
+++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests/DependencyExtractorTests.cs
@@ -133,7 +133,7 @@ public sealed class DependencyExtractorTests
     }
 
     [Fact]
-    public void Ignores_non_ctx_method_named_GetTag()
+    public void Ignores_non_ctx_method_named_GetTag_free_function()
     {
         // Scripts are free to define their own helper called "GetTag" — as long as it's
         // not on the ctx instance, the extractor doesn't pick it up. The sandbox
@@ -147,6 +147,24 @@ public sealed class DependencyExtractorTests
         result.Reads.ShouldBeEmpty();
     }
 
+    [Fact]
+    public void Ignores_member_access_GetTag_on_non_ctx_receiver()
+    {
+        // A member-access call to GetTag on a non-ctx identifier must NOT be treated as
+        // a ScriptContext dependency. The old walker accepted any receiver; the fix
+        // requires the receiver to be the identifier "ctx". (Core.Scripting-004.)
+        var result = DependencyExtractor.Extract(
+            """
+            class Helper { public object GetTag(string p) => p; }
+            var h = new Helper();
+            var v = h.GetTag("X");
+            return ctx.GetTag("RealTag").Value;
+            """);
+        result.IsValid.ShouldBeTrue();
+        result.Reads.ShouldContain("RealTag");
+        result.Reads.ShouldNotContain("X");
+    }
+
     [Fact]
     public void Empty_source_is_a_no_op()
     {