From e1925770e03ffea2f0b36c21c94d60eec4e33bb4 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sat, 4 Feb 2023 13:01:35 -0500 Subject: [PATCH 1/6] Initial WIP work for reactivity plugin api. --- .eslintignore | 3 +- bin/reactivity.cjs | 79 ++++++++++++++ scripts/docs.ts | 16 +-- src/rules/reactivity/analyze.ts | 81 ++++++++++++++ src/rules/reactivity/pluginApi.ts | 93 ++++++++++++++++ src/rules/reactivity/rule.ts | 174 ++++++++++++++++++++++++++++++ src/utils.ts | 7 +- 7 files changed, 444 insertions(+), 9 deletions(-) create mode 100755 bin/reactivity.cjs create mode 100644 src/rules/reactivity/analyze.ts create mode 100644 src/rules/reactivity/pluginApi.ts create mode 100644 src/rules/reactivity/rule.ts diff --git a/.eslintignore b/.eslintignore index c780bdc..b89b74e 100644 --- a/.eslintignore +++ b/.eslintignore @@ -2,4 +2,5 @@ dist ./configs node_modules jest.config.js -jest.setup.js \ No newline at end of file +jest.setup.js +bin diff --git a/bin/reactivity.cjs b/bin/reactivity.cjs new file mode 100755 index 0000000..93e211c --- /dev/null +++ b/bin/reactivity.cjs @@ -0,0 +1,79 @@ +#!/usr/bin/env node + +const fs = require("fs/promises"); +const path = require("path"); +const glob = require("fast-glob"); + +const KEY = "solid/reactivity"; +const WRAPPED_KEY = `"${KEY}"`; + +async function findPlugins() { + const pluginsMap = new Map(); + + const files = await glob("**/package.json", { onlyFiles: true, unique: true, deep: 10 }); + await Promise.all( + files.map(async (file) => { + const contents = await fs.readFile(file, "utf-8"); + + if (contents.includes(WRAPPED_KEY)) { + const parsed = JSON.parse(contents); + if (parsed && Object.prototype.hasOwnProperty.call(parsed, KEY)) { + const pluginPath = parsed[KEY]; + if (file === "package.json") { + pluginsMap.set(".", pluginPath); // associate current CWD with the plugin path + } else { + pluginsMap.set(parsed.name, pluginPath); // associate the package name with the plugin path + } + } + } + }) + ); + + return pluginsMap; +} + +async function reactivityCLI() { + if (process.argv.some((token) => token.match(/\-h|\-v/i))) { + console.log( + `eslint-plugin-solid v${require("../package.json").version} + +This CLI command searches for any plugins for the "solid/reactivity" ESLint rule that your dependencies make available. + +If any are found, an ESLint rule config will be printed out with the necessary options to load and use those "solid/reactivity" plugins. +If you are authoring a framework or template repository for SolidJS, this can help your users get the best possible linter feedback. + +For instructions on authoring a "solid/reactivity" plugin, see TODO PROVIDE URL.` + ); + } else { + console.log( + `Searching for ${WRAPPED_KEY} keys in all package.json files... (this could take a minute)` + ); + const pluginsMap = await findPlugins(); + if (pluginsMap.size > 0) { + // create a resolve function relative from the root of the current working directory + const { resolve } = require("module").createRequire(path.join(process.cwd(), "index.js")); + + const plugins = Array.from(pluginsMap) + .map(([packageName, pluginPath]) => { + const absPluginPath = resolve( + (packageName + "/" + pluginPath).replace(/(\.\/){2,}/g, "./") + ); + return path.relative(process.cwd(), absPluginPath); + }) + .filter((v, i, a) => a.indexOf(v) === i); // remove duplicates + + const config = [1, { plugins }]; + + console.log( + `Found ${plugins.length} "solid/reactivity" plugin${plugins.length !== 1 ? "s" : ""}. Add ${ + plugins.length !== 1 ? "them" : "it" + } to your ESLint config by adding the following to the "rules" section:\n` + ); + console.log(`"solid/reactivity": ${JSON.stringify(config, null, 2)}`); + } else { + console.log(`No "solid/reactivity" plugins found.`); + } + } +} + +reactivityCLI(); diff --git a/scripts/docs.ts b/scripts/docs.ts index 487a5bf..defa646 100644 --- a/scripts/docs.ts +++ b/scripts/docs.ts @@ -181,13 +181,15 @@ async function run() { const docRoot = path.resolve(__dirname, "..", "docs"); const docFiles = (await fs.readdir(docRoot)).filter((p) => p.endsWith(".md")); for (const docFile of docFiles) { - markdownMagic(path.join(docRoot, docFile), { - transforms: { - HEADER: () => buildHeader(docFile), - OPTIONS: () => buildOptions(docFile), - CASES: (content: string) => buildCases(content, docFile), - }, - }); + if (docFile !== "reactivity-customization.md") { + markdownMagic(path.join(docRoot, docFile), { + transforms: { + HEADER: () => buildHeader(docFile), + OPTIONS: () => buildOptions(docFile), + CASES: (content: string) => buildCases(content, docFile), + }, + }); + } } } diff --git a/src/rules/reactivity/analyze.ts b/src/rules/reactivity/analyze.ts new file mode 100644 index 0000000..4346cb3 --- /dev/null +++ b/src/rules/reactivity/analyze.ts @@ -0,0 +1,81 @@ +import { TSESLint, TSESTree as T } from "@typescript-eslint/utils"; +import { ProgramOrFunctionNode, FunctionNode } from "../../utils"; +import type { ReactivityPluginApi } from "./pluginApi"; + +interface TrackedScope { + /** + * The root node, usually a function or JSX expression container, to allow + * reactive variables under. + */ + node: T.Node; + /** + * The reactive variable should be one of these types: + * - "tracked-scope": synchronous function or signal variable, or JSX container/spread + * - "called-function": synchronous or asynchronous function like a timer or + * event handler that isn't really a tracked scope but allows reactivity + */ + expect: "tracked-scope" | "called-function"; +} + +export interface VirtualReference { + reference: TSESLint.Scope.Reference | T.Node; // store references directly instead of pushing variables? + declarationScope: ReactivityScope; +} + +function isRangeWithin(inner: T.Range, outer: T.Range): boolean { + return inner[0] >= outer[0] && inner[1] <= outer[1]; +} + +export class ReactivityScope { + constructor( + public node: ProgramOrFunctionNode, + public parentScope: ReactivityScope | null + ) {} + + childScopes: Array = []; + trackedScopes: Array = []; + errorContexts: Array = []; + references: Array = []; + hasJSX = false; + + deepestScopeContaining(node: T.Node | T.Range): ReactivityScope | null { + const range = Array.isArray(node) ? node : node.range; + if (isRangeWithin(range, this.node.range)) { + const matchedChildRange = this.childScopes.find((scope) => + scope.deepestScopeContaining(range) + ); + return matchedChildRange ?? this; + } + return null; + } + + isDeepestScopeFor(node: T.Node | T.Range) { + return this.deepestScopeContaining(Array.isArray(node) ? node : node.range) === this; + } + + *walk(): Iterable { + yield this; + for (const scope of this.childScopes) { + yield* scope.walk(); + } + } + + private *iterateUpwards(prop: (scope: ReactivityScope) => Iterable): Iterable { + yield* prop(this); + if (this.parentScope) { + yield* this.parentScope.iterateUpwards(prop); + } + } +} + +// class ReactivityScopeTree { +// constructor(public root: ReactivityScope) {} +// syncCallbacks = new Set(); + +// /** Finds the deepest ReactivityScope containing a given range, or null if the range extends beyond the code length. */ +// deepestScopeContaining = this.root.deepestScopeContaining.bind(this.root); +// // (node: T.Node | T.Range): ReactivityScope | null { +// // return this.root.deepestScopeContaining(range); +// // } + +// } diff --git a/src/rules/reactivity/pluginApi.ts b/src/rules/reactivity/pluginApi.ts new file mode 100644 index 0000000..fc8fbfb --- /dev/null +++ b/src/rules/reactivity/pluginApi.ts @@ -0,0 +1,93 @@ +import type { TSESTree as T, TSESLint, TSESTree } from "@typescript-eslint/utils"; +import { FunctionNode, ProgramOrFunctionNode } from "../../utils"; + +type PathSegment = `[${number}]`; +export type ExprPath = PathSegment; // PathSegment | `${PathSegment}${Path}`; + +export interface ReactivityPluginApi { + /** + * Mark a node as a function in which reactive variables may be polled (not tracked). Can be + * async. + */ + calledFunction(node: T.Node): void; + /** + * Mark a node as a tracked scope, like `createEffect`'s callback or a JSXExpressionContainer. + */ + trackedScope(node: T.Node): void; + /** + * Mark a node as a function that will be synchronously called in the current scope, like + * Array#map callbacks. + */ + syncCallback(node: FunctionNode): void; + + /** + * Alter the error message within a scope to provide additional details. + */ + provideErrorContext(node: T.Node, errorMessage: string): void; + + /** + * Mark that a node evaluates to a signal (or memo, etc.). + */ + signal(node: T.Node, path?: ExprPath): void; + + /** + * Mark that a node evaluates to a store or props. + */ + store(node: T.Node, path?: ExprPath, options?: { mutable?: boolean }): void; + + /** + * Mark that a node is reactive in some way--either a signal-like or a store-like. + */ + reactive(node: T.Node, path?: ExprPath): void; + + /** + * Convenience method for checking if a node is a call expression. If `primitive` is provided, + * checks that the callee is a Solid import with that name (or one of that name), handling aliases. + */ + isCall(node: T.Node, primitive?: string | Array): node is T.CallExpression; +} + +export interface ReactivityPlugin { + package: string; + create: (api: ReactivityPluginApi) => TSESLint.RuleListener; +} + +// Defeats type widening +export function plugin(p: ReactivityPlugin): ReactivityPlugin { + return p; +} + +// =========================== + +class Reactivity implements ReactivityPluginApi { + trackedScope(node: T.Node): void { + throw new Error("Method not implemented."); + } + calledFunction(node: T.Node): void { + throw new Error("Method not implemented."); + } + trackedFunction(node: T.Node): void { + throw new Error("Method not implemented."); + } + trackedExpression(node: T.Node): void { + throw new Error("Method not implemented."); + } + syncCallback(node: T.Node): void { + throw new Error("Method not implemented."); + } + provideErrorContext(node: T.Node, errorMessage: string): void { + throw new Error("Method not implemented."); + } + signal(node: T.Node, path?: `[${number}]` | undefined): void { + throw new Error("Method not implemented."); + } + store(node: T.Node, path?: `[${number}]` | undefined): void { + throw new Error("Method not implemented."); + } + reactive(node: T.Node, path?: `[${number}]` | undefined): void { + throw new Error("Method not implemented."); + } + isCall(node: T.Node, primitive?: string | string[] | undefined): node is T.CallExpression { + throw new Error("Method not implemented."); + } +} diff --git a/src/rules/reactivity/rule.ts b/src/rules/reactivity/rule.ts new file mode 100644 index 0000000..0421481 --- /dev/null +++ b/src/rules/reactivity/rule.ts @@ -0,0 +1,174 @@ +import { TSESLint, TSESTree as T } from "@typescript-eslint/utils"; +import { ProgramOrFunctionNode, FunctionNode, trackImports, isFunctionNode } from "../../utils"; +import { ReactivityScope, VirtualReference } from "./analyze"; +import type { ExprPath, ReactivityPlugin, ReactivityPluginApi } from "./pluginApi"; + +type MessageIds = + | "noWrite" + | "untrackedReactive" + | "expectedFunctionGotExpression" + | "badSignal" + | "badUnnamedDerivedSignal" + | "shouldDestructure" + | "shouldAssign" + | "noAsyncTrackedScope"; + +const rule: TSESLint.RuleModule = { + meta: { + type: "problem", + docs: { + recommended: "warn", + description: + "Enforce that reactive expressions (props, signals, memos, etc.) are only used in tracked scopes; otherwise, they won't update the view as expected.", + url: "https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/reactivity.md", + }, + schema: [], + messages: { + 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.", + 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: + "The reactive variable '{{name}}' should be called as a function when used in {{where}}.", + badUnnamedDerivedSignal: + "This function should be passed to a tracked scope (like createEffect) or an event handler because it contains reactivity.", + shouldDestructure: + "For proper analysis, array destructuring should be used to capture the {{nth}}result of this function call.", + shouldAssign: + "For proper analysis, a variable should be used to capture the result of this function call.", + noAsyncTrackedScope: + "This tracked scope should not be async. Solid's reactivity only tracks synchronously.", + }, + }, + create(context) { + const sourceCode = context.getSourceCode(); + + const { handleImportDeclaration, matchImport, matchLocalToModule } = trackImports(/^/); + + const root = new ReactivityScope(sourceCode.ast, null); + const syncCallbacks = new Set(); + const undistributedReferences: Array = []; + + let currentScope = root; + + function onFunctionEnter(node: FunctionNode) { + // if (syncCallbacks.has(node)) { + // // Ignore sync callbacks like Array#forEach and certain Solid primitives + // return; + // } + const childScope = new ReactivityScope(node, currentScope); + currentScope.childScopes.push(childScope); + currentScope = childScope; + } + + function onJSX() { + currentScope.hasJSX = true; + } + + function onFunctionExit() { + currentScope = currentScope.parentScope!; + } + + function stripSyncCallbacks(root: ReactivityScope) { + for (const scope of root.walk()) { + if (scope.childScopes.length > 0) { + // Though we're about to walk these childNodes, hit them now to avoid iterator + // invalidation. If a child scope is a sync callback, merge its data into this scope and + // remove it from the tree. + const addedChildScopes: Array = []; + scope.childScopes = scope.childScopes.filter((childScope) => { + if (isFunctionNode(childScope.node) && syncCallbacks.has(childScope.node)) { + addedChildScopes.push(...childScope.childScopes); + scope.trackedScopes.push(...childScope.trackedScopes); + scope.references.push(...childScope.references); + scope.hasJSX = scope.hasJSX || childScope.hasJSX; + return false; + } + return true; + }); + scope.childScopes.push(...addedChildScopes); + } + } + } + + function getReferences(node: T.Expression, path: ExprPath) { + if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { + + } + } + + function distributeReferences(root: ReactivityScope, references: Array) { + references.forEach((ref) => { + const range = + "range" in ref.reference ? ref.reference.range : ref.reference.identifier.range; + const scope = root.deepestScopeContaining(range)!; + scope.references.push(ref); + }); + } + + + const pluginApi: ReactivityPluginApi = { + calledFunction(node) { + currentScope.trackedScopes.push({ node, expect: "called-function" }); + }, + trackedScope(node) { + currentScope.trackedScopes.push({ node, expect: "tracked-scope" }); + }, + syncCallback(node) { + syncCallbacks.add(node); + }, + provideErrorContext(node) { + currentScope.errorContexts.push(node); + }, + signal(node, path) { + + const references = []; // TODO generate virtual signal references + undistributedReferences.push(...references); + }, + store(node, path, options) { + const references = []; // TODO generate virtual store references + undistributedReferences.push(...references); + }, + reactive(node, path) { + const references = []; // TODO generate virtual reactive references + undistributedReferences.push(...references); + }, + isCall(node, primitive): node is T.CallExpression { + return ( + node.type === "CallExpression" && + (!primitive || (node.callee.type === "Identifier" && node.callee.name === primitive)) + ); + }, + }; + const visitors: Array> = []; + + return { + /* Function enter/exit */ + FunctionExpression: onFunctionEnter, + ArrowFunctionExpression: onFunctionEnter, + FunctionDeclaration: onFunctionEnter, + "FunctionExpression:exit": onFunctionExit, + "ArrowFunctionExpression:exit": onFunctionExit, + "FunctionDeclaration:exit": onFunctionExit, + + /* Detect JSX for adding props */ + JSXElement: onJSX, + JSXFragment: onJSX, + + ImportDeclaration: handleImportDeclaration, + + "*"(node: T.Node) { + // eslint-disable-next-line + // @ts-ignore + visitors.forEach((visitor) => visitor[node.type]?.(node)); + }, + + "Program:exit"() { + stripSyncCallbacks(root); + distributeReferences(root, undistributedReferences); + analyze(root); + }, + }; + }, +}; diff --git a/src/utils.ts b/src/utils.ts index 7aa5671..ed18d94 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -148,11 +148,14 @@ export const getCommentAfter = ( export const trackImports = (fromModule = /^solid-js(?:\/?|\b)/) => { const importMap = new Map(); + const moduleMap = new Map(); + const handleImportDeclaration = (node: T.ImportDeclaration) => { if (fromModule.test(node.source.value)) { for (const specifier of node.specifiers) { if (specifier.type === "ImportSpecifier") { importMap.set(specifier.imported.name, specifier.local.name); + moduleMap.set(specifier.local.name, node.source.value); } } } @@ -161,7 +164,9 @@ export const trackImports = (fromModule = /^solid-js(?:\/?|\b)/) => { const importArr = Array.isArray(imports) ? imports : [imports]; return importArr.find((i) => importMap.get(i) === str); }; - return { matchImport, handleImportDeclaration }; + const matchLocalToModule = (local: string): string | undefined => moduleMap.get(local); + + return { matchImport, handleImportDeclaration, matchLocalToModule }; }; export function appendImports( From e9e4768236cb5996ce62c7146cbc6649d2e1c23e Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sat, 18 Mar 2023 10:55:02 -0400 Subject: [PATCH 2/6] [skip ci] WIP building out getReferences for approximate feature parity --- package.json | 3 +- pnpm-lock.yaml | 7 ++ src/rules/reactivity.ts | 2 +- src/rules/reactivity/analyze.ts | 24 +++- src/rules/reactivity/pluginApi.ts | 75 ++++++------ src/rules/reactivity/rule.ts | 109 ++++++++++++++---- src/utils.ts | 12 +- .../fixture/valid/async/suspense/greeting.tsx | 1 + 8 files changed, 158 insertions(+), 75 deletions(-) diff --git a/package.json b/package.json index c6a8140..860a986 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,8 @@ "is-html": "^2.0.0", "kebab-case": "^1.0.2", "known-css-properties": "^0.24.0", - "style-to-object": "^0.3.0" + "style-to-object": "^0.3.0", + "tiny-invariant": "^1.3.1" }, "devDependencies": { "@babel/core": "^7.21.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0e07dec..9b712ff 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,6 +23,9 @@ dependencies: style-to-object: specifier: ^0.3.0 version: 0.3.0 + tiny-invariant: + specifier: ^1.3.1 + version: 1.3.1 devDependencies: '@babel/core': @@ -5054,6 +5057,10 @@ packages: resolution: {integrity: sha512-w89qg7PI8wAdvX60bMDP+bFoD5Dvhm9oLheFp5O4a2QF0cSBGsBX4qZmadPMvVqlLJBBci+WqGGOAPvcDeNSVg==} dev: true + /tiny-invariant@1.3.1: + resolution: {integrity: sha512-AD5ih2NlSssTCwsMznbvwMZpJ1cbhkGd2uueNxzv2jDlEeZdU04JQfRnggJQ8DrcVBGjAsCKwFBbDlVNtEMlzw==} + dev: false + /tmp@0.0.33: resolution: {integrity: sha512-jRCJlojKnZ3addtTOjdIqoRuPEKBvNXcGYqzO6zWZX8KfKEpnGY5jfggJQ3EjKuu8D4bJRr0y+cYJFmYbImXGw==} engines: {node: '>=0.6.0'} diff --git a/src/rules/reactivity.ts b/src/rules/reactivity.ts index 624ecd3..5ea73c1 100644 --- a/src/rules/reactivity.ts +++ b/src/rules/reactivity.ts @@ -627,7 +627,7 @@ export default createRule({ }; /* - * Sync array functions (forEach, map, reduce, reduceRight, flatMap), + * Sync array functions (forEach, map, reduce, reduceRight, flatMap), IIFEs, * store update fn params (ex. setState("todos", (t) => [...t.slice(0, i()), * ...t.slice(i() + 1)])), batch, onCleanup, and onError fn params, and * maybe a few others don't actually create a new scope. That is, any diff --git a/src/rules/reactivity/analyze.ts b/src/rules/reactivity/analyze.ts index 4346cb3..bcf1e31 100644 --- a/src/rules/reactivity/analyze.ts +++ b/src/rules/reactivity/analyze.ts @@ -18,8 +18,28 @@ interface TrackedScope { } export interface VirtualReference { - reference: TSESLint.Scope.Reference | T.Node; // store references directly instead of pushing variables? - declarationScope: ReactivityScope; + /** + * The node, commonly an identifier, where reactivity is referenced. + */ + node: T.Node; + /** + * The scope where the variable `node` is referencing is declared, or `null` if no variable. + * Since reactivity is, by definition, set up once and accessed multiple times, the final + * call or property access in a reactive expression's `path` is the access that's reactive. + * Previous `path` segments just describe the API. It's safe to assume that the second-to-last + * `path` segment defines the declaration scope unless the reactive expression is a derived signal. + * + * @example + * function Component() { + * const [signal1] = createSignal(42); + * const signal2 = createSignal(42)[0]; + * const signal3 = createSignal(42)[0]; + * const d = () => { + * console.log(signal()); // declarationScope === Component + * } + * console.log(createSignal(42)[0]()); // declarationScope === + */ + declarationScope: ReactivityScope | null; } function isRangeWithin(inner: T.Range, outer: T.Range): boolean { diff --git a/src/rules/reactivity/pluginApi.ts b/src/rules/reactivity/pluginApi.ts index fc8fbfb..14a645e 100644 --- a/src/rules/reactivity/pluginApi.ts +++ b/src/rules/reactivity/pluginApi.ts @@ -1,8 +1,7 @@ import type { TSESTree as T, TSESLint, TSESTree } from "@typescript-eslint/utils"; import { FunctionNode, ProgramOrFunctionNode } from "../../utils"; -type PathSegment = `[${number}]`; -export type ExprPath = PathSegment; // PathSegment | `${PathSegment}${Path}`; + export interface ReactivityPluginApi { /** @@ -26,25 +25,50 @@ export interface ReactivityPluginApi { provideErrorContext(node: T.Node, errorMessage: string): void; /** - * Mark that a node evaluates to a signal (or memo, etc.). + * Mark that a node evaluates to a signal (or memo, etc.). Short for `.reactive(node, (path ?? '') + '()')` */ - signal(node: T.Node, path?: ExprPath): void; + signal(node: T.Node, path?: string): void; /** - * Mark that a node evaluates to a store or props. + * Mark that a node evaluates to a store or props. Short for `.reactive(node, (path ?? '') + '.**')` */ - store(node: T.Node, path?: ExprPath, options?: { mutable?: boolean }): void; + store(node: T.Node, path?: string, options?: { mutable?: boolean }): void; /** - * Mark that a node is reactive in some way--either a signal-like or a store-like. + * Mark that a node is reactive in some way--either a signal-like or a store-like, or something + * more specialized. + * @param path a string with a special pattern, akin to a "type" for reactivity. Composed of the + * following segments: + * - `()`: when called + * - `[0]`: when the first element is accessed + * - `[]`: when any element is accessed + * - `.foo`: when the 'foo' property is accessed + * - `.*`: when any direct property is accessed + * - `.**`: when any direct or nested property is accessed + * @example + * if (isCall(node, "createSignal")) { + * reactive(node, '[0]()'); + * } else if (isCall(node, "createMemo")) { + * reactive(node, '()'); + * } else if (isCall(node, "splitProps")) { + * reactive(node, '[].**'); + * } else if (isCall(node, "createResource")) { + * reactive(node, '()'); + * reactive(node, '.loading'); + * reactive(node, '.error'); + * } */ - reactive(node: T.Node, path?: ExprPath): void; + reactive(node: T.Node, path: string): void; /** * Convenience method for checking if a node is a call expression. If `primitive` is provided, * checks that the callee is a Solid import with that name (or one of that name), handling aliases. */ - isCall(node: T.Node, primitive?: string | Array): node is T.CallExpression; + isCall( + node: T.Node, + primitive?: string | Array | RegExp, + module?: string | RegExp + ): node is T.CallExpression; } export interface ReactivityPlugin { @@ -58,36 +82,3 @@ export function plugin(p: ReactivityPlugin): ReactivityPlugin { } // =========================== - -class Reactivity implements ReactivityPluginApi { - trackedScope(node: T.Node): void { - throw new Error("Method not implemented."); - } - calledFunction(node: T.Node): void { - throw new Error("Method not implemented."); - } - trackedFunction(node: T.Node): void { - throw new Error("Method not implemented."); - } - trackedExpression(node: T.Node): void { - throw new Error("Method not implemented."); - } - syncCallback(node: T.Node): void { - throw new Error("Method not implemented."); - } - provideErrorContext(node: T.Node, errorMessage: string): void { - throw new Error("Method not implemented."); - } - signal(node: T.Node, path?: `[${number}]` | undefined): void { - throw new Error("Method not implemented."); - } - store(node: T.Node, path?: `[${number}]` | undefined): void { - throw new Error("Method not implemented."); - } - reactive(node: T.Node, path?: `[${number}]` | undefined): void { - throw new Error("Method not implemented."); - } - isCall(node: T.Node, primitive?: string | string[] | undefined): node is T.CallExpression { - throw new Error("Method not implemented."); - } -} diff --git a/src/rules/reactivity/rule.ts b/src/rules/reactivity/rule.ts index 0421481..1fda916 100644 --- a/src/rules/reactivity/rule.ts +++ b/src/rules/reactivity/rule.ts @@ -1,7 +1,28 @@ -import { TSESLint, TSESTree as T } from "@typescript-eslint/utils"; -import { ProgramOrFunctionNode, FunctionNode, trackImports, isFunctionNode } from "../../utils"; +import { TSESLint, TSESTree as T, ASTUtils } from "@typescript-eslint/utils"; +import invariant from 'tiny-invariant' +import { + ProgramOrFunctionNode, + FunctionNode, + trackImports, + isFunctionNode, + ignoreTransparentWrappers, +} from "../../utils"; import { ReactivityScope, VirtualReference } from "./analyze"; -import type { ExprPath, ReactivityPlugin, ReactivityPluginApi } from "./pluginApi"; +import type { ReactivityPlugin, ReactivityPluginApi } from "./pluginApi"; + +const { findVariable } = ASTUtils; + +function parsePath(path: string): Array | null { + if (path) { + const regex = /\(\)|\[\d*\]|\.(?:\w+|**?)/g; + const matches = path.match(regex); + // ensure the whole string is matched + if (matches && matches.reduce((acc, match) => acc + match.length, 0) === path.length) { + return matches; + } + } + return null; +} type MessageIds = | "noWrite" @@ -11,7 +32,8 @@ type MessageIds = | "badUnnamedDerivedSignal" | "shouldDestructure" | "shouldAssign" - | "noAsyncTrackedScope"; + | "noAsyncTrackedScope" + | "jsxReactiveVariable"; const rule: TSESLint.RuleModule = { meta: { @@ -39,12 +61,13 @@ const rule: TSESLint.RuleModule = { "For proper analysis, a variable should be used to capture the result of this function call.", noAsyncTrackedScope: "This tracked scope should not be async. Solid's reactivity only tracks synchronously.", + jsxReactiveVariable: "This variable should not be used as a JSX element.", }, }, create(context) { const sourceCode = context.getSourceCode(); - const { handleImportDeclaration, matchImport, matchLocalToModule } = trackImports(/^/); + const { handleImportDeclaration, matchImport, matchLocalToModule } = trackImports(); const root = new ReactivityScope(sourceCode.ast, null); const syncCallbacks = new Set(); @@ -92,22 +115,64 @@ const rule: TSESLint.RuleModule = { } } - function getReferences(node: T.Expression, path: ExprPath) { - if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { - + function VirtualReference(node: T.Node): VirtualReference { + return { node, declarationScope: } + } + + /** + * Given what's usually a CallExpression and a description of how the expression must be used + * in order to be accessed reactively, return a list of virtual references for each place where + * a reactive expression is accessed. + * `path` is a string formatted according to `pluginApi`. + */ + function* getReferences(node: T.Expression, path: string, allowMutable = false): Generator { + node = ignoreTransparentWrappers(node, "up"); + if (!path) { + yield VirtualReference(node); + } else if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { + const { id } = node.parent; + if (id.type === "Identifier") { + const variable = findVariable(context.getScope(), id); + if (variable) { + for (const reference of variable.references) { + if (reference.init) { + // ignore + } else if (reference.identifier.type === "JSXIdentifier") { + context.report({ node: reference.identifier, messageId: "jsxReactiveVariable" }); + } else if (reference.isWrite()) { + if (!allowMutable) { + context.report({ node: reference.identifier, messageId: "noWrite" }); + } + } else { + yield* getReferences(reference.identifier, path); + } + } + } + } else if (id.type === "ArrayPattern") { + const parsedPath = parsePath(path) + if (parsedPath) { + const newPath = path.substring(match[0].length); + const index = match[1] + if (index === '*') { + + } else { + + } + } + + } } } function distributeReferences(root: ReactivityScope, references: Array) { references.forEach((ref) => { - const range = - "range" in ref.reference ? ref.reference.range : ref.reference.identifier.range; - const scope = root.deepestScopeContaining(range)!; + const range = ref.node.range; + const scope = root.deepestScopeContaining(range); + invariant(scope != null) scope.references.push(ref); }); } - const pluginApi: ReactivityPluginApi = { calledFunction(node) { currentScope.trackedScopes.push({ node, expect: "called-function" }); @@ -121,19 +186,21 @@ const rule: TSESLint.RuleModule = { provideErrorContext(node) { currentScope.errorContexts.push(node); }, - signal(node, path) { - - const references = []; // TODO generate virtual signal references - undistributedReferences.push(...references); - }, - store(node, path, options) { - const references = []; // TODO generate virtual store references - undistributedReferences.push(...references); - }, + // signal(node, path) { + // const references = []; // TODO generate virtual signal references + // undistributedReferences.push(...references); + // }, + // store(node, path, options) { + // const references = []; // TODO generate virtual store references + // undistributedReferences.push(...references); + // }, reactive(node, path) { const references = []; // TODO generate virtual reactive references undistributedReferences.push(...references); }, + getReferences(node, path) { + return Array.from(getReferences(node, path)); + }, isCall(node, primitive): node is T.CallExpression { return ( node.type === "CallExpression" && diff --git a/src/utils.ts b/src/utils.ts index ed18d94..c7fa5f5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -72,15 +72,11 @@ export function trace(node: T.Node, initialScope: TSESLint.Scope.Scope): T.Node } /** Get the relevant node when wrapped by a node that doesn't change the behavior */ -export function ignoreTransparentWrappers(node: T.Node, up = false): T.Node { - if ( - node.type === "TSAsExpression" || - node.type === "TSNonNullExpression" || - node.type === "TSSatisfiesExpression" - ) { - const next = up ? node.parent : node.expression; +export function ignoreTransparentWrappers(node: T.Expression, dir = 'down'): T.Expression { + if (node.type === "TSAsExpression" || node.type === "TSNonNullExpression") { + const next = dir === 'up' ? node.parent as T.Expression : node.expression; if (next) { - return ignoreTransparentWrappers(next, up); + return ignoreTransparentWrappers(next, dir); } } return node; diff --git a/test/fixture/valid/async/suspense/greeting.tsx b/test/fixture/valid/async/suspense/greeting.tsx index e69de29..8cec2e9 100644 --- a/test/fixture/valid/async/suspense/greeting.tsx +++ b/test/fixture/valid/async/suspense/greeting.tsx @@ -0,0 +1 @@ +export {}; \ No newline at end of file From b4481ee584e18602121d0f0677dd1475c1fa0bf3 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sat, 18 Mar 2023 12:48:47 -0400 Subject: [PATCH 3/6] Add mutable option to reactive() --- src/rules/reactivity/pluginApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/reactivity/pluginApi.ts b/src/rules/reactivity/pluginApi.ts index 14a645e..a6b6410 100644 --- a/src/rules/reactivity/pluginApi.ts +++ b/src/rules/reactivity/pluginApi.ts @@ -58,7 +58,7 @@ export interface ReactivityPluginApi { * reactive(node, '.error'); * } */ - reactive(node: T.Node, path: string): void; + reactive(node: T.Node, path: string, options?: { mutable?: boolean }): void; /** * Convenience method for checking if a node is a call expression. If `primitive` is provided, From bb1b556e60dc85e2efab3d407a1d3938e67f7546 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Wed, 12 Apr 2023 22:07:35 -0400 Subject: [PATCH 4/6] WIP getReferences --- src/rules/reactivity.ts | 5 +- src/rules/reactivity/pluginApi.ts | 5 +- src/rules/reactivity/rule.ts | 78 ++++++++++++++++++------------- src/utils.ts | 40 ++++++++++------ test/rules/reactivity/api.test.ts | 0 5 files changed, 77 insertions(+), 51 deletions(-) create mode 100644 test/rules/reactivity/api.test.ts diff --git a/src/rules/reactivity.ts b/src/rules/reactivity.ts index 5ea73c1..a33cc38 100644 --- a/src/rules/reactivity.ts +++ b/src/rules/reactivity.ts @@ -295,7 +295,7 @@ export default createRule({ const { currentScope, parentScope } = scopeStack; /** Tracks imports from 'solid-js', handling aliases. */ - const { matchImport, handleImportDeclaration } = trackImports(); + const { matchImport } = trackImports(sourceCode.ast); /** Workaround for #61 */ const markPropsOnCondition = (node: FunctionNode, cb: (props: T.Identifier) => boolean) => { @@ -1144,7 +1144,6 @@ export default createRule({ }; return { - ImportDeclaration: handleImportDeclaration, JSXExpressionContainer(node: T.JSXExpressionContainer) { checkForTrackedScopes(node); }, @@ -1156,7 +1155,7 @@ export default createRule({ checkForSyncCallbacks(node); // ensure calls to reactive primitives use the results. - const parent = node.parent && ignoreTransparentWrappers(node.parent, true); + const parent = node.parent && ignoreTransparentWrappers(node.parent, "up"); if (parent?.type !== "AssignmentExpression" && parent?.type !== "VariableDeclarator") { checkForReactiveAssignment(null, node); } diff --git a/src/rules/reactivity/pluginApi.ts b/src/rules/reactivity/pluginApi.ts index a6b6410..0ec58b8 100644 --- a/src/rules/reactivity/pluginApi.ts +++ b/src/rules/reactivity/pluginApi.ts @@ -45,9 +45,11 @@ export interface ReactivityPluginApi { * - `.foo`: when the 'foo' property is accessed * - `.*`: when any direct property is accessed * - `.**`: when any direct or nested property is accessed + * - `=`: defines the declaration scope, no other effect; implicitly current scope if not given, + * only one `=` is allowed * @example * if (isCall(node, "createSignal")) { - * reactive(node, '[0]()'); + * reactive(node, '[0]=()'); * } else if (isCall(node, "createMemo")) { * reactive(node, '()'); * } else if (isCall(node, "splitProps")) { @@ -73,6 +75,7 @@ export interface ReactivityPluginApi { export interface ReactivityPlugin { package: string; + version: string; create: (api: ReactivityPluginApi) => TSESLint.RuleListener; } diff --git a/src/rules/reactivity/rule.ts b/src/rules/reactivity/rule.ts index 1fda916..9941dc7 100644 --- a/src/rules/reactivity/rule.ts +++ b/src/rules/reactivity/rule.ts @@ -1,5 +1,5 @@ -import { TSESLint, TSESTree as T, ASTUtils } from "@typescript-eslint/utils"; -import invariant from 'tiny-invariant' +import { TSESLint, TSESTree as T, ESLintUtils, ASTUtils } from "@typescript-eslint/utils"; +import invariant from "tiny-invariant"; import { ProgramOrFunctionNode, FunctionNode, @@ -10,6 +10,7 @@ import { import { ReactivityScope, VirtualReference } from "./analyze"; import type { ReactivityPlugin, ReactivityPluginApi } from "./pluginApi"; +const createRule = ESLintUtils.RuleCreator.withoutDocs; const { findVariable } = ASTUtils; function parsePath(path: string): Array | null { @@ -24,18 +25,7 @@ function parsePath(path: string): Array | null { return null; } -type MessageIds = - | "noWrite" - | "untrackedReactive" - | "expectedFunctionGotExpression" - | "badSignal" - | "badUnnamedDerivedSignal" - | "shouldDestructure" - | "shouldAssign" - | "noAsyncTrackedScope" - | "jsxReactiveVariable"; - -const rule: TSESLint.RuleModule = { +export default createRule({ meta: { type: "problem", docs: { @@ -64,10 +54,11 @@ const rule: TSESLint.RuleModule = { jsxReactiveVariable: "This variable should not be used as a JSX element.", }, }, + defaultOptions: [], create(context) { const sourceCode = context.getSourceCode(); - const { handleImportDeclaration, matchImport, matchLocalToModule } = trackImports(); + const { matchImport, matchLocalToModule } = trackImports(sourceCode.ast); const root = new ReactivityScope(sourceCode.ast, null); const syncCallbacks = new Set(); @@ -116,20 +107,40 @@ const rule: TSESLint.RuleModule = { } function VirtualReference(node: T.Node): VirtualReference { - return { node, declarationScope: } + return { node, declarationScope: null }; } /** * Given what's usually a CallExpression and a description of how the expression must be used * in order to be accessed reactively, return a list of virtual references for each place where * a reactive expression is accessed. - * `path` is a string formatted according to `pluginApi`. + * `path` is a array of segments parsed by `parsePath` according to `pluginApi`. */ - function* getReferences(node: T.Expression, path: string, allowMutable = false): Generator { + function getReferences( + node: T.Node, + path: string | null, + allowMutable = false + ): Array { node = ignoreTransparentWrappers(node, "up"); - if (!path) { + const parsedPathOuter = path != null ? parsePath(path) : null; + const eqCount = parsedPathOuter?.reduce((c, segment) => c + +(segment === '='), 0) ?? 0; + if (eqCount > 1) { + throw new Error(`'${path}' must have 0 or 1 '=' characters, has ${eqCount}`) + } + const hasEq = eqCount === 1; + + let declarationScope = hasEq ? null : context.getScope(); + + function* recursiveGenerator(node: T.Node, parsedPath: Array | null) { + + + if (!parsedPath) { yield VirtualReference(node); } else if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { + yield getReferences(node.parent.id); + } else if (node.type === "Identifier") { + + } const { id } = node.parent; if (id.type === "Identifier") { const variable = findVariable(context.getScope(), id); @@ -144,31 +155,34 @@ const rule: TSESLint.RuleModule = { context.report({ node: reference.identifier, messageId: "noWrite" }); } } else { - yield* getReferences(reference.identifier, path); + yield* getReferences(reference.identifier, parsedPath, allowMutable); } } } } else if (id.type === "ArrayPattern") { - const parsedPath = parsePath(path) - if (parsedPath) { - const newPath = path.substring(match[0].length); - const index = match[1] - if (index === '*') { - - } else { - + if (parsedPath[0] === "[]") { + for (const el of id.elements) { + if (!el) { + // ignore + } else if (el.type === "Identifier") { + yield* getReferences(el, parsedPath.slice(1), allowMutable); + } else if (el.type === "RestElement") { + yield* getReferences(el.argument, parsedPath, allowMutable); + } } + } else { } - } - } + + + return Array.from(recursiveGenerator(node, parsePath(path))); } function distributeReferences(root: ReactivityScope, references: Array) { references.forEach((ref) => { const range = ref.node.range; const scope = root.deepestScopeContaining(range); - invariant(scope != null) + invariant(scope != null); scope.references.push(ref); }); } @@ -238,4 +252,4 @@ const rule: TSESLint.RuleModule = { }, }; }, -}; +}); diff --git a/src/utils.ts b/src/utils.ts index c7fa5f5..e282542 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -72,9 +72,9 @@ export function trace(node: T.Node, initialScope: TSESLint.Scope.Scope): T.Node } /** Get the relevant node when wrapped by a node that doesn't change the behavior */ -export function ignoreTransparentWrappers(node: T.Expression, dir = 'down'): T.Expression { +export function ignoreTransparentWrappers(node: T.Node, dir = "down"): T.Node { if (node.type === "TSAsExpression" || node.type === "TSNonNullExpression") { - const next = dir === 'up' ? node.parent as T.Expression : node.expression; + const next = dir === "up" ? node.parent : node.expression; if (next) { return ignoreTransparentWrappers(next, dir); } @@ -142,27 +142,37 @@ export const getCommentAfter = ( .getCommentsAfter(node) .find((comment) => comment.loc!.start.line === node.loc!.end.line); -export const trackImports = (fromModule = /^solid-js(?:\/?|\b)/) => { - const importMap = new Map(); - const moduleMap = new Map(); +export const trackImports = (program: T.Program) => { + const solidRegex = /^solid-js(?:\/?|\b)/; + const importMap = new Map(); - const handleImportDeclaration = (node: T.ImportDeclaration) => { - if (fromModule.test(node.source.value)) { + for (const node of program.body) { + if (node.type === "ImportDeclaration") { for (const specifier of node.specifiers) { - if (specifier.type === "ImportSpecifier") { - importMap.set(specifier.imported.name, specifier.local.name); - moduleMap.set(specifier.local.name, node.source.value); + if (specifier.type === "ImportSpecifier" && specifier.importKind !== "type") { + importMap.set(specifier.local.name, { + imported: specifier.imported.name, + source: node.source.value, + }); } } } - }; - const matchImport = (imports: string | Array, str: string): string | undefined => { + } + const matchImport = ( + imports: string | Array, + local: string, + module = solidRegex + ): string | undefined => { + const match = importMap.get(local); + if (!match || !module.test(match.source)) { + return; + } const importArr = Array.isArray(imports) ? imports : [imports]; - return importArr.find((i) => importMap.get(i) === str); + return importArr.find((i) => i === match.imported); }; - const matchLocalToModule = (local: string): string | undefined => moduleMap.get(local); + const matchLocalToModule = (local: string): string | undefined => importMap.get(local)?.source; - return { matchImport, handleImportDeclaration, matchLocalToModule }; + return { matchImport, matchLocalToModule }; }; export function appendImports( diff --git a/test/rules/reactivity/api.test.ts b/test/rules/reactivity/api.test.ts new file mode 100644 index 0000000..e69de29 From e37af6c902f20015f853b85fc9864f2cbc910281 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sun, 18 Jun 2023 10:54:19 -0400 Subject: [PATCH 5/6] WIP getReferences --- src/rules/reactivity/rule.ts | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/rules/reactivity/rule.ts b/src/rules/reactivity/rule.ts index 9941dc7..2344738 100644 --- a/src/rules/reactivity/rule.ts +++ b/src/rules/reactivity/rule.ts @@ -123,24 +123,21 @@ export default createRule({ ): Array { node = ignoreTransparentWrappers(node, "up"); const parsedPathOuter = path != null ? parsePath(path) : null; - const eqCount = parsedPathOuter?.reduce((c, segment) => c + +(segment === '='), 0) ?? 0; + const eqCount = parsedPathOuter?.reduce((c, segment) => c + +(segment === "="), 0) ?? 0; if (eqCount > 1) { - throw new Error(`'${path}' must have 0 or 1 '=' characters, has ${eqCount}`) + throw new Error(`'${path}' must have 0 or 1 '=' characters, has ${eqCount}`); } const hasEq = eqCount === 1; - let declarationScope = hasEq ? null : context.getScope(); + const declarationScope = hasEq ? null : context.getScope(); function* recursiveGenerator(node: T.Node, parsedPath: Array | null) { - - - if (!parsedPath) { - yield VirtualReference(node); - } else if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { - yield getReferences(node.parent.id); - } else if (node.type === "Identifier") { - - } + if (!parsedPath) { + yield VirtualReference(node); + } else if (node.parent?.type === "VariableDeclarator" && node.parent.init === node) { + yield getReferences(node.parent.id); + } else if (node.type === "Identifier") { + } const { id } = node.parent; if (id.type === "Identifier") { const variable = findVariable(context.getScope(), id); @@ -173,9 +170,9 @@ export default createRule({ } else { } } - - - return Array.from(recursiveGenerator(node, parsePath(path))); + + return Array.from(recursiveGenerator(node, parsePath(path))); + } } function distributeReferences(root: ReactivityScope, references: Array) { From c985a2845c4ef2a125f46777a730f0b3186c6ef6 Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sat, 30 Dec 2023 10:38:01 -0500 Subject: [PATCH 6/6] WIP getReferences --- src/rules/reactivity/pluginApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/reactivity/pluginApi.ts b/src/rules/reactivity/pluginApi.ts index 0ec58b8..15d1a32 100644 --- a/src/rules/reactivity/pluginApi.ts +++ b/src/rules/reactivity/pluginApi.ts @@ -79,7 +79,7 @@ export interface ReactivityPlugin { create: (api: ReactivityPluginApi) => TSESLint.RuleListener; } -// Defeats type widening +// Defeats type widening, could also use `satisfies ReactivityPlugin` export function plugin(p: ReactivityPlugin): ReactivityPlugin { return p; }