Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] export: False positive for exported overloaded functions in TS #3065

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
- `ExportMap` / flat config: include `languageOptions` in context ([#3052], thanks [@michaelfaith])
- [`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export ([#3032], thanks [@akwodkiewicz])
- [`export`]: False positive for exported overloaded functions in TS ([#3065], thanks [@liuxingbaoyu])

### Changed
- [Docs] [`no-relative-packages`]: fix typo ([#3066], thanks [@joshuaobrien])
Expand Down Expand Up @@ -1140,6 +1141,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#3066]: https://github.com/import-js/eslint-plugin-import/pull/3066
[#3065]: https://github.com/import-js/eslint-plugin-import/pull/3065
[#3052]: https://github.com/import-js/eslint-plugin-import/pull/3052
[#3043]: https://github.com/import-js/eslint-plugin-import/pull/3043
[#3036]: https://github.com/import-js/eslint-plugin-import/pull/3036
Expand Down Expand Up @@ -1875,6 +1877,7 @@ for info on changes for earlier releases.
[@lilling]: https://github.com/lilling
[@ljharb]: https://github.com/ljharb
[@ljqx]: https://github.com/ljqx
[@liuxingbaoyu]: https://github.com/liuxingbaoyu
[@lo1tuma]: https://github.com/lo1tuma
[@loganfsmyth]: https://github.com/loganfsmyth
[@luczsoma]: https://github.com/luczsoma
Expand Down
48 changes: 16 additions & 32 deletions src/rules/export.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import recursivePatternCapture from '../exportMap/patternCapture';
import docsUrl from '../docsUrl';
import includes from 'array-includes';
import flatMap from 'array.prototype.flatmap';

/*
Notes on TypeScript namespaces aka TSModuleDeclaration:
Expand All @@ -27,42 +26,25 @@
const tsTypePrefix = 'type:';

/**
* Detect function overloads like:
* remove function overloads like:
* ```ts
* export function foo(a: number);
* export function foo(a: string);
* export function foo(a: number|string) { return a; }
* ```
* @param {Set<Object>} nodes
* @returns {boolean}
*/
function isTypescriptFunctionOverloads(nodes) {
const nodesArr = Array.from(nodes);

const idents = flatMap(
nodesArr,
(node) => node.declaration && (
node.declaration.type === 'TSDeclareFunction' // eslint 6+
|| node.declaration.type === 'TSEmptyBodyFunctionDeclaration' // eslint 4-5
)
? node.declaration.id.name
: [],
);
if (new Set(idents).size !== idents.length) {
return true;
}

const types = new Set(nodesArr.map((node) => node.parent.type));
if (!types.has('TSDeclareFunction')) {
return false;
}
if (types.size === 1) {
return true;
}
if (types.size === 2 && types.has('FunctionDeclaration')) {
return true;
}
return false;
function removeTypescriptFunctionOverloads(nodes) {
nodes.forEach((node) => {
const declType = node.type === 'ExportDefaultDeclaration' ? node.declaration.type : node.parent.type;
if (
// eslint 6+
declType === 'TSDeclareFunction'
// eslint 4-5
|| declType === 'TSEmptyBodyFunctionDeclaration'
) {
nodes.delete(node);

Check warning on line 45 in src/rules/export.js

View check run for this annotation

Codecov / codecov/patch

src/rules/export.js#L45

Added line #L45 was not covered by tests
}
});
}

/**
Expand Down Expand Up @@ -227,9 +209,11 @@
'Program:exit'() {
for (const [, named] of namespace) {
for (const [name, nodes] of named) {
removeTypescriptFunctionOverloads(nodes);

if (nodes.size <= 1) { continue; }

if (isTypescriptFunctionOverloads(nodes) || isTypescriptNamespaceMerging(nodes)) { continue; }
if (isTypescriptNamespaceMerging(nodes)) { continue; }

for (const node of nodes) {
if (shouldSkipTypescriptNamespace(node, nodes)) { continue; }
Expand Down
26 changes: 24 additions & 2 deletions tests/src/rules/export.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ ruleTester.run('export', rule, {
`,
parser,
})),
getTSParsers().map((parser) => ({
code: `
export default function foo(param: string): boolean;
export default function foo(param: string, param1?: number): boolean {
return param && param1;
}
`,
parser,
})),
),

invalid: [].concat(
Expand Down Expand Up @@ -154,6 +163,19 @@ ruleTester.run('export', rule, {
ecmaVersion: 2022,
},
})),

getTSParsers().map((parser) => ({
code: `
export default function a(): void;
export default function a() {}
export { x as default };
`,
errors: [
'Multiple default exports.',
'Multiple default exports.',
],
parser,
})),
),
});

Expand Down Expand Up @@ -510,7 +532,7 @@ context('TypeScript', function () {
}),
test({
code: `
export function Foo();
export function Foo() { };
ljharb marked this conversation as resolved.
Show resolved Hide resolved
export class Foo { }
export namespace Foo { }
`,
Expand All @@ -529,7 +551,7 @@ context('TypeScript', function () {
test({
code: `
export const Foo = 'bar';
export function Foo();
export function Foo() { };
export namespace Foo { }
`,
errors: [
Expand Down