From f123e5830ea2a74f25a5a990413552284030938c Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 14:23:12 -0600 Subject: [PATCH 1/6] feat(recommended)!: add four rules to recommended --- src/configs/recommended.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/configs/recommended.ts b/src/configs/recommended.ts index 61ffca1e..5f6e53fd 100644 --- a/src/configs/recommended.ts +++ b/src/configs/recommended.ts @@ -19,8 +19,12 @@ export const createRecommendedConfig = ( 'rxjs-x/no-redundant-notify': 'error', 'rxjs-x/no-sharereplay': 'error', 'rxjs-x/no-subject-unsubscribe': 'error', + 'rxjs-x/no-topromise': 'error', 'rxjs-x/no-unbound-methods': 'error', 'rxjs-x/no-unsafe-subject-next': 'error', 'rxjs-x/no-unsafe-takeuntil': 'error', + 'rxjs-x/prefer-observer': 'error', + 'rxjs-x/prefer-root-operators': 'error', + 'rxjs-x/throw-error': 'error', }, } satisfies TSESLint.FlatConfig.Config); From 817341d6866fbf19d52e4df3be56b7522da699f8 Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 14:24:57 -0600 Subject: [PATCH 2/6] chore!: bump rxjs peer dependency to 7.2.0 BREAKING CHANGE: now that prefer-root-operators is a recommended rule, we need to enforce 7.2.0 because that's when root operators were released. --- package.json | 2 +- yarn.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 65f9c7b3..4418eee0 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ }, "peerDependencies": { "eslint": "^8.57.0 || ^9.0.0", - "rxjs": ">=7.0.0", + "rxjs": ">=7.2.0", "typescript": ">=4.7.4" }, "peerDependenciesMeta": { diff --git a/yarn.lock b/yarn.lock index fa82bb7e..9744f9fc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2202,7 +2202,7 @@ __metadata: vitest: "npm:^2.1.5" peerDependencies: eslint: ^8.57.0 || ^9.0.0 - rxjs: ">=7.0.0" + rxjs: ">=7.2.0" typescript: ">=4.7.4" peerDependenciesMeta: rxjs: From b4264039f065931b561070164241cfa2a4117f7c Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 14:28:21 -0600 Subject: [PATCH 3/6] docs(recommended): generate docs for changes --- README.md | 8 ++++---- docs/rules/no-topromise.md | 2 +- docs/rules/prefer-observer.md | 2 +- docs/rules/prefer-root-operators.md | 2 +- docs/rules/throw-error.md | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 5e1bd97e..b2b8e189 100644 --- a/README.md +++ b/README.md @@ -107,16 +107,16 @@ The package includes the following rules. | [no-subject-value](docs/rules/no-subject-value.md) | Disallow accessing the `value` property of a `BehaviorSubject` instance. | | | | 💭 | | | [no-subscribe-handlers](docs/rules/no-subscribe-handlers.md) | Disallow passing handlers to `subscribe`. | | | | 💭 | | | [no-tap](docs/rules/no-tap.md) | Disallow the `tap` operator. | | | | | ❌ | -| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | 🔒 | | 💡 | 💭 | | +| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | ✅ 🔒 | | 💡 | 💭 | | | [no-unbound-methods](docs/rules/no-unbound-methods.md) | Disallow passing unbound methods. | ✅ 🔒 | | | 💭 | | | [no-unsafe-catch](docs/rules/no-unsafe-catch.md) | Disallow unsafe `catchError` usage in effects and epics. | | | | 💭 | | | [no-unsafe-first](docs/rules/no-unsafe-first.md) | Disallow unsafe `first`/`take` usage in effects and epics. | | | | 💭 | | | [no-unsafe-subject-next](docs/rules/no-unsafe-subject-next.md) | Disallow unsafe optional `next` calls. | ✅ 🔒 | | | 💭 | | | [no-unsafe-switchmap](docs/rules/no-unsafe-switchmap.md) | Disallow unsafe `switchMap` usage in effects and epics. | | | | 💭 | | | [no-unsafe-takeuntil](docs/rules/no-unsafe-takeuntil.md) | Disallow applying operators after `takeUntil`. | ✅ 🔒 | | | 💭 | | -| [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | 🔒 | 🔧 | 💡 | 💭 | | -| [prefer-root-operators](docs/rules/prefer-root-operators.md) | Disallow importing operators from `rxjs/operators`. | 🔒 | 🔧 | 💡 | | | +| [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | ✅ 🔒 | 🔧 | 💡 | 💭 | | +| [prefer-root-operators](docs/rules/prefer-root-operators.md) | Disallow importing operators from `rxjs/operators`. | ✅ 🔒 | 🔧 | 💡 | | | | [suffix-subjects](docs/rules/suffix-subjects.md) | Enforce the use of a suffix in subject identifiers. | | | | 💭 | | -| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError`. | 🔒 | | | 💭 | | +| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError`. | ✅ 🔒 | | | 💭 | | diff --git a/docs/rules/no-topromise.md b/docs/rules/no-topromise.md index 5359dce2..b62d3aa6 100644 --- a/docs/rules/no-topromise.md +++ b/docs/rules/no-topromise.md @@ -1,6 +1,6 @@ # Disallow use of the `toPromise` method (`rxjs-x/no-topromise`) -💼 This rule is enabled in the 🔒 `strict` config. +💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`. 💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/prefer-observer.md b/docs/rules/prefer-observer.md index 4b71e7a4..ffb454f1 100644 --- a/docs/rules/prefer-observer.md +++ b/docs/rules/prefer-observer.md @@ -1,6 +1,6 @@ # Disallow passing separate handlers to `subscribe` and `tap` (`rxjs-x/prefer-observer`) -💼 This rule is enabled in the 🔒 `strict` config. +💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`. 🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/prefer-root-operators.md b/docs/rules/prefer-root-operators.md index 8cff5fab..1e6c33cf 100644 --- a/docs/rules/prefer-root-operators.md +++ b/docs/rules/prefer-root-operators.md @@ -1,6 +1,6 @@ # Disallow importing operators from `rxjs/operators` (`rxjs-x/prefer-root-operators`) -💼 This rule is enabled in the 🔒 `strict` config. +💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`. 🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/throw-error.md b/docs/rules/throw-error.md index 7c3a98b2..8f4f91f8 100644 --- a/docs/rules/throw-error.md +++ b/docs/rules/throw-error.md @@ -1,6 +1,6 @@ # Enforce passing only `Error` values to `throwError` (`rxjs-x/throw-error`) -💼 This rule is enabled in the 🔒 `strict` config. +💼 This rule is enabled in the following configs: ✅ `recommended`, 🔒 `strict`. 💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). From 93db34c8d06d75984416e7e8a146b0016b9b5053 Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 14:32:34 -0600 Subject: [PATCH 4/6] feat: correctly flag dual recommended rules Also fix test to ensure this doesn't get missed again. --- src/rules/no-topromise.ts | 2 +- src/rules/prefer-observer.ts | 2 +- src/rules/prefer-root-operators.ts | 2 +- src/rules/throw-error.ts | 1 + tests/package.test.ts | 3 +++ 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/rules/no-topromise.ts b/src/rules/no-topromise.ts index d508e9d0..ee9dde70 100644 --- a/src/rules/no-topromise.ts +++ b/src/rules/no-topromise.ts @@ -7,7 +7,7 @@ export const noTopromiseRule = ruleCreator({ meta: { docs: { description: 'Disallow use of the `toPromise` method.', - recommended: 'strict', + recommended: 'recommended', requiresTypeChecking: true, }, hasSuggestions: true, diff --git a/src/rules/prefer-observer.ts b/src/rules/prefer-observer.ts index 9efb6fde..0c29c5c4 100644 --- a/src/rules/prefer-observer.ts +++ b/src/rules/prefer-observer.ts @@ -20,7 +20,7 @@ export const preferObserverRule = ruleCreator({ docs: { description: 'Disallow passing separate handlers to `subscribe` and `tap`.', - recommended: 'strict', + recommended: 'recommended', requiresTypeChecking: true, }, fixable: 'code', diff --git a/src/rules/prefer-root-operators.ts b/src/rules/prefer-root-operators.ts index e62989d7..60c69ecf 100644 --- a/src/rules/prefer-root-operators.ts +++ b/src/rules/prefer-root-operators.ts @@ -22,7 +22,7 @@ export const preferRootOperatorsRule = ruleCreator({ meta: { docs: { description: 'Disallow importing operators from `rxjs/operators`.', - recommended: 'strict', + recommended: 'recommended', }, fixable: 'code', hasSuggestions: true, diff --git a/src/rules/throw-error.ts b/src/rules/throw-error.ts index 0baab379..e63e027c 100644 --- a/src/rules/throw-error.ts +++ b/src/rules/throw-error.ts @@ -16,6 +16,7 @@ export const throwErrorRule = ruleCreator({ description: 'Enforce passing only `Error` values to `throwError`.', recommended: { + recommended: true, strict: [{ allowThrowingAny: false, allowThrowingUnknown: false }], }, requiresTypeChecking: true, diff --git a/tests/package.test.ts b/tests/package.test.ts index 07624399..69f6f88f 100644 --- a/tests/package.test.ts +++ b/tests/package.test.ts @@ -70,6 +70,7 @@ describe('package', () => { } else if (ruleRec === 'strict') { expect(strictRules).toHaveProperty(fullRuleName); expect(strictRules[fullRuleName as keyof typeof strictRules]).toBe('error'); + expect(recommendedRules).not.toHaveProperty(fullRuleName); } else { expect.fail(`Invalid recommended value for rule ${fullRuleName}: ${ruleRec}`); } @@ -77,6 +78,8 @@ describe('package', () => { // Rule is part of several configs. if (ruleRec.recommended) { expect(recommendedRules).toHaveProperty(fullRuleName); + } else { + expect(recommendedRules).not.toHaveProperty(fullRuleName); } expect(strictRules).toHaveProperty(fullRuleName); expect(strictRules[fullRuleName as keyof typeof strictRules]).toBeInstanceOf(Array); From 992c52a71074b3997ad8aac29f5609f66f5f93fc Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 15:14:33 -0600 Subject: [PATCH 5/6] test: split out rule config test per-rule using `it.for` --- tests/package.test.ts | 61 +++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/tests/package.test.ts b/tests/package.test.ts index 69f6f88f..05013770 100644 --- a/tests/package.test.ts +++ b/tests/package.test.ts @@ -46,45 +46,38 @@ describe('package', () => { } }); - it('has rules flagged according to their configs', () => { - if (!plugin.configs) { - expect.fail('No configs found.'); - } - + it.for(Object.keys(plugin.rules))('includes rule %s in configurations based on meta.docs.recommended', (ruleName, { expect }) => { + const rule = plugin.rules[ruleName as keyof typeof plugin.rules]; const namespace = 'rxjs-x'; - const recommendedRules = plugin.configs.recommended.rules; - const strictRules = plugin.configs.strict.rules; + const fullRuleName = `${namespace}/${ruleName}`; - for (const [ruleName, rule] of Object.entries(plugin.rules)) { - const fullRuleName = `${namespace}/${ruleName}`; - const ruleRec = rule.meta.docs?.recommended; + if (!rule.meta.docs?.recommended) { + // Rule is not included in any configuration. + expect(plugin.configs.recommended.rules).not.toHaveProperty(fullRuleName); + expect(plugin.configs.strict.rules).not.toHaveProperty(fullRuleName); + } else if (typeof rule.meta.docs.recommended === 'string') { + // Rule specifies only a configuration name. + expect(rule.meta.docs.recommended).toMatch(/^(recommended|strict)$/); + if (rule.meta.docs.recommended === 'recommended') { + expect(plugin.configs.recommended.rules).toHaveProperty(fullRuleName); + } else { + expect(plugin.configs.recommended.rules).not.toHaveProperty(fullRuleName); + } - if (!ruleRec) { - // Rule is not part of any config. - expect(recommendedRules).not.toHaveProperty(fullRuleName); - expect(strictRules).not.toHaveProperty(fullRuleName); - } else if (typeof ruleRec === 'string') { - // Rule is part of a single config. - if (ruleRec === 'recommended') { - expect(recommendedRules).toHaveProperty(fullRuleName); - } else if (ruleRec === 'strict') { - expect(strictRules).toHaveProperty(fullRuleName); - expect(strictRules[fullRuleName as keyof typeof strictRules]).toBe('error'); - expect(recommendedRules).not.toHaveProperty(fullRuleName); - } else { - expect.fail(`Invalid recommended value for rule ${fullRuleName}: ${ruleRec}`); - } + // Strict configuration always includes all recommended rules. + expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName); + // Not allowed to specify non-default options if rule only specifies a configuration name. + expect(typeof plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules]).toBe('string'); + } else { + // Rule specifies non-default options for strict. + if (rule.meta.docs.recommended.recommended) { + expect(plugin.configs.recommended.rules).toHaveProperty(fullRuleName); } else { - // Rule is part of several configs. - if (ruleRec.recommended) { - expect(recommendedRules).toHaveProperty(fullRuleName); - } else { - expect(recommendedRules).not.toHaveProperty(fullRuleName); - } - expect(strictRules).toHaveProperty(fullRuleName); - expect(strictRules[fullRuleName as keyof typeof strictRules]).toBeInstanceOf(Array); - expect(strictRules[fullRuleName as keyof typeof strictRules][1]).toEqual(ruleRec.strict[0]); + expect(plugin.configs.recommended.rules).not.toHaveProperty(fullRuleName); } + expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName); + expect(plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules]).toBeInstanceOf(Array); + expect(plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules][1]).toEqual(rule.meta.docs.recommended.strict[0]); } }); }); From f5010e8eaa57ae207c3d38c0a0da965db59d60a9 Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Tue, 26 Nov 2024 15:38:36 -0600 Subject: [PATCH 6/6] test: use toHaveProperty 2nd param to simplify test --- tests/package.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/package.test.ts b/tests/package.test.ts index 05013770..aa388512 100644 --- a/tests/package.test.ts +++ b/tests/package.test.ts @@ -65,9 +65,8 @@ describe('package', () => { } // Strict configuration always includes all recommended rules. - expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName); - // Not allowed to specify non-default options if rule only specifies a configuration name. - expect(typeof plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules]).toBe('string'); + // Not allowed to specify non-default options since rule only specifies a configuration name. + expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName, expect.any(String)); } else { // Rule specifies non-default options for strict. if (rule.meta.docs.recommended.recommended) { @@ -75,9 +74,7 @@ describe('package', () => { } else { expect(plugin.configs.recommended.rules).not.toHaveProperty(fullRuleName); } - expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName); - expect(plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules]).toBeInstanceOf(Array); - expect(plugin.configs.strict.rules[fullRuleName as keyof typeof plugin.configs.strict.rules][1]).toEqual(rule.meta.docs.recommended.strict[0]); + expect(plugin.configs.strict.rules).toHaveProperty(fullRuleName, [expect.any(String), rule.meta.docs.recommended.strict[0]]); } }); });