Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(lint/useEnumInitializers): make code fix safe #393

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

Conaclos
Copy link
Member

Summary

No bugs were reported for useEnumInitializers.
Moreover, its code fix doesn't change the program behavior.

Test Plan

I updated the snapshots and added more tests.

@Conaclos Conaclos temporarily deployed to Website deployment September 22, 2023 22:31 — with GitHub Actions Inactive
@github-actions github-actions bot added A-Linter Area: linter A-Website Area: website L-JavaScript Language: JavaScript and super languages A-Changelog Area: changelog labels Sep 22, 2023
@ematipico
Copy link
Member

ematipico commented Sep 23, 2023

Personally, I don't agree.

We're changing the value of an enum. If the user is already using the values somewhere, we'll break the user's code.

In the documentation we suggest a = Blue value, which will change from 0 to a string "Blue". This is super unsafe, especially if the user is using that value in other files, for example inside a switch or if.

@Conaclos
Copy link
Member Author

Conaclos commented Sep 23, 2023

We're changing the value of an enum. If the user is already using the values somewhere, we'll break the user's code.

In the documentation we suggest a = Blue value, which will change from 0 to a string "Blue". This is super unsafe, especially if the user is using that value in other files, for example inside a switch or if.

In TypeScript, the following enum is invalid:

enum Color {
  Red = "Red",
  Green = "Green",
  Blue,
}

In fact, all suggestions of strings are made for invalid enums.
The idea is to provide code suggestions while someone is typing the enum.

For integer-valued enums, I designed the code fix such as it suggests only the actual value of the enum member.
Thus, it is safe.

However, I accept that changing an enum member in such a way that an invalid enum (producing a compiler error), now compiles is questionable.
We could either keep the fix unsafe, or remove the suggestion for string enum members.
What do you think?

EDIT: After a second though, I think we should not suggest any code fix for invalid enum members. We could remove string suggestions and make the code fix safe.

@ematipico
Copy link
Member

Thank you for providing such a good explanation, your contextual knowledge is very valuable.

I'm sold, let's ship it!

@Conaclos Conaclos merged commit b0f47cc into main Sep 23, 2023
@Conaclos Conaclos deleted the conaclos/lint/useEnumInitializers/safe-fix branch September 23, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter A-Website Area: website L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants