Fix: Create body for multiple call signatures (fixes #92)

[soda0289]

An attempt to fix the multiple call signatures exception by creating a body with the type of 'EmptyFunctionStatement'. Still need to add tests.

@JamesHenry What are your thoughts?

[eslintbot]

Thanks for the pull request, @soda0289! I took a look to make sure it's ready for merging and found some changes are needed:

• Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

[eslintbot]

LGTM

[nzakas]

I don't think we want to add a node type that represents nothing, as that's a bit confusing. Can you explain what you're trying to accomplish here?

[soda0289]

@nzakas

There are two long standing issue #92 and #80 that cause exceptions to be thrown in escope when node.body is null. @JamesHenry attempted to submit a pull request, to escope, that would check for a node.body being set before attempting to visit its children (estools/escope#111) but it was denied because all FunctionExpression nodes must have a body. (estools/escope#111 (comment)). This pull request, as well as #142, simply add a body to over come this limitation.

I think another suggested solution was to mark these emptied body functions as TypeAnnotations. (#92 (comment)). I feel like that would further complicate the issue as we would be unable to check rules like (space-before-func-parn and explicit-member-accessibility). We could always modify those rules to check type annotations as well.

What are you thoughts? I was really just trying to find the simplest solution that would still allow most existing rules to work.

[corbinu]

@soda0289 I believe the solution @nzakas came up with was to fork escope.

https://github.com/eslint/tsc-meetings/blob/38e38c748fde3094e32d471ff2fc42c1484adc78/notes/2016/2016-10-13.md

I believe that change is happening for eslint 4 however.

@nzakas might this be a possible temp solution to be removed after 4?

[soda0289]

@corbinu
This makes sense and is a better long term solution.

Maybe we should merge this, with extra comments, until eslint 4 is released. Unless there is a better solution. Being unable to use abstract methods and eslint sucks. I could look into monkey patching escope, as babel-eslint has done, but that sounds like a lot of work for a temporary solution.

@nzakas Would it be possible to create a fork of escope we could use with eslint 4 or should I create one in my own account? I'd like to create a branch of this repo that uses eslint 4 to prepare for the changes. I think we have a bug for parsing comments that is blocked until the new version of eslint is released.

[JamesHenry]

Thanks again, @soda0289! We'll get back to you ASAP on how we want to proceed in the short-term. I would also love to address this.

[nzakas]

We are still going to fork escope to solve the higher-level problem. I'd like to do that soon as the other breaking changes in ESLint get finished up. I'm not a big fan of bandaid approaches that need to be removed later, so I'd rather wait than go with this approach.

[corbinu]

@nzakas No worries .. I think we are all just excited! This is the blocker preventing from switching my Typescript projects over :) So we are just jumping the gun a little!

Doesn't help that in a recent release TSLint broke a bunch of stuff on me :(

Thank you guys so much for the work that you do!

[soda0289]

Thanks for taking the time to explain the situation. Will wait for fork of escope and continue using a private fork until we are ready.

[corbinu]

@soda0289 I am going to switch my smoke tests to your fork for now as they should work much better! :)

Edit: nvm just implodes later will wait for the escope work :)