Skip to content

Commit 443bee0

Browse files
magarciamagarcia
committed
[Fix] export: false positive for typescript namespace merging
1 parent 41d4500 commit 443bee0

File tree

3 files changed

+195
-3
lines changed

3 files changed

+195
-3
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: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,41 @@ function isTypescriptFunctionOverloads(nodes) {
4545
);
4646
}
4747

48+
/**
49+
* Detect merging Namespaces with Classes, Functions, or Enums like:
50+
* ```ts
51+
* export class Foo { }
52+
* export namespace Foo { }
53+
* ```
54+
* @param {Set<Object>} nodes
55+
* @returns {boolean}
56+
*/
57+
function isTypescriptNamespaceMerging(nodes) {
58+
const types = new Set(Array.from(nodes, node => node.parent.type));
59+
const noNamespaceNodes = Array.from(nodes).filter((node) => node.parent.type !== 'TSModuleDeclaration');
60+
61+
const isMerging = (
62+
types.has('TSModuleDeclaration') &&
63+
(
64+
types.size === 1 ||
65+
// Merging with functions
66+
(types.size === 2 && (types.has('FunctionDeclaration') || types.has('TSDeclareFunction'))) ||
67+
(types.size === 3 && types.has('FunctionDeclaration') && types.has('TSDeclareFunction')) ||
68+
// Merging with classes or enums
69+
(types.size === 2 && (types.has('ClassDeclaration') || types.has('TSEnumDeclaration')) && noNamespaceNodes.length === 1)
70+
)
71+
);
72+
73+
if (!isMerging && types.has('TSModuleDeclaration') && (types.has('TSEnumDeclaration') || types.has('ClassDeclaration') || types.has('FunctionDeclaration') || types.has('TSDeclareFunction'))) {
74+
// Remove namespace nodes to error only on other types
75+
Array.from(nodes).forEach(node => {
76+
if (node.parent.type === 'TSModuleDeclaration') nodes.delete(node);
77+
});
78+
}
79+
80+
return isMerging;
81+
}
82+
4883
module.exports = {
4984
meta: {
5085
type: 'problem',
@@ -156,7 +191,7 @@ module.exports = {
156191
for (const [name, nodes] of named) {
157192
if (nodes.size <= 1) continue;
158193

159-
if (isTypescriptFunctionOverloads(nodes)) continue;
194+
if (isTypescriptFunctionOverloads(nodes) || isTypescriptNamespaceMerging(nodes)) continue;
160195

161196
for (const node of nodes) {
162197
if (name === 'default') {

‎tests/src/rules/export.js

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,42 @@ context('TypeScript', function () {
220220
}
221221
`,
222222
}, parserConfig)),
223+
...(semver.satisfies(eslintPkg.version, '< 6') ? [] : [
224+
test(Object.assign({
225+
code: `
226+
export class Foo { }
227+
export namespace Foo { }
228+
export namespace Foo {
229+
export class Bar {}
230+
}
231+
`,
232+
}, parserConfig)),
233+
test(Object.assign({
234+
code: `
235+
export function Foo();
236+
export namespace Foo { }
237+
`,
238+
}, parserConfig)),
239+
test(Object.assign({
240+
code: `
241+
export function Foo(a: string);
242+
export namespace Foo { }
243+
`,
244+
}, parserConfig)),
245+
test(Object.assign({
246+
code: `
247+
export function Foo(a: string);
248+
export function Foo(a: number);
249+
export namespace Foo { }
250+
`,
251+
}, parserConfig)),
252+
test(Object.assign({
253+
code: `
254+
export enum Foo { }
255+
export namespace Foo { }
256+
`,
257+
}, parserConfig)),
258+
]),
223259
test(Object.assign({
224260
code: 'export * from "./file1.ts"',
225261
filename: testFilePath('typescript-d-ts/file-2.ts'),
@@ -361,6 +397,126 @@ context('TypeScript', function () {
361397
},
362398
],
363399
}, parserConfig)),
400+
...(semver.satisfies(eslintPkg.version, '< 6') ? [] : [
401+
test(Object.assign({
402+
code: `
403+
export class Foo { }
404+
export class Foo { }
405+
export namespace Foo { }
406+
`,
407+
errors: [
408+
{
409+
message: `Multiple exports of name 'Foo'.`,
410+
line: 2,
411+
},
412+
{
413+
message: `Multiple exports of name 'Foo'.`,
414+
line: 3,
415+
},
416+
],
417+
}, parserConfig)),
418+
test(Object.assign({
419+
code: `
420+
export enum Foo { }
421+
export enum Foo { }
422+
export namespace Foo { }
423+
`,
424+
errors: [
425+
{
426+
message: `Multiple exports of name 'Foo'.`,
427+
line: 2,
428+
},
429+
{
430+
message: `Multiple exports of name 'Foo'.`,
431+
line: 3,
432+
},
433+
],
434+
}, parserConfig)),
435+
test(Object.assign({
436+
code: `
437+
export enum Foo { }
438+
export class Foo { }
439+
export namespace Foo { }
440+
`,
441+
errors: [
442+
{
443+
message: `Multiple exports of name 'Foo'.`,
444+
line: 2,
445+
},
446+
{
447+
message: `Multiple exports of name 'Foo'.`,
448+
line: 3,
449+
},
450+
],
451+
}, parserConfig)),
452+
test(Object.assign({
453+
code: `
454+
export const Foo = 'bar';
455+
export class Foo { }
456+
export namespace Foo { }
457+
`,
458+
errors: [
459+
{
460+
message: `Multiple exports of name 'Foo'.`,
461+
line: 2,
462+
},
463+
{
464+
message: `Multiple exports of name 'Foo'.`,
465+
line: 3,
466+
},
467+
],
468+
}, parserConfig)),
469+
test(Object.assign({
470+
code: `
471+
export function Foo();
472+
export class Foo { }
473+
export namespace Foo { }
474+
`,
475+
errors: [
476+
{
477+
message: `Multiple exports of name 'Foo'.`,
478+
line: 2,
479+
},
480+
{
481+
message: `Multiple exports of name 'Foo'.`,
482+
line: 3,
483+
},
484+
],
485+
}, parserConfig)),
486+
test(Object.assign({
487+
code: `
488+
export const Foo = 'bar';
489+
export function Foo();
490+
export namespace Foo { }
491+
`,
492+
errors: [
493+
{
494+
message: `Multiple exports of name 'Foo'.`,
495+
line: 2,
496+
},
497+
{
498+
message: `Multiple exports of name 'Foo'.`,
499+
line: 3,
500+
},
501+
],
502+
}, parserConfig)),
503+
test(Object.assign({
504+
code: `
505+
export const Foo = 'bar';
506+
export namespace Foo { }
507+
`,
508+
errors: [
509+
{
510+
message: `Multiple exports of name 'Foo'.`,
511+
line: 2,
512+
},
513+
{
514+
message: `Multiple exports of name 'Foo'.`,
515+
line: 3,
516+
},
517+
],
518+
}, parserConfig)),
519+
]),
364520

365521
// Exports in ambient modules
366522
test(Object.assign({

0 commit comments

Comments
 (0)