Skip to content

Commit 48d3070

Browse files
magarciamagarcia
committed
[Fix] export: false positive for typescript namespace merging
1 parent fc98de2 commit 48d3070

File tree

3 files changed

+197
-2
lines changed

3 files changed

+197
-2
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +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+
### Fixed
14+
- [`export`]/TypeScript: false positive for typescript namespace merging ([#1964], thanks [@magarcia])
15+
1316
### Changed
1417
- [Tests] `no-nodejs-modules`: add tests for node protocol URL ([#2367], thanks [@sosukesuzuki])
1518
- [Tests] `default`, `no-anonymous-default-export`, `no-mutable-exports`, `no-named-as-default-member`, `no-named-as-default`: add tests for arbitrary module namespace names ([#2358], thanks [@sosukesuzuki])
@@ -1571,6 +1574,7 @@ for info on changes for earlier releases.
15711574
[@ludofischer]: https://github.com/ludofischer
15721575
[@lukeapage]: https://github.com/lukeapage
15731576
[@lydell]: https://github.com/lydell
1577+
[@magarcia]: https://github.com/magarcia
15741578
[@Mairu]: https://github.com/Mairu
15751579
[@malykhinvi]: https://github.com/malykhinvi
15761580
[@manovotny]: https://github.com/manovotny
@@ -1661,4 +1665,4 @@ for info on changes for earlier releases.
16611665
[@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
16621666
[@xpl]: https://github.com/xpl
16631667
[@yordis]: https://github.com/yordis
1664-
[@zloirock]: https://github.com/zloirock
1668+
[@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
@@ -219,6 +219,42 @@ context('TypeScript', function () {
219219
}
220220
`,
221221
}, parserConfig)),
222+
...(semver.satisfies(eslintPkg.version, '< 6') ? [] : [
223+
test(Object.assign({
224+
code: `
225+
export class Foo { }
226+
export namespace Foo { }
227+
export namespace Foo {
228+
export class Bar {}
229+
}
230+
`,
231+
}, parserConfig)),
232+
test(Object.assign({
233+
code: `
234+
export function Foo();
235+
export namespace Foo { }
236+
`,
237+
}, parserConfig)),
238+
test(Object.assign({
239+
code: `
240+
export function Foo(a: string);
241+
export namespace Foo { }
242+
`,
243+
}, parserConfig)),
244+
test(Object.assign({
245+
code: `
246+
export function Foo(a: string);
247+
export function Foo(a: number);
248+
export namespace Foo { }
249+
`,
250+
}, parserConfig)),
251+
test(Object.assign({
252+
code: `
253+
export enum Foo { }
254+
export namespace Foo { }
255+
`,
256+
}, parserConfig)),
257+
]),
222258
test(Object.assign({
223259
code: 'export * from "./file1.ts"',
224260
filename: testFilePath('typescript-d-ts/file-2.ts'),
@@ -360,6 +396,126 @@ context('TypeScript', function () {
360396
},
361397
],
362398
}, parserConfig)),
399+
...(semver.satisfies(eslintPkg.version, '< 6') ? [] : [
400+
test(Object.assign({
401+
code: `
402+
export class Foo { }
403+
export class Foo { }
404+
export namespace Foo { }
405+
`,
406+
errors: [
407+
{
408+
message: `Multiple exports of name 'Foo'.`,
409+
line: 2,
410+
},
411+
{
412+
message: `Multiple exports of name 'Foo'.`,
413+
line: 3,
414+
},
415+
],
416+
}, parserConfig)),
417+
test(Object.assign({
418+
code: `
419+
export enum Foo { }
420+
export enum Foo { }
421+
export namespace Foo { }
422+
`,
423+
errors: [
424+
{
425+
message: `Multiple exports of name 'Foo'.`,
426+
line: 2,
427+
},
428+
{
429+
message: `Multiple exports of name 'Foo'.`,
430+
line: 3,
431+
},
432+
],
433+
}, parserConfig)),
434+
test(Object.assign({
435+
code: `
436+
export enum Foo { }
437+
export class Foo { }
438+
export namespace Foo { }
439+
`,
440+
errors: [
441+
{
442+
message: `Multiple exports of name 'Foo'.`,
443+
line: 2,
444+
},
445+
{
446+
message: `Multiple exports of name 'Foo'.`,
447+
line: 3,
448+
},
449+
],
450+
}, parserConfig)),
451+
test(Object.assign({
452+
code: `
453+
export const Foo = 'bar';
454+
export class Foo { }
455+
export namespace Foo { }
456+
`,
457+
errors: [
458+
{
459+
message: `Multiple exports of name 'Foo'.`,
460+
line: 2,
461+
},
462+
{
463+
message: `Multiple exports of name 'Foo'.`,
464+
line: 3,
465+
},
466+
],
467+
}, parserConfig)),
468+
test(Object.assign({
469+
code: `
470+
export function Foo();
471+
export class Foo { }
472+
export namespace Foo { }
473+
`,
474+
errors: [
475+
{
476+
message: `Multiple exports of name 'Foo'.`,
477+
line: 2,
478+
},
479+
{
480+
message: `Multiple exports of name 'Foo'.`,
481+
line: 3,
482+
},
483+
],
484+
}, parserConfig)),
485+
test(Object.assign({
486+
code: `
487+
export const Foo = 'bar';
488+
export function Foo();
489+
export namespace Foo { }
490+
`,
491+
errors: [
492+
{
493+
message: `Multiple exports of name 'Foo'.`,
494+
line: 2,
495+
},
496+
{
497+
message: `Multiple exports of name 'Foo'.`,
498+
line: 3,
499+
},
500+
],
501+
}, parserConfig)),
502+
test(Object.assign({
503+
code: `
504+
export const Foo = 'bar';
505+
export namespace Foo { }
506+
`,
507+
errors: [
508+
{
509+
message: `Multiple exports of name 'Foo'.`,
510+
line: 2,
511+
},
512+
{
513+
message: `Multiple exports of name 'Foo'.`,
514+
line: 3,
515+
},
516+
],
517+
}, parserConfig)),
518+
]),
363519

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

0 commit comments

Comments
 (0)