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

Issue #13809: Kill mutation in Checker #13981

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

nrmancuso
Copy link
Member

@nrmancuso nrmancuso commented Nov 2, 2023

Part of #13809

https://sonarcloud.io/project/issues?resolved=false&severities=MAJOR&sinceLeakPeriod=true&pullRequest=13981&id=org.checkstyle%3Acheckstyle&open=AYuOAJXquUWup32NLidh

Change from method call -> variable must be enough to fool sonar into thinking this is new code; sonar violation should be suppressed after merge to master.

@romani
Copy link
Member

romani commented Nov 2, 2023

We can use probably https://docs.oracle.com/javase%2F7%2Fdocs%2Fapi%2F%2F/java/lang/AssertionError.html

It is pretty generic.

All other subclasses of Error is very specific.

@rnveach , are you ok to throw AssertionError ?

@rnveach
Copy link
Member

rnveach commented Nov 2, 2023

@romani AssertionError seems unrelated to this issue of killing the mutation with file path.

Thrown to indicate that an assertion has failed.

As for the question, I am not sure I see this area as asserting anything, so I don't see using AssertionError making more sense than just a general error.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

ok to merge,
we will suppress in sonar as wontfix.

@romani romani merged commit 81ac4f5 into checkstyle:master Nov 2, 2023
111 of 112 checks passed
@nrmancuso nrmancuso deleted the checker-mutation branch November 2, 2023 23:30
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.

None yet

3 participants