Skip to content
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

Add rule for prevent from printing undefined to HTML #807

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions .node-version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
18.16.1
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
| [svelte/no-reactive-reassign](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-reassign/) | disallow reassigning reactive values | |
| [svelte/no-shorthand-style-property-overrides](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shorthand-style-property-overrides/) | disallow shorthand style properties that override related longhand properties | :star: |
| [svelte/no-store-async](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | |
| [svelte/no-undefined-print](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-undefined-print/) | Disallow from printing `undefined` | :star: |
| [svelte/no-unknown-style-directive-property](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: |
| [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | |
| [svelte/require-store-reactive-access](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ These rules relate to possible syntax or logic errors in Svelte code:
| [svelte/no-reactive-reassign](./rules/no-reactive-reassign.md) | disallow reassigning reactive values | |
| [svelte/no-shorthand-style-property-overrides](./rules/no-shorthand-style-property-overrides.md) | disallow shorthand style properties that override related longhand properties | :star: |
| [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | |
| [svelte/no-undefined-print](./rules/no-undefined-print.md) | Disallow from printing `undefined` | :star: |
| [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: |
| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | |
| [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :wrench: |
Expand Down
56 changes: 56 additions & 0 deletions docs/rules/no-undefined-print.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/no-undefined-print'
description: 'Disallow from printing `undefined`'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is grammatically better?

Suggested change
description: 'Disallow from printing `undefined`'
description: 'Disallow printing `undefined`'

since: 'v0.0.1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use pnpm update to update docs automatically.

---

# svelte/no-undefined-print

> Disallow from printing `undefined`

- :gear: This rule is included in `"plugin:svelte/recommended"`.

## :book: Rule Details

This rule reports all uses of `{@html}` in order to reduce the risk of injecting potentially unsafe / unescaped html into the browser leading to Cross-Site Scripting (XSS) attacks.

<ESLintCodeBlock>

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/no-at-html-tags: "error" */
</script>

<!-- ✓ GOOD -->
{foo}

<!-- ✗ BAD -->
{@html foo}
```
Comment on lines +17 to +33
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write proper document.


</ESLintCodeBlock>

## :wrench: Options

Nothing.

## :mute: When Not To Use It

If you are certain the content passed to `{@html}` is sanitized HTML you can disable this rule.

## :books: Further Reading

- [Svelte - Tutorial > 1. Introduction / HTML tags](https://svelte.dev/tutorial/html-tags)

## :rocket: Version

This rule was introduced in eslint-plugin-svelte v0.0.1

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-undefined-print.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-undefined-print.ts)
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const config: Linter.FlatConfig[] = [
'svelte/no-not-function-handler': 'error',
'svelte/no-object-in-text-mustaches': 'error',
'svelte/no-shorthand-style-property-overrides': 'error',
'svelte/no-undefined-print': 'error',
'svelte/no-unknown-style-directive-property': 'error',
'svelte/no-unused-svelte-ignore': 'error',
'svelte/system': 'error',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-svelte/src/configs/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const config: Linter.Config = {
'svelte/no-not-function-handler': 'error',
'svelte/no-object-in-text-mustaches': 'error',
'svelte/no-shorthand-style-property-overrides': 'error',
'svelte/no-undefined-print': 'error',
'svelte/no-unknown-style-directive-property': 'error',
'svelte/no-unused-svelte-ignore': 'error',
'svelte/system': 'error',
Expand Down
5 changes: 5 additions & 0 deletions packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ export interface RuleOptions {
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-trailing-spaces/
*/
'svelte/no-trailing-spaces'?: Linter.RuleEntry<SvelteNoTrailingSpaces>
/**
* Disallow from printing `undefined`
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-undefined-print/
*/
'svelte/no-undefined-print'?: Linter.RuleEntry<[]>
/**
* disallow unknown `style:property`
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/
Expand Down
68 changes: 68 additions & 0 deletions packages/eslint-plugin-svelte/src/rules/no-undefined-print.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import type { AST } from 'svelte-eslint-parser';
import { createRule } from '../utils';

export default createRule('no-undefined-print', {
meta: {
docs: {
description: 'Disallow from printing `undefined`',
category: 'Possible Errors',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think printing undefined is possible error.

recommended: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not add recommended rules without major version up.

Suggested change
recommended: true
recommended: false

},
schema: [],
messages: {
unexpected: 'Unexpected `undefined`.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unexpected: 'Unexpected `undefined`.'
unexpected: 'Disallow printing `undefined`'

},
type: 'problem'
},
create(context) {
return {
'SvelteMustacheTag[kind=text]'(node: AST.SvelteMustacheTag) {
if (node.expression.type === 'Identifier' && node.expression.name === 'undefined') {
context.report({
node,
messageId: 'unexpected'
});
}

if (node.expression.type === 'LogicalExpression' && node.expression.operator === '||') {
const left = node.expression.left;
const right = node.expression.right;

if (left.type === 'Identifier' && right.type === 'Literal' && right.value === undefined) {
context.report({
node,
messageId: 'unexpected'
});
}
}

if (node.expression.type === 'LogicalExpression' && node.expression.operator === '??') {
const left = node.expression.left;
const right = node.expression.right;

if (left.type === 'Identifier' && right.type === 'Literal' && right.value === undefined) {
context.report({
node,
messageId: 'unexpected'
});
}
}

if (node.expression.type === 'ConditionalExpression') {
const consequent = node.expression.consequent;
const alternate = node.expression.alternate;

if (
(consequent.type === 'Literal' && consequent.value === undefined) ||
(alternate.type === 'Literal' && alternate.value === undefined)
) {
context.report({
node,
messageId: 'unexpected'
});
}
}
}
};
}
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import noStoreAsync from '../rules/no-store-async';
import noSvelteInternal from '../rules/no-svelte-internal';
import noTargetBlank from '../rules/no-target-blank';
import noTrailingSpaces from '../rules/no-trailing-spaces';
import noUndefinedPrint from '../rules/no-undefined-print';
import noUnknownStyleDirectiveProperty from '../rules/no-unknown-style-directive-property';
import noUnusedClassName from '../rules/no-unused-class-name';
import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore';
Expand Down Expand Up @@ -109,6 +110,7 @@ export const rules = [
noSvelteInternal,
noTargetBlank,
noTrailingSpaces,
noUndefinedPrint,
noUnknownStyleDirectiveProperty,
noUnusedClassName,
noUnusedSvelteIgnore,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- message: 'Unexpected `undefined`aa.'
line: 52
column: 44
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let string = `this string contains some <strong>HTML!!!</strong>`;
</script>

<p>{@html string}</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test is not for this rule.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
let user = {
firstname: 'Ada',
lastname: 'Lovelace'
};
</script>

<input bind:value={user.firstname} />
<input bind:value={user.lastname} />

{@debug}

<h1>Hello {user.firstname}!</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
let user = {
firstname: 'Ada',
lastname: 'Lovelace'
};
</script>

<input bind:value={user.firstname} />
<input bind:value={user.lastname} />

{@debug user}

<h1>Hello {user.firstname}!</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let string;
</script>

<p>{string}</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let string = 'abc';
</script>

<p>{string}</p>
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-undefined-print';
import { loadTestCases } from '../../utils/utils';

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

tester.run('no-undefined-print', rule as any, loadTestCases('no-undefined-print'));