Breaking: Convert Signature types to be more ESTree like (fixes #262)

[soda0289]

I noticed that optional is used twice. Once in identifier and once in the TSPropertySignature. Maybe we should clean that up.

Here is what this produces:
#262 (comment)

[eslintbot]

LGTM

[eslintbot]

LGTM

[soda0289]

We can move optional and computed to the Identifier node. I think that might make more sense

[JamesHenry]

A number of these aren't semantically valid TypeScript, but I guess we are further evaluating errorRecovery here, so it should be ok

[soda0289]

I can split them into two test. Not sure which ones are invalid exactly but I think the computed property don't make sense in interfaces.

[JamesHenry]

LGTM overall. Are you planning any more breaking changes after this?

It would be great if we could have a period of stability to allow for the prettier implementation to catch up a bit.

We done a lot of breaking changes recently (albeit a lot of those were in response to prettier issues 😄 )

[JamesHenry]

Worth a util function?

[JamesHenry]

Worth a util function?

[soda0289]

@JamesHenry Thanks for the review. Will make the changes.

The only other breaking changes we could do are:

• Change how we label declare variable, functions, and classes. I think prettier no longer depends on this since they use the source code to check if the keyword is present but we should still have a better solution for the long term.
• If we implement this a fix for Empty body function cuase errors for several eslint rules #253 it would be a breaking change but not sure if we could just patch all the eslint rules, even thou some are in plugins.
• There are these two requests but they break current spec JSXText nodes are mislabeled as Literal nodes #266 "use strict"/etc directives should be labeled as such #267

[eslintbot]

LGTM

[soda0289]

I added two functions to node-utils:

• isComputedProperty()
• isOptional()

[JamesHenry]

Again, just want to check how/if the AST alignment work affects this?

[soda0289]

I need to change this so that only optional and typeAnnoations appear on Identifier node. Everything else should be on parent node.

[JamesHenry]

I think we should look to finalise this after #292 gets resolved right?

[eslintbot]

LGTM

[eslintbot]

LGTM

[eslintbot]

LGTM

[soda0289]

@JamesHenry Should be good.

[JamesHenry]

This LGTM, @vjeux there are a lot of changes here, would you mind just confirming that this is all what you are expecting?

[vjeux]

Yup, I actually made prettier use this branch :) Shipit!

[JamesHenry]

Thanks, chaps! Awesome work