This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
Refactor Class AST Tree Structure #1791
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with
|
| Latest commit: |
25b1fe2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5035af02.tools-8rn.pages.dev |
MichaReiser
commented
Nov 17, 2021
| @@ -0,0 +1,854 @@ | |||
| use crate::state::StateGuard; | |||
There was a problem hiding this comment.
Extracted from decl. You can review the individual commits if you want to see what I've refactored (quiet a bit)
Test262 comparison coverage results on windows-latest
|
1 similar comment
Test262 comparison coverage results on windows-latest
|
MichaReiser
commented
Nov 17, 2021
| // class bodies are implicitly strict | ||
| let mut guard = p.with_state(ParserState { | ||
| strict: Some(StrictMode::Class(p.cur_tok().range)), | ||
| ..p.state.clone() | ||
| }); | ||
|
|
||
| let idt = if (guard.at_ts(token_set![T![await], T![yield], T![ident]]) |
There was a problem hiding this comment.
The opt_binding_identifier already checks (guard.at_ts(token_set![T![await], T![yield], T![ident]]) so we can remove it from here
Comment on lines
64
to
68
| if guard.at(T![this]) { | ||
| let m = guard.start(); | ||
| guard.bump_remap(T![ident]); | ||
| Some(m.complete(&mut *guard, NAME)) | ||
| } else { |
There was a problem hiding this comment.
I played around with typescript but didn't find a way how to make this a valid class id. So I decided to remove it
Comment on lines
253
to
255
| if p.nth_at(offset, T![?]) { | ||
| offset += 1; | ||
| } |
There was a problem hiding this comment.
is_method_class_member and the other is helpers already handle this.
| @@ -534,97 +537,97 @@ fn class_member_no_semi(p: &mut Parser) -> Option<CompletedMarker> { | |||
| None | |||
| } | |||
|
|
|||
| fn is_prop(p: &Parser, offset: usize) -> bool { | |||
| (p.at(T![?]) && is_prop(p, offset + 1)) | |||
There was a problem hiding this comment.
This must-have recursed forever because p.at(T![?]) doesn't respect the offset
MichaReiser
commented
Nov 17, 2021
| @@ -0,0 +1,39 @@ | |||
| use crate::{ | |||
There was a problem hiding this comment.
Hmm, sorry. Don't know why git didn't recognise my renames of the formatter files, maybe because I rebased the commits?
Test262 comparison coverage results on ubuntu-latest
|
Test262 comparison coverage results on windows-latest
|
Test262 comparison coverage results on ubuntu-latest
|
Test262 comparison coverage results on windows-latest
|
MichaReiser
requested review from
ematipico and
jamiebuilds
and removed request for
ematipico
47 tasks
Test262 comparison coverage results on ubuntu-latest
|
Test262 comparison coverage results on windows-latest
|
yassere
reviewed
Nov 17, 2021
yassere
reviewed
Nov 17, 2021
crates/rslint_parser/test_data/inline/ok/getter_class_member.rast
Outdated
Show resolved
Hide resolved
ematipico
approved these changes
Nov 17, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the AST structures for class declarations and class expressions as defined in #1725.
ClassDecl/ClassExprtoJsClassDeclaration/JsClassExpressionnametoidJsExtendsClausethat wraps theextendskeyword and the parent classbodytomembersand removed theClassBodynodeConstructortoJsConstructorClassMembernametoidJsConstructorClassMembersMethodtoJsMethodClassMemberClassProptoJsPropertyClassMemberGettertoJsGetterClassMemberSettertoJsSetterClassMemberparameterPrivatePropand instead introduce aJsPrivatePropertyClassMemberNametype since all members can be private (except constructors)JsEmptyStatementwithJsEmptyMember(these aren't proper statements)JsComputedObjectMemberNameandJsStaticObjectMemberNametoJsComputedMemberNameandJsLiteralMemberbecause they can be shared between classes and objectsTsReturnTypetoTsTypeAnnotationso that it can also be used for properties, parameters, etc.This PR also adds a set of new tests for classes to verify the member parsing. I did so because I had to refactor much of the parsing logic to make sense of it (and reduce some repetitive code and fix some parsing issues related to semicolons).
I further extracted the class parsing from the
decl.rsinto its ownclass.rs. You can see my refactorings if you look at the individual commits.Test Plan
Added new tests, verified that coverage isn't changing.
Disclaimer
I didn't verify if the typescript parsing is working correctly or if the typescript grammar is correct. We need to tackle typescript support separately.