Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

Empty body function cuase errors for several eslint rules #253

Closed
soda0289 opened this issue May 4, 2017 · 3 comments
Closed

Empty body function cuase errors for several eslint rules #253

soda0289 opened this issue May 4, 2017 · 3 comments
Labels

Comments

@soda0289
Copy link
Member

soda0289 commented May 4, 2017

There was a long standing issue with this parser and escope that caused a crash when encountering an empty body function. This is because ESTree defines that all Function/Method Declarations and Function expressions should have a body.

Escope has since been forked as eslint-scope and can now parse empty body function nodes. There are still serveral eslint rules that expect functions to have bodies.

Issues this caused are: #162 #92 #80 #78

These issue have caused the parser to change the node type of the following function nodes:

There are other cases where we do not change the node type of empty body functions. These include:

  • Declared Classes
  • Declared Functions
  • Function overloading
  • Abstract Function expression (found within an abstract method declaration)

There are two options to fix this. We can change the type of these node or modify the failing eslint rules.

I have proposed the following PRs: #166 #165 #163

All of these change the type of both the function declaration and function expression. Maybe we should just remove the function expression entierly. We can create the following new nodes:

TSAbstractMethodDefinition {
  id: Identifier;
  params: [ Parameters ];
  returnType: TypeAnnotation;
  typeParameters: [ TSTypeParameter ];
}
TSAmbientFunctionDefinition {
  id: Identifier;
  params: [ Parameters ];
  returnType: TypeAnnotation;
  typeParameters: [ TSTypeParameter ];
  declared: boolean;
}
TSOverloadFunctionDefinition {
  id: Identifier;
  params: [ Parameters ];
  returnType: TypeAnnotation;
  typeParameters: [ TSTypeParameter ];
}

Sorry about the long post but wanted to create a summary of the current situation and allow for easier discussion. If we keep keep the nodes the way they are we might want to change back the TSNamespaceFunction declaration since it is causing another issue.

@JamesHenry thoughts?

@soda0289
Copy link
Member Author

soda0289 commented May 7, 2017

@JamesHenry Any chance we could get fix in for this in the next release.
The follow eslint rules will fail currently:

  • indent v4
  • array-callback-return
  • lines-around-directive
  • unicorn/custom-error-definition

@ismail-syed
Copy link

👋 @JamesHenry @soda0289, is this issue being worked on? I'm having a related issue that I've described here.

ddunkin added a commit to ddunkin/typescript-eslint-parser that referenced this issue Nov 30, 2017
ddunkin added a commit to ddunkin/typescript-eslint-parser that referenced this issue Nov 30, 2017
@JamesHenry
Copy link
Member

This was finally fixed by #412.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants