Skip to content

Commit 1aaf1c1

Browse files
lidge-junlidge-jun
committed
fix(tests): end a guarded statement without needing its semicolon
A semicolonless guarded registration left the conditional flag set for the next line, so the unconditional hook after it was refused too. Statement level now also ends at a line break after a token a statement can end on, with the token that begins a guarded statement exempted so if (false) on its own line still guards what follows it.
1 parent 4ae990a commit 1aaf1c1

2 files changed

Lines changed: 80 additions & 3 deletions

File tree

‎tests/ci-workflows/warmup-registration.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,22 @@ describe("warm-up judge: the hook registration", () => {
206206
);
207207
expect(warmupIsRegistered(inner)).toBe(false);
208208
});
209+
210+
test("a semicolonless guarded hook does not poison the registration after it", () => {
211+
// The guarded one is refused and the plain one is not. Without a statement boundary that does
212+
// not need a semicolon, the condition would still be set on the next line, and a file whose
213+
// style omits semicolons would lose a warm-up it really has.
214+
const report = judge(
215+
BUN_TEST,
216+
HELPER,
217+
"if (false) beforeAll(() => warmModuleGraph(options))",
218+
"beforeAll(async () => {",
219+
" await warmModuleGraph(options);",
220+
"}, COLD_SPAWN_WARMUP_HOOK_BUDGET_MS);",
221+
);
222+
expect(warmupIsRegistered(report)).toBe(true);
223+
expect(warmupRegistrationComplaints(report).join(" ")).toContain("cannot see it run");
224+
});
209225
});
210226

211227
describe("warm-up judge: call ownership", () => {

‎tests/helpers/warmup-registration.ts‎

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,14 @@ const WARMUP_ENTRY_POINTS: ReadonlySet<string> = new Set(["warmColdSpawn", "warm
7171
/** The bun:test hook a warm-up belongs in, so the cost lands in setup and not in an assertion. */
7272
const REGISTRATION_HOOK = "beforeAll";
7373

74-
type Token = Readonly<{ kind: SyntaxKind; text: string; value: string; start: number }>;
74+
type Token = Readonly<{
75+
kind: SyntaxKind;
76+
text: string;
77+
value: string;
78+
start: number;
79+
/** Whether trivia before this token held a line break, which is where a statement can end. */
80+
newline: boolean;
81+
}>;
7582

7683
export type WarmupRegistration = Readonly<{
7784
/** The helper entry point the hook waits for. */
@@ -182,6 +189,43 @@ const CONDITIONAL_TOKENS: ReadonlySet<SyntaxKind> = new Set([
182189
SyntaxKind.QuestionQuestionToken,
183190
]);
184191

192+
/**
193+
* Conditionals with a parenthesized head. Their closing paren can end a statement, so the token
194+
* after it needs an exemption from the line-break rule below: it begins the guarded statement
195+
* rather than a new one.
196+
*/
197+
const HEADED_CONDITIONALS: ReadonlySet<SyntaxKind> = new Set([
198+
SyntaxKind.IfKeyword,
199+
SyntaxKind.ForKeyword,
200+
SyntaxKind.WhileKeyword,
201+
SyntaxKind.SwitchKeyword,
202+
]);
203+
204+
/**
205+
* Tokens a statement can end on. This repository writes its semicolons, but the judge cannot
206+
* assume that: with no automatic semicolon insertion, one semicolonless guarded statement would
207+
* leave the conditional flag set and refuse the next, unconditional registration after it.
208+
*/
209+
const CAN_END_STATEMENT: ReadonlySet<SyntaxKind> = new Set([
210+
SyntaxKind.Identifier,
211+
SyntaxKind.PrivateIdentifier,
212+
SyntaxKind.CloseParenToken,
213+
SyntaxKind.CloseBracketToken,
214+
SyntaxKind.CloseBraceToken,
215+
SyntaxKind.StringLiteral,
216+
SyntaxKind.NumericLiteral,
217+
SyntaxKind.BigIntLiteral,
218+
SyntaxKind.NoSubstitutionTemplateLiteral,
219+
SyntaxKind.TemplateTail,
220+
SyntaxKind.RegularExpressionLiteral,
221+
SyntaxKind.TrueKeyword,
222+
SyntaxKind.FalseKeyword,
223+
SyntaxKind.NullKeyword,
224+
SyntaxKind.ThisKeyword,
225+
SyntaxKind.PlusPlusToken,
226+
SyntaxKind.MinusMinusToken,
227+
]);
228+
185229
/**
186230
* The only call a registration may sit inside. A hook registered in an uncalled helper function,
187231
* in a test body, or in an immediately-invoked function is not a hook the file is known to run, and
@@ -253,6 +297,10 @@ export function analyzeWarmupRegistration(fileName: string, source: string): War
253297
const callStack: string[] = [];
254298
const frames: { owner: string; conditional: boolean; parenDepth: number }[] = [];
255299
let statementConditional = false;
300+
// The token that begins a guarded statement: the one place where a line break after a closing
301+
// paren continues the statement rather than ending it.
302+
let consequentStart = -1;
303+
let awaitingConsequent = false;
256304
for (let i = 0; i < tokens.length; i += 1) {
257305
const token = tokens[i];
258306
if (token.kind === SyntaxKind.CloseParenToken) callStack.pop();
@@ -264,7 +312,19 @@ export function analyzeWarmupRegistration(fileName: string, source: string): War
264312
// describe sits one paren deep, so a file-level test would never see a condition in there.
265313
const frame = frames[frames.length - 1];
266314
const atStatementLevel = callStack.length === (frame === undefined ? 0 : frame.parenDepth);
267-
if (atStatementLevel && CONDITIONAL_TOKENS.has(token.kind)) statementConditional = true;
315+
const previousToken = tokens[i - 1];
316+
if (atStatementLevel && token.newline && i !== consequentStart
317+
&& previousToken !== undefined && CAN_END_STATEMENT.has(previousToken.kind)) {
318+
statementConditional = false;
319+
}
320+
if (atStatementLevel && awaitingConsequent && token.kind === SyntaxKind.CloseParenToken) {
321+
awaitingConsequent = false;
322+
consequentStart = i + 1;
323+
}
324+
if (atStatementLevel && CONDITIONAL_TOKENS.has(token.kind)) {
325+
statementConditional = true;
326+
if (HEADED_CONDITIONALS.has(token.kind)) awaitingConsequent = true;
327+
}
268328
if (atStatementLevel && token.kind === SyntaxKind.SemicolonToken) statementConditional = false;
269329
if (token.kind === SyntaxKind.OpenBraceToken) {
270330
frames.push({
@@ -406,6 +466,7 @@ function tokenize(source: string): { tokens: Token[]; unreadable: string[] } {
406466
if (kind === SyntaxKind.EndOfFile) break;
407467
let start = scanner.getTokenStart();
408468
let text = scanner.getTokenText();
469+
const newline = scanner.hasPrecedingLineBreak();
409470
if (kind === SyntaxKind.SlashToken || kind === SyntaxKind.SlashEqualsToken) {
410471
const afterSlash = scanner.getTokenEnd();
411472
const previous = tokens[tokens.length - 1];
@@ -442,7 +503,7 @@ function tokenize(source: string): { tokens: Token[]; unreadable: string[] } {
442503
unreadable.push(at(source, start) + "a closing delimiter with nothing open, so this judge is reading the file wrong");
443504
return { tokens, unreadable };
444505
}
445-
tokens.push({ kind, text, value: value ?? "", start });
506+
tokens.push({ kind, text, value: value ?? "", start, newline });
446507
}
447508
if (braces !== 0 || parens !== 0 || brackets !== 0 || templates.length > 0) {
448509
unreadable.push("the file does not close every delimiter this judge opened (braces " + braces

0 commit comments

Comments
 (0)