Skip to content

Commit 1dd595e

Browse files
JasonWeinzierlJasonWeinzierl
committed
fix(no-import-operators): fix alias===operator bug
- When alias name and operator name equal, we also need to check range to determine if the alias exists or not. - Refactored rule for less duplication.
1 parent f3602fd commit 1dd595e

File tree

2 files changed

+93
-94
lines changed

2 files changed

+93
-94
lines changed

‎src/rules/no-import-operators.ts‎

Lines changed: 72 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -54,111 +54,100 @@ export const noImportOperatorsRule = ruleCreator({
5454
return undefined;
5555
}
5656

57+
function hasDeprecatedOperators(specifiers?: es.ImportSpecifier[] | es.ExportSpecifier[]): boolean {
58+
return !!specifiers?.some(s => DEPRECATED_OPERATORS.includes(getName(getOperatorNode(s))));
59+
}
60+
5761
function getName(node: es.Identifier | es.StringLiteral): string {
5862
return isIdentifier(node) ? node.name : node.value;
5963
}
6064

61-
function getSpecifierReplacement(name: string): string | undefined {
65+
function getOperatorNode(node: es.ImportSpecifier | es.ExportSpecifier): es.Identifier | es.StringLiteral {
66+
return isImportSpecifier(node) ? node.imported : node.local;
67+
}
68+
69+
function getAliasNode(node: es.ImportSpecifier | es.ExportSpecifier): es.Identifier | es.StringLiteral {
70+
return isImportSpecifier(node) ? node.local : node.exported;
71+
}
72+
73+
function getOperatorReplacement(name: string): string | undefined {
6274
return RENAMED_OPERATORS[name];
6375
}
6476

65-
function reportNode(source: es.Literal, importSpecifiers?: es.ImportSpecifier[], exportSpecifiers?: es.ExportSpecifier[]): void {
66-
const replacement = getSourceReplacement(source.raw);
67-
if (
68-
replacement
69-
&& !importSpecifiers?.some(s => DEPRECATED_OPERATORS.includes(getName(s.imported)))
70-
&& !exportSpecifiers?.some(s => DEPRECATED_OPERATORS.includes(getName(s.exported)))
71-
) {
72-
if (importSpecifiers) {
73-
function* fix(fixer: TSESLint.RuleFixer) {
74-
// Rename the module name.
75-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
76-
yield fixer.replaceText(source, replacement!);
77-
78-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
79-
for (const specifier of importSpecifiers!) {
80-
const operatorName = getName(specifier.imported);
81-
const specifierReplacement = getSpecifierReplacement(operatorName);
82-
if (specifierReplacement) {
83-
if (specifier.local.name === operatorName) {
84-
// concat -> concatWith as concat
85-
yield fixer.insertTextBefore(specifier.imported, specifierReplacement + ' as ');
86-
} else if (isIdentifier(specifier.imported)) {
87-
// concat as c -> concatWith as c
88-
yield fixer.replaceText(specifier.imported, specifierReplacement);
89-
} else {
90-
// 'concat' as c -> 'concatWith' as c
91-
const quote = getQuote(specifier.imported.raw);
92-
if (!quote) {
93-
continue;
94-
}
95-
yield fixer.replaceText(specifier.imported, quote + specifierReplacement + quote);
96-
}
97-
}
98-
}
77+
function isNodesEqual(a: es.Node, b: es.Node): boolean {
78+
return a.range[0] === b.range[0] && a.range[1] === b.range[1];
79+
}
80+
81+
function createFix(source: es.Node, replacement: string, specifiers: es.ImportSpecifier[] | es.ExportSpecifier[]) {
82+
return function* fix(fixer: TSESLint.RuleFixer) {
83+
// Rename the module name.
84+
yield fixer.replaceText(source, replacement);
85+
86+
// Rename the imported operators if necessary.
87+
for (const specifier of specifiers) {
88+
const operatorNode = getOperatorNode(specifier);
89+
const operatorName = getName(operatorNode);
90+
91+
const operatorReplacement = getOperatorReplacement(operatorName);
92+
if (!operatorReplacement) {
93+
// The operator has the same name.
94+
continue;
9995
}
100-
context.report({
101-
fix,
102-
messageId: 'forbidden',
103-
node: source,
104-
suggest: [{ messageId: 'suggest', fix }],
105-
});
106-
} else if (exportSpecifiers) {
107-
function* fix(fixer: TSESLint.RuleFixer) {
108-
// Rename the module name.
109-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
110-
yield fixer.replaceText(source, replacement!);
111-
112-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
113-
for (const specifier of exportSpecifiers!) {
114-
const operatorName = getName(specifier.local);
115-
const specifierReplacement = getSpecifierReplacement(operatorName);
116-
if (specifierReplacement) {
117-
const exportedName = getName(specifier.exported);
118-
if (exportedName === operatorName) {
119-
// concat -> concatWith as concat
120-
yield fixer.insertTextBefore(specifier.exported, specifierReplacement + ' as ');
121-
} else if (isIdentifier(specifier.local)) {
122-
// concat as c -> concatWith as c
123-
yield fixer.replaceText(specifier.local, specifierReplacement);
124-
} else {
125-
// 'concat' as c -> 'concatWith' as c
126-
const quote = getQuote(specifier.local.raw);
127-
if (!quote) {
128-
continue;
129-
}
130-
yield fixer.replaceText(specifier.local, quote + specifierReplacement + quote);
131-
}
132-
}
96+
97+
const aliasNode = getAliasNode(specifier);
98+
if (isNodesEqual(aliasNode, operatorNode)) {
99+
// concat -> concatWith as concat
100+
yield fixer.insertTextBefore(operatorNode, operatorReplacement + ' as ');
101+
} else if (isIdentifier(operatorNode)) {
102+
// concat as c -> concatWith as c
103+
yield fixer.replaceText(operatorNode, operatorReplacement);
104+
} else {
105+
// 'concat' as c -> 'concatWith' as c
106+
const quote = getQuote(operatorNode.raw);
107+
if (!quote) {
108+
continue;
133109
}
110+
yield fixer.replaceText(operatorNode, quote + operatorReplacement + quote);
134111
}
135-
context.report({
136-
fix,
137-
messageId: 'forbidden',
138-
node: source,
139-
suggest: [{ messageId: 'suggest', fix }],
140-
});
141-
} else {
142-
context.report({
143-
messageId: 'forbidden',
144-
node: source,
145-
suggest: [{ messageId: 'suggest', fix: (fixer) => fixer.replaceText(source, replacement) }],
146-
});
147112
}
148-
} else {
113+
};
114+
}
115+
116+
function reportNode(source: es.Literal, specifiers?: es.ImportSpecifier[] | es.ExportSpecifier[]): void {
117+
const replacement = getSourceReplacement(source.raw);
118+
if (!replacement || hasDeprecatedOperators(specifiers)) {
119+
context.report({
120+
messageId: 'forbidden',
121+
node: source,
122+
});
123+
return;
124+
}
125+
126+
if (!specifiers) {
149127
context.report({
150128
messageId: 'forbidden',
151129
node: source,
130+
suggest: [{ messageId: 'suggest', fix: (fixer) => fixer.replaceText(source, replacement) }],
152131
});
132+
return;
153133
}
134+
135+
const fix = createFix(source, replacement, specifiers);
136+
context.report({
137+
fix,
138+
messageId: 'forbidden',
139+
node: source,
140+
suggest: [{ messageId: 'suggest', fix }],
141+
});
154142
}
155143

156144
return {
157145
'ImportDeclaration[source.value="rxjs/operators"]': (node: es.ImportDeclaration) => {
158146
// Exclude side effect imports, default imports, and namespace imports.
159-
const specifiers = node.specifiers.length && node.specifiers.every(s => isImportSpecifier(s))
147+
const specifiers = node.specifiers.length && node.specifiers.every(importClause => isImportSpecifier(importClause))
160148
? node.specifiers
161149
: undefined;
150+
162151
reportNode(node.source, specifiers);
163152
},
164153
'ImportExpression[source.value="rxjs/operators"]': (node: es.ImportExpression) => {
@@ -167,7 +156,7 @@ export const noImportOperatorsRule = ruleCreator({
167156
}
168157
},
169158
'ExportNamedDeclaration[source.value="rxjs/operators"]': (node: es.ExportNamedDeclarationWithSource) => {
170-
reportNode(node.source, undefined, node.specifiers);
159+
reportNode(node.source, node.specifiers);
171160
},
172161
'ExportAllDeclaration[source.value="rxjs/operators"]': (node: es.ExportAllDeclaration) => {
173162
reportNode(node.source);

‎tests/rules/no-import-operators.test.ts‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,32 +44,42 @@ ruleTester({ types: false }).run('no-import-operators', noImportOperatorsRule, {
4444
fromFixture(
4545
stripIndent`
4646
// import declaration named
47-
import { concat, map } from "rxjs/operators";
48-
~~~~~~~~~~~~~~~~ [forbidden suggest 0]
49-
import { 'merge' as m, race as r } from 'rxjs/operators';
50-
~~~~~~~~~~~~~~~~ [forbidden suggest 1]
47+
import { map as m, filter, 'tap' as tap } from "rxjs/operators";
48+
~~~~~~~~~~~~~~~~ [forbidden suggest]
5149
`,
5250
{
5351
output: stripIndent`
5452
// import declaration named
55-
import { concatWith as concat, map } from "rxjs";
56-
import { 'mergeWith' as m, raceWith as r } from 'rxjs';
53+
import { map as m, filter, 'tap' as tap } from "rxjs";
5754
`,
5855
suggestions: [
5956
{
6057
messageId: 'suggest',
6158
output: stripIndent`
6259
// import declaration named
63-
import { concatWith as concat, map } from "rxjs";
64-
import { 'merge' as m, race as r } from 'rxjs/operators';
60+
import { map as m, filter, 'tap' as tap } from "rxjs";
6561
`,
6662
},
63+
],
64+
},
65+
),
66+
fromFixture(
67+
stripIndent`
68+
// import declaration named, renamed operators
69+
import { 'merge' as m, race as race } from 'rxjs/operators';
70+
~~~~~~~~~~~~~~~~ [forbidden suggest]
71+
`,
72+
{
73+
output: stripIndent`
74+
// import declaration named, renamed operators
75+
import { 'mergeWith' as m, raceWith as race } from 'rxjs';
76+
`,
77+
suggestions: [
6778
{
6879
messageId: 'suggest',
6980
output: stripIndent`
70-
// import declaration named
71-
import { concat, map } from "rxjs/operators";
72-
import { 'mergeWith' as m, raceWith as r } from 'rxjs';
81+
// import declaration named, renamed operators
82+
import { 'mergeWith' as m, raceWith as race } from 'rxjs';
7383
`,
7484
},
7585
],

0 commit comments

Comments
 (0)