Add check: Use InvocationTargetException.getCause instead of getTargetException #2541
Conversation
Generate changelog in
|
...-prone/src/main/java/com/palantir/baseline/errorprone/InvocationTargetExceptionGetCause.java
Outdated
Show resolved
Hide resolved
...-prone/src/main/java/com/palantir/baseline/errorprone/InvocationTargetExceptionGetCause.java
Outdated
Show resolved
Hide resolved
baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/CardinalityEqualsZero.java
Outdated
Show resolved
Hide resolved
...-prone/src/main/java/com/palantir/baseline/errorprone/InvocationTargetExceptionGetCause.java
Outdated
Show resolved
Hide resolved
...-prone/src/main/java/com/palantir/baseline/errorprone/InvocationTargetExceptionGetCause.java
Outdated
Show resolved
Hide resolved
| link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", | ||
| linkType = BugPattern.LinkType.CUSTOM, | ||
| severity = SeverityLevel.SUGGESTION, | ||
| summary = "Use InvocationTargetException.getCause instead of InvocationTargetException.getTargetException") |
There was a problem hiding this comment.
It's helpful to describe the reason rather than state rules. We can link+summarize the javadoc (which is actually a super interesting -- if you're into that sort of thing -- detail):
This method predates the general-purpose exception chaining facility. The Throwable.getCause() method is now the preferred means of obtaining this information.
There was a problem hiding this comment.
Makes sense, will update.
I was deciding between adding more detail (like the API note from the javadoc) or being super concise. Decided on being concise because I thought a dev would just want the fastest resolution, and wouldn't think too much about the difference.
But you're right. Since we have a suggested fix anyway, they will know how to change their code, and we can use the summary to provide context. For times we don't have a suggested fix, maybe we'd favor conciseness over context.
There was a problem hiding this comment.
Oh. I have also missed updating the README for this check, and the collections.size() == 0 check... will update.
There was a problem hiding this comment.
I haven't been updating hte readme lately -- imo we should codegen the list of checks and use the summary from annotations, but it's not high enough on my list to implement at the moment!
There was a problem hiding this comment.
edit: posted this before I read your latest comment.
Also, what are your thoughts on moving the giant list of checks out of the main README, and into a separate md file? Each check can be a heading, so we could link directly to the check in the link argument of the BugPattern, and add as much detail as we'd like.
There was a problem hiding this comment.
cool. Made a task in the backlog.
###### _excavator_ is a bot for automating changes across repositories. Changes produced by the roomba/latest-baseline-oss check. # Release Notes ## 5.4.0 | Type | Description | Link | | ---- | ----------- | ---- | | Feature | Add check: Use InvocationTargetException.getCause instead of getTargetException. | palantir/gradle-baseline#2541 | To enable or disable this check, please contact the maintainers of Excavator.
mpritham
changed the title
Before this PR
One of the refaster templates removed in this PR, replaced calls to
InvocationTargetException.getTargetExceptionwithInvocationTargetException.getCause.After this PR
==COMMIT_MSG==
Add check: Use InvocationTargetException.getCause instead of getTargetException.
==COMMIT_MSG==