From a8e9e9952d3282c5e7619a3987142c65211c04ee Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 05:21:23 -0400 Subject: [PATCH] =?UTF-8?q?fix(template):=20M2.7=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20comment-aware=20arg=20tokenizer=20+=20stricter=20nu?= =?UTF-8?q?meric-literal=20inference=20(#20/#21)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SplitCallArguments now skips C# line (`//`) and block (`/* */`) comments when tokenizing the argument list, so a comma inside a comment no longer produces a spurious arg-count mismatch. IsNumericLiteral now explicitly rejects tokens whose first non-sign character is `_` or a letter (e.g. `_2`), and restricts underscore digit-separators to positions after at least one digit, preventing identifier-shaped tokens from being inferred as Integer/Float. --- .../Validation/SemanticValidator.cs | 38 +++++++- .../Validation/SemanticValidatorTests.cs | 88 +++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/SemanticValidator.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/SemanticValidator.cs index ced53abe..adc54210 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/SemanticValidator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/SemanticValidator.cs @@ -612,6 +612,28 @@ public class SemanticValidator pos++; } break; + case '/': + // Skip C# line and block comments so commas inside them are ignored. + // A `/` inside a string literal is already consumed above, so we only + // reach here for real `/` tokens in code. + if (pos + 1 < code.Length) + { + if (code[pos + 1] == '/') + { + // Line comment: skip to end-of-line. + pos += 2; + while (pos < code.Length && code[pos] != '\n') pos++; + } + else if (code[pos + 1] == '*') + { + // Block comment: skip to closing `*/`. + pos += 2; + while (pos + 1 < code.Length && !(code[pos] == '*' && code[pos + 1] == '/')) + pos++; + if (pos + 1 < code.Length) pos++; // step over the `/` + } + } + break; } pos++; } @@ -844,6 +866,20 @@ public class SemanticValidator if (expr.Length == 0) return false; if (expr[0] == '+' || expr[0] == '-') i++; + // A genuine numeric literal must start with a digit or a `.` followed by a + // digit. Identifiers that start with `_` or a letter (e.g. `_2`, `count`) + // are explicitly rejected here so they are inferred as Unknown, not Integer. + if (i >= expr.Length) return false; + var first = expr[i]; + if (first == '.') + { + if (i + 1 >= expr.Length || !char.IsDigit(expr[i + 1])) return false; + } + else if (!char.IsDigit(first)) + { + return false; // starts with `_`, letter, or anything else → not a literal + } + var sawDigit = false; var sawDot = false; var sawExp = false; @@ -851,7 +887,7 @@ public class SemanticValidator { var c = expr[i]; if (char.IsDigit(c)) { sawDigit = true; continue; } - if (c == '_') continue; // digit separator + if (c == '_' && sawDigit) continue; // digit separator — only valid between digits if (c == '.' && !sawDot && !sawExp) { sawDot = true; isFloat = true; continue; } if ((c == 'e' || c == 'E') && !sawExp && sawDigit) { diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/SemanticValidatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/SemanticValidatorTests.cs index 5c061091..672090bb 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/SemanticValidatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/SemanticValidatorTests.cs @@ -968,4 +968,92 @@ public class SemanticValidatorTests Assert.DoesNotContain(result.Warnings, w => w.Category == ValidationCategory.TriggerOperandType); } + + // ── M2.7 review nits — comment-aware arg tokenizer ───────────────────── + + [Fact] + public void Validate_ArgSplit_LineCommentWithCommaInsideArgs_NoFalsePositive() + { + // A `//` line comment containing a comma must NOT be counted as an arg separator. + // "Target" expects (a: Integer) — one real arg; the comment comma is noise. + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Scripts = + [ + new ResolvedScript + { + CanonicalName = "Target", + Code = "var x = 1;", + ParameterDefinitions = "[{\"name\":\"a\",\"type\":\"Integer\"}]" + }, + new ResolvedScript + { + CanonicalName = "Caller", + Code = "CallScript(\"Target\", 42 /* , extra */);" + } + ] + }; + + var result = _sut.Validate(config); + Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.ParameterMismatch); + } + + [Fact] + public void Validate_ArgSplit_BlockCommentWithCommaInsideArgs_NoFalsePositive() + { + // A `/* */` block comment containing a comma must NOT be counted as an arg separator. + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Scripts = + [ + new ResolvedScript + { + CanonicalName = "Target", + Code = "var x = 1;", + ParameterDefinitions = "[{\"name\":\"a\",\"type\":\"Integer\"},{\"name\":\"b\",\"type\":\"String\"}]" + }, + new ResolvedScript + { + CanonicalName = "Caller", + // Two real args, but the block comment adds a spurious comma if tokenizer is not comment-aware. + Code = "CallScript(\"Target\", 42 /* ,bogus */, \"hi\");" + } + ] + }; + + var result = _sut.Validate(config); + Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.ParameterMismatch); + } + + // ── M2.7 review nits — stricter numeric-literal inference ─────────────── + + [Fact] + public void Validate_ArgumentType_UnderscoreLeadingIdentifier_NoFalsePositive() + { + // `_2` starts with an underscore — it is a C# identifier, not a numeric literal. + // IsNumericLiteral must return false → type inferred as Unknown → no mismatch. + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Scripts = + [ + new ResolvedScript + { + CanonicalName = "Target", + Code = "var x = 1;", + ParameterDefinitions = "[{\"name\":\"a\",\"type\":\"Integer\"}]" + }, + new ResolvedScript + { + CanonicalName = "Caller", + Code = "CallScript(\"Target\", _2);" + } + ] + }; + + var result = _sut.Validate(config); + Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.ParameterMismatch); + } }