Roslyn analyzer — detect unwrapped driver-capability calls (OTOPCUA0001). Closes task #200. New netstandard2.0 analyzer project src/ZB.MOM.WW.OtOpcUa.Analyzers registered as an <Analyzer>-item ProjectReference from the Server csproj so the warning fires at every Server compile. First (and only so far) rule OTOPCUA0001 — "Driver capability call must be wrapped in CapabilityInvoker" — walks every InvocationOperation in the AST + trips when (a) the target method implements one of the seven guarded capability interfaces (IReadable / IWritable / ITagDiscovery / ISubscribable / IHostConnectivityProbe / IAlarmSource / IHistoryProvider) AND (b) the method's return type is Task, Task<T>, ValueTask, or ValueTask<T> — the async-wire-call constraint narrows the rule to the surfaces the Phase 6.1 pipeline actually wraps + sidesteps pure in-memory accessors like IHostConnectivityProbe.GetHostStatuses() which would trigger false positives AND (c) the call does NOT sit inside a lambda argument passed to CapabilityInvoker.ExecuteAsync / ExecuteWriteAsync / AlarmSurfaceInvoker.*. The wrapper detection walks up the syntax tree from the call site, finds any enclosing InvocationExpressionSyntax whose method's containing type is one of the wrapper classes, + verifies the call lives transitively inside that invocation's AnonymousFunctionExpressionSyntax argument — a sibling "result = await driver.ReadAsync(...)" followed by a separate invoker.ExecuteAsync(...) call does NOT satisfy the wrapping rule + the analyzer flags it (regression guard in the 5th test). Five xunit-v3 + Shouldly tests at tests/ZB.MOM.WW.OtOpcUa.Analyzers.Tests: direct ReadAsync in server namespace trips; wrapped ReadAsync inside CapabilityInvoker.ExecuteAsync lambda passes; direct WriteAsync trips; direct DiscoverAsync trips; sneaky pattern — read outside the lambda + ExecuteAsync with unrelated lambda nearby — still trips. Hand-rolled test harness compiles a stub-plus-user snippet via CSharpCompilation.WithAnalyzers + runs GetAnalyzerDiagnosticsAsync directly, deliberately avoiding Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.XUnit because that package pins to xunit v2 + this repo is on xunit.v3 everywhere else. RS2008 release-tracking noise suppressed by adding AnalyzerReleases.Shipped.md + AnalyzerReleases.Unshipped.md as AdditionalFiles, which is the canonical Roslyn-analyzer hygiene path. Analyzer DLL referenced from Server.csproj via ProjectReference with OutputItemType=Analyzer + ReferenceOutputAssembly=false — the DLL ships as a compiler plugin, not a runtime dependency. Server build validates clean: the analyzer activates on every Server file but finds zero violations, which confirms the Phase 6.1 wrapping work done in prior PRs is complete + the analyzer is now the regression guard preventing the next new capability surface from being added raw. slnx updated with both the src + tests project entries. Full solution build clean, analyzer suite 5/5 passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 00:52:40 -04:00
parent 74067e7d7e
commit 5c0d3154c1
8 changed files with 404 additions and 0 deletions
--- a/src/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs
+++ b/src/ZB.MOM.WW.OtOpcUa.Analyzers/UnwrappedCapabilityCallAnalyzer.cs
@@ -0,0 +1,143 @@
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace ZB.MOM.WW.OtOpcUa.Analyzers;
+
+/// <summary>
+///     Diagnostic analyzer that flags direct invocations of Phase 6.1-wrapped driver-capability
+///     methods when the call is NOT already running inside a <c>CapabilityInvoker.ExecuteAsync</c>,
+///     <c>CapabilityInvoker.ExecuteWriteAsync</c>, or <c>AlarmSurfaceInvoker.*Async</c> lambda.
+///     The wrapping is what gives us per-host breaker isolation, retry semantics, bulkhead-depth
+///     accounting, and alarm-ack idempotence guards — raw calls bypass all of that.
+/// </summary>
+/// <remarks>
+///     The analyzer matches by receiver-interface identity using Roslyn's semantic model, not by
+///     method name, so a driver with an unusually-named method implementing <c>IReadable.ReadAsync</c>
+///     still trips the rule. Lambda-context detection walks up the syntax tree from the call site
+///     + checks whether any enclosing <c>InvocationExpressionSyntax</c> targets a member whose
+///     containing type is <c>CapabilityInvoker</c> or <c>AlarmSurfaceInvoker</c>. The rule is
+///     intentionally narrow: it does NOT try to enforce the capability argument matches the
+///     method (e.g. ReadAsync wrapped in <c>ExecuteAsync(DriverCapability.Write, ...)</c> still
+///     passes) — that'd require flow analysis beyond single-expression scope.
+/// </remarks>
+[DiagnosticAnalyzer(Microsoft.CodeAnalysis.LanguageNames.CSharp)]
+public sealed class UnwrappedCapabilityCallAnalyzer : DiagnosticAnalyzer
+{
+    public const string DiagnosticId = "OTOPCUA0001";
+
+    /// <summary>Interfaces whose methods must be called through the capability invoker.</summary>
+    private static readonly string[] GuardedInterfaces =
+    [
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.IReadable",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.IWritable",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.ITagDiscovery",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.ISubscribable",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.IHostConnectivityProbe",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.IAlarmSource",
+        "ZB.MOM.WW.OtOpcUa.Core.Abstractions.IHistoryProvider",
+    ];
+
+    /// <summary>Wrapper types whose lambda arguments are the allowed home for guarded calls.</summary>
+    private static readonly string[] WrapperTypes =
+    [
+        "ZB.MOM.WW.OtOpcUa.Core.Resilience.CapabilityInvoker",
+        "ZB.MOM.WW.OtOpcUa.Core.Resilience.AlarmSurfaceInvoker",
+    ];
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: DiagnosticId,
+        title: "Driver capability call must be wrapped in CapabilityInvoker",
+        messageFormat: "Call to '{0}' is not wrapped in CapabilityInvoker.ExecuteAsync / ExecuteWriteAsync / AlarmSurfaceInvoker.*. Without the wrapping, Phase 6.1 resilience (retry, breaker, bulkhead, tracker telemetry) is bypassed for this call.",
+        category: "OtOpcUa.Resilience",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: "Phase 6.1 Stream A requires every IReadable/IWritable/ITagDiscovery/ISubscribable/IHostConnectivityProbe/IAlarmSource/IHistoryProvider call to route through the shared Polly pipeline. Direct calls skip the pipeline + lose per-host isolation, retry semantics, and telemetry. If the caller is Core/Server/Driver dispatch code, wrap the call in CapabilityInvoker.ExecuteAsync. If the caller is a unit test invoking the driver directly to test its wire-level behavior, either suppress with a pragma or move the suppression into a NoWarn for the test project.");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation);
+    }
+
+    private static void AnalyzeInvocation(OperationAnalysisContext context)
+    {
+        var invocation = (Microsoft.CodeAnalysis.Operations.IInvocationOperation)context.Operation;
+        var method = invocation.TargetMethod;
+
+        // Narrow the rule to async wire calls. Synchronous accessors like
+        // IHostConnectivityProbe.GetHostStatuses() are pure in-memory snapshots + would never
+        // benefit from the Polly pipeline; flagging them just creates false-positives.
+        if (!IsAsyncReturningType(method.ReturnType)) return;
+        if (!ImplementsGuardedInterface(method)) return;
+        if (IsInsideWrapperLambda(invocation.Syntax, context.Operation.SemanticModel, context.CancellationToken)) return;
+
+        var diag = Diagnostic.Create(Rule, invocation.Syntax.GetLocation(), $"{method.ContainingType.Name}.{method.Name}");
+        context.ReportDiagnostic(diag);
+    }
+
+    private static bool IsAsyncReturningType(ITypeSymbol type)
+    {
+        var name = type.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
+        return name is "global::System.Threading.Tasks.Task"
+            or "global::System.Threading.Tasks.Task<TResult>"
+            or "global::System.Threading.Tasks.ValueTask"
+            or "global::System.Threading.Tasks.ValueTask<TResult>";
+    }
+
+    private static bool ImplementsGuardedInterface(IMethodSymbol method)
+    {
+        foreach (var iface in method.ContainingType.AllInterfaces.Concat(new[] { method.ContainingType }))
+        {
+            var ifaceFqn = iface.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)
+                .Replace("global::", string.Empty);
+            if (!GuardedInterfaces.Contains(ifaceFqn)) continue;
+
+            foreach (var member in iface.GetMembers().OfType<IMethodSymbol>())
+            {
+                var impl = method.ContainingType.FindImplementationForInterfaceMember(member);
+                if (SymbolEqualityComparer.Default.Equals(impl, method) ||
+                    SymbolEqualityComparer.Default.Equals(method.OriginalDefinition, member))
+                    return true;
+            }
+        }
+        return false;
+    }
+
+    private static bool IsInsideWrapperLambda(SyntaxNode startNode, SemanticModel? semanticModel, System.Threading.CancellationToken ct)
+    {
+        if (semanticModel is null) return false;
+
+        for (var node = startNode.Parent; node is not null; node = node.Parent)
+        {
+            // We only care about an enclosing invocation — the call we're auditing must literally
+            // live inside a lambda (ParenthesizedLambda / SimpleLambda / AnonymousMethod) that is
+            // an argument of a CapabilityInvoker.Execute* / AlarmSurfaceInvoker.* call.
+            if (node is not InvocationExpressionSyntax outer) continue;
+
+            var sym = semanticModel.GetSymbolInfo(outer, ct).Symbol as IMethodSymbol;
+            if (sym is null) continue;
+
+            var outerTypeFqn = sym.ContainingType.OriginalDefinition.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)
+                .Replace("global::", string.Empty);
+            if (!WrapperTypes.Contains(outerTypeFqn)) continue;
+
+            // The call is wrapped IFF our startNode is transitively inside one of the outer
+            // call's argument lambdas. Walk the outer invocation's argument list + check whether
+            // any lambda body contains the startNode's position.
+            foreach (var arg in outer.ArgumentList.Arguments)
+            {
+                if (arg.Expression is not AnonymousFunctionExpressionSyntax lambda) continue;
+                if (lambda.Span.Contains(startNode.Span)) return true;
+            }
+        }
+        return false;
+    }
+}