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

linter: do not require 'default' case for 'unique case' #2000

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IEncinas10
Copy link
Collaborator

Fixes #1278

Currently, case-missing-default rule requires the default case for every case statement. This PR makes an exception for unique case statements as requested by the issue.

I also changed where the diagnostics point to. Previously they pointed to the leftmost token of the kCaseItemList, but now they point to the case keyword.

case-missing-default rule required the 'default' case for every
case statement. This commit changes it so that case statements
with the 'unique' qualifier are not marked as errors by this
check.
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0b8cb4e) 92.85% compared to head (ab245b5) 92.85%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2000   +/-   ##
=======================================
  Coverage   92.85%   92.85%           
=======================================
  Files         355      355           
  Lines       26272    26273    +1     
=======================================
+ Hits        24395    24396    +1     
  Misses       1877     1877           
Files Changed Coverage Δ
...log/analysis/checkers/case_missing_default_rule.cc 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hzeller
Copy link
Collaborator

hzeller commented Aug 20, 2023

(currently on vacation, slow to respond with less internet bandwidth; will be back in a week or so for review)

@IEncinas10
Copy link
Collaborator Author

IEncinas10 commented Aug 21, 2023

Thanks for the heads up. Enjoy the vacation!

@goekce
Copy link

goekce commented Apr 15, 2024

@hzeller This patch is waiting for ~8 months. I would be glad to see this patch in verible.

@IEncinas10
Copy link
Collaborator Author

@hzeller This patch is waiting for ~8 months. I would be glad to see this patch in verible.

I'm sure he'll review it when he feels like it. People are busy, things take time.

Please be patient. If you need this feature, rebase this branch onto master and compile the project for yourself.

@hzeller
Copy link
Collaborator

hzeller commented Apr 15, 2024

Ah sorry everyone. This fell off my radar. Will see if I can make some time to review.

@IEncinas10
Copy link
Collaborator Author

Ah sorry everyone. This fell off my radar. Will see if I can make some time to review.

There is no hurry at all, thanks for all your work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unique case should not require default
4 participants