From 6c30131ec2307bb057469b371bcf56ec8cdde2f1 Mon Sep 17 00:00:00 2001 From: Paul <72733450+paul1995tu@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:48:17 +0200 Subject: [PATCH 1/4] fix(typebuddy): repair the oxlint autofixes and close the rules' blind spots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes produced code that does not parse or that changes string values. `prefer-maybe-promise` inserted `return err()` directly before a catch block's closing brace; `require-try-catch` rebuilt the body line by line and stripped leading whitespace from every line, template literals included. Both now use zero-width insertions and leave the body byte for byte intact. The catch branch settles on `err()` and is no longer fed through the try block's code path, which used to produce two reports with opposite fixes for one return and append unreachable code. Returns that hand back a value are reported without a fix: rewriting them would drop the author's fallback. Nested returns, type-level `Promise` signatures, `MaybePromise`/`AsyncResult` annotations and expression-bodied async arrows were all invisible to the rules. The shared walk and the shared import insertion now live in `own_subtree.ts` and `typebuddy_import.ts` — both bugs existed twice because both helpers did. The fix smoke runs two fixtures now: one file with imports, one without, since the insertion behaves differently in each and only the second one caught the import bug. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CkmW7HzpL6zqpjuo1ETm2w --- .../typebuddy-oxlint-autofix-repairs.md | 43 ++ packages/typebuddy/rules/async_rule.ts | 258 +++++------ packages/typebuddy/rules/deliberate_throw.ts | 52 +-- .../typebuddy/rules/maybe_promise_rule.ts | 422 ++++++++++-------- packages/typebuddy/rules/optional_rule.ts | 6 +- packages/typebuddy/rules/own_subtree.ts | 70 +++ packages/typebuddy/rules/typebuddy_import.ts | 134 ++++++ .../smoke/oxlint/fix-expected-bare.ts | 15 + .../typebuddy/smoke/oxlint/fix-expected.ts | 144 ++++++ .../typebuddy/smoke/oxlint/fix-input-bare.ts | 11 + packages/typebuddy/smoke/oxlint/fix-input.ts | 118 +++++ .../typebuddy/smoke/oxlint/run-fix-smoke.ts | 99 ++-- 12 files changed, 952 insertions(+), 420 deletions(-) create mode 100644 .changeset/typebuddy-oxlint-autofix-repairs.md create mode 100644 packages/typebuddy/rules/own_subtree.ts create mode 100644 packages/typebuddy/rules/typebuddy_import.ts create mode 100644 packages/typebuddy/smoke/oxlint/fix-expected-bare.ts create mode 100644 packages/typebuddy/smoke/oxlint/fix-input-bare.ts diff --git a/.changeset/typebuddy-oxlint-autofix-repairs.md b/.changeset/typebuddy-oxlint-autofix-repairs.md new file mode 100644 index 0000000..e501b20 --- /dev/null +++ b/.changeset/typebuddy-oxlint-autofix-repairs.md @@ -0,0 +1,43 @@ +--- +"@murky-web/typebuddy": minor +--- + +Repair the oxlint autofixes and close the rules' blind spots. + +Two fixes were producing broken code. `prefer-maybe-promise` inserted +`return err()` directly before a catch block's closing brace, so +`catch { log("boom") }` became `catch { log("boom") return err(); }`, which does +not parse. `require-try-catch` rebuilt the function body line by line and +stripped leading whitespace from every line — including lines inside template +literals, silently changing string values. It now wraps the body with two +zero-width insertions and leaves it byte for byte intact. + +Six blind spots closed: + +- The catch branch settles on `err()`. It was fed to the same code path as the + try block, so a catch return got two reports with opposite fixes (`ok(value)` + and `err()`), and `--fix` appended an unreachable `return err()` after an + existing return. Returns that hand back a value are now reported without a + fix — rewriting them to `err()` would drop the author's fallback, and where + the default belongs is the author's decision. +- Returns nested inside an `if`, loop or `switch` in a try block are wrapped. + Only the try block's direct children were, so `try { if (flag) return "early"; + return "late"; }` wrapped just `"late"`. +- `Promise` in a type-level signature is reported. `TSDeclareFunction`, + `TSFunctionType` and `TSMethodSignature` were routed through a check for + `async`, which a type signature can never carry, so all three visitors were + unreachable. Ambient `declare` context stays exempt: rewriting a third-party + callback shape would misdescribe that API. +- `MaybePromise` and `AsyncResult` count as awaited return types. Matching only + `Promise` meant a bare `return;` in a `MaybePromise` function was left + alone while its `Promise` twin was fixed — the rule went blind on + already-migrated code, and on the spelling the package recommends. +- Expression-bodied async arrows are reported and fixed. `async () => fetch(url)` + has no block for the try/catch to live in and escaped entirely. +- `null | undefined` is left alone instead of being rewritten to the + equivalent-but-longer `Optional`. + +Also fixed: an inserted helper import landed glued to the following statement in +any file that starts with a comment. Whether to append after an existing import +was inferred from the anchor's byte offset, which only answers that question in +a file whose first statement begins at byte zero. diff --git a/packages/typebuddy/rules/async_rule.ts b/packages/typebuddy/rules/async_rule.ts index c074473..0ffedb4 100644 --- a/packages/typebuddy/rules/async_rule.ts +++ b/packages/typebuddy/rules/async_rule.ts @@ -1,4 +1,5 @@ import { throwsDeliberately } from "./deliberate_throw.js"; +import { getTypeBuddyImportInsertion } from "./typebuddy_import.js"; type AstNode = { type: string; @@ -29,99 +30,6 @@ type RuleFixer = { insertTextBeforeRange(range: [number, number], text: string): unknown; }; -function isNode(value: unknown): value is AstNode { - return typeof value === "object" && value !== null && "type" in value; -} - -function getProgram(node: AstNode): AstNode | null { - let current: AstNode | undefined = node; - - while (current?.parent) { - current = current.parent; - } - - return current?.type === "Program" ? current : null; -} - -function isIdentifierNamed(node: unknown, name: string): boolean { - return isNode(node) && node.type === "Identifier" && node["name"] === name; -} - -function isImportDeclaration(node: unknown): node is AstNode { - return isNode(node) && node.type === "ImportDeclaration"; -} - -function getStringLiteralValue(node: unknown): string | null { - if (!isNode(node)) { - return null; - } - - if ( - (node.type === "Literal" || node.type === "StringLiteral") && - typeof node["value"] === "string" - ) { - return node["value"]; - } - - return null; -} - -function getProgramBody(node: AstNode): AstNode[] { - if (!Array.isArray(node.body)) { - return []; - } - - return node.body.filter(isNode); -} - -function hasTypeBuddyHelperImport(program: AstNode, helperName: "err") { - return getProgramBody(program).some((statement) => { - if (!isImportDeclaration(statement)) { - return false; - } - - if ( - getStringLiteralValue(statement["source"]) !== "@murky-web/typebuddy" - ) { - return false; - } - - if (statement["importKind"] === "type") { - return false; - } - - const specifiers = Array.isArray(statement["specifiers"]) - ? statement["specifiers"] - : []; - - return specifiers.some((specifier) => { - return ( - isNode(specifier) && - specifier.type === "ImportSpecifier" && - isIdentifierNamed(specifier["local"], helperName) - ); - }); - }); -} - -function getTypeBuddyImportInsertRange( - program: AstNode, -): [number, number] | null { - const body = getProgramBody(program); - const imports = body.filter(isImportDeclaration); - const anchor = imports.at(-1) ?? body[0] ?? program; - - if (!Array.isArray(anchor.range) || anchor.range.length < 2) { - return null; - } - - if (imports.length > 0) { - return [anchor.range[1], anchor.range[1]]; - } - - return [anchor.range[0], anchor.range[0]]; -} - function hasTryCatch(nodes: AstNode[]): boolean { return nodes.some((node) => { return node.type === "TryStatement"; @@ -138,7 +46,9 @@ function isCallArgumentCallback(node: AstNode): boolean { return false; } - return Array.isArray(parent["arguments"]) && parent["arguments"].includes(node); + return ( + Array.isArray(parent["arguments"]) && parent["arguments"].includes(node) + ); } const rule = { @@ -154,81 +64,143 @@ const rule = { return []; } - const program = getProgram(node); - if (!program || hasTypeBuddyHelperImport(program, "err")) { - return []; - } - - const insertRange = getTypeBuddyImportInsertRange(program); - if (!insertRange) { + const insertion = getTypeBuddyImportInsertion(node, "err"); + if (!insertion) { return []; } scheduledErrImport = true; - const hasImports = insertRange[0] !== 0; - const importText = hasImports - ? '\nimport { err } from "@murky-web/typebuddy";' - : 'import { err } from "@murky-web/typebuddy";\n'; - - return [fixer.insertTextBeforeRange(insertRange, importText)]; + return [ + fixer.insertTextBeforeRange(insertion.range, insertion.text), + ]; } - function getIndentation(node: AstNode): string { - const lines = sourceCode.getText(node).split("\n"); - // `split` always yields at least one element, but - // `noUncheckedIndexedAccess` cannot know that. - const firstLine = lines[0] ?? ""; - const match = /^\s*/.exec(firstLine); - return match ? match[0] : ""; + // Two zero-width insertions leave the body's own text untouched, byte + // for byte. Rebuilding it statement by statement used to strip leading + // whitespace on every line — including lines inside template literals, + // which silently changed string values. + function wrapBlockFixes( + bodyNode: AstNode, + fixer: RuleFixer, + ): unknown[] | null { + const range = bodyNode.range; + if (!Array.isArray(range) || range.length < 2) { + return null; + } + + return [ + fixer.insertTextBeforeRange( + [range[0] + 1, range[0] + 1], + "\ntry {", + ), + fixer.insertTextBeforeRange( + [range[1] - 1, range[1] - 1], + "} catch {\nreturn err();\n}\n", + ), + ]; } - function wrapInTryCatch(node: AstNode): string { - const body = Array.isArray(node.body) ? node.body : []; - const indent = getIndentation(node); - const innerIndent = `${indent} `; - const bodyText = body - .map((statement) => { - const text = sourceCode.getText(statement); - return `${innerIndent}${text.replaceAll(/^\s*/gm, "")}`; - }) - .join("\n"); + /** + * Wrap an expression-bodied arrow's value in a block with try/catch. + * + * The body node's range stops inside any wrapping parentheses, so + * replacing it in `async () => ({ a: 1 })` would leave the parentheses + * around a block and produce `({ try { ... } })`. Walking left over + * those parentheses first, then inserting on either side, keeps the + * expression itself untouched — parentheses and all. + */ + function wrapExpressionBodyFixes( + node: AstNode, + bodyNode: AstNode, + fixer: RuleFixer, + ): unknown[] | null { + const arrowRange = node.range; + const bodyRange = bodyNode.range; + if ( + !Array.isArray(arrowRange) || + arrowRange.length < 2 || + !Array.isArray(bodyRange) || + bodyRange.length < 2 + ) { + return null; + } + + let prefix = sourceCode + .getText(node) + .slice(0, bodyRange[0] - arrowRange[0]); + while (prefix.trimEnd().endsWith("(")) { + prefix = prefix.trimEnd().slice(0, -1); + } + + const start = arrowRange[0] + prefix.length; + // The arrow's own end already sits past any closing parenthesis. + const end = arrowRange[1]; + if (start >= end) { + return null; + } - return `{ -${indent}try { -${bodyText} -${indent}} catch { -${innerIndent}return err(); -${indent}} -}`; + return [ + fixer.insertTextBeforeRange( + [start, start], + "{\ntry {\nreturn ", + ), + fixer.insertTextBeforeRange( + [end, end], + ";\n} catch {\nreturn err();\n}\n}", + ), + ]; } function checkFunction(node: AstNode) { if (node.async !== true) return; if (isCallArgumentCallback(node)) return; const bodyNode = node.body; - if ( - !bodyNode || - Array.isArray(bodyNode) || - bodyNode.type !== "BlockStatement" - ) { + if (!bodyNode || Array.isArray(bodyNode)) return; + + // A function that throws has already decided its failure is not a + // value. Wrapping it in try/catch would swallow that decision. + if (throwsDeliberately(bodyNode)) return; + + // An expression-bodied arrow (`async () => fetch(url)`) returns the + // one value the try/catch exists to guard, so it needs a block + // before it can get one. + if (bodyNode.type !== "BlockStatement") { + context.report({ + node, + messageId: "missingTryCatch", + fix(fixer) { + const wrapFixes = wrapExpressionBodyFixes( + node, + bodyNode, + fixer, + ); + if (!wrapFixes) { + return null; + } + + return [ + ...ensureErrImportFixes(node, fixer), + ...wrapFixes, + ]; + }, + }); return; } const body = Array.isArray(bodyNode.body) ? bodyNode.body : []; if (hasTryCatch(body)) return; - // A function that throws has already decided its failure is not a - // value. Wrapping it in try/catch would swallow that decision. - if (throwsDeliberately(bodyNode)) return; context.report({ node, messageId: "missingTryCatch", fix(fixer) { - return [ - ...ensureErrImportFixes(node, fixer), - fixer.replaceText(bodyNode, wrapInTryCatch(bodyNode)), - ]; + const wrapFixes = wrapBlockFixes(bodyNode, fixer); + if (!wrapFixes) { + return null; + } + + return [...ensureErrImportFixes(node, fixer), ...wrapFixes]; }, }); } diff --git a/packages/typebuddy/rules/deliberate_throw.ts b/packages/typebuddy/rules/deliberate_throw.ts index a9668f8..e62dbed 100644 --- a/packages/typebuddy/rules/deliberate_throw.ts +++ b/packages/typebuddy/rules/deliberate_throw.ts @@ -1,17 +1,4 @@ -type AstNode = { - type: string; - [key: string]: unknown; -}; - -const FUNCTION_TYPES = new Set([ - "ArrowFunctionExpression", - "FunctionDeclaration", - "FunctionExpression", -]); - -function isNode(value: unknown): value is AstNode { - return typeof value === "object" && value !== null && "type" in value; -} +import { walkOwnSubtree } from "./own_subtree.js"; /** * Whether a function body throws on purpose. @@ -34,40 +21,15 @@ function isNode(value: unknown): value is AstNode { * @returns {boolean} True when the body throws outside any nested function. */ function throwsDeliberately(body: unknown): boolean { - if (!isNode(body)) { - return false; - } - - const pending: AstNode[] = [body]; - while (pending.length > 0) { - // `pop` on a non-empty array always yields a node; the length check above is - // the loop condition, but `noUncheckedIndexedAccess` cannot see that. - const current = pending.pop(); - if (current === undefined) { - break; - } - - if (current.type === "ThrowStatement") { - return true; - } - - for (const [key, value] of Object.entries(current)) { - // `parent` points back up the tree; following it would never terminate. - if (key === "parent") { - continue; - } + let found = false; - const candidates = Array.isArray(value) ? value : [value]; - for (const candidate of candidates) { - if (!isNode(candidate) || FUNCTION_TYPES.has(candidate.type)) { - continue; - } - pending.push(candidate); - } + walkOwnSubtree(body, (node) => { + if (node.type === "ThrowStatement") { + found = true; } - } + }); - return false; + return found; } export { throwsDeliberately }; diff --git a/packages/typebuddy/rules/maybe_promise_rule.ts b/packages/typebuddy/rules/maybe_promise_rule.ts index 0a71774..37de4a8 100644 --- a/packages/typebuddy/rules/maybe_promise_rule.ts +++ b/packages/typebuddy/rules/maybe_promise_rule.ts @@ -1,4 +1,6 @@ import { throwsDeliberately } from "./deliberate_throw.js"; +import { getTypeBuddyImportInsertion } from "./typebuddy_import.js"; +import { walkOwnSubtree } from "./own_subtree.js"; type AstNode = { type: string; @@ -11,7 +13,7 @@ type RuleContext = { report(descriptor: { node: unknown; messageId: string; - fix(fixer: { + fix?(fixer: { replaceText(node: unknown, text: string): unknown; insertTextBeforeRange( range: [number, number], @@ -48,48 +50,34 @@ function isCallArgumentCallback(node: AstNode): boolean { return false; } - return Array.isArray(parent["arguments"]) && parent["arguments"].includes(node); + return ( + Array.isArray(parent["arguments"]) && parent["arguments"].includes(node) + ); } -function isIdentifierNamed(node: unknown, name: string): boolean { - return isNode(node) && node.type === "Identifier" && node["name"] === name; -} +/** + * Return-type names whose type argument is the value an async function settles + * on. `AsyncResult` is the preferred spelling of `MaybePromise`, so a function + * annotated with it has to be read the same way — otherwise the rule goes blind + * on exactly the code the package tells people to write. + */ +const AWAITED_TYPE_NAMES = new Set(["AsyncResult", "MaybePromise", "Promise"]); -function getProgram(node: AstNode): AstNode | null { +function isAmbient(node: AstNode): boolean { let current: AstNode | undefined = node; - while (current?.parent) { + while (current) { + if (current["declare"] === true) { + return true; + } current = current.parent; } - return current?.type === "Program" ? current : null; -} - -function isImportDeclaration(node: unknown): node is AstNode { - return isNode(node) && node.type === "ImportDeclaration"; -} - -function getStringLiteralValue(node: unknown): string | null { - if (!isNode(node)) { - return null; - } - - if ( - (node.type === "Literal" || node.type === "StringLiteral") && - typeof node["value"] === "string" - ) { - return node["value"]; - } - - return null; + return false; } -function getProgramBody(node: AstNode): AstNode[] { - if (!Array.isArray(node["body"])) { - return []; - } - - return node["body"].filter(isNode); +function isIdentifierNamed(node: unknown, name: string): boolean { + return isNode(node) && node.type === "Identifier" && node["name"] === name; } function isResultHelperCall(node: unknown, name: "ok" | "err"): boolean { @@ -148,61 +136,15 @@ function getObjectPropertyValue( } function isBooleanLiteral(node: unknown, expected: boolean): boolean { - return isNode(node) && node.type === "Literal" && node["value"] === expected; + return ( + isNode(node) && node.type === "Literal" && node["value"] === expected + ); } function isNullLiteral(node: unknown): boolean { return isNode(node) && node.type === "Literal" && node["value"] === null; } -function hasTypeBuddyHelperImport(program: AstNode, helperName: "ok" | "err") { - return getProgramBody(program).some((statement) => { - if (!isImportDeclaration(statement)) { - return false; - } - - if ( - getStringLiteralValue(statement["source"]) !== "@murky-web/typebuddy" - ) { - return false; - } - - if (statement["importKind"] === "type") { - return false; - } - - const specifiers = Array.isArray(statement["specifiers"]) - ? statement["specifiers"] - : []; - - return specifiers.some((specifier) => { - return ( - isNode(specifier) && - specifier.type === "ImportSpecifier" && - isIdentifierNamed(specifier["local"], helperName) - ); - }); - }); -} - -function getTypeBuddyImportInsertRange( - program: AstNode, -): [number, number] | null { - const body = getProgramBody(program); - const imports = body.filter(isImportDeclaration); - const anchor = imports.at(-1) ?? body[0] ?? program; - - if (!Array.isArray(anchor["range"]) || anchor["range"].length < 2) { - return null; - } - - if (imports.length > 0) { - return [anchor["range"][1], anchor["range"][1]]; - } - - return [anchor["range"][0], anchor["range"][0]]; -} - const rule = { create(context: RuleContext) { const sourceCode = context.getSourceCode(); @@ -216,7 +158,9 @@ const rule = { const typeName = node["typeName"]; if (!isNode(typeName) || typeName.type !== "Identifier") return null; - return typeof typeName["name"] === "string" ? typeName["name"] : null; + return typeof typeName["name"] === "string" + ? typeName["name"] + : null; } function getTypeArgument(node: AstNode): AstNode | null { @@ -227,8 +171,23 @@ const rule = { return firstParam; } - function getPromiseTypeArgument(node: AstNode): AstNode | null { - if (getTypeName(node) !== "Promise") return null; + /** + * The awaited type behind an async function's return annotation. + * + * Every spelling counts. Matching only `Promise` meant the rule read the + * return type of code it had not migrated yet and went blind the moment + * `MaybePromise` was there — so a bare `return;` in a + * `MaybePromise` function never became `return ok();`, while the + * identical `Promise` function was fixed. It only ever worked + * because the rename and this lookup happen in the same pass, off the + * same AST. + */ + function getAwaitedTypeArgument(node: AstNode): AstNode | null { + const typeName = getTypeName(node); + if (typeName === null || !AWAITED_TYPE_NAMES.has(typeName)) { + return null; + } + return getTypeArgument(node); } @@ -241,32 +200,19 @@ const rule = { return []; } - const program = getProgram(node); - if (!program || hasTypeBuddyHelperImport(program, helperName)) { - return []; - } - - const insertRange = getTypeBuddyImportInsertRange(program); - if (!insertRange) { + const insertion = getTypeBuddyImportInsertion(node, helperName); + if (!insertion) { return []; } scheduledHelperImports.add(helperName); - const hasImports = insertRange[0] !== 0; - const importText = hasImports - ? `\nimport { ${helperName} } from "@murky-web/typebuddy";` - : `import { ${helperName} } from "@murky-web/typebuddy";\n`; - - return [fixer.insertTextBeforeRange(insertRange, importText)]; + return [ + fixer.insertTextBeforeRange(insertion.range, insertion.text), + ]; } - function checkReturnType(node: AstNode) { - if (!isAsyncFunction(node)) return; - if (isCallArgumentCallback(node)) return; - // Same exemption as `require-try-catch`: a deliberate throw is not a - // result waiting to be wrapped. - if (throwsDeliberately(node["body"])) return; + function reportPromiseReturnType(node: AstNode) { if (!isNode(node["returnType"])) return; const typeAnnotation = node["returnType"]["typeAnnotation"]; @@ -283,6 +229,33 @@ const rule = { }); } + function checkReturnType(node: AstNode) { + if (!isAsyncFunction(node)) return; + if (isCallArgumentCallback(node)) return; + // Same exemption as `require-try-catch`: a deliberate throw is not a + // result waiting to be wrapped. + if (throwsDeliberately(node["body"])) return; + + reportPromiseReturnType(node); + } + + /** + * A type-level signature — an interface method, a function type, an + * overload — can never carry `async`, so routing it through + * `checkReturnType` meant these three visitors never fired at all. + * `Promise` in a signature is the same contract the rule rewrites + * everywhere else, so it gets reported here without the async gate. + * + * Ambient declarations are the exception: `declare` describes code the + * project does not own, and rewriting a third-party framework's + * callback shape to `MaybePromise` would be a lie about that API. + */ + function checkSignatureReturnType(node: AstNode) { + if (isAmbient(node)) return; + + reportPromiseReturnType(node); + } + function wrapReturnValue( node: AstNode, isAsync: boolean, @@ -383,6 +356,95 @@ const rule = { } } + function collectOwnReturns(root: unknown): AstNode[] { + const returns: AstNode[] = []; + + walkOwnSubtree(root, (candidate) => { + if (candidate.type === "TryStatement") { + return false; + } + + if (candidate.type === "ReturnStatement") { + returns.push(candidate); + } + + return true; + }); + + return returns; + } + + function isSettledFailure(argument: unknown): boolean { + return ( + (isNode(argument) && + argument.type === "Identifier" && + argument["name"] === "FAILED_PROMISE") || + isResultHelperCall(argument, "err") + ); + } + + /** + * Push a single `return` inside a catch block towards `err()`. + * + * Only the lossless rewrites carry a fix. Replacing + * `return ok("defaults")` with `return err()` would silently drop the + * author's fallback, and the old behaviour — appending `return err()` + * after it — produced unreachable code and a report that could never be + * satisfied. Where the fallback matters, moving it into the try block is + * a decision the author has to make, so the rule reports and stops. + */ + function reportCatchReturn(tryNode: AstNode, statement: AstNode) { + const argument = statement["argument"]; + + if (isSettledFailure(argument)) return; + + // `return;` — adding the result is pure addition, nothing is lost. + if (!isNode(argument)) { + context.report({ + node: statement, + messageId: "returnFailedPromise", + fix(fixer) { + return [ + ...ensureResultHelperImportFixes( + tryNode, + fixer, + "err", + ), + fixer.replaceText(statement, "return err()"), + ]; + }, + }); + return; + } + + if (hasIsErrorFlag(argument, true)) { + // `{ isError: true, value: null }` says the right thing the + // long way round; `err()` is the same value. + if (isNullLiteral(getObjectPropertyValue(argument, "value"))) { + context.report({ + node: argument, + messageId: "preferErrResult", + fix(fixer) { + return [ + ...ensureResultHelperImportFixes( + tryNode, + fixer, + "err", + ), + fixer.replaceText(argument, "err()"), + ]; + }, + }); + } + return; + } + + context.report({ + node: statement, + messageId: "returnFailedPromise", + }); + } + function processTryCatch(node: AstNode) { let parent = node.parent; let isAsync = false; @@ -407,109 +469,89 @@ const rule = { let returnType: AstNode | null = null; if (isNode(parentFunction["returnType"])) { - const typeAnnotation = parentFunction["returnType"]["typeAnnotation"]; + const typeAnnotation = + parentFunction["returnType"]["typeAnnotation"]; if (isTypeReference(typeAnnotation)) { - returnType = getPromiseTypeArgument(typeAnnotation); + returnType = getAwaitedTypeArgument(typeAnnotation); } } - const blockBody = - isNode(node["block"]) && Array.isArray(node["block"]["body"]) - ? node["block"]["body"] - : []; - for (const statement of blockBody) { - if (isNode(statement) && statement.type === "ReturnStatement") { - wrapReturnValue(statement, isAsync, returnType); - } + // Every return the try block owns, not just the ones sitting + // directly in it. `try { if (flag) return "early"; return "late"; }` + // used to have only `"late"` wrapped, because the walk never looked + // inside the `if`. + // + // A nested `try` is left alone: the visitor fires for it separately, + // and its own catch branch has to stay the failure path rather than + // be read as another try-block return. + for (const statement of collectOwnReturns(node["block"])) { + wrapReturnValue(statement, isAsync, returnType); } - if (!isNode(node["handler"]) || !isNode(node["handler"]["body"])) return; - const catchBody = Array.isArray(node["handler"]["body"]["body"]) - ? node["handler"]["body"]["body"].filter(isNode) - : []; - - for (const statement of catchBody) { - if ( - statement.type === "ReturnStatement" && - isNode(statement["argument"]) - ) { - wrapReturnValue(statement, isAsync, returnType); - } - } + const handler = node["handler"]; + if (!isNode(handler) || !isNode(handler["body"])) return; - const hasCorrectReturn = catchBody.some( - (statement) => - statement.type === "ReturnStatement" && - isNode(statement["argument"]) && - ((statement["argument"].type === "Identifier" && - statement["argument"]["name"] === "FAILED_PROMISE") || - isResultHelperCall(statement["argument"], "err") || - hasIsErrorFlag(statement["argument"], true)), - ); + // The catch branch is the failure path, and it settles on `err()` + // — nothing else. A default belongs in the try block, where the + // absence is read as a value instead of being recovered from a + // throw. So a catch return is never offered `ok(...)`; feeding both + // suggestions to the same statement used to produce two reports + // with opposite fixes. + const catchReturns = collectOwnReturns(handler["body"]); - if (!hasCorrectReturn) { - context.report({ - node: node["handler"], - messageId: "returnFailedPromise", - fix(fixer) { - const lastStatement = catchBody.at(-1); - if ( - lastStatement?.type === "ReturnStatement" && - isNode(lastStatement["argument"]) && - lastStatement["argument"].type === "ObjectExpression" - ) { - return [ - ...ensureResultHelperImportFixes( - node, - fixer, - "err", - ), - fixer.replaceText( - lastStatement, - "return err()", - ), - ]; - } + for (const statement of catchReturns) { + reportCatchReturn(node, statement); + } - const handler = node["handler"]; - if (!isNode(handler)) { - return null; - } + // A nested try/catch inside this catch settles the branch on its + // own, so its returns count here even though `collectOwnReturns` + // hands them to the inner `processTryCatch`. Appending another + // `return err()` after them would be unreachable. + let settlesItself = false; + walkOwnSubtree(handler["body"], (candidate) => { + if (candidate.type === "ReturnStatement") { + settlesItself = true; + } + }); - const bodyRange = handler["body"]; - if ( - !isNode(bodyRange) || - !Array.isArray(bodyRange["range"]) - ) { - return null; - } + if (settlesItself) return; - return [ - ...ensureResultHelperImportFixes( - node, - fixer, - "err", - ), - fixer.insertTextBeforeRange( - [ - bodyRange["range"][1] - 1, - bodyRange["range"][1] - 1, - ], - "return err(); ", - ), - ]; - }, - }); - } + context.report({ + node: handler, + messageId: "returnFailedPromise", + fix(fixer) { + const handlerBody = handler["body"]; + if ( + !isNode(handlerBody) || + !Array.isArray(handlerBody["range"]) + ) { + return null; + } + + return [ + ...ensureResultHelperImportFixes(node, fixer, "err"), + fixer.insertTextBeforeRange( + [ + handlerBody["range"][1] - 1, + handlerBody["range"][1] - 1, + ], + // The newline matters: `catch { log() }` would + // otherwise become `catch { log() return err(); }`, + // which does not parse. + "\nreturn err();\n", + ), + ]; + }, + }); } return { FunctionDeclaration: checkReturnType, FunctionExpression: checkReturnType, ArrowFunctionExpression: checkReturnType, - TSDeclareFunction: checkReturnType, - TSFunctionType: checkReturnType, - TSMethodSignature: checkReturnType, + TSDeclareFunction: checkSignatureReturnType, + TSFunctionType: checkSignatureReturnType, + TSMethodSignature: checkSignatureReturnType, TryStatement: processTryCatch, }; }, diff --git a/packages/typebuddy/rules/optional_rule.ts b/packages/typebuddy/rules/optional_rule.ts index 5950aef..f43dca1 100644 --- a/packages/typebuddy/rules/optional_rule.ts +++ b/packages/typebuddy/rules/optional_rule.ts @@ -17,8 +17,12 @@ const rule = { const undefinedType = node.types.find( (typeNode) => typeNode.type === "TSUndefinedKeyword", ); + // `null | undefined` has no non-nullish member to name, and rewriting + // it to `Optional` only restated it with more syntax. const otherType = node.types.find( - (typeNode) => typeNode.type !== "TSUndefinedKeyword", + (typeNode) => + typeNode.type !== "TSUndefinedKeyword" && + typeNode.type !== "TSNullKeyword", ); if (undefinedType && otherType) { diff --git a/packages/typebuddy/rules/own_subtree.ts b/packages/typebuddy/rules/own_subtree.ts new file mode 100644 index 0000000..332f6ef --- /dev/null +++ b/packages/typebuddy/rules/own_subtree.ts @@ -0,0 +1,70 @@ +type AstNode = { + type: string; + [key: string]: unknown; +}; + +const FUNCTION_TYPES = new Set([ + "ArrowFunctionExpression", + "FunctionDeclaration", + "FunctionExpression", +]); + +function isNode(value: unknown): value is AstNode { + return typeof value === "object" && value !== null && "type" in value; +} + +/** + * Visit every node that belongs to `root`, stopping at nested functions. + * + * Both async rules need the same notion of "this function's own code". A + * `throw` or a `return` inside a nested function belongs to that function, not + * to the one being linted, so the walk never descends into one — and `parent` + * points back up the tree, so following it would not terminate. + * + * `visit` may return `false` to keep the walk out of a node's children. That is + * how a rule declines a subtree that some other visitor already owns. + * + * @param {unknown} root - The node to start from. Visited itself, and never + * skipped for being a function. + * @param {(node: AstNode) => boolean | void} visit - Called once per node. + * Return `false` to skip that node's children. + * @returns {void} + */ +function walkOwnSubtree( + root: unknown, + visit: (node: AstNode) => boolean | void, +): void { + if (!isNode(root)) { + return; + } + + const pending: AstNode[] = [root]; + while (pending.length > 0) { + // `pop` on a non-empty array always yields a node; the length check above + // is the loop condition, but `noUncheckedIndexedAccess` cannot see that. + const current = pending.pop(); + if (current === undefined) { + break; + } + + if (visit(current) === false) { + continue; + } + + for (const [key, value] of Object.entries(current)) { + if (key === "parent") { + continue; + } + + const candidates = Array.isArray(value) ? value : [value]; + for (const candidate of candidates) { + if (!isNode(candidate) || FUNCTION_TYPES.has(candidate.type)) { + continue; + } + pending.push(candidate); + } + } + } +} + +export { walkOwnSubtree }; diff --git a/packages/typebuddy/rules/typebuddy_import.ts b/packages/typebuddy/rules/typebuddy_import.ts new file mode 100644 index 0000000..a8bfbaf --- /dev/null +++ b/packages/typebuddy/rules/typebuddy_import.ts @@ -0,0 +1,134 @@ +type AstNode = { + type: string; + parent?: AstNode; + [key: string]: unknown; +}; + +const PACKAGE_NAME = "@murky-web/typebuddy"; + +function isNode(value: unknown): value is AstNode { + return typeof value === "object" && value !== null && "type" in value; +} + +function isIdentifierNamed(node: unknown, name: string): boolean { + return isNode(node) && node.type === "Identifier" && node["name"] === name; +} + +function isImportDeclaration(node: unknown): node is AstNode { + return isNode(node) && node.type === "ImportDeclaration"; +} + +function getStringLiteralValue(node: unknown): string | null { + if (!isNode(node)) { + return null; + } + + if ( + (node.type === "Literal" || node.type === "StringLiteral") && + typeof node["value"] === "string" + ) { + return node["value"]; + } + + return null; +} + +function getProgram(node: unknown): AstNode | null { + if (!isNode(node)) { + return null; + } + + let current: AstNode | undefined = node; + while (current?.parent) { + current = current.parent; + } + + return current?.type === "Program" ? current : null; +} + +function getProgramBody(program: AstNode): AstNode[] { + if (!Array.isArray(program["body"])) { + return []; + } + + return program["body"].filter(isNode); +} + +function importsHelper(program: AstNode, helperName: string): boolean { + return getProgramBody(program).some((statement) => { + if (!isImportDeclaration(statement)) { + return false; + } + + if (getStringLiteralValue(statement["source"]) !== PACKAGE_NAME) { + return false; + } + + // A type-only import brings no runtime binding, so it does not satisfy a + // fix that is about to call the helper. + if (statement["importKind"] === "type") { + return false; + } + + const specifiers = Array.isArray(statement["specifiers"]) + ? statement["specifiers"] + : []; + + return specifiers.some((specifier) => { + return ( + isNode(specifier) && + specifier.type === "ImportSpecifier" && + isIdentifierNamed(specifier["local"], helperName) + ); + }); + }); +} + +/** + * Where and what to insert so `helperName` is imported from the package. + * + * Both async rules add `ok`/`err` imports, and both used to carry their own + * copy of this — including the same bug. Whether the import lands after an + * existing one was inferred from the anchor's offset (`range[0] !== 0`), which + * is only the same question in a file whose first statement starts at byte + * zero. A leading comment moved the first statement off zero, the wrong branch + * won, and the fix emitted a leading newline instead of a trailing one: + * + * import { err } from "@murky-web/typebuddy";export async function load() + * + * The list of imports answers it directly, so that is what decides now. + * + * @param {unknown} node - Any node in the file; the program is found from it. + * @param {string} helperName - The named export to import, e.g. `"err"`. + * @returns {{ range: [number, number]; text: string } | null} The zero-width + * insertion to apply, or null when the helper is already imported or no + * anchor carries a usable range. + */ +function getTypeBuddyImportInsertion( + node: unknown, + helperName: string, +): { range: [number, number]; text: string } | null { + const program = getProgram(node); + if (!program || importsHelper(program, helperName)) { + return null; + } + + const body = getProgramBody(program); + const imports = body.filter(isImportDeclaration); + const anchor = imports.at(-1) ?? body[0] ?? program; + const range = anchor["range"]; + if (!Array.isArray(range) || range.length < 2) { + return null; + } + + const statement = `import { ${helperName} } from "${PACKAGE_NAME}";`; + if (imports.length > 0) { + const end: number = range[1]; + return { range: [end, end], text: `\n${statement}` }; + } + + const start: number = range[0]; + return { range: [start, start], text: `${statement}\n` }; +} + +export { getTypeBuddyImportInsertion }; diff --git a/packages/typebuddy/smoke/oxlint/fix-expected-bare.ts b/packages/typebuddy/smoke/oxlint/fix-expected-bare.ts new file mode 100644 index 0000000..7f96219 --- /dev/null +++ b/packages/typebuddy/smoke/oxlint/fix-expected-bare.ts @@ -0,0 +1,15 @@ +// A leading comment and no imports at all. +// +// The insertion point used to be classified by the anchor's byte offset, so a +// comment that pushed the first statement off zero flipped the branch and the +// import was emitted with a leading newline instead of a trailing one — landing +// glued to the statement it was inserted before. +import { ok } from "@murky-web/typebuddy"; +import { err } from "@murky-web/typebuddy"; +export async function needsHelpers(): AsyncResult { + try { + return ok("value"); + } catch { + return err(); + } +} diff --git a/packages/typebuddy/smoke/oxlint/fix-expected.ts b/packages/typebuddy/smoke/oxlint/fix-expected.ts index a5afe84..436b3af 100644 --- a/packages/typebuddy/smoke/oxlint/fix-expected.ts +++ b/packages/typebuddy/smoke/oxlint/fix-expected.ts @@ -46,3 +46,147 @@ export async function wrapMe(): MaybePromise { return err(); } } + +declare function log(message: string): void; + +// A catch whose last statement has no semicolon: the inserted `return err()` +// used to be glued onto it, producing code that does not parse. +export async function oneLineCatch(): MaybePromise { + try { + return ok("one-line"); + } catch { + log("boom"); + return err(); + } +} + +// The wrapping fix used to strip leading whitespace on every line, which +// silently rewrote the contents of template literals. +export async function keepsTemplate(): MaybePromise { + try { + return ok(`multi + line template`); + } catch { + return err(); + } +} + +// An expression-bodied arrow has no block for the try/catch to live in. +export const arrowBody = async () => { + try { + return ok(await Promise.resolve("arrow")); + } catch { + return err(); + } +}; + +// A parenthesised expression body: the parentheses sit outside the body node. +export const parenthesisedArrowBody = async () => { + try { + return ok({ wrapped: true }); + } catch { + return err(); + } +}; + +// Type-level signatures can never be `async`, so the async gate used to make +// these three visitors unreachable. +export interface Loader { + load(): MaybePromise; +} + +export type LoadFn = () => MaybePromise; + +// Already migrated by hand. The awaited-type lookup used to match only +// `Promise`, so this bare `return` stayed unwrapped while its `Promise` +// twin two functions up was fixed. +export async function alreadyMigrated(): MaybePromise { + try { + return ok(); + } catch { + return err(); + } +} + +// `null | undefined` names no value to put in the brackets; it used to be +// rewritten to the equivalent-but-longer `Optional`. +export type NeitherCandidate = null | undefined; + +// `AsyncResult` is the preferred spelling of `MaybePromise`, so the awaited-type +// lookup has to know it too — otherwise the rule goes blind on exactly the +// annotation the package tells people to write. +export async function preferredSpelling(): AsyncResult { + try { + return ok(); + } catch { + return err(); + } +} + +// A value handed back from the failure path. The rule reports it — the default +// belongs in the try block — but must not rewrite it: replacing it with `err()` +// would drop the fallback, and appending one produced unreachable code. +export async function fallbackInCatch(): AsyncResult { + try { + return ok("real"); + } catch { + return "fallback"; + } +} + +// Same for a fallback already spelled as a result: reported, never rewritten. +export async function okInCatch(): AsyncResult { + try { + return ok("real"); + } catch { + return ok("defaults"); + } +} + +// A bare `return` in a catch loses nothing by becoming `err()`, so it is fixed. +export async function bareCatchReturn(): AsyncResult { + try { + return ok(); + } catch { + return err(); + } +} + +// Returns the try block owns, but does not hold directly. Only the last one +// used to be wrapped, because the walk never looked inside the `if`, the loop +// or the `switch`. The callback's return belongs to the callback, not here. +export async function nestedReturns( + flag: boolean, + n: number, +): AsyncResult { + try { + if (flag) return ok("early"); + for (const x of [1]) { + if (x) return ok("loop"); + } + switch (n) { + case 1: + return ok("switch"); + } + const cb = () => { + return "belongs to the callback"; + }; + return ok(cb()); + } catch { + return err(); + } +} + +// The inner try/catch settles this catch branch on its own, so no `return err()` +// may be appended after it — that would be unreachable. +export async function settledByNestedTry(): AsyncResult { + try { + return ok("real"); + } catch { + try { + return ok("retry"); + } catch { + return err(); + } + } +} diff --git a/packages/typebuddy/smoke/oxlint/fix-input-bare.ts b/packages/typebuddy/smoke/oxlint/fix-input-bare.ts new file mode 100644 index 0000000..15881dd --- /dev/null +++ b/packages/typebuddy/smoke/oxlint/fix-input-bare.ts @@ -0,0 +1,11 @@ +// A leading comment and no imports at all. +// +// The insertion point used to be classified by the anchor's byte offset, so a +// comment that pushed the first statement off zero flipped the branch and the +// import was emitted with a leading newline instead of a trailing one — landing +// glued to the statement it was inserted before. +export async function needsHelpers(): AsyncResult { + try { + return "value"; + } catch {} +} diff --git a/packages/typebuddy/smoke/oxlint/fix-input.ts b/packages/typebuddy/smoke/oxlint/fix-input.ts index 4b916c8..b7c13b6 100644 --- a/packages/typebuddy/smoke/oxlint/fix-input.ts +++ b/packages/typebuddy/smoke/oxlint/fix-input.ts @@ -33,3 +33,121 @@ export async function fromObject(): MaybePromise { export async function wrapMe(): Promise { return "wrapped"; } + +declare function log(message: string): void; + +// A catch whose last statement has no semicolon: the inserted `return err()` +// used to be glued onto it, producing code that does not parse. +export async function oneLineCatch(): Promise { + try { + return "one-line"; + } catch { log("boom") } +} + +// The wrapping fix used to strip leading whitespace on every line, which +// silently rewrote the contents of template literals. +export async function keepsTemplate(): Promise { + return `multi + line template`; +} + +// An expression-bodied arrow has no block for the try/catch to live in. +export const arrowBody = async () => await Promise.resolve("arrow"); + +// A parenthesised expression body: the parentheses sit outside the body node. +export const parenthesisedArrowBody = async () => ({ wrapped: true }); + +// Type-level signatures can never be `async`, so the async gate used to make +// these three visitors unreachable. +export interface Loader { + load(): Promise; +} + +export type LoadFn = () => Promise; + +// Already migrated by hand. The awaited-type lookup used to match only +// `Promise`, so this bare `return` stayed unwrapped while its `Promise` +// twin two functions up was fixed. +export async function alreadyMigrated(): MaybePromise { + try { + return; + } catch {} +} + +// `null | undefined` names no value to put in the brackets; it used to be +// rewritten to the equivalent-but-longer `Optional`. +export type NeitherCandidate = null | undefined; + +// `AsyncResult` is the preferred spelling of `MaybePromise`, so the awaited-type +// lookup has to know it too — otherwise the rule goes blind on exactly the +// annotation the package tells people to write. +export async function preferredSpelling(): AsyncResult { + try { + return; + } catch {} +} + +// A value handed back from the failure path. The rule reports it — the default +// belongs in the try block — but must not rewrite it: replacing it with `err()` +// would drop the fallback, and appending one produced unreachable code. +export async function fallbackInCatch(): AsyncResult { + try { + return ok("real"); + } catch { + return "fallback"; + } +} + +// Same for a fallback already spelled as a result: reported, never rewritten. +export async function okInCatch(): AsyncResult { + try { + return ok("real"); + } catch { + return ok("defaults"); + } +} + +// A bare `return` in a catch loses nothing by becoming `err()`, so it is fixed. +export async function bareCatchReturn(): AsyncResult { + try { + return ok(); + } catch { + return; + } +} + +// Returns the try block owns, but does not hold directly. Only the last one +// used to be wrapped, because the walk never looked inside the `if`, the loop +// or the `switch`. The callback's return belongs to the callback, not here. +export async function nestedReturns(flag: boolean, n: number): AsyncResult { + try { + if (flag) return "early"; + for (const x of [1]) { + if (x) return "loop"; + } + switch (n) { + case 1: + return "switch"; + } + const cb = () => { + return "belongs to the callback"; + }; + return cb(); + } catch { + return err(); + } +} + +// The inner try/catch settles this catch branch on its own, so no `return err()` +// may be appended after it — that would be unreachable. +export async function settledByNestedTry(): AsyncResult { + try { + return ok("real"); + } catch { + try { + return ok("retry"); + } catch { + return err(); + } + } +} diff --git a/packages/typebuddy/smoke/oxlint/run-fix-smoke.ts b/packages/typebuddy/smoke/oxlint/run-fix-smoke.ts index 15d3abf..bd9358d 100644 --- a/packages/typebuddy/smoke/oxlint/run-fix-smoke.ts +++ b/packages/typebuddy/smoke/oxlint/run-fix-smoke.ts @@ -3,61 +3,78 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; const smokeDir = new URL("./", import.meta.url); -const sourcePath = new URL("./fix-input.ts", smokeDir); -const expectedPath = new URL("./fix-expected.ts", smokeDir); const configPath = new URL("./.oxlintrc.jsonc", smokeDir); const formatConfigPath = new URL( "../../../config/oxc/.oxfmtrc.jsonc", smokeDir, ); -const tempDir = await mkdtemp(join(tmpdir(), "typebuddy-oxlint-fix-")); -const tempFilePath = join(tempDir, "fix-input.ts"); +// Two shapes, because the import insertion behaves differently in each: a file +// that already imports something gets the helper appended after the last +// import, one that does not gets it prepended before the first statement. +const cases = [ + { input: "./fix-input.ts", expected: "./fix-expected.ts" }, + { input: "./fix-input-bare.ts", expected: "./fix-expected-bare.ts" }, +] as const; -try { - const input = await readFile(sourcePath, "utf8"); - await writeFile(tempFilePath, input, "utf8"); +async function runCase(input: string, expected: string): Promise { + const sourcePath = new URL(input, smokeDir); + const expectedPath = new URL(expected, smokeDir); - let previous = input; + const tempDir = await mkdtemp(join(tmpdir(), "typebuddy-oxlint-fix-")); + const tempFilePath = join(tempDir, "fixture.ts"); - for (let pass = 0; pass < 6; pass += 1) { - const result = - await Bun.$`oxlint -c ${configPath.pathname} --fix ${tempFilePath}` - .quiet() - .nothrow(); + try { + const source = await readFile(sourcePath, "utf8"); + await writeFile(tempFilePath, source, "utf8"); - if (![0, 1].includes(result.exitCode)) { - console.error("Oxlint fix smoke test failed unexpectedly."); - process.exit(result.exitCode); - } + let previous = source; + + for (let pass = 0; pass < 6; pass += 1) { + const result = + await Bun.$`oxlint -c ${configPath.pathname} --fix ${tempFilePath}` + .quiet() + .nothrow(); + + if (![0, 1].includes(result.exitCode)) { + console.error( + `Oxlint fix smoke test failed unexpectedly on ${input}.`, + ); + process.exit(result.exitCode); + } - const next = await readFile(tempFilePath, "utf8"); - if (next === previous) { - break; + const next = await readFile(tempFilePath, "utf8"); + if (next === previous) { + break; + } + + previous = next; } - previous = next; - } + await Bun.$`oxfmt -c ${formatConfigPath.pathname} ${tempFilePath}`.quiet(); - await Bun.$`oxfmt -c ${formatConfigPath.pathname} ${tempFilePath}`.quiet(); - - const [actual, expected] = await Promise.all([ - readFile(tempFilePath, "utf8"), - readFile(expectedPath, "utf8"), - ]); - - if (actual !== expected) { - console.error( - "Oxlint fix smoke test did not produce the expected output.", - ); - console.error("\n--- Expected ---\n"); - console.error(expected); - console.error("\n--- Actual ---\n"); - console.error(actual); - process.exit(1); + const [actual, want] = await Promise.all([ + readFile(tempFilePath, "utf8"), + readFile(expectedPath, "utf8"), + ]); + + if (actual !== want) { + console.error( + `Oxlint fix smoke test did not produce the expected output for ${input}.`, + ); + console.error("\n--- Expected ---\n"); + console.error(want); + console.error("\n--- Actual ---\n"); + console.error(actual); + process.exit(1); + } + } finally { + await rm(tempDir, { recursive: true, force: true }); } +} - console.log("Oxlint fix smoke test passed."); -} finally { - await rm(tempDir, { recursive: true, force: true }); +for (const { input, expected } of cases) { + await runCase(input, expected); } + +console.log(`Oxlint fix smoke test passed (${cases.length} fixtures).`); From 8fdeefbc796781bd23ced02dabf10631b2497c38 Mon Sep 17 00:00:00 2001 From: Paul <72733450+paul1995tu@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:48:17 +0200 Subject: [PATCH 2/4] feat(oxlint-plugin-solid): add effect and ref rules for Solid 2.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `solid/no-untracked-effect-read` reports a reactive read in the untracked apply phase of a two-argument `createEffect`. `reactivity` cannot report this: it marks the apply phase as a tracked scope, and its model has no state for "matches, but does not subscribe". `solid/no-owned-primitives-in-ref` reports owner-bound primitives inside a ref callback, which Solid 2.0 runs unowned — nothing disposes them. Directive factories keep working. `solid/no-setter-in-effect` also reports an async apply phase that awaits and writes the result back; its existing check requires every statement to be a setter call, which an `await` breaks. Two repairs: `prefer-class-object` matched only lowercase `classlist` and so missed every real `classList` call site, and `jsx-no-duplicate-props` recommended that same removed prop. `solid/imports` gained the three boundary primitives. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CkmW7HzpL6zqpjuo1ETm2w --- .changeset/solid-2-effect-and-ref-rules.md | 34 ++++ packages/config/oxc/linting/solid.jsonc | 3 + packages/oxlint-plugin-solid/README.md | 10 +- packages/oxlint-plugin-solid/src/index.mjs | 6 + .../oxlint-plugin-solid/src/rules/imports.mjs | 7 + .../src/rules/jsx_no_duplicate_props.mjs | 2 +- .../src/rules/no_owned_primitives_in_ref.mjs | 126 +++++++++++++ .../src/rules/no_react_deps.mjs | 2 +- .../src/rules/no_setter_in_effect.mjs | 171 +++++++++++++++++ .../src/rules/no_untracked_effect_read.mjs | 178 ++++++++++++++++++ .../src/rules/prefer_class_object.mjs | 11 +- .../src/rules/reactivity.mjs | 2 +- .../oxlint-plugin-solid/tests/rule_cases.mjs | 94 +++++++++ 13 files changed, 638 insertions(+), 8 deletions(-) create mode 100644 .changeset/solid-2-effect-and-ref-rules.md create mode 100644 packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs create mode 100644 packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs create mode 100644 packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs diff --git a/.changeset/solid-2-effect-and-ref-rules.md b/.changeset/solid-2-effect-and-ref-rules.md new file mode 100644 index 0000000..f965737 --- /dev/null +++ b/.changeset/solid-2-effect-and-ref-rules.md @@ -0,0 +1,34 @@ +--- +"@murky-web/oxlint-plugin-solid": minor +--- + +Add two rules for Solid 2.0's split effects and unowned ref callbacks, and +repair two existing ones. + +`solid/no-untracked-effect-read` reports a reactive read in the apply phase of a +two-argument `createEffect`. That phase runs untracked, so +`createEffect(() => roomId(), (id) => connect(id, theme()))` never re-runs when +`theme` changes. Nothing reported this: `reactivity` marks the apply phase as a +tracked scope, and its model has no state for "matches, but does not subscribe". + +`solid/no-owned-primitives-in-ref` reports `createEffect`, `onCleanup` and +related primitives inside a ref callback. Solid 2.0 ref callbacks are unowned — +`getOwner()` returns null — so the effect is never disposed and the cleanup +never runs. Directive factories, which create these primitives while they still +have an owner, are unaffected. + +`solid/no-setter-in-effect` now also reports an async apply phase that awaits and +writes the result back into a signal. Its existing check requires every +statement to be a setter call, which an `await` breaks. + +`solid/prefer-class-object` matched only the lowercase `classlist`. JSX prop +names are case-sensitive and the Solid 1 prop is `classList`, so the migration +rule missed every real call site. + +`solid/imports` now knows `createLoadingBoundary`, `createErrorBoundary` and +`createRevealOrder`, so importing one from the wrong module is reported like +every neighbouring primitive. + +`solid/jsx-no-duplicate-props` recommended `classList`, which Solid 2.0 removed +and the neighbouring rule reports. It now points at the object and array forms +of `class`. diff --git a/packages/config/oxc/linting/solid.jsonc b/packages/config/oxc/linting/solid.jsonc index f2e70f2..01dea36 100644 --- a/packages/config/oxc/linting/solid.jsonc +++ b/packages/config/oxc/linting/solid.jsonc @@ -12,10 +12,13 @@ "solid/no-array-handlers": "error", "solid/no-destructure": "error", "solid/no-innerhtml": "error", + "solid/no-owned-primitives-in-ref": "error", "solid/no-proxy-apis": "error", "solid/no-react-deps": "error", "solid/no-react-specific-props": "error", + "solid/no-setter-in-effect": "error", "solid/no-unknown-namespaces": "error", + "solid/no-untracked-effect-read": "error", "solid/prefer-arrow-components": "error", "solid/prefer-class-object": "error", "solid/prefer-for": "error", diff --git a/packages/oxlint-plugin-solid/README.md b/packages/oxlint-plugin-solid/README.md index 6984dbb..308dfb3 100644 --- a/packages/oxlint-plugin-solid/README.md +++ b/packages/oxlint-plugin-solid/README.md @@ -11,7 +11,15 @@ und laufen ohne `eslint-plugin-solid` als Zielprojekt-Dependency. Aktuell sind enthalten: - die komplette von `eslint-plugin-solid` exportierte Regelmenge -- die zusaetzliche Projektregel `solid/prefer-arrow-components` +- die zusaetzlichen Projektregeln `solid/prefer-arrow-components`, + `solid/no-setter-in-effect` (meldet Effects, die nur in ein Signal oder einen + Store schreiben, statt den Wert abzuleiten -- und Effects, die etwas awaiten + und das Ergebnis zurueckschreiben, statt es aus einer Derivation zu liefern) + `solid/no-untracked-effect-read` (meldet reaktive Reads in der + apply-Phase eines zweiphasigen `createEffect`, die dort nicht tracken) und + `solid/no-owned-primitives-in-ref` (meldet `createEffect`/`onCleanup` und + Verwandte in einem Ref-Callback -- der laeuft ohne Owner, nichts raeumt sie + je wieder ab) - ein Test-Harness, der die exportierte Rule-Surface und echte Diagnostik gegen Temp-Projekte prueft diff --git a/packages/oxlint-plugin-solid/src/index.mjs b/packages/oxlint-plugin-solid/src/index.mjs index 65d8207..f66bae1 100644 --- a/packages/oxlint-plugin-solid/src/index.mjs +++ b/packages/oxlint-plugin-solid/src/index.mjs @@ -8,10 +8,13 @@ import { jsxUsesVarsRule } from "./rules/jsx_uses_vars.mjs"; import { noArrayHandlersRule } from "./rules/no_array_handlers.mjs"; import noDestructureRule from "./rules/no_destructure.mjs"; import noInnerhtmlRule from "./rules/no_innerhtml.mjs"; +import noOwnedPrimitivesInRefRule from "./rules/no_owned_primitives_in_ref.mjs"; import noProxyApisRule from "./rules/no_proxy_apis.mjs"; import noReactDepsRule from "./rules/no_react_deps.mjs"; import { noReactSpecificPropsRule } from "./rules/no_react_specific_props.mjs"; +import noSetterInEffectRule from "./rules/no_setter_in_effect.mjs"; import noUnknownNamespacesRule from "./rules/no_unknown_namespaces.mjs"; +import noUntrackedEffectReadRule from "./rules/no_untracked_effect_read.mjs"; import { preferArrowComponentsRule } from "./rules/prefer_arrow_components.mjs"; import preferClassObjectRule from "./rules/prefer_class_object.mjs"; import preferForRule from "./rules/prefer_for.mjs"; @@ -37,10 +40,13 @@ const extendedPlugin = { "no-array-handlers": noArrayHandlersRule, "no-destructure": noDestructureRule, "no-innerhtml": noInnerhtmlRule, + "no-owned-primitives-in-ref": noOwnedPrimitivesInRefRule, "no-proxy-apis": noProxyApisRule, "no-react-deps": noReactDepsRule, "no-react-specific-props": noReactSpecificPropsRule, + "no-setter-in-effect": noSetterInEffectRule, "no-unknown-namespaces": noUnknownNamespacesRule, + "no-untracked-effect-read": noUntrackedEffectReadRule, "prefer-arrow-components": preferArrowComponentsRule, "prefer-class-object": preferClassObjectRule, "prefer-for": preferForRule, diff --git a/packages/oxlint-plugin-solid/src/rules/imports.mjs b/packages/oxlint-plugin-solid/src/rules/imports.mjs index 24b6ee0..319450e 100644 --- a/packages/oxlint-plugin-solid/src/rules/imports.mjs +++ b/packages/oxlint-plugin-solid/src/rules/imports.mjs @@ -14,12 +14,19 @@ for (const primitive of [ "children", "createContext", "createEffect", + // The primitive forms of `Loading`, `Errored` and `Reveal`. Only custom + // boundary components and renderer integrations reach for them, which is + // why they were missing here — and why a wrong-module import of one went + // unreported while every neighbouring primitive was checked. + "createErrorBoundary", + "createLoadingBoundary", "createMemo", "createOptimistic", "createOptimisticStore", "createProjection", "createReaction", "createRenderEffect", + "createRevealOrder", "createRoot", "createSignal", "createStore", diff --git a/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs b/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs index a409e13..1a90942 100644 --- a/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs +++ b/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs @@ -26,7 +26,7 @@ export default createRule({ messages: { noDuplicateProps: "Duplicate props are not allowed.", noDuplicateClass: - "Duplicate `class` props are not allowed; while it might seem to work, it can break unexpectedly. Use `classList` instead.", + "Duplicate `class` props are not allowed; while it might seem to work, it can break unexpectedly. Pass one `class` with the object or array form instead.", noDuplicateChildren: "Using {{used}} at the same time is not allowed.", }, diff --git a/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs b/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs new file mode 100644 index 0000000..8239af6 --- /dev/null +++ b/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs @@ -0,0 +1,126 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +import { isFunctionNode, jsxPropName, trackImports } from "../utils.mjs"; + +const createRule = ESLintUtils.RuleCreator.withoutDocs; + +/** + * Primitives that attach to the current owner. + * + * An effect or memo created without an owner is never disposed, and a cleanup + * registered without one never runs. + */ +const OWNED_PRIMITIVES = [ + "createEffect", + "createMemo", + "createProjection", + "createReaction", + "createRenderEffect", + "onCleanup", + "onSettled", +]; + +/** The callbacks a `ref` prop applies, including the ones in a ref array. */ +function getRefCallbacks(expression) { + if (isFunctionNode(expression)) { + return [expression]; + } + + // `ref={[storeElement, autofocus, listen(...)]}` — Solid flattens the array + // and calls each entry, so an inline callback anywhere in it is a ref + // callback like any other. + if (expression?.type === "ArrayExpression") { + return expression.elements.flatMap((element) => + element !== null && isFunctionNode(element) ? [element] : [], + ); + } + + return []; +} + +/** + * Walk a callback body, stopping at nested functions. + * + * Only calls the ref callback makes itself are reported. A nested function may + * be handed to something that supplies an owner, and guessing wrong would flag + * working code. + */ +function walkOwnBody(node, visit) { + if (node === null || typeof node !== "object") { + return; + } + + visit(node); + + for (const [key, value] of Object.entries(node)) { + if (key === "parent") { + continue; + } + + const candidates = Array.isArray(value) ? value : [value]; + for (const candidate of candidates) { + if ( + candidate !== null && + typeof candidate === "object" && + typeof candidate.type === "string" && + !isFunctionNode(candidate) + ) { + walkOwnBody(candidate, visit); + } + } + } +} + +export default createRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow creating owner-bound primitives inside a ref callback.", + url: "https://docs.solidjs.com/", + }, + schema: [], + messages: { + ownedPrimitiveInRef: + "`{{name}}` is called inside a ref callback, which runs without an owner — nothing will ever dispose it. Create it in the directive factory instead, and let the returned callback only store the element.", + }, + }, + defaultOptions: [], + create(context) { + const { matchImport, handleImportDeclaration } = trackImports(); + + return { + ImportDeclaration: handleImportDeclaration, + JSXAttribute(node) { + if (jsxPropName(node) !== "ref") { + return; + } + + if (node.value?.type !== "JSXExpressionContainer") { + return; + } + + for (const callback of getRefCallbacks(node.value.expression)) { + walkOwnBody(callback.body, (candidate) => { + if ( + candidate.type !== "CallExpression" || + candidate.callee.type !== "Identifier" || + !matchImport( + OWNED_PRIMITIVES, + candidate.callee.name, + ) + ) { + return; + } + + context.report({ + node: candidate, + messageId: "ownedPrimitiveInRef", + data: { name: candidate.callee.name }, + }); + }); + } + }, + }; + }, +}); diff --git a/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs b/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs index a6ae05f..bdfbef3 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs @@ -14,7 +14,7 @@ export default createRule({ schema: [], messages: { noUselessDep: - "In Solid, `{{name}}` doesn't accept a dependency array because it automatically tracks its dependencies. If you really need to override the list of dependencies, use `on`.", + "In Solid, `{{name}}` doesn't accept a dependency array because it automatically tracks its dependencies. To narrow what is tracked, read only those values in the compute phase.", }, }, defaultOptions: [], diff --git a/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs b/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs new file mode 100644 index 0000000..30e2df6 --- /dev/null +++ b/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs @@ -0,0 +1,171 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +import { isFunctionNode, trackImports } from "../utils.mjs"; + +const createRule = ESLintUtils.RuleCreator.withoutDocs; +const SETTER_INDEX = 1; + +/** `const [value, setValue] = createSignal(...)` -> "setValue". */ +function getSetterName(declarator) { + if (declarator.id.type !== "ArrayPattern") { + return null; + } + + const element = declarator.id.elements[SETTER_INDEX]; + + return element?.type === "Identifier" ? element.name : null; +} + +/** + * The phase that runs untracked and performs the work. Solid 2.0 passes it as + * the second argument; a lone function is still the whole effect. + */ +function getEffectPhase(node) { + const [compute, apply] = node.arguments; + + if (apply) { + return isFunctionNode(apply) ? apply : null; + } + + return isFunctionNode(compute) ? compute : null; +} + +function isSetterCall(node, setterNames) { + return ( + node?.type === "CallExpression" && + node.callee.type === "Identifier" && + setterNames.has(node.callee.name) + ); +} + +/** True when any statement anywhere in the phase writes to a signal or store. */ +function writesSignalsAnywhere(node, setterNames) { + if (node === null || typeof node !== "object") { + return false; + } + + if (isSetterCall(node, setterNames)) { + return true; + } + + return Object.entries(node).some(([key, value]) => { + // `parent` points back up the tree; following it would not terminate. + if (key === "parent") { + return false; + } + + const candidates = Array.isArray(value) ? value : [value]; + + return candidates.some( + (candidate) => + candidate !== null && + typeof candidate === "object" && + !isFunctionNode(candidate) && + writesSignalsAnywhere(candidate, setterNames), + ); + }); +} + +/** True when the phase does nothing but write to signals or stores. */ +function onlyWritesSignals(phase, setterNames) { + if (phase.body.type !== "BlockStatement") { + return isSetterCall(phase.body, setterNames); + } + + const { body } = phase.body; + + return ( + body.length > 0 && + body.every( + (statement) => + statement.type === "ExpressionStatement" && + isSetterCall(statement.expression, setterNames), + ) + ); +} + +export default createRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow effects that only copy a reactive value into another signal or store.", + url: "https://docs.solidjs.com/", + }, + schema: [], + messages: { + asyncSetterInEffect: + "This effect awaits and then writes the result into a signal or store. Return the value from a derivation instead: `createMemo(async () => ...)`, or the function form of `createStore` for nested data. Solid coordinates readiness, error propagation, and dropping superseded runs.", + noSetterInEffect: + "This effect only writes to a signal or store. Calculate the value where it is read instead: a derived function, `createMemo`, or a writable derived `createSignal(() => ...)` for a local override.", + }, + }, + defaultOptions: [], + create(context) { + const { matchImport, handleImportDeclaration } = trackImports(); + const setterNames = new Set(); + + return { + ImportDeclaration: handleImportDeclaration, + VariableDeclarator(node) { + if ( + node.init?.type !== "CallExpression" || + node.init.callee.type !== "Identifier" || + !matchImport( + ["createSignal", "createStore"], + node.init.callee.name, + ) + ) { + return; + } + + const setterName = getSetterName(node); + if (setterName) { + setterNames.add(setterName); + } + }, + CallExpression(node) { + if ( + node.callee.type !== "Identifier" || + !matchImport( + ["createEffect", "createRenderEffect"], + node.callee.name, + ) + ) { + return; + } + + const phase = getEffectPhase(node); + if (!phase) { + return; + } + + // Asynchrony is checked first because it is the more specific + // diagnosis: `onlyWritesSignals` also matches a lone + // `setResults(await search(q))`, and the async message is the + // one that names readiness and superseded runs. + // + // `onlyWritesSignals` never sees the multi-statement shape at + // all — the `await` is not a setter call — which is why an + // async phase gets its own search. + if ( + phase.async === true && + writesSignalsAnywhere(phase.body, setterNames) + ) { + context.report({ + node: phase, + messageId: "asyncSetterInEffect", + }); + return; + } + + if (onlyWritesSignals(phase, setterNames)) { + context.report({ + node: phase, + messageId: "noSetterInEffect", + }); + } + }, + }; + }, +}); diff --git a/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs b/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs new file mode 100644 index 0000000..609361b --- /dev/null +++ b/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs @@ -0,0 +1,178 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +import { isFunctionNode, isPropsByName, trackImports } from "../utils.mjs"; + +const createRule = ESLintUtils.RuleCreator.withoutDocs; +const GETTER_INDEX = 0; + +/** `const [value, setValue] = createSignal(...)` -> "value". */ +function getGetterName(declarator) { + if (declarator.id.type !== "ArrayPattern") { + return null; + } + + const element = declarator.id.elements[GETTER_INDEX]; + + return element?.type === "Identifier" ? element.name : null; +} + +/** Every name a function's parameters bind, however they are destructured. */ +function collectParameterNames(fn, into) { + const pending = [...fn.params]; + + while (pending.length > 0) { + const pattern = pending.pop(); + if (pattern === null || typeof pattern !== "object") { + continue; + } + + if (pattern.type === "Identifier") { + into.add(pattern.name); + continue; + } + + for (const [key, value] of Object.entries(pattern)) { + if (key === "parent") { + continue; + } + + const candidates = Array.isArray(value) ? value : [value]; + for (const candidate of candidates) { + if (candidate !== null && typeof candidate === "object") { + pending.push(candidate); + } + } + } + } +} + +/** + * Walk the phase body, stopping at nested functions. + * + * A read inside a nested function runs whenever that function is called, which + * is a different question from whether this phase subscribes to it. + */ +function walkPhase(node, visit) { + if (node === null || typeof node !== "object") { + return; + } + + visit(node); + + for (const [key, value] of Object.entries(node)) { + if (key === "parent") { + continue; + } + + const candidates = Array.isArray(value) ? value : [value]; + for (const candidate of candidates) { + if ( + candidate !== null && + typeof candidate === "object" && + typeof candidate.type === "string" && + !isFunctionNode(candidate) + ) { + walkPhase(candidate, visit); + } + } + } +} + +export default createRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow reading reactive values in the untracked apply phase of an effect.", + url: "https://docs.solidjs.com/", + }, + schema: [], + messages: { + untrackedEffectRead: + "'{{name}}' is read in the effect's apply phase, which runs untracked — changes to it will not re-run this effect. Read it in the compute phase and take it as an argument instead.", + }, + }, + defaultOptions: [], + create(context) { + const { matchImport, handleImportDeclaration } = trackImports(); + const getterNames = new Set(); + + return { + ImportDeclaration: handleImportDeclaration, + VariableDeclarator(node) { + if ( + node.init?.type !== "CallExpression" || + node.init.callee.type !== "Identifier" || + !matchImport(["createSignal"], node.init.callee.name) + ) { + return; + } + + const getterName = getGetterName(node); + if (getterName) { + getterNames.add(getterName); + } + }, + CallExpression(node) { + if ( + node.callee.type !== "Identifier" || + !matchImport( + ["createEffect", "createRenderEffect"], + node.callee.name, + ) + ) { + return; + } + + // Only the two-argument form has an apply phase. A lone + // function is the whole effect and tracks throughout. + const [compute, apply] = node.arguments; + if (!compute || !apply || !isFunctionNode(apply)) { + return; + } + + // Whatever the compute phase handed over arrives as a + // parameter, so those names are already settled values. + const settled = new Set(); + collectParameterNames(apply, settled); + + walkPhase(apply.body, (candidate) => { + // `signal()` — a getter call that did not come through the + // compute phase. + if ( + candidate.type === "CallExpression" && + candidate.callee.type === "Identifier" && + getterNames.has(candidate.callee.name) && + !settled.has(candidate.callee.name) + ) { + context.report({ + node: candidate, + messageId: "untrackedEffectRead", + data: { name: candidate.callee.name }, + }); + return; + } + + // `props.value` — a props read is reactive for the same + // reason and is just as invisible to the tracker here. + if ( + candidate.type === "MemberExpression" && + !candidate.computed && + candidate.object.type === "Identifier" && + isPropsByName(candidate.object.name) && + !settled.has(candidate.object.name) && + candidate.property.type === "Identifier" + ) { + context.report({ + node: candidate, + messageId: "untrackedEffectRead", + data: { + name: `${candidate.object.name}.${candidate.property.name}`, + }, + }); + } + }); + }, + }; + }, +}); diff --git a/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs b/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs index e8a734b..320fb93 100644 --- a/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs +++ b/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs @@ -8,7 +8,7 @@ export default createRule({ type: "problem", docs: { description: - "Enforce the `class` object/array form over the removed `classlist` prop or a classnames helper. Solid 2.0 accepts `{ [class: string]: boolean }` on `class` directly.", + "Enforce the `class` object/array form over the removed `classList` prop or a classnames helper. Solid 2.0 accepts `{ [class: string]: boolean }` on `class` directly.", url: "https://docs.solidjs.com/", }, fixable: "code", @@ -33,7 +33,7 @@ export default createRule({ ], messages: { classlistRemoved: - "The `classlist` prop was removed in Solid 2.0. Pass the object to `class` instead.", + "The `classList` prop was removed in Solid 2.0. Pass the object to `class` instead.", preferClassObject: "Pass the object to `class` directly instead of through {{ classnames }}.", }, @@ -46,8 +46,11 @@ export default createRule({ JSXAttribute(node) { const name = jsxPropName(node); - // `classlist={{...}}` no longer exists; the object belongs on `class`. - if (name === "classlist") { + // `classList={{...}}` no longer exists; the object belongs on + // `class`. JSX prop names are case-sensitive, so matching only + // the lowercase spelling missed every real Solid 1 call site — + // which is the code this rule exists to migrate. + if (name === "classList" || name === "classlist") { context.report({ node, messageId: "classlistRemoved", diff --git a/packages/oxlint-plugin-solid/src/rules/reactivity.mjs b/packages/oxlint-plugin-solid/src/rules/reactivity.mjs index 19541fb..bc7ef4e 100644 --- a/packages/oxlint-plugin-solid/src/rules/reactivity.mjs +++ b/packages/oxlint-plugin-solid/src/rules/reactivity.mjs @@ -203,7 +203,7 @@ export default createRule({ noWrite: "The reactive variable '{{name}}' should not be reassigned or altered directly.", untrackedReactive: - "The reactive variable '{{name}}' should be used within JSX, a tracked scope (like createEffect), or inside an event handler function, or else changes will be ignored.", + "The reactive variable '{{name}}' should be used within JSX, a tracked scope (like createEffect), or inside an event handler function, or else changes will be ignored. To keep it reactive, derive it rather than copy it: a plain function, `createMemo`, or `createSignal(() => ...)` / `createStore(() => ...)` for a local override that a new source value should replace.", expectedFunctionGotExpression: "The reactive variable '{{name}}' should be wrapped in a function for reactivity. This includes event handler bindings on native elements, which are not reactive like other JSX props.", badSignal: diff --git a/packages/oxlint-plugin-solid/tests/rule_cases.mjs b/packages/oxlint-plugin-solid/tests/rule_cases.mjs index 4622bdf..3db0988 100644 --- a/packages/oxlint-plugin-solid/tests/rule_cases.mjs +++ b/packages/oxlint-plugin-solid/tests/rule_cases.mjs @@ -9,10 +9,13 @@ const expectedRuleIds = Object.freeze([ "no-array-handlers", "no-destructure", "no-innerhtml", + "no-owned-primitives-in-ref", "no-proxy-apis", "no-react-deps", "no-react-specific-props", + "no-setter-in-effect", "no-unknown-namespaces", + "no-untracked-effect-read", "prefer-arrow-components", "prefer-class-object", "prefer-for", @@ -123,6 +126,52 @@ createEffect(() => { `, ruleId: "no-react-specific-props", }), + Object.freeze({ + code: `import { createEffect, createSignal } from "solid-js"; + +const [firstName] = createSignal("Ada"); +const [, setFullName] = createSignal(""); + +createEffect( + () => firstName(), + (first) => { + setFullName(first + " Lovelace"); + }, +); +`, + ruleId: "no-setter-in-effect", + }), + Object.freeze({ + code: `import { createEffect, createSignal } from "solid-js"; + +const [query] = createSignal(""); +const [, setResults] = createSignal([]); + +createEffect( + () => query(), + async (value) => { + const found = await fetch(value).then((response) => response.json()); + setResults(found); + }, +); +`, + ruleId: "no-setter-in-effect", + }), + Object.freeze({ + code: `import { createEffect, createSignal } from "solid-js"; + +const [roomId] = createSignal("lobby"); +const [theme] = createSignal("dark"); + +createEffect( + () => roomId(), + (id) => { + connect(id, theme()); + }, +); +`, + ruleId: "no-untracked-effect-read", + }), Object.freeze({ code: `export const View = () => { return
Hello
; @@ -141,6 +190,51 @@ export function View(props: Props): JSX.Element { `, ruleId: "prefer-arrow-components", }), + Object.freeze({ + code: `export const View = (props) => { + return
Hello
; +}; +`, + ruleId: "prefer-class-object", + }), + Object.freeze({ + code: `import { + createErrorBoundary, + createLoadingBoundary, + createRevealOrder, +} from "@solidjs/web"; + +export const boundaries = [ + createErrorBoundary, + createLoadingBoundary, + createRevealOrder, +]; +`, + ruleId: "imports", + }), + Object.freeze({ + code: `import { createEffect, createSignal, onCleanup } from "solid-js"; + +export const View = () => { + const [width] = createSignal(0); + + return ( +
{ + createEffect( + () => width(), + (value) => element.setAttribute("data-width", value), + ); + onCleanup(() => element.remove()); + }} + > + Hello +
+ ); +}; +`, + ruleId: "no-owned-primitives-in-ref", + }), Object.freeze({ code: `const cn = (classes) => { return classes; From 9ba08912c7bd429c09a3961cbcfc73ca9472e957 Mon Sep 17 00:00:00 2001 From: Paul <72733450+paul1995tu@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:22:37 +0200 Subject: [PATCH 3/4] ci: run the solid plugin and the subpath smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither gate existed. `packages/oxlint-plugin-solid` had no CI job at all, so its suite only ran through `release:verify`. `smoke:subpath` — the guard added with the subpath entry points, which checks that every `exports` target was actually emitted — was wired into neither CI nor `release:verify:typebuddy`. The push trigger also listened on `master` while the default branch is `main`, so it never fired; pull requests were the only thing running CI. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CkmW7HzpL6zqpjuo1ETm2w --- .github/workflows/ci.yml | 23 ++++++++++++++++++++++- package.json | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc8b875..d0313d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: pull_request: push: branches: - - master + - main workflow_dispatch: jobs: @@ -43,3 +43,24 @@ jobs: - name: Smoke treeshaking run: bun --cwd packages/typebuddy run smoke:treeshake + + - name: Smoke subpaths + run: bun --cwd packages/typebuddy run smoke:subpath + + oxlint-plugin-solid: + name: Oxlint plugin solid + runs-on: ubuntu-latest + steps: + - name: Checkout repo + uses: actions/checkout@v4 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: "package.json" + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Test oxlint-plugin-solid + run: bun --cwd packages/oxlint-plugin-solid run test diff --git a/package.json b/package.json index b0be472..94b12e8 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "release:verify": "bun run release:verify:typebuddy && bun run release:verify:simplelog && bun run release:verify:oxlint-plugin-solid", "release:verify:oxlint-plugin-solid": "bun run --cwd ./packages/oxlint-plugin-solid test", "release:verify:simplelog": "bun run --cwd ./packages/simplelog lint && bun run --cwd ./packages/simplelog typecheck && bun run --cwd ./packages/simplelog test && bun run --cwd ./packages/simplelog smoke:oxlint && bun run --cwd ./packages/simplelog smoke:entries", - "release:verify:typebuddy": "bun run --cwd ./packages/typebuddy lint && bun run --cwd ./packages/typebuddy typecheck && bun run --cwd ./packages/typebuddy test && bun run --cwd ./packages/typebuddy smoke:globals && bun run --cwd ./packages/typebuddy smoke:oxlint && bun run --cwd ./packages/typebuddy smoke:oxlint:fix && bun run --cwd ./packages/typebuddy smoke:treeshake", + "release:verify:typebuddy": "bun run --cwd ./packages/typebuddy lint && bun run --cwd ./packages/typebuddy typecheck && bun run --cwd ./packages/typebuddy test && bun run --cwd ./packages/typebuddy smoke:globals && bun run --cwd ./packages/typebuddy smoke:oxlint && bun run --cwd ./packages/typebuddy smoke:oxlint:fix && bun run --cwd ./packages/typebuddy smoke:subpath && bun run --cwd ./packages/typebuddy smoke:treeshake", "test": "bun run --workspaces --if-present test", "typecheck": "bun run --workspaces --if-present typecheck", "lint": "oxlint -c ./packages/config/oxc/.oxlintrc.jsonc --type-aware ./packages", From a161c8fc96dda770547b4cb812ec605551212c79 Mon Sep 17 00:00:00 2001 From: Paul <72733450+paul1995tu@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:43:38 +0200 Subject: [PATCH 4/4] refactor(oxlint-plugin-solid): drop eslint and @typescript-eslint/utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin targets oxlint, but handed the whole TypeScript-ESLint toolchain to every consumer. Two things kept it there, and only one of them was real. `ESLintUtils.RuleCreator.withoutDocs` wrapped all 23 rules. It exists for TypeScript type inference, which a `.mjs` plugin gets nothing from: oxlint takes the rule object as-is, so the wrapper is gone and the rules are plain object literals now. `@oxlint/plugins` offers `defineRule` for the same purpose, but it is likewise a type helper — adopting it would trade one dependency for another and buy nothing until this package moves to TypeScript. The wrapper did do one thing at runtime: it merged `defaultOptions` into `context.options` and passed the result to a second `create` parameter. `reactivity` is the only rule that reads that parameter, and it now resolves its own default. Without this the rule threw on every file. The five real helpers — `findVariable`, `getStaticValue`, `getStringIfConstant`, `getPropertyName`, `getFunctionHeadLocation` — now come straight from `@eslint-community/eslint-utils`, which `@typescript-eslint/utils` was only re-exporting. That drops `@typescript-eslint/typescript-estree` and `@typescript-eslint/scope-manager` from the tree. Runtime dependencies go from 7 to 6, and the two heaviest are the ones removed. This does not change the advisory count: measured from an identical baseline, before and after are both 15 high / 32 moderate / 3 low, since the remaining findings come from vite, vitest, tsdown and changesets — workspace devDeps that are never published. The gain is what this package hands to its consumers, not the audit number. 45 pass, 0 fail. Every rule probe re-run against the doc-sanctioned patterns behaves exactly as before. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CkmW7HzpL6zqpjuo1ETm2w --- packages/oxlint-plugin-solid/README.md | 7 +++++++ packages/oxlint-plugin-solid/package.json | 3 +-- packages/oxlint-plugin-solid/src/compat.mjs | 4 ++-- .../src/rules/components_return_once.mjs | 7 ++----- .../src/rules/event_handlers.mjs | 8 +++----- .../oxlint-plugin-solid/src/rules/imports.mjs | 7 ++----- .../src/rules/jsx_no_duplicate_props.mjs | 7 ++----- .../src/rules/jsx_no_script_url.mjs | 8 +++----- .../src/rules/jsx_no_undef.mjs | 7 ++----- .../src/rules/jsx_uses_vars.mjs | 8 ++------ .../src/rules/no_array_handlers.mjs | 8 ++------ .../src/rules/no_destructure.mjs | 8 +++----- .../src/rules/no_innerhtml.mjs | 8 +++----- .../src/rules/no_owned_primitives_in_ref.mjs | 8 ++------ .../src/rules/no_proxy_apis.mjs | 8 ++------ .../src/rules/no_react_deps.mjs | 7 ++----- .../src/rules/no_react_specific_props.mjs | 7 ++----- .../src/rules/no_setter_in_effect.mjs | 7 ++----- .../src/rules/no_unknown_namespaces.mjs | 7 ++----- .../src/rules/no_untracked_effect_read.mjs | 7 ++----- .../src/rules/prefer_class_object.mjs | 7 ++----- .../oxlint-plugin-solid/src/rules/prefer_for.mjs | 8 +++----- .../src/rules/prefer_show.mjs | 7 ++----- .../oxlint-plugin-solid/src/rules/reactivity.mjs | 16 +++++++++------- .../src/rules/self_closing_comp.mjs | 7 ++----- .../oxlint-plugin-solid/src/rules/style_prop.mjs | 8 +++----- packages/oxlint-plugin-solid/tests/helpers.mjs | 3 +-- 27 files changed, 70 insertions(+), 127 deletions(-) diff --git a/packages/oxlint-plugin-solid/README.md b/packages/oxlint-plugin-solid/README.md index 308dfb3..7ff35cd 100644 --- a/packages/oxlint-plugin-solid/README.md +++ b/packages/oxlint-plugin-solid/README.md @@ -8,6 +8,13 @@ projektspezifische Regeln wie `solid/prefer-arrow-components`. Die Regelmodule unter `src/rules/` sind aus dem Upstream-Quellstand abgeleitet und laufen ohne `eslint-plugin-solid` als Zielprojekt-Dependency. +Das Paket zieht auch ESLint selbst nicht mehr nach. Die Regeln sind schlichte +Objekte, wie Oxlint sie erwartet -- der `createRule`-Wrapper aus +`@typescript-eslint/utils` war nur eine TypeScript-Typhilfe und ist entfallen. +Geblieben sind fuenf AST-Helfer, die jetzt direkt aus +`@eslint-community/eslint-utils` kommen, statt die komplette +TypeScript-ESLint-Toolchain in den Baum jedes Konsumenten zu ziehen. + Aktuell sind enthalten: - die komplette von `eslint-plugin-solid` exportierte Regelmenge diff --git a/packages/oxlint-plugin-solid/package.json b/packages/oxlint-plugin-solid/package.json index 8f8c397..4b36e03 100644 --- a/packages/oxlint-plugin-solid/package.json +++ b/packages/oxlint-plugin-solid/package.json @@ -26,8 +26,7 @@ }, "sideEffects": false, "dependencies": { - "@typescript-eslint/utils": "^8.57.1", - "eslint": "^9.38.0", + "@eslint-community/eslint-utils": "^4.9.1", "estraverse": "^5.3.0", "is-html": "^2.0.0", "kebab-case": "^1.0.2", diff --git a/packages/oxlint-plugin-solid/src/compat.mjs b/packages/oxlint-plugin-solid/src/compat.mjs index 5420476..2ee86ec 100644 --- a/packages/oxlint-plugin-solid/src/compat.mjs +++ b/packages/oxlint-plugin-solid/src/compat.mjs @@ -1,4 +1,4 @@ -import { ASTUtils } from "@typescript-eslint/utils"; +import { findVariable as utilsFindVariable } from "@eslint-community/eslint-utils"; export function getSourceCode(context) { if (typeof context.getSourceCode === "function") { @@ -23,7 +23,7 @@ export function getScope(context, node) { } export function findVariable(context, node) { - return ASTUtils.findVariable(getScope(context, node), node); + return utilsFindVariable(getScope(context, node), node); } export function markVariableAsUsed(context, name, node) { diff --git a/packages/oxlint-plugin-solid/src/rules/components_return_once.mjs b/packages/oxlint-plugin-solid/src/rules/components_return_once.mjs index 89a60c3..6e6ff0f 100644 --- a/packages/oxlint-plugin-solid/src/rules/components_return_once.mjs +++ b/packages/oxlint-plugin-solid/src/rules/components_return_once.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { getSourceCode } from "../compat.mjs"; import { getFunctionName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const isNothing = (node) => { if (!node) { return true; @@ -17,7 +14,7 @@ const isNothing = (node) => { } }; const getLineLength = (loc) => loc.end.line - loc.start.line + 1; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -199,4 +196,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/event_handlers.mjs b/packages/oxlint-plugin-solid/src/rules/event_handlers.mjs index 1c5b1a8..15811fd 100644 --- a/packages/oxlint-plugin-solid/src/rules/event_handlers.mjs +++ b/packages/oxlint-plugin-solid/src/rules/event_handlers.mjs @@ -1,9 +1,7 @@ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getStaticValue } from "@eslint-community/eslint-utils"; import { getScope, getSourceCode } from "../compat.mjs"; import { isDOMElementName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getStaticValue } = ASTUtils; const COMMON_EVENTS = [ "onAnimationEnd", "onAnimationIteration", @@ -82,7 +80,7 @@ const isNonstandardEventName = (lowercaseEventName) => Boolean(NONSTANDARD_EVENTS_MAP[lowercaseEventName]); const getStandardEventHandlerName = (lowercaseEventName) => NONSTANDARD_EVENTS_MAP[lowercaseEventName]; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -295,4 +293,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/imports.mjs b/packages/oxlint-plugin-solid/src/rules/imports.mjs index 319450e..b405f9e 100644 --- a/packages/oxlint-plugin-solid/src/rules/imports.mjs +++ b/packages/oxlint-plugin-solid/src/rules/imports.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { getSourceCode } from "../compat.mjs"; import { appendImports, insertImports, removeSpecifier } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; // Solid 2.0 moved the renderers into `@solidjs/*` packages and pulled the store // APIs into the core. Symbols that are legitimately exported from more than one // package (the control-flow components, `ComponentProps`) are deliberately left @@ -105,7 +102,7 @@ for (const type of ["ClassValue", "IntrinsicElement", "JSX", "RequestEvent"]) { } const sourceRegex = /^(?:solid-js|@solidjs\/(?:web|h|html|universal))$/; const isSource = (source) => sourceRegex.test(source); -export default createRule({ +export default { meta: { type: "suggestion", docs: { @@ -196,4 +193,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs b/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs index 1a90942..39dfe53 100644 --- a/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs +++ b/packages/oxlint-plugin-solid/src/rules/jsx_no_duplicate_props.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { jsxGetAllProps } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -84,4 +81,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/jsx_no_script_url.mjs b/packages/oxlint-plugin-solid/src/rules/jsx_no_script_url.mjs index ee6390f..6707a09 100644 --- a/packages/oxlint-plugin-solid/src/rules/jsx_no_script_url.mjs +++ b/packages/oxlint-plugin-solid/src/rules/jsx_no_script_url.mjs @@ -1,14 +1,12 @@ -import { ASTUtils, ESLintUtils } from "@typescript-eslint/utils"; +import { getStaticValue } from "@eslint-community/eslint-utils"; import { getScope } from "../compat.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getStaticValue } = ASTUtils; const JAVASCRIPT_PROTOCOL_PATTERN = "^[\\\\u0000-\\\\u001F ]*j[\\\\r\\\\n\\\\t]*a[\\\\r\\\\n\\\\t]*v[\\\\r\\\\n\\\\t]*a[\\\\r\\\\n\\\\t]*s[\\\\r\\\\n\\\\t]*c[\\\\r\\\\n\\\\t]*r[\\\\r\\\\n\\\\t]*i[\\\\r\\\\n\\\\t]*p[\\\\r\\\\n\\\\t]*t[\\\\r\\\\n\\\\t]*:"; const JAVASCRIPT_PROTOCOL_REGEX = new RegExp(JAVASCRIPT_PROTOCOL_PATTERN, "i"); -export const jsxNoScriptUrlRule = createRule({ +export const jsxNoScriptUrlRule = { meta: { docs: { description: "Disallow javascript: URLs.", @@ -51,4 +49,4 @@ export const jsxNoScriptUrlRule = createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/jsx_no_undef.mjs b/packages/oxlint-plugin-solid/src/rules/jsx_no_undef.mjs index b631f27..9376869 100644 --- a/packages/oxlint-plugin-solid/src/rules/jsx_no_undef.mjs +++ b/packages/oxlint-plugin-solid/src/rules/jsx_no_undef.mjs @@ -1,5 +1,3 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { getScope, getSourceCode } from "../compat.mjs"; import { isDOMElementName, @@ -7,7 +5,6 @@ import { appendImports, insertImports, } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; // Currently all of the control flow components are from 'solid-js'. // Solid 2.0 dropped `Index` (use ``) and renamed the // async/error boundaries. All of these are exported from `solid-js`. @@ -22,7 +19,7 @@ const AUTO_COMPONENTS = [ "Reveal", ]; const SOURCE_MODULE = "solid-js"; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -225,4 +222,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/jsx_uses_vars.mjs b/packages/oxlint-plugin-solid/src/rules/jsx_uses_vars.mjs index 898cdb7..3409890 100644 --- a/packages/oxlint-plugin-solid/src/rules/jsx_uses_vars.mjs +++ b/packages/oxlint-plugin-solid/src/rules/jsx_uses_vars.mjs @@ -1,10 +1,6 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { markVariableAsUsed } from "../compat.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; - -export const jsxUsesVarsRule = createRule({ +export const jsxUsesVarsRule = { meta: { docs: { description: @@ -52,4 +48,4 @@ export const jsxUsesVarsRule = createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_array_handlers.mjs b/packages/oxlint-plugin-solid/src/rules/no_array_handlers.mjs index ed50ebc..90eab5c 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_array_handlers.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_array_handlers.mjs @@ -1,10 +1,6 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isDOMElementName, trace } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; - -export const noArrayHandlersRule = createRule({ +export const noArrayHandlersRule = { meta: { docs: { description: "Disallow usage of type-unsafe event handlers.", @@ -50,4 +46,4 @@ export const noArrayHandlersRule = createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_destructure.mjs b/packages/oxlint-plugin-solid/src/rules/no_destructure.mjs index a6fa799..db2f4e8 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_destructure.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_destructure.mjs @@ -1,8 +1,6 @@ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getStringIfConstant } from "@eslint-community/eslint-utils"; import { getSourceCode } from "../compat.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getStringIfConstant } = ASTUtils; const getName = (node) => { switch (node.type) { case "Literal": @@ -32,7 +30,7 @@ const getPropertyInfo = (prop) => { return null; } }; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -212,4 +210,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_innerhtml.mjs b/packages/oxlint-plugin-solid/src/rules/no_innerhtml.mjs index a5b810d..518faae 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_innerhtml.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_innerhtml.mjs @@ -1,10 +1,8 @@ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getStringIfConstant } from "@eslint-community/eslint-utils"; import isHtml from "is-html"; import { jsxPropName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getStringIfConstant } = ASTUtils; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -142,4 +140,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs b/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs index 8239af6..348615f 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_owned_primitives_in_ref.mjs @@ -1,9 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isFunctionNode, jsxPropName, trackImports } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; - /** * Primitives that attach to the current owner. * @@ -71,7 +67,7 @@ function walkOwnBody(node, visit) { } } -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -123,4 +119,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_proxy_apis.mjs b/packages/oxlint-plugin-solid/src/rules/no_proxy_apis.mjs index 831c01b..4740fc6 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_proxy_apis.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_proxy_apis.mjs @@ -1,20 +1,16 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isFunctionNode, trackImports, isPropsByName, trace, } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; - // Store APIs that hand back a Proxy. Solid 2.0 exports these from `solid-js`. const PROXY_BACKED_STORE_APIS = new Set([ "createStore", "createProjection", "createOptimisticStore", ]); -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -116,4 +112,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs b/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs index bdfbef3..80360d3 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_react_deps.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isFunctionNode, trace, trackImports } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -62,4 +59,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_react_specific_props.mjs b/packages/oxlint-plugin-solid/src/rules/no_react_specific_props.mjs index 944b367..4be5f8c 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_react_specific_props.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_react_specific_props.mjs @@ -1,14 +1,11 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isDOMElementName, jsxGetProp, jsxHasProp } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const REACT_SPECIFIC_PROPS = [ { from: "className", to: "class" }, { from: "htmlFor", to: "for" }, ]; -export const noReactSpecificPropsRule = createRule({ +export const noReactSpecificPropsRule = { meta: { docs: { description: @@ -68,4 +65,4 @@ export const noReactSpecificPropsRule = createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs b/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs index 30e2df6..322725e 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_setter_in_effect.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isFunctionNode, trackImports } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const SETTER_INDEX = 1; /** `const [value, setValue] = createSignal(...)` -> "setValue". */ @@ -84,7 +81,7 @@ function onlyWritesSignals(phase, setterNames) { ); } -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -168,4 +165,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_unknown_namespaces.mjs b/packages/oxlint-plugin-solid/src/rules/no_unknown_namespaces.mjs index 47695d1..93243c6 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_unknown_namespaces.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_unknown_namespaces.mjs @@ -1,7 +1,4 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isDOMElementName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; // Solid 2.0 removed every Solid-specific JSX namespace. What is left are the // XML namespaces the DOM itself defines. const xmlNamespaces = ["xmlns", "xlink"]; @@ -17,7 +14,7 @@ const replacements = { style: "the `style` prop with an object value", use: "a ref callback or directive factory, composing with a ref array", }; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -107,4 +104,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs b/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs index 609361b..007885c 100644 --- a/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs +++ b/packages/oxlint-plugin-solid/src/rules/no_untracked_effect_read.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { isFunctionNode, isPropsByName, trackImports } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const GETTER_INDEX = 0; /** `const [value, setValue] = createSignal(...)` -> "value". */ @@ -78,7 +75,7 @@ function walkPhase(node, visit) { } } -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -175,4 +172,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs b/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs index 320fb93..12e5d88 100644 --- a/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs +++ b/packages/oxlint-plugin-solid/src/rules/prefer_class_object.mjs @@ -1,9 +1,6 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { jsxPropName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const DEFAULT_CLASSNAMES = ["cn", "clsx", "classnames"]; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -98,4 +95,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/prefer_for.mjs b/packages/oxlint-plugin-solid/src/rules/prefer_for.mjs index 9bf9395..63c39d4 100644 --- a/packages/oxlint-plugin-solid/src/rules/prefer_for.mjs +++ b/packages/oxlint-plugin-solid/src/rules/prefer_for.mjs @@ -1,9 +1,7 @@ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getPropertyName } from "@eslint-community/eslint-utils"; import { isFunctionNode, isJSXElementOrFragment } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getPropertyName } = ASTUtils; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -89,4 +87,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/prefer_show.mjs b/packages/oxlint-plugin-solid/src/rules/prefer_show.mjs index 74d4126..51114c7 100644 --- a/packages/oxlint-plugin-solid/src/rules/prefer_show.mjs +++ b/packages/oxlint-plugin-solid/src/rules/prefer_show.mjs @@ -1,10 +1,7 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { getSourceCode } from "../compat.mjs"; import { isJSXElementOrFragment } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; const EXPENSIVE_TYPES = ["JSXElement", "JSXFragment", "Identifier"]; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -89,4 +86,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/reactivity.mjs b/packages/oxlint-plugin-solid/src/rules/reactivity.mjs index bc7ef4e..2cc1ca0 100644 --- a/packages/oxlint-plugin-solid/src/rules/reactivity.mjs +++ b/packages/oxlint-plugin-solid/src/rules/reactivity.mjs @@ -2,7 +2,7 @@ * File overview here, scroll to bottom. * @link https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/reactivity.md */ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getFunctionHeadLocation } from "@eslint-community/eslint-utils"; import { traverse } from "estraverse"; import { findVariable, getSourceCode } from "../compat.mjs"; @@ -19,9 +19,6 @@ import { isJSXElementOrFragment, trace, } from "../utils.mjs"; -const { getFunctionHeadLocation } = ASTUtils; -const createRule = ESLintUtils.RuleCreator.withoutDocs; - // Solid 2.0 removed `createResource`, `createMutable`, `indexArray` and // ``. Their branches below are kept structurally but matched against // `__removed_*` names so they can never fire; delete them once no Solid 1 @@ -174,7 +171,7 @@ const getReturnedVar = (id, context) => { } return null; }; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -223,7 +220,12 @@ export default createRule({ customReactiveFunctions: [], }, ], - create(context, [options]) { + create(context) { + // `RuleCreator` used to merge `defaultOptions` into `context.options` + // and hand the result to a second `create` parameter. Without that + // wrapper the parameter is undefined, so the default is applied here. + const options = context.options[0] ?? { customReactiveFunctions: [] }; + const warnShouldDestructure = (node, nth) => context.report({ node, @@ -1397,4 +1399,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/self_closing_comp.mjs b/packages/oxlint-plugin-solid/src/rules/self_closing_comp.mjs index d69b12f..1236362 100644 --- a/packages/oxlint-plugin-solid/src/rules/self_closing_comp.mjs +++ b/packages/oxlint-plugin-solid/src/rules/self_closing_comp.mjs @@ -1,8 +1,5 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - import { getSourceCode } from "../compat.mjs"; import { isDOMElementName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; function isComponent(node) { return ( (node.name.type === "JSXIdentifier" && @@ -31,7 +28,7 @@ function childrenIsMultilineSpaces(node) { * This rule is adapted from eslint-plugin-react's self-closing-comp rule under the MIT license, * with some enhancements. Thank you for your work! */ -export default createRule({ +export default { meta: { type: "layout", docs: { @@ -150,4 +147,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/src/rules/style_prop.mjs b/packages/oxlint-plugin-solid/src/rules/style_prop.mjs index efb7719..74983f3 100644 --- a/packages/oxlint-plugin-solid/src/rules/style_prop.mjs +++ b/packages/oxlint-plugin-solid/src/rules/style_prop.mjs @@ -1,15 +1,13 @@ -import { ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import { getPropertyName, getStaticValue } from "@eslint-community/eslint-utils"; import kebabCase from "kebab-case"; import { all as allCssProperties } from "known-css-properties"; import parse from "style-to-object"; import { getScope } from "../compat.mjs"; import { jsxPropName } from "../utils.mjs"; -const createRule = ESLintUtils.RuleCreator.withoutDocs; -const { getPropertyName, getStaticValue } = ASTUtils; const lengthPercentageRegex = /\b(?:width|height|margin|padding|border-width|font-size)\b/i; -export default createRule({ +export default { meta: { type: "problem", docs: { @@ -152,4 +150,4 @@ export default createRule({ }, }; }, -}); +}; diff --git a/packages/oxlint-plugin-solid/tests/helpers.mjs b/packages/oxlint-plugin-solid/tests/helpers.mjs index 5d053c6..0b873db 100644 --- a/packages/oxlint-plugin-solid/tests/helpers.mjs +++ b/packages/oxlint-plugin-solid/tests/helpers.mjs @@ -5,8 +5,7 @@ const FIXTURE_FILE_PATH = "src/rule_case.tsx"; const OXLINT_CONFIG_PATH = "./.oxlintrc.jsonc"; const TEMP_DIRECTORY_PATTERN = "/tmp/oxlint-plugin-solid-XXXXXX"; const PACKAGE_RUNTIME_DEPENDENCIES = [ - "@typescript-eslint", - "eslint", + "@eslint-community", "estraverse", "is-html", "kebab-case",