Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feature(rome_js_semantic): matching declarations and scopes #2690

Merged
merged 8 commits into from
Jun 14, 2022

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Jun 9, 2022

This PR is the part that matches declarations and scopes of the main PR (#2489).
For more details: #2488

This is just the first PR for this match, as we are solving only the most naive cases.

This also introduces a new assertion to test if the declaration is actually pointing to the correct scope. The important part here would be the @A, which means that the current declaration should be pointing to a scope named A.

if/*START A*/ (true) { let b/*#b @A*/ = 1; }

Better Diagnostics

Now the diagnostic specifies more clearly which test failed.

image

@xunilrj xunilrj temporarily deployed to aws June 9, 2022 13:57 Inactive
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 9, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 49c7664
Status: ✅  Deploy successful!
Preview URL: https://e4c79ef2.tools-8rn.pages.dev
Branch Preview URL: https://feature-semantic-declaration.tools-8rn.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked the JS spec on what introduces a new scope? I believe this is (partially) the relevant section. https://tc39.es/ecma262/#sec-static-semantics-lexicallydeclarednames

Another alternative would be to look at rome classic's implementation. Maybe @ematipico has a pointer for you?

crates/rome_js_semantic/src/events.rs Outdated Show resolved Hide resolved
crates/rome_js_semantic/src/events.rs Show resolved Hide resolved
crates/rome_js_semantic/src/events.rs Show resolved Hide resolved
crates/rome_js_semantic/src/tests/declarations.rs Outdated Show resolved Hide resolved
crates/rome_js_semantic/src/events.rs Show resolved Hide resolved
crates/rome_js_semantic/src/events.rs Show resolved Hide resolved
crates/rome_js_semantic/src/tests/declarations.rs Outdated Show resolved Hide resolved
@ematipico
Copy link
Contributor

Have you checked the JS spec on what introduces a new scope? I believe this is (partially) the relevant section. tc39.es/ecma262/#sec-static-semantics-lexicallydeclarednames

Another alternative would be to look at rome classic's implementation. Maybe @ematipico has a pointer for you?

Rome classic had a nice way to test things, although it was via snapshot tests: https://github.com/rome/tools/blob/archived-js/internal/compiler/scope/Scope.test.md

Maybe you can use it as inspiration.

This is where we stored all the globals: https://github.com/rome/tools/blob/archived-js/internal/compiler/scope/globals.ts, they were excluded from being tracked as bindings inside scopes.

This is where the bindings were created, and as you can see they have different kinds: https://github.com/rome/tools/blob/archived-js/internal/compiler/scope/bindings.ts

The creation of the scopes was done at node level by using the evaluators. I believe this is pretty nifty for various reasons:

  • each evaluator that we create is bound to a specific node, which means that we know that specific evaluator will create bindings/scopes
  • easy to debug

Here's the list of evaluators: https://github.com/rome/tools/tree/archived-js/internal/compiler/scope/evaluators
Here's the example for block statement: https://github.com/rome/tools/blob/archived-js/internal/compiler/scope/evaluators/JSBlockStatement.ts

We use the function .fork which creates a new scope but it keeps the same root scope.

@xunilrj xunilrj temporarily deployed to aws June 13, 2022 11:05 Inactive
@xunilrj xunilrj temporarily deployed to aws June 13, 2022 11:40 Inactive
@xunilrj xunilrj merged commit 3bb42ec into main Jun 14, 2022
@xunilrj xunilrj deleted the feature/semantic-declaration-scope branch June 14, 2022 16:55
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Jun 15, 2022
* variable declaration pointing to its parent scope
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants