feat(no-navigation-without-resolve): add allowSuffix option

.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`.

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 `<a>` 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
+<script>
+  import { resolve } from '$app/paths';
+  import { goto, pushState, replaceState } from '$app/navigation';
+
+  goto(resolve('/foo') + '?q=1');
+  goto(`${resolve('/bar')}#frag`);
+
+  const base = resolve('/baz');
+  pushState(base + '&x=1');
+  replaceState(`${base}?k=v`);
+</script>
+
+<a href={resolve('/qux') + '?a=1'}>OK</a>
+```
+
+When `allowSuffix` is enabled (default), any suffix after `resolve()` is accepted. Examples:
+
+```svelte
+goto(resolve('/foo') + window.location.search); const base = resolve('/baz');
+<a href={`${base}${window.location.hash}`}>OK</a>
+```
+
+If you want to report partial `resolve()` concatenations (such as `resolve('/foo') + '?foo=bar'`), set `allowSuffix` to `false`.
 
 ## :books: Further Reading
 

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 = []|[{

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<TSESTree.Identifier> = new Set<TSESTree.Identifier>();
+		let assetReferences: Set<TSESTree.Identifier> = new Set<TSESTree.Identifier>();
 		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<TSESTree.Identifier> {
-	const set = new Set<TSESTree.Identifier>();
-	for (const { node } of referenceTracker.iterateEsmReferences({
+): { resolve: Set<TSESTree.Identifier>; asset: Set<TSESTree.Identifier> } {
+	const resolveSet = new Set<TSESTree.Identifier>();
+	const assetSet = new Set<TSESTree.Identifier>();
+	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<boolean>[]): 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<TSESTree.Identifier>,
+	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<TSESTree.Identifier>
+): 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<TSESTree.Identifier>
+): 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 === '') ||

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

@@ -0,0 +1,7 @@
+{
+	"options": [
+		{
+			"allowSuffix": false
+		}
+	]
+}

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

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

@@ -0,0 +1,7 @@
+{
+	"options": [
+		{
+			"allowSuffix": false
+		}
+	]
+}

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

@@ -0,0 +1,7 @@
+{
+	"options": [
+		{
+			"allowSuffix": false
+		}
+	]
+}

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

@@ -0,0 +1,7 @@
+{
+	"options": [
+		{
+			"allowSuffix": false
+		}
+	]
+}

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

@@ -0,0 +1,9 @@
+{
+	"options": [
+		{
+			"allowSuffix": true
+		}
+	]
+}
+
+

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

@@ -0,0 +1,12 @@
+<script>
+	import { resolve } from '$app/paths';
+	import { goto } from '$app/navigation';
+
+	goto(resolve('/foo') + '?q=1');
+	goto(`${resolve('/bar')}&p=2`);
+
+	const base = resolve('/baz');
+	goto(base + '#frag');
+</script>
+
+

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

@@ -0,0 +1,11 @@
+<script>
+	import { resolve } from '$app/paths';
+
+	const base = resolve('/foo');
+</script>
+
+<a href={resolve('/foo') + '?q=1'}>Click me!</a>
+<a href={`${resolve('/bar')}#frag`}>Click me!</a>
+<a href={base + '&x=1'}>Click me!</a>
+
+

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

@@ -0,0 +1,10 @@
+<script>
+	import { resolve } from '$app/paths';
+	import { pushState } from '$app/navigation';
+
+	pushState(resolve('/foo') + '?q=1');
+	pushState(`${resolve('/bar')}#frag`);
+
+	const base = resolve('/baz');
+	pushState(base + '&x=1');
+</script>