fix(template): M2.7 review nits — comment-aware arg tokenizer + stricter numeric-literal inference (#20/#21)
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.
This commit is contained in:
@@ -612,6 +612,28 @@ public class SemanticValidator
|
|||||||
pos++;
|
pos++;
|
||||||
}
|
}
|
||||||
break;
|
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++;
|
pos++;
|
||||||
}
|
}
|
||||||
@@ -844,6 +866,20 @@ public class SemanticValidator
|
|||||||
if (expr.Length == 0) return false;
|
if (expr.Length == 0) return false;
|
||||||
if (expr[0] == '+' || expr[0] == '-') i++;
|
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 sawDigit = false;
|
||||||
var sawDot = false;
|
var sawDot = false;
|
||||||
var sawExp = false;
|
var sawExp = false;
|
||||||
@@ -851,7 +887,7 @@ public class SemanticValidator
|
|||||||
{
|
{
|
||||||
var c = expr[i];
|
var c = expr[i];
|
||||||
if (char.IsDigit(c)) { sawDigit = true; continue; }
|
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 == '.' && !sawDot && !sawExp) { sawDot = true; isFloat = true; continue; }
|
||||||
if ((c == 'e' || c == 'E') && !sawExp && sawDigit)
|
if ((c == 'e' || c == 'E') && !sawExp && sawDigit)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -968,4 +968,92 @@ public class SemanticValidatorTests
|
|||||||
Assert.DoesNotContain(result.Warnings,
|
Assert.DoesNotContain(result.Warnings,
|
||||||
w => w.Category == ValidationCategory.TriggerOperandType);
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user