Skip to content

Commit 798c571

Browse files
fix: find comments after a backtick in a JS regex literal (#53)
Copy the fixed scripts/check-comment-quality.mjs and its tests from fallow-rs/fallow (fallow-rs/fallow#4230). The check treated every backtick in a JavaScript file as the start of a template literal. A backtick inside a regex literal, a quoted string, or a comment opened a template that never closed, and the check skipped every comment after that line. The opener scan now skips these constructs. Signed-off-by: Bart Waardenburg <bart@waardenburg.dev>
1 parent 83baa88 commit 798c571

2 files changed

Lines changed: 93 additions & 1 deletion

File tree

‎scripts/check-comment-quality.mjs‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,83 @@ const JAVASCRIPT_EXTENSIONS = new Set([
8383

8484
const isSourcePath = (path) => SOURCE_EXTENSIONS.has(extname(path).toLowerCase());
8585

86+
// A `/` after one of these characters, or at the start of the line, starts a
87+
// regex literal. After an identifier, a number or `)`, it is a division.
88+
const REGEX_PRECEDER = /[(,=:[!&|?{};+\-*%<>~^]/u;
89+
const REGEX_KEYWORD = /\b(?:return|typeof|case|do|else|in|of|void|yield|await)$/u;
90+
91+
const startsRegexLiteral = (line, index) => {
92+
const before = line.slice(0, index).trimEnd();
93+
return before.length === 0 || REGEX_PRECEDER.test(before.at(-1)) || REGEX_KEYWORD.test(before);
94+
};
95+
96+
const regexLiteralEnd = (line, index) => {
97+
let inClass = false;
98+
for (let cursor = index + 1; cursor < line.length; cursor += 1) {
99+
const character = line[cursor];
100+
if (character === "\\") {
101+
cursor += 1;
102+
} else if (character === "[") {
103+
inClass = true;
104+
} else if (character === "]") {
105+
inClass = false;
106+
} else if (character === "/" && !inClass) {
107+
return cursor;
108+
}
109+
}
110+
return -1;
111+
};
112+
113+
// Finds the first backtick that opens a template literal. A backtick inside a
114+
// quoted string, a regex literal, or a comment opens nothing. Without this
115+
// scan, the rest of the file reads as one template and hides every comment.
116+
// Returns the last index of the slash construct at `index`, or -1 when the
117+
// rest of the line is a comment or an unclosed block comment.
118+
const slashConstructEnd = (line, index) => {
119+
const next = line[index + 1];
120+
if (next === "/") {
121+
return -1;
122+
}
123+
if (next === "*") {
124+
const close = line.indexOf("*/", index + 2);
125+
return close === -1 ? -1 : close + 1;
126+
}
127+
if (!startsRegexLiteral(line, index)) {
128+
return index;
129+
}
130+
const close = regexLiteralEnd(line, index);
131+
return close === -1 ? index : close;
132+
};
133+
134+
// Returns the last index of the construct at `index` that cannot hold a
135+
// template opener, or -1 when nothing after it can.
136+
const skippedConstructEnd = (line, index) => {
137+
const character = line[index];
138+
if (character === '"' || character === "'") {
139+
return closingDelimiterIndex(line, character, index + 1, true);
140+
}
141+
return character === "/" ? slashConstructEnd(line, index) : index;
142+
};
143+
144+
const templateOpenerIndex = (line, start) => {
145+
for (let index = start; index < line.length; index += 1) {
146+
if (line[index] === "`") {
147+
return index;
148+
}
149+
index = skippedConstructEnd(line, index);
150+
if (index === -1) {
151+
return -1;
152+
}
153+
}
154+
return -1;
155+
};
156+
86157
const multilineOpener = (path, line, start) => {
87158
const extension = extname(path).toLowerCase();
88159
const candidates = [];
89160

90161
if (JAVASCRIPT_EXTENSIONS.has(extension)) {
91-
const index = line.indexOf("`", start);
162+
const index = templateOpenerIndex(line, start);
92163
if (index !== -1) {
93164
candidates.push({ index, length: 1, delimiter: "`", escaped: true });
94165
}

‎scripts/check-comment-quality.test.mjs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,24 @@ test("pre-tool mode blocks an edit that adds a history comment", () => {
328328
rmSync(root, { recursive: true, force: true });
329329
}
330330
});
331+
332+
test("a backtick in a regex literal, string, or comment does not open a template", () => {
333+
const source = [
334+
"const fence = /`{3}/u;",
335+
"const quote = '`';",
336+
'const other = "a ` b";',
337+
"// Wrap the name in a ` pair.",
338+
"const ratio = total / count; const tick = /`/u;",
339+
"/* a ` b */ const done = true;",
340+
"// This used to be a map.",
341+
"const template = `",
342+
"// Here we show the example.",
343+
"`;",
344+
"// Previously the loop ran twice.",
345+
].join("\n");
346+
347+
assert.deepEqual(
348+
scanSourceText("scripts/check.mjs", source).map(({ line }) => line),
349+
[7, 11],
350+
);
351+
});

0 commit comments

Comments
 (0)