Add format string validation (#292)
Validates format() function calls for: - Invalid syntax (missing closing brace, empty placeholder, non-numeric placeholder) - Argument count mismatch (placeholder references arg that doesn't exist) Port of Go's format_validator.go from actions-workflow-parser.
This commit is contained in:
eric sciple
@@ -0,0 +1,199 @@
|
||||
/**
|
||||
* Format string validation for format() function calls.
|
||||
* Port of Go's format_validator.go from actions-workflow-parser.
|
||||
*/
|
||||
|
||||
import {Expr, FunctionCall, Literal, Binary, Unary, Logical, Grouping, IndexAccess} from "@actions/expressions/ast";
|
||||
import {Kind} from "@actions/expressions/data/expressiondata";
|
||||
|
||||
/**
|
||||
* Error types for format string validation
|
||||
*/
|
||||
export type FormatStringError =
|
||||
| {type: "invalid-syntax"; message: string}
|
||||
| {type: "arg-count-mismatch"; expected: number; provided: number};
|
||||
|
||||
/**
|
||||
* Validates a format string and returns the maximum placeholder index.
|
||||
* Port of Go's validateFormatString from format_validator.go.
|
||||
*
|
||||
* @param formatString The format string to validate
|
||||
* @returns { valid: boolean, maxArgIndex: number } where maxArgIndex is -1 if no placeholders
|
||||
*/
|
||||
export function validateFormatString(formatString: string): {valid: boolean; maxArgIndex: number} {
|
||||
let maxIndex = -1;
|
||||
let i = 0;
|
||||
|
||||
while (i < formatString.length) {
|
||||
// Find next left brace
|
||||
let lbrace = -1;
|
||||
for (let j = i; j < formatString.length; j++) {
|
||||
if (formatString[j] === "{") {
|
||||
lbrace = j;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Find next right brace
|
||||
let rbrace = -1;
|
||||
for (let j = i; j < formatString.length; j++) {
|
||||
if (formatString[j] === "}") {
|
||||
rbrace = j;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// No more braces
|
||||
if (lbrace < 0 && rbrace < 0) {
|
||||
break;
|
||||
}
|
||||
|
||||
// Left brace comes first (or only left brace exists)
|
||||
if (lbrace >= 0 && (rbrace < 0 || lbrace < rbrace)) {
|
||||
// Check if it's escaped
|
||||
if (lbrace + 1 < formatString.length && formatString[lbrace + 1] === "{") {
|
||||
// Escaped left brace
|
||||
i = lbrace + 2;
|
||||
continue;
|
||||
}
|
||||
|
||||
// This is a placeholder opening - find the closing brace
|
||||
rbrace = -1;
|
||||
for (let j = lbrace + 1; j < formatString.length; j++) {
|
||||
if (formatString[j] === "}") {
|
||||
rbrace = j;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (rbrace < 0) {
|
||||
// Missing closing brace
|
||||
return {valid: false, maxArgIndex: -1};
|
||||
}
|
||||
|
||||
// Validate placeholder content (must be digits only)
|
||||
if (rbrace === lbrace + 1) {
|
||||
// Empty placeholder {}
|
||||
return {valid: false, maxArgIndex: -1};
|
||||
}
|
||||
|
||||
// Parse the index and validate it's all digits
|
||||
let index = 0;
|
||||
for (let j = lbrace + 1; j < rbrace; j++) {
|
||||
const c = formatString[j];
|
||||
if (c < "0" || c > "9") {
|
||||
// Non-numeric character
|
||||
return {valid: false, maxArgIndex: -1};
|
||||
}
|
||||
index = index * 10 + (c.charCodeAt(0) - "0".charCodeAt(0));
|
||||
}
|
||||
|
||||
if (index > maxIndex) {
|
||||
maxIndex = index;
|
||||
}
|
||||
|
||||
i = rbrace + 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Right brace comes first (or only right brace exists)
|
||||
// Check if it's escaped
|
||||
if (rbrace + 1 < formatString.length && formatString[rbrace + 1] === "}") {
|
||||
// Escaped right brace
|
||||
i = rbrace + 2;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Unescaped right brace outside of placeholder
|
||||
return {valid: false, maxArgIndex: -1};
|
||||
}
|
||||
|
||||
return {valid: true, maxArgIndex: maxIndex};
|
||||
}
|
||||
|
||||
/**
|
||||
* Walks an expression AST to find and validate all format() function calls.
|
||||
*
|
||||
* @param expr The expression AST to validate
|
||||
* @returns Array of validation errors found
|
||||
*/
|
||||
export function validateFormatCalls(expr: Expr): FormatStringError[] {
|
||||
const errors: FormatStringError[] = [];
|
||||
const stack: Expr[] = [expr];
|
||||
|
||||
while (stack.length > 0) {
|
||||
const node = stack.pop();
|
||||
if (!node) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (node instanceof FunctionCall) {
|
||||
if (node.functionName.lexeme.toLowerCase() === "format") {
|
||||
const error = validateSingleFormatCall(node);
|
||||
if (error) {
|
||||
errors.push(error);
|
||||
}
|
||||
}
|
||||
// Push args for further processing (to find nested format calls)
|
||||
for (const arg of node.args) {
|
||||
stack.push(arg);
|
||||
}
|
||||
} else if (node instanceof Binary) {
|
||||
stack.push(node.left, node.right);
|
||||
} else if (node instanceof Unary) {
|
||||
stack.push(node.expr);
|
||||
} else if (node instanceof Logical) {
|
||||
for (const arg of node.args) {
|
||||
stack.push(arg);
|
||||
}
|
||||
} else if (node instanceof Grouping) {
|
||||
stack.push(node.group);
|
||||
} else if (node instanceof IndexAccess) {
|
||||
stack.push(node.expr, node.index);
|
||||
}
|
||||
// Literal, ContextAccess - no children to process
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a single format() function call.
|
||||
*
|
||||
* @param fc The FunctionCall AST node
|
||||
* @returns Validation error if found, undefined if valid
|
||||
*/
|
||||
function validateSingleFormatCall(fc: FunctionCall): FormatStringError | undefined {
|
||||
// Must have at least one argument (the format string)
|
||||
if (fc.args.length < 1) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// First argument must be a string literal
|
||||
const firstArg = fc.args[0];
|
||||
if (!(firstArg instanceof Literal) || firstArg.literal.kind !== Kind.String) {
|
||||
return undefined; // Can't validate dynamic format strings
|
||||
}
|
||||
|
||||
const formatString = firstArg.literal.coerceString();
|
||||
const numArgs = fc.args.length - 1; // Subtract 1 for format string itself
|
||||
|
||||
const {valid, maxArgIndex} = validateFormatString(formatString);
|
||||
|
||||
if (!valid) {
|
||||
return {
|
||||
type: "invalid-syntax",
|
||||
message: "Format string has invalid syntax (missing closing brace, unescaped braces, or invalid placeholder)"
|
||||
};
|
||||
}
|
||||
|
||||
if (maxArgIndex >= numArgs) {
|
||||
return {
|
||||
type: "arg-count-mismatch",
|
||||
expected: maxArgIndex + 1, // Convert 0-based index to count
|
||||
provided: numArgs
|
||||
};
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
@@ -0,0 +1,273 @@
|
||||
import {DiagnosticSeverity} from "vscode-languageserver-types";
|
||||
import {createDocument} from "./test-utils/document.js";
|
||||
import {validate} from "./validate.js";
|
||||
import {clearCache} from "./utils/workflow-cache.js";
|
||||
import {validateFormatString} from "./validate-format-string.js";
|
||||
|
||||
beforeEach(() => {
|
||||
clearCache();
|
||||
});
|
||||
|
||||
describe("format string validation", () => {
|
||||
describe("validateFormatString unit tests", () => {
|
||||
it("returns valid for simple placeholder", () => {
|
||||
const result = validateFormatString("{0}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 0});
|
||||
});
|
||||
|
||||
it("returns valid for multiple placeholders", () => {
|
||||
const result = validateFormatString("{0} {1} {2}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 2});
|
||||
});
|
||||
|
||||
it("returns valid for text with placeholder", () => {
|
||||
const result = validateFormatString("hello {0} world");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 0});
|
||||
});
|
||||
|
||||
it("returns valid for escaped left braces", () => {
|
||||
const result = validateFormatString("{{0}} {0}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 0});
|
||||
});
|
||||
|
||||
it("returns valid for escaped right braces", () => {
|
||||
const result = validateFormatString("{0}}}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 0});
|
||||
});
|
||||
|
||||
it("returns valid for no placeholders", () => {
|
||||
const result = validateFormatString("hello world");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: -1});
|
||||
});
|
||||
|
||||
it("returns invalid for missing closing brace", () => {
|
||||
const result = validateFormatString("{0");
|
||||
expect(result).toEqual({valid: false, maxArgIndex: -1});
|
||||
});
|
||||
|
||||
it("returns invalid for empty placeholder", () => {
|
||||
const result = validateFormatString("{}");
|
||||
expect(result).toEqual({valid: false, maxArgIndex: -1});
|
||||
});
|
||||
|
||||
it("returns invalid for non-numeric placeholder", () => {
|
||||
const result = validateFormatString("{abc}");
|
||||
expect(result).toEqual({valid: false, maxArgIndex: -1});
|
||||
});
|
||||
|
||||
it("returns invalid for unescaped closing brace", () => {
|
||||
const result = validateFormatString("text } more");
|
||||
expect(result).toEqual({valid: false, maxArgIndex: -1});
|
||||
});
|
||||
|
||||
it("handles out-of-order placeholders", () => {
|
||||
const result = validateFormatString("{2} {0} {1}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 2});
|
||||
});
|
||||
|
||||
it("handles repeated placeholders", () => {
|
||||
const result = validateFormatString("{0} {0} {0}");
|
||||
expect(result).toEqual({valid: true, maxArgIndex: 0});
|
||||
});
|
||||
});
|
||||
|
||||
describe("InvalidFormatString workflow validation", () => {
|
||||
it("errors on missing closing brace", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{0', github.event_name) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string",
|
||||
severity: DiagnosticSeverity.Error
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("errors on empty braces", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{}', github.event_name) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("errors on non-numeric placeholder", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{abc}', github.event_name) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("allows valid format strings", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{0} {1}', github.event_name, github.ref) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("allows escaped braces", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{{0}} {0}', github.event_name) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string"
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("FormatArgCountMismatch workflow validation", () => {
|
||||
it("errors when placeholder exceeds arg count", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{2}', 'arg0', 'arg1') }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch",
|
||||
severity: DiagnosticSeverity.Error
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("errors when referencing arg 0 with no args", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{0}') }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("allows when arg count matches", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{0} {1} {2}', 'a', 'b', 'c') }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("handles no placeholders correctly", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('hello world') }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("skips validation for dynamic format strings", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format(env.FORMAT_STRING, 'arg') }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
// Should not have format errors since we can't validate dynamic strings
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "invalid-format-string"
|
||||
})
|
||||
);
|
||||
expect(result).not.toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch"
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it("validates nested format calls", async () => {
|
||||
const input = `
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo \${{ format('{0}', format('{2}', 'a')) }}
|
||||
`;
|
||||
const result = await validate(createDocument("wf.yaml", input));
|
||||
// The inner format call has an error
|
||||
expect(result).toContainEqual(
|
||||
expect.objectContaining({
|
||||
code: "format-arg-count-mismatch"
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,7 @@ import {mapRange} from "./utils/range.js";
|
||||
import {getOrConvertWorkflowTemplate, getOrParseWorkflow} from "./utils/workflow-cache.js";
|
||||
import {validateActionReference} from "./validate-action-reference.js";
|
||||
import {validateAction} from "./validate-action.js";
|
||||
import {validateFormatCalls} from "./validate-format-string.js";
|
||||
import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config.js";
|
||||
import {defaultValueProviders} from "./value-providers/default.js";
|
||||
|
||||
@@ -735,6 +736,28 @@ async function validateExpression(
|
||||
continue;
|
||||
}
|
||||
|
||||
// Validate format() function calls
|
||||
const formatErrors = validateFormatCalls(expr);
|
||||
for (const formatError of formatErrors) {
|
||||
if (formatError.type === "invalid-syntax") {
|
||||
diagnostics.push({
|
||||
message: `Invalid format string: ${formatError.message}`,
|
||||
range: mapRange(expression.range),
|
||||
severity: DiagnosticSeverity.Error,
|
||||
code: "invalid-format-string"
|
||||
});
|
||||
} else if (formatError.type === "arg-count-mismatch") {
|
||||
diagnostics.push({
|
||||
message: `Format string references argument {${formatError.expected - 1}} but only ${
|
||||
formatError.provided
|
||||
} argument(s) provided`,
|
||||
range: mapRange(expression.range),
|
||||
severity: DiagnosticSeverity.Error,
|
||||
code: "format-arg-count-mismatch"
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const context = await getWorkflowExpressionContext(
|
||||
namedContexts,
|
||||
contextProviderConfig,
|
||||
|
||||
Reference in New Issue
Block a user