Skip to content

Commit ade33a4

Browse files
magarcialjharb
authored andcommitted
[Fix] export: false positive for typescript namespace merging
1 parent 41d4500 commit ade33a4

File tree

3 files changed

+221
-14
lines changed

3 files changed

+221
-14
lines changed

CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
1010
- [`no-named-default`, `no-default-export`, `prefer-default-export`, `no-named-export`, `export`, `named`, `namespace`, `no-unused-modules`]: support arbitrary module namespace names ([#2358], thanks [@sosukesuzuki])
1111
- [`no-dynamic-require`]: support dynamic import with espree ([#2371], thanks [@sosukesuzuki])
1212

13-
1413
### Fixed
1514
- [`default`]: `typescript-eslint-parser`: avoid a crash on exporting as namespace (thanks [@ljharb])
15+
- [`export`]/TypeScript: false positive for typescript namespace merging ([#1964], thanks [@magarcia])
1616

1717
### Changed
1818
- [Tests] `no-nodejs-modules`: add tests for node protocol URL ([#2367], thanks [@sosukesuzuki])
@@ -1575,6 +1575,7 @@ for info on changes for earlier releases.
15751575
[@ludofischer]: https://github.com/ludofischer
15761576
[@lukeapage]: https://github.com/lukeapage
15771577
[@lydell]: https://github.com/lydell
1578+
[@magarcia]: https://github.com/magarcia
15781579
[@Mairu]: https://github.com/Mairu
15791580
[@malykhinvi]: https://github.com/malykhinvi
15801581
[@manovotny]: https://github.com/manovotny
@@ -1665,4 +1666,4 @@ for info on changes for earlier releases.
16651666
[@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
16661667
[@xpl]: https://github.com/xpl
16671668
[@yordis]: https://github.com/yordis
1668-
[@zloirock]: https://github.com/zloirock
1669+
[@zloirock]: https://github.com/zloirock

src/rules/export.js

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,59 @@ const tsTypePrefix = 'type:';
3636
*/
3737
function isTypescriptFunctionOverloads(nodes) {
3838
const types = new Set(Array.from(nodes, node => node.parent.type));
39-
return (
40-
types.has('TSDeclareFunction') &&
41-
(
42-
types.size === 1 ||
43-
(types.size === 2 && types.has('FunctionDeclaration'))
44-
)
45-
);
39+
return types.has('TSDeclareFunction')
40+
&& (
41+
types.size === 1
42+
|| (types.size === 2 && types.has('FunctionDeclaration'))
43+
);
44+
}
45+
46+
/**
47+
* Detect merging Namespaces with Classes, Functions, or Enums like:
48+
* ```ts
49+
* export class Foo { }
50+
* export namespace Foo { }
51+
* ```
52+
* @param {Set<Object>} nodes
53+
* @returns {boolean}
54+
*/
55+
function isTypescriptNamespaceMerging(nodes) {
56+
const types = new Set(Array.from(nodes, node => node.parent.type));
57+
const noNamespaceNodes = Array.from(nodes).filter((node) => node.parent.type !== 'TSModuleDeclaration');
58+
59+
return types.has('TSModuleDeclaration')
60+
&& (
61+
types.size === 1
62+
// Merging with functions
63+
|| (types.size === 2 && (types.has('FunctionDeclaration') || types.has('TSDeclareFunction')))
64+
|| (types.size === 3 && types.has('FunctionDeclaration') && types.has('TSDeclareFunction'))
65+
// Merging with classes or enums
66+
|| (types.size === 2 && (types.has('ClassDeclaration') || types.has('TSEnumDeclaration')) && noNamespaceNodes.length === 1)
67+
);
68+
}
69+
70+
/**
71+
* Detect if a typescript namespace node should be reported as multiple export:
72+
* ```ts
73+
* export class Foo { }
74+
* export function Foo();
75+
* export namespace Foo { }
76+
* ```
77+
* @param {Object} node
78+
* @param {Set<Object>} nodes
79+
* @returns {boolean}
80+
*/
81+
function shouldSkipTypescriptNamespace(node, nodes) {
82+
const types = new Set(Array.from(nodes, node => node.parent.type));
83+
84+
return !isTypescriptNamespaceMerging(nodes)
85+
&& node.parent.type === 'TSModuleDeclaration'
86+
&& (
87+
types.has('TSEnumDeclaration')
88+
|| types.has('ClassDeclaration')
89+
|| types.has('FunctionDeclaration')
90+
|| types.has('TSDeclareFunction')
91+
);
4692
}
4793

4894
module.exports = {
@@ -156,9 +202,11 @@ module.exports = {
156202
for (const [name, nodes] of named) {
157203
if (nodes.size <= 1) continue;
158204

159-
if (isTypescriptFunctionOverloads(nodes)) continue;
205+
if (isTypescriptFunctionOverloads(nodes) || isTypescriptNamespaceMerging(nodes)) continue;
160206

161207
for (const node of nodes) {
208+
if (shouldSkipTypescriptNamespace(node, nodes)) continue;
209+
162210
if (name === 'default') {
163211
context.report(node, 'Multiple default exports.');
164212
} else {

tests/src/rules/export.js

Lines changed: 162 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,20 +220,58 @@ context('TypeScript', function () {
220220
}
221221
`,
222222
}, parserConfig)),
223+
224+
semver.satisfies(eslintPkg.version, '>= 6') ? [
225+
test(Object.assign({
226+
code: `
227+
export class Foo { }
228+
export namespace Foo { }
229+
export namespace Foo {
230+
export class Bar {}
231+
}
232+
`,
233+
}, parserConfig)),
234+
test(Object.assign({
235+
code: `
236+
export function Foo();
237+
export namespace Foo { }
238+
`,
239+
}, parserConfig)),
240+
test(Object.assign({
241+
code: `
242+
export function Foo(a: string);
243+
export namespace Foo { }
244+
`,
245+
}, parserConfig)),
246+
test(Object.assign({
247+
code: `
248+
export function Foo(a: string);
249+
export function Foo(a: number);
250+
export namespace Foo { }
251+
`,
252+
}, parserConfig)),
253+
test(Object.assign({
254+
code: `
255+
export enum Foo { }
256+
export namespace Foo { }
257+
`,
258+
}, parserConfig)),
259+
] : [],
260+
223261
test(Object.assign({
224262
code: 'export * from "./file1.ts"',
225263
filename: testFilePath('typescript-d-ts/file-2.ts'),
226264
}, parserConfig)),
227265

228-
(semver.satisfies(eslintPkg.version, '< 6') ? [] : [
266+
semver.satisfies(eslintPkg.version, '>= 6') ? [
229267
test({
230268
code: `
231269
export * as A from './named-export-collision/a';
232270
export * as B from './named-export-collision/b';
233271
`,
234272
parser,
235273
}),
236-
]),
274+
] : [],
237275

238276
// Exports in ambient modules
239277
test(Object.assign({
@@ -271,7 +309,7 @@ context('TypeScript', function () {
271309
},
272310
})),
273311
),
274-
invalid: [
312+
invalid: [].concat(
275313
// type/value name clash
276314
test(Object.assign({
277315
code: `
@@ -361,6 +399,126 @@ context('TypeScript', function () {
361399
},
362400
],
363401
}, parserConfig)),
402+
semver.satisfies(eslintPkg.version, '< 6') ? [] : [
403+
test(Object.assign({
404+
code: `
405+
export class Foo { }
406+
export class Foo { }
407+
export namespace Foo { }
408+
`,
409+
errors: [
410+
{
411+
message: `Multiple exports of name 'Foo'.`,
412+
line: 2,
413+
},
414+
{
415+
message: `Multiple exports of name 'Foo'.`,
416+
line: 3,
417+
},
418+
],
419+
}, parserConfig)),
420+
test(Object.assign({
421+
code: `
422+
export enum Foo { }
423+
export enum Foo { }
424+
export namespace Foo { }
425+
`,
426+
errors: [
427+
{
428+
message: `Multiple exports of name 'Foo'.`,
429+
line: 2,
430+
},
431+
{
432+
message: `Multiple exports of name 'Foo'.`,
433+
line: 3,
434+
},
435+
],
436+
}, parserConfig)),
437+
test(Object.assign({
438+
code: `
439+
export enum Foo { }
440+
export class Foo { }
441+
export namespace Foo { }
442+
`,
443+
errors: [
444+
{
445+
message: `Multiple exports of name 'Foo'.`,
446+
line: 2,
447+
},
448+
{
449+
message: `Multiple exports of name 'Foo'.`,
450+
line: 3,
451+
},
452+
],
453+
}, parserConfig)),
454+
test(Object.assign({
455+
code: `
456+
export const Foo = 'bar';
457+
export class Foo { }
458+
export namespace Foo { }
459+
`,
460+
errors: [
461+
{
462+
message: `Multiple exports of name 'Foo'.`,
463+
line: 2,
464+
},
465+
{
466+
message: `Multiple exports of name 'Foo'.`,
467+
line: 3,
468+
},
469+
],
470+
}, parserConfig)),
471+
test(Object.assign({
472+
code: `
473+
export function Foo();
474+
export class Foo { }
475+
export namespace Foo { }
476+
`,
477+
errors: [
478+
{
479+
message: `Multiple exports of name 'Foo'.`,
480+
line: 2,
481+
},
482+
{
483+
message: `Multiple exports of name 'Foo'.`,
484+
line: 3,
485+
},
486+
],
487+
}, parserConfig)),
488+
test(Object.assign({
489+
code: `
490+
export const Foo = 'bar';
491+
export function Foo();
492+
export namespace Foo { }
493+
`,
494+
errors: [
495+
{
496+
message: `Multiple exports of name 'Foo'.`,
497+
line: 2,
498+
},
499+
{
500+
message: `Multiple exports of name 'Foo'.`,
501+
line: 3,
502+
},
503+
],
504+
}, parserConfig)),
505+
test(Object.assign({
506+
code: `
507+
export const Foo = 'bar';
508+
export namespace Foo { }
509+
`,
510+
errors: [
511+
{
512+
message: `Multiple exports of name 'Foo'.`,
513+
line: 2,
514+
},
515+
{
516+
message: `Multiple exports of name 'Foo'.`,
517+
line: 3,
518+
},
519+
],
520+
}, parserConfig)),
521+
],
364522

365523
// Exports in ambient modules
366524
test(Object.assign({
@@ -385,7 +543,7 @@ context('TypeScript', function () {
385543
},
386544
],
387545
}, parserConfig)),
388-
],
546+
),
389547
});
390548
});
391549
});

0 commit comments

Comments
 (0)