Skip to content

Commit

Permalink
Small fixes in BugInstance and BugInstanceMatcher (spotbugs#3187)
Browse files Browse the repository at this point in the history 
* fix small issues in BugInstanceMatcher

* small fixes in BugInstance

* spotlessApply

* add changelog entry

* flip logic of condition in BugInstance according to @ThrawnCA's comment
  • Loading branch information
@JuditKnoll
JuditKnoll authored Nov 13, 2024
1 parent d389cf9 commit 466f99b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 76 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
- Do not report UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR for fields initialized in method annotated with TestNG's @BeforeClass. ([#3152](https://github.com/spotbugs/spotbugs/issues/3152))
- Fixed detector `FindReturnRef` not finding references exposed from nested and inner classes ([#2042](https://github.com/spotbugs/spotbugs/issues/2042))
- Fix call graph, include non-parametric void methods ([#3160](https://github.com/spotbugs/spotbugs/pull/3160))
- Added missing comma between line number and confidence when describing matching and mismatching bugs for tests ([#3187](https://github.com/spotbugs/spotbugs/pull/3187))

### Cleanup
- Cleanup thread issue and regex issue in test-harness ([#3130](https://github.com/spotbugs/spotbugs/issues/3130))
Expand Down
98 changes: 27 additions & 71 deletions spotbugs/src/main/java/edu/umd/cs/findbugs/BugInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public class BugInstance implements Comparable<BugInstance>, XMLWriteable, Clone
*/
private static boolean adjustExperimental;

private static Set<String> missingBugTypes = Collections.synchronizedSet(new HashSet<String>());
private static final Set<String> missingBugTypes = Collections.synchronizedSet(new HashSet<>());

public static class NoSuchBugPattern extends IllegalArgumentException {
private static final long serialVersionUID = 1L;
Expand Down Expand Up @@ -203,12 +203,6 @@ public static DateFormat firstSeenXMLFormat() {
return new SimpleDateFormat("M/d/yy h:mm a", Locale.ENGLISH);
}

/*
private boolean isFakeBugType(String type) {
return "MISSING".equals(type) || "FOUND".equals(type);
}
*/

private void boundPriority() {
priority = boundedPriority(priority);
}
Expand Down Expand Up @@ -540,7 +534,7 @@ public SourceLineAnnotation getPrimarySourceLineAnnotation() {
+ " must contain at least one class, method, or field annotation");
}

public Collection<? extends SourceLineAnnotation> getAnotherInstanceSourceLineAnnotations() {
public Collection<SourceLineAnnotation> getAnotherInstanceSourceLineAnnotations() {
// Highest priority: return the first top level source line annotation
Collection<SourceLineAnnotation> result = new ArrayList<>();
for (BugAnnotation annotation : annotationList) {
Expand Down Expand Up @@ -595,7 +589,7 @@ public Iterator<BugAnnotation> annotationIterator() {
/**
* Get an Iterator over all bug annotations.
*/
public List<? extends BugAnnotation> getAnnotations() {
public List<BugAnnotation> getAnnotations() {
return annotationList;
}

Expand Down Expand Up @@ -1044,10 +1038,7 @@ public BugInstance addSuperclass(PreorderVisitor visitor) {
*
* <p>
* For information on type descriptors, <br>
* see http://java.sun.com/docs/books/vmspec/2nd-edition/html/ClassFile.doc.
* html#14152 <br>
* 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>.
*
* @param typeDescriptor
* a jvm type descriptor, such as "[I"
Expand Down Expand Up @@ -1139,7 +1130,7 @@ public BugInstance addType(ClassDescriptor c) {
* @param fieldSig
* type signature of the field
* @param isStatic
* whether or not the field is static
* whether the field is static
* @return this object
*/
@Nonnull
Expand Down Expand Up @@ -1617,11 +1608,7 @@ public BugInstance addSourceLine(SourceLineAnnotation sourceLine) {
*/
@Nonnull
public BugInstance addSourceLine(BytecodeScanningDetector visitor, int pc) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(visitor.getClassContext(),
visitor, pc);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.fromVisitedInstruction(visitor.getClassContext(), visitor, pc));
return this;
}

Expand All @@ -1641,10 +1628,7 @@ public BugInstance addSourceLine(BytecodeScanningDetector visitor, int pc) {
*/
@Nonnull
public BugInstance addSourceLine(ClassContext classContext, PreorderVisitor visitor, int pc) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(classContext, visitor, pc);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.fromVisitedInstruction(classContext, visitor, pc));
return this;
}

Expand All @@ -1664,13 +1648,8 @@ public BugInstance addSourceLine(ClassContext classContext, PreorderVisitor visi
* @return this object
*/
@Nonnull
public BugInstance addSourceLine(ClassContext classContext, MethodGen methodGen, String sourceFile,
@Nonnull InstructionHandle handle) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(classContext, methodGen,
sourceFile, handle);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
public BugInstance addSourceLine(ClassContext classContext, MethodGen methodGen, String sourceFile, @Nonnull InstructionHandle handle) {
add(SourceLineAnnotation.fromVisitedInstruction(classContext, methodGen, sourceFile, handle));
return this;
}

Expand Down Expand Up @@ -1698,11 +1677,7 @@ public BugInstance addSourceLine(ClassContext classContext, MethodGen methodGen,
start = end;
end = tmp;
}
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstructionRange(classContext, methodGen,
sourceFile, start, end);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.fromVisitedInstructionRange(classContext, methodGen, sourceFile, start, end));
return this;
}

Expand Down Expand Up @@ -1739,8 +1714,7 @@ public BugInstance addSourceLine(MethodDescriptor methodDescriptor, Location loc
Method method = analysisCache.getMethodAnalysis(Method.class, methodDescriptor);
return addSourceLine(classContext, method, location);
} catch (CheckedAnalysisException e) {
return addSourceLine(SourceLineAnnotation.createReallyUnknown(methodDescriptor.getClassDescriptor()
.getDottedClassName()));
return addSourceLine(SourceLineAnnotation.createReallyUnknown(methodDescriptor.getClassDescriptor().getDottedClassName()));
}
}

Expand All @@ -1757,11 +1731,7 @@ public BugInstance addSourceLine(MethodDescriptor methodDescriptor, Location loc
*/
@Nonnull
public BugInstance addSourceLine(ClassContext classContext, Method method, InstructionHandle handle) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(classContext, method,
handle.getPosition());
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.fromVisitedInstruction(classContext, method, handle.getPosition()));
return this;
}

Expand All @@ -1781,8 +1751,8 @@ public BugInstance addSourceLine(ClassContext classContext, Method method, Instr
*/
@Nonnull
public BugInstance addSourceLineRange(BytecodeScanningDetector visitor, int startPC, int endPC) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstructionRange(visitor.getClassContext(),
visitor, startPC, endPC);
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstructionRange(visitor.getClassContext(), visitor, startPC,
endPC);
requireNonNull(sourceLineAnnotation);
add(sourceLineAnnotation);
return this;
Expand Down Expand Up @@ -1825,10 +1795,7 @@ public BugInstance addSourceLineRange(ClassContext classContext, PreorderVisitor
*/
@Nonnull
public BugInstance addSourceLine(BytecodeScanningDetector visitor) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.fromVisitedInstruction(visitor);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.fromVisitedInstruction(visitor));
return this;
}

Expand All @@ -1844,10 +1811,7 @@ public BugInstance addSourceLine(BytecodeScanningDetector visitor) {
*/
@Nonnull
public BugInstance addUnknownSourceLine(String className, String sourceFile) {
SourceLineAnnotation sourceLineAnnotation = SourceLineAnnotation.createUnknown(className, sourceFile);
if (sourceLineAnnotation != null) {
add(sourceLineAnnotation);
}
add(SourceLineAnnotation.createUnknown(className, sourceFile));
return this;
}

Expand Down Expand Up @@ -1880,7 +1844,7 @@ public String getMessageWithoutPrefix() {
}

String getLongDescription() {
return getBugPattern().getLongDescription().replaceAll("BUG_PATTERN", type);
return getBugPattern().getLongDescription().replace("BUG_PATTERN", type);
}

public String getAbridgedMessage() {
Expand Down Expand Up @@ -1975,8 +1939,7 @@ public int getCWEid() {
}

public void writeXML(XMLOutput xmlOutput, BugCollection bugCollection, boolean addMessages) throws IOException {
XMLAttributeList attributeList = new XMLAttributeList().addAttribute("type", type).addAttribute("priority",
String.valueOf(priority));
XMLAttributeList attributeList = new XMLAttributeList().addAttribute("type", type).addAttribute("priority", String.valueOf(priority));

// Always add the rank attribute.
attributeList.addAttribute("rank", Integer.toString(getBugRank()));
Expand All @@ -1992,7 +1955,7 @@ public void writeXML(XMLOutput xmlOutput, BugCollection bugCollection, boolean a
attributeList.addAttribute("category", pattern.getCategory());

if (addMessages) {
// Add a uid attribute, if we have a unique id.
// Add an uid attribute, if we have a unique id.

attributeList.addAttribute("instanceHash", getInstanceHash());
attributeList.addAttribute("instanceOccurrenceNum", Integer.toString(getInstanceOccurrenceNum()));
Expand Down Expand Up @@ -2045,7 +2008,7 @@ public void writeXML(XMLOutput xmlOutput, BugCollection bugCollection, boolean a
primaryAnnotations.put(getPrimaryField(), null);
primaryAnnotations.put(getPrimaryMethod(), null);
} else {
primaryAnnotations = Collections.<BugAnnotation, Void>emptyMap();
primaryAnnotations = Collections.emptyMap();
}

boolean foundSourceAnnotation = false;
Expand All @@ -2066,7 +2029,7 @@ public void writeXML(XMLOutput xmlOutput, BugCollection bugCollection, boolean a
for (BugProperty prop = propertyListHead; prop != null; prop = prop.getNext()) {
props.add(prop);
}
Collections.sort(props, (o1, o2) -> o1.getName().compareTo(o2.getName()));
props.sort(Comparator.comparing(BugProperty::getName));
for (BugProperty prop : props) {
prop.writeXML(xmlOutput);
}
Expand Down Expand Up @@ -2129,7 +2092,7 @@ public BugInstance add(@Nonnull BugAnnotation annotation) {
requireNonNull(annotation, "Missing BugAnnotation!");

// The java annotations for the class were not stored before,
// thus we add a post-hook to lookup the java annotations for
// thus we add a post-hook to look up the java annotations for
// bug annotation types that have a class descriptor. Then
// post-analysis matcher can easily filter based on the java
// annotations (without having to look at the bytecode again).
Expand Down Expand Up @@ -2197,9 +2160,7 @@ public BugInstance addSourceForTopStackValue(ClassContext classContext, Method m
if (result != null) {
return result;
}
} catch (DataflowAnalysisException e) {
AnalysisContext.logError("Couldn't find value source", e);
} catch (CFGBuilderException e) {
} catch (DataflowAnalysisException | CFGBuilderException e) {
AnalysisContext.logError("Couldn't find value source", e);
}

Expand Down Expand Up @@ -2551,16 +2512,11 @@ public List<BugAnnotation> getAnnotationsForMessage(boolean showContext) {
}

for (BugAnnotation b : getAnnotations()) {
if (primaryAnnotations.contains(b)) {
continue;
}
if (b instanceof LocalVariableAnnotation && !((LocalVariableAnnotation) b).isNamed()) {
continue;
}
if (b instanceof SourceLineAnnotation && ((SourceLineAnnotation) b).isUnknown()) {
continue;
if (!primaryAnnotations.contains(b)
&& !(b instanceof LocalVariableAnnotation && !((LocalVariableAnnotation) b).isNamed())
&& !(b instanceof SourceLineAnnotation && ((SourceLineAnnotation) b).isUnknown())) {
result.add(b);
}
result.add(b);
}
return result;
}
Expand Down
11 changes: 6 additions & 5 deletions test-harness/src/main/java/edu/umd/cs/findbugs/test/matcher/BugInstanceMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ public boolean matches(Object obj) {
String fullName = classAnn.getClassName();
int startDot = fullName.lastIndexOf(".") + 1;
int endDollar = fullName.indexOf('$');
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);
criteriaMatches &= fullName.equals(className) || simpleName.equals(className) || simpleNameInner.equals(className);
}
if (methodName != null) {
Expand Down Expand Up @@ -160,7 +160,7 @@ public boolean matches(Object obj) {
}
if (jspFile != null) {
ClassAnnotation classAnn = extractBugAnnotation(bugInstance, ClassAnnotation.class);
String fullName = classAnn.getClassName().replaceAll("\\.", "/").replaceAll("_005f", "_").replaceAll("_jsp", ".jsp");
String fullName = classAnn.getClassName().replace(".", "/").replace("_005f", "_").replace("_jsp", ".jsp");
//String simpleName = fullName.substring(fullName.lastIndexOf("/") + 1);
criteriaMatches &= fullName.endsWith(jspFile);
}
Expand All @@ -173,6 +173,7 @@ public boolean matches(Object obj) {
for (Integer potentialMatch : multipleChoicesLine) {
if (srcAnn.getStartLine() - 1 <= potentialMatch && potentialMatch <= srcAnn.getEndLine() + 1) {
found = true;
break;
}
}
//if(!found) {
Expand Down Expand Up @@ -213,7 +214,7 @@ public void describeTo(Description description) {
description.appendText("variableName=").appendValue(variableName).appendText(",");
}
if (lineNumber != null) {
description.appendText("lineNumber=").appendValue(lineNumber);
description.appendText("lineNumber=").appendValue(lineNumber).appendText(",");
}
if (confidence != null) {
description.appendText("confidence=").appendValue(confidence);
Expand Down Expand Up @@ -254,7 +255,7 @@ public void describeMismatch(Object item, Description description) {
}
if (lineNumber != null || !hasCriteria) {
SourceLineAnnotation srcAnn = extractBugAnnotation(bugInstance, SourceLineAnnotation.class);
description.appendText("lineNumber=").appendValue(srcAnn);
description.appendText("lineNumber=").appendValue(srcAnn).appendText(",");
}
if (confidence != null || !hasCriteria) {
description.appendText("confidence=").appendValue(bugInstance.getPriority());
Expand Down

0 comments on commit 466f99b

Please sign in to comment.