Skip to content

Commit 7527002

Browse files
JasonWeinzierlJasonWeinzierl
committed
feat(ban-operators)!: type check, report in place
1 parent 4bd72fd commit 7527002

File tree

4 files changed

+119
-41
lines changed

4 files changed

+119
-41
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ The package includes the following rules.
6767
| Name                       | Description | 💼 | 🔧 | 💡 | 💭 | ❌ |
6868
| :--------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------- | :- | :- | :- | :- | :- |
6969
| [ban-observables](docs/rules/ban-observables.md) | Disallow banned observable creators. | | | | | |
70-
| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | | |
70+
| [ban-operators](docs/rules/ban-operators.md) | Disallow banned operators. | | | | 💭 | |
7171
| [finnish](docs/rules/finnish.md) | Enforce Finnish notation. | | | | 💭 | |
7272
| [just](docs/rules/just.md) | Require the use of `just` instead of `of`. | | 🔧 | | | |
7373
| [macro](docs/rules/macro.md) | Require the use of the RxJS Tools Babel macro. | | 🔧 | | | ❌ |

‎docs/rules/ban-operators.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Disallow banned operators (`rxjs-x/ban-operators`)
22

3+
💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting).
4+
35
<!-- end auto-generated rule header -->
46

57
This rule can be configured so that developers can ban any operators they want to avoid in their project.

‎src/rules/ban-operators.ts‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { AST_NODE_TYPES, TSESTree as es } from '@typescript-eslint/utils';
1+
import { TSESTree as es } from '@typescript-eslint/utils';
22
import { stripIndent } from 'common-tags';
3+
import { getTypeServices } from '../etc';
34
import { ruleCreator } from '../utils';
45

56
const defaultOptions: readonly Record<string, boolean | string>[] = [];
@@ -9,6 +10,7 @@ export const banOperatorsRule = ruleCreator({
910
meta: {
1011
docs: {
1112
description: 'Disallow banned operators.',
13+
requiresTypeChecking: true,
1214
},
1315
messages: {
1416
forbidden: 'RxJS operator is banned: {{name}}{{explanation}}.',
@@ -25,7 +27,8 @@ export const banOperatorsRule = ruleCreator({
2527
},
2628
name: 'ban-operators',
2729
create: (context) => {
28-
const bans: { explanation: string; regExp: RegExp }[] = [];
30+
const { couldBeType } = getTypeServices(context);
31+
const bans: { name: string; explanation: string }[] = [];
2932

3033
const [config] = context.options;
3134
if (!config) {
@@ -35,39 +38,34 @@ export const banOperatorsRule = ruleCreator({
3538
Object.entries(config).forEach(([key, value]) => {
3639
if (value !== false) {
3740
bans.push({
41+
name: key,
3842
explanation: typeof value === 'string' ? value : '',
39-
regExp: new RegExp(`^${key}$`),
4043
});
4144
}
4245
});
4346

44-
function getFailure(name: string) {
45-
for (let b = 0, length = bans.length; b < length; ++b) {
46-
const ban = bans[b];
47-
if (ban.regExp.test(name)) {
47+
function checkNode(node: es.Node) {
48+
for (const ban of bans) {
49+
if (couldBeType(node, ban.name, { name: /[/\\]rxjs[/\\]/ })) {
4850
const explanation = ban.explanation ? `: ${ban.explanation}` : '';
49-
return {
51+
context.report({
5052
messageId: 'forbidden',
51-
data: { name, explanation },
52-
} as const;
53+
data: { name: ban.name, explanation },
54+
node,
55+
});
56+
return;
5357
}
5458
}
55-
return undefined;
5659
}
5760

5861
return {
59-
[String.raw`ImportDeclaration[source.value=/^rxjs\u002foperators$/] > ImportSpecifier`]:
60-
(node: es.ImportSpecifier) => {
61-
const identifier = node.imported;
62-
const name = identifier.type === AST_NODE_TYPES.Identifier ? identifier.name : identifier.value;
63-
const failure = getFailure(name);
64-
if (failure) {
65-
context.report({
66-
...failure,
67-
node: identifier,
68-
});
69-
}
70-
},
62+
'CallExpression[callee.name]': (node: es.CallExpression) => {
63+
checkNode(node.callee);
64+
},
65+
'CallExpression[callee.type="MemberExpression"]': (node: es.CallExpression) => {
66+
const callee = node.callee as es.MemberExpression;
67+
checkNode(callee.property);
68+
},
7169
};
7270
},
7371
});

‎tests/rules/ban-operators.test.ts‎

Lines changed: 95 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,112 @@ import { banOperatorsRule } from '../../src/rules/ban-operators';
33
import { fromFixture } from '../etc';
44
import { ruleTester } from '../rule-tester';
55

6-
ruleTester({ types: false }).run('ban-operators', banOperatorsRule, {
6+
ruleTester({ types: true }).run('ban-operators', banOperatorsRule, {
77
valid: [
88
{
9-
code: `import { concat, merge as m, mergeMap as mm } from "rxjs/operators";`,
9+
code: stripIndent`
10+
// root import
11+
import { of, concat, merge as m, mergeMap as mm } from "rxjs";
12+
13+
of('a').pipe(concat(of('b')));
14+
of(1).pipe(m(of(2)));
15+
of('a').pipe(mm(x => of(x + '1')));
16+
`,
17+
options: [{}],
18+
},
19+
{
20+
code: stripIndent`
21+
// namespace import
22+
import * as Rx from "rxjs";
23+
24+
Rx.of('a').pipe(Rx.concat(Rx.of('b')));
25+
Rx.of(1).pipe(Rx.merge(Rx.of(2)));
26+
Rx.of('a').pipe(Rx.mergeMap(x => Rx.of(x + '1')));
27+
`,
28+
options: [{}],
1029
},
1130
{
12-
code: `import { concat, merge as m, mergeMap as mm } from "rxjs";`,
31+
code: stripIndent`
32+
// operators path import (deprecated)
33+
import { of, concat, merge as m, mergeMap as mm } from "rxjs/operators";
34+
35+
of('a').pipe(concat(of('b')));
36+
of(1).pipe(m(of(2)));
37+
of('a').pipe(mm(x => of(x + '1')));
38+
`,
39+
options: [{}],
1340
},
41+
stripIndent`
42+
// no options
43+
import { of, concat } from "rxjs";
44+
45+
of('a').pipe(concat(of('b')));
46+
`,
1447
{
15-
// This won't effect errors, because only imports from "rxjs/operators"
16-
// are checked. To support banning operators from "rxjs", it'll need to
17-
// check types.
18-
code: `import { concat, merge as m, mergeMap as mm } from "rxjs";`,
19-
options: [
20-
{
21-
concat: true,
22-
merge: 'because I say so',
23-
mergeMap: false,
24-
},
25-
],
48+
code: stripIndent`
49+
// non-RxJS operator
50+
import { of } from "rxjs";
51+
52+
function concat() {}
53+
54+
of('a').pipe(concat());
55+
`,
56+
options: [{ concat: true }],
2657
},
2758
],
2859
invalid: [
2960
fromFixture(
3061
stripIndent`
31-
import { concat, merge as m, mergeMap as mm } from "rxjs/operators";
32-
~~~~~~ [forbidden { "name": "concat", "explanation": "" }]
33-
~~~~~ [forbidden { "name": "merge", "explanation": ": because I say so" }]
62+
// root import
63+
import { of, concat, merge as m, mergeMap as mm } from "rxjs";
64+
65+
of('a').pipe(concat(of('b')));
66+
~~~~~~ [forbidden { "name": "concat", "explanation": "" }]
67+
of(1).pipe(m(of(2)));
68+
~ [forbidden { "name": "merge", "explanation": ": because I say so" }]
69+
of('a').pipe(mm(x => of(x + '1')));
70+
`,
71+
{
72+
options: [
73+
{
74+
concat: true,
75+
merge: 'because I say so',
76+
mergeMap: false,
77+
},
78+
],
79+
},
80+
),
81+
fromFixture(
82+
stripIndent`
83+
// namespace import
84+
import * as Rx from "rxjs";
85+
86+
Rx.of('a').pipe(Rx.concat(Rx.of('b')));
87+
~~~~~~ [forbidden { "name": "concat", "explanation": "" }]
88+
Rx.of(1).pipe(Rx.merge(Rx.of(2)));
89+
~~~~~ [forbidden { "name": "merge", "explanation": "" }]
90+
Rx.of('a').pipe(Rx.mergeMap(x => Rx.of(x + '1')));
91+
`,
92+
{
93+
options: [
94+
{
95+
concat: true,
96+
merge: true,
97+
mergeMap: false,
98+
},
99+
],
100+
},
101+
),
102+
fromFixture(
103+
stripIndent`
104+
// operators path import (deprecated)
105+
import { of, concat, merge as m, mergeMap as mm } from "rxjs/operators";
106+
107+
of('a').pipe(concat(of('b')));
108+
~~~~~~ [forbidden { "name": "concat", "explanation": "" }]
109+
of(1).pipe(m(of(2)));
110+
~ [forbidden { "name": "merge", "explanation": ": because I say so" }]
111+
of('a').pipe(mm(x => of(x + '1')));
34112
`,
35113
{
36114
options: [

0 commit comments

Comments
 (0)