diff --git a/.changeset/warm-eagles-sing.md b/.changeset/warm-eagles-sing.md new file mode 100644 index 000000000..8cc32575d --- /dev/null +++ b/.changeset/warm-eagles-sing.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-svelte": patch +--- + +feat: add `svelte/require-store-reactive-access` rule diff --git a/.stylelintignore b/.stylelintignore index e0a593652..9a04d6db8 100644 --- a/.stylelintignore +++ b/.stylelintignore @@ -13,3 +13,4 @@ LICENSE # should we ignore markdown files? *.md /docs-svelte-kit/ +/coverage diff --git a/README.md b/README.md index 22714ad4c..1415dfc3c 100644 --- a/README.md +++ b/README.md @@ -308,6 +308,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-store-async](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | | | [svelte/no-unknown-style-directive-property](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: | | [svelte/require-store-callbacks-use-set-param](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | | +| [svelte/require-store-reactive-access](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: | | [svelte/valid-compile](https://ota-meshi.github.io/eslint-plugin-svelte/rules/valid-compile/) | disallow warnings when compiling. | :star: | | [svelte/valid-prop-names-in-kit-pages](https://ota-meshi.github.io/eslint-plugin-svelte/rules/valid-prop-names-in-kit-pages/) | disallow props other than data or errors in Svelte Kit page components. | | diff --git a/docs-svelte-kit/shim/path.mjs b/docs-svelte-kit/shim/path.mjs index ff591d4a2..de66d18fb 100644 --- a/docs-svelte-kit/shim/path.mjs +++ b/docs-svelte-kit/shim/path.mjs @@ -21,12 +21,44 @@ function isAbsolute() { } function join(...args) { - return args.join("/") + return args.length ? normalize(args.join("/")) : "." } -const sep = "/" +function normalize(path) { + let result = [] + for (const part of path.replace(/\/+/gu, "/").split("/")) { + if (part === "..") { + if (result[0] && result[0] !== ".." && result[0] !== ".") result.shift() + } else if (part === "." && result.length) { + // noop + } else { + result.unshift(part) + } + } + return result.reverse().join("/") +} -const posix = { dirname, extname, resolve, relative, sep, isAbsolute, join } +const sep = "/" +const posix = { + dirname, + extname, + resolve, + relative, + sep, + isAbsolute, + join, + normalize, +} posix.posix = posix -export { dirname, extname, posix, resolve, relative, sep, isAbsolute, join } +export { + dirname, + extname, + posix, + resolve, + relative, + sep, + isAbsolute, + join, + normalize, +} export default posix diff --git a/docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts b/docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts index c3d891203..dedb52302 100644 --- a/docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts +++ b/docs-svelte-kit/src/lib/eslint/scripts/ts-create-program.mts @@ -1,5 +1,6 @@ import type typescript from "typescript" import type tsvfs from "@typescript/vfs" +import path from "path" type TS = typeof typescript type TSVFS = typeof tsvfs @@ -68,10 +69,53 @@ export async function createVirtualCompilerHost( true, ts, ) + + // Setup svelte type definition modules + for (const [key, get] of Object.entries( + // @ts-expect-error -- ignore + import.meta.glob("../../../../../node_modules/svelte/**/*.d.ts", { + as: "raw", + }), + )) { + const modulePath = key.slice("../../../../..".length) + + fsMap.set( + modulePath, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore + await (get as any)(), + ) + } + const system = tsvfs.createSystem(fsMap) const host = tsvfs.createVirtualCompilerHost(system, compilerOptions, ts) - // eslint-disable-next-line @typescript-eslint/unbound-method -- backup original - const original = { getSourceFile: host.compilerHost.getSourceFile } + const original = { + // eslint-disable-next-line @typescript-eslint/unbound-method -- backup original + getSourceFile: host.compilerHost.getSourceFile, + } + host.compilerHost.resolveModuleNames = function ( + moduleNames, + containingFile, + ) { + return moduleNames.map((m) => { + const targetPaths: string[] = [] + if (m.startsWith(".")) { + targetPaths.push(path.join(path.dirname(containingFile), m)) + } else { + targetPaths.push(`/node_modules/${m}`) + } + for (const modulePath of targetPaths.flatMap((m) => [ + `${m}.d.ts`, + `${m}.ts`, + `${m}/index.d.ts`, + `${m}/index.ts`, + ])) { + if (fsMap.has(modulePath)) { + return { resolvedFileName: modulePath } + } + } + return undefined + }) + } host.compilerHost.getSourceFile = function ( fileName, languageVersionOrOptions, diff --git a/docs/rules.md b/docs/rules.md index bc5e7a401..075eb2911 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -26,6 +26,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | | | [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: | | [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | | +| [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: | | [svelte/valid-compile](./rules/valid-compile.md) | disallow warnings when compiling. | :star: | | [svelte/valid-prop-names-in-kit-pages](./rules/valid-prop-names-in-kit-pages.md) | disallow props other than data or errors in Svelte Kit page components. | | diff --git a/docs/rules/require-store-reactive-access.md b/docs/rules/require-store-reactive-access.md new file mode 100644 index 000000000..21262b360 --- /dev/null +++ b/docs/rules/require-store-reactive-access.md @@ -0,0 +1,97 @@ +--- +pageClass: "rule-details" +sidebarDepth: 0 +title: "svelte/require-store-reactive-access" +description: "disallow to use of the store itself as an operand. Need to use $ prefix or get function." +--- + +# svelte/require-store-reactive-access + +> disallow to use of the store itself as an operand. Need to use $ prefix or get function. + +- :exclamation: **_This rule has not been released yet._** +- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule. + +## :book: Rule Details + +This rule disallow to use of the store itself as an operand. +You should access the store value using the `$` prefix or the `get` function. + + + + + +```svelte + + + +

{$storeValue}

+

{get(storeValue)}

+ +

+

+ + + + + + + +

{storeValue}

+ +

+

+ + + + + +``` + + + +This rule checks the usage of store variables only if the store can be determined within a single file. +However, when using `@typescript-eslint/parser` and full type information, this rule uses the type information to determine if the expression is a store. + + + +```ts +// fileName: my-stores.ts +import { writable } from "svelte/store" +export const storeValue = writable("hello") +``` + + + +```svelte + + + +

{$storeValue}

+ + +

{storeValue}

+``` + +## :wrench: Options + +Nothing. + +## :mag: Implementation + +- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/require-store-reactive-access.ts) +- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/require-store-reactive-access.ts) diff --git a/package.json b/package.json index 5c3b6b646..331c858c7 100644 --- a/package.json +++ b/package.json @@ -163,6 +163,7 @@ "stylus": "^0.59.0", "svelte": "^3.46.1", "svelte-adapter-ghpages": "0.0.2", + "svelte-i18n": "^3.4.0", "type-coverage": "^2.22.0", "typescript": "^4.5.2", "vite": "^3.1.0-0", @@ -174,7 +175,7 @@ "access": "public" }, "typeCoverage": { - "atLeast": 98.69, + "atLeast": 98.72, "cache": true, "detail": true, "ignoreAsAssertion": true, diff --git a/src/rules/indent-helpers/es.ts b/src/rules/indent-helpers/es.ts index 5f702f4df..78d5b818a 100644 --- a/src/rules/indent-helpers/es.ts +++ b/src/rules/indent-helpers/es.ts @@ -17,6 +17,7 @@ import { isSemicolonToken, } from "eslint-utils" import type { ESNodeListener } from "../../types-for-node" +import { getParent } from "../../utils/ast-utils" type NodeListener = ESNodeListener @@ -1097,12 +1098,6 @@ export function defineVisitor(context: IndentContext): NodeListener { } } -/** Get the parent node from the given node */ -function getParent(node: ESTree.Node): ESTree.Node | null { - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore - return (node as any).parent || null -} - /** * Checks whether given text is known button type */ diff --git a/src/rules/reference-helpers/svelte-store.ts b/src/rules/reference-helpers/svelte-store.ts index ba58abff3..5d41a63fd 100644 --- a/src/rules/reference-helpers/svelte-store.ts +++ b/src/rules/reference-helpers/svelte-store.ts @@ -1,6 +1,11 @@ import type * as ESTree from "estree" +import type { TSESTree } from "@typescript-eslint/types" import { ReferenceTracker } from "eslint-utils" +import type { Variable } from "eslint-scope" import type { RuleContext } from "../../types" +import type { TS, TSTools } from "../../utils/ts-utils" +import { getTypeScriptTools } from "../../utils/ts-utils" +import { findVariable, getParent } from "../../utils/ast-utils" type StoreName = "writable" | "readable" | "derived" @@ -30,3 +35,162 @@ export function* extractStoreReferences( } } } + +export type StoreChecker = ( + node: ESTree.Expression | TSESTree.Expression, + options?: { consistent?: boolean }, +) => boolean +type StoreCheckerWithOptions = ( + node: ESTree.Expression, + options: { consistent: boolean }, +) => boolean + +/** + * Creates a function that checks whether the given expression node is a store instance or not. + */ +export function createStoreChecker(context: RuleContext): StoreChecker { + const tools = getTypeScriptTools(context) + const checker = tools + ? createStoreCheckerForTS(tools) + : createStoreCheckerForES(context) + + return (node, options) => + checker(node as ESTree.Expression, { + consistent: options?.consistent ?? false, + }) +} + +/** + * Creates a function that checks whether the given expression node is a store instance or not, for EcmaScript. + */ +function createStoreCheckerForES( + context: RuleContext, +): StoreCheckerWithOptions { + const storeVariables = new Map() + for (const { node } of extractStoreReferences(context)) { + const parent = getParent(node) + if ( + !parent || + parent.type !== "VariableDeclarator" || + parent.id.type !== "Identifier" + ) { + continue + } + const decl = getParent(parent) + if (!decl || decl.type !== "VariableDeclaration") { + continue + } + + const variable = findVariable(context, parent.id) + if (variable) { + storeVariables.set(variable, { const: decl.kind === "const" }) + } + } + + return (node, options) => { + if (node.type !== "Identifier" || node.name.startsWith("$")) { + return false + } + const variable = findVariable(context, node) + if (!variable) { + return false + } + const info = storeVariables.get(variable) + if (!info) { + return false + } + return options.consistent ? info.const : true + } +} + +/** + * Creates a function that checks whether the given expression node is a store instance or not, for TypeScript. + */ +function createStoreCheckerForTS(tools: TSTools): StoreCheckerWithOptions { + const { service } = tools + const checker = service.program.getTypeChecker() + const tsNodeMap = service.esTreeNodeToTSNodeMap + + return (node, options) => { + const tsNode = tsNodeMap.get(node) + if (!tsNode) { + return false + } + const type = checker.getTypeAtLocation(tsNode) + + return isStoreType(checker.getApparentType(type)) + + /** + * Checks whether the given type is a store or not + */ + function isStoreType(type: TS.Type): boolean { + return eachTypeCheck(type, options, (type) => { + const subscribe = type.getProperty("subscribe") + if (!subscribe) { + return false + } + const subscribeType = checker.getTypeOfSymbolAtLocation( + subscribe, + tsNode!, + ) + return isStoreSubscribeSignatureType(subscribeType) + }) + } + + /** + * Checks whether the given type is a store's subscribe or not + */ + function isStoreSubscribeSignatureType(type: TS.Type): boolean { + return eachTypeCheck(type, options, (type) => { + for (const signature of type.getCallSignatures()) { + if ( + signature.parameters.length >= 2 && + maybeFunctionSymbol(signature.parameters[0]) && + maybeFunctionSymbol(signature.parameters[1]) + ) { + return true + } + } + return false + }) + } + + /** + * Checks whether the given symbol maybe function param or not + */ + function maybeFunctionSymbol(param: TS.Symbol): boolean { + const type: TS.Type | undefined = checker.getApparentType( + checker.getTypeOfSymbolAtLocation(param, tsNode!), + ) + return maybeFunctionType(type) + } + + /** + * Checks whether the given type is maybe function param or not + */ + function maybeFunctionType(type: TS.Type): boolean { + return eachTypeCheck(type, { consistent: false }, (type) => { + return type.getCallSignatures().length > 0 + }) + } + } +} + +/** + * Check the given type with the given check function. + * For union types, `options.consistent: true` requires all types to pass the check function. + * `options.consistent: false` considers a match if any type passes the check function. + */ +function eachTypeCheck( + type: TS.Type, + options: { consistent: boolean }, + check: (t: TS.Type) => boolean, +): boolean { + if (type.isUnion()) { + if (options.consistent) { + return type.types.every((t) => eachTypeCheck(t, options, check)) + } + return type.types.some((t) => eachTypeCheck(t, options, check)) + } + return check(type) +} diff --git a/src/rules/require-store-reactive-access.ts b/src/rules/require-store-reactive-access.ts new file mode 100644 index 000000000..0ea2bdd3d --- /dev/null +++ b/src/rules/require-store-reactive-access.ts @@ -0,0 +1,302 @@ +import type * as ESTree from "estree" +import type { TSESTree } from "@typescript-eslint/types" +import type { AST } from "svelte-eslint-parser" +import { createRule } from "../utils" +import { createStoreChecker } from "./reference-helpers/svelte-store" + +export default createRule("require-store-reactive-access", { + meta: { + docs: { + description: + "disallow to use of the store itself as an operand. Need to use $ prefix or get function.", + category: "Possible Errors", + // TODO Switch to recommended in the major version. + // recommended: true, + recommended: false, + }, + fixable: "code", + schema: [], + messages: { + usingRawStoreInText: + "Use the $ prefix or the get function to access reactive values instead of accessing the raw store.", + }, + type: "problem", + }, + create(context) { + if (!context.parserServices.isSvelte) { + return {} + } + const isStore = createStoreChecker(context) + + /** Verify for expression node */ + function verifyExpression( + node: ESTree.Expression | null | undefined | TSESTree.Expression, + options?: { disableFix?: boolean; consistent?: boolean }, + ) { + if (!node) return + if (isStore(node, { consistent: options?.consistent })) { + context.report({ + node, + messageId: "usingRawStoreInText", + fix: + node.type === "Identifier" && !options?.disableFix + ? (fixer) => fixer.insertTextBefore(node, "$") + : null, + }) + } + } + + return { + SvelteMustacheTag(node) { + if (canAcceptStoreMustache(node)) { + return + } + // Check for

{store}

, etc. + verifyExpression(node.expression) + }, + SvelteShorthandAttribute(node) { + if (canAcceptStoreAttributeElement(node.parent.parent)) { + return + } + // Check for

+ verifyExpression(node.value, { disableFix: true }) + }, + SvelteSpreadAttribute(node) { + // Check for + verifyExpression(node.argument) + }, + SvelteDirective(node) { + if ( + node.kind === "Action" || + node.kind === "Animation" || + node.kind === "Transition" + ) { + if (node.key.name.type !== "Identifier") { + return + } + // Check for