Skip to content

Commit 4cc7ea4

Browse files
camc314camc314
committed
fix(codegen): reject invalid indent options (#25807)
## Summary - throw a TypeError when indent is empty, contains characters other than spaces or tabs, or is not a string - preserve the existing default tab indentation and unchanged printIndent hot path - update API documentation and validation coverage
1 parent f6b2265 commit 4cc7ea4

4 files changed

Lines changed: 31 additions & 34 deletions

File tree

‎packages/codegen/src-js/print/options.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export interface Position {
4444
export interface Options {
4545
/**
4646
* String to use for indentation, defaults to `"\t"`.
47-
* Must consist only of spaces and/or tabs - anything else falls back to a tab.
47+
* Must be a non-empty string consisting only of spaces and/or tabs.
48+
* Throws a `TypeError` otherwise.
4849
*/
4950
indent?: string;
5051

‎packages/codegen/src-js/state.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@ let indentString = "\t";
2828
*/
2929
const indents = [""];
3030

31-
/**
32-
* The `indent` option must be made up of only spaces and tabs, and not empty string.
33-
* Anything else falls back to a tab.
34-
*/
31+
/** The `indent` option must be a non-empty string made up of only spaces and tabs. */
3532
const INDENT_REGEX = /^[ \t]+$/;
3633

3734
/** Upper bound for the process-wide indentation cache. */
@@ -105,8 +102,13 @@ export class State {
105102
// The `indent` option is validated here, not in the printer, and changing of it discards
106103
// the cache grown for the old `indentString`.
107104
// That should be rare - most users have an indent style they prefer, and use it consistently.
108-
let { indent } = options;
109-
if (typeof indent !== "string" || !INDENT_REGEX.test(indent)) indent = "\t";
105+
const { indent: indentOption } = options;
106+
let indent = indentOption;
107+
if (indent === undefined) {
108+
indent = "\t";
109+
} else if (typeof indent !== "string" || !INDENT_REGEX.test(indent)) {
110+
throw new TypeError("`indent` must be a non-empty string containing only spaces and tabs");
111+
}
110112
if (indent !== indentString) {
111113
indentString = indent;
112114
indents.length = 1;

‎packages/codegen/test/print.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,22 @@ function checkCases(cases: Case[]): void {
107107

108108
// --- Tests ----------------------------------------------------------------------------------
109109

110+
describe("indent", () => {
111+
const ast = e(id("x"));
112+
113+
test.each(["", "x", "\n", "\r\n", " x "])("rejects %j", (indent) => {
114+
expect(() => printSync(ast, { indent })).toThrowError(
115+
new TypeError("`indent` must be a non-empty string containing only spaces and tabs"),
116+
);
117+
});
118+
119+
test.each([4, null, {}])("rejects non-string value %j", (indent) => {
120+
expect(() => printSync(ast, { indent: indent as unknown as string })).toThrowError(
121+
new TypeError("`indent` must be a non-empty string containing only spaces and tabs"),
122+
);
123+
});
124+
});
125+
110126
describe("starting indent level", () => {
111127
const ast = e(id("x"));
112128

‎packages/codegen/test/source-maps.test.ts‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -329,31 +329,9 @@ for (const fixture of FIXTURES) {
329329
// --- Indentation ----------------------------------------------------------------------------
330330

331331
// `printIndent` goes through the same write path as everything else, so the `indent` option and
332-
// the mappings have to agree: indentation is honoured (or falls back to a tab), and no mapping
333-
// ever points into the middle of an indent run.
334-
//
335-
// The invalid values are typed `unknown` because they are what an untyped JS caller can pass -
336-
// that they fall back to a tab rather than corrupting the output is the point of testing them.
337-
const INDENT_CASES: unknown[] = [
338-
// Valid - used as given
339-
undefined,
340-
"\t",
341-
" ",
342-
" ",
343-
"\t\t",
344-
" \t",
345-
"\t ",
346-
// Invalid - must fall back to a tab
347-
"",
348-
"x",
349-
"\n",
350-
"\r\n",
351-
" x ",
352-
" ",
353-
4,
354-
null,
355-
{},
356-
];
332+
// the mappings have to agree: indentation is honoured, and no mapping ever points into the middle
333+
// of an indent run.
334+
const INDENT_CASES = [undefined, "\t", " ", " ", "\t\t", " \t", "\t "];
357335

358336
describe("indent option", () => {
359337
test.each(
@@ -362,11 +340,11 @@ describe("indent option", () => {
362340
const program = parseWithLocs("indent.js", INLINE_JS);
363341
const collector = new Collector("indent.js");
364342
const out = printSync(program, {
365-
indent: indent as string | undefined,
343+
indent,
366344
sourceMap: collector,
367345
}).code;
368346

369-
const expectedIndent = typeof indent === "string" && /^[ \t]+$/.test(indent) ? indent : "\t";
347+
const expectedIndent = indent ?? "\t";
370348
const lines = out.split("\n");
371349
const indented = lines.filter((line) => line.startsWith("\t") || line.startsWith(" "));
372350
expect(indented.length).toBeGreaterThan(0);

0 commit comments

Comments
 (0)
Sponsor
SponsoredKunjungi sekarang
Promo