[fix] export: false positive for typescript overloads

[chiawendt]

Fixes #1357.

Fixes false positives like this:

export function foo(a: number) {}
export function foo(a: string) {}

[coveralls]

Coverage increased (+0.003%) to 97.963% when pulling 22d5440 on golopot:issue-1327 into 5abd5ed on benmosher:master.

[coveralls]

Coverage increased (+0.003%) to 97.757% when pulling 5703fb0 on golopot:issue-1327 into 6512110 on benmosher:master.

[ljharb]

this seems like something that would need to be determined based on parser settings, not hardcoded extensions - also, what about flow?

[chiawendt]

1. Will it suffice to do something like \/@typescript-eslint\/parser\/.test(context.parserPath)?(context.parserPath is like /home/some-user/some-project/node_modules/@typescript-eslint/parser/dist/parser.js) 2. It seems that flow does not accept function overload like the case here.

[ljharb]

that would force people to install the typescript parser even if they weren’t using typescript, i think - ideally we’d find a more reliable mechanism than “its typescript” to handle this case.

[chiawendt]

Just find out find that these function nodes without body has node type TSDeclareFunction. Therefore I change the bail out logic to some nodes is TSDeclareFunction && every nodes is TSDeclareFunction or FunctionDeclaration.

[benmosher]

why the spread into an array? is nodes not an Array, just an iterable? Set maybe? if so I might Array.from(nodes) to make it a little clearer. but definitely nitpicking, this is a great improvement

[ljharb]

why not pass the Set in directly, and convert it inside the function?

[chiawendt]

Ready for review

[ljharb]

let's definitely not do Array.from twice. maybe something like this for the body of the function?

const types = new Set(Array.from(nodes, node => node.parent.type));
if (!types.has('TSDeclareFunction') || types.size > 2) {
  return false;
}
return types.size === 1 || types.has('FunctionDeclaration');

[chiawendt]

I end up with this:

const types = new Set(Array.from(nodes, node => node.parent.type))
return types.size === 2 && types.has('TSDeclareFunction') && types.has('FunctionDeclaration')

The case when types is the same as new Set(["TSDeclareFunction"]) is when the function lacks implementation, which is a tsc error.

[ljharb]

That isn't the same logic though - your current logic allows it to be only "TSDeclareFunction". Functions don't need an implementation in tsc if they're just defining a type overload.

[chiawendt]

That case is a tsc error https://www.typescriptlang.org/play/#code/KYDwDg9gTgLgBAMwK4DsDGMCWEWIBQCUQA . And for the purpose of this rule, that case does not matter.