diff --git a/packages/lint/src/rules/composition.test.ts b/packages/lint/src/rules/composition.test.ts index 6b70817dc6..2c00acc035 100644 --- a/packages/lint/src/rules/composition.test.ts +++ b/packages/lint/src/rules/composition.test.ts @@ -430,6 +430,59 @@ describe("composition rules", () => { expect(findings[0]?.fixHint).toContain('[data-composition-id="scene"][data-start="0"]'); }); + it("does not report a template-literal selector that only appears in a comment or a string", async () => { + const html = ` + +
+ +`; + const result = await lintHyperframeHtml(html); + expect(result.findings.find((f) => f.code === "template_literal_selector")).toBeUndefined(); + }); + + it("reports a template-literal selector in code position and quotes the real source", async () => { + const html = ` + +
+ +`; + const result = await lintHyperframeHtml(html); + const finding = result.findings.find((f) => f.code === "template_literal_selector"); + expect(finding?.severity).toBe("error"); + expect(finding?.snippet).toContain("data-composition-id"); + }); + + it("does not report a split data-attribute selector written inside a CSS comment", async () => { + const html = ` + +
+ + +`; + const result = await lintHyperframeHtml(html); + expect(result.findings.filter((f) => f.code === "split_data_attribute_selector")).toHaveLength( + 0, + ); + }); + describe("timed_element_missing_clip_class", () => { it("flags element with data-start but no class='clip'", async () => { const html = ` @@ -768,6 +821,41 @@ describe("composition rules", () => { ); expect(finding).toBeUndefined(); }); + + it("does not flag a call the composition only renders as on-screen text", async () => { + const html = ` + +
+ +`; + const result = await lintHyperframeHtml(html); + expect( + result.findings.find((f) => f.code === "requestanimationframe_in_composition"), + ).toBeUndefined(); + }); + + it("still flags a call inside a template interpolation, which is code", async () => { + const html = ` + +
+ +`; + const result = await lintHyperframeHtml(html); + expect( + result.findings.find((f) => f.code === "requestanimationframe_in_composition")?.severity, + ).toBe("error"); + }); }); describe("missing_data_no_timeline", () => { diff --git a/packages/lint/src/rules/composition.ts b/packages/lint/src/rules/composition.ts index cbc8799aa3..3aee42ef5b 100644 --- a/packages/lint/src/rules/composition.ts +++ b/packages/lint/src/rules/composition.ts @@ -4,7 +4,9 @@ import { readAttr, readDecodedAttr, readJsonAttr, + stripCssComments, stripJsComments, + stripJsStringLiterals, truncateSnippet, WINDOW_TIMELINE_ASSIGN_PATTERN, } from "../utils"; @@ -513,8 +515,9 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding }); } }; - for (const style of styles) scan(style.content); - for (const script of scripts) scan(script.content); + // Comments only, not string literals: the genuine defect lives inside a query string. + for (const style of styles) scan(stripCssComments(style.content)); + for (const script of scripts) scan(stripJsComments(script.content)); return findings; }, @@ -524,8 +527,10 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding for (const script of scripts) { const templateLiteralSelectorPattern = /(?:querySelector|querySelectorAll)\s*\(\s*`[^`]*\$\{[^}]+\}[^`]*`\s*\)/g; + // Offsets survive both strippers, so the snippet still quotes the real source. + const scanned = stripJsStringLiterals(stripJsComments(script.content)); let tlMatch: RegExpExecArray | null; - while ((tlMatch = templateLiteralSelectorPattern.exec(script.content)) !== null) { + while ((tlMatch = templateLiteralSelectorPattern.exec(scanned)) !== null) { findings.push({ code: "template_literal_selector", severity: "error", @@ -534,7 +539,9 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding "The HTML bundler's CSS parser crashes on these. Use a hardcoded string instead.", fixHint: "Replace the template literal variable with a hardcoded string. The bundler's CSS parser cannot handle interpolated variables in script content.", - snippet: truncateSnippet(tlMatch[0]), + snippet: truncateSnippet( + script.content.slice(tlMatch.index, tlMatch.index + tlMatch[0].length), + ), }); } } @@ -731,7 +738,8 @@ export const compositionRules: Array<(ctx: LintContext) => HyperframeLintFinding if (isRegistrySourceFile(options.filePath) || isRegistryInstalledFile(rawSource)) return []; const findings: HyperframeLintFinding[] = []; for (const script of scripts) { - const stripped = stripJsComments(script.content); + // Strings too, not just comments: a code-explainer composition renders the call as text. + const stripped = stripJsStringLiterals(stripJsComments(script.content)); if (/requestAnimationFrame\s*\(/.test(stripped)) { findings.push({ code: "requestanimationframe_in_composition", diff --git a/packages/lint/src/utils.test.ts b/packages/lint/src/utils.test.ts new file mode 100644 index 0000000000..29660dfbc6 --- /dev/null +++ b/packages/lint/src/utils.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect } from "vitest"; +import { stripCssComments, stripJsComments, stripJsStringLiterals } from "./utils.js"; + +const scan = (src: string) => stripJsStringLiterals(stripJsComments(src)); +const findsRaf = (src: string) => /requestAnimationFrame\s*\(/.test(scan(src)); + +describe("stripJsStringLiterals", () => { + it("blanks a call the composition only renders as text", () => { + expect(findsRaf('const CODE = "requestAnimationFrame(step);";')).toBe(false); + expect(findsRaf("const CODE = `requestAnimationFrame(${fn});`;")).toBe(false); + }); + + it("keeps a call in a template interpolation, which is code", () => { + expect(findsRaf("const label = `frame ${requestAnimationFrame(cb)}`;")).toBe(true); + }); + + // A regex literal's own quotes must not open a phantom string: `/[^a-z']/g` ships in + // registry/components, and blanking its tail hid every later call in the script. + it.each([ + "const re = /[\"']/g;", + "const re = /'/;", + "const re = /[`]/;", + "const r = s.split(/['\"]/);", + 'const r = s.replace(/[^a-z\']/g, "");', + "if (a) { } /'/.test(x);", + "const a = b / c / d;", + "const p = i++ / total;", + "const q = --i / total;", + ])("does not let %j blank the rest of the script", (prefix) => { + expect(findsRaf(`${prefix}\nrequestAnimationFrame(step);`)).toBe(true); + }); + + // Whitespace must end the trailing identifier: fusing `flag` + `return` across a + // newline read the regex as a division, and two such regexes balance the quote so + // the mid-literal fallback never fires — an unbounded blanked span. + it.each([ + "function a(s) {\n let ok = flag\n return /'/.test(s)\n}\n", + "for (const x of /'/.source) {}\n", + ])("keeps a call bracketed by two quote-bearing regexes after %j", (prefix) => { + expect(findsRaf(`${prefix}requestAnimationFrame(step);\n${prefix}`)).toBe(true); + }); + + // Damage from a mis-read regex is line-bounded, so the call must sit on the SAME + // line to pin it. A reserved word used as a property name is not keyword position. + it.each([ + "const clip = { in: 0.5, out: 5.5 }; const r = clip.in / clip.out;", + "const r = data.new / 2;", + "const r = list.of / 2;", + "const r = sw.case / 2;", + "const r = o?.in / 2;", + "const p = i++ / total;", + "const q = --i / total;", + ])("finds a call on the same line as %j", (prefix) => { + expect(findsRaf(`${prefix} requestAnimationFrame(step);`)).toBe(true); + }); + + it("falls back to the source when the scan ends mid-literal", () => { + // The trailing backslash escapes the closing quote, so the string runs to EOF. + const src = 'const p = "C:\\Users\\demo\\";\nrequestAnimationFrame(step);'; + expect(scan(src)).toBe(src); + expect(findsRaf(src)).toBe(true); + }); + + it("preserves length and newline positions", () => { + for (const src of [ + 'const a = "x\\\ny";\nrequestAnimationFrame(step);', + "const t = `a\nb${x}c\nd`;", + "const r = /a\\/b/g;\n", + ]) { + const out = scan(src); + expect(out.length).toBe(src.length); + expect([...out].filter((c) => c === "\n").length).toBe( + [...src].filter((c) => c === "\n").length, + ); + } + }); +}); + +describe("stripJsStringLiterals scaling", () => { + // Re-scanning the accumulated output per candidate slash was quadratic: a composition + // with one inlined vendor bundle linted 58x slower. + it("stays linear in slash-dense input", () => { + // Min of three: a single sample picks up GC and CI contention, which can both + // inflate `small` (hiding a regression) and inflate `large` (failing spuriously). + const time = (n: number) => { + const src = "a=b/c;".repeat(n); + let best = Infinity; + for (let run = 0; run < 3; run += 1) { + const started = performance.now(); + stripJsStringLiterals(src); + best = Math.min(best, performance.now() - started); + } + return best; + }; + const small = Math.max(time(20_000), 0.5); + const large = time(160_000); + // 8x the input; quadratic would be ~64x. Generous headroom for a noisy CI box. + expect(large / small).toBeLessThan(24); + // Absolute ceiling too: a noise-inflated `small` can hide a quadratic regression. + expect(large).toBeLessThan(400); + }); +}); + +describe("stripCssComments", () => { + it("keeps a rule sandwiched between comment markers printed as content", () => { + const css = + '#o::before{content:"/*"}\n[data-composition-id="main" data-start="0"]{color:red}\n#c::after{content:"*/"}'; + const out = stripCssComments(css); + expect(out.length).toBe(css.length); + expect(out).toContain('data-composition-id="main" data-start="0"'); + }); + + it("blanks a real comment and an unterminated one, keeping length", () => { + for (const css of ["/* gone */#a{color:red}", "#a{color:red}/* open"]) { + const out = stripCssComments(css); + expect(out.length).toBe(css.length); + expect(out).not.toContain("/*"); + expect(out).toContain("#a{color:red}"); + } + }); +}); diff --git a/packages/lint/src/utils.ts b/packages/lint/src/utils.ts index 39b2315381..4f72a948db 100644 --- a/packages/lint/src/utils.ts +++ b/packages/lint/src/utils.ts @@ -337,6 +337,240 @@ export function stripJsComments(source: string): string { return out; } +// A `/` after any of these starts a regex literal, not a division. +const REGEX_ALLOWED_BEFORE = new Set("=(,:[!&|?{};+-*%^~<>"); +const REGEX_ALLOWED_KEYWORDS = new Set([ + "return", + "typeof", + "instanceof", + "in", + "of", + "new", + "delete", + "void", + "case", + "do", + "else", + "yield", + "await", +]); + +const WORD_CHAR = /[A-Za-z0-9_$]/; + +/** + * Tracks just enough emitted context to tell a regex literal from a division: the last + * two significant characters and the trailing identifier. Carried incrementally because + * re-scanning the accumulated output per candidate slash is quadratic — a composition + * with one inlined vendor bundle took 58x longer to lint. + */ +class CodeContext { + private last = ""; + private prev = ""; + private word = ""; + // Whitespace ends the identifier without clearing it — clearing would break the + // keyword path (`return /re/`), while fusing it across a newline read `flag` + + // `return` as one word and mis-classified the regex as a division. + private wordEnded = false; + // `clip.in / clip.out` is a division: a reserved word used as a property name is + // not a keyword, and `{ in, out }` is the usual clip in/out convention here. + private wordAfterDot = false; + + push(ch: string): void { + if (ch === " " || ch === "\t" || ch === "\n" || ch === "\r") { + this.wordEnded = true; + return; + } + if (WORD_CHAR.test(ch)) { + if (this.wordEnded || this.word === "") this.wordAfterDot = this.last === "."; + this.word = this.wordEnded ? ch : this.word + ch; + } else { + this.word = ""; + this.wordAfterDot = false; + } + this.wordEnded = false; + this.prev = this.last; + this.last = ch; + } + + /** A regex literal's own quotes must not open a phantom string — `/[^a-z']/g` ships + * in this repo, and treating its `'` as a string start blanked the whole script tail. */ + startsRegexLiteral(): boolean { + if (this.last === "") return true; + if (WORD_CHAR.test(this.last)) + return !this.wordAfterDot && REGEX_ALLOWED_KEYWORDS.has(this.word); + // `i++ / total` and `--i / n` are divisions, though `+` and `-` alone are not. + if ((this.last === "+" || this.last === "-") && this.prev === this.last) return false; + return REGEX_ALLOWED_BEFORE.has(this.last); + } +} + +/** + * Blanks string, template-literal and regex-literal *contents* (delimiters, length + * and newline positions kept) so a rule scanning for an API call does not match one + * a composition merely renders as on-screen text. Template `${…}` expressions stay — + * they are code. Returns the source untouched if the scan ends mid-literal, so a + * parse this scanner cannot model degrades to the caller's pre-existing behaviour + * rather than silently blanking real code on an `error`-severity gate. + */ +// A character-level scanner over four literal modes; splitting it pushes the state into +// per-character objects without making it clearer, same as `stripJsComments` above. +// fallow-ignore-next-line complexity +export function stripJsStringLiterals(source: string): string { + let out = ""; + let i = 0; + // Each open template literal pushes a frame; `${` inside one returns to code mode. + const templateBraces: number[] = []; + const ctx = new CodeContext(); + let quote: "'" | '"' | "`" | null = null; + let escaped = false; + let inRegex = false; + let inRegexClass = false; + + const blank = (ch: string) => (ch === "\n" || ch === "\r" ? ch : " "); + const emit = (text: string) => { + out += text; + for (const ch of text) ctx.push(ch); + }; + + while (i < source.length) { + const ch = source[i] ?? ""; + const next = source[i + 1] ?? ""; + + if (inRegex) { + if (escaped) { + escaped = false; + emit(blank(ch)); + } else if (ch === "\\") { + escaped = true; + emit(" "); + } else if (ch === "[") { + inRegexClass = true; + emit(" "); + } else if (ch === "]") { + inRegexClass = false; + emit(" "); + } else if (ch === "/" && !inRegexClass) { + inRegex = false; + emit(ch); + } else if (ch === "\n" || ch === "\r") { + // An unterminated regex cannot span a line; treat it as division after all. + inRegex = false; + inRegexClass = false; + emit(ch); + } else { + emit(" "); + } + i += 1; + continue; + } + + if (quote) { + if (escaped) { + escaped = false; + emit(blank(ch)); + } else if (ch === "\\") { + escaped = true; + emit(" "); + } else if (ch === quote) { + quote = null; + emit(ch); + } else if (ch === "`" || quote !== "`" || ch !== "$" || next !== "{") { + emit(blank(ch)); + } else { + templateBraces.push(0); + quote = null; + emit("${"); + i += 2; + continue; + } + i += 1; + continue; + } + + if (ch === "'" || ch === '"' || ch === "`") { + quote = ch; + emit(ch); + i += 1; + continue; + } + + if (ch === "/" && next !== "/" && next !== "*" && ctx.startsRegexLiteral()) { + inRegex = true; + emit(ch); + i += 1; + continue; + } + + if (templateBraces.length > 0) { + const depth = templateBraces[templateBraces.length - 1] ?? 0; + if (ch === "{") templateBraces[templateBraces.length - 1] = depth + 1; + else if (ch === "}") { + if (depth === 0) { + templateBraces.pop(); + quote = "`"; + emit(ch); + i += 1; + continue; + } + templateBraces[templateBraces.length - 1] = depth - 1; + } + } + + emit(ch); + i += 1; + } + + if (quote !== null || templateBraces.length > 0) return source; + return out; +} + +/** + * Drops CSS comments without following a `/*` that only appears inside a string — + * a slide printing comment markers as content otherwise pairs two of them and + * deletes the real rules in between. + */ +// fallow-ignore-next-line complexity +export function stripCssComments(source: string): string { + let out = ""; + let i = 0; + let quote: "'" | '"' | null = null; + + while (i < source.length) { + const ch = source[i] ?? ""; + if (quote) { + out += ch; + if (ch === "\\") { + out += source[i + 1] ?? ""; + i += 2; + continue; + } + if (ch === quote) quote = null; + i += 1; + continue; + } + if (ch === '"' || ch === "'") { + quote = ch; + out += ch; + i += 1; + continue; + } + if (ch === "/" && source[i + 1] === "*") { + const end = source.indexOf("*/", i + 2); + const stop = end === -1 ? source.length : end + 2; + for (let j = i; j < stop; j += 1) { + const c = source[j] ?? ""; + out += c === "\n" || c === "\r" ? c : " "; + } + i = stop; + continue; + } + out += ch; + i += 1; + } + + return out; +} + // One linear pass that drops every `` region. Uses indexOf, not a // `//` regex: that pattern backtracks O(n²) on inputs with many // unterminated "