Skip to content

feat: added the no-goto-without-base rule #679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/shaggy-dryers-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-svelte": minor
---

feat: added the no-goto-without-base rule
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,14 @@ These rules extend the rules provided by ESLint itself, or other plugins to work
| [svelte/no-inner-declarations](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-inner-declarations/) | disallow variable or `function` declarations in nested blocks | :star: |
| [svelte/no-trailing-spaces](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-trailing-spaces/) | disallow trailing whitespace at the end of lines | :wrench: |

## SvelteKit

These rules relate to SvelteKit and its best Practices.

| Rule ID | Description | |
|:--------|:------------|:---|
| [svelte/no-goto-without-base](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-goto-without-base/) | disallow using goto() without the base path | |

## Experimental

:warning: These rules are considered experimental and may change or be removed in the future:
Expand Down
5 changes: 5 additions & 0 deletions docs-svelte-kit/src/lib/eslint/scripts/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export const categories = [
classes: 'svelte-category',
rules: []
},
{
title: 'SvelteKit',
classes: 'svelte-category',
rules: []
},
{
title: 'Experimental',
classes: 'svelte-category',
Expand Down
1 change: 1 addition & 0 deletions docs-svelte-kit/src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const categories = [
'Best Practices',
'Stylistic Issues',
'Extension Rules',
'SvelteKit',
'Experimental',
'System'
];
Expand Down
8 changes: 8 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ These rules extend the rules provided by ESLint itself, or other plugins to work
| [svelte/no-inner-declarations](./rules/no-inner-declarations.md) | disallow variable or `function` declarations in nested blocks | :star: |
| [svelte/no-trailing-spaces](./rules/no-trailing-spaces.md) | disallow trailing whitespace at the end of lines | :wrench: |

## SvelteKit

These rules relate to SvelteKit and its best Practices.

| Rule ID | Description | |
| :------------------------------------------------------------- | :------------------------------------------ | :-- |
| [svelte/no-goto-without-base](./rules/no-goto-without-base.md) | disallow using goto() without the base path | |

## Experimental

:warning: These rules are considered experimental and may change or be removed in the future:
Expand Down
61 changes: 61 additions & 0 deletions docs/rules/no-goto-without-base.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/no-goto-without-base'
description: 'disallow using goto() without the base path'
---

# svelte/no-goto-without-base

> disallow using goto() without the base path

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>

## :book: Rule Details

This rule reports navigation using SvelteKit's `goto()` function without prefixing a relative URL with the base path. If a non-prefixed relative URL is used for navigation, the `goto` function navigates away from the base path, which is usually not what you wanted to do (for external URLs, `window.location = url` should be used instead).

<ESLintCodeBlock>

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/no-goto-without-base: "error" */

import { goto } from '$app/navigation';
import { base } from '$app/paths';
import { base as baseAlias } from '$app/paths';

// ✓ GOOD
goto(base + '/foo/');
goto(`${base}/foo/`);

goto(baseAlias + '/foo/');
goto(`${baseAlias}/foo/`);

goto('https://localhost/foo/');

// ✗ BAD
goto('/foo');

goto('/foo/' + base);
goto(`/foo/${base}`);
</script>
```

</ESLintCodeBlock>

## :wrench: Options

Nothing.

## :books: Further Reading

- [`goto()` documentation](https://kit.svelte.dev/docs/modules#$app-navigation-goto)
- [`base` documentation](https://kit.svelte.dev/docs/modules#$app-paths-base)

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-goto-without-base.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-goto-without-base.ts)
134 changes: 134 additions & 0 deletions src/rules/no-goto-without-base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import type { TSESTree } from '@typescript-eslint/types';
import { createRule } from '../utils';
import { ReferenceTracker } from '@eslint-community/eslint-utils';
import { getSourceCode } from '../utils/compat';
import { findVariable } from '../utils/ast-utils';
import type { RuleContext } from '../types';

export default createRule('no-goto-without-base', {
meta: {
docs: {
description: 'disallow using goto() without the base path',
category: 'SvelteKit',
recommended: false
},
schema: [],
messages: {
isNotPrefixedWithBasePath:
"Found a goto() call with a url that isn't prefixed with the base path."
},
type: 'suggestion'
},
create(context) {
return {
Program() {
const referenceTracker = new ReferenceTracker(
getSourceCode(context).scopeManager.globalScope!
);
const basePathNames = extractBasePathReferences(referenceTracker, context);
for (const gotoCall of extractGotoReferences(referenceTracker)) {
if (gotoCall.arguments.length < 1) {
continue;
}
const path = gotoCall.arguments[0];
switch (path.type) {
case 'BinaryExpression':
checkBinaryExpression(context, path, basePathNames);
break;
case 'Literal':
checkLiteral(context, path);
break;
case 'TemplateLiteral':
checkTemplateLiteral(context, path, basePathNames);
break;
default:
context.report({ loc: path.loc, messageId: 'isNotPrefixedWithBasePath' });
}
}
}
};
}
});

function checkBinaryExpression(
context: RuleContext,
path: TSESTree.BinaryExpression,
basePathNames: Set<TSESTree.Identifier>
): void {
if (path.left.type !== 'Identifier' || !basePathNames.has(path.left)) {
context.report({ loc: path.loc, messageId: 'isNotPrefixedWithBasePath' });
}
}

function checkTemplateLiteral(
context: RuleContext,
path: TSESTree.TemplateLiteral,
basePathNames: Set<TSESTree.Identifier>
): void {
const startingIdentifier = extractStartingIdentifier(path);
if (startingIdentifier === undefined || !basePathNames.has(startingIdentifier)) {
context.report({ loc: path.loc, messageId: 'isNotPrefixedWithBasePath' });
}
}

function checkLiteral(context: RuleContext, path: TSESTree.Literal): void {
const absolutePathRegex = /^(?:[+a-z]+:)?\/\//i;
if (!absolutePathRegex.test(path.value?.toString() ?? '')) {
context.report({ loc: path.loc, messageId: 'isNotPrefixedWithBasePath' });
}
}

function extractStartingIdentifier(
templateLiteral: TSESTree.TemplateLiteral
): TSESTree.Identifier | undefined {
const literalParts = [...templateLiteral.expressions, ...templateLiteral.quasis].sort((a, b) =>
a.range[0] < b.range[0] ? -1 : 1
);
for (const part of literalParts) {
if (part.type === 'TemplateElement' && part.value.raw === '') {
// Skip empty quasi in the begining
continue;
}
if (part.type === 'Identifier') {
return part;
}
return undefined;
}
return undefined;
}

function extractGotoReferences(referenceTracker: ReferenceTracker): TSESTree.CallExpression[] {
return Array.from(
referenceTracker.iterateEsmReferences({
'$app/navigation': {
[ReferenceTracker.ESM]: true,
goto: {
[ReferenceTracker.CALL]: true
}
}
}),
({ node }) => node
);
}

function extractBasePathReferences(
referenceTracker: ReferenceTracker,
context: RuleContext
): Set<TSESTree.Identifier> {
const set = new Set<TSESTree.Identifier>();
for (const { node } of referenceTracker.iterateEsmReferences({
'$app/paths': {
[ReferenceTracker.ESM]: true,
base: {
[ReferenceTracker.READ]: true
}
}
})) {
const variable = findVariable(context, (node as TSESTree.ImportSpecifier).local);
if (!variable) continue;
for (const reference of variable.references) {
if (reference.identifier.type === 'Identifier') set.add(reference.identifier);
}
}
return set;
}
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export type RuleCategory =
| 'Best Practices'
| 'Stylistic Issues'
| 'Extension Rules'
| 'SvelteKit'
| 'Experimental'
| 'System';

Expand Down
2 changes: 2 additions & 0 deletions src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import noDupeUseDirectives from '../rules/no-dupe-use-directives';
import noDynamicSlotName from '../rules/no-dynamic-slot-name';
import noExportLoadInSvelteModuleInKitPages from '../rules/no-export-load-in-svelte-module-in-kit-pages';
import noExtraReactiveCurlies from '../rules/no-extra-reactive-curlies';
import noGotoWithoutBase from '../rules/no-goto-without-base';
import noIgnoredUnsubscribe from '../rules/no-ignored-unsubscribe';
import noImmutableReactiveStatements from '../rules/no-immutable-reactive-statements';
import noInlineStyles from '../rules/no-inline-styles';
Expand Down Expand Up @@ -90,6 +91,7 @@ export const rules = [
noDynamicSlotName,
noExportLoadInSvelteModuleInKitPages,
noExtraReactiveCurlies,
noGotoWithoutBase,
noIgnoredUnsubscribe,
noImmutableReactiveStatements,
noInlineStyles,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- message: Found a goto() call with a url that isn't prefixed with the base path.
line: 4
column: 8
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import { goto as alias } from '$app/navigation';

alias('/foo');
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
- message: Found a goto() call with a url that isn't prefixed with the base path.
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import { base } from '$app/paths';
import { goto } from '$app/navigation';

// eslint-disable-next-line prefer-template -- Testing both variants
goto('/foo/' + base);
goto(`/foo/${base}`);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- message: Found a goto() call with a url that isn't prefixed with the base path.
line: 4
column: 7
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
import { goto } from '$app/navigation';

goto('/foo');
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
import { goto } from '$app/navigation';

goto('http://localhost/foo/');
goto('https://localhost/foo/');
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import { base as alias } from '$app/paths';
import { goto } from '$app/navigation';

// eslint-disable-next-line prefer-template -- Testing both variants
goto(alias + '/foo/');
goto(`${alias}/foo/`);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import { base } from '$app/paths';
import { goto } from '$app/navigation';

// eslint-disable-next-line prefer-template -- Testing both variants
goto(base + '/foo/');
goto(`${base}/foo/`);
</script>
12 changes: 12 additions & 0 deletions tests/src/rules/no-goto-without-base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { RuleTester } from '../../utils/eslint-compat';
import rule from '../../../src/rules/no-goto-without-base';
import { loadTestCases } from '../../utils/utils';

const tester = new RuleTester({
languageOptions: {
ecmaVersion: 2020,
sourceType: 'module'
}
});

tester.run('no-goto-without-base', rule as any, loadTestCases('no-goto-without-base'));
2 changes: 2 additions & 0 deletions tools/render-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const categories = [
'Best Practices',
'Stylistic Issues',
'Extension Rules',
'SvelteKit',
'Experimental',
'System'
] as const;
Expand All @@ -18,6 +19,7 @@ const descriptions: Record<(typeof categories)[number], string> = {
'Stylistic Issues': 'These rules relate to style guidelines, and are therefore quite subjective:',
'Extension Rules':
'These rules extend the rules provided by ESLint itself, or other plugins to work well in Svelte:',
SvelteKit: 'These rules relate to SvelteKit and its best Practices.',
Experimental:
':warning: These rules are considered experimental and may change or be removed in the future:',
System: 'These rules relate to this plugin works:'
Expand Down