Skip to content

Commit 8c82b7b

Browse files
committed
Add format string validation in expressions package
Format string validation now happens at parse time in the Parser: - Invalid format string syntax throws ErrorInvalidFormatString - Argument count mismatch throws ErrorFormatArgCountMismatch This catches errors early and automatically for all consumers.
1 parent dbf7752 commit 8c82b7b

File tree

7 files changed

+445
-38
lines changed

7 files changed

+445
-38
lines changed

expressions/src/errors.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,18 @@ export enum ErrorType {
1313
ErrorTooFewParameters,
1414
ErrorTooManyParameters,
1515
ErrorUnrecognizedContext,
16-
ErrorUnrecognizedFunction
16+
ErrorUnrecognizedFunction,
17+
ErrorInvalidFormatString,
18+
ErrorFormatArgCountMismatch
1719
}
1820

1921
export class ExpressionError extends Error {
20-
constructor(private typ: ErrorType, private tok: Token) {
21-
super(`${errorDescription(typ)}: '${tokenString(tok)}'`);
22+
constructor(
23+
private typ: ErrorType,
24+
private tok: Token,
25+
customMessage?: string
26+
) {
27+
super(customMessage ?? `${errorDescription(typ)}: '${tokenString(tok)}'`);
2228

2329
this.pos = this.tok.range.start;
2430
}
@@ -46,6 +52,10 @@ function errorDescription(typ: ErrorType): string {
4652
return "Unrecognized named-value";
4753
case ErrorType.ErrorUnrecognizedFunction:
4854
return "Unrecognized function";
55+
case ErrorType.ErrorInvalidFormatString:
56+
return "Invalid format string";
57+
case ErrorType.ErrorFormatArgCountMismatch:
58+
return "Format string argument count mismatch";
4959
default: // Should never reach here.
5060
return "Unknown error";
5161
}

expressions/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ export {Expr} from "./ast.js";
22
export {complete, CompletionItem} from "./completion.js";
33
export {DescriptionDictionary, DescriptionPair, isDescriptionDictionary} from "./completion/descriptionDictionary.js";
44
export * as data from "./data/index.js";
5-
export {ExpressionError, ExpressionEvaluationError} from "./errors.js";
5+
export {ErrorType, ExpressionError, ExpressionEvaluationError} from "./errors.js";
66
export {Evaluator} from "./evaluator.js";
77
export {ExperimentalFeatureKey, ExperimentalFeatures, FeatureFlags} from "./features.js";
88
export {wellKnownFunctions} from "./funcs.js";
99
export {Lexer, Result} from "./lexer.js";
1010
export {Parser} from "./parser.js";
11+
export {validateFormatString} from "./validate-format.js";

expressions/src/parser.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {ErrorType, ExpressionError, MAX_PARSER_DEPTH} from "./errors.js";
1515
import {ParseContext, validateFunction} from "./funcs.js";
1616
import {FunctionInfo} from "./funcs/info.js";
1717
import {Token, TokenType} from "./lexer.js";
18+
import {validateFormatString} from "./validate-format.js";
1819

1920
export class Parser {
2021
private extContexts: Map<string, boolean>;
@@ -261,6 +262,30 @@ export class Parser {
261262

262263
validateFunction(this.context, identifier, args.length);
263264

265+
// Validate format() calls
266+
if (identifier.lexeme.toLowerCase() === "format" && args.length > 0) {
267+
const firstArg = args[0];
268+
if (firstArg instanceof Literal && firstArg.literal.kind === data.Kind.String) {
269+
const formatString = firstArg.literal.coerceString();
270+
const result = validateFormatString(formatString);
271+
272+
if (!result.valid) {
273+
throw new ExpressionError(ErrorType.ErrorInvalidFormatString, identifier);
274+
}
275+
276+
// Check argument count: format string uses {0} to {N}, so need N+1 args after format string
277+
const providedArgs = args.length - 1;
278+
const requiredArgs = result.maxArgIndex + 1;
279+
if (requiredArgs > providedArgs) {
280+
throw new ExpressionError(
281+
ErrorType.ErrorFormatArgCountMismatch,
282+
identifier,
283+
`Format string references {${result.maxArgIndex}} but only ${providedArgs} argument(s) provided`
284+
);
285+
}
286+
}
287+
}
288+
264289
return new FunctionCall(identifier, args);
265290
}
266291

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import {validateFormatString} from "./validate-format.js";
2+
3+
describe("validateFormatString", () => {
4+
it("returns valid for simple placeholder", () => {
5+
const result = validateFormatString("{0}");
6+
expect(result).toEqual({valid: true, maxArgIndex: 0});
7+
});
8+
9+
it("returns valid for multiple placeholders", () => {
10+
const result = validateFormatString("{0} {1} {2}");
11+
expect(result).toEqual({valid: true, maxArgIndex: 2});
12+
});
13+
14+
it("returns valid for text with placeholder", () => {
15+
const result = validateFormatString("hello {0} world");
16+
expect(result).toEqual({valid: true, maxArgIndex: 0});
17+
});
18+
19+
it("returns valid for escaped left braces", () => {
20+
const result = validateFormatString("{{0}} {0}");
21+
expect(result).toEqual({valid: true, maxArgIndex: 0});
22+
});
23+
24+
it("returns valid for escaped right braces", () => {
25+
const result = validateFormatString("{0}}}");
26+
expect(result).toEqual({valid: true, maxArgIndex: 0});
27+
});
28+
29+
it("returns valid for no placeholders", () => {
30+
const result = validateFormatString("hello world");
31+
expect(result).toEqual({valid: true, maxArgIndex: -1});
32+
});
33+
34+
it("returns invalid for missing closing brace", () => {
35+
const result = validateFormatString("{0");
36+
expect(result).toEqual({valid: false, maxArgIndex: -1});
37+
});
38+
39+
it("returns invalid for empty placeholder", () => {
40+
const result = validateFormatString("{}");
41+
expect(result).toEqual({valid: false, maxArgIndex: -1});
42+
});
43+
44+
it("returns invalid for non-numeric placeholder", () => {
45+
const result = validateFormatString("{abc}");
46+
expect(result).toEqual({valid: false, maxArgIndex: -1});
47+
});
48+
49+
it("returns invalid for unescaped closing brace", () => {
50+
const result = validateFormatString("text } more");
51+
expect(result).toEqual({valid: false, maxArgIndex: -1});
52+
});
53+
54+
it("handles out-of-order placeholders", () => {
55+
const result = validateFormatString("{2} {0} {1}");
56+
expect(result).toEqual({valid: true, maxArgIndex: 2});
57+
});
58+
59+
it("handles repeated placeholders", () => {
60+
const result = validateFormatString("{0} {0} {0}");
61+
expect(result).toEqual({valid: true, maxArgIndex: 0});
62+
});
63+
});

expressions/src/validate-format.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Format string validation for format() function calls.
3+
* Validates format string syntax and argument count at parse time.
4+
*/
5+
6+
/**
7+
* Validates a format string and returns the maximum placeholder index.
8+
*
9+
* @param formatString The format string to validate
10+
* @returns { valid: boolean, maxArgIndex: number } where maxArgIndex is -1 if no placeholders
11+
*/
12+
export function validateFormatString(formatString: string): {valid: boolean; maxArgIndex: number} {
13+
let maxIndex = -1;
14+
let i = 0;
15+
16+
while (i < formatString.length) {
17+
// Find next left brace
18+
let lbrace = -1;
19+
for (let j = i; j < formatString.length; j++) {
20+
if (formatString[j] === "{") {
21+
lbrace = j;
22+
break;
23+
}
24+
}
25+
26+
// Find next right brace
27+
let rbrace = -1;
28+
for (let j = i; j < formatString.length; j++) {
29+
if (formatString[j] === "}") {
30+
rbrace = j;
31+
break;
32+
}
33+
}
34+
35+
// No more braces
36+
if (lbrace < 0 && rbrace < 0) {
37+
break;
38+
}
39+
40+
// Left brace comes first (or only left brace exists)
41+
if (lbrace >= 0 && (rbrace < 0 || lbrace < rbrace)) {
42+
// Check if it's escaped
43+
if (lbrace + 1 < formatString.length && formatString[lbrace + 1] === "{") {
44+
// Escaped left brace
45+
i = lbrace + 2;
46+
continue;
47+
}
48+
49+
// This is a placeholder opening - find the closing brace
50+
rbrace = -1;
51+
for (let j = lbrace + 1; j < formatString.length; j++) {
52+
if (formatString[j] === "}") {
53+
rbrace = j;
54+
break;
55+
}
56+
}
57+
58+
if (rbrace < 0) {
59+
// Missing closing brace
60+
return {valid: false, maxArgIndex: -1};
61+
}
62+
63+
// Validate placeholder content (must be digits only)
64+
if (rbrace === lbrace + 1) {
65+
// Empty placeholder {}
66+
return {valid: false, maxArgIndex: -1};
67+
}
68+
69+
// Parse the index and validate it's all digits
70+
let index = 0;
71+
for (let j = lbrace + 1; j < rbrace; j++) {
72+
const c = formatString[j];
73+
if (c < "0" || c > "9") {
74+
// Non-numeric character
75+
return {valid: false, maxArgIndex: -1};
76+
}
77+
index = index * 10 + (c.charCodeAt(0) - "0".charCodeAt(0));
78+
}
79+
80+
if (index > maxIndex) {
81+
maxIndex = index;
82+
}
83+
84+
i = rbrace + 1;
85+
continue;
86+
}
87+
88+
// Right brace comes first (or only right brace exists)
89+
// Check if it's escaped
90+
if (rbrace + 1 < formatString.length && formatString[rbrace + 1] === "}") {
91+
// Escaped right brace
92+
i = rbrace + 2;
93+
continue;
94+
}
95+
96+
// Unescaped right brace outside of placeholder
97+
return {valid: false, maxArgIndex: -1};
98+
}
99+
100+
return {valid: true, maxArgIndex: maxIndex};
101+
}

0 commit comments

Comments
 (0)