From 5573a9eeb95ac8b13f474fd763c47f3a40f5f1d5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 09:45:04 -0400 Subject: [PATCH] fix(template-engine): trigger-expression syntax check reports ALL violations/compile errors, not just the first (plan R2-05 T1) --- .../Validation/ValidationService.cs | 8 +++++--- .../Validation/ScriptCompilerTests.cs | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs index c29b3053..48ee0706 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs @@ -530,15 +530,17 @@ public class ValidationService /// A human-readable error message if the expression is invalid; null if well-formed. internal static string? CheckExpressionSyntax(string expression) { - // Authoritative forbidden-API verdict first. + // Authoritative forbidden-API verdict first. Report ALL violations, not just + // the first, so an operator fixing one forbidden API isn't surprised by a + // second on the next deploy attempt (mirrors ScriptCompiler.TryCompile). var violations = ScriptTrustValidator.FindViolations(expression); if (violations.Count > 0) - return $"uses forbidden API: {violations[0]}"; + return $"uses forbidden API: {string.Join("; ", violations)}"; // Real compile of the bare boolean expression against the trigger globals. var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface)); if (errors.Count > 0) - return $"is not a valid expression: {errors[0]}"; + return $"is not a valid expression: {string.Join("; ", errors)}"; return null; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs index 17df857d..1bf79106 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ScriptCompilerTests.cs @@ -149,6 +149,25 @@ public class ScriptCompilerTests Assert.Contains("forbidden", error, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void CheckExpressionSyntax_MultipleForbiddenApis_ReportsAll() + { + var error = ValidationService.CheckExpressionSyntax( + "System.IO.File.Exists(\"x\") && System.Diagnostics.Process.GetProcesses().Length > 0"); + Assert.NotNull(error); + Assert.Contains("System.IO", error); // FAILS today: only violations[0] is surfaced + Assert.Contains("Process", error); + } + + [Fact] + public void CheckExpressionSyntax_MultipleCompileErrors_ReportsAll() + { + var error = ValidationService.CheckExpressionSyntax("NoSuchThingA > 1 && NoSuchThingB < 2"); + Assert.NotNull(error); + Assert.Contains("NoSuchThingA", error); // FAILS today: only errors[0] is surfaced + Assert.Contains("NoSuchThingB", error); + } + // --- Compile-verdict cache: unchanged code compiles once per process --- [Fact]