diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d339757e..39401934c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange - [`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]) - [`no-dynamic-require`]: support dynamic import with espree ([#2371], thanks [@sosukesuzuki]) - ### Fixed - [`default`]: `typescript-eslint-parser`: avoid a crash on exporting as namespace (thanks [@ljharb]) +- [`export`]/TypeScript: false positive for typescript namespace merging ([#1964], thanks [@magarcia]) ### Changed - [Tests] `no-nodejs-modules`: add tests for node protocol URL ([#2367], thanks [@sosukesuzuki]) @@ -1575,6 +1575,7 @@ for info on changes for earlier releases. [@ludofischer]: https://github.com/ludofischer [@lukeapage]: https://github.com/lukeapage [@lydell]: https://github.com/lydell +[@magarcia]: https://github.com/magarcia [@Mairu]: https://github.com/Mairu [@malykhinvi]: https://github.com/malykhinvi [@manovotny]: https://github.com/manovotny @@ -1665,4 +1666,4 @@ for info on changes for earlier releases. [@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg [@xpl]: https://github.com/xpl [@yordis]: https://github.com/yordis -[@zloirock]: https://github.com/zloirock \ No newline at end of file +[@zloirock]: https://github.com/zloirock diff --git a/src/rules/export.js b/src/rules/export.js index b9378f091..4cae10740 100644 --- a/src/rules/export.js +++ b/src/rules/export.js @@ -36,13 +36,59 @@ const tsTypePrefix = 'type:'; */ function isTypescriptFunctionOverloads(nodes) { const types = new Set(Array.from(nodes, node => node.parent.type)); - return ( - types.has('TSDeclareFunction') && - ( - types.size === 1 || - (types.size === 2 && types.has('FunctionDeclaration')) - ) - ); + return types.has('TSDeclareFunction') + && ( + types.size === 1 + || (types.size === 2 && types.has('FunctionDeclaration')) + ); +} + +/** + * Detect merging Namespaces with Classes, Functions, or Enums like: + * ```ts + * export class Foo { } + * export namespace Foo { } + * ``` + * @param {Set} nodes + * @returns {boolean} + */ +function isTypescriptNamespaceMerging(nodes) { + const types = new Set(Array.from(nodes, node => node.parent.type)); + const noNamespaceNodes = Array.from(nodes).filter((node) => node.parent.type !== 'TSModuleDeclaration'); + + return types.has('TSModuleDeclaration') + && ( + types.size === 1 + // Merging with functions + || (types.size === 2 && (types.has('FunctionDeclaration') || types.has('TSDeclareFunction'))) + || (types.size === 3 && types.has('FunctionDeclaration') && types.has('TSDeclareFunction')) + // Merging with classes or enums + || (types.size === 2 && (types.has('ClassDeclaration') || types.has('TSEnumDeclaration')) && noNamespaceNodes.length === 1) + ); +} + +/** + * Detect if a typescript namespace node should be reported as multiple export: + * ```ts + * export class Foo { } + * export function Foo(); + * export namespace Foo { } + * ``` + * @param {Object} node + * @param {Set} nodes + * @returns {boolean} + */ +function shouldSkipTypescriptNamespace(node, nodes) { + const types = new Set(Array.from(nodes, node => node.parent.type)); + + return !isTypescriptNamespaceMerging(nodes) + && node.parent.type === 'TSModuleDeclaration' + && ( + types.has('TSEnumDeclaration') + || types.has('ClassDeclaration') + || types.has('FunctionDeclaration') + || types.has('TSDeclareFunction') + ); } module.exports = { @@ -85,15 +131,19 @@ module.exports = { } return { - 'ExportDefaultDeclaration': (node) => addNamed('default', node, getParent(node)), + ExportDefaultDeclaration(node) { + addNamed('default', node, getParent(node)); + }, - 'ExportSpecifier': (node) => addNamed( - node.exported.name || node.exported.value, - node.exported, - getParent(node.parent), - ), + ExportSpecifier(node) { + addNamed( + node.exported.name || node.exported.value, + node.exported, + getParent(node.parent), + ); + }, - 'ExportNamedDeclaration': function (node) { + ExportNamedDeclaration(node) { if (node.declaration == null) return; const parent = getParent(node); @@ -119,7 +169,7 @@ module.exports = { } }, - 'ExportAllDeclaration': function (node) { + ExportAllDeclaration(node) { if (node.source == null) return; // not sure if this is ever true // `export * as X from 'path'` does not conflict @@ -156,9 +206,11 @@ module.exports = { for (const [name, nodes] of named) { if (nodes.size <= 1) continue; - if (isTypescriptFunctionOverloads(nodes)) continue; + if (isTypescriptFunctionOverloads(nodes) || isTypescriptNamespaceMerging(nodes)) continue; for (const node of nodes) { + if (shouldSkipTypescriptNamespace(node, nodes)) continue; + if (name === 'default') { context.report(node, 'Multiple default exports.'); } else { diff --git a/tests/src/rules/export.js b/tests/src/rules/export.js index e95efa032..5996e9fa3 100644 --- a/tests/src/rules/export.js +++ b/tests/src/rules/export.js @@ -26,7 +26,7 @@ ruleTester.run('export', rule, { // #328: "export * from" does not export a default test({ code: 'export default foo; export * from "./bar"' }), - ...SYNTAX_CASES, + SYNTAX_CASES, test({ code: ` @@ -82,22 +82,31 @@ ruleTester.run('export', rule, { // }), test({ code: 'let foo; export { foo }; export * from "./export-all"', - errors: ['Multiple exports of name \'foo\'.', - 'Multiple exports of name \'foo\'.'], + errors: [ + 'Multiple exports of name \'foo\'.', + 'Multiple exports of name \'foo\'.', + ], }), - // test({ code: 'export * from "./default-export"' - // , errors: [{ message: 'No named exports found in module ' + - // '\'./default-export\'.' - // , type: 'Literal' }] }), + // test({ + // code: 'export * from "./default-export"', + // errors: [ + // { + // message: 'No named exports found in module \'./default-export\'.', + // type: 'Literal', + // }, + // ], + // }), // note: Espree bump to Acorn 4+ changed this test's error message. // `npm up` first if it's failing. test({ code: 'export * from "./malformed.js"', - errors: [{ - message: "Parse errors in imported module './malformed.js': 'return' outside of function (1:1)", - type: 'Literal', - }], + errors: [ + { + message: "Parse errors in imported module './malformed.js': 'return' outside of function (1:1)", + type: 'Literal', + }, + ], }), // test({ @@ -152,52 +161,58 @@ context('TypeScript', function () { ruleTester.run('export', rule, { valid: [].concat( // type/value name clash - test(Object.assign({ + test({ code: ` export const Foo = 1; export type Foo = number; `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` export const Foo = 1; export interface Foo {} `, - }, parserConfig)), + ...parserConfig, + }), - semver.satisfies(tsEslintVersion, '>= 22') ? test(Object.assign({ + semver.satisfies(tsEslintVersion, '>= 22') ? test({ code: ` export function fff(a: string); export function fff(a: number); `, - }, parserConfig)) : [], + ...parserConfig, + }) : [], - semver.satisfies(tsEslintVersion, '>= 22') ? test(Object.assign({ + semver.satisfies(tsEslintVersion, '>= 22') ? test({ code: ` export function fff(a: string); export function fff(a: number); export function fff(a: string|number) {}; `, - }, parserConfig)) : [], + ...parserConfig, + }) : [], // namespace - test(Object.assign({ + test({ code: ` export const Bar = 1; export namespace Foo { export const Bar = 1; } `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` export type Bar = string; export namespace Foo { export type Bar = string; } `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` export const Bar = 1; export type Bar = string; @@ -206,8 +221,9 @@ context('TypeScript', function () { export type Bar = string; } `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` export namespace Foo { export const Foo = 1; @@ -219,13 +235,56 @@ context('TypeScript', function () { } } `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + semver.satisfies(eslintPkg.version, '>= 6') ? [ + test({ + code: ` + export class Foo { } + export namespace Foo { } + export namespace Foo { + export class Bar {} + } + `, + ...parserConfig, + }), + test({ + code: ` + export function Foo(); + export namespace Foo { } + `, + ...parserConfig, + }), + test({ + code: ` + export function Foo(a: string); + export namespace Foo { } + `, + ...parserConfig, + }), + test({ + code: ` + export function Foo(a: string); + export function Foo(a: number); + export namespace Foo { } + `, + ...parserConfig, + }), + test({ + code: ` + export enum Foo { } + export namespace Foo { } + `, + ...parserConfig, + }), + ] : [], + test({ code: 'export * from "./file1.ts"', filename: testFilePath('typescript-d-ts/file-2.ts'), - }, parserConfig)), + ...parserConfig, + }), - (semver.satisfies(eslintPkg.version, '< 6') ? [] : [ + semver.satisfies(eslintPkg.version, '>= 6') ? [ test({ code: ` export * as A from './named-export-collision/a'; @@ -233,10 +292,10 @@ context('TypeScript', function () { `, parser, }), - ]), + ] : [], // Exports in ambient modules - test(Object.assign({ + test({ code: ` declare module "a" { const Foo = 1; @@ -247,8 +306,9 @@ context('TypeScript', function () { export {Bar as default}; } `, - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` declare module "a" { const Foo = 1; @@ -257,9 +317,10 @@ context('TypeScript', function () { const Bar = 2; export {Bar as default}; `, - }, parserConfig)), + ...parserConfig, + }), - (semver.satisfies(process.version, '< 8') && semver.satisfies(eslintPkg.version, '< 6') ? [] : test({ + semver.satisfies(process.version, '< 8') && semver.satisfies(eslintPkg.version, '< 6') ? [] : test({ ...parserConfig, code: ` export * from './module'; @@ -269,11 +330,11 @@ context('TypeScript', function () { ...parserConfig.settings, 'import/extensions': ['.js', '.ts', '.jsx'], }, - })), + }), ), - invalid: [ + invalid: [].concat( // type/value name clash - test(Object.assign({ + test({ code: ` export type Foo = string; export type Foo = number; @@ -288,10 +349,11 @@ context('TypeScript', function () { line: 3, }, ], - }, parserConfig)), + ...parserConfig, + }), // namespace - test(Object.assign({ + test({ code: ` export const a = 1 export namespace Foo { @@ -309,8 +371,9 @@ context('TypeScript', function () { line: 5, }, ], - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` declare module 'foo' { const Foo = 1; @@ -328,8 +391,9 @@ context('TypeScript', function () { line: 5, }, ], - }, parserConfig)), - test(Object.assign({ + ...parserConfig, + }), + test({ code: ` export namespace Foo { export namespace Bar { @@ -360,10 +424,138 @@ context('TypeScript', function () { line: 9, }, ], - }, parserConfig)), + ...parserConfig, + }), + semver.satisfies(eslintPkg.version, '< 6') ? [] : [ + test({ + code: ` + export class Foo { } + export class Foo { } + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export enum Foo { } + export enum Foo { } + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export enum Foo { } + export class Foo { } + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export const Foo = 'bar'; + export class Foo { } + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export function Foo(); + export class Foo { } + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export const Foo = 'bar'; + export function Foo(); + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + test({ + code: ` + export const Foo = 'bar'; + export namespace Foo { } + `, + errors: [ + { + message: `Multiple exports of name 'Foo'.`, + line: 2, + }, + { + message: `Multiple exports of name 'Foo'.`, + line: 3, + }, + ], + ...parserConfig, + }), + ], // Exports in ambient modules - test(Object.assign({ + test({ code: ` declare module "a" { const Foo = 1; @@ -384,8 +576,9 @@ context('TypeScript', function () { line: 9, }, ], - }, parserConfig)), - ], + ...parserConfig, + }), + ), }); }); });