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

feat: add useDefaultSwitchClause rule #2605

Merged
merged 22 commits into from
Apr 26, 2024

Conversation

michellocana
Copy link
Contributor

Implementation of the default-case ESLint rule in Biome.

Summary

In order to migrate the project I work on from ESLint to biome, there are some rules that still need to be implemented in the Biome Linter, so I decided to give a try on the default-case rule. This is my first PR on the repository, so let me know if I'm missing something or can do something in a better way.

Test Plan

Added snapshots, for valid and invalid cases according to the default-case ESLint tests.

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Apr 25, 2024
@michellocana michellocana changed the title Add useDefaultCase rule feat: add useDefaultCase rule Apr 25, 2024
Copy link

codspeed-hq bot commented Apr 26, 2024

CodSpeed Performance Report

Merging #2605 will not alter performance

Comparing michellocana:feat/use-default-case (b7f50c8) with main (b3ed181)

Summary

✅ 90 untouched benchmarks

@Conaclos
Copy link
Member

Hi @michellocana. Thanks for your contribution :)

Next time, please add a comment to Linter rules from other sources to indicate your interest in implementing this rule. After some discussion, we usually open a task issue to implement the rule.

I didn't add this rule to the list of rules to implement because I thought it was unhelpful and even problematic.
It may conflict with TypeScript's completeness check.

Can you motivate why you need this rule?

@michellocana
Copy link
Contributor Author

Hey @Conaclos, thanks for the quick review :D, and sorry, I didn't see the linked discussion before, next time I'm gonna take a look on that first.

While I completely agree that this rule becomes pretty much useless in a TypeScript environment, I found myself using this rule a lot on a plain Javascript scenario, which is the reality of some of the projects I currently work on:

const ORDER_STATUS = {
  WAITING_FOR_PAYMENT: 'WAITING_FOR_PAYMENT',
  PAID: 'PAID',
  CANCELED: 'CANCELED',
}

function getOrderLabel(status) {
  switch (status) {
    case ORDER_STATUS.WAITING_FOR_PAYMENT:
      return 'Waiting for payment.'
    case ORDER_STATUS.PAID:
      return 'Paid.'
  }
}

In the function above, getOrderLabel will return undefined if the order status is canceled or if I pass an unknown order status, and on a plain JS environment there's nothing to warn me that this code might generate a bug.

@michellocana michellocana changed the title feat: add useDefaultCase rule feat: add useDefaultSwitchClause rule Apr 26, 2024
Copy link
Member

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I think it is ready for merging :) Waiting for CI status.

@michellocana
Copy link
Contributor Author

cool, thanks for the feedbacks, hopefully I can help more in the project ❤️

@Conaclos Conaclos merged commit de063b4 into biomejs:main Apr 26, 2024
12 checks passed
@Conaclos Conaclos added the A-Changelog Area: changelog label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants