8000 feat: add stable PrivateIdentifier based on estree spec by armano2 · Pull Request #2933 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

feat: add stable PrivateIdentifier based on estree spec #2933

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
wants to merge 3 commits into from
Closed

feat: add stable PrivateIdentifier based on estree spec #2933

wants to merge 3 commits into from

Conversation

armano2
Copy link
Collaborator
@armano2 armano2 commented Jan 12, 2021 •
edited by bradzacher
Loading

Add stable PrivateIdentifier ts-estree base on estree spec,

Due to lack of spec this field field was read as is from typescript,

Changes to produced AST:

value field got renamed to name without initial #

TODO:

  • add unit tests for naming-convention rule
  • add unit tests for no-unsafe-assignment rule

fixes #1436
ref #3430

estree spec: https://github.com/estree/estree/blob/master/experimental/class-features.md

@typescript-eslint

This comment has been minimized.

Sign in to view
@armano2 armano2 added the AST PRs and Issues about the AST structure label Jan 12, 2021
@codecov
Copy link
codecov bot commented Jan 12, 2021 •
edited
Loading

Codecov Report

Merging #2933 (b249029) into v5 (763a252) will decrease coverage by 0.00%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##               v5    #2933      +/-   ##
==========================================
- Coverage   92.83%   92.82%   -0.01%     
==========================================
  Files         314      314              
  Lines       10675    10680       +5     
  Branches     3027     3031       +4     
==========================================
+ Hits         9910     9914       +4     
  Misses        348      348              
- Partials      417      418       +1
Flag Coverage Δ
unittest 92.82% <87.50%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...es/eslint-plugin/src/rules/no-unsafe-assignment.ts 92.43% <0.00%> (-0.79%) ⬇️
packages/eslint-plugin/src/rules/prefer-for-of.ts 90.14% <ø> (ø)
...pt-estree/src/ts-estree/estree-to-ts-node-types.ts 100.00% <ø> (ø)
packages/visitor-keys/src/visitor-keys.ts 100.00% <ø> (ø)
...gin/src/rules/naming-convention-utils/validator.ts 94.44% <100.00%> (+0.03%) ⬆️
...kages/eslint-plugin/src/rules/naming-convention.ts 81.28% <100.00%> (+0.09%) ⬆️
packages/eslint-plugin/src/util/misc.ts 92.85% <100.00%> (+0.26%) ⬆️
packages/typescript-estree/src/convert.ts 98.38% <100.00%> (+<0.01%) ⬆️

@armano2 armano2 changed the title feat: add stable PrivateIdentifier base on estree spec feat: add stable PrivateIdentifier based on estree spec Jan 12, 2021
@bradzacher bradzacher added the enhancement New feature or request label Jan 13, 2021
@bradzacher
Copy link
Member

Based on my discussion with them in estree/estree#240 - I'm happy for us to add these to the AST.
I'd like to wait until we move them from the experimental folder to be 100% certain nothing is going to change (it shouldn't, but better safe than sorry!)
I'll probs raise a PR there on the weekend.

@bradzacher bradzacher marked this pull request as draft January 13, 2021 07:10
@armano2 armano2 added breaking change This change will require a new major version to be released and removed breaking change This change will require a new major version to be released labels Feb 6, 2021
@tchetwin
Copy link
tchetwin commented Feb 12, 2021 •
edited
Loading

@bradzacher I've raised such a PR here: estree/estree#241

Update: Resulted in estree/estree#242 which has now been merged. Full steam ahead!

@armano2 armano2 force-pushed the fix-private-identifier branch from aedb035 to f3a65a1 Compare February 12, 2021 13:00
@tchetwin
Copy link

This might make sense in its own Issue rather than this PR, but adding here for continuity of the ESTree class-features spec theme:

Can we reconcile the existing ClassProperty AST node with the ESTree proposed spec which spells it PropertyDefinition?

@bradzacher
Copy link
Member
bradzacher commented Feb 17, 2021 •
edited
Loading

Yes that's definitely on the cards to do soon, but that is a breaking change, so it would be better as a separate change.

@tchetwin tchetwin mentioned this pull request Feb 17, 2021
Closed
@tchetwin
Copy link

I've created #3068 to track this following the completion of this PR.
I agree that it would be a breaking change and should be addressed separately.

@armano2 armano2 force-pushed the fix-private-identifier branch from 0d788cf to b249029 Compare February 19, 2021 20:00
@armano2 armano2 mentioned this pull request Feb 20, 2021
Closed
@bradzacher bradzacher added this to the 5.0.0 milestone Apr 7, 2021
@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label May 28, 2021
@bradzacher bradzacher added the DO NOT MERGE PRs which should not be merged yet label Jul 31, 2021
@bradzacher bradzacher mentioned this pull request Aug 16, 2021
Closed
@bradzacher bradzacher changed the base branch from master to v5 August 21, 2021 22:18
This was referenced Aug 23, 2021
Draft
Closed
Closed
bradzacher added a commit that referenced this pull request Aug 29, 2021
@bradzacher
feat: support PrivateIdentifier
7d3d771 
Closes #3430
Resolves #2933
@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 29, 2021
Closed
@bradzacher bradzacher mentioned this pull request Aug 29, 2021
Merged
bradzacher added a commit that referenced this pull request Aug 29, 2021
@bradzacher
feat: support PrivateIdentifier
e2c8a23 
Closes #3430
Resolves #2933
bradzacher added a commit that referenced this pull request Aug 29, 2021
@bradzacher
feat: support PrivateIdentifier (#3808)
93f3180 
Closes #3430
Resolves #2933
@bradzacher
Copy link
Member

Closing in favour of #3808

@bradzacher bradzacher closed this Aug 29, 2021
@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 29, 2021
Merged
44 tasks
@bradzacher bradzacher deleted the fix-private-identifier branch September 3, 2021 17:58
bradzacher added a commit that referenced this pull request Sep 3, 2021
@bradzacher
feat: support PrivateIdentifier (#3808)
31a6ffc 
Closes #3430
Resolves #2933
bradzacher added a commit that referenced this pull request Sep 21, 2021
@bradzacher
feat: support PrivateIdentifier (#3808)
41e7338 
Closes #3430
Resolves #2933
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2021
bradzacher added a commit that referenced this pull request Oct 10, 2021
@bradzacher
feat: support PrivateIdentifier (#3808)
6c299c5 
Closes #3430
Resolves #2933
bradzacher added a commit that referenced this pull request Oct 11, 2021
@bradzacher
feat: support PrivateIdentifier (#3808)
0eefe5e 
Closes #3430
Resolves #2933
73CB
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
AST PRs and Issues about the AST structure awaiting response Issues waiting for a reply from the OP or another party DO NOT MERGE PRs which should not be merged yet enhancement New feature or request
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

TypeScript 3.8 Syntax Support
3 participants
@armano2 @bradzacher @tchetwin
0