diff --git a/.changeset/crazy-lights-wave.md b/.changeset/crazy-lights-wave.md new file mode 100644 index 000000000..0e18f6ec5 --- /dev/null +++ b/.changeset/crazy-lights-wave.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': patch +--- + +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`. diff --git a/docs/rules/no-navigation-without-resolve.md b/docs/rules/no-navigation-without-resolve.md index 05ea2ad9a..775de8ccb 100644 --- a/docs/rules/no-navigation-without-resolve.md +++ b/docs/rules/no-navigation-without-resolve.md @@ -39,7 +39,6 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu // ✗ BAD goto('/foo'); goto('/foo' + resolve('/bar')); - goto(resolve('/foo') + '/bar'); pushState('/foo', {}); replaceState('/foo', {}); @@ -64,7 +63,8 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu "ignoreGoto": false, "ignoreLinks": false, "ignorePushState": false, - "ignoreReplaceState": false + "ignoreReplaceState": false, + "allowSuffix": true } ] } @@ -74,6 +74,34 @@ This rule checks all 4 navigation options for the presence of the `resolve()` fu - `ignoreLinks` ... Whether to ignore all `` tags. Default `false`. - `ignorePushState` ... Whether to ignore all `pushState()` calls. Default `false`. - `ignoreReplaceState` ... Whether to ignore all `replaceState()` calls. Default `false`. +- `allowSuffix` ... Whether to allow adding any suffix to a `resolve()` result. Default `true`. + +When `allowSuffix` is enabled (default), the following patterns are allowed: + +```svelte + + +OK +``` + +When `allowSuffix` is enabled (default), any suffix after `resolve()` is accepted. Examples: + +```svelte +goto(resolve('/foo') + window.location.search); const base = resolve('/baz'); +OK +``` + +If you want to report partial `resolve()` concatenations (such as `resolve('/foo') + '?foo=bar'`), set `allowSuffix` to `false`. ## :books: Further Reading diff --git a/packages/eslint-plugin-svelte/src/rule-types.ts b/packages/eslint-plugin-svelte/src/rule-types.ts index f58b636f9..bac138629 100644 --- a/packages/eslint-plugin-svelte/src/rule-types.ts +++ b/packages/eslint-plugin-svelte/src/rule-types.ts @@ -530,6 +530,7 @@ type SvelteNoNavigationWithoutResolve = []|[{ ignoreLinks?: boolean ignorePushState?: boolean ignoreReplaceState?: boolean + allowSuffix?: boolean }] // ----- svelte/no-reactive-reassign ----- type SvelteNoReactiveReassign = []|[{ diff --git a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts index 06ad91df8..aed1161a8 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-resolve.ts @@ -1,5 +1,6 @@ import type { TSESTree } from '@typescript-eslint/types'; import { createRule } from '../utils/index.js'; +import type { TrackedReferences } from '@eslint-community/eslint-utils'; import { ReferenceTracker } from '@eslint-community/eslint-utils'; import { FindVariableContext } from '../utils/ast-utils.js'; import { findVariable } from '../utils/ast-utils.js'; @@ -29,6 +30,9 @@ export default createRule('no-navigation-without-resolve', { }, ignoreReplaceState: { type: 'boolean' + }, + allowSuffix: { + type: 'boolean' } }, additionalProperties: false @@ -49,10 +53,14 @@ export default createRule('no-navigation-without-resolve', { }, create(context) { let resolveReferences: Set = new Set(); + let assetReferences: Set = new Set(); return { Program() { const referenceTracker = new ReferenceTracker(context.sourceCode.scopeManager.globalScope!); - resolveReferences = extractResolveReferences(referenceTracker, context); + ({ resolve: resolveReferences, asset: assetReferences } = extractResolveReferences( + referenceTracker, + context + )); const { goto: gotoCalls, pushState: pushStateCalls, @@ -102,10 +110,16 @@ export default createRule('no-navigation-without-resolve', { (node.value[0].type === 'SvelteMustacheTag' && !expressionIsAbsolute(new FindVariableContext(context), node.value[0].expression) && !expressionIsFragment(new FindVariableContext(context), node.value[0].expression) && - !isResolveCall( + !isResolveWithOptionalSuffix( + new FindVariableContext(context), + node.value[0].expression, + resolveReferences, + context.options[0]?.allowSuffix !== false + ) && + !isAssetOnly( new FindVariableContext(context), node.value[0].expression, - resolveReferences + assetReferences )) ) { context.report({ loc: node.value[0].loc, messageId: 'linkWithoutResolve' }); @@ -120,9 +134,10 @@ export default createRule('no-navigation-without-resolve', { function extractResolveReferences( referenceTracker: ReferenceTracker, context: RuleContext -): Set { - const set = new Set(); - for (const { node } of referenceTracker.iterateEsmReferences({ +): { resolve: Set; asset: Set } { + const resolveSet = new Set(); + const assetSet = new Set(); + for (const { node, path } of referenceTracker.iterateEsmReferences({ '$app/paths': { [ReferenceTracker.ESM]: true, asset: { @@ -139,17 +154,16 @@ function extractResolveReferences( continue; } for (const reference of variable.references) { - if (reference.identifier.type === 'Identifier') set.add(reference.identifier); + if (reference.identifier.type !== 'Identifier') continue; + if (path[path.length - 1] === 'resolve') resolveSet.add(reference.identifier); + if (path[path.length - 1] === 'asset') assetSet.add(reference.identifier); } - } else if ( - node.type === 'MemberExpression' && - node.property.type === 'Identifier' && - node.property.name === 'resolve' - ) { - set.add(node.property); + } else if (node.type === 'MemberExpression' && node.property.type === 'Identifier') { + if (node.property.name === 'resolve') resolveSet.add(node.property); + if (node.property.name === 'asset') assetSet.add(node.property); } } - return set; + return { resolve: resolveSet, asset: assetSet }; } // Extract all references to goto, pushState and replaceState @@ -175,16 +189,21 @@ function extractFunctionCallReferences(referenceTracker: ReferenceTracker): { } }) ); + + function onlyCallExpressions(list: TrackedReferences[]): TSESTree.CallExpression[] { + return list + .filter((r) => r.node.type === 'CallExpression') + .map((r) => r.node as TSESTree.CallExpression); + } + return { - goto: rawReferences - .filter(({ path }) => path[path.length - 1] === 'goto') - .map(({ node }) => node as TSESTree.CallExpression), - pushState: rawReferences - .filter(({ path }) => path[path.length - 1] === 'pushState') - .map(({ node }) => node as TSESTree.CallExpression), - replaceState: rawReferences - .filter(({ path }) => path[path.length - 1] === 'replaceState') - .map(({ node }) => node as TSESTree.CallExpression) + goto: onlyCallExpressions(rawReferences.filter(({ path }) => path[path.length - 1] === 'goto')), + pushState: onlyCallExpressions( + rawReferences.filter(({ path }) => path[path.length - 1] === 'pushState') + ), + replaceState: onlyCallExpressions( + rawReferences.filter(({ path }) => path[path.length - 1] === 'replaceState') + ) }; } @@ -199,7 +218,14 @@ function checkGotoCall( return; } const url = call.arguments[0]; - if (!isResolveCall(new FindVariableContext(context), url, resolveReferences)) { + if ( + !isResolveWithOptionalSuffix( + new FindVariableContext(context), + url, + resolveReferences, + context.options[0]?.allowSuffix !== false + ) + ) { context.report({ loc: url.loc, messageId: 'gotoWithoutResolve' }); } } @@ -216,7 +242,12 @@ function checkShallowNavigationCall( const url = call.arguments[0]; if ( !expressionIsEmpty(url) && - !isResolveCall(new FindVariableContext(context), url, resolveReferences) + !isResolveWithOptionalSuffix( + new FindVariableContext(context), + url, + resolveReferences, + context.options[0]?.allowSuffix !== false + ) ) { context.report({ loc: url.loc, messageId }); } @@ -253,6 +284,90 @@ function isResolveCall( return isResolveCall(ctx, variable.identifiers[0].parent.init, resolveReferences); } +function isResolveWithOptionalSuffix( + ctx: FindVariableContext, + node: TSESTree.Expression | TSESTree.CallExpressionArgument, + resolveReferences: Set, + allowSuffix: boolean +): boolean { + if ( + (node.type === 'CallExpression' || node.type === 'Identifier') && + isResolveCall(ctx, node, resolveReferences) + ) { + return true; + } + + if (!allowSuffix) return false; + return expressionStartsWithResolve(ctx, node, resolveReferences); +} + +function expressionStartsWithResolve( + ctx: FindVariableContext, + node: TSESTree.Expression | TSESTree.CallExpressionArgument, + resolveReferences: Set +): boolean { + // Direct call + if (node.type === 'CallExpression') { + return isResolveCall(ctx, node, resolveReferences); + } + // Binary chain: ensure the left-most operand is resolve(); any right-hand content is allowed + if (node.type === 'BinaryExpression') { + if (node.operator !== '+' || node.left.type === 'PrivateIdentifier') return false; + return expressionStartsWithResolve(ctx, node.left, resolveReferences); + } + // Template literal: must start with expression and that expression starts with resolve(); content after is allowed + if (node.type === 'TemplateLiteral') { + if ( + node.expressions.length === 0 || + (node.quasis.length >= 1 && node.quasis[0].value.raw !== '') + ) + return false; + return expressionStartsWithResolve(ctx, node.expressions[0], resolveReferences); + } + // Identifier indirection + if (node.type === 'Identifier') { + const variable = ctx.findVariable(node); + if ( + variable === null || + variable.identifiers.length === 0 || + variable.identifiers[0].parent.type !== 'VariableDeclarator' || + variable.identifiers[0].parent.init === null + ) { + return false; + } + return expressionStartsWithResolve(ctx, variable.identifiers[0].parent.init, resolveReferences); + } + return false; +} + +function isAssetOnly( + ctx: FindVariableContext, + node: TSESTree.Expression | TSESTree.CallExpressionArgument, + assetReferences: Set +): boolean { + if (node.type === 'CallExpression') { + return ( + (node.callee.type === 'Identifier' && assetReferences.has(node.callee)) || + (node.callee.type === 'MemberExpression' && + node.callee.property.type === 'Identifier' && + assetReferences.has(node.callee.property)) + ); + } + if (node.type === 'Identifier') { + const variable = ctx.findVariable(node); + if ( + variable === null || + variable.identifiers.length === 0 || + variable.identifiers[0].parent.type !== 'VariableDeclarator' || + variable.identifiers[0].parent.init === null + ) { + return false; + } + return isAssetOnly(ctx, variable.identifiers[0].parent.init, assetReferences); + } + return false; +} + function expressionIsEmpty(url: TSESTree.CallExpressionArgument): boolean { return ( (url.type === 'Literal' && url.value === '') || diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/goto-partial-resolve01-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/goto-partial-resolve01-config.json new file mode 100644 index 000000000..9ea88d169 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/goto-partial-resolve01-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowSuffix": false + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-asset01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-asset01-errors.yaml index 7b8b81c68..2d1f90f38 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-asset01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-asset01-errors.yaml @@ -1,8 +1,8 @@ -- message: Found a link with a url that isn't resolved. +- message: Unexpected href link without resolve(). line: 5 column: 9 suggestions: null -- message: Found a link with a url that isn't resolved. +- message: Unexpected href link without resolve(). line: 6 column: 9 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-resolve01-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-resolve01-config.json new file mode 100644 index 000000000..9ea88d169 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/link-partial-resolve01-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowSuffix": false + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/pushState-partial-resolve01-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/pushState-partial-resolve01-config.json new file mode 100644 index 000000000..9ea88d169 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/pushState-partial-resolve01-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowSuffix": false + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/replaceState-partial-resolve01-config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/replaceState-partial-resolve01-config.json new file mode 100644 index 000000000..9ea88d169 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/invalid/replaceState-partial-resolve01-config.json @@ -0,0 +1,7 @@ +{ + "options": [ + { + "allowSuffix": false + } + ] +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/_config.json b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/_config.json new file mode 100644 index 000000000..0b08e5923 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/_config.json @@ -0,0 +1,9 @@ +{ + "options": [ + { + "allowSuffix": true + } + ] +} + + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/goto-allow-suffix01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/goto-allow-suffix01-input.svelte new file mode 100644 index 000000000..5343420af --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/goto-allow-suffix01-input.svelte @@ -0,0 +1,12 @@ + + + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/link-allow-suffix01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/link-allow-suffix01-input.svelte new file mode 100644 index 000000000..380390be3 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/link-allow-suffix01-input.svelte @@ -0,0 +1,11 @@ + + +Click me! +Click me! +Click me! + + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/pushState-allow-suffix01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/pushState-allow-suffix01-input.svelte new file mode 100644 index 000000000..5c4f7f7c1 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/pushState-allow-suffix01-input.svelte @@ -0,0 +1,10 @@ + diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/replaceState-allow-suffix01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/replaceState-allow-suffix01-input.svelte new file mode 100644 index 000000000..495aef676 --- /dev/null +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-resolve/valid/allowSuffix/replaceState-allow-suffix01-input.svelte @@ -0,0 +1,10 @@ +