From 1fe38d770928bb890d4292e6a10028b93d9ba843 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20N=C3=A9grier?= <d.negrier@thecodingmachine.com>
Date: Wed, 4 Oct 2023 10:57:09 +0200
Subject: [PATCH] feat: Adding a new no-ignored-unsubscribe rule. (#592)

Co-authored-by: Yosuke Ota <otameshiyo23@gmail.com>
---
 .changeset/hip-tips-smell.md                  |  5 ++
 README.md                                     |  1 +
 docs/rules.md                                 |  1 +
 docs/rules/no-ignored-unsubscribe.md          | 48 +++++++++++++++++++
 src/rules/no-ignored-unsubscribe.ts           | 32 +++++++++++++
 src/utils/rules.ts                            |  2 +
 .../invalid/test01-errors.yaml                |  4 ++
 .../invalid/test01-input.svelte               |  9 ++++
 .../valid/test01-input.svelte                 |  9 ++++
 .../valid/test02-input.svelte                 | 12 +++++
 .../valid/test03-input.svelte                 |  3 ++
 tests/src/rules/no-ignored-unsubscribe.ts     | 12 +++++
 12 files changed, 138 insertions(+)
 create mode 100644 .changeset/hip-tips-smell.md
 create mode 100644 docs/rules/no-ignored-unsubscribe.md
 create mode 100644 src/rules/no-ignored-unsubscribe.ts
 create mode 100644 tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-errors.yaml
 create mode 100644 tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-input.svelte
 create mode 100644 tests/fixtures/rules/no-ignored-unsubscribe/valid/test01-input.svelte
 create mode 100644 tests/fixtures/rules/no-ignored-unsubscribe/valid/test02-input.svelte
 create mode 100644 tests/fixtures/rules/no-ignored-unsubscribe/valid/test03-input.svelte
 create mode 100644 tests/src/rules/no-ignored-unsubscribe.ts

diff --git a/.changeset/hip-tips-smell.md b/.changeset/hip-tips-smell.md
new file mode 100644
index 000000000..60dd0cd3b
--- /dev/null
+++ b/.changeset/hip-tips-smell.md
@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": minor
+---
+
+feat: add new `svelte/no-ignored-unsubscribe` rule.
diff --git a/README.md b/README.md
index 80f6f903f..7df272aa1 100644
--- a/README.md
+++ b/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: |
diff --git a/docs/rules.md b/docs/rules.md
index d35419cad..1f982942b 100644
--- a/docs/rules.md
+++ b/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:   |
diff --git a/docs/rules/no-ignored-unsubscribe.md b/docs/rules/no-ignored-unsubscribe.md
new file mode 100644
index 000000000..caabed8c2
--- /dev/null
+++ b/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)
diff --git a/src/rules/no-ignored-unsubscribe.ts b/src/rules/no-ignored-unsubscribe.ts
new file mode 100644
index 000000000..471f38a36
--- /dev/null
+++ b/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
+				});
+			}
+		};
+	}
+});
diff --git a/src/utils/rules.ts b/src/utils/rules.ts
index f1181de8d..c496dec7b 100644
--- a/src/utils/rules.ts
+++ b/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,
diff --git a/tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-errors.yaml b/tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-errors.yaml
new file mode 100644
index 000000000..9b0c1ee4c
--- /dev/null
+++ b/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
diff --git a/tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-input.svelte b/tests/fixtures/rules/no-ignored-unsubscribe/invalid/test01-input.svelte
new file mode 100644
index 000000000..47f230d91
--- /dev/null
+++ b/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>
diff --git a/tests/fixtures/rules/no-ignored-unsubscribe/valid/test01-input.svelte b/tests/fixtures/rules/no-ignored-unsubscribe/valid/test01-input.svelte
new file mode 100644
index 000000000..f4f68b2df
--- /dev/null
+++ b/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>
diff --git a/tests/fixtures/rules/no-ignored-unsubscribe/valid/test02-input.svelte b/tests/fixtures/rules/no-ignored-unsubscribe/valid/test02-input.svelte
new file mode 100644
index 000000000..e255f63c4
--- /dev/null
+++ b/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>
diff --git a/tests/fixtures/rules/no-ignored-unsubscribe/valid/test03-input.svelte b/tests/fixtures/rules/no-ignored-unsubscribe/valid/test03-input.svelte
new file mode 100644
index 000000000..19615225d
--- /dev/null
+++ b/tests/fixtures/rules/no-ignored-unsubscribe/valid/test03-input.svelte
@@ -0,0 +1,3 @@
+<script>
+	a(foo.subscribe);
+</script>
diff --git a/tests/src/rules/no-ignored-unsubscribe.ts b/tests/src/rules/no-ignored-unsubscribe.ts
new file mode 100644
index 000000000..2d3bd2d3e
--- /dev/null
+++ b/tests/src/rules/no-ignored-unsubscribe.ts
@@ -0,0 +1,12 @@
+import { RuleTester } from 'eslint';
+import rule from '../../../src/rules/no-ignored-unsubscribe';
+import { loadTestCases } from '../../utils/utils';
+
+const tester = new RuleTester({
+	parserOptions: {
+		ecmaVersion: 2020,
+		sourceType: 'module'
+	}
+});
+
+tester.run('no-ignored-unsubscribe', rule as any, loadTestCases('no-ignored-unsubscribe'));