From f34871d20df3dcd2fb7da4d15cd6ffcfc8f2d964 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 12 Aug 2026 11:18:53 +0000 Subject: [PATCH 1/2] Reduce false positives across Three.js rules --- .changeset/calm-threejs-signals.md | 5 + .../src/plugin/constants/js.ts | 14 + .../src/plugin/core-rule-registry-data.json | 6 +- .../src/plugin/liveness/liveness-fixtures.ts | 2 +- .../async-await-in-loop.regressions.test.ts | 110 +++++ .../js-performance/async-await-in-loop.ts | 140 ++++++ .../async-parallel.regressions.test.ts | 6 + ...-cache-property-access.regressions.test.ts | 33 ++ .../js-cache-property-access.ts | 50 +- .../js-index-maps.regressions.test.ts | 6 + .../js-length-check-first.regressions.test.ts | 6 + .../js-performance/js-length-check-first.ts | 33 +- ...map-lookups-reactbench-regressions.test.ts | 55 +++ .../js-performance/js-set-map-lookups.ts | 68 ++- ...o-create-object-url-without-revoke.test.ts | 13 + .../no-create-object-url-without-revoke.ts | 33 +- .../src/plugin/rules/r3f/constants.ts | 1 - .../r3f/r3f-prefer-gpu-position-animation.ts | 1 + .../r3f/r3f-valid-orthographic-camera.test.ts | 3 +- .../r3f/r3f-valid-orthographic-camera.ts | 2 +- ...ct-composer-require-size-on-resize.test.ts | 2 + ...-effect-composer-require-size-on-resize.ts | 33 +- ...three-no-compile-in-animation-loop.test.ts | 34 ++ .../r3f/three-no-compile-in-animation-loop.ts | 49 +- .../three-no-new-in-animation-loop.test.ts | 22 + .../r3f/three-no-new-in-animation-loop.ts | 16 +- ...ree-prefer-gpu-instanced-animation.test.ts | 18 + .../three-prefer-gpu-instanced-animation.ts | 3 +- ...hree-prefer-gpu-position-animation.test.ts | 17 + .../three-prefer-gpu-position-animation.ts | 7 +- .../three-prefer-set-animation-loop.test.ts | 27 +- .../r3f/three-prefer-set-animation-loop.ts | 53 ++- ...ee-require-instanced-buffer-update.test.ts | 201 ++++++++ .../three-require-instanced-buffer-update.ts | 448 ++++++++++++++++-- ...ader-require-position-on-all-paths.test.ts | 2 + .../three-valid-orthographic-camera.test.ts | 5 +- .../r3f/three-valid-orthographic-camera.ts | 24 +- .../three-valid-texture-color-space.test.ts | 14 + .../r3f/three-valid-texture-color-space.ts | 21 + ...s-glsl-main-write-position-on-all-paths.ts | 12 + ...find-repeated-position-buffer-mutations.ts | 4 +- ...t-invalid-orthographic-camera-parameter.ts | 7 - .../dangerous-html-sink.regressions.test.ts | 43 +- .../security-scan/dangerous-html-sink.ts | 48 +- ...rpc-native-bridge-risk.regressions.test.ts | 16 + .../local-rpc-native-bridge-risk.ts | 2 +- ...ostmessage-origin-risk.regressions.test.ts | 9 + .../security-scan/postmessage-origin-risk.ts | 14 + ...tial-independent-await.regressions.test.ts | 39 ++ .../server-sequential-independent-await.ts | 56 +++ .../utils/create-loop-aware-visitors.ts | 21 +- 51 files changed, 1680 insertions(+), 174 deletions(-) create mode 100644 .changeset/calm-threejs-signals.md diff --git a/.changeset/calm-threejs-signals.md b/.changeset/calm-threejs-signals.md new file mode 100644 index 0000000000..7921b94e48 --- /dev/null +++ b/.changeset/calm-threejs-signals.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Reduce false positives across Three.js, JavaScript performance, security, and async rules by recognizing stable lifecycle guards, intentional sequencing, non-escaping values, renderer aliases, and safe DOM data flows. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/js.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/js.ts index 49d540ebc6..0d7c54bd99 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/js.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/js.ts @@ -258,6 +258,20 @@ export const INTENTIONAL_SEQUENCING_CALLEE_NAMES: ReadonlySet = new Set( "queueMicrotask", "requestAnimationFrame", "requestIdleCallback", + "yieldNow", + "yieldTo", + "yieldToBrowser", + "slice", + "sliceYield", + "nextFrame", + "breathe", + "onYield", + "onProgress", + "progress", + "onStep", + "step", + "stage", + "runStage", "animate", "transition", "spring", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/core-rule-registry-data.json b/packages/oxlint-plugin-react-doctor/src/plugin/core-rule-registry-data.json index 138160cde4..19546e7401 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/core-rule-registry-data.json +++ b/packages/oxlint-plugin-react-doctor/src/plugin/core-rule-registry-data.json @@ -1848,7 +1848,7 @@ "id": "js-cache-property-access", "title": "Repeated property access in a loop", "severity": "warn", - "recommendation": "Read the value once into a variable at the top of the loop: `const { x, y } = obj.deeply.nested`", + "recommendation": "When the value is unchanged between reads, cache it immediately before the first read inside the loop", "category": "Performance", "framework": "global", "tags": ["test-noise"], @@ -9608,7 +9608,7 @@ "id": "r3f-valid-orthographic-camera", "title": "Invalid R3F orthographic camera", "severity": "error", - "recommendation": "Use distinct horizontal and vertical frustum planes, a nonnegative near plane, and a far plane greater than near", + "recommendation": "Use distinct horizontal and vertical frustum planes and a far plane greater than near", "category": "Bugs", "framework": "global", "requires": ["react", "r3f"], @@ -14092,7 +14092,7 @@ "id": "three-valid-orthographic-camera", "title": "Invalid Three.js orthographic camera", "severity": "error", - "recommendation": "Use distinct horizontal and vertical frustum planes, a nonnegative near plane, and a far plane greater than near", + "recommendation": "Use distinct horizontal and vertical frustum planes and a far plane greater than near", "category": "Bugs", "framework": "global", "requires": ["three"], diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/liveness/liveness-fixtures.ts b/packages/oxlint-plugin-react-doctor/src/plugin/liveness/liveness-fixtures.ts index a9af340be5..160aea3009 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/liveness/liveness-fixtures.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/liveness/liveness-fixtures.ts @@ -2725,7 +2725,7 @@ export const useListNavigation = ({ selectedIndex, focusItem }) => { code: 'import { WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); renderer.setAnimationLoop(() => renderer.readRenderTargetPixels(target, 0, 0, 1, 1, pixels));', }, "three-prefer-set-animation-loop": { - code: 'import { WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); const frame = () => { renderer.render(scene, camera); requestAnimationFrame(frame); }; requestAnimationFrame(frame);', + code: 'import { WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); renderer.xr.enabled = true; const frame = () => { renderer.render(scene, camera); requestAnimationFrame(frame); }; requestAnimationFrame(frame);', }, "three-require-animation-mixer-update": { code: 'import { AnimationMixer, WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); const mixer = new AnimationMixer(model); mixer.clipAction(clip).play(); renderer.setAnimationLoop(() => renderer.render(scene, camera));', diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.regressions.test.ts index 7e0494152e..95da747942 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.regressions.test.ts @@ -23,6 +23,89 @@ describe("js-performance/async-await-in-loop — regressions", () => { } }); + it.each([ + "yieldNow", + "yieldToBrowser", + "sliceYield", + "nextFrame", + "breathe", + "onYield", + "onProgress", + "progress", + "onStep", + "step", + "runStage", + ])("keeps opaque pacing helper %s sequential", (helperName) => { + const result = runRule( + asyncAwaitInLoop, + `async function build(items) { for (const item of items) { await ${helperName}(item); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + + it("keeps a scheduler check sequential without trusting arbitrary check methods", () => { + const paced = runRule( + asyncAwaitInLoop, + `async function build(scheduler, items) { for (const item of items) { await scheduler.check(item.progress); } }`, + ); + const independent = runRule( + asyncAwaitInLoop, + `async function build(api, items) { for (const item of items) { results.push(await api.check(item)); } }`, + ); + expect(paced.parseErrors).toEqual([]); + expect(paced.diagnostics).toEqual([]); + expect(independent.parseErrors).toEqual([]); + expect(independent.diagnostics.length).toBeGreaterThan(0); + }); + + it("follows local wrappers to a host-yielding Promise", () => { + const result = runRule( + asyncAwaitInLoop, + `const nextTask = () => new Promise((resolve) => setTimeout(resolve, 0)); + const sliceYield = () => nextTask(); + async function build(items) { for (const item of items) { consume(item); await sliceYield(); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + + it("does not mistake an unrelated scheduled Promise for the awaited result", () => { + const result = runRule( + asyncAwaitInLoop, + `const load = async (item) => { void new Promise((resolve) => setTimeout(resolve, 0)); return fetch(item); }; + async function build(items) { for (const item of items) { results.push(await load(item)); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("keeps work paced by an explicit scheduler argument sequential", () => { + const result = runRule( + asyncAwaitInLoop, + `async function build(jobs, scheduler) { for (const job of jobs) { results.push(await job(scheduler)); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + + it("skips ordered browser-automation driver loops", () => { + const browserDriver = runRule( + asyncAwaitInLoop, + `import { chromium } from "playwright"; + async function drive(page, steps) { for (const step of steps) { await page.evaluate(step); } }`, + { filename: "dev/capture.mjs" }, + ); + const production = runRule( + asyncAwaitInLoop, + `async function load(api, items) { for (const item of items) { results.push(await api.read(item)); } }`, + ); + expect(browserDriver.parseErrors).toEqual([]); + expect(browserDriver.diagnostics).toEqual([]); + expect(production.parseErrors).toEqual([]); + expect(production.diagnostics.length).toBeGreaterThan(0); + }); + it.each(["query", "execute", "wait"])( "flags the pure local %s spelling without trusting its name", (helperName) => { @@ -133,6 +216,15 @@ describe("js-performance/async-await-in-loop — regressions", () => { expect(result.diagnostics).toEqual([]); }); + it("stays silent when each awaited result becomes the next member-call receiver", () => { + const result = runRule( + asyncAwaitInLoop, + `async function descend(root, parts) { let directory = root; for (const part of parts) directory = await directory.getDirectoryHandle(part); return directory; }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + it("stays silent when an awaited shared receiver call bridges ordered state snapshots", () => { const result = runRule( asyncAwaitInLoop, @@ -761,4 +853,22 @@ describe("js-performance/async-await-in-loop — regressions", () => { expect(result.parseErrors).toEqual([]); expect(result.diagnostics.length).toBeGreaterThan(0); }); + + it("stays silent on resolved helpers that deliberately yield to the browser", () => { + const result = runRule( + asyncAwaitInLoop, + `const nextFrame = () => new Promise((resolve) => requestAnimationFrame(() => resolve())); async function load(steps) { for (const step of steps) { build(step); await nextFrame(); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + + it("still flags local helpers whose names imply waiting but whose work is independent", () => { + const result = runRule( + asyncAwaitInLoop, + `const nextFrame = async (item) => Promise.resolve(item * 2); async function load(items) { for (const item of items) { await nextFrame(item); } }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.ts index fd41f81088..562be54418 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-await-in-loop.ts @@ -20,6 +20,7 @@ import { isAstDescendant } from "../../utils/is-ast-descendant.js"; import { isFunctionLike } from "../../utils/is-function-like.js"; import { isInlineFunctionExpression } from "../../utils/is-inline-function-expression.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; +import { isTestLibraryImportSource } from "../../utils/is-test-library-import-source.js"; import { nodeDominatesNode } from "../../utils/node-dominates-node.js"; import { resolveExpressionKey } from "../../utils/resolve-expression-key.js"; import type { RuleContext } from "../../utils/rule-context.js"; @@ -28,6 +29,36 @@ import { walkAst } from "../../utils/walk-ast.js"; const LOOP_STATEMENT_TYPES: ReadonlySet = new Set(LOOP_TYPES); const ORDERED_OUTPUT_INSERTION_METHOD_NAMES = new Set(["push", "unshift"]); +const HOST_YIELD_SCHEDULER_NAMES: ReadonlySet = new Set([ + "queueMicrotask", + "requestAnimationFrame", + "requestIdleCallback", + "setImmediate", + "setTimeout", +]); +const OPAQUE_PACING_CALLEE_NAMES: ReadonlySet = new Set([ + "yieldNow", + "yieldTo", + "yieldToBrowser", + "slice", + "sliceYield", + "nextFrame", + "breathe", + "onYield", + "onProgress", + "progress", + "onStep", + "step", + "runStage", +]); + +const getCalleeName = (callee: EsTreeNode | null | undefined): string | null => { + if (isNodeOfType(callee, "Identifier")) return callee.name; + if (isNodeOfType(callee, "MemberExpression") && isNodeOfType(callee.property, "Identifier")) { + return callee.property.name; + } + return null; +}; const getLoopBody = (loopNode: EsTreeNode): EsTreeNode | null => { if ( @@ -81,17 +112,120 @@ const isIntentionalSequencingCallee = (callee: EsTreeNode | null | undefined): b return INTENTIONAL_SEQUENCING_CALLEE_NAMES.has(callee.name); } if (isNodeOfType(callee, "MemberExpression") && isNodeOfType(callee.property, "Identifier")) { + const receiver = stripParenExpression(callee.object); + if ( + callee.property.name === "check" && + isNodeOfType(receiver, "Identifier") && + /sched(?:uler)?/i.test(receiver.name) + ) { + return true; + } return INTENTIONAL_SEQUENCING_CALLEE_NAMES.has(callee.property.name); } return false; }; +const isGlobalHostSchedulerCall = ( + callExpression: EsTreeNodeOfType<"CallExpression">, + context: RuleContext, +): boolean => { + if ( + isNodeOfType(callExpression.callee, "Identifier") && + HOST_YIELD_SCHEDULER_NAMES.has(callExpression.callee.name) && + context.scopes.isGlobalReference(callExpression.callee) + ) { + return true; + } + if ( + !isNodeOfType(callExpression.callee, "MemberExpression") || + !isNodeOfType(callExpression.callee.property, "Identifier") || + !HOST_YIELD_SCHEDULER_NAMES.has(callExpression.callee.property.name) + ) { + return false; + } + const receiver = stripParenExpression(callExpression.callee.object); + return ( + isNodeOfType(receiver, "Identifier") && + (receiver.name === "globalThis" || receiver.name === "window") && + context.scopes.isGlobalReference(receiver) + ); +}; + +const doesLocalFunctionYieldToHost = ( + localFunction: EsTreeNode, + context: RuleContext, + visitedFunctions: Set = new Set(), +): boolean => { + if (!isFunctionLike(localFunction) || visitedFunctions.has(localFunction)) return false; + visitedFunctions.add(localFunction); + let hasDirectHostYieldPromise = false; + let hasNestedPacingCall = false; + walkAst(localFunction.body, (child: EsTreeNode): boolean | void => { + if (child !== localFunction.body && isFunctionLike(child)) return false; + if ( + isNodeOfType(child, "NewExpression") && + isNodeOfType(child.callee, "Identifier") && + child.callee.name === "Promise" && + context.scopes.isGlobalReference(child.callee) + ) { + const promiseRoot = findTransparentExpressionRoot(child); + const promiseParent = promiseRoot.parent; + const producesFunctionResult = + (isNodeOfType(promiseParent, "ReturnStatement") && + promiseParent.argument === promiseRoot) || + (isNodeOfType(promiseParent, "AwaitExpression") && + promiseParent.argument === promiseRoot) || + (isFunctionLike(promiseParent) && promiseParent.body === promiseRoot); + const executor = child.arguments[0]; + if (producesFunctionResult && isFunctionLike(executor)) { + walkAst(executor.body, (executorChild: EsTreeNode): boolean | void => { + if (executorChild !== executor.body && isFunctionLike(executorChild)) return false; + if ( + isNodeOfType(executorChild, "CallExpression") && + isGlobalHostSchedulerCall(executorChild, context) + ) { + hasDirectHostYieldPromise = true; + } + }); + } + } + if (!isNodeOfType(child, "CallExpression")) return; + if (isGlobalHostSchedulerCall(child, context)) return; + const nestedLocalFunction = resolveStaticLocalCallFunction(child, context.scopes); + if (nestedLocalFunction !== null) { + if (doesLocalFunctionYieldToHost(nestedLocalFunction, context, visitedFunctions)) { + hasNestedPacingCall = true; + } + return; + } + if (isIntentionalSequencingCallee(child.callee)) hasNestedPacingCall = true; + }); + return hasDirectHostYieldPromise || hasNestedPacingCall; +}; + const isAwaitingSleepLikeCall = (awaitNode: EsTreeNode, context: RuleContext): boolean => { if (!isNodeOfType(awaitNode, "AwaitExpression")) return false; const argument = awaitNode.argument; if (!argument) return false; if (!isNodeOfType(argument, "CallExpression")) return false; + if ( + argument.arguments.some((callArgument) => { + if (isNodeOfType(callArgument, "SpreadElement")) return false; + const pacingArgument = stripParenExpression(callArgument); + return ( + isNodeOfType(pacingArgument, "Identifier") && /sched(?:uler)?/i.test(pacingArgument.name) + ); + }) + ) { + return true; + } + const localFunction = resolveStaticLocalCallFunction(argument, context.scopes); + if (localFunction !== null && doesLocalFunctionYieldToHost(localFunction, context)) return true; if (getOrderIndependentLocalFunction(argument, context.scopes) !== null) return false; + const calleeName = getCalleeName(argument.callee); + if (localFunction !== null && calleeName && OPAQUE_PACING_CALLEE_NAMES.has(calleeName)) { + return false; + } return isIntentionalSequencingCallee(argument.callee); }; @@ -1181,6 +1315,7 @@ export const asyncAwaitInLoop = defineRule({ recommendation: "Collect the items, then use `await Promise.all(items.map(...))` so independent work runs at the same time", create: (context: RuleContext) => { + let hasTestLibraryImport = false; const inspectLoop = ( loopNode: | EsTreeNodeOfType<"ForStatement"> @@ -1190,6 +1325,7 @@ export const asyncAwaitInLoop = defineRule({ | EsTreeNodeOfType<"DoWhileStatement">, label: string, ): void => { + if (hasTestLibraryImport) return; const loopBody = loopNode.body; if (!loopBody) return; if (loopBodyHasIntentionallySequentialAwait(loopBody, context)) return; @@ -1214,6 +1350,9 @@ export const asyncAwaitInLoop = defineRule({ }; return { + ImportDeclaration(node: EsTreeNodeOfType<"ImportDeclaration">) { + if (isTestLibraryImportSource(node.source?.value)) hasTestLibraryImport = true; + }, ForStatement(node: EsTreeNodeOfType<"ForStatement">) { inspectLoop(node, "for-loop"); }, @@ -1233,6 +1372,7 @@ export const asyncAwaitInLoop = defineRule({ inspectLoop(node, "do-while loop"); }, CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (hasTestLibraryImport) return; // arr.forEach(async item => { await fn(item); }) — sequential // because forEach doesn't await; even worse, the awaits are // dropped on the floor (forEach ignores return values). diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.regressions.test.ts index 030a14c130..d897d7f1c3 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/async-parallel.regressions.test.ts @@ -297,6 +297,12 @@ describe("js-performance/async-parallel — regressions", () => { ); }); + it("keeps repeated named loading stages sequential", () => { + expectPass( + `async function buildScene() { const terrain = await stage("terrain", () => buildTerrain()); const props = await stage("props", () => buildProps()); const lights = await stage("lights", () => buildLights()); return { terrain, props, lights }; }`, + ); + }); + it("does not flag when a bare expression-statement await depends on an earlier result", () => { expectPass( `async function load(){ const user = await getUser(); await trackVisit(user.id); const posts = await getPosts(); }`, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.regressions.test.ts index 2588a8c128..11c1435a01 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.regressions.test.ts @@ -104,6 +104,22 @@ function f(state, n, use, next) { expect(result.diagnostics).toEqual([]); }); + it("stays silent when a method can mutate a mid-chain receiver between reads", () => { + const result = runRule( + jsCachePropertyAccess, + `function f(points, use) { + for (const point of points) { + use(point.position.x); + point.position.set(1, 2, 3); + use(point.position.x); + use(point.position.x); + } + }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); + it("still flags when an inner callback assigns to a param-shadowed same-named binding", () => { const result = runRule( jsCachePropertyAccess, @@ -124,4 +140,21 @@ function f(items, theme, render, fallback) { expect(result.parseErrors).toEqual([]); expect(result.diagnostics.length).toBeGreaterThan(0); }); + + it("does not count reads in a deferred event callback as outer-loop reads", () => { + const result = runRule( + jsCachePropertyAccess, + `function bind(buttons, theme, render) { + for (const button of buttons) { + button.addEventListener("click", () => { + render(theme.colors.primary); + render(theme.colors.primary); + render(theme.colors.primary); + }); + } + }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toEqual([]); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.ts index f7d63a923a..ba1c5b833a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-cache-property-access.ts @@ -1,5 +1,4 @@ import { PROPERTY_ACCESS_REPEAT_THRESHOLD } from "../../constants/thresholds.js"; -import { collectPatternNames } from "../../utils/collect-pattern-names.js"; import { defineRule } from "../../utils/define-rule.js"; import { isFunctionLike } from "../../utils/is-function-like.js"; import { walkAst } from "../../utils/walk-ast.js"; @@ -28,32 +27,10 @@ const buildMemberAccessKey = (node: EsTreeNode): string | null => { return `${objectKey}.${node.property.name}`; }; -// An assignment to a name that is shadowed by an enclosing nested -// function's parameter rebinds the INNER binding, not the loop-level one, -// so it must not suppress a report about the outer chain. -const isNameShadowedByEnclosingFunctionParameter = ( - node: EsTreeNode, - name: string, - boundary: EsTreeNode, -): boolean => { - let ancestor: EsTreeNode | null | undefined = node.parent; - while (ancestor && ancestor !== boundary) { - if (isFunctionLike(ancestor)) { - const parameterNames = new Set(); - for (const parameter of ancestor.params ?? []) { - collectPatternNames(parameter, parameterNames); - } - if (parameterNames.has(name)) return true; - } - ancestor = ancestor.parent ?? null; - } - return false; -}; - // HACK: detect repeated deep `obj.a.b.c` reads inside the same loop — // JS engines can sometimes optimize, but reads through proxies, getters, // or hot user-code paths often benefit from caching the access in a const -// at the top of the loop body. We require a member-expression depth ≥ 2 +// immediately before its repeated reads. We require a member-expression depth ≥ 2 // (two dots) and ≥ 3 occurrences in the same loop block to fire. export const jsCachePropertyAccess = defineRule({ id: "js-cache-property-access", @@ -61,7 +38,7 @@ export const jsCachePropertyAccess = defineRule({ tags: ["test-noise"], severity: "warn", recommendation: - "Read the value once into a variable at the top of the loop: `const { x, y } = obj.deeply.nested`", + "When the value is unchanged between reads, cache it immediately before the first read inside the loop", create: (context: RuleContext) => { const inspectLoopBody = (loopBody: EsTreeNode): void => { const counts = new Map(); @@ -71,18 +48,25 @@ export const jsCachePropertyAccess = defineRule({ // extends a written prefix, later reads dereference a different // object, so caching the first read would snapshot a stale value. const writtenAccessPrefixes = new Set(); + const calledReceiverPrefixes = new Set(); const recordWriteTarget = (writeTarget: EsTreeNode): void => { const writtenKey = buildMemberAccessKey(writeTarget); if (!writtenKey) return; - const rootName = writtenKey.split(".")[0]; - if (isNameShadowedByEnclosingFunctionParameter(writeTarget, rootName, loopBody)) return; writtenAccessPrefixes.add(writtenKey); }; // Write targets and read counts inspect disjoint node types, and the // write-prefix set is only consulted after the walk — one pass fills both. walkAst(loopBody, (child: EsTreeNode) => { + if (child !== loopBody && isFunctionLike(child)) return false; if (isNodeOfType(child, "AssignmentExpression")) recordWriteTarget(child.left); if (isNodeOfType(child, "UpdateExpression")) recordWriteTarget(child.argument); + if ( + isNodeOfType(child, "CallExpression") && + isNodeOfType(child.callee, "MemberExpression") + ) { + const receiverKey = buildMemberAccessKey(child.callee.object); + if (receiverKey?.includes(".")) calledReceiverPrefixes.add(receiverKey); + } if (!isNodeOfType(child, "MemberExpression")) return; if (child.computed) return; // Skip if this MemberExpression is itself nested inside another (only @@ -115,19 +99,21 @@ export const jsCachePropertyAccess = defineRule({ if (count < PROPERTY_ACCESS_REPEAT_THRESHOLD) continue; const segments = key.split("."); let accessPrefix = segments[0]; - let doesExtendWrittenPrefix = writtenAccessPrefixes.has(accessPrefix); + let doesExtendUnstablePrefix = + writtenAccessPrefixes.has(accessPrefix) || calledReceiverPrefixes.has(accessPrefix); for ( let segmentIndex = 1; - segmentIndex < segments.length && !doesExtendWrittenPrefix; + segmentIndex < segments.length && !doesExtendUnstablePrefix; segmentIndex++ ) { accessPrefix = `${accessPrefix}.${segments[segmentIndex]}`; - doesExtendWrittenPrefix = writtenAccessPrefixes.has(accessPrefix); + doesExtendUnstablePrefix = + writtenAccessPrefixes.has(accessPrefix) || calledReceiverPrefixes.has(accessPrefix); } - if (doesExtendWrittenPrefix) continue; + if (doesExtendUnstablePrefix) continue; context.report({ node: firstNode, - message: `This slows the loop because ${key} is read ${count} times inside it, so read it once into a variable at the top`, + message: `This may slow the loop because ${key} is read ${count} times inside it; if the value stays unchanged between those reads, cache it immediately before the first read`, }); } }; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-index-maps.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-index-maps.regressions.test.ts index f73e30bdbf..118885d986 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-index-maps.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-index-maps.regressions.test.ts @@ -120,4 +120,10 @@ describe("js-performance/js-index-maps — regressions", () => { `function f(values, options){ const labels = []; for (const value of values){ const opt = options.find((o)=> o.value === value); labels.push(opt?.label ?? value); } return labels; }`, ); }); + + it("does not treat a deferred event callback as part of its registration loop", () => { + expectPass( + `function bindButtons(buttons, users){ for (const button of buttons) { button.addEventListener("click", () => { const user = users.find((candidate) => candidate.id === button.dataset.id); show(user); }); } }`, + ); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.regressions.test.ts index db01d6ec2e..e02cc48113 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.regressions.test.ts @@ -134,6 +134,12 @@ describe("js-performance/js-length-check-first — regressions", () => { }`); }); + it("stays silent when every applies a non-equality constraint to a fixed tuple", () => { + expectPass(`function canAfford(cost: [number, number, number], owned: [number, number, number]) { + return cost.every((amount, index) => owned[index] >= amount); + }`); + }); + it("stays silent when the receiver is a map of the indexed array", () => { expectPass(`function unchanged(state) { const updatedServers = state.servers.map((s) => update(s)); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.ts index 69e9c9f9e9..24d51f37d3 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-length-check-first.ts @@ -129,6 +129,31 @@ const findIndexedArrayObject = ( return indexedArrayObject; }; +const EQUALITY_OPERATORS: ReadonlySet = new Set(["===", "==", "!==", "!="]); + +const hasIndexedAccessWithinEquality = ( + callbackBody: EsTreeNode, + indexParameterName: string, +): boolean => { + let hasIndexedEquality = false; + walkAst(callbackBody, (child: EsTreeNode) => { + if (hasIndexedEquality || !isNodeOfType(child, "BinaryExpression")) return; + if (!EQUALITY_OPERATORS.has(child.operator)) return; + walkAst(child, (comparisonChild: EsTreeNode) => { + if ( + isNodeOfType(comparisonChild, "MemberExpression") && + comparisonChild.computed && + isNodeOfType(comparisonChild.property, "Identifier") && + comparisonChild.property.name === indexParameterName + ) { + hasIndexedEquality = true; + return false; + } + }); + }); + return hasIndexedEquality; +}; + const unwrapChainExpression = (node: EsTreeNode): EsTreeNode => isNodeOfType(node, "ChainExpression") ? node.expression : node; @@ -346,18 +371,19 @@ const peelLengthPreservingDerivation = (expression: EsTreeNode): EsTreeNode => { const callee = unwrapChainExpression(current.callee); if (isNodeOfType(callee, "MemberExpression") && isNodeOfType(callee.property, "Identifier")) { const calleeObject = unwrapChainExpression(callee.object); + const calleePropertyName = callee.property.name; if ( isNodeOfType(calleeObject, "Identifier") && calleeObject.name === "Array" && - callee.property.name === "from" && + calleePropertyName === "from" && current.arguments?.length === 1 ) { current = unwrapChainExpression(current.arguments[0]); continue; } - if (LENGTH_PRESERVING_METHOD_NAMES.has(callee.property.name)) { + if (LENGTH_PRESERVING_METHOD_NAMES.has(calleePropertyName)) { const isBoundedSlice = - callee.property.name === "slice" && (current.arguments ?? []).length > 0; + calleePropertyName === "slice" && (current.arguments ?? []).length > 0; if (!isBoundedSlice) { current = calleeObject; continue; @@ -477,6 +503,7 @@ export const jsLengthCheckFirst = defineRule({ if (callbackParameters.length < 2) return; // need (item, index, ...) to address other array const indexParameter = callbackParameters[1]; if (!isNodeOfType(indexParameter, "Identifier")) return; + if (!hasIndexedAccessWithinEquality(callback.body, indexParameter.name)) return; const indexedArrayObject = findIndexedArrayObject(callback.body, indexParameter.name); if (!indexedArrayObject) return; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups-reactbench-regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups-reactbench-regressions.test.ts index fbb7a0722b..1b3e50849b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups-reactbench-regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups-reactbench-regressions.test.ts @@ -98,6 +98,45 @@ describe("js-set-map-lookups — ReactBench regressions", () => { `); }); + it("stays silent when slice rebuilds the receiver for each lookup", () => { + expectNoDiagnostics(` + for (let index = 0; index < sequence.length; index += 1) { + if (sequence.slice(0, index).includes(index)) continue; + } + `); + }); + + it("stays silent when a classic for-loop index selects a different receiver", () => { + expectNoDiagnostics(` + function findOwner(targets, selected) { + for (let index = 0; index < targets.length; index += 1) { + if (targets[index].children.includes(selected)) return index; + } + } + `); + }); + + it("stays silent when a call receiver depends on the iteration binding", () => { + expectNoDiagnostics(` + function findBuilding(buildings, selected) { + for (const building of buildings) { + if (building.getGlassMeshes().includes(selected)) return building; + } + } + `); + }); + + it("stays silent when a call receiver is declared inside the loop", () => { + expectNoDiagnostics(` + function findBuilding(level, hits) { + for (const hit of hits) { + const building = level.buildingForMesh(hit); + if (building.getGlassMeshes().includes(hit)) return building; + } + } + `); + }); + it("reports an unproven userland transform method that can return a cached large array", () => { expectDiagnostic(` const cachedValues = ["a", "b", "c", "d", "e", "f", "g", "h", "i"]; @@ -115,6 +154,22 @@ describe("js-set-map-lookups — ReactBench regressions", () => { `); }); + it("uses an explicit string annotation instead of guessing from a generic name", () => { + expectNoDiagnostics(` + function findMatches(rows: string[], name: string) { + return rows.filter((row) => name.includes(row)); + } + `); + }); + + it("still reports an explicitly typed array with a string-like variable name", () => { + expectDiagnostic(` + function findMatches(rows: string[], name: string[]) { + return rows.filter((row) => name.includes(row)); + } + `); + }); + it("stays silent through transparent wrappers on proven strings", () => { expectNoDiagnostics(` const rows: Array<{ offset: number; query: string }> = []; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups.ts index 37cc44e162..96049a345f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/js-set-map-lookups.ts @@ -348,6 +348,7 @@ const isFreshArrayReceiver = (receiver: EsTreeNode): boolean => { if (receiver.callee.property.name === "split") { return isLikelyStringReceiver(receiver.callee.object); } + if (receiver.callee.property.name === "slice") return true; const sourceReceiver = stripParenExpression(receiver.callee.object); return isKnownNativeArrayReceiver(sourceReceiver) || isFreshArrayReceiver(sourceReceiver); }; @@ -768,7 +769,11 @@ const getArrayElementType = (typeNode: EsTreeNode | null): EsTreeNode | null => return typeNode.typeArguments?.params?.[0] ?? null; } if (isNodeOfType(typeNode, "TSUnionType")) { - const arrayElementTypes = typeNode.types.map(getArrayElementType).filter(Boolean); + const arrayElementTypes: EsTreeNode[] = []; + for (const unionMemberType of typeNode.types) { + const arrayElementType = getArrayElementType(unionMemberType); + if (arrayElementType) arrayElementTypes.push(arrayElementType); + } return arrayElementTypes.length === 1 ? arrayElementTypes[0] : null; } return null; @@ -860,6 +865,21 @@ const getIdentifierDeclaredType = ( return null; }; +const isStringType = (typeNode: EsTreeNode | null): boolean => { + if (!typeNode) return false; + if (isNodeOfType(typeNode, "TSStringKeyword")) return true; + if (isNodeOfType(typeNode, "TSLiteralType")) { + return isNodeOfType(typeNode.literal, "Literal") && typeof typeNode.literal.value === "string"; + } + if (isNodeOfType(typeNode, "TSUnionType")) { + return typeNode.types.length > 0 && typeNode.types.every(isStringType); + } + return false; +}; + +const isDeclaredStringReceiver = (receiver: EsTreeNode): boolean => + isNodeOfType(receiver, "Identifier") && isStringType(getIdentifierDeclaredType(receiver)); + const isNativeIterationIndex = (identifier: EsTreeNodeOfType<"Identifier">): boolean => { const binding = findVariableInitializer(identifier, identifier.name); if (!binding) return false; @@ -1530,15 +1550,16 @@ const findNearestLoopContext = (node: EsTreeNode): EsTreeNode | null => { // pass — converting it to a Set each iteration costs more than the scan, // so hoisting advice does not apply. const isReceiverDeclaredInNearestLoop = (receiver: EsTreeNode, lookupCall: EsTreeNode): boolean => { - if (!isNodeOfType(receiver, "Identifier")) return false; - const binding = findVariableInitializer(receiver, receiver.name); - if (!binding || !binding.initializer) return false; const nearestLoop = findNearestLoopContext(lookupCall); if (!nearestLoop) return false; - let ancestor: EsTreeNode | null | undefined = binding.bindingIdentifier; - while (ancestor) { - if (ancestor === nearestLoop) return true; - ancestor = ancestor.parent; + for (const dependencyName of collectReceiverDependencyNames(receiver)) { + const binding = findVariableInitializer(receiver, dependencyName); + if (!binding?.initializer) continue; + let ancestor: EsTreeNode | null | undefined = binding.bindingIdentifier; + while (ancestor) { + if (ancestor === nearestLoop) return true; + ancestor = ancestor.parent; + } } return false; }; @@ -1553,6 +1574,14 @@ const collectEnclosingLoopIterationBindingNames = (lookupCall: EsTreeNode): Set< const iterationNames = new Set(); let ancestor: EsTreeNode | null | undefined = lookupCall.parent; while (ancestor) { + if (isNodeOfType(ancestor, "ForStatement")) { + const initializer = ancestor.init; + if (isNodeOfType(initializer, "VariableDeclaration")) { + for (const declarator of initializer.declarations ?? []) { + if (declarator.id) collectPatternNames(declarator.id, iterationNames); + } + } + } if (isNodeOfType(ancestor, "ForOfStatement") || isNodeOfType(ancestor, "ForInStatement")) { const left = ancestor.left; if (isNodeOfType(left, "VariableDeclaration")) { @@ -1580,14 +1609,22 @@ const collectEnclosingLoopIterationBindingNames = (lookupCall: EsTreeNode): Set< // chain: `BACKEND_URLS[key]` depends on both `BACKEND_URLS` and `key`. const collectReceiverDependencyNames = (receiver: EsTreeNode): Set => { const dependencyNames = new Set(); - let current = stripParenExpression(receiver); - while (isNodeOfType(current, "MemberExpression")) { - if (current.computed && isNodeOfType(current.property, "Identifier")) { - dependencyNames.add(current.property.name); + const collectFromExpression = (expression: EsTreeNode): void => { + const current = stripParenExpression(expression); + if (isNodeOfType(current, "Identifier")) { + dependencyNames.add(current.name); + return; } - current = stripParenExpression(current.object); - } - if (isNodeOfType(current, "Identifier")) dependencyNames.add(current.name); + if (isNodeOfType(current, "MemberExpression")) { + collectFromExpression(current.object); + if (current.computed) collectFromExpression(current.property); + return; + } + if (isNodeOfType(current, "CallExpression")) { + collectFromExpression(current.callee); + } + }; + collectFromExpression(receiver); return dependencyNames; }; @@ -1702,6 +1739,7 @@ export const jsSetMapLookups = defineRule({ return; } if (isLikelyStringReceiver(receiver)) return; + if (isDeclaredStringReceiver(receiver)) return; if (isFreshArrayReceiver(receiver)) return; if (isTypeScriptRestHelperLookup(node, receiver, context.scopes)) return; if (isSmallInlineLiteralArray(receiver)) return; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.test.ts index 3dd731b739..b18926a934 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.test.ts @@ -1756,4 +1756,17 @@ describe("no-create-object-url-without-revoke", () => { ); expect(result.diagnostics).toHaveLength(0); }); + + it("accepts a revoke scheduled through the global window", () => { + const result = runRule( + noCreateObjectUrlWithoutRevoke, + `const download = (blob) => { + const url = URL.createObjectURL(blob); + link.href = url; + link.click(); + window.setTimeout(() => URL.revokeObjectURL(url), 4000); + };`, + ); + expect(result.diagnostics).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.ts index 8d807e2f61..b7d220e74f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/js-performance/no-create-object-url-without-revoke.ts @@ -952,13 +952,32 @@ const isGuaranteedScheduledRevoke = ( return false; } const scheduler = stripParenExpression(schedulerCall.callee); - if ( - !isNodeOfType(scheduler, "Identifier") || - (scheduler.name !== "queueMicrotask" && scheduler.name !== "setTimeout") || - !isProvenGlobalNamespaceReference(scheduler, scheduler.name, context.scopes) - ) { - return false; - } + const schedulerName = isNodeOfType(scheduler, "Identifier") + ? scheduler.name + : isNodeOfType(scheduler, "MemberExpression") + ? getStaticPropertyName(scheduler) + : null; + if (schedulerName !== "queueMicrotask" && schedulerName !== "setTimeout") return false; + const isDirectGlobalScheduler = + isNodeOfType(scheduler, "Identifier") && + isProvenGlobalNamespaceReference(scheduler, schedulerName, context.scopes); + const schedulerObject = isNodeOfType(scheduler, "MemberExpression") + ? stripParenExpression(scheduler.object) + : null; + const isGlobalObjectScheduler = Boolean( + schedulerObject && + isNodeOfType(schedulerObject, "Identifier") && + (schedulerObject.name === "window" || + schedulerObject.name === "globalThis" || + schedulerObject.name === "self") && + isProvenUnmodifiedGlobalNamespaceReference( + schedulerObject, + schedulerObject.name, + context.scopes, + schedulerName, + ), + ); + if (!isDirectGlobalScheduler && !isGlobalObjectScheduler) return false; return ( context.cfg.enclosingFunction(schedulerCall) === executionBoundary && context.cfg.isUnconditionalFromEntry(revokeCall) && diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/constants.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/constants.ts index c52e16d053..20f3087741 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/constants.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/constants.ts @@ -14,7 +14,6 @@ export const ORTHOGRAPHIC_CAMERA_TOP_ARGUMENT_INDEX = 2; export const ORTHOGRAPHIC_CAMERA_BOTTOM_ARGUMENT_INDEX = 3; export const ORTHOGRAPHIC_CAMERA_NEAR_ARGUMENT_INDEX = 4; export const ORTHOGRAPHIC_CAMERA_FAR_ARGUMENT_INDEX = 5; -export const MINIMUM_ORTHOGRAPHIC_CAMERA_NEAR = 0; export const RAYCASTER_NEAR_ARGUMENT_INDEX = 2; export const RAYCASTER_FAR_ARGUMENT_INDEX = 3; export const MINIMUM_RAYCASTER_NEAR = 0; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-prefer-gpu-position-animation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-prefer-gpu-position-animation.ts index 207475b88d..47f289e91d 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-prefer-gpu-position-animation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-prefer-gpu-position-animation.ts @@ -31,6 +31,7 @@ export const r3fPreferGpuPositionAnimation = defineRule({ callback, context, managedPositionBufferRefSymbolIds, + false, )[0]; if (!firstMutation) return; context.report({ diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.test.ts index 8b862eed15..b1b1702a1a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.test.ts @@ -12,7 +12,7 @@ describe("r3f-valid-orthographic-camera", () => { ; `; - expect(runRule(r3fValidOrthographicCamera, code).diagnostics).toHaveLength(3); + expect(runRule(r3fValidOrthographicCamera, code).diagnostics).toHaveLength(2); }); it("allows valid, dynamic, spread, and perspective configurations", () => { @@ -20,6 +20,7 @@ describe("r3f-valid-orthographic-camera", () => { import { Canvas } from "@react-three/fiber"; export const Scene = (props) => <> + diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.ts index 9761cad35b..09ece0d1da 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/r3f-valid-orthographic-camera.ts @@ -94,7 +94,7 @@ export const r3fValidOrthographicCamera = defineRule({ tags: ["react-jsx-only"], severity: "error", recommendation: - "Use distinct horizontal and vertical frustum planes, a nonnegative near plane, and a far plane greater than near", + "Use distinct horizontal and vertical frustum planes and a far plane greater than near", create: (context: RuleContext) => { let importsReactThreeFiber = false; return { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.test.ts index fa060ebe66..7f3008008c 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.test.ts @@ -13,6 +13,8 @@ describe("three-effect-composer-require-size-on-resize", () => { it.each([ `import { WebGLRenderer } from "three"; import { EffectComposer } from "three/addons/postprocessing/EffectComposer.js"; const renderer = new WebGLRenderer(); const composer = new EffectComposer(renderer); window.addEventListener("resize", () => { renderer.setSize(width, height); composer.setSize(width, height); });`, + `import { WebGLRenderer } from "three"; import { EffectComposer } from "three/addons/postprocessing/EffectComposer.js"; const renderer = new WebGLRenderer(); let composer: EffectComposer | undefined = new EffectComposer(renderer); window.addEventListener("resize", () => { renderer.setSize(width, height); composer?.setSize(width, height); });`, + `import { WebGLRenderer } from "three"; import { EffectComposer } from "three/addons/postprocessing/EffectComposer.js"; const renderer = new WebGLRenderer(); let composer: EffectComposer | null = null; if (enabled) { const nextComposer = new EffectComposer(renderer); composer = nextComposer; } window.addEventListener("resize", () => { renderer.setSize(width, height); composer?.setSize(width, height); });`, `import { WebGLRenderer } from "three"; import { EffectComposer } from "three/addons/postprocessing/EffectComposer.js"; import { resizeComposer } from "./resize.js"; const renderer = new WebGLRenderer(); const composer = new EffectComposer(renderer); window.addEventListener("resize", () => { renderer.setSize(width, height); resizeComposer(composer, width, height); });`, `import { WebGLRenderer } from "three"; import { EffectComposer } from "three/addons/postprocessing/EffectComposer.js"; const firstRenderer = new WebGLRenderer(); const secondRenderer = new WebGLRenderer(); const composer = new EffectComposer(firstRenderer); window.addEventListener("resize", () => secondRenderer.setSize(width, height));`, `class EffectComposer {} class WebGLRenderer { setSize() {} } const renderer = new WebGLRenderer(); const composer = new EffectComposer(renderer); window.onresize = () => renderer.setSize(1, 1);`, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.ts index b3e67b42eb..a56e16ea16 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-effect-composer-require-size-on-resize.ts @@ -55,10 +55,6 @@ const collectResizeFacts = ( const methodName = getStaticPropertyName(candidate.callee); const targetKey = resolveExpressionKey(candidate.callee.object, context); const constructorName = getThreeConstructorName(candidate.callee.object, context.scopes); - if (methodName === "setSize" && targetKey && constructorName === "EffectComposer") { - resizedComposerKeys.add(targetKey); - return; - } if ( methodName === "setSize" && targetKey && @@ -67,6 +63,10 @@ const collectResizeFacts = ( rendererResizes.set(targetKey, candidate); return; } + if (methodName === "setSize" && targetKey) { + resizedComposerKeys.add(targetKey); + return; + } } if (!isImportedOrStableParameterCall(candidate, context.scopes)) return; for (const argument of candidate.arguments) { @@ -96,6 +96,7 @@ export const threeEffectComposerRequireSizeOnResize = defineRule({ recommendation: "Resize every EffectComposer associated with a resized renderer", create: (context: RuleContext) => { const analyzedCallbacks = new Set(); + const assignmentAliases: Array = []; const composerBindings: EffectComposerBinding[] = []; const resizeFacts: EffectComposerResizeFact[] = []; const analyzeResizeSource = ( @@ -108,6 +109,11 @@ export const threeEffectComposerRequireSizeOnResize = defineRule({ }; return { AssignmentExpression(node: EsTreeNodeOfType<"AssignmentExpression">) { + if (node.operator === "=") { + const targetKey = resolveExpressionKey(node.left, context); + const sourceKey = resolveExpressionKey(node.right, context); + if (targetKey && sourceKey) assignmentAliases.push([targetKey, sourceKey]); + } analyzeResizeSource(node); }, CallExpression(node: EsTreeNodeOfType<"CallExpression">) { @@ -121,11 +127,28 @@ export const threeEffectComposerRequireSizeOnResize = defineRule({ if (binding) composerBindings.push(binding); }, "Program:exit"() { + const areKeysAliased = (leftKey: string, rightKey: string): boolean => { + const pendingKeys = [leftKey]; + const visitedKeys = new Set(); + while (pendingKeys.length > 0) { + const currentKey = pendingKeys.pop(); + if (!currentKey || visitedKeys.has(currentKey)) continue; + if (currentKey === rightKey) return true; + visitedKeys.add(currentKey); + for (const [aliasTarget, aliasSource] of assignmentAliases) { + if (aliasTarget === currentKey) pendingKeys.push(aliasSource); + if (aliasSource === currentKey) pendingKeys.push(aliasTarget); + } + } + return false; + }; for (const resizeFact of resizeFacts) { const staleComposer = composerBindings.find( (binding) => binding.rendererKey === resizeFact.rendererKey && - !resizeFact.resizedComposerKeys.has(binding.composerKey) && + ![...resizeFact.resizedComposerKeys].some((resizedKey) => + areKeysAliased(binding.composerKey, resizedKey), + ) && !resizeFact.delegatedComposerKeys.has(binding.composerKey), ); if (!staleComposer) continue; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.test.ts index e7ae197296..3b07579f21 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.test.ts @@ -28,4 +28,38 @@ describe("three-no-compile-in-animation-loop", () => { `; expect(runRule(threeNoCompileInAnimationLoop, code).diagnostics).toHaveLength(0); }); + + it("allows a one-shot compile stage that advances after its switch", () => { + const code = ` + import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + let loadingStage = 0; + const buildNextStage = () => { + switch (loadingStage) { + case 4: + renderer.compileAsync(scene, camera); + break; + } + loadingStage += 1; + }; + renderer.setAnimationLoop(() => buildNextStage()); + `; + expect(runRule(threeNoCompileInAnimationLoop, code).diagnostics).toHaveLength(0); + }); + + it("reports a compile in a switch that does not advance its stage", () => { + const code = ` + import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + let loadingStage = 0; + renderer.setAnimationLoop(() => { + switch (loadingStage) { + case 0: + renderer.compile(scene, camera); + break; + } + }); + `; + expect(runRule(threeNoCompileInAnimationLoop, code).diagnostics).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.ts index a2f75699aa..be8285fe6b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-compile-in-animation-loop.ts @@ -4,12 +4,58 @@ import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { getStaticPropertyName } from "../../utils/get-static-property-name.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; import type { RuleContext } from "../../utils/rule-context.js"; +import { walkAst } from "../../utils/walk-ast.js"; import { isThreeRendererReference } from "./utils/is-three-renderer-reference.js"; import { resolveThreeAnimationLoopCallback } from "./utils/resolve-three-animation-loop-callback.js"; import { walkFunctionExecution } from "./utils/walk-function-execution.js"; const THREE_COMPILE_METHOD_NAMES: ReadonlySet = new Set(["compile", "compileAsync"]); +const isInsideAdvancingSwitchStage = (node: EsTreeNode): boolean => { + let switchCase: EsTreeNode | null = null; + let ancestor: EsTreeNode | null | undefined = node.parent; + while (ancestor) { + if (isNodeOfType(ancestor, "SwitchCase")) { + switchCase = ancestor; + break; + } + ancestor = ancestor.parent; + } + const switchStatement = switchCase?.parent; + if (!switchStatement || !isNodeOfType(switchStatement, "SwitchStatement")) return false; + if (!isNodeOfType(switchStatement.discriminant, "Identifier")) return false; + const stageName = switchStatement.discriminant.name; + const block = switchStatement.parent; + if (!block || !isNodeOfType(block, "BlockStatement")) return false; + const switchIndex = block.body.findIndex((statement) => statement === switchStatement); + if (switchIndex === -1) return false; + let advancesStage = false; + for (const statement of block.body.slice(switchIndex + 1)) { + walkAst(statement, (candidate) => { + if (advancesStage) return false; + if ( + isNodeOfType(candidate, "UpdateExpression") && + isNodeOfType(candidate.argument, "Identifier") && + candidate.argument.name === stageName + ) { + advancesStage = true; + return false; + } + if ( + isNodeOfType(candidate, "AssignmentExpression") && + candidate.operator !== "=" && + isNodeOfType(candidate.left, "Identifier") && + candidate.left.name === stageName + ) { + advancesStage = true; + return false; + } + }); + if (advancesStage) return true; + } + return false; +}; + export const threeNoCompileInAnimationLoop = defineRule({ id: "three-no-compile-in-animation-loop", title: "Three.js shader precompilation inside animation loop", @@ -29,7 +75,8 @@ export const threeNoCompileInAnimationLoop = defineRule({ !isNodeOfType(candidate, "CallExpression") || !isNodeOfType(candidate.callee, "MemberExpression") || !THREE_COMPILE_METHOD_NAMES.has(getStaticPropertyName(candidate.callee) ?? "") || - !isThreeRendererReference(candidate.callee.object, context.scopes) + !isThreeRendererReference(candidate.callee.object, context.scopes) || + isInsideAdvancingSwitchStage(candidate) ) { return; } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.test.ts index 7770bdf8b9..8d168b69cd 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.test.ts @@ -49,6 +49,28 @@ describe("three-no-new-in-animation-loop", () => { expect(result.diagnostics).toHaveLength(0); }); + it.each(["??=", "||="])("ignores a truthy lazy initializer using %s", (operator) => { + const result = runRule( + threeNoNewInAnimationLoop, + `import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + let positions; + renderer.setAnimationLoop(() => { positions ${operator} new Float32Array(12); });`, + ); + expect(result.diagnostics).toHaveLength(0); + }); + + it("retains a repeated &&= allocation", () => { + const result = runRule( + threeNoNewInAnimationLoop, + `import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + let positions = new Float32Array(12); + renderer.setAnimationLoop(() => { positions &&= new Float32Array(12); });`, + ); + expect(result.diagnostics).toHaveLength(1); + }); + it.each([ `const renderer = createRenderer(); renderer.setAnimationLoop(() => new Vector3());`, `import { WebGLRenderer } from "other-renderer"; const renderer = new WebGLRenderer(); renderer.setAnimationLoop(() => new Vector3());`, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.ts index c6482ee929..c588a53420 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-no-new-in-animation-loop.ts @@ -5,6 +5,15 @@ import type { RuleContext } from "../../utils/rule-context.js"; import { resolveThreeAnimationLoopCallback } from "./utils/resolve-three-animation-loop-callback.js"; import { walkFunctionExecution } from "./utils/walk-function-execution.js"; +const isTruthyLazyInitializer = (node: EsTreeNode): boolean => { + const parent = node.parent; + return ( + parent?.type === "AssignmentExpression" && + parent.right === node && + (parent.operator === "??=" || parent.operator === "||=") + ); +}; + export const threeNoNewInAnimationLoop = defineRule({ id: "three-no-new-in-animation-loop", title: "Allocation inside Three.js animation loop", @@ -19,7 +28,12 @@ export const threeNoNewInAnimationLoop = defineRule({ if (!callback || analyzedCallbacks.has(callback)) return; analyzedCallbacks.add(callback); walkFunctionExecution(callback, context.scopes, (candidate, isConditionallyExecuted) => { - if (candidate.type !== "NewExpression" || isConditionallyExecuted) return; + if ( + candidate.type !== "NewExpression" || + isConditionallyExecuted || + isTruthyLazyInitializer(candidate) + ) + return; context.report({ node: candidate, message: diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.test.ts index b0ab910373..25cbb7e2f6 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.test.ts @@ -33,4 +33,22 @@ describe("three-prefer-gpu-instanced-animation", () => { `; expect(runRule(threePreferGpuInstancedAnimation, code).diagnostics).toHaveLength(0); }); + + it("allows a dirty-gated batch that does not animate every frame", () => { + const code = ` + import { InstancedMesh, WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + const instances = new InstancedMesh(geometry, material, count); + const updateInstances = () => { + for (let index = 0; index < count; index += 1) { + instances.setMatrixAt(index, matrix); + } + }; + renderer.setAnimationLoop(() => { + if (isDirty) updateInstances(); + renderer.render(scene, camera); + }); + `; + expect(runRule(threePreferGpuInstancedAnimation, code).diagnostics).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.ts index ad4d30d8e0..97439cbf9e 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-instanced-animation.ts @@ -23,8 +23,9 @@ export const threePreferGpuInstancedAnimation = defineRule({ const callback = resolveThreeAnimationLoopCallback(node, context.scopes); if (!callback || analyzedCallbacks.has(callback)) return; analyzedCallbacks.add(callback); - walkFunctionExecution(callback, context.scopes, (candidate) => { + walkFunctionExecution(callback, context.scopes, (candidate, isConditionallyExecuted) => { if ( + isConditionallyExecuted || !isNodeOfType(candidate, "CallExpression") || !isNodeOfType(candidate.callee, "MemberExpression") || getStaticPropertyName(candidate.callee) !== "setMatrixAt" || diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.test.ts index fa8869d943..824f563f23 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.test.ts @@ -86,6 +86,23 @@ describe("three-prefer-gpu-position-animation", () => { expect(runRule(threePreferGpuPositionAnimation, code).diagnostics).toHaveLength(0); }); + it("allows conditionally streamed position data that is not continuous animation", () => { + const code = ` + import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + const positions = particles.geometry.attributes.position; + renderer.setAnimationLoop(() => { + if (hasNewTrace) { + for (let index = 0; index < positions.count; index += 1) { + positions.setXYZ(index, trace[index].x, trace[index].y, trace[index].z); + } + positions.needsUpdate = true; + } + }); + `; + expect(runRule(threePreferGpuPositionAnimation, code).diagnostics).toHaveLength(0); + }); + it("ignores non-rendering loops and unrelated renderers", () => { const code = ` const update = () => { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.ts index a0e06929f0..67e0e0788e 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-gpu-position-animation.ts @@ -19,7 +19,12 @@ export const threePreferGpuPositionAnimation = defineRule({ const callback = resolveThreeAnimationLoopCallback(node, context.scopes); if (!callback || analyzedCallbacks.has(callback)) return; analyzedCallbacks.add(callback); - const firstMutation = findRepeatedPositionBufferMutations(callback, context)[0]; + const firstMutation = findRepeatedPositionBufferMutations( + callback, + context, + new Set(), + false, + )[0]; if (!firstMutation) return; context.report({ node: firstMutation, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.test.ts index d9d4e24a10..75ef4ae65a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.test.ts @@ -3,24 +3,47 @@ import { runRule } from "../../../test-utils/run-rule.js"; import { threePreferSetAnimationLoop } from "./three-prefer-set-animation-loop.js"; describe("three-prefer-set-animation-loop", () => { - it("reports recursive animation frames that render with Three.js", () => { + it("reports recursive animation frames in a WebXR renderer", () => { const code = ` import { WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); + renderer.xr.enabled = true; function frame() { renderer.render(scene, camera); requestAnimationFrame(frame); } requestAnimationFrame(frame); `; expect(runRule(threePreferSetAnimationLoop, code).diagnostics).toHaveLength(1); }); - it("allows renderer-managed, unrelated, shadowed, and nonrendering animation frames", () => { + it("allows non-XR manual frames, renderer-managed frames, and unrelated callbacks", () => { const code = ` import { WebGLRenderer } from "three"; const renderer = new WebGLRenderer(); + function frame() { renderer.render(scene, camera); requestAnimationFrame(frame); } + requestAnimationFrame(frame); renderer.setAnimationLoop(() => renderer.render(scene, camera)); requestAnimationFrame(() => updateDom()); const run = (requestAnimationFrame) => requestAnimationFrame(() => renderer.render(scene, camera)); `; expect(runRule(threePreferSetAnimationLoop, code).diagnostics).toHaveLength(0); }); + + it("recognizes an imported WebXR session button without trusting unrelated xr properties", () => { + const webXr = ` + import { WebGLRenderer } from "three"; + import { VRButton } from "three/addons/webxr/VRButton.js"; + const renderer = new WebGLRenderer(); + function frame() { renderer.render(scene, camera); requestAnimationFrame(frame); } + requestAnimationFrame(frame); + document.body.append(VRButton.createButton(renderer)); + `; + const unrelated = ` + import { WebGLRenderer } from "three"; + const renderer = new WebGLRenderer(); + state.xr.enabled = true; + function frame() { renderer.render(scene, camera); requestAnimationFrame(frame); } + requestAnimationFrame(frame); + `; + expect(runRule(threePreferSetAnimationLoop, webXr).diagnostics).toHaveLength(1); + expect(runRule(threePreferSetAnimationLoop, unrelated).diagnostics).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.ts index 3ae16d1a1d..adf4aad752 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-prefer-set-animation-loop.ts @@ -2,8 +2,19 @@ import { defineRule } from "../../utils/define-rule.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { isGlobalAnimationFrameCallee } from "../../utils/is-global-animation-frame-callee.js"; +import { getStaticPropertyName } from "../../utils/get-static-property-name.js"; +import { isNodeOfType } from "../../utils/is-node-of-type.js"; import type { RuleContext } from "../../utils/rule-context.js"; import { resolveThreeAnimationLoopCallback } from "./utils/resolve-three-animation-loop-callback.js"; +import { getThreeConstructorName } from "./utils/get-three-constructor-name.js"; + +const WEB_XR_BUTTON_EXPORT_NAMES = new Set(["ARButton", "VRButton", "XRButton"]); + +const isThreeRendererXrMember = (node: EsTreeNode, context: RuleContext): boolean => { + if (!isNodeOfType(node, "MemberExpression") || getStaticPropertyName(node) !== "xr") return false; + const constructorName = getThreeConstructorName(node.object, context.scopes); + return constructorName === "WebGLRenderer" || constructorName === "WebGPURenderer"; +}; export const threePreferSetAnimationLoop = defineRule({ id: "three-prefer-set-animation-loop", @@ -14,17 +25,49 @@ export const threePreferSetAnimationLoop = defineRule({ "Use renderer.setAnimationLoop for Three.js animation-loop compatibility, including WebXR", create: (context: RuleContext) => { const reportedCallbacks = new Set(); + const manualAnimationFrames: EsTreeNodeOfType<"CallExpression">[] = []; + let usesWebXr = false; return { + ImportDeclaration(node: EsTreeNodeOfType<"ImportDeclaration">) { + if (!/\b(?:webxr|xr)\b/i.test(String(node.source.value))) return; + if ( + node.specifiers.some( + (specifier) => + isNodeOfType(specifier, "ImportSpecifier") && + isNodeOfType(specifier.imported, "Identifier") && + WEB_XR_BUTTON_EXPORT_NAMES.has(specifier.imported.name), + ) + ) { + usesWebXr = true; + } + }, + MemberExpression(node: EsTreeNodeOfType<"MemberExpression">) { + if (isThreeRendererXrMember(node, context)) usesWebXr = true; + if ( + getStaticPropertyName(node) === "xr" && + isNodeOfType(node.object, "Identifier") && + node.object.name === "navigator" && + context.scopes.isGlobalReference(node.object) + ) { + usesWebXr = true; + } + }, CallExpression(node: EsTreeNodeOfType<"CallExpression">) { if (!isGlobalAnimationFrameCallee(node.callee, context.scopes)) return; const callback = resolveThreeAnimationLoopCallback(node, context.scopes); if (!callback || reportedCallbacks.has(callback)) return; reportedCallbacks.add(callback); - context.report({ - node, - message: - "This Three.js render loop is driven by requestAnimationFrame. Use renderer.setAnimationLoop(callback) for renderer-managed timing and WebXR compatibility", - }); + manualAnimationFrames.push(node); + }, + "Program:exit"() { + if (!usesWebXr) return; + for (const node of manualAnimationFrames) { + context.report({ + node, + message: + "This WebXR-capable Three.js render loop is driven by requestAnimationFrame. Use renderer.setAnimationLoop(callback) so immersive sessions receive frames", + }); + } }, }; }, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.test.ts index f2fd6ca64e..9498a62fa1 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.test.ts @@ -59,4 +59,205 @@ describe("three-require-instanced-buffer-update", () => { expect(runRule(threeRequireInstancedBufferUpdate, missing).diagnostics).toHaveLength(1); expect(runRule(threeRequireInstancedBufferUpdate, covered).diagnostics).toHaveLength(0); }); + + it("allows an upload after a synchronous iteration callback", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + const update = () => { + matrices.forEach((matrix, index) => mesh.setMatrixAt(index, matrix)); + mesh.instanceMatrix.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("allows a matching instance-color existence guard after setColorAt", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + const update = () => { + mesh.setColorAt(0, color); + if (mesh.instanceColor) mesh.instanceColor.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("allows an upload guarded by a mutation flag", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + const update = () => { + let dirty = false; + for (const entry of entries) { + if (!entry.visible) continue; + mesh.setColorAt(entry.index, entry.color); + dirty = true; + } + if (dirty && mesh.instanceColor) mesh.instanceColor.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("does not trust a mutation flag that is not set on every path after the mutation", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + const update = () => { + let dirty = false; + for (const entry of entries) { + mesh.setColorAt(entry.index, entry.color); + if (entry.skipUpload) continue; + dirty = true; + } + if (dirty && mesh.instanceColor) mesh.instanceColor.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); + + it("allows uploads through a for-of alias over a static mesh list", () => { + const code = ` + import { InstancedMesh } from "three"; + const bodies = new InstancedMesh(geometry, material, count); + const caps = new InstancedMesh(geometry, material, count); + const update = () => { + bodies.setMatrixAt(0, matrix); + caps.setMatrixAt(0, matrix); + for (const mesh of [bodies, caps]) mesh.instanceMatrix.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("does not let an unrelated mesh-list alias cover a mutation", () => { + const code = ` + import { InstancedMesh } from "three"; + const changed = new InstancedMesh(geometry, material, count); + const other = new InstancedMesh(geometry, material, count); + const update = () => { + changed.setMatrixAt(0, matrix); + for (const mesh of [other]) mesh.instanceMatrix.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); + + it("allows initial instance data populated before a new mesh escapes", () => { + const code = ` + import { InstancedMesh } from "three"; + const createMesh = () => { + const mesh = new InstancedMesh(geometry, material, count); + matrices.forEach((matrix, index) => mesh.setMatrixAt(index, matrix)); + mesh.castShadow = true; + scene.add(mesh); + return mesh; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("requires an upload when a previously escaped mesh is mutated", () => { + const code = ` + import { InstancedMesh } from "three"; + const createMesh = () => { + const mesh = new InstancedMesh(geometry, material, count); + scene.add(mesh); + mesh.setMatrixAt(0, matrix); + return mesh; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); + + it("allows a caller to upload mutations made by a local helper", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + scene.add(mesh); + const setSlot = (index, matrix) => mesh.setMatrixAt(index, matrix); + const update = () => { + for (let index = 0; index < count; index++) setSlot(index, matrices[index]); + mesh.instanceMatrix.needsUpdate = true; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("requires every local-helper call site to upload the mutation", () => { + const code = ` + import { InstancedMesh } from "three"; + const mesh = new InstancedMesh(geometry, material, count); + scene.add(mesh); + const setSlot = (index, matrix) => mesh.setMatrixAt(index, matrix); + const covered = () => { setSlot(0, first); mesh.instanceMatrix.needsUpdate = true; }; + const uncovered = () => { setSlot(1, second); }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); + + it("keeps initialization safe after attaching a new mesh to a local Three group", () => { + const code = ` + import { Group, InstancedMesh } from "three"; + const createMesh = () => { + const group = new Group(); + const mesh = new InstancedMesh(geometry, material, count); + group.add(mesh); + mesh.setColorAt(0, color); + return group; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("keeps initialization safe through a nonescaping same-class helper", () => { + const code = ` + import { InstancedMesh } from "three"; + class SceneBuilder { + create() { + const mesh = new InstancedMesh(geometry, material, count); + this.setSlot(mesh, 0, matrix); + mesh.setMatrixAt(1, matrix); + return mesh; + } + setSlot(mesh, index, matrix) { + mesh.setMatrixAt(index, matrix); + } + } + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(0); + }); + + it("does not trust a same-class helper that lets the mesh escape", () => { + const code = ` + import { InstancedMesh } from "three"; + class SceneBuilder { + create() { + const mesh = new InstancedMesh(geometry, material, count); + this.publish(mesh); + mesh.setMatrixAt(0, matrix); + return mesh; + } + publish(mesh) { + registry.mesh = mesh; + } + } + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); + + it("does not treat an arbitrary collection add as pre-render initialization", () => { + const code = ` + import { InstancedMesh } from "three"; + const createMesh = (registry) => { + const mesh = new InstancedMesh(geometry, material, count); + registry.add(mesh); + mesh.setColorAt(0, color); + return mesh; + }; + `; + expect(runRule(threeRequireInstancedBufferUpdate, code).diagnostics).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.ts index 082f8611dd..c9f99298ad 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-require-instanced-buffer-update.ts @@ -4,9 +4,13 @@ import type { EsTreeNode } from "../../utils/es-tree-node.js"; import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { getRangeStart } from "../../utils/get-range-start.js"; import { getStaticPropertyName } from "../../utils/get-static-property-name.js"; +import { getStaticPropertyKeyName } from "../../utils/get-static-property-key-name.js"; +import { resolveStaticLocalCallFunction } from "../../utils/get-order-independent-local-function.js"; import { isNodeConditionallyExecuted } from "../../utils/is-node-conditionally-executed.js"; +import { isFunctionLike } from "../../utils/is-function-like.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; import { isImportedOrStableParameterCall } from "../../utils/is-imported-or-stable-parameter-call.js"; +import { isSynchronousIteratorCallback } from "../../utils/is-synchronous-iterator-callback.js"; import { resolveExpressionKey } from "../../utils/resolve-expression-key.js"; import type { RuleContext } from "../../utils/rule-context.js"; import { stripParenExpression } from "../../utils/strip-paren-expression.js"; @@ -16,15 +20,58 @@ interface DirectInstancedBufferMutation { readonly bufferPropertyName: "instanceColor" | "instanceMatrix" | "morphTexture"; readonly methodName: "setColorAt" | "setMatrixAt" | "setMorphAt"; readonly node: EsTreeNodeOfType<"CallExpression">; + readonly receiver: EsTreeNode; readonly receiverKey: string; } interface DirectInstancedBufferCompletion { readonly bufferPropertyName: "instanceColor" | "instanceMatrix" | "morphTexture"; + readonly coverageNode: EsTreeNode; readonly node: EsTreeNode; - readonly receiverKey: string; + readonly receiverKeys: ReadonlySet; +} + +interface InstancedMeshReceiverKeys { + readonly keys: ReadonlySet; + readonly staticIterationNode: EsTreeNode | null; } +const getInstancedMeshReceiverKeys = ( + expression: EsTreeNode, + context: RuleContext, +): InstancedMeshReceiverKeys => { + const receiverKey = resolveExpressionKey(expression, context); + const receiverKeys = new Set(); + if (receiverKey) receiverKeys.add(receiverKey); + const receiver = stripParenExpression(expression); + if (!isNodeOfType(receiver, "Identifier")) { + return { keys: receiverKeys, staticIterationNode: null }; + } + const symbol = context.scopes.symbolFor(receiver); + if (!symbol || !isNodeOfType(symbol.declarationNode, "VariableDeclarator")) { + return { keys: receiverKeys, staticIterationNode: null }; + } + const declaration = symbol.declarationNode.parent; + const forOfStatement = declaration?.parent; + if ( + !isNodeOfType(declaration, "VariableDeclaration") || + !isNodeOfType(forOfStatement, "ForOfStatement") || + forOfStatement.left !== declaration + ) { + return { keys: receiverKeys, staticIterationNode: null }; + } + const collection = stripParenExpression(forOfStatement.right); + if (!isNodeOfType(collection, "ArrayExpression") || collection.elements.length === 0) { + return { keys: receiverKeys, staticIterationNode: null }; + } + for (const element of collection.elements) { + if (!element || isNodeOfType(element, "SpreadElement")) continue; + const elementKey = resolveExpressionKey(element, context); + if (elementKey) receiverKeys.add(elementKey); + } + return { keys: receiverKeys, staticIterationNode: forOfStatement }; +}; + const getInstancedBufferMutation = ( node: EsTreeNodeOfType<"CallExpression">, context: RuleContext, @@ -47,10 +94,182 @@ const getInstancedBufferMutation = ( : "morphTexture", methodName, node, + receiver: callee.object, receiverKey, }; }; +const isDirectNonEscapingReceiverReference = (reference: EsTreeNode): boolean => { + let referenceRoot = reference; + while ( + isNodeOfType(referenceRoot.parent, "MemberExpression") && + referenceRoot.parent.object === referenceRoot + ) { + referenceRoot = referenceRoot.parent; + } + const parent = referenceRoot.parent; + if (isNodeOfType(parent, "CallExpression") && parent.callee === referenceRoot) return true; + if ( + isNodeOfType(parent, "CallExpression") && + parent.arguments.some((argument) => argument === referenceRoot) && + isNodeOfType(parent.callee, "MemberExpression") && + getStaticPropertyName(parent.callee) === "add" + ) { + return false; + } + if (isNodeOfType(parent, "AssignmentExpression") && parent.left === referenceRoot) return true; + if (isNodeOfType(parent, "UpdateExpression") && parent.argument === referenceRoot) return true; + return false; +}; + +const resolveSameClassMethod = ( + callExpression: EsTreeNodeOfType<"CallExpression">, + context: RuleContext, +): EsTreeNode | null => { + const callee = stripParenExpression(callExpression.callee); + if ( + !isNodeOfType(callee, "MemberExpression") || + !isNodeOfType(stripParenExpression(callee.object), "ThisExpression") + ) { + return null; + } + const methodName = getStaticPropertyName(callee); + const caller = context.cfg.enclosingFunction(callExpression); + const callerDefinition = caller?.parent; + const classBody = callerDefinition?.parent; + if ( + !methodName || + !isNodeOfType(callerDefinition, "MethodDefinition") || + callerDefinition.value !== caller || + !isNodeOfType(classBody, "ClassBody") + ) { + return null; + } + for (const member of classBody.body) { + if ( + isNodeOfType(member, "MethodDefinition") && + !member.static && + getStaticPropertyKeyName(member, { allowComputedString: true }) === methodName && + isFunctionLike(member.value) + ) { + return member.value; + } + } + return null; +}; + +const doesSameClassMethodKeepArgumentLocal = ( + callExpression: EsTreeNodeOfType<"CallExpression">, + argument: EsTreeNode, + context: RuleContext, +): boolean => { + const argumentIndex = callExpression.arguments.findIndex((candidate) => candidate === argument); + if (argumentIndex < 0) return false; + const method = resolveSameClassMethod(callExpression, context); + const parameter = method && isFunctionLike(method) ? method.params[argumentIndex] : null; + if (!parameter || !isNodeOfType(parameter, "Identifier")) return false; + const parameterSymbol = context.scopes.symbolFor(parameter); + return Boolean( + parameterSymbol && + parameterSymbol.references.every((reference) => + isDirectNonEscapingReceiverReference(reference.identifier), + ), + ); +}; + +const isNonEscapingReceiverReference = (reference: EsTreeNode, context: RuleContext): boolean => { + if (isDirectNonEscapingReceiverReference(reference)) return true; + let referenceRoot = reference; + while ( + isNodeOfType(referenceRoot.parent, "MemberExpression") && + referenceRoot.parent.object === referenceRoot + ) { + referenceRoot = referenceRoot.parent; + } + const parent = referenceRoot.parent; + if ( + isNodeOfType(parent, "CallExpression") && + parent.arguments.some((argument) => argument === referenceRoot) && + isNodeOfType(parent.callee, "MemberExpression") && + getStaticPropertyName(parent.callee) === "add" + ) { + const ownerName = getThreeConstructorName(parent.callee.object, context.scopes); + if (ownerName === "Group" || ownerName === "Object3D" || ownerName === "Scene") return true; + } + return Boolean( + isNodeOfType(parent, "CallExpression") && + parent.arguments.some((argument) => argument === referenceRoot) && + doesSameClassMethodKeepArgumentLocal(parent, referenceRoot, context), + ); +}; + +const isPreEscapeInitializationMutation = ( + mutation: DirectInstancedBufferMutation, + context: RuleContext, +): boolean => { + const receiver = stripParenExpression(mutation.receiver); + if (!isNodeOfType(receiver, "Identifier")) return false; + const symbol = context.scopes.symbolFor(receiver); + let mutationOwner = context.cfg.enclosingFunction(mutation.node); + while ( + isFunctionLike(mutationOwner) && + !mutationOwner.async && + !mutationOwner.generator && + isSynchronousIteratorCallback(mutationOwner) && + isNodeOfType(mutationOwner.parent, "CallExpression") + ) { + mutationOwner = context.cfg.enclosingFunction(mutationOwner.parent); + } + if ( + !symbol || + symbol.kind !== "const" || + !isNodeOfType(symbol.initializer, "NewExpression") || + getThreeConstructorName(symbol.initializer, context.scopes) !== "InstancedMesh" || + context.cfg.enclosingFunction(symbol.declarationNode) !== mutationOwner + ) { + return false; + } + const mutationStart = getRangeStart(mutation.node); + if (mutationStart === null) return false; + return symbol.references.every((reference) => { + const referenceStart = getRangeStart(reference.identifier); + return ( + referenceStart === null || + referenceStart >= mutationStart || + isNonEscapingReceiverReference(reference.identifier, context) + ); + }); +}; + +const getDirectLocalFunctionCallSites = ( + localFunction: EsTreeNode, + context: RuleContext, +): ReadonlyArray> | null => { + let bindingIdentifier: EsTreeNode | null = null; + if (isNodeOfType(localFunction, "FunctionDeclaration")) { + bindingIdentifier = localFunction.id; + } else if ( + isNodeOfType(localFunction.parent, "VariableDeclarator") && + localFunction.parent.init === localFunction && + isNodeOfType(localFunction.parent.id, "Identifier") + ) { + bindingIdentifier = localFunction.parent.id; + } + if (!bindingIdentifier) return null; + const symbol = context.scopes.symbolFor(bindingIdentifier); + if (!symbol || symbol.references.length === 0) return null; + const callSites: EsTreeNodeOfType<"CallExpression">[] = []; + for (const reference of symbol.references) { + const parent = reference.identifier.parent; + if (!isNodeOfType(parent, "CallExpression") || parent.callee !== reference.identifier) { + return null; + } + if (resolveStaticLocalCallFunction(parent, context.scopes) !== localFunction) return null; + callSites.push(parent); + } + return callSites; +}; + const getInstancedBufferCompletion = ( node: EsTreeNodeOfType<"AssignmentExpression">, context: RuleContext, @@ -76,9 +295,21 @@ const getInstancedBufferCompletion = ( ) { return null; } - if (getThreeConstructorName(bufferMember.object, context.scopes) !== "InstancedMesh") return null; - const receiverKey = resolveExpressionKey(bufferMember.object, context); - return receiverKey ? { bufferPropertyName, node, receiverKey } : null; + const receiverProof = getInstancedMeshReceiverKeys(bufferMember.object, context); + if ( + getThreeConstructorName(bufferMember.object, context.scopes) !== "InstancedMesh" && + receiverProof.keys.size < 2 + ) { + return null; + } + return receiverProof.keys.size > 0 + ? { + bufferPropertyName, + coverageNode: receiverProof.staticIterationNode ?? node, + node, + receiverKeys: receiverProof.keys, + } + : null; }; const getOpaqueInstancedBufferCompletions = ( @@ -98,51 +329,199 @@ const getOpaqueInstancedBufferCompletions = ( bufferPropertyName === "morphTexture") && getThreeConstructorName(candidate.object, context.scopes) === "InstancedMesh" ) { - const receiverKey = resolveExpressionKey(candidate.object, context); - if (receiverKey) completions.push({ bufferPropertyName, node, receiverKey }); + const receiverProof = getInstancedMeshReceiverKeys(candidate.object, context); + if (receiverProof.keys.size > 0) { + completions.push({ + bufferPropertyName, + coverageNode: receiverProof.staticIterationNode ?? node, + node, + receiverKeys: receiverProof.keys, + }); + } continue; } } if (getThreeConstructorName(candidate, context.scopes) !== "InstancedMesh") continue; - const receiverKey = resolveExpressionKey(candidate, context); - if (!receiverKey) continue; - completions.push({ bufferPropertyName: "instanceMatrix", node, receiverKey }); - completions.push({ bufferPropertyName: "instanceColor", node, receiverKey }); - completions.push({ bufferPropertyName: "morphTexture", node, receiverKey }); + const receiverProof = getInstancedMeshReceiverKeys(candidate, context); + if (receiverProof.keys.size === 0) continue; + const coverageNode = receiverProof.staticIterationNode ?? node; + completions.push({ + bufferPropertyName: "instanceMatrix", + coverageNode, + node, + receiverKeys: receiverProof.keys, + }); + completions.push({ + bufferPropertyName: "instanceColor", + coverageNode, + node, + receiverKeys: receiverProof.keys, + }); + completions.push({ + bufferPropertyName: "morphTexture", + coverageNode, + node, + receiverKeys: receiverProof.keys, + }); } return completions; }; -const completionCoversMutation = ( +const isTrueLiteral = (node: EsTreeNode): boolean => { + const expression = stripParenExpression(node); + return isNodeOfType(expression, "Literal") && expression.value === true; +}; + +const isMutationFlagGuard = ( + candidate: EsTreeNode, mutation: DirectInstancedBufferMutation, - completions: ReadonlyArray, - program: EsTreeNode, + completion: DirectInstancedBufferCompletion, context: RuleContext, ): boolean => { - const owner = context.cfg.enclosingFunction(mutation.node); - const matchingCompletions = completions.filter( - (completion) => - completion.receiverKey === mutation.receiverKey && - completion.bufferPropertyName === mutation.bufferPropertyName && - context.cfg.enclosingFunction(completion.node) === owner, - ); - if (owner) { - return doNodesCoverEveryPathAfterNode( - mutation.node, - matchingCompletions.map((completion) => completion.node), - context, - ); + const identifier = stripParenExpression(candidate); + if (!isNodeOfType(identifier, "Identifier")) return false; + const symbol = context.scopes.symbolFor(identifier); + const initializer = symbol?.initializer && stripParenExpression(symbol.initializer); + if ( + !symbol || + !initializer || + !isNodeOfType(initializer, "Literal") || + initializer.value !== false + ) { + return false; } - const mutationStart = getRangeStart(mutation.node); - return matchingCompletions.some((completion) => { - const completionStart = getRangeStart(completion.node); + const trueAssignments: EsTreeNode[] = []; + for (const reference of symbol.references) { + const assignment = reference.identifier.parent; + if ( + !isNodeOfType(assignment, "AssignmentExpression") || + assignment.left !== reference.identifier + ) { + continue; + } + if (assignment.operator !== "=" || !isTrueLiteral(assignment.right)) return false; + trueAssignments.push(assignment); + } + const completionStart = getRangeStart(completion.coverageNode); + const precedingTrueAssignments = trueAssignments.filter((assignment) => { + const assignmentStart = getRangeStart(assignment); return ( - mutationStart !== null && - completionStart !== null && - completionStart > mutationStart && - !isNodeConditionallyExecuted(completion.node, program) + completionStart !== null && assignmentStart !== null && assignmentStart < completionStart ); }); + return ( + precedingTrueAssignments.length > 0 && + doNodesCoverEveryPathAfterNode(mutation.node, precedingTrueAssignments, context) + ); +}; + +const isGuardGuaranteedAfterMutation = ( + candidate: EsTreeNode, + mutation: DirectInstancedBufferMutation, + completion: DirectInstancedBufferCompletion, + context: RuleContext, +): boolean => { + const guard = stripParenExpression(candidate); + if (isNodeOfType(guard, "LogicalExpression") && guard.operator === "&&") { + return ( + isGuardGuaranteedAfterMutation(guard.left, mutation, completion, context) && + isGuardGuaranteedAfterMutation(guard.right, mutation, completion, context) + ); + } + if ( + isNodeOfType(guard, "MemberExpression") && + getStaticPropertyName(guard) === completion.bufferPropertyName && + completion.receiverKeys.has(resolveExpressionKey(guard.object, context) ?? "") + ) { + return true; + } + return isMutationFlagGuard(guard, mutation, completion, context); +}; + +const completionCoversMutation = ( + mutation: DirectInstancedBufferMutation, + completions: ReadonlyArray, + program: EsTreeNode, + context: RuleContext, +): boolean => { + const doesCompletionCoverAnchor = ( + initialPathAnchor: EsTreeNode, + visitedFunctions: Set, + ): boolean => { + let pathAnchor = initialPathAnchor; + while (true) { + const owner = context.cfg.enclosingFunction(pathAnchor); + const matchingCompletions = completions.filter( + (completion) => + completion.receiverKeys.has(mutation.receiverKey) && + completion.bufferPropertyName === mutation.bufferPropertyName && + context.cfg.enclosingFunction(completion.coverageNode) === owner, + ); + if (owner) { + const anchorStart = getRangeStart(pathAnchor); + const hasMatchingBufferGuard = matchingCompletions.some((completion) => { + const completionStart = getRangeStart(completion.coverageNode); + if (anchorStart === null || completionStart === null || completionStart <= anchorStart) { + return false; + } + let currentChild = completion.node; + let currentAncestor = completion.node.parent; + while (currentAncestor && currentAncestor !== owner) { + if ( + isNodeOfType(currentAncestor, "IfStatement") && + currentAncestor.consequent === currentChild && + !isNodeConditionallyExecuted(completion.node, currentAncestor.consequent) + ) { + if ( + isGuardGuaranteedAfterMutation(currentAncestor.test, mutation, completion, context) + ) { + return true; + } + } + currentChild = currentAncestor; + currentAncestor = currentAncestor.parent; + } + return false; + }); + if (hasMatchingBufferGuard) return true; + if ( + doNodesCoverEveryPathAfterNode( + pathAnchor, + matchingCompletions.map((completion) => completion.coverageNode), + context, + ) + ) { + return true; + } + if (!isFunctionLike(owner) || owner.async || owner.generator) return false; + if (isSynchronousIteratorCallback(owner)) { + const iteratorCall = owner.parent; + if (!isNodeOfType(iteratorCall, "CallExpression")) return false; + pathAnchor = iteratorCall; + continue; + } + const callSites = getDirectLocalFunctionCallSites(owner, context); + if (!callSites || visitedFunctions.has(owner)) return false; + visitedFunctions.add(owner); + const areAllCallSitesCovered = callSites.every((callSite) => + doesCompletionCoverAnchor(callSite, visitedFunctions), + ); + visitedFunctions.delete(owner); + return areAllCallSitesCovered; + } + const anchorStart = getRangeStart(pathAnchor); + return matchingCompletions.some((completion) => { + const completionStart = getRangeStart(completion.coverageNode); + return ( + anchorStart !== null && + completionStart !== null && + completionStart > anchorStart && + !isNodeConditionallyExecuted(completion.coverageNode, program) + ); + }); + } + }; + return doesCompletionCoverAnchor(mutation.node, new Set()); }; export const threeRequireInstancedBufferUpdate = defineRule({ @@ -175,6 +554,7 @@ export const threeRequireInstancedBufferUpdate = defineRule({ "Program:exit"() { if (!program) return; for (const mutation of mutations) { + if (isPreEscapeInitializationMutation(mutation, context)) continue; if (completionCoversMutation(mutation, completions, program, context)) continue; context.report({ node: mutation.node, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-shader-require-position-on-all-paths.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-shader-require-position-on-all-paths.test.ts index 40ce2c914e..13a0e7bd4b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-shader-require-position-on-all-paths.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-shader-require-position-on-all-paths.test.ts @@ -10,6 +10,7 @@ describe("three-shader-require-position-on-all-paths", () => { `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "void main() { for (int index = 0; index < 1; index++) gl_Position = vec4(0.0); }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "/* #define SET_POSITION() gl_Position = vec4(0.0) */ void main() { float value = 1.0; }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "void main() { gl_Position.x = 0.0; }" });`, + `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "#include \\nvoid main() { float value = 1.0; }" });`, ])("reports a proven path without a position write", (code) => { expect(runRule(threeShaderRequirePositionOnAllPaths, code).diagnostics).toHaveLength(1); }); @@ -31,6 +32,7 @@ describe("three-shader-require-position-on-all-paths", () => { `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "void main() { sin((gl_Position = vec4(0.0)).x); }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "void setPosition() { gl_Position = vec4(0.0); } void main() { setPosition(); }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "#define SET_POSITION() gl_Position = vec4(0.0)\\nvoid main() { SET_POSITION(); }" });`, + `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: "void main() { #include \\n#include }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ fragmentShader: "void main() { gl_FragColor = vec4(1.0); }" });`, `import { ShaderMaterial } from "three"; new ShaderMaterial({ vertexShader: shader });`, `class ShaderMaterial {}; new ShaderMaterial({ vertexShader: "void main() {}" });`, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.test.ts index 6ff84ddd2b..75ad7fd897 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.test.ts @@ -3,7 +3,7 @@ import { runRule } from "../../../test-utils/run-rule.js"; import { threeValidOrthographicCamera } from "./three-valid-orthographic-camera.js"; describe("three-valid-orthographic-camera", () => { - it("reports degenerate frusta and invalid clipping planes", () => { + it("reports degenerate frusta and reversed clipping planes", () => { const code = ` import { OrthographicCamera as Camera } from "three"; import * as THREE from "three"; @@ -14,7 +14,7 @@ describe("three-valid-orthographic-camera", () => { const camera = new Camera(); camera.near = -0.1; `; - expect(runRule(threeValidOrthographicCamera, code).diagnostics).toHaveLength(5); + expect(runRule(threeValidOrthographicCamera, code).diagnostics).toHaveLength(3); }); it("allows valid, dynamic, and unrelated cameras", () => { @@ -22,6 +22,7 @@ describe("three-valid-orthographic-camera", () => { import { OrthographicCamera } from "three"; import { OrthographicCamera as OtherCamera } from "camera-kit"; new OrthographicCamera(-1, 1, 1, -1, 0, 100); + new OrthographicCamera(-1, 1, 1, -1, -1, 1); new OrthographicCamera(left, right, top, bottom, near, far); new OtherCamera(1, 1, 1, 1, -1, -1); `; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.ts index 7dfdb6ce29..fac9f6f1fe 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-orthographic-camera.ts @@ -14,7 +14,6 @@ import { import { getApiReferenceProvenance } from "./utils/get-api-reference-provenance.js"; import { getInvalidOrthographicCameraParameter } from "./utils/get-invalid-orthographic-camera-parameter.js"; import { getStaticNumber } from "./utils/get-static-number.js"; -import { getThreePropertyAssignment } from "./utils/get-three-property-assignment.js"; import { isThreeModuleSource } from "./utils/is-three-module-source.js"; interface StaticCameraParameter { @@ -37,7 +36,7 @@ export const threeValidOrthographicCamera = defineRule({ category: "Correctness", severity: "error", recommendation: - "Use distinct horizontal and vertical frustum planes, a nonnegative near plane, and a far plane greater than near", + "Use distinct horizontal and vertical frustum planes and a far plane greater than near", create: (context: RuleContext) => ({ NewExpression(node: EsTreeNodeOfType<"NewExpression">) { const provenance = getApiReferenceProvenance(node.callee, context.scopes); @@ -76,26 +75,5 @@ export const threeValidOrthographicCamera = defineRule({ if (invalidParameter) context.report({ node: invalidParameter.node, message: invalidParameter.message }); }, - AssignmentExpression(node: EsTreeNodeOfType<"AssignmentExpression">) { - const assignment = getThreePropertyAssignment(node, context); - if ( - assignment?.constructorName !== "OrthographicCamera" || - assignment.propertyName !== "near" - ) { - return; - } - const value = getStaticNumber(assignment.value, context.scopes); - if (value === null) return; - const invalidParameter = getInvalidOrthographicCameraParameter({ - bottom: null, - far: null, - left: null, - near: { node: assignment.value, value }, - right: null, - top: null, - }); - if (invalidParameter) - context.report({ node: invalidParameter.node, message: invalidParameter.message }); - }, }), }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.test.ts index b15dbca142..b224186cc2 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.test.ts @@ -34,4 +34,18 @@ describe("three-valid-texture-color-space", () => { `; expect(runRule(threeValidTextureColorSpace, code).diagnostics).toHaveLength(0); }); + + it("allows a linear render-target texture used as a generated material map", () => { + const code = ` + import * as THREE from "three"; + class ContactShadow { + constructor() { + this.target = new THREE.WebGLRenderTarget(512, 512); + this.target.texture.colorSpace = THREE.NoColorSpace; + this.material = new THREE.MeshBasicMaterial({ map: this.target.texture }); + } + } + `; + expect(runRule(threeValidTextureColorSpace, code).diagnostics).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.ts index 936ad2606d..79810d565a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/three-valid-texture-color-space.ts @@ -2,8 +2,11 @@ import { defineRule } from "../../utils/define-rule.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { getStaticObjectPropertyValue } from "../../utils/get-static-object-property-value.js"; +import { getStaticPropertyName } from "../../utils/get-static-property-name.js"; +import { isNodeOfType } from "../../utils/is-node-of-type.js"; import { resolveExpressionKey } from "../../utils/resolve-expression-key.js"; import type { RuleContext } from "../../utils/rule-context.js"; +import { stripParenExpression } from "../../utils/strip-paren-expression.js"; import { getExpectedTextureColorSpace } from "./utils/get-expected-texture-color-space.js"; import { getExplicitTextureColorSpaceAssignment } from "./utils/get-explicit-texture-color-space-assignment.js"; import { getThreeConstructorName } from "./utils/get-three-constructor-name.js"; @@ -44,8 +47,15 @@ export const threeValidTextureColorSpace = defineRule({ create: (context: RuleContext) => { const materialNodes: EsTreeNodeOfType<"NewExpression">[] = []; const assignmentsByTextureKey = new Map(); + const renderTargetKeys = new Set(); + const recordRenderTarget = (target: EsTreeNode, value: EsTreeNode): void => { + const constructorName = getThreeConstructorName(value, context.scopes); + const targetKey = resolveExpressionKey(target, context); + if (targetKey && constructorName?.endsWith("RenderTarget")) renderTargetKeys.add(targetKey); + }; return { AssignmentExpression(node: EsTreeNodeOfType<"AssignmentExpression">) { + recordRenderTarget(node.left, node.right); const assignment = getExplicitTextureColorSpaceAssignment(node, context); if (!assignment) return; const previous = assignmentsByTextureKey.get(assignment.textureKey); @@ -59,6 +69,9 @@ export const threeValidTextureColorSpace = defineRule({ materialNodes.push(node); } }, + VariableDeclarator(node: EsTreeNodeOfType<"VariableDeclarator">) { + if (node.init) recordRenderTarget(node.id, node.init); + }, "Program:exit"() { for (const materialNode of materialNodes) { const parameters = materialNode.arguments[0]; @@ -66,6 +79,14 @@ export const threeValidTextureColorSpace = defineRule({ for (const propertyName of MATERIAL_TEXTURE_PROPERTY_NAMES) { const textureExpression = getStaticObjectPropertyValue(parameters, propertyName); if (!textureExpression) continue; + const textureMember = stripParenExpression(textureExpression); + if ( + isNodeOfType(textureMember, "MemberExpression") && + getStaticPropertyName(textureMember) === "texture" && + renderTargetKeys.has(resolveExpressionKey(textureMember.object, context) ?? "") + ) { + continue; + } const textureKey = resolveExpressionKey(textureExpression, context); const assignment = textureKey ? assignmentsByTextureKey.get(textureKey) : undefined; const expectedColorSpace = getExpectedTextureColorSpace(propertyName); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/does-glsl-main-write-position-on-all-paths.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/does-glsl-main-write-position-on-all-paths.ts index 09a5887b6c..fe9b290440 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/does-glsl-main-write-position-on-all-paths.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/does-glsl-main-write-position-on-all-paths.ts @@ -302,6 +302,18 @@ export const doesGlslMainWritePositionOnAllPaths = ( source: string, ): GlslPositionPathAnalysis => { const analysis = doesGlslMainWriteVectorOnAllPaths(program, source, "gl_Position"); + const mainStart = analysis.mainFunction?.location?.start.offset; + const mainEnd = analysis.mainFunction?.location?.end.offset; + const mainSource = + mainStart !== undefined && mainEnd !== undefined ? source.slice(mainStart, mainEnd) : ""; + if ( + /^[ \t]*#[ \t]*include[ \t]*<[ \t]*project_vertex[ \t]*>/m.test(maskGlslComments(mainSource)) + ) { + return { + mainFunction: analysis.mainFunction, + writesPositionOnAllPaths: true, + }; + } return { mainFunction: analysis.mainFunction, writesPositionOnAllPaths: analysis.writesVectorOnAllPaths, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/find-repeated-position-buffer-mutations.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/find-repeated-position-buffer-mutations.ts index fa84ec38cb..fb84b96ce6 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/find-repeated-position-buffer-mutations.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/find-repeated-position-buffer-mutations.ts @@ -170,9 +170,11 @@ export const findRepeatedPositionBufferMutations = ( callback: EsTreeNode, context: RuleContext, managedPositionBufferRefSymbolIds: ReadonlySet = new Set(), + includeConditionallyExecuted = true, ): ReadonlyArray => { const mutations = new Set(); - walkFunctionExecution(callback, context.scopes, (candidate) => { + walkFunctionExecution(callback, context.scopes, (candidate, isConditionallyExecuted) => { + if (!includeConditionallyExecuted && isConditionallyExecuted) return; if ( isRepeatedPositionBufferMutation( candidate, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/get-invalid-orthographic-camera-parameter.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/get-invalid-orthographic-camera-parameter.ts index 868be82f09..d4a9bb530d 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/get-invalid-orthographic-camera-parameter.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/r3f/utils/get-invalid-orthographic-camera-parameter.ts @@ -1,5 +1,4 @@ import type { EsTreeNode } from "../../../utils/es-tree-node.js"; -import { MINIMUM_ORTHOGRAPHIC_CAMERA_NEAR } from "../constants.js"; interface StaticOrthographicCameraParameter { readonly node: EsTreeNode; @@ -35,12 +34,6 @@ export const getInvalidOrthographicCameraParameter = ( message: "OrthographicCamera top and bottom planes must differ", }; } - if (parameters.near && parameters.near.value < MINIMUM_ORTHOGRAPHIC_CAMERA_NEAR) { - return { - node: parameters.near.node, - message: "OrthographicCamera near must be zero or greater", - }; - } if (parameters.near && parameters.far && parameters.far.value <= parameters.near.value) { return { node: parameters.far.node, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.regressions.test.ts index 20b67892ca..23dec91d6e 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.regressions.test.ts @@ -673,7 +673,7 @@ return
; it("still flags a non-HTML encoder that does not escape markup", () => { const findings = runScanRule(dangerousHtmlSink, { relativePath: "src/widgets/display.ts", - content: `el.innerHTML = encodeForDisplay(data.body);\n`, + content: `el.innerHTML = encodeForDisplay(response.data.body);\n`, }); expect(findings).toHaveLength(1); }); @@ -1000,6 +1000,35 @@ return
; expect(findings).toHaveLength(0); }); + it("stays silent when a local HTML helper only receives values without taint provenance", () => { + const content = [ + "const renderPreview = (element, html) => {", + " element.innerHTML = html;", + "};", + "renderPreview(titleElement, game.title);", + "renderPreview(scoreElement, formatScore(game.score));", + ].join("\n"); + const findings = runScanRule(dangerousHtmlSink, { + relativePath: "src/game/hud.ts", + content, + }); + expect(findings).toHaveLength(0); + }); + + it("stays silent on internally generated game markup and result data", () => { + const findings = runScanRule(dangerousHtmlSink, { + relativePath: "src/ui/hud.ts", + content: ` +interface GameResult { title: string; score: number } +const renderHud = (element: HTMLElement, result: GameResult) => { + const html = \`

\${result.title}

\${result.score}\`; + element.innerHTML = html; +}; +`, + }); + expect(findings).toHaveLength(0); + }); + it("stays silent when trusted helper arguments pass through local aliases", () => { const content = [ "const renderPreview = (element, html) => {", @@ -1049,6 +1078,18 @@ return
; expect(findings).toHaveLength(1); }); + it("flags an exported async helper parameter without local call sites", () => { + const findings = runScanRule(dangerousHtmlSink, { + relativePath: "src/components/preview.ts", + content: `export async function renderPreview(element, html) { + await preparePreview(element); + element.innerHTML = html; +} +`, + }); + expect(findings).toHaveLength(1); + }); + it("ignores recursive calls when every external helper argument is trusted", () => { const content = [ "function renderPreview(element, html, remaining) {", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.ts index 6be8676ca0..f84c2d411b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/dangerous-html-sink.ts @@ -25,7 +25,7 @@ const HTML_VALUE_START_PATTERN = // `document.cookie`/`.referrer`, `window.name`, web/session storage, // `URLSearchParams`) — attacker-controllable channels that must be flagged. const HTML_TAINT_PATTERN = - /searchParams|query|params|request|req\.|response\.|result\.|data\.|await|fetch|props\.|children|content|html|body|text|message|markup|\blocation\b|document\.cookie|\breferrer\b|\blocalStorage\b|\bsessionStorage\b|URLSearchParams|window\.name/i; + /searchParams|query|params|request|req\.|response\.|await|fetch|props\.|children|(?:user|untrusted|unsafe|raw|comment|message|cms|remote|external)\.|(?:user|untrusted|unsafe|raw|comment|message|cms|remote|external|content|profile|signature|subtitle)\w*(?:html|markup)|(?:html|markup)\w*(?:input|payload)|\b(?:load|read|receive|decode)\w*\s*\(|\bget\w*(?:Html|Markup|Content|Page|Message)\b|\blocation\b|document\.cookie|\breferrer\b|\blocalStorage\b|\bsessionStorage\b|URLSearchParams|window\.name/i; // A trailing line comment (`innerHTML = "" // clear`) must not defeat the // literal/constant exemptions: without tolerating it the value never matches, @@ -307,9 +307,11 @@ const splitTopLevelByPlus = (text: string): string[] => { const isAllOperandsDomContentConcat = (valueExpression: string): boolean => { const body = valueExpression.replace(/[;}]\s*$/, "").trim(); if (!body.includes("+")) return false; - const operands = splitTopLevelByPlus(body) - .map((operand) => operand.trim()) - .filter((operand) => operand.length > 0); + const operands: string[] = []; + for (const operand of splitTopLevelByPlus(body)) { + const trimmedOperand = operand.trim(); + if (trimmedOperand.length > 0) operands.push(trimmedOperand); + } if (operands.length < 2) return false; return operands.every((operand) => { const withoutCast = operand.replace(/\(\s*([\w$]+(?:\??\.[\w$]+)*)\s+as\s+[^)]*\)/g, "$1"); @@ -542,6 +544,18 @@ interface FunctionParameterSource { readonly parameterIndex: number; } +const isDestructuredInputParameter = ( + identifier: string, + sinkIndex: number, + fileContent: string, +): boolean => { + const prefix = fileContent.slice(Math.max(0, sinkIndex - STATIC_TEMPLATE_MAX_CHARS), sinkIndex); + const escapedIdentifier = escapeRegExp(identifier); + return new RegExp( + `(?:function\\s+[\\w$]+|(?:const|let|var)\\s+[\\w$]+\\s*=)\\s*\\(\\s*\\{[^}]*\\b${escapedIdentifier}\\b[^}]*\\}`, + ).test(prefix); +}; + const findContainingFunctionParameterSource = ( identifier: string, sinkIndex: number, @@ -641,8 +655,11 @@ const isHtmlTainted = ( ): boolean => { const trimmedExpression = expression.trim(); if (isExplicitlyTrustedHtmlValue(trimmedExpression, fileContent, sinkIndex)) return false; - const identifier = trimmedExpression.match(/^([\w$]+)(?:\.|\s*(?:[;,})\n]|$))/)?.[1]; + const identifier = trimmedExpression.match( + /^([\w$]+)(?:\.|\s*(?:(?:\|\||\?\?)|[;,})\n]|$))/, + )?.[1]; if (identifier === undefined) return HTML_TAINT_PATTERN.test(trimmedExpression); + if (isDestructuredInputParameter(identifier, sinkIndex, fileContent)) return true; if (visitedIdentifiers.has(identifier)) return false; visitedIdentifiers.add(identifier); @@ -686,7 +703,6 @@ const isHtmlTainted = ( if (parameterSource !== null && parameterSource.functionName.length > 0) { const callPattern = new RegExp(`\\b${escapeRegExp(parameterSource.functionName)}\\s*\\(`, "g"); let didInspectCallArgument = false; - let didInspectOnlyExplicitlyTrustedArguments = true; for (const callMatch of fileContent.matchAll(callPattern)) { if ( callMatch.index === parameterSource.declarationNameIndex || @@ -711,11 +727,6 @@ const isHtmlTainted = ( continue; } didInspectCallArgument = true; - didInspectOnlyExplicitlyTrustedArguments &&= isExplicitlyTrustedHtmlValue( - argument, - fileContent, - callMatch.index, - ); const callVisitedIdentifiers = new Set(visitedIdentifiers); callVisitedIdentifiers.delete(identifier); const nextVisitedCallSites = new Set(visitedCallSites); @@ -732,10 +743,17 @@ const isHtmlTainted = ( return true; } } - if (didInspectCallArgument) { - return ( - !didInspectOnlyExplicitlyTrustedArguments && HTML_TAINT_PATTERN.test(trimmedExpression) - ); + if (didInspectCallArgument) return false; + const declarationPrefix = fileContent.slice( + Math.max(0, parameterSource.declarationNameIndex - 48), + parameterSource.declarationNameIndex, + ); + if ( + /\bexport\s+(?:(?:default\s+)?(?:async\s+)?function|const|let|var)\s+$/.test( + declarationPrefix, + ) + ) { + return true; } return HTML_TAINT_PATTERN.test(trimmedExpression); } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.regressions.test.ts index c9969629ab..90c8781ca1 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.regressions.test.ts @@ -18,4 +18,20 @@ describe("security-scan/local-rpc-native-bridge-risk — regressions", () => { }); expect(findings).toHaveLength(1); }); + + it("stays silent on a loopback browser-test server process", () => { + const findings = runScanRule(localRpcNativeBridgeRisk, { + relativePath: "tools/shot.mjs", + content: `const port = await freePort();\nconst origin = \`http://127.0.0.1:\${port}\`;\nconst server = spawn("node", ["tools/serve.mjs", String(port), "dist"]);\nawait waitForServer(origin);\nconst browser = await chromium.launch();\n`, + }); + expect(findings).toHaveLength(0); + }); + + it("flags a localhost request handler that executes commands", () => { + const findings = runScanRule(localRpcNativeBridgeRisk, { + relativePath: "src/bridge.ts", + content: `server.listen(9001, "localhost");\nserver.on("request", (req) => {\n spawn(req.url.slice(1));\n});\n`, + }); + expect(findings).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.ts index 635e0c8d46..9d31915662 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/local-rpc-native-bridge-risk.ts @@ -13,7 +13,7 @@ export const localRpcNativeBridgeRisk = defineRule({ scan: scanByPattern({ shouldScan: (file) => isProductionSourcePath(file.relativePath), pattern: - /\b(?:127\.0\.0\.1|localhost|Access-Control-Allow-Origin|websocket|WebSocket)\b[\s\S]{0,700}(?:\b(?:UpdateApp|InstallApp|child_process)\b|(? { expect(findings).toHaveLength(0); }); + it("stays silent on a short receiver with an explicit Worker parameter type", () => { + const findings = runScanRule(postmessageOriginRisk, { + relativePath: "src/textures.ts", + content: + "const connect = (w: Worker) => {\n w.onmessage = (event) => applyTexture(event.data);\n};\n", + }); + expect(findings).toHaveLength(0); + }); + it("stays silent on the _self worker-global polyfill handler (prism shape)", () => { const findings = runScanRule(postmessageOriginRisk, { relativePath: "src/lib/prism-runtime.js", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/postmessage-origin-risk.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/postmessage-origin-risk.ts index bd6d70f82a..8e3e435aa4 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/postmessage-origin-risk.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/postmessage-origin-risk.ts @@ -50,6 +50,19 @@ const isSameApplicationChannelInstance = (targetText: string, fileContent: strin return constructorAssignmentPattern.test(fileContent); }; +const isSameApplicationChannelTypedReceiver = ( + targetText: string, + fileContent: string, +): boolean => { + const receiverRoot = /^[\w$]+/.exec(targetText)?.[0]; + if (receiverRoot === undefined) return false; + const typedReceiverPattern = new RegExp( + `(?:^|[(,{;]\\s*)${escapeRegExp(receiverRoot)}\\s*[?!]?\\s*:\\s*(?:Worker|SharedWorker|MessagePort|BroadcastChannel|WebSocket|EventSource)\\b`, + "m", + ); + return typedReceiverPattern.test(fileContent); +}; + const WORKER_FILE_PATH_PATTERN = /worker/i; const getNodeText = (content: string, node: EsTreeNode): string => { @@ -107,6 +120,7 @@ export const postmessageOriginRisk = defineRule({ if (targetText === null) return; if (SAME_APPLICATION_CHANNEL_TARGET_PATTERN.test(targetText)) return; if (isSameApplicationChannelInstance(targetText, file.content)) return; + if (isSameApplicationChannelTypedReceiver(targetText, file.content)) return; const nodeText = getNodeText(file.content, node); const messageDataIndex = nodeText.search(MESSAGE_DATA_READ_PATTERN); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.regressions.test.ts index c9f7f3e720..134c396ee9 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.regressions.test.ts @@ -56,6 +56,45 @@ describe("server-sequential-independent-await — regressions", () => { expect(result.diagnostics).toHaveLength(0); }); + it("keeps named loading stages sequential", () => { + const result = runRule( + serverSequentialIndependentAwait, + `async function buildScene() { + const terrain = await runStage(buildTerrain()); + const props = await runStage(buildProps()); + return { terrain, props }; + }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("keeps asset builds with observable progress callbacks sequential", () => { + const result = runRule( + serverSequentialIndependentAwait, + `async function buildScene() { + const terrain = await buildTerrain((label, fraction) => hud.setProgress(label, fraction)); + const props = await buildProps(async (label) => await advanceProgress(label)); + return { terrain, props }; + }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(0); + }); + + it("flags independent awaits with ordinary side-effect callbacks", () => { + const result = runRule( + serverSequentialIndependentAwait, + `async function loadData() { + const profile = await loadProfile((error) => console.error(error)); + const preferences = await loadPreferences((value) => cache.set("preferences", value)); + return { profile, preferences }; + }`, + ); + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(1); + }); + it("flags an independent visible helper even when its name starts with initialize", () => { const result = runRule( serverSequentialIndependentAwait, diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts index 62f249cf78..4376f55eff 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts @@ -1,3 +1,4 @@ +import { INTENTIONAL_SEQUENCING_CALLEE_NAMES } from "../../constants/js.js"; import { defineRule } from "../../utils/define-rule.js"; import { expressionReadsPatternBinding } from "../../utils/expression-reads-pattern-binding.js"; import { getCalleeName } from "../../utils/get-callee-name.js"; @@ -5,7 +6,9 @@ import { getOrderIndependentLocalFunction } from "../../utils/get-order-independ import { hasPossibleStaticMemberCallWrite } from "../../utils/has-static-property-write-before.js"; import { getImportedNameFromModule } from "../../utils/find-import-source-for-name.js"; import { isAuthGuardName } from "../../utils/is-auth-guard-name.js"; +import { isFunctionLike } from "../../utils/is-function-like.js"; import { tokenizeIdentifierWords } from "../../utils/tokenize-identifier-words.js"; +import { walkAst } from "../../utils/walk-ast.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; import type { RuleContext } from "../../utils/rule-context.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; @@ -145,6 +148,54 @@ const declarationAwaitsGate = (declaration: EsTreeNode, context: RuleContext): b return false; }; +const PROGRESS_CALLBACK_WORDS = new Set(["progress", "stage", "step"]); + +const callbackReportsProgress = (callback: EsTreeNode): boolean => { + let doesReportProgress = false; + walkAst(callback, (candidate) => { + if (doesReportProgress || (candidate !== callback && isFunctionLike(candidate))) return false; + if (!isNodeOfType(candidate, "CallExpression")) return; + const calleeName = getCalleeName(candidate); + if (!calleeName) return; + doesReportProgress = tokenizeIdentifierWords(calleeName).some((word) => + PROGRESS_CALLBACK_WORDS.has(word), + ); + }); + return doesReportProgress; +}; + +const declarationAwaitsIntentionalSequence = ( + declaration: EsTreeNode, + context: RuleContext, +): boolean => { + if (!isNodeOfType(declaration, "VariableDeclaration")) return false; + for (const declarator of declaration.declarations ?? []) { + if (!isNodeOfType(declarator.init, "AwaitExpression")) continue; + const argument = declarator.init.argument; + if (!isNodeOfType(argument, "CallExpression")) continue; + const localFunction = getOrderIndependentLocalFunction(argument, context.scopes); + const calleeName = getCalleeName(argument); + if ( + localFunction === null && + calleeName && + INTENTIONAL_SEQUENCING_CALLEE_NAMES.has(calleeName) + ) { + return true; + } + if ( + argument.arguments.some( + (callArgument) => + !isNodeOfType(callArgument, "SpreadElement") && + isFunctionLike(callArgument) && + callbackReportsProgress(callArgument), + ) + ) { + return true; + } + } + return false; +}; + export const serverSequentialIndependentAwait = defineRule({ id: "server-sequential-independent-await", title: "Sequential independent awaits", @@ -178,6 +229,11 @@ export const serverSequentialIndependentAwait = defineRule({ declarationAwaitsRequestScopedCall(nextStatement) ) continue; + if ( + declarationAwaitsIntentionalSequence(currentStatement, context) || + declarationAwaitsIntentionalSequence(nextStatement, context) + ) + continue; // A guard / side-effect gate (`await requireSession()`, `db.connect()`) // must run before the next await — its ordering is intentional, not a // parallelizable waterfall. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/create-loop-aware-visitors.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/create-loop-aware-visitors.ts index cd3d537a01..e7a98287f3 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/utils/create-loop-aware-visitors.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/create-loop-aware-visitors.ts @@ -31,7 +31,7 @@ const ITERATOR_CALLBACK_METHOD_NAMES: ReadonlySet = new Set([ "reduceRight", ]); -const FUNCTION_EXPRESSION_TYPES = ["ArrowFunctionExpression", "FunctionExpression"]; +const FUNCTION_TYPES = ["ArrowFunctionExpression", "FunctionExpression", "FunctionDeclaration"]; const isIteratorCallback = (node: EsTreeNode): boolean => { const parent = node.parent; @@ -49,6 +49,7 @@ export const createLoopAwareVisitors = ( options: LoopAwareVisitorOptions = {}, ): RuleVisitors => { let loopDepth = 0; + const functionLoopDepthStack: number[] = []; const incrementLoopDepth = (): void => { loopDepth++; }; @@ -63,15 +64,15 @@ export const createLoopAwareVisitors = ( visitors[`${loopType}:exit`] = decrementLoopDepth; } - if (options.treatIteratorCallbacksAsLoops) { - for (const functionType of FUNCTION_EXPRESSION_TYPES) { - visitors[functionType] = (node: EsTreeNode) => { - if (isIteratorCallback(node)) loopDepth++; - }; - visitors[`${functionType}:exit`] = (node: EsTreeNode) => { - if (isIteratorCallback(node)) loopDepth--; - }; - } + for (const functionType of FUNCTION_TYPES) { + visitors[functionType] = (node: EsTreeNode) => { + functionLoopDepthStack.push(loopDepth); + loopDepth = + options.treatIteratorCallbacksAsLoops && isIteratorCallback(node) ? loopDepth + 1 : 0; + }; + visitors[`${functionType}:exit`] = () => { + loopDepth = functionLoopDepthStack.pop() ?? 0; + }; } for (const [nodeType, handler] of Object.entries(innerVisitors)) { From 6e0a84c8855873c8172599a39d4ed5bae4e64d0f Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 12 Aug 2026 11:33:52 +0000 Subject: [PATCH 2/2] Trigger sanitized CI validation