-
Notifications
You must be signed in to change notification settings - Fork 578
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
Fix for IndexOutOfBoundsException in sarif placeholder #2649
Fix for IndexOutOfBoundsException in sarif placeholder #2649
Conversation
When the bug description has a placeholder for a non-existent annotation the sarif placeholder class causes a IndexOutOfBoundsException
edu.umd.cs.findbugs.FindBugsMessageFormat#format handles the case when the bug description contains a place holder but the corresponding annotation does not exist. Let the sarif placeholder handle this the same way
@@ -28,6 +28,12 @@ class Placeholder { | |||
|
|||
@NonNull | |||
String toArgument(List<? extends BugAnnotation> bugAnnotations, @Nullable ClassAnnotation primaryClass) { | |||
return bugAnnotations.get(index).format(key, primaryClass); | |||
if (index < 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gtoison Can you combine the 'if' here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two look similar but are different (< vs. >), I used the same format as edu.umd.cs.findbugs.FindBugsMessageFormat#format so the same problem will produce a similar output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks, I looked at that multiple times but wasn't seeing it. All good.
When the bug description has a placeholder for a non-existent annotation the sarif placeholder class causes a IndexOutOfBoundsException
This is most likely a side effect of #2627
Fixes #2632