From 5a495dc28482c4b719d5f80e1f69cea717110c02 Mon Sep 17 00:00:00 2001 From: baseballyama Date: Sun, 5 Jan 2025 17:07:49 +0900 Subject: [PATCH] fix(@typescript-eslint/no-shadow): ignore `{#snippet}` if it uses under component --- .changeset/quiet-mangos-nail.md | 5 ++ docs/rules/@typescript-eslint/no-shadow.md | 75 +++++++++++++++++ .../src/rules/@typescript-eslint/no-shadow.ts | 83 +++++++++++++++++++ .../src/utils/eslint-core.ts | 29 +++++++ .../eslint-plugin-svelte/src/utils/rules.ts | 2 + .../no-shadow/invalid/basic-errors.yaml | 4 + .../no-shadow/invalid/basic-input.svelte | 7 ++ .../no-shadow/invalid/const-errors.yaml | 4 + .../no-shadow/invalid/const-input.svelte | 10 +++ .../no-shadow/valid/basic-input.svelte | 10 +++ .../no-shadow/valid/snippet1-input.svelte | 13 +++ .../valid/snippet1-requirements.json | 3 + .../no-shadow/valid/snippet2-input.svelte | 10 +++ .../valid/snippet2-requirements.json | 3 + .../src/rules/@typescript-eslint/no-shadow.ts | 24 ++++++ 15 files changed, 282 insertions(+) create mode 100644 .changeset/quiet-mangos-nail.md create mode 100644 docs/rules/@typescript-eslint/no-shadow.md create mode 100644 packages/eslint-plugin-svelte/src/rules/@typescript-eslint/no-shadow.ts create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-errors.yaml create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/basic-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-requirements.json create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-input.svelte create mode 100644 packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-requirements.json create mode 100644 packages/eslint-plugin-svelte/tests/src/rules/@typescript-eslint/no-shadow.ts diff --git a/.changeset/quiet-mangos-nail.md b/.changeset/quiet-mangos-nail.md new file mode 100644 index 000000000..0e07443d6 --- /dev/null +++ b/.changeset/quiet-mangos-nail.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': patch +--- + +fix(@typescript-eslint/no-shadow): ignore `{#snippet}` if it uses under component diff --git a/docs/rules/@typescript-eslint/no-shadow.md b/docs/rules/@typescript-eslint/no-shadow.md new file mode 100644 index 000000000..c4da5b529 --- /dev/null +++ b/docs/rules/@typescript-eslint/no-shadow.md @@ -0,0 +1,75 @@ +--- +pageClass: 'rule-details' +sidebarDepth: 0 +title: 'svelte/@typescript-eslint/no-shadow' +description: 'Disallow variable declarations from shadowing variables declared in the outer scope' +--- + +# svelte/@typescript-eslint/no-shadow + +> Disallow variable declarations from shadowing variables declared in the outer scope + +- :exclamation: **_This rule has not been released yet._** + +## :book: Rule Details + +This rule reports shadowed variables, similar to the base ESLint `@typescript-eslint/no-shadow` rule. However, it ignores cases where `{#snippet}` is used as a named slot in Svelte components. If this rule is active, make sure to disable the base `@typescript-eslint/no-shadow` and `svelte/no-shadow` and `no-shadow` rule, as it will conflict with this rule. + + + +```svelte + + + + + {#snippet children()} + + {#snippet children()} + Hello! + {/snippet} + + {/snippet} + + + + {@const foo = 1} + + {@const foo = 2} + + +``` + +## :wrench: Options + +```json +{ + "svelte/no-shadow": [ + "error", + { "builtinGlobals": false, "hoist": "functions", "allow": [], "ignoreOnInitialization": false } + ] +} +``` + +- `builtinGlobals`: The `builtinGlobals` option is `false` by default. If it is `true`, the rule prevents shadowing of built-in global variables: `Object`, `Array`, `Number`, and so on. +- `hoist`: The `hoist` option has three settings: + - `functions` (by default) - reports shadowing before the outer functions are defined. + - `all` - reports all shadowing before the outer variables/functions are defined. + - `never` - never report shadowing before the outer variables/functions are defined. +- `allow`: The `allow` option is an array of identifier names for which shadowing is allowed. For example, `"resolve"`, `"reject"`, `"done"`, `"cb"`. +- `ignoreOnInitialization`: The `ignoreOnInitialization` option is `false` by default. If it is `true`, it prevents reporting shadowing of variables in their initializers when the shadowed variable is presumably still uninitialized. The shadowed variable must be on the left side. The shadowing variable must be on the right side and declared in a callback function or in an IIFE. +- `ignoreTypeValueShadow`: Whether to ignore types named the same as a variable. Default: `true`. This is generally safe because you cannot use variables in type locations without a `typeof` operator, so there's little risk of confusion. +- `ignoreFunctionTypeParameterNameValueShadow`: Whether to ignore function parameters named the same as a variable. Default: `true`. Each of a function type's arguments creates a value variable within the scope of the function type. This is done so that you can reference the type later using the `typeof` operator. + +## :books: Further Reading + +- See [typescript-eslint `no-shadow` rule](https://typescript-eslint.io/rules/no-shadow/) for more information about the base rule. + +## :mag: Implementation + +- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-shadow.ts) +- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/no-shadow.ts) + +Taken with ❤️ [from @typescript-eslint/eslint-plugin](https://typescript-eslint.io/rules/no-shadow/) diff --git a/packages/eslint-plugin-svelte/src/rules/@typescript-eslint/no-shadow.ts b/packages/eslint-plugin-svelte/src/rules/@typescript-eslint/no-shadow.ts new file mode 100644 index 000000000..509d89764 --- /dev/null +++ b/packages/eslint-plugin-svelte/src/rules/@typescript-eslint/no-shadow.ts @@ -0,0 +1,83 @@ +import { createRule } from '../../utils/index.js'; +import { defineWrapperListener, getProxyContent, getCoreRule } from '../../utils/eslint-core.js'; +import type { TSESTree } from '@typescript-eslint/types'; +import type { Scope } from '@typescript-eslint/scope-manager'; +import type { Range } from 'svelte-eslint-parser/lib/ast/common.js'; +import { getScope as getScopeUtil } from '../../utils/ast-utils.js'; +import { getSourceCode as getSourceCodeCompat } from '../../utils/compat.js'; + +const coreRule = getCoreRule('@typescript-eslint/no-shadow'); + +function removeSnippetIdentifiers(snippetIdentifierNodeLocations: Range[], scope: Scope): Scope { + return { + ...scope, + variables: scope.variables.filter((variable) => { + return !snippetIdentifierNodeLocations.some(([start, end]) => { + return variable.identifiers.every((identifier) => { + const { range } = identifier; + return range[0] === start && range[1] === end; + }); + }); + }), + childScopes: scope.childScopes.map((scope) => { + return removeSnippetIdentifiers(snippetIdentifierNodeLocations, scope); + }) + } as Scope; +} + +export default createRule('@typescript-eslint/no-shadow', { + meta: { + ...coreRule.meta, + docs: { + description: coreRule.meta.docs.description, + category: 'Best Practices', + recommended: false, + extensionRule: '@typescript-eslint/no-shadow' + } + }, + create(context) { + const snippetIdentifierNodeLocations: Range[] = []; + + function getScope(node: TSESTree.Node) { + const scope = getScopeUtil(context, node); + return removeSnippetIdentifiers(snippetIdentifierNodeLocations, scope); + } + + function getSourceCode() { + const sourceCode = getSourceCodeCompat(context); + return new Proxy(sourceCode, { + get(target, key) { + if (key === 'getScope') { + return getScope; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore + return (target as any)[key]; + } + }); + } + + return defineWrapperListener( + coreRule, + getProxyContent(context, { + sourceCode: getSourceCode() + }), + { + createListenerProxy(coreListener) { + return { + ...coreListener, + SvelteSnippetBlock(node) { + const parent = node.parent; + if (parent.type === 'SvelteElement' && parent.kind === 'component') { + snippetIdentifierNodeLocations.push(node.id.range); + } + coreListener.SvelteSnippetBlock?.(node); + }, + 'Program:exit'(node) { + coreListener['Program:exit']?.(node); + } + }; + } + } + ); + } +}); diff --git a/packages/eslint-plugin-svelte/src/utils/eslint-core.ts b/packages/eslint-plugin-svelte/src/utils/eslint-core.ts index d78f293e7..5289ca5c4 100644 --- a/packages/eslint-plugin-svelte/src/utils/eslint-core.ts +++ b/packages/eslint-plugin-svelte/src/utils/eslint-core.ts @@ -6,6 +6,22 @@ import { Linter } from 'eslint'; import Module from 'module'; const require = Module.createRequire(import.meta.url); + +export function getProxyContent(context: RuleContext, overrides: any): RuleContext { + const cache: any = {}; + return new Proxy(context, { + get(_t, key) { + if (key in cache) { + return cache[key]; + } + if (key in overrides) { + return (cache[key] = overrides[key]); + } + return (context as any)[key]; + } + }); +} + /** * Define the wrapped core rule. */ @@ -73,11 +89,24 @@ export function buildProxyListener( } let ruleMap: Map | null = null; +let tsRuleMap: Map | null = null; /** * Get the core rule implementation from the rule name */ export function getCoreRule(ruleName: string): RuleModule { + if (ruleName.startsWith('@typescript-eslint/')) { + if (tsRuleMap == null) { + const rules = require('@typescript-eslint/eslint-plugin').rules; + for (const [name, rule] of Object.entries(rules)) { + if (tsRuleMap == null) { + tsRuleMap = new Map(); + } + tsRuleMap.set(`@typescript-eslint/${name}`, rule as RuleModule); + } + } + return tsRuleMap!.get(ruleName)!; + } try { const map: Map = ruleMap ? ruleMap diff --git a/packages/eslint-plugin-svelte/src/utils/rules.ts b/packages/eslint-plugin-svelte/src/utils/rules.ts index cb0cbe896..7a2c421b5 100644 --- a/packages/eslint-plugin-svelte/src/utils/rules.ts +++ b/packages/eslint-plugin-svelte/src/utils/rules.ts @@ -3,6 +3,7 @@ // in order to update its content execute "pnpm run update" import type { RuleModule } from '../types.js'; import typescriptEslintNoUnnecessaryCondition from '../rules/@typescript-eslint/no-unnecessary-condition.js'; +import typescriptEslintNoShadow from '../rules/@typescript-eslint/no-shadow.js'; import blockLang from '../rules/block-lang.js'; import buttonHasType from '../rules/button-has-type.js'; import commentDirective from '../rules/comment-directive.js'; @@ -74,6 +75,7 @@ import validPropNamesInKitPages from '../rules/valid-prop-names-in-kit-pages.js' export const rules = [ typescriptEslintNoUnnecessaryCondition, + typescriptEslintNoShadow, blockLang, buttonHasType, commentDirective, diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-errors.yaml new file mode 100644 index 000000000..8f4b845d8 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-errors.yaml @@ -0,0 +1,4 @@ +- message: "'x' is already declared in the upper scope on line 2 column 13." + line: 4 + column: 8 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-input.svelte new file mode 100644 index 000000000..0b867af18 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/basic-input.svelte @@ -0,0 +1,7 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-errors.yaml new file mode 100644 index 000000000..a1ed736a7 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-errors.yaml @@ -0,0 +1,4 @@ +- message: "'foo' is already declared in the upper scope on line 6 column 10." + line: 8 + column: 11 + suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-input.svelte new file mode 100644 index 000000000..837414743 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/invalid/const-input.svelte @@ -0,0 +1,10 @@ + + + + {@const foo = 1} + + {@const foo = 2} + + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/basic-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/basic-input.svelte new file mode 100644 index 000000000..c90809ca6 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/basic-input.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-input.svelte new file mode 100644 index 000000000..e286d3982 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-input.svelte @@ -0,0 +1,13 @@ + + + + {#snippet children()} + + {#snippet children()} + Hello! + {/snippet} + + {/snippet} + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-requirements.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-requirements.json new file mode 100644 index 000000000..0192b1098 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet1-requirements.json @@ -0,0 +1,3 @@ +{ + "svelte": ">=5.0.0-0" +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-input.svelte new file mode 100644 index 000000000..cd5cbdd07 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-input.svelte @@ -0,0 +1,10 @@ + + + + {#snippet children()} + Hello! + {/snippet} + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-requirements.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-requirements.json new file mode 100644 index 000000000..0192b1098 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/@typescript-eslint/no-shadow/valid/snippet2-requirements.json @@ -0,0 +1,3 @@ +{ + "svelte": ">=5.0.0-0" +} diff --git a/packages/eslint-plugin-svelte/tests/src/rules/@typescript-eslint/no-shadow.ts b/packages/eslint-plugin-svelte/tests/src/rules/@typescript-eslint/no-shadow.ts new file mode 100644 index 000000000..fe0f36495 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/src/rules/@typescript-eslint/no-shadow.ts @@ -0,0 +1,24 @@ +import { RuleTester } from '../../../utils/eslint-compat.js'; +import rule from '../../../../src/rules/@typescript-eslint/no-shadow.js'; +import { loadTestCases, RULES_PROJECT } from '../../../utils/utils.js'; + +const tester = new RuleTester({ + languageOptions: { + ecmaVersion: 2020, + sourceType: 'module', + parserOptions: { + parser: { + ts: '@typescript-eslint/parser', + js: 'espree' + }, + project: RULES_PROJECT, + disallowAutomaticSingleRunInference: true + } + } +}); + +tester.run( + '@typescript-eslint/no-shadow', + rule as any, + loadTestCases('@typescript-eslint/no-shadow') +);