Skip to content

Java: Limit the amount of results that MissingEnumInSwitch produces per switch #15961

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

Merged
merged 5 commits into from
Mar 28, 2024

Conversation

igfoo
Copy link
Contributor

@igfoo igfoo commented Mar 18, 2024

The tool status page warns:

    An analysis file contained multiple alerts that included more related
    locations than our allowed limit of 100.
    These alerts correspond to the rule java/missing-case-in-switch.
    Only 100 locations were stored for these alerts.

Here's one possible fix: This change the query so that we only get a single alert for a switch, but it gives up to 3 cases that are not matched, along with the count of how many more cases there are. What do you think?

Another possibility, that will fix some cases (silencing them completely) but miss others, is C++'s approach of requiring a certain percentage of cases to be covered: https://github.com/github/codeql/blob/main/cpp/ql/src/Likely%20Bugs/Likely%20Typos/MissingEnumCaseInSwitch.ql#L21

igfoo added 2 commits March 18, 2024 15:56
…er switch

The tool status page warns:
    An analysis file contained multiple alerts that included more related
    locations than our allowed limit of 100.
    These alerts correspond to the rule java/missing-case-in-switch.
    Only 100 locations were stored for these alerts.
@github-actions github-actions bot added the Java label Mar 18, 2024
@igfoo igfoo marked this pull request as ready for review March 21, 2024 18:28
@igfoo igfoo requested a review from a team as a code owner March 21, 2024 18:28
joefarebrother
joefarebrother previously approved these changes Mar 22, 2024
Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

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

Looks good 👍
Minor style comment

@igfoo igfoo force-pushed the igfoo/MissingEnumInSwitch branch from a2602cc to b6a1266 Compare March 27, 2024 18:48
Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

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

👍

@igfoo igfoo merged commit 5dcd635 into github:main Mar 28, 2024
@igfoo igfoo deleted the igfoo/MissingEnumInSwitch branch March 28, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants