Skip to content

Incosistent AST for typeArgument and typeParameters #220

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.

Sign up for GitHub

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

Jump to bottom
Closed
armano2 opened this issue Feb 6, 2019 · 2 comments · Fixed by #223
Closed

Incosistent AST for typeArgument and typeParameters #220

armano2 opened this issue Feb 6, 2019 · 2 comments · Fixed by #223
Labels
package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@armano2
Copy link
Collaborator

armano2 commented Feb 6, 2019 •
edited
Loading

While doing #219 i notice that we have small inconsistency in AST, sometimes we are outputting TSTypeParameterDeclaration and TSTypeParameterInstantiation without nodes and sometimes we are omitting this node completely.

typescript-eslint/packages/typescript-estree/src/convert.ts

Lines 1364 to 1368 in 690bff3

if (superClass.types[0] && superClass.types[0].typeArguments) {
result.superTypeParameters = this.convertTypeArgumentsToTypeParameters(
superClass.types[0].typeArguments
);
}

typescript-eslint/packages/typescript-estree/src/convert.ts

Lines 1643 to 1647 in 690bff3

if (node.typeArguments && node.typeArguments.length) {
result.typeParameters = this.convertTypeArgumentsToTypeParameters(
node.typeArguments
);
}

typescript-eslint/packages/typescript-estree/src/convert.ts

Lines 693 to 697 in 690bff3

if (node.typeParameters && node.typeParameters.length) {
result.typeParameters = this.convertTSTypeParametersToTypeParametersDeclaration(
node.typeParameters
);
}

typescript-eslint/packages/typescript-estree/src/convert.ts

Lines 2093 to 2097 in 690bff3

if (node.typeParameters) {
result.typeParameters = this.convertTSTypeParametersToTypeParametersDeclaration(
node.typeParameters
);
}

we should align it, but i'm unsure if we should always have it or not

additionally we should handle #146 in update for it
@armano2 armano2 added package: typescript-estree Issues related to @typescript-eslint/typescript-estree triage Waiting for team members to take a look and removed triage Waiting for team members to take a look labels Feb 6, 2019
@armano2
Copy link
Collaborator Author

armano2 commented Feb 6, 2019 •
edited
Loading

@typescript-eslint/prettier-team example prettier issue due to inconsistency

interface foo {
    foo<>();
}

@bradzacher
Copy link
Member

bradzacher commented Feb 6, 2019 •
edited
Loading

We should document our property names so it's clear what each one means and when it should be put on a node

I.e.

  • typeArgument = generic usage?
  • typeParameter = generic definition?
  • typeAnnotation = type on a node
  • ....

This was referenced Feb 6, 2019
Merged
Closed
kaicataldo pushed a commit to kaicataldo/typescript-eslint that referenced this issue Aug 27, 2019
@armano2 @JamesHenry
[CHORE] enable report-message-format (typescript-eslint#220) (eslint-…
757daaf 
…plugin-eslint-plugin)
@typescript-eslint typescript-eslint locked as resolved and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ts-estree): align typeArguments and typeParameters across nodes
2 participants
@armano2 @bradzacher