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

fix(js_semantic):handle exported modules and their members #4205

Merged
merged 1 commit into from
Feb 14, 2023
Merged

fix(js_semantic):handle exported modules and their members #4205

merged 1 commit into from
Feb 14, 2023

Conversation

Conaclos
Copy link
Contributor

Summary

Fixes #4179.

This also recognize exported members of a module.

Test Plan

Test included.
By the way I am not sure to understand why noUnusedVariables is not in tests/specs.

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Feb 13, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit eb22d23
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63eac5631f0b1500089a6064

@ematipico
Copy link
Contributor

ematipico commented Feb 14, 2023

@Conaclos the tests are under the folder: https://github.com/rome/tools/tree/main/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables

You can create a file with the same name as a rule or a folder. Having a folder allows the creation of more test cases

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

What's the reason behind the deleted code?

@Conaclos
Copy link
Contributor Author

@Conaclos the tests are under the folder: https://github.com/rome/tools/tree/main/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables

You can create a file with the same name as a rule or a folder. Having a folder allows the creation of more test cases

Actually, the tests are located in rome_js_analyze/tests/suppression/correctness/noUnusedVariables. The folder was preexisting. My question was why rome_js_analyze/tests/suppression instead of rome_js_analyze/tests/specs?

@ematipico
Copy link
Contributor

ematipico commented Feb 14, 2023

My question was why rome_js_analyze/tests/suppression instead of rome_js_analyze/tests/specs?

Clearly an issue :D it should not be there

@Conaclos
Copy link
Contributor Author

Conaclos commented Feb 14, 2023

What's the reason behind the deleted code?

This handles exports in modules and namespaces. The removed check restricted the analysis to exports at the ESM level.

As a example:

expot const Z ="Z" // esm level export
export namespace M1 { // esm level export
  export const X = "X" // inner eexport
  export namespace M2 { // inner eexport
    export const Y = "Y" // inner eexport
  }
}

@Conaclos Conaclos added this pull request to the merge queue Feb 14, 2023
Merged via the queue into rome:main with commit 65325e5 Feb 14, 2023
@Conaclos Conaclos deleted the lint/noUnusedVariables/4179 branch February 14, 2023 16:43
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.

🐛 Export a module marks it as unused variable
2 participants