From 8a9a43d5a3e29eb87932e1b9b7fa1f9842e93676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20D=C4=9Bdi=C4=8D?= Date: Sun, 3 Nov 2024 22:15:40 +0100 Subject: [PATCH] feat(no-navigation-without-base): added support for urls defined in variables --- .../src/rules/no-navigation-without-base.ts | 73 ++++++++++++++----- .../invalid/goto-no-base01-errors.yaml | 6 +- .../invalid/goto-no-base01-input.svelte | 3 + .../invalid/link-no-base01-errors.yaml | 10 ++- .../invalid/link-no-base01-input.svelte | 4 + .../invalid/pushState-no-base01-errors.yaml | 6 +- .../invalid/pushState-no-base01-input.svelte | 3 + .../replaceState-no-base01-errors.yaml | 7 +- .../replaceState-no-base01-input.svelte | 3 + .../valid/goto-base-prefixed01-input.svelte | 5 ++ .../valid/link-base-prefixed01-input.svelte | 5 ++ .../pushState-base-prefixed01-input.svelte | 5 ++ .../replaceState-base-prefixed01-input.svelte | 5 ++ 13 files changed, 110 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts index 3befcad7c..429315545 100644 --- a/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts +++ b/packages/eslint-plugin-svelte/src/rules/no-navigation-without-base.ts @@ -96,14 +96,14 @@ export default createRule('no-navigation-without-base', { } const hrefValue = node.value[0]; if (hrefValue.type === 'SvelteLiteral') { - if (!urlIsAbsolute(hrefValue)) { + if (!expressionIsAbsolute(hrefValue)) { context.report({ loc: hrefValue.loc, messageId: 'linkNotPrefixed' }); } return; } if ( - !urlStartsWithBase(hrefValue.expression, basePathNames) && - !urlIsAbsolute(hrefValue.expression) + !expressionStartsWithBase(context, hrefValue.expression, basePathNames) && + !expressionIsAbsolute(hrefValue.expression) ) { context.report({ loc: hrefValue.loc, messageId: 'linkNotPrefixed' }); } @@ -183,7 +183,7 @@ function checkGotoCall( return; } const url = call.arguments[0]; - if (!urlStartsWithBase(url, basePathNames)) { + if (url.type === 'SpreadElement' || !expressionStartsWithBase(context, url, basePathNames)) { context.report({ loc: url.loc, messageId: 'gotoNotPrefixed' }); } } @@ -198,45 +198,79 @@ function checkShallowNavigationCall( return; } const url = call.arguments[0]; - if (!urlIsEmpty(url) && !urlStartsWithBase(url, basePathNames)) { + if ( + url.type === 'SpreadElement' || + (!expressionIsEmpty(url) && !expressionStartsWithBase(context, url, basePathNames)) + ) { context.report({ loc: url.loc, messageId }); } } // Helper functions -function urlStartsWithBase( - url: TSESTree.CallExpressionArgument, +function expressionStartsWithBase( + context: RuleContext, + url: TSESTree.Expression, basePathNames: Set ): boolean { switch (url.type) { case 'BinaryExpression': - return binaryExpressionStartsWithBase(url, basePathNames); + return binaryExpressionStartsWithBase(context, url, basePathNames); + case 'Identifier': + return variableStartsWithBase(context, url, basePathNames); case 'TemplateLiteral': - return templateLiteralStartsWithBase(url, basePathNames); + return templateLiteralStartsWithBase(context, url, basePathNames); default: return false; } } function binaryExpressionStartsWithBase( + context: RuleContext, url: TSESTree.BinaryExpression, basePathNames: Set ): boolean { - return url.left.type === 'Identifier' && basePathNames.has(url.left); + return ( + url.left.type !== 'PrivateIdentifier' && + expressionStartsWithBase(context, url.left, basePathNames) + ); +} + +function variableStartsWithBase( + context: RuleContext, + url: TSESTree.Identifier, + basePathNames: Set +): boolean { + if (basePathNames.has(url)) { + return true; + } + const variable = findVariable(context, url); + if ( + variable === null || + variable.identifiers.length !== 1 || + variable.identifiers[0].parent.type !== 'VariableDeclarator' || + variable.identifiers[0].parent.init === null + ) { + return false; + } + return expressionStartsWithBase(context, variable.identifiers[0].parent.init, basePathNames); } function templateLiteralStartsWithBase( + context: RuleContext, url: TSESTree.TemplateLiteral, basePathNames: Set ): boolean { - const startingIdentifier = extractLiteralStartingIdentifier(url); - return startingIdentifier !== undefined && basePathNames.has(startingIdentifier); + const startingIdentifier = extractLiteralStartingExpression(url); + return ( + startingIdentifier !== undefined && + expressionStartsWithBase(context, startingIdentifier, basePathNames) + ); } -function extractLiteralStartingIdentifier( +function extractLiteralStartingExpression( templateLiteral: TSESTree.TemplateLiteral -): TSESTree.Identifier | undefined { +): TSESTree.Expression | undefined { const literalParts = [...templateLiteral.expressions, ...templateLiteral.quasis].sort((a, b) => a.range[0] < b.range[0] ? -1 : 1 ); @@ -245,7 +279,7 @@ function extractLiteralStartingIdentifier( // Skip empty quasi in the begining continue; } - if (part.type === 'Identifier') { + if (part.type !== 'TemplateElement') { return part; } return undefined; @@ -253,7 +287,7 @@ function extractLiteralStartingIdentifier( return undefined; } -function urlIsEmpty(url: TSESTree.CallExpressionArgument): boolean { +function expressionIsEmpty(url: TSESTree.Expression): boolean { return ( (url.type === 'Literal' && url.value === '') || (url.type === 'TemplateLiteral' && @@ -263,7 +297,7 @@ function urlIsEmpty(url: TSESTree.CallExpressionArgument): boolean { ); } -function urlIsAbsolute(url: SvelteLiteral | TSESTree.Expression): boolean { +function expressionIsAbsolute(url: SvelteLiteral | TSESTree.Expression): boolean { switch (url.type) { case 'BinaryExpression': return binaryExpressionIsAbsolute(url); @@ -280,13 +314,14 @@ function urlIsAbsolute(url: SvelteLiteral | TSESTree.Expression): boolean { function binaryExpressionIsAbsolute(url: TSESTree.BinaryExpression): boolean { return ( - (url.left.type !== 'PrivateIdentifier' && urlIsAbsolute(url.left)) || urlIsAbsolute(url.right) + (url.left.type !== 'PrivateIdentifier' && expressionIsAbsolute(url.left)) || + expressionIsAbsolute(url.right) ); } function templateLiteralIsAbsolute(url: TSESTree.TemplateLiteral): boolean { return ( - url.expressions.some(urlIsAbsolute) || + url.expressions.some(expressionIsAbsolute) || url.quasis.some((quasi) => urlValueIsAbsolute(quasi.value.raw)) ); } diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-errors.yaml index 658fcb47d..ed8b9578e 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-errors.yaml @@ -1,4 +1,8 @@ - message: Found a goto() call with a url that isn't prefixed with the base path. - line: 4 + line: 6 + column: 7 + suggestions: null +- message: Found a goto() call with a url that isn't prefixed with the base path. + line: 7 column: 7 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-input.svelte index 6f011fe2d..7cb58ea20 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/goto-no-base01-input.svelte @@ -1,5 +1,8 @@ diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-errors.yaml index 6b8f7a36e..e006d4d27 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-errors.yaml @@ -1,12 +1,16 @@ - message: Found a link with a url that isn't prefixed with the base path. - line: 1 + line: 4 column: 10 suggestions: null - message: Found a link with a url that isn't prefixed with the base path. - line: 2 + line: 5 column: 9 suggestions: null - message: Found a link with a url that isn't prefixed with the base path. - line: 3 + line: 6 + column: 9 + suggestions: null +- message: Found a link with a url that isn't prefixed with the base path. + line: 7 column: 9 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-input.svelte index 546ecda29..0e0f6df25 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/link-no-base01-input.svelte @@ -1,3 +1,7 @@ + Click me! Click me! Click me! +Click me! diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-errors.yaml index a814d0c45..e70710ad0 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-errors.yaml @@ -1,4 +1,8 @@ - message: Found a pushState() call with a url that isn't prefixed with the base path. - line: 4 + line: 6 + column: 12 + suggestions: null +- message: Found a pushState() call with a url that isn't prefixed with the base path. + line: 7 column: 12 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-input.svelte index 37ea2b65d..19fe76cf3 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/pushState-no-base01-input.svelte @@ -1,5 +1,8 @@ diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-errors.yaml index fdd0c4620..dd4d0d177 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-errors.yaml @@ -1,5 +1,10 @@ - message: Found a replaceState() call with a url that isn't prefixed with the base path. - line: 4 + line: 6 + column: 15 + suggestions: null +- message: Found a replaceState() call with a url that isn't prefixed with the + base path. + line: 7 column: 15 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-input.svelte index 5826d76b0..4738c2ccb 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/invalid/replaceState-no-base01-input.svelte @@ -1,5 +1,8 @@ diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/goto-base-prefixed01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/goto-base-prefixed01-input.svelte index cd7177deb..22602dfc8 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/goto-base-prefixed01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/goto-base-prefixed01-input.svelte @@ -2,7 +2,12 @@ import { base } from '$app/paths'; import { goto } from '$app/navigation'; + const value1 = base + '/foo/'; + const value2 = `${base}/foo/`; + // eslint-disable-next-line prefer-template -- Testing both variants goto(base + '/foo/'); goto(`${base}/foo/`); + goto(value1); + goto(value2); diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/link-base-prefixed01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/link-base-prefixed01-input.svelte index 3bdaf021c..b86295824 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/link-base-prefixed01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/link-base-prefixed01-input.svelte @@ -1,6 +1,11 @@ Click me! Click me! +Click me! +Click me! diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/pushState-base-prefixed01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/pushState-base-prefixed01-input.svelte index 66854e812..468201793 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/pushState-base-prefixed01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/pushState-base-prefixed01-input.svelte @@ -2,7 +2,12 @@ import { base } from '$app/paths'; import { pushState } from '$app/navigation'; + const value1 = base + '/foo/'; + const value2 = `${base}/foo/`; + // eslint-disable-next-line prefer-template -- Testing both variants pushState(base + '/foo/'); pushState(`${base}/foo/`); + pushState(value1); + pushState(value2); diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/replaceState-base-prefixed01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/replaceState-base-prefixed01-input.svelte index 35b7082c2..7df67a5d5 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/replaceState-base-prefixed01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/no-navigation-without-base/valid/replaceState-base-prefixed01-input.svelte @@ -2,7 +2,12 @@ import { base } from '$app/paths'; import { replaceState } from '$app/navigation'; + const value1 = base + '/foo/'; + const value2 = `${base}/foo/`; + // eslint-disable-next-line prefer-template -- Testing both variants replaceState(base + '/foo/'); replaceState(`${base}/foo/`); + replaceState(value1); + replaceState(value2);