feat: Adding a new no-ignored-unsubscribe rule. (#592)

Co-authored-by: Yosuke Ota <[email protected]>

Author: moufmouf

.changeset/hip-tips-smell.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": minor
+---
+
+feat: add new `svelte/no-ignored-unsubscribe` rule.

README.md

@@ -340,6 +340,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [svelte/block-lang](https://sveltejs.github.io/eslint-plugin-svelte/rules/block-lang/) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. |  |
 | [svelte/button-has-type](https://sveltejs.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute |  |
 | [svelte/no-at-debug-tags](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
+| [svelte/no-ignored-unsubscribe](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-ignored-unsubscribe/) | disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores. |  |
 | [svelte/no-immutable-reactive-statements](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-immutable-reactive-statements/) | disallow reactive statements that don't reference reactive values. |  |
 | [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
 | [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |

docs/rules.md

@@ -53,6 +53,7 @@ These rules relate to better ways of doing things to help you avoid problems:
 | [svelte/block-lang](./rules/block-lang.md)                                               | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. |          |
 | [svelte/button-has-type](./rules/button-has-type.md)                                     | disallow usage of button without an explicit type attribute                                                                               |          |
 | [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md)                                   | disallow the use of `{@debug}`                                                                                                            | :star:   |
+| [svelte/no-ignored-unsubscribe](./rules/no-ignored-unsubscribe.md)                       | disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.                                                  |          |
 | [svelte/no-immutable-reactive-statements](./rules/no-immutable-reactive-statements.md)   | disallow reactive statements that don't reference reactive values.                                                                        |          |
 | [svelte/no-reactive-functions](./rules/no-reactive-functions.md)                         | it's not necessary to define functions in reactive statements                                                                             | :bulb:   |
 | [svelte/no-reactive-literals](./rules/no-reactive-literals.md)                           | don't assign literal values in reactive statements                                                                                        | :bulb:   |

docs/rules/no-ignored-unsubscribe.md

@@ -0,0 +1,48 @@
+---
+pageClass: 'rule-details'
+sidebarDepth: 0
+title: 'svelte/no-ignored-unsubscribe'
+description: 'disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.'
+---
+
+# svelte/no-ignored-unsubscribe
+
+> disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.
+
+- :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 fails if an "unsubscriber" returned by call to `subscribe()` is neither assigned to a variable or property or passed to a function.
+
+One should always unsubscribe from a store when it is no longer needed. Otherwise, the subscription will remain active and constitute a **memory leak**.
+This rule helps to find such cases by ensuring that the unsubscriber (the return value from the store's `subscribe` method) is not ignored.
+
+<ESLintCodeBlock>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-ignored-unsubscribe: "error" */
+
+  import myStore from './my-stores';
+
+  /* ✓ GOOD */
+  const unsubscribe = myStore.subscribe(() => {});
+
+  /* ✗ BAD */
+  myStore.subscribe(() => {});
+</script>
+```
+
+</ESLintCodeBlock>
+
+## :wrench: Options
+
+Nothing.
+
+## :mag: Implementation
+
+- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-ignored-unsubscribe.ts)
+- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-ignored-unsubscribe.ts)

src/rules/no-ignored-unsubscribe.ts

@@ -0,0 +1,32 @@
+import type { TSESTree } from '@typescript-eslint/types';
+import { createRule } from '../utils';
+
+export default createRule('no-ignored-unsubscribe', {
+	meta: {
+		docs: {
+			description:
+				'disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.',
+			category: 'Best Practices',
+			recommended: false
+		},
+		fixable: undefined,
+		hasSuggestions: false,
+		messages: {
+			forbidden: 'Ignoring returned value of the subscribe method is forbidden.'
+		},
+		schema: [],
+		type: 'problem'
+	},
+	create: (context) => {
+		return {
+			"ExpressionStatement > CallExpression > MemberExpression.callee[property.name='subscribe']": (
+				node: TSESTree.MemberExpression
+			) => {
+				context.report({
+					messageId: 'forbidden',
+					node: node.property
+				});
+			}
+		};
+	}
+});

src/utils/rules.ts

@@ -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 noIgnoredUnsubscribe from '../rules/no-ignored-unsubscribe';
 import noImmutableReactiveStatements from '../rules/no-immutable-reactive-statements';
 import noInnerDeclarations from '../rules/no-inner-declarations';
 import noNotFunctionHandler from '../rules/no-not-function-handler';
@@ -88,6 +89,7 @@ export const rules = [
 	noDynamicSlotName,
 	noExportLoadInSvelteModuleInKitPages,
 	noExtraReactiveCurlies,
+	noIgnoredUnsubscribe,
 	noImmutableReactiveStatements,
 	noInnerDeclarations,
 	noNotFunctionHandler,

tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-errors.yaml

@@ -0,0 +1,4 @@
+- message: Ignoring returned value of the subscribe method is forbidden.
+  line: 6
+  column: 6
+  suggestions: null

tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-input.svelte

@@ -0,0 +1,9 @@
+<script>
+	import { writable } from 'svelte/store';
+
+	const foo = writable(0);
+
+	foo.subscribe(() => {
+		console.log('foo changed');
+	});
+</script>

tests/fixtures/rules/no-ignored-unsubscribe/valid/test01-input.svelte

@@ -0,0 +1,9 @@
+<script>
+	import { writable } from 'svelte/store';
+
+	const foo = writable(0);
+
+	const unsubscribe = foo.subscribe(() => {
+		console.log('foo changed');
+	});
+</script>

tests/fixtures/rules/no-ignored-unsubscribe/valid/test02-input.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { writable } from 'svelte/store';
+
+	const foo = writable(0);
+
+	const unsubscribers = [];
+	unsubscribers.push(
+		foo.subscribe(() => {
+			console.log('foo changed');
+		})
+	);
+</script>