feat(adminui): script diagnostics (Roslyn + forbidden-type + dynamic-path)

2026-06-09 14:22:49 -04:00
parent b54a6ad29f
commit 6a9b052fc7
2 changed files with 124 additions and 1 deletions
@@ -1,5 +1,6 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.Extensions.Logging;
 using ZB.MOM.WW.OtOpcUa.Core.Scripting;
 using ZB.MOM.WW.OtOpcUa.Core.VirtualTags;

@@ -27,6 +28,9 @@ public sealed class ScriptAnalysisService
        OutputKind.DynamicallyLinkedLibrary, optimizationLevel: OptimizationLevel.Release,
        allowUnsafe: false, warningLevel: 4, nullableContextOptions: NullableContextOptions.Enable);

+    private readonly ILogger<ScriptAnalysisService>? _logger;
+    public ScriptAnalysisService(ILogger<ScriptAnalysisService>? logger = null) => _logger = logger;
+
    // Mirrors ScriptEvaluator's wrapper EXACTLY (usings from the sandbox + the
    // Run(ScriptGlobals<VirtualTagContext>) shape), except the analysis return type is
    // `object` so a shared script stays valid regardless of any virtual tag's DataType.
@@ -67,7 +71,76 @@ public sealed class ScriptAnalysisService
        return code.Length;
    }

-    public DiagnoseResponse Diagnose(DiagnoseRequest req) => new(Array.Empty<DiagnosticMarker>());          // Task 3
+    private static string Normalize(string? s) => (s ?? "").Replace("\r\n", "\n").Replace("\r", "\n");
+
+    public DiagnoseResponse Diagnose(DiagnoseRequest req)
+    {
+        if (string.IsNullOrEmpty(req.Code)) return new DiagnoseResponse(Array.Empty<DiagnosticMarker>());
+        try
+        {
+            var code = Normalize(req.Code);
+            var (tree, compilation, _, preambleLength) = Analyze(code);
+            var markers = new List<DiagnosticMarker>();
+
+            // (1) Roslyn — surface ONLY compile errors, the same severity that actually blocks a
+            // publish in ScriptEvaluator.Compile (it filters GetDiagnostics() to
+            // DiagnosticSeverity.Error). Mirroring that here keeps editor markers in lockstep with
+            // what fails publish: a nullable warning like CS8605 on the canonical passthrough shape
+            // `return ctx.GetTag("X").Value;` compiles + publishes fine under the evaluator's
+            // nullable-enabled options, so flagging it in the editor would be misleading noise.
+            // Restricted to diagnostics physically inside the user source — #line 1 in the preamble
+            // makes GetMappedLineSpan report user-source coords (GetLineSpan would return the
+            // physical wrapper line and be wrong); the preambleLength guard also drops phantom
+            // wrapper diagnostics like CS0161 on Run() when the user hasn't typed `return` yet.
+            foreach (var d in compilation.GetDiagnostics())
+            {
+                if (d.Severity != DiagnosticSeverity.Error || !d.Location.IsInSource) continue;
+                if (d.Location.SourceSpan.Start < preambleLength) continue;
+                var span = d.Location.GetMappedLineSpan();          // honors #line 1 → user coords
+                markers.Add(ToMarker(span, Sev(d.Severity), d.GetMessage(), d.Id));
+            }
+            // (2) ForbiddenTypeAnalyzer — spans are in the wrapped tree; map via #line.
+            foreach (var r in ForbiddenTypeAnalyzer.Analyze(compilation))
+                markers.Add(ToMarker(tree.GetMappedLineSpan(r.Span), 8, r.Message, "OTSCRIPT_FORBIDDEN"));
+            // (3) DependencyExtractor — spans are offsets in the normalized user source; map directly.
+            foreach (var r in DependencyExtractor.Extract(code).Rejections)
+                markers.Add(ToUserMarker(code, r.Span, r.Message, "OTSCRIPT_DYNPATH"));
+
+            return new DiagnoseResponse(markers.Distinct().ToList());
+        }
+        catch (Exception ex)
+        {
+            _logger?.LogWarning(ex, "Script diagnostics failed; returning no markers.");
+            return new DiagnoseResponse(Array.Empty<DiagnosticMarker>());
+        }
+    }
+
+    private static int Sev(DiagnosticSeverity s) => s switch
+    {
+        DiagnosticSeverity.Error => 8,
+        DiagnosticSeverity.Warning => 4,
+        DiagnosticSeverity.Info => 2,
+        _ => 1,
+    };
+
+    private static DiagnosticMarker ToMarker(FileLinePositionSpan span, int sev, string msg, string code) => new(
+        sev, span.StartLinePosition.Line + 1, span.StartLinePosition.Character + 1,
+        span.EndLinePosition.Line + 1, span.EndLinePosition.Character + 1, msg, code);
+
+    private static DiagnosticMarker ToUserMarker(string userSource, Microsoft.CodeAnalysis.Text.TextSpan span, string msg, string code)
+    {
+        var (sl, sc) = LineCol(userSource, span.Start);
+        var (el, ec) = LineCol(userSource, span.End);
+        return new DiagnosticMarker(8, sl, sc, el, ec, msg, code);
+    }
+
+    private static (int Line, int Col) LineCol(string code, int offset)
+    {
+        int line = 1, col = 1;
+        for (int i = 0; i < offset && i < code.Length; i++)
+            if (code[i] == '\n') { line++; col = 1; } else col++;
+        return (line, col);
+    }
    public Task<CompletionsResponse> CompleteAsync(CompletionsRequest req)
        => Task.FromResult(new CompletionsResponse(Array.Empty<CompletionItem>()));                          // Tasks 4,6
    public HoverResponse Hover(HoverRequest req) => new((string?)null);                                      // Task 7
@@ -0,0 +1,50 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis;
+
+namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis;
+
+public sealed class DiagnoseTests
+{
+    private static readonly ScriptAnalysisService Svc = new();
+    private static IReadOnlyList<DiagnosticMarker> Diag(string code) => Svc.Diagnose(new DiagnoseRequest(code)).Markers;
+
+    [Fact] public void Clean_script_has_no_markers()
+        => Diag("""return (double)ctx.GetTag("A").Value * 2.0;""").ShouldBeEmpty();
+
+    [Fact] public void Roslyn_error_is_reported_on_user_line_1()
+    {
+        var m = Diag("return ctx.NoSuchMember();");
+        m.ShouldNotBeEmpty();
+        m.ShouldContain(x => x.Severity == 8);
+        m.First(x => x.Severity == 8).StartLineNumber.ShouldBe(1);   // #line 1 → user coords (GetMappedLineSpan)
+    }
+
+    [Fact] public void Roslyn_error_maps_to_correct_user_line()
+    {
+        // error on the THIRD user line — proves mapping isn't off by the preamble height
+        var m = Diag("var a = 1;\nvar b = 2;\nreturn ctx.Nope();");
+        m.ShouldContain(x => x.Severity == 8 && x.StartLineNumber == 3);
+    }
+
+    [Fact] public void Forbidden_or_unresolved_dangerous_type_is_flagged()
+        => Diag("""var c = new System.Net.Http.HttpClient(); return 0;""")
+            .ShouldContain(x => x.Severity == 8);
+
+    [Fact] public void Dynamic_tag_path_is_flagged()
+        => Diag("""var p = "A"; return ctx.GetTag(p).Value;""")
+            .ShouldContain(x => x.Message.Contains("literal"));
+
+    [Fact] public void Missing_return_does_not_produce_a_phantom_preamble_marker()
+        => Diag("var x = 1;").ShouldNotContain(m => m.Code == "CS0161");
+
+    [Fact] public void Empty_code_has_no_markers()
+        => Diag("").ShouldBeEmpty();
+
+    [Fact] public void Crlf_source_maps_dynamic_path_to_the_correct_line()
+    {
+        // \r\n line endings; the dynamic-path rejection must land on user line 3, not be column-shifted.
+        var m = Diag("var a = 1;\r\nvar b = 2;\r\nreturn ctx.GetTag(b > 0 ? \"x\" : \"y\").Value;");
+        m.ShouldContain(x => x.Message.Contains("literal") && x.StartLineNumber == 3);
+    }
+}