Small fixes in BugInstance and BugInstanceMatcher #3187
Conversation
| * see http://java.sun.com/docs/books/vmspec/2nd-edition/html/ClassFile.doc. | ||
| * html#14152 <br> |
There was a problem hiding this comment.
This page doesn't exist anymore.
| * or http://www.murrayc.com/learning/java/java_classfileformat.shtml# | ||
| * TypeDescriptors | ||
| * see <a href="https://www.murrayc.com/permalink/1998/03/13/the-java-class-file-format/#TypeDescriptors">TypeDescriptors</a>. |
There was a problem hiding this comment.
This link forwards to the new link.
| SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(visitor.getClassContext(), | ||
| visitor, pc); | ||
| if (sourceLineAnnotation != null) { | ||
| add(sourceLineAnnotation); | ||
| } | ||
| add(SourceLineAnnotation.fromVisitedInstruction(visitor.getClassContext(), visitor, pc)); |
There was a problem hiding this comment.
All of these cases, the variable can never be null, so the null check is not necessary.
| return getBugPattern().getLongDescription().replaceAll("BUG_PATTERN", type); | ||
| return getBugPattern().getLongDescription().replace("BUG_PATTERN", type); |
There was a problem hiding this comment.
The first argument doesn't contain any regex, so we can use replace() instead of replaceAll().
| String simpleName = fullName.substring(startDot != -1 ? startDot : 0, endDollar != -1 ? endDollar : fullName.length()); | ||
| String simpleNameInner = fullName.substring(startDot != -1 ? startDot : 0, fullName.length()); | ||
| String simpleName = fullName.substring(startDot, endDollar != -1 ? endDollar : fullName.length()); | ||
| String simpleNameInner = fullName.substring(startDot); |
There was a problem hiding this comment.
The value of startDot can never be -1, since the smallest value of lastIndexOf() is -1, but there is a +1 (see line 110).
Since appendText of comma is a fix to the description, I think a change log for that would be nice. Otherwise agree rest is just cleanup. |
Thanks, you are absolutely right. Added a changelog entry to mention this. |
| continue; | ||
| } | ||
| if (b instanceof SourceLineAnnotation && ((SourceLineAnnotation) b).isUnknown()) { | ||
| if (primaryAnnotations.contains(b) |
There was a problem hiding this comment.
Alternatively, flip the logic to "if not any of these then add".
This PR should not cause any change in functionality.
I haven't added any changelog entry, since I don't think this type of fix needs it.