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.

Sign up for GitHub

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

Jump to bottom

ThrowingExceptionsWithoutMessageOrCause false positive #7448

Closed
o-shevchenko opened this issue Jul 9, 2024 · 5 comments · Fixed by #7715
Closed

ThrowingExceptionsWithoutMessageOrCause false positive #7448

o-shevchenko opened this issue Jul 9, 2024 · 5 comments · Fixed by #7715
Labels
false-positive never gets stale Mark Issues/PRs that should not be closed as they won't get stale rules

Comments

@o-shevchenko
Copy link

o-shevchenko commented Jul 9, 2024 •
edited
Loading

Expected Behavior

loader.errors().firstOrNull()?.exception()?.let { throw it }

The following code should work fine and don't violate ThrowingExceptionsWithoutMessageOrCause since we already have a message for an exception

Observed Behavior

loader.errors().firstOrNull()?.exception()?.let { throw it }

Throws ThrowingExceptionsWithoutMessageOrCause
Even the following code violates the rule

loader.errors().firstOrNull()?.exception()?.let { throw DataAccessException(it.message, it.cause) }

For some reason it reacts on method invocation .exception()

Steps to Reproduce

Context

I'm using jooq Loader API
https://www.jooq.org/doc/latest/manual/sql-execution/importing/importing-api/

Your Environment

  • Version of detekt used: 1.23.6
  • Version of Gradle used (if applicable): 8.6
  • Gradle scan link (add --scan option when running the gradle task):
  • Operating System and version: MacOS
  • Link to your project (if it's a public repository):
@detekt-ci
Copy link
Collaborator

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@detekt-ci detekt-ci added the stale label Oct 8, 2024
@o-shevchenko
Copy link
Author

The problem still exists

@o-shevchenko
Copy link
Author

Can we remove stale label to prevent issue closing?
Thanks

@cortinico cortinico added never gets stale Mark Issues/PRs that should not be closed as they won't get stale and removed stale labels Oct 8, 2024
@BraisGabin
Copy link
Member

I think that this Rule is wrong. We shouldn't fail when an exception without message nor cause is throwed. We should fail when an exception without message nor cause is created.

@BraisGabin BraisGabin mentioned this issue Oct 13, 2024
Merged
@BraisGabin
Copy link
Member

Ok, this rule doesn't care about throw. It only cares about the creation of the rule (the name is a bit missleading). And I found the issue. It thinks that .exception() is a call to the Exception constructor and for that reason it raises an issue. I created a PR to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
false-positive never gets stale Mark Issues/PRs that should not be closed as they won't get stale rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix ThrowingExceptionsWithoutMessageOrCause false positive detekt/detekt
5 participants
@3flex @BraisGabin @cortinico @o-shevchenko @detekt-ci