Skip to content

Commit 1a779d1

Browse files
committed
fix(no-shadow): ignore {#snippet} if it uses under component.
1 parent b97a13e commit 1a779d1

18 files changed

+271
-4
lines changed

.changeset/silly-lions-build.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-svelte': patch
3+
---
4+
5+
fix(no-shadow): ignore `{#snippet}` if it uses under component.

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ These rules relate to better ways of doing things to help you avoid problems:
363363
| [svelte/no-inspect](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-inspect/) | Warns against the use of `$inspect` directive | |
364364
| [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
365365
| [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |
366+
| [svelte/no-shadow](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shadow/) | Disallow variable declarations from shadowing variables declared in the outer scope | |
366367
| [svelte/no-svelte-internal](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-svelte-internal/) | svelte/internal will be removed in Svelte 6. | |
367368
| [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | |
368369
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |

docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ These rules relate to better ways of doing things to help you avoid problems:
6060
| [svelte/no-inspect](./rules/no-inspect.md) | Warns against the use of `$inspect` directive | |
6161
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: |
6262
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |
63+
| [svelte/no-shadow](./rules/no-shadow.md) | Disallow variable declarations from shadowing variables declared in the outer scope | |
6364
| [svelte/no-svelte-internal](./rules/no-svelte-internal.md) | svelte/internal will be removed in Svelte 6. | |
6465
| [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | |
6566
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |

docs/rules/no-shadow.md

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
pageClass: 'rule-details'
3+
sidebarDepth: 0
4+
title: 'svelte/no-shadow'
5+
description: 'Disallow variable declarations from shadowing variables declared in the outer scope'
6+
---
7+
8+
# svelte/no-shadow
9+
10+
> Disallow variable declarations from shadowing variables declared in the outer scope
11+
12+
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
13+
14+
## :book: Rule Details
15+
16+
This rule reports shadowed variables, similar to the base 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 `no-shadow` rule, as it will conflict with this rule.
17+
18+
<!--eslint-skip-->
19+
20+
```svelte
21+
<script>
22+
/* eslint svelte/no-shadow: "error" */
23+
import ComponentWithSnippet from './ComponentWithSnippet.svelte';
24+
</script>
25+
26+
<!-- ✓ GOOD -->
27+
<ComponentWithSnippet>
28+
{#snippet children()}
29+
<AnotherComponentWithSnippet>
30+
{#snippet children()}
31+
Hello!
32+
{/snippet}
33+
</AnotherComponentWithSnippet>
34+
{/snippet}
35+
</ComponentWithSnippet>
36+
37+
<!-- ✗ BAD -->
38+
<ComponentWithSnippet>
39+
{@const foo = 1}
40+
<ComponentWithSnippet>
41+
{@const foo = 2}
42+
</ComponentWithSnippet>
43+
</ComponentWithSnippet>
44+
```
45+
46+
## :wrench: Options
47+
48+
```json
49+
{
50+
"svelte/no-shadow": [
51+
"error",
52+
{ "builtinGlobals": false, "hoist": "functions", "allow": [], "ignoreOnInitialization": false }
53+
]
54+
}
55+
```
56+
57+
- `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.
58+
- `hoist`: The `hoist` option has three settings:
59+
- `functions` (by default) - reports shadowing before the outer functions are defined.
60+
- `all` - reports all shadowing before the outer variables/functions are defined.
61+
- `never` - never report shadowing before the outer variables/functions are defined.
62+
- `allow`: The `allow` option is an array of identifier names for which shadowing is allowed. For example, `"resolve"`, `"reject"`, `"done"`, `"cb"`.
63+
- `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.
64+
65+
## :books: Further Reading
66+
67+
- See [ESLint `no-shadow` rule](https://eslint.org/docs/latest/rules/no-shadow) for more information about the base rule.
68+
69+
## :mag: Implementation
70+
71+
- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/no-shadow.ts)
72+
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/no-shadow.ts)
73+
74+
<sup>Taken with ❤️ [from ESLint core](https://eslint.org/docs/rules/no-shadow)</sup>

packages/eslint-plugin-svelte/src/rule-types.ts

+12
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,11 @@ export interface RuleOptions {
215215
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-restricted-html-elements/
216216
*/
217217
'svelte/no-restricted-html-elements'?: Linter.RuleEntry<SvelteNoRestrictedHtmlElements>
218+
/**
219+
* Disallow variable declarations from shadowing variables declared in the outer scope
220+
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shadow/
221+
*/
222+
'svelte/no-shadow'?: Linter.RuleEntry<SvelteNoShadow>
218223
/**
219224
* disallow shorthand style properties that override related longhand properties
220225
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shorthand-style-property-overrides/
@@ -479,6 +484,13 @@ type SvelteNoRestrictedHtmlElements = [(string | {
479484
elements?: [string, ...(string)[]]
480485
message?: string
481486
}))[]]
487+
// ----- svelte/no-shadow -----
488+
type SvelteNoShadow = []|[{
489+
builtinGlobals?: boolean
490+
hoist?: ("all" | "functions" | "never")
491+
allow?: string[]
492+
ignoreOnInitialization?: boolean
493+
}]
482494
// ----- svelte/no-target-blank -----
483495
type SvelteNoTargetBlank = []|[{
484496
allowReferrer?: boolean
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { createRule } from '../utils/index.js';
2+
import { defineWrapperListener, getProxyContent, getCoreRule } from '../utils/eslint-core.js';
3+
import type { TSESTree } from '@typescript-eslint/types';
4+
import type { Scope } from '@typescript-eslint/scope-manager';
5+
import type { Range } from 'svelte-eslint-parser/lib/ast/common.js';
6+
import { getScope as getScopeUtil } from '../utils/ast-utils.js';
7+
import { getSourceCode as getSourceCodeCompat } from '../utils/compat.js';
8+
9+
const coreRule = getCoreRule('no-shadow');
10+
11+
function removeSnippetIdentifiers(snippetIdentifierNodeLocations: Range[], scope: Scope): Scope {
12+
return {
13+
...scope,
14+
variables: scope.variables.filter((variable) => {
15+
return !snippetIdentifierNodeLocations.some(([start, end]) => {
16+
return variable.identifiers.every((identifier) => {
17+
const { range } = identifier;
18+
return range[0] === start && range[1] === end;
19+
});
20+
});
21+
}),
22+
childScopes: scope.childScopes.map((scope) => {
23+
return removeSnippetIdentifiers(snippetIdentifierNodeLocations, scope);
24+
})
25+
} as Scope;
26+
}
27+
28+
export default createRule('no-shadow', {
29+
meta: {
30+
...coreRule.meta,
31+
docs: {
32+
description: coreRule.meta.docs.description,
33+
category: 'Best Practices',
34+
recommended: false,
35+
extensionRule: 'no-shadow'
36+
}
37+
},
38+
create(context) {
39+
const snippetIdentifierNodeLocations: Range[] = [];
40+
41+
function getScope(node: TSESTree.Node) {
42+
const scope = getScopeUtil(context, node);
43+
return removeSnippetIdentifiers(snippetIdentifierNodeLocations, scope);
44+
}
45+
46+
function getSourceCode() {
47+
const sourceCode = getSourceCodeCompat(context);
48+
return new Proxy(sourceCode, {
49+
get(target, key) {
50+
if (key === 'getScope') {
51+
return getScope;
52+
}
53+
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ignore
54+
return (target as any)[key];
55+
}
56+
});
57+
}
58+
59+
return defineWrapperListener(
60+
coreRule,
61+
getProxyContent(context, {
62+
sourceCode: getSourceCode()
63+
}),
64+
{
65+
createListenerProxy(coreListener) {
66+
return {
67+
...coreListener,
68+
SvelteSnippetBlock(node) {
69+
const parent = node.parent;
70+
if (parent.type === 'SvelteElement' && parent.kind === 'component') {
71+
snippetIdentifierNodeLocations.push(node.id.range);
72+
}
73+
coreListener.SvelteSnippetBlock?.(node);
74+
},
75+
'Program:exit'(node) {
76+
coreListener['Program:exit']?.(node);
77+
}
78+
};
79+
}
80+
}
81+
);
82+
}
83+
});

packages/eslint-plugin-svelte/src/utils/eslint-core.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ import { Linter } from 'eslint';
66
import Module from 'module';
77

88
const require = Module.createRequire(import.meta.url);
9+
10+
export function getProxyContent(context: RuleContext, overrides: any): RuleContext {
11+
const cache: any = {};
12+
return new Proxy(context, {
13+
get(_t, key) {
14+
if (key in cache) {
15+
return cache[key];
16+
}
17+
if (key in overrides) {
18+
return (cache[key] = overrides[key]);
19+
}
20+
return (context as any)[key];
21+
}
22+
});
23+
}
24+
925
/**
1026
* Define the wrapped core rule.
1127
*/
@@ -17,10 +33,7 @@ export function defineWrapperListener(
1733
}
1834
): RuleListener {
1935
const listener = coreRule.create(context as any);
20-
21-
const svelteListener = proxyOptions.createListenerProxy?.(listener) ?? listener;
22-
23-
return svelteListener;
36+
return proxyOptions.createListenerProxy?.(listener) ?? listener;
2437
}
2538

2639
/**

packages/eslint-plugin-svelte/src/utils/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import noReactiveFunctions from '../rules/no-reactive-functions.js';
4242
import noReactiveLiterals from '../rules/no-reactive-literals.js';
4343
import noReactiveReassign from '../rules/no-reactive-reassign.js';
4444
import noRestrictedHtmlElements from '../rules/no-restricted-html-elements.js';
45+
import noShadow from '../rules/no-shadow.js';
4546
import noShorthandStylePropertyOverrides from '../rules/no-shorthand-style-property-overrides.js';
4647
import noSpacesAroundEqualSignsInAttribute from '../rules/no-spaces-around-equal-signs-in-attribute.js';
4748
import noStoreAsync from '../rules/no-store-async.js';
@@ -113,6 +114,7 @@ export const rules = [
113114
noReactiveLiterals,
114115
noReactiveReassign,
115116
noRestrictedHtmlElements,
117+
noShadow,
116118
noShorthandStylePropertyOverrides,
117119
noSpacesAroundEqualSignsInAttribute,
118120
noStoreAsync,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- message: "'x' is already declared in the upper scope on line 2 column 13."
2+
line: 4
3+
column: 8
4+
suggestions: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script>
2+
function a(x) {
3+
var b = function c() {
4+
var x = 'foo';
5+
};
6+
}
7+
</script>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- message: "'foo' is already declared in the upper scope on line 6 column 10."
2+
line: 8
3+
column: 11
4+
suggestions: null
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
import ComponentWithSnippet from './ComponentWithSnippet.svelte';
3+
</script>
4+
5+
<ComponentWithSnippet>
6+
{@const foo = 1}
7+
<ComponentWithSnippet>
8+
{@const foo = 2}
9+
</ComponentWithSnippet>
10+
</ComponentWithSnippet>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
var a = 3;
3+
var b = (x) => {
4+
a++;
5+
return x + a;
6+
};
7+
setTimeout(() => {
8+
b(a);
9+
}, 0);
10+
</script>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<script>
2+
import ComponentWithSnippet from './ComponentWithSnippet.svelte';
3+
</script>
4+
5+
<ComponentWithSnippet>
6+
{#snippet children()}
7+
<AnotherComponentWithSnippet>
8+
{#snippet children()}
9+
Hello!
10+
{/snippet}
11+
</AnotherComponentWithSnippet>
12+
{/snippet}
13+
</ComponentWithSnippet>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"svelte": ">=5.0.0-0"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script>
2+
import ComponentWithSnippet from './ComponentWithSnippet.svelte';
3+
const children = 1;
4+
</script>
5+
6+
<ComponentWithSnippet>
7+
{#snippet children()}
8+
Hello!
9+
{/snippet}
10+
</ComponentWithSnippet>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"svelte": ">=5.0.0-0"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { RuleTester } from '../../utils/eslint-compat.js';
2+
import rule from '../../../src/rules/no-shadow.js';
3+
import { loadTestCases } from '../../utils/utils.js';
4+
5+
const tester = new RuleTester({
6+
languageOptions: {
7+
ecmaVersion: 2020,
8+
sourceType: 'module'
9+
}
10+
});
11+
12+
tester.run('no-shadow', rule as any, loadTestCases('no-shadow'));

0 commit comments

Comments
 (0)