Skip to content

Commit

Permalink
fix #386, if a method is constrained by interface to report need for …
Browse files Browse the repository at this point in the history 
…'OptionalBoxed'
  • Loading branch information
@mebigfatguy
mebigfatguy committed Nov 4, 2024
1 parent bfc5132 commit 240933c
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .classpath
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
<classpathentry kind="lib" path="lib/asm-9.6.jar"/>
<classpathentry kind="lib" path="lib/asm-tree-9.6.jar"/>
<classpathentry kind="lib" path="lib/bcel-6.8.0.jar"/>
<classpathentry kind="lib" path="lib/spotbugs-4.8.3.jar"/>
<classpathentry kind="lib" path="lib/spotbugs-4.8.3.jar" sourcepath="lib/sources/spotbugs-4.8.3-sources.jar"/>
<classpathentry kind="lib" path="lib/spotbugs-annotations-4.8.3.jar"/>
<classpathentry kind="lib" path="lib/commons-codec-1.16.0.jar"/>
<classpathentry kind="lib" path="lib/commons-io-2.15.1.jar"/>
Expand Down
54 changes: 45 additions & 9 deletions src/main/java/com/mebigfatguy/fbcontrib/detect/OptionalIssues.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import java.util.ArrayDeque;
import java.util.BitSet;
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import javax.annotation.Nullable;
Expand All @@ -35,6 +37,8 @@
import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Method;

import com.mebigfatguy.fbcontrib.collect.MethodInfo;
import com.mebigfatguy.fbcontrib.collect.Statistics;
import com.mebigfatguy.fbcontrib.utils.BugType;
import com.mebigfatguy.fbcontrib.utils.FQMethod;
import com.mebigfatguy.fbcontrib.utils.SignatureBuilder;
Expand All @@ -46,7 +50,9 @@
import edu.umd.cs.findbugs.BytecodeScanningDetector;
import edu.umd.cs.findbugs.OpcodeStack;
import edu.umd.cs.findbugs.OpcodeStack.CustomUserValue;
import edu.umd.cs.findbugs.SourceLineAnnotation;
import edu.umd.cs.findbugs.ba.ClassContext;
import edu.umd.cs.findbugs.ba.SignatureParser;
import edu.umd.cs.findbugs.ba.XMethod;

/**
Expand All @@ -55,6 +61,12 @@
@CustomUserValue
public class OptionalIssues extends BytecodeScanningDetector {

private enum OptionalType {
PLAIN,
BOXED
}

private static final String OPTIONAL_SIGNATURE = "Ljava/util/Optional;";
private static Set<String> BOXED_OPTIONAL_TYPES = UnmodifiableSet.create("Ljava/lang/Integer;", "Ljava/lang/Long;",
"Ljava/lang/Double;");

Expand Down Expand Up @@ -99,6 +111,8 @@ OPTIONAL_OR_ELSE_METHOD, new FQMethod("java/util/OptionalDouble", "orElse", "(D)
private OpcodeStack stack;
private JavaClass currentClass;
private Deque<ActiveStackOp> activeStackOps;
private Map<OpcodeStack.Item, SourceLineAnnotation> boxedItems = new HashMap<>();
private boolean methodIsConstrained;

static {
INVOKE_OPS.set(Const.INVOKEINTERFACE);
Expand Down Expand Up @@ -155,7 +169,21 @@ public void visitClassContext(ClassContext classContext) {
public void visitCode(Code obj) {
stack.resetForMethodEntry(this);
activeStackOps.clear();
boxedItems.clear();
methodIsConstrained = false;

String returnType = new SignatureParser(getMethodSig()).getReturnTypeSignature();
if (OPTIONAL_SIGNATURE.equals(returnType)) {
MethodInfo mi = Statistics.getStatistics().getMethodStatistics(getClassName(), getMethodName(), getMethodSig());
methodIsConstrained = mi != null && mi.isDerived();
}
super.visitCode(obj);

for (SourceLineAnnotation slAnno : boxedItems.values()) {
bugReporter.reportBug(
new BugInstance(this, BugType.OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED.name(),
LOW_PRIORITY).addClass(this).addMethod(this).addSourceLine(slAnno));
}
}

/**
Expand All @@ -168,7 +196,7 @@ public void visitCode(Code obj) {
@Override
public void sawOpcode(int seen) {
FQMethod curCalledMethod = null;
Boolean sawPlainOptional = null;
OptionalType optionalType = null;

try {
switch (seen) {
Expand Down Expand Up @@ -207,9 +235,7 @@ public void sawOpcode(int seen) {
OpcodeStack.Item itm = stack.getStackItem(0);
String itmSig = itm.getSignature();
if (BOXED_OPTIONAL_TYPES.contains(itmSig)) {
bugReporter.reportBug(
new BugInstance(this, BugType.OI_OPTIONAL_ISSUES_PRIMITIVE_VARIANT_PREFERRED.name(),
LOW_PRIORITY).addClass(this).addMethod(this).addSourceLine(this));
optionalType = OptionalType.BOXED;
}
}
}
Expand Down Expand Up @@ -245,7 +271,7 @@ public void sawOpcode(int seen) {
}
}
if (OPTIONAL_OR_ELSE_METHOD.equals(curCalledMethod)) {
sawPlainOptional = Boolean.TRUE;
; optionalType = OptionalType.PLAIN;
}
} else if (OR_ELSE_GET_METHODS.contains(curCalledMethod)) {
if (!activeStackOps.isEmpty()) {
Expand Down Expand Up @@ -276,12 +302,18 @@ public void sawOpcode(int seen) {
}
}
if (OPTIONAL_OR_ELSE_GET_METHOD.equals(curCalledMethod)) {
sawPlainOptional = Boolean.TRUE;
optionalType = OptionalType.PLAIN;
}
} else if (OPTIONAL_GET_METHOD.equals(curCalledMethod)) {
sawPlainOptional = Boolean.TRUE;
optionalType = OptionalType.PLAIN;
}
break;

case Const.ARETURN:
if (methodIsConstrained && stack.getStackDepth() > 0) {
boxedItems.remove(stack.getStackItem(0));
}
break;
}
} catch (ClassNotFoundException e) {
bugReporter.reportMissingClass(e);
Expand All @@ -295,9 +327,13 @@ public void sawOpcode(int seen) {
while (activeStackOps.size() > stackDepth) {
activeStackOps.removeFirst();
}
if (sawPlainOptional != null) {
if (optionalType != null) {
OpcodeStack.Item itm = stack.getStackItem(0);
itm.setUserValue(sawPlainOptional);
itm.setUserValue(optionalType);
if (optionalType == OptionalType.BOXED) {
boxedItems.put(itm, SourceLineAnnotation.fromVisitedInstruction(OptionalIssues.this.getClassContext(),
OptionalIssues.this, getPC()));
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/samples/java/ex/OI_Sample.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ public boolean equalsToEmpty(Optional<String> foo) {
return foo.equals(Optional.empty());
}

public Optional<Object> getOpt386() {
public Optional<Object> fpGetOpt386() {
return Optional.of(Double.valueOf(10));
}
}

interface OptInf386 {
Optional<Object> getOpt386();
Optional<Object> fpGetOpt386();
}

0 comments on commit 240933c

Please sign in to comment.