Skip to content
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

Exclude methods throwing UnsupportedOperationException from setter-like methods when considering a class as immutable #1672

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
66 changes: 55 additions & 11 deletions spotbugs/src/main/java/edu/umd/cs/findbugs/util/MutableClasses.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import org.apache.bcel.Repository;
import org.apache.bcel.classfile.ExceptionTable;
import org.apache.bcel.classfile.AnnotationEntry;
import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Method;

Expand Down Expand Up @@ -137,6 +138,14 @@ private boolean computeMutable() {

private boolean looksLikeASetter(Method method) {
final String methodName = method.getName();
// If the method throws an UnsupportedOperationException then we ignore it.
ExceptionTable exceptions = method.getExceptionTable();
if (exceptions != null) {
baloghadamsoftware marked this conversation as resolved.
Show resolved Hide resolved
if (Arrays.asList(exceptions.getExceptionNames()).contains("java.lang.UnsupportedOperationException")) {
return false;
}
}

for (String name : SETTER_LIKE_PREFIXES) {
if (methodName.startsWith(name)) {
// If setter-like methods returns an object of the same type then we suppose that it
Expand All @@ -147,20 +156,55 @@ private boolean looksLikeASetter(Method method) {
return false;
}

// If the method throws an UnsupportedOperationException then we ignore it.
ExceptionTable exceptions = method.getExceptionTable();
if (exceptions != null) {
if (Arrays.asList(exceptions.getExceptionNames()).contains("java.lang.UnsupportedOperationException")) {
return false;
private String getSig() {
String local = sig;
baloghadamsoftware marked this conversation as resolved.
Show resolved Hide resolved
if (local == null) {
sig = local = "L" + cls.getClassName().replace('.', '/') + ";";
}
return local;
}

private boolean isImmutableByContract() {
Boolean local = immutableByContract;
baloghadamsoftware marked this conversation as resolved.
Show resolved Hide resolved
if (local == null) {
immutableByContract = local = computeByImmutableContract();
}
return local;
}

for (String name : SETTER_LIKE_NAMES) {
if (method.getName().startsWith(name)) {
String retSig = method.getReturnType().getSignature();
// If setter-like methods returns an object of the same type then we suppose that it
// is not a setter but creates a new instance instead.
return !("L" + cls.getClassName().replace('.', '/') + ";").equals(retSig);
private boolean computeByImmutableContract() {
for (AnnotationEntry entry : cls.getAnnotationEntries()) {
// Error-Prone's @Immutable annotation is @Inherited, hence it applies to subclasses as well
if (entry.getAnnotationType().equals("Lcom/google/errorprone/annotations/Immutable;")) {
return true;
}
}

final ClassAnalysis maybeSuper = getSuperAnalysis();
return maybeSuper != null && maybeSuper.isImmutableByContract();
}

private ClassAnalysis getSuperAnalysis() {
ClassAnalysis local = superAnalysis;
if (local == null) {
superAnalysis = local = loadSuperAnalysis();
}
return local;
}

private ClassAnalysis loadSuperAnalysis() {
// Quick check if there is a superclass of importance
final String superName = cls.getSuperclassName();
if (superName == null || superName.equals("java.lang.Object")) {
return null;
}

final JavaClass superClass;
try {
superClass = cls.getSuperClass();
} catch (ClassNotFoundException e) {
AnalysisContext.reportMissingClass(e);
return null;
}
if (superClass == null) {
return null;
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.