diff --git a/README.md b/README.md index 7b819887..c7ce70e4 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ The package includes the following rules. | Name                       | Description | 💼 | 🔧 | 💡 | 💭 | ❌ | | :--------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------- | :- | :- | :- | :- | :- | | [ban-observables](docs/rules/ban-observables.md) | Disallow banned observable creators. | | | | | | -| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | | | +| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | 💭 | | | [finnish](docs/rules/finnish.md) | Enforce Finnish notation. | | | | 💭 | | | [just](docs/rules/just.md) | Require the use of `just` instead of `of`. | | 🔧 | | | | | [macro](docs/rules/macro.md) | Require the use of the RxJS Tools Babel macro. | | 🔧 | | | ❌ | diff --git a/docs/rules/ban-operators.md b/docs/rules/ban-operators.md index 372ec147..38d824c2 100644 --- a/docs/rules/ban-operators.md +++ b/docs/rules/ban-operators.md @@ -1,5 +1,7 @@ # Disallow banned operators (`rxjs-x/ban-operators`) +💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). + This rule can be configured so that developers can ban any operators they want to avoid in their project. diff --git a/src/rules/ban-operators.ts b/src/rules/ban-operators.ts index dfd522c9..2c232a16 100644 --- a/src/rules/ban-operators.ts +++ b/src/rules/ban-operators.ts @@ -1,5 +1,6 @@ -import { AST_NODE_TYPES, TSESTree as es } from '@typescript-eslint/utils'; +import { TSESTree as es } from '@typescript-eslint/utils'; import { stripIndent } from 'common-tags'; +import { getTypeServices } from '../etc'; import { ruleCreator } from '../utils'; const defaultOptions: readonly Record[] = []; @@ -9,6 +10,7 @@ export const banOperatorsRule = ruleCreator({ meta: { docs: { description: 'Disallow banned operators.', + requiresTypeChecking: true, }, messages: { forbidden: 'RxJS operator is banned: {{name}}{{explanation}}.', @@ -25,7 +27,8 @@ export const banOperatorsRule = ruleCreator({ }, name: 'ban-operators', create: (context) => { - const bans: { explanation: string; regExp: RegExp }[] = []; + const { couldBeType } = getTypeServices(context); + const bans: { name: string; explanation: string }[] = []; const [config] = context.options; if (!config) { @@ -35,39 +38,34 @@ export const banOperatorsRule = ruleCreator({ Object.entries(config).forEach(([key, value]) => { if (value !== false) { bans.push({ + name: key, explanation: typeof value === 'string' ? value : '', - regExp: new RegExp(`^${key}$`), }); } }); - function getFailure(name: string) { - for (let b = 0, length = bans.length; b < length; ++b) { - const ban = bans[b]; - if (ban.regExp.test(name)) { + function checkNode(node: es.Node) { + for (const ban of bans) { + if (couldBeType(node, ban.name, { name: /[/\\]rxjs[/\\]/ })) { const explanation = ban.explanation ? `: ${ban.explanation}` : ''; - return { + context.report({ messageId: 'forbidden', - data: { name, explanation }, - } as const; + data: { name: ban.name, explanation }, + node, + }); + return; } } - return undefined; } return { - [String.raw`ImportDeclaration[source.value=/^rxjs\u002foperators$/] > ImportSpecifier`]: - (node: es.ImportSpecifier) => { - const identifier = node.imported; - const name = identifier.type === AST_NODE_TYPES.Identifier ? identifier.name : identifier.value; - const failure = getFailure(name); - if (failure) { - context.report({ - ...failure, - node: identifier, - }); - } - }, + 'CallExpression[callee.property.name=\'pipe\'] > CallExpression[callee.name]': (node: es.CallExpression) => { + checkNode(node.callee); + }, + 'CallExpression[callee.property.name=\'pipe\'] > CallExpression[callee.type="MemberExpression"]': (node: es.CallExpression) => { + const callee = node.callee as es.MemberExpression; + checkNode(callee.property); + }, }; }, }); diff --git a/tests/rules/ban-operators.test.ts b/tests/rules/ban-operators.test.ts index ed346516..ac62ec3b 100644 --- a/tests/rules/ban-operators.test.ts +++ b/tests/rules/ban-operators.test.ts @@ -3,34 +3,122 @@ import { banOperatorsRule } from '../../src/rules/ban-operators'; import { fromFixture } from '../etc'; import { ruleTester } from '../rule-tester'; -ruleTester({ types: false }).run('ban-operators', banOperatorsRule, { +ruleTester({ types: true }).run('ban-operators', banOperatorsRule, { valid: [ { - code: `import { concat, merge as m, mergeMap as mm } from "rxjs/operators";`, + code: stripIndent` + // root import + import { of, concat, merge as m, mergeMap as mm } from "rxjs"; + + of('a').pipe(concat(of('b'))); + of(1).pipe(m(of(2))); + of('a').pipe(mm(x => of(x + '1'))); + `, + options: [{}], + }, + { + code: stripIndent` + // namespace import + import * as Rx from "rxjs"; + + Rx.of('a').pipe(Rx.concat(Rx.of('b'))); + Rx.of(1).pipe(Rx.merge(Rx.of(2))); + Rx.of('a').pipe(Rx.mergeMap(x => Rx.of(x + '1'))); + `, + options: [{}], }, { - code: `import { concat, merge as m, mergeMap as mm } from "rxjs";`, + code: stripIndent` + // operators path import (deprecated) + import { of, concat, merge as m, mergeMap as mm } from "rxjs/operators"; + + of('a').pipe(concat(of('b'))); + of(1).pipe(m(of(2))); + of('a').pipe(mm(x => of(x + '1'))); + `, + options: [{}], + }, + stripIndent` + // no options + import { of, concat } from "rxjs"; + + of('a').pipe(concat(of('b'))); + `, + { + code: stripIndent` + // non-RxJS operator + import { of } from "rxjs"; + + function concat() {} + + of('a').pipe(concat()); + `, + options: [{ concat: true }], }, { - // This won't effect errors, because only imports from "rxjs/operators" - // are checked. To support banning operators from "rxjs", it'll need to - // check types. - code: `import { concat, merge as m, mergeMap as mm } from "rxjs";`, - options: [ - { - concat: true, - merge: 'because I say so', - mergeMap: false, - }, - ], + code: stripIndent` + // only within pipe + import { of, concat } from "rxjs"; + + // For performance reasons, we don't lint operators used outside of pipe. + concat(of('a')); + `, + options: [{ concat: true }], }, ], invalid: [ fromFixture( stripIndent` - import { concat, merge as m, mergeMap as mm } from "rxjs/operators"; - ~~~~~~ [forbidden { "name": "concat", "explanation": "" }] - ~~~~~ [forbidden { "name": "merge", "explanation": ": because I say so" }] + // root import + import { of, concat, merge as m, mergeMap as mm } from "rxjs"; + + of('a').pipe(concat(of('b'))); + ~~~~~~ [forbidden { "name": "concat", "explanation": "" }] + of(1).pipe(m(of(2))); + ~ [forbidden { "name": "merge", "explanation": ": because I say so" }] + of('a').pipe(mm(x => of(x + '1'))); + `, + { + options: [ + { + concat: true, + merge: 'because I say so', + mergeMap: false, + }, + ], + }, + ), + fromFixture( + stripIndent` + // namespace import + import * as Rx from "rxjs"; + + Rx.of('a').pipe(Rx.concat(Rx.of('b'))); + ~~~~~~ [forbidden { "name": "concat", "explanation": "" }] + Rx.of(1).pipe(Rx.merge(Rx.of(2))); + ~~~~~ [forbidden { "name": "merge", "explanation": "" }] + Rx.of('a').pipe(Rx.mergeMap(x => Rx.of(x + '1'))); + `, + { + options: [ + { + concat: true, + merge: true, + mergeMap: false, + }, + ], + }, + ), + fromFixture( + stripIndent` + // operators path import (deprecated) + import { of, concat, merge as m, mergeMap as mm } from "rxjs/operators"; + + of('a').pipe(concat(of('b'))); + ~~~~~~ [forbidden { "name": "concat", "explanation": "" }] + of(1).pipe(m(of(2))); + ~ [forbidden { "name": "merge", "explanation": ": because I say so" }] + of('a').pipe(mm(x => of(x + '1'))); `, { options: [