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

feat(rome_js_analyze): noConstEnum #3849

Merged
merged 1 commit into from
Nov 28, 2022
Merged

feat(rome_js_analyze): noConstEnum #3849

merged 1 commit into from
Nov 28, 2022

Conversation

Conaclos
Copy link
Contributor

@Conaclos Conaclos commented Nov 24, 2022

Summary

This is an exclusive rule for Rome!
This rule disallows TypeScript const enums.

Const enums pitfalls are described on TS docs.

See my comment for more context.

Test Plan

Unit test and doc-tests.

@Conaclos Conaclos requested a review from a team November 24, 2022 17:38
@netlify
Copy link

netlify bot commented Nov 24, 2022

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 51f8d48
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63812e6ee7f6e00008cddfef

@Conaclos Conaclos changed the title feat(rome_js_analyze): noConstEnum feat(rome_js_analyze): noConstEnum Nov 24, 2022
@ematipico
Copy link
Contributor

The const enum is well reported with lintdoc (cf generated rule web page). However, it is not reported in unit tests. Any idea why?

No idea to be honest. First time I see this kind of issue. You could copy the snippet inside the quick_test we have inside rome_js_analyze/lib.rs and see if you get some output (it's ignored in purpose, you can remove the directive, test it, and put the directive again).

@Conaclos Conaclos requested a review from a team as a code owner November 25, 2022 20:59
@Conaclos
Copy link
Contributor Author

No idea to be honest. First time I see this kind of issue. You could copy the snippet inside the quick_test we have inside rome_js_analyze/lib.rs and see if you get some output (it's ignored in purpose, you can remove the directive, test it, and put the directive again).

Thanks for the hint. I misnamed the test: no_const_enum.ts instead of noCOnstEnum.ts. It could be nice to fail if a file does not match a rule name.

@Conaclos Conaclos requested review from ematipico and removed request for xunilrj and leops November 25, 2022 21:09
@ematipico ematipico added the A-Linter Area: linter label Nov 28, 2022
@ematipico ematipico added this to the 11.0.0 milestone Nov 28, 2022
@ematipico ematipico merged commit 68ebebc into rome:main Nov 28, 2022
@Conaclos Conaclos deleted the no_const_enum branch March 7, 2023 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants