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

fix(rome_js_analyze): fix ts const assertion on noUndeclaredVariables #3981

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Dec 6, 2022

Summary

Fix a problem with noUndeclaredVariables regarding Typescript const assertion.

Test Plan

> cargo test -p rome_js_analyze -- no_undeclared_variables

@xunilrj xunilrj requested review from leops, ematipico and a team as code owners December 6, 2022 11:22
@netlify
Copy link

netlify bot commented Dec 6, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 76c2561
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6391eb9fd3e53b0009b387c7
😎 Deploy Preview https://deploy-preview-3981--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44936 44936 0
Failed 943 943 0
Panics 0 0 0
Coverage 97.94% 97.94% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1684 1684 0
Failed 4262 4262 0
Panics 0 0 0
Coverage 28.32% 28.32% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

Copy link
Contributor

@leops leops left a comment

Choose a reason for hiding this comment

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

While this works well enough as a simple fix, we may want to revise how as const is represented in the AST by introducing a specialized TsConstType node for this keyword instead of using an identifier (this would align with other "special" types like TsVoidType, TsUndefinedType, TsNeverType or TsThisType)

@xunilrj xunilrj merged commit 89323c6 into main Dec 8, 2022
@xunilrj xunilrj deleted the fix/no-undeclared-vars-const-assertion branch December 8, 2022 14:20
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.

3 participants