Skip to content

Commit 425224c

Browse files
baseballyamabaseballyama
committed
feat: implement
1 parent 27cf677 commit 425224c

13 files changed

+258
-29
lines changed

‎.changeset/crazy-lights-wave.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-svelte': patch
3+
---
4+
5+
feat(no-navigation-without-resolve): add `allowSuffix` option. This option disables error reporting when there is a suffix such as query parameters after the `resolve()` function. The default is `true`.

‎docs/rules/no-navigation-without-resolve.md‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu
3939
// ✗ BAD
4040
goto('/foo');
4141
goto('/foo' + resolve('/bar'));
42-
goto(resolve('/foo') + '/bar');
4342
4443
pushState('/foo', {});
4544
replaceState('/foo', {});
@@ -64,7 +63,8 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu
6463
"ignoreGoto": false,
6564
"ignoreLinks": false,
6665
"ignorePushState": false,
67-
"ignoreReplaceState": false
66+
"ignoreReplaceState": false,
67+
"allowSuffix": true
6868
}
6969
]
7070
}
@@ -74,6 +74,35 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu
7474
- `ignoreLinks` ... Whether to ignore all `<a>` tags. Default `false`.
7575
- `ignorePushState` ... Whether to ignore all `pushState()` calls. Default `false`.
7676
- `ignoreReplaceState` ... Whether to ignore all `replaceState()` calls. Default `false`.
77+
- `allowSuffix` ... Whether to allow adding any suffix to a `resolve()` result. Default `true`.
78+
79+
When `allowSuffix` is enabled (default), the following patterns are allowed:
80+
81+
```svelte
82+
<script>
83+
import { resolve } from '$app/paths';
84+
import { goto, pushState, replaceState } from '$app/navigation';
85+
86+
goto(resolve('/foo') + '?q=1');
87+
goto(`${resolve('/bar')}#frag`);
88+
89+
const base = resolve('/baz');
90+
pushState(base + '&x=1');
91+
replaceState(`${base}?k=v`);
92+
</script>
93+
94+
<a href={resolve('/qux') + '?a=1'}>OK</a>
95+
```
96+
97+
When `allowSuffix` is enabled (default), any suffix after `resolve()` is accepted. Examples:
98+
99+
```svelte
100+
goto(resolve('/foo') + window.location.search);
101+
const base = resolve('/baz');
102+
<a href={`${base}${window.location.hash}`}>OK</a>
103+
```
104+
105+
If you want to report partial `resolve()` concatenations (such as `resolve('/foo') + '?foo=bar'`), set `allowSuffix` to `false`.
77106

78107
## :books: Further Reading
79108

‎packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts‎

Lines changed: 140 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { TSESTree } from '@typescript-eslint/types';
22
import { createRule } from '../utils/index.js';
3+
import type { TrackedReferences } from '@eslint-community/eslint-utils';
34
import { ReferenceTracker } from '@eslint-community/eslint-utils';
45
import { FindVariableContext } from '../utils/ast-utils.js';
56
import { findVariable } from '../utils/ast-utils.js';
@@ -29,6 +30,9 @@ export default createRule('no-navigation-without-resolve', {
2930
},
3031
ignoreReplaceState: {
3132
type: 'boolean'
33+
},
34+
allowSuffix: {
35+
type: 'boolean'
3236
}
3337
},
3438
additionalProperties: false
@@ -49,10 +53,14 @@ export default createRule('no-navigation-without-resolve', {
4953
},
5054
create(context) {
5155
let resolveReferences: Set<TSESTree.Identifier> = new Set<TSESTree.Identifier>();
56+
let assetReferences: Set<TSESTree.Identifier> = new Set<TSESTree.Identifier>();
5257
return {
5358
Program() {
5459
const referenceTracker = new ReferenceTracker(context.sourceCode.scopeManager.globalScope!);
55-
resolveReferences = extractResolveReferences(referenceTracker, context);
60+
({ resolve: resolveReferences, asset: assetReferences } = extractResolveReferences(
61+
referenceTracker,
62+
context
63+
));
5664
const {
5765
goto: gotoCalls,
5866
pushState: pushStateCalls,
@@ -102,10 +110,16 @@ export default createRule('no-navigation-without-resolve', {
102110
(node.value[0].type === 'SvelteMustacheTag' &&
103111
!expressionIsAbsolute(new FindVariableContext(context), node.value[0].expression) &&
104112
!expressionIsFragment(new FindVariableContext(context), node.value[0].expression) &&
105-
!isResolveCall(
113+
!isResolveWithOptionalSuffix(
114+
new FindVariableContext(context),
115+
node.value[0].expression,
116+
resolveReferences,
117+
context.options[0]?.allowSuffix !== false
118+
) &&
119+
!isAssetOnly(
106120
new FindVariableContext(context),
107121
node.value[0].expression,
108-
resolveReferences
122+
assetReferences
109123
))
110124
) {
111125
context.report({ loc: node.value[0].loc, messageId: 'linkWithoutResolve' });
@@ -120,9 +134,10 @@ export default createRule('no-navigation-without-resolve', {
120134
function extractResolveReferences(
121135
referenceTracker: ReferenceTracker,
122136
context: RuleContext
123-
): Set<TSESTree.Identifier> {
124-
const set = new Set<TSESTree.Identifier>();
125-
for (const { node } of referenceTracker.iterateEsmReferences({
137+
): { resolve: Set<TSESTree.Identifier>; asset: Set<TSESTree.Identifier> } {
138+
const resolveSet = new Set<TSESTree.Identifier>();
139+
const assetSet = new Set<TSESTree.Identifier>();
140+
for (const { node, path } of referenceTracker.iterateEsmReferences({
126141
'$app/paths': {
127142
[ReferenceTracker.ESM]: true,
128143
asset: {
@@ -139,17 +154,16 @@ function extractResolveReferences(
139154
continue;
140155
}
141156
for (const reference of variable.references) {
142-
if (reference.identifier.type === 'Identifier') set.add(reference.identifier);
157+
if (reference.identifier.type !== 'Identifier') continue;
158+
if (path[path.length - 1] === 'resolve') resolveSet.add(reference.identifier);
159+
if (path[path.length - 1] === 'asset') assetSet.add(reference.identifier);
143160
}
144-
} else if (
145-
node.type === 'MemberExpression' &&
146-
node.property.type === 'Identifier' &&
147-
node.property.name === 'resolve'
148-
) {
149-
set.add(node.property);
161+
} else if (node.type === 'MemberExpression' && node.property.type === 'Identifier') {
162+
if (node.property.name === 'resolve') resolveSet.add(node.property);
163+
if (node.property.name === 'asset') assetSet.add(node.property);
150164
}
151165
}
152-
return set;
166+
return { resolve: resolveSet, asset: assetSet };
153167
}
154168

155169
// Extract all references to goto, pushState and replaceState
@@ -175,16 +189,21 @@ function extractFunctionCallReferences(referenceTracker: ReferenceTracker): {
175189
}
176190
})
177191
);
192+
193+
function onlyCallExpressions(list: TrackedReferences<boolean>[]): TSESTree.CallExpression[] {
194+
return list
195+
.filter((r) => r.node.type === 'CallExpression')
196+
.map((r) => r.node as TSESTree.CallExpression);
197+
}
198+
178199
return {
179-
goto: rawReferences
180-
.filter(({ path }) => path[path.length - 1] === 'goto')
181-
.map(({ node }) => node as TSESTree.CallExpression),
182-
pushState: rawReferences
183-
.filter(({ path }) => path[path.length - 1] === 'pushState')
184-
.map(({ node }) => node as TSESTree.CallExpression),
185-
replaceState: rawReferences
186-
.filter(({ path }) => path[path.length - 1] === 'replaceState')
187-
.map(({ node }) => node as TSESTree.CallExpression)
200+
goto: onlyCallExpressions(rawReferences.filter(({ path }) => path[path.length - 1] === 'goto')),
201+
pushState: onlyCallExpressions(
202+
rawReferences.filter(({ path }) => path[path.length - 1] === 'pushState')
203+
),
204+
replaceState: onlyCallExpressions(
205+
rawReferences.filter(({ path }) => path[path.length - 1] === 'replaceState')
206+
)
188207
};
189208
}
190209

@@ -199,7 +218,14 @@ function checkGotoCall(
199218
return;
200219
}
201220
const url = call.arguments[0];
202-
if (!isResolveCall(new FindVariableContext(context), url, resolveReferences)) {
221+
if (
222+
!isResolveWithOptionalSuffix(
223+
new FindVariableContext(context),
224+
url,
225+
resolveReferences,
226+
context.options[0]?.allowSuffix !== false
227+
)
228+
) {
203229
context.report({ loc: url.loc, messageId: 'gotoWithoutResolve' });
204230
}
205231
}
@@ -216,7 +242,12 @@ function checkShallowNavigationCall(
216242
const url = call.arguments[0];
217243
if (
218244
!expressionIsEmpty(url) &&
219-
!isResolveCall(new FindVariableContext(context), url, resolveReferences)
245+
!isResolveWithOptionalSuffix(
246+
new FindVariableContext(context),
247+
url,
248+
resolveReferences,
249+
context.options[0]?.allowSuffix !== false
250+
)
220251
) {
221252
context.report({ loc: url.loc, messageId });
222253
}
@@ -253,6 +284,90 @@ function isResolveCall(
253284
return isResolveCall(ctx, variable.identifiers[0].parent.init, resolveReferences);
254285
}
255286

287+
function isResolveWithOptionalSuffix(
288+
ctx: FindVariableContext,
289+
node: TSESTree.Expression | TSESTree.CallExpressionArgument,
290+
resolveReferences: Set<TSESTree.Identifier>,
291+
allowSuffix: boolean
292+
): boolean {
293+
if (
294+
(node.type === 'CallExpression' || node.type === 'Identifier') &&
295+
isResolveCall(ctx, node, resolveReferences)
296+
) {
297+
return true;
298+
}
299+
300+
if (!allowSuffix) return false;
301+
return expressionStartsWithResolve(ctx, node, resolveReferences);
302+
}
303+
304+
function expressionStartsWithResolve(
305+
ctx: FindVariableContext,
306+
node: TSESTree.Expression | TSESTree.CallExpressionArgument,
307+
resolveReferences: Set<TSESTree.Identifier>
308+
): boolean {
309+
// Direct call
310+
if (node.type === 'CallExpression') {
311+
return isResolveCall(ctx, node, resolveReferences);
312+
}
313+
// Binary chain: ensure the left-most operand is resolve(); any right-hand content is allowed
314+
if (node.type === 'BinaryExpression') {
315+
if (node.operator !== '+' || node.left.type === 'PrivateIdentifier') return false;
316+
return expressionStartsWithResolve(ctx, node.left, resolveReferences);
317+
}
318+
// Template literal: must start with expression and that expression starts with resolve(); content after is allowed
319+
if (node.type === 'TemplateLiteral') {
320+
if (
321+
node.expressions.length === 0 ||
322+
(node.quasis.length >= 1 && node.quasis[0].value.raw !== '')
323+
)
324+
return false;
325+
return expressionStartsWithResolve(ctx, node.expressions[0], resolveReferences);
326+
}
327+
// Identifier indirection
328+
if (node.type === 'Identifier') {
329+
const variable = ctx.findVariable(node);
330+
if (
331+
variable === null ||
332+
variable.identifiers.length === 0 ||
333+
variable.identifiers[0].parent.type !== 'VariableDeclarator' ||
334+
variable.identifiers[0].parent.init === null
335+
) {
336+
return false;
337+
}
338+
return expressionStartsWithResolve(ctx, variable.identifiers[0].parent.init, resolveReferences);
339+
}
340+
return false;
341+
}
342+
343+
function isAssetOnly(
344+
ctx: FindVariableContext,
345+
node: TSESTree.Expression | TSESTree.CallExpressionArgument,
346+
assetReferences: Set<TSESTree.Identifier>
347+
): boolean {
348+
if (node.type === 'CallExpression') {
349+
return (
350+
(node.callee.type === 'Identifier' && assetReferences.has(node.callee)) ||
351+
(node.callee.type === 'MemberExpression' &&
352+
node.callee.property.type === 'Identifier' &&
353+
assetReferences.has(node.callee.property))
354+
);
355+
}
356+
if (node.type === 'Identifier') {
357+
const variable = ctx.findVariable(node);
358+
if (
359+
variable === null ||
360+
variable.identifiers.length === 0 ||
361+
variable.identifiers[0].parent.type !== 'VariableDeclarator' ||
362+
variable.identifiers[0].parent.init === null
363+
) {
364+
return false;
365+
}
366+
return isAssetOnly(ctx, variable.identifiers[0].parent.init, assetReferences);
367+
}
368+
return false;
369+
}
370+
256371
function expressionIsEmpty(url: TSESTree.CallExpressionArgument): boolean {
257372
return (
258373
(url.type === 'Literal' && url.value === '') ||

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/goto-partial-resolve01-config.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"allowSuffix": false
5+
}
6+
]
7+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-asset01-errors.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
- message: Found a link with a url that isn't resolved.
1+
- message: Unexpected href link without resolve().
22
line: 5
33
column: 9
44
suggestions: null
5-
- message: Found a link with a url that isn't resolved.
5+
- message: Unexpected href link without resolve().
66
line: 6
77
column: 9
88
suggestions: null

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-resolve01-config.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"allowSuffix": false
5+
}
6+
]
7+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/pushState-partial-resolve01-config.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"allowSuffix": false
5+
}
6+
]
7+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/replaceState-partial-resolve01-config.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"options": [
3+
{
4+
"allowSuffix": false
5+
}
6+
]
7+
}

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/_config.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"options": [
3+
{
4+
"allowSuffix": true
5+
}
6+
]
7+
}
8+
9+

‎packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/goto-allow-suffix01-input.svelte‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<script>
2+
import { resolve } from '$app/paths';
3+
import { goto } from '$app/navigation';
4+
5+
goto(resolve('/foo') + '?q=1');
6+
goto(`${resolve('/bar')}&p=2`);
7+
8+
const base = resolve('/baz');
9+
goto(base + '#frag');
10+
</script>
11+
12+

0 commit comments

Comments
 (0)