diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/CollectionNamingConfusion.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/CollectionNamingConfusion.java index 493b7717..947f3d1a 100644 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/CollectionNamingConfusion.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/CollectionNamingConfusion.java @@ -43,124 +43,124 @@ */ public class CollectionNamingConfusion extends PreorderVisitor implements Detector { - private JavaClass mapInterface; - private JavaClass setInterface; - private JavaClass listInterface; - private JavaClass queueInterface; - - private BugReporter bugReporter; - private ClassContext clsContext; - - /** - * constructs a CNC detector given the reporter to report bugs on - * - * @param bugReporter the sync of bug reports - */ - public CollectionNamingConfusion(BugReporter bugReporter) { - this.bugReporter = bugReporter; - - try { - mapInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_MAP); - setInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_SET); - listInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_LIST); - queueInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_QUEUE); - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - mapInterface = null; - setInterface = null; - listInterface = null; - queueInterface = null; - - } - } - - /** - * overrides the visitor to make sure that the static initializer was able to - * load the map class - * - * @param classContext the currently parsed class - */ - @Override - public void visitClassContext(ClassContext classContext) { - if (mapInterface != null) { - this.clsContext = classContext; - classContext.getJavaClass().accept(this); - } - } - - /** - * overrides the visitor to look for fields where the name has 'Map', 'Set', - * 'List' in it but the type of that field isn't that. - * - * @param obj the currently parsed field - */ - @Override - public void visitField(Field obj) { - if (checkConfusedName(obj.getName(), obj.getSignature())) { - bugReporter.reportBug(new BugInstance(this, BugType.CNC_COLLECTION_NAMING_CONFUSION.name(), NORMAL_PRIORITY) - .addClass(this).addField(this).addString(obj.getName())); - } - } - - /** - * overrides the visitor to look for local variables where the name has 'Map', - * 'Set', 'List' in it but the type of that field isn't that. note that this - * only is useful if compiled with debug labels. - * - * @param obj the currently parsed method - */ - @Override - public void visitMethod(Method obj) { - LocalVariableTable lvt = obj.getLocalVariableTable(); - if (lvt != null) { - LocalVariable[] lvs = lvt.getLocalVariableTable(); - for (LocalVariable lv : lvs) { - if (checkConfusedName(lv.getName(), lv.getSignature())) { - bugReporter.reportBug( - new BugInstance(this, BugType.CNC_COLLECTION_NAMING_CONFUSION.name(), NORMAL_PRIORITY) - .addClass(this).addString(lv.getName()) - .addSourceLine(this.clsContext, this, lv.getStartPC())); - } - } - } - } - - /** - * looks for a name that mentions a collection type but the wrong type for the - * variable - * - * @param methodOrVariableName the method or variable name - * @param signature the variable signature - * @return whether the name doesn't match the type - */ - @edu.umd.cs.findbugs.annotations.SuppressFBWarnings(value = "EXS_EXCEPTION_SOFTENING_RETURN_FALSE", justification = "No other simple way to determine whether class exists") - private boolean checkConfusedName(String methodOrVariableName, String signature) { - try { - String name = methodOrVariableName.toLowerCase(Locale.ENGLISH); - if ((name.endsWith("map") || (name.endsWith("set") && !name.endsWith("offset")) || name.endsWith("list") - || name.endsWith("deque") || name.endsWith("queue") || name.endsWith("stack")) - && signature.startsWith("Ljava/util/")) { - String clsName = SignatureUtils.stripSignature(signature); - JavaClass cls = Repository.lookupClass(clsName); - if ((cls.implementationOf(mapInterface) && !name.endsWith("map")) - || (cls.implementationOf(setInterface) && !name.endsWith("set")) - || ((cls.implementationOf(listInterface) || cls.implementationOf(queueInterface)) - && !name.endsWith("list") && !name.endsWith("queue"))) { - return true; - } - } - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - } - - return false; - } - - /** - * implements the visitor by does nothing - */ - @Override - public void report() { - // not used, implements the interface - } + private JavaClass mapInterface; + private JavaClass setInterface; + private JavaClass listInterface; + private JavaClass queueInterface; + + private BugReporter bugReporter; + private ClassContext clsContext; + + /** + * constructs a CNC detector given the reporter to report bugs on + * + * @param bugReporter the sync of bug reports + */ + public CollectionNamingConfusion(BugReporter bugReporter) { + this.bugReporter = bugReporter; + + try { + mapInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_MAP); + setInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_SET); + listInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_LIST); + queueInterface = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_QUEUE); + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + mapInterface = null; + setInterface = null; + listInterface = null; + queueInterface = null; + + } + } + + /** + * overrides the visitor to make sure that the static initializer was able to + * load the map class + * + * @param classContext the currently parsed class + */ + @Override + public void visitClassContext(ClassContext classContext) { + if (mapInterface != null) { + this.clsContext = classContext; + classContext.getJavaClass().accept(this); + } + } + + /** + * overrides the visitor to look for fields where the name has 'Map', 'Set', + * 'List' in it but the type of that field isn't that. + * + * @param obj the currently parsed field + */ + @Override + public void visitField(Field obj) { + if (checkConfusedName(obj.getName(), obj.getSignature())) { + bugReporter.reportBug(new BugInstance(this, BugType.CNC_COLLECTION_NAMING_CONFUSION.name(), NORMAL_PRIORITY) + .addClass(this).addField(this).addString(obj.getName())); + } + } + + /** + * overrides the visitor to look for local variables where the name has 'Map', + * 'Set', 'List' in it but the type of that field isn't that. note that this + * only is useful if compiled with debug labels. + * + * @param obj the currently parsed method + */ + @Override + public void visitMethod(Method obj) { + LocalVariableTable lvt = obj.getLocalVariableTable(); + if (lvt != null) { + LocalVariable[] lvs = lvt.getLocalVariableTable(); + for (LocalVariable lv : lvs) { + if (checkConfusedName(lv.getName(), lv.getSignature())) { + bugReporter.reportBug( + new BugInstance(this, BugType.CNC_COLLECTION_NAMING_CONFUSION.name(), NORMAL_PRIORITY) + .addClass(this).addString(lv.getName()) + .addSourceLine(this.clsContext, this, lv.getStartPC())); + } + } + } + } + + /** + * looks for a name that mentions a collection type but the wrong type for the + * variable + * + * @param methodOrVariableName the method or variable name + * @param signature the variable signature + * @return whether the name doesn't match the type + */ + @edu.umd.cs.findbugs.annotations.SuppressFBWarnings(value = "EXS_EXCEPTION_SOFTENING_RETURN_FALSE", justification = "No other simple way to determine whether class exists") + private boolean checkConfusedName(String methodOrVariableName, String signature) { + try { + String name = methodOrVariableName.toLowerCase(Locale.ENGLISH); + if ((name.endsWith("map") || (name.endsWith("set") && !name.endsWith("offset")) || name.endsWith("list") + || name.endsWith("deque") || name.endsWith("queue") || name.endsWith("stack")) + && signature.startsWith("Ljava/util/")) { + String clsName = SignatureUtils.stripSignature(signature); + JavaClass cls = Repository.lookupClass(clsName); + if ((cls.implementationOf(mapInterface) && !name.endsWith("map")) + || (cls.implementationOf(setInterface) && !name.endsWith("set")) + || ((cls.implementationOf(listInterface) || cls.implementationOf(queueInterface)) + && !name.endsWith("list") && !name.endsWith("queue"))) { + return true; + } + } + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + } + + return false; + } + + /** + * implements the visitor by does nothing + */ + @Override + public void report() { + // not used, implements the interface + } } diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/CommonsStringBuilderToString.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/CommonsStringBuilderToString.java index 640b7dd4..c5024ac8 100644 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/CommonsStringBuilderToString.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/CommonsStringBuilderToString.java @@ -52,140 +52,140 @@ */ public class CommonsStringBuilderToString extends OpcodeStackDetector { - private static final Set TOSTRINGBUILDER_CTOR_SIGS = UnmodifiableSet.create( - new SignatureBuilder().withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT).toString(), - new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, "org/apache/commons/lang/builder/ToStringStyle") - .toString(), - new SignatureBuilder() - .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, "org/apache/commons/lang3/builder/ToStringStyle") - .toString()); - - private final BugReporter bugReporter; - private final Stack stackTracker = new Stack<>(); - private final Map registerTracker = new HashMap<>(10); - - /** - * constructs a CSBTS detector given the reporter to report bugs on. - * - * @param bugReporter the sync of bug reports - */ - public CommonsStringBuilderToString(final BugReporter bugReporter) { - this.bugReporter = bugReporter; - } - - @Override - public void visit(Code obj) { - registerTracker.clear(); - stackTracker.clear(); - super.visit(obj); - } - - @Override - public boolean shouldVisitCode(Code obj) { - LocalVariableTable lvt = getMethod().getLocalVariableTable(); - return lvt != null; - } - - @Override - public void sawOpcode(int seen) { - switch (seen) { - case ALOAD: - case ALOAD_0: - case ALOAD_1: - case ALOAD_2: - case ALOAD_3: - LocalVariable lv = getMethod().getLocalVariableTable() - .getLocalVariable(RegisterUtils.getALoadReg(this, seen), getNextPC()); - if (lv != null) { - String signature = lv.getSignature(); - if (isToStringBuilder(signature)) { - Integer loadReg = Integer.valueOf(getRegisterOperand()); - Boolean appendInvoked = registerTracker.get(loadReg); - if (appendInvoked != null) { - stackTracker - .add(new StringBuilderInvokedStatus(loadReg.intValue(), appendInvoked.booleanValue())); - } - } - } - break; - case ASTORE: - case ASTORE_0: - case ASTORE_1: - case ASTORE_2: - case ASTORE_3: - Item si = stack.getStackItem(0); - String signature = si.getSignature(); - if (isToStringBuilder(signature)) { - int storeReg = getRegisterOperand(); - StringBuilderInvokedStatus p = stackTracker.pop(); - registerTracker.put(Integer.valueOf(storeReg), - p.register == -1 ? Boolean.FALSE : registerTracker.get(Integer.valueOf(p.register))); - } - break; - case POP: - si = stack.getStackItem(0); - signature = si.getSignature(); - if (isToStringBuilder(signature) && !stackTracker.isEmpty()) { - StringBuilderInvokedStatus p = stackTracker.pop(); - registerTracker.put(Integer.valueOf(p.register), Boolean.valueOf(p.appendInvoked)); - } - break; - case INVOKESPECIAL: - case INVOKEVIRTUAL: - String loadClassName = getClassConstantOperand(); - String calledMethodName = getNameConstantOperand(); - - if ("org/apache/commons/lang3/builder/ToStringBuilder".equals(loadClassName) - || "org/apache/commons/lang/builder/ToStringBuilder".equals(loadClassName)) { - String calledMethodSig = getSigConstantOperand(); - if (Values.CONSTRUCTOR.equals(calledMethodName) - && TOSTRINGBUILDER_CTOR_SIGS.contains(calledMethodSig)) { - stackTracker.add(new StringBuilderInvokedStatus(-1, false)); - } else if ("append".equals(calledMethodName)) { - if (stackTracker.size() > 0) { - StringBuilderInvokedStatus p = stackTracker.pop(); - stackTracker.add(new StringBuilderInvokedStatus(p.register, true)); - } else { - stackTracker.add(new StringBuilderInvokedStatus(0, true)); - } - } else if (Values.TOSTRING.equals(calledMethodName) - && SignatureBuilder.SIG_VOID_TO_STRING.equals(calledMethodSig)) { - StringBuilderInvokedStatus p = stackTracker.pop(); - if (!p.appendInvoked) { - bugReporter - .reportBug(new BugInstance(this, "CSBTS_COMMONS_STRING_BUILDER_TOSTRING", HIGH_PRIORITY) - .addClass(this).addMethod(this).addSourceLine(this)); - } - } - } - break; - - default: - break; - } - } - - private static boolean isToStringBuilder(String signature) { - return "Lorg/apache/commons/lang3/builder/ToStringBuilder;".equals(signature) - || "Lorg/apache/commons/lang/builder/ToStringBuilder;".equals(signature); - } - - /** - * represents an stack item that is an append of a StringBuilder - */ - static final class StringBuilderInvokedStatus { - public final int register; - public final boolean appendInvoked; - - StringBuilderInvokedStatus(int register, boolean appendInvoked) { - this.register = register; - this.appendInvoked = appendInvoked; - } - - @Override - public String toString() { - return ToString.build(this); - } - } + private static final Set TOSTRINGBUILDER_CTOR_SIGS = UnmodifiableSet.create( + new SignatureBuilder().withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT).toString(), + new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, "org/apache/commons/lang/builder/ToStringStyle") + .toString(), + new SignatureBuilder() + .withParamTypes(Values.SLASHED_JAVA_LANG_OBJECT, "org/apache/commons/lang3/builder/ToStringStyle") + .toString()); + + private final BugReporter bugReporter; + private final Stack stackTracker = new Stack<>(); + private final Map registerTracker = new HashMap<>(10); + + /** + * constructs a CSBTS detector given the reporter to report bugs on. + * + * @param bugReporter the sync of bug reports + */ + public CommonsStringBuilderToString(final BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + @Override + public void visit(Code obj) { + registerTracker.clear(); + stackTracker.clear(); + super.visit(obj); + } + + @Override + public boolean shouldVisitCode(Code obj) { + LocalVariableTable lvt = getMethod().getLocalVariableTable(); + return lvt != null; + } + + @Override + public void sawOpcode(int seen) { + switch (seen) { + case ALOAD: + case ALOAD_0: + case ALOAD_1: + case ALOAD_2: + case ALOAD_3: + LocalVariable lv = getMethod().getLocalVariableTable() + .getLocalVariable(RegisterUtils.getALoadReg(this, seen), getNextPC()); + if (lv != null) { + String signature = lv.getSignature(); + if (isToStringBuilder(signature)) { + Integer loadReg = Integer.valueOf(getRegisterOperand()); + Boolean appendInvoked = registerTracker.get(loadReg); + if (appendInvoked != null) { + stackTracker + .add(new StringBuilderInvokedStatus(loadReg.intValue(), appendInvoked.booleanValue())); + } + } + } + break; + case ASTORE: + case ASTORE_0: + case ASTORE_1: + case ASTORE_2: + case ASTORE_3: + Item si = stack.getStackItem(0); + String signature = si.getSignature(); + if (isToStringBuilder(signature)) { + int storeReg = getRegisterOperand(); + StringBuilderInvokedStatus p = stackTracker.pop(); + registerTracker.put(Integer.valueOf(storeReg), + p.register == -1 ? Boolean.FALSE : registerTracker.get(Integer.valueOf(p.register))); + } + break; + case POP: + si = stack.getStackItem(0); + signature = si.getSignature(); + if (isToStringBuilder(signature) && !stackTracker.isEmpty()) { + StringBuilderInvokedStatus p = stackTracker.pop(); + registerTracker.put(Integer.valueOf(p.register), Boolean.valueOf(p.appendInvoked)); + } + break; + case INVOKESPECIAL: + case INVOKEVIRTUAL: + String loadClassName = getClassConstantOperand(); + String calledMethodName = getNameConstantOperand(); + + if ("org/apache/commons/lang3/builder/ToStringBuilder".equals(loadClassName) + || "org/apache/commons/lang/builder/ToStringBuilder".equals(loadClassName)) { + String calledMethodSig = getSigConstantOperand(); + if (Values.CONSTRUCTOR.equals(calledMethodName) + && TOSTRINGBUILDER_CTOR_SIGS.contains(calledMethodSig)) { + stackTracker.add(new StringBuilderInvokedStatus(-1, false)); + } else if ("append".equals(calledMethodName)) { + if (stackTracker.size() > 0) { + StringBuilderInvokedStatus p = stackTracker.pop(); + stackTracker.add(new StringBuilderInvokedStatus(p.register, true)); + } else { + stackTracker.add(new StringBuilderInvokedStatus(0, true)); + } + } else if (Values.TOSTRING.equals(calledMethodName) + && SignatureBuilder.SIG_VOID_TO_STRING.equals(calledMethodSig)) { + StringBuilderInvokedStatus p = stackTracker.pop(); + if (!p.appendInvoked) { + bugReporter + .reportBug(new BugInstance(this, "CSBTS_COMMONS_STRING_BUILDER_TOSTRING", HIGH_PRIORITY) + .addClass(this).addMethod(this).addSourceLine(this)); + } + } + } + break; + + default: + break; + } + } + + private static boolean isToStringBuilder(String signature) { + return "Lorg/apache/commons/lang3/builder/ToStringBuilder;".equals(signature) + || "Lorg/apache/commons/lang/builder/ToStringBuilder;".equals(signature); + } + + /** + * represents an stack item that is an append of a StringBuilder + */ + static final class StringBuilderInvokedStatus { + public final int register; + public final boolean appendInvoked; + + StringBuilderInvokedStatus(int register, boolean appendInvoked) { + this.register = register; + this.appendInvoked = appendInvoked; + } + + @Override + public String toString() { + return ToString.build(this); + } + } } diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java index 4c53bd25..8f664f7f 100755 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/NeedlessMemberCollectionSynchronization.java @@ -51,365 +51,365 @@ */ @CustomUserValue public class NeedlessMemberCollectionSynchronization extends BytecodeScanningDetector { - private static JavaClass collectionClass; - - static { - try { - collectionClass = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_COLLECTION); - } catch (ClassNotFoundException cnfe) { - collectionClass = null; - } - } - - private static JavaClass mapClass; - - static { - try { - mapClass = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_MAP); - } catch (ClassNotFoundException cnfe) { - mapClass = null; - } - } - - private static Set syncCollections = UnmodifiableSet.create("java/util/Vector", "java/util/Hashtable"); - - private static Set modifyingMethods = UnmodifiableSet.create("add", "addAll", "addFirst", "addElement", - "addLast", "clear", "compute", "computeIfAbsent", "computeIfPresent", "insertElementAt", "merge", "put", - "putAll", "putIfAbsent", "remove", "removeAll", "removeAllElements", "removeElement", "removeElementAt", - "removeFirst", "removeLast", "removeRange", "replace", "replaceAll", "retainAll", "set", "setElementAt", - "setSize"); - - private enum State { - IN_METHOD, IN_CLINIT, IN_INIT - }; - - private BugReporter bugReporter; - private Map collectionFields; - private Map aliases; - private OpcodeStack stack; - private State state; - private String className; - - /** - * constructs a NMCS detector given the reporter to report bugs on - * - * @param bugReporter the sync of bug reports - */ - public NeedlessMemberCollectionSynchronization(BugReporter bugReporter) { - this.bugReporter = bugReporter; - } - - /** - * implements the visitor to clear the collectionFields and stack and to report - * collections that remain unmodified out of clinit or init - * - * @param classContext the context object of the currently parsed class - */ - @Override - public void visitClassContext(ClassContext classContext) { - try { - if ((collectionClass != null) && (mapClass != null)) { - collectionFields = new HashMap<>(); - aliases = new HashMap<>(); - stack = new OpcodeStack(); - JavaClass cls = classContext.getJavaClass(); - className = cls.getClassName(); - super.visitClassContext(classContext); - for (FieldInfo fi : collectionFields.values()) { - if (fi.isSynchronized()) { - bugReporter.reportBug( - new BugInstance(this, BugType.NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION.name(), - NORMAL_PRIORITY).addClass(this).addField(fi.getFieldAnnotation())); - } - } - } - } finally { - collectionFields = null; - aliases = null; - stack = null; - } - } - - /** - * implements the visitor to find collection fields - * - * @param obj the context object of the currently parse field - */ - @Override - public void visitField(Field obj) { - if (obj.isPrivate()) { - String signature = obj.getSignature(); - if (signature.startsWith(Values.SIG_QUALIFIED_CLASS_PREFIX)) { - try { - JavaClass cls = Repository.lookupClass(SignatureUtils.stripSignature(signature)); - if (cls.implementationOf(collectionClass) || cls.implementationOf(mapClass)) { - FieldAnnotation fa = FieldAnnotation.fromVisitedField(this); - collectionFields.put(fa.getFieldName(), new FieldInfo(fa)); - } - } catch (ClassNotFoundException cnfe) { - bugReporter.reportMissingClass(cnfe); - } - } - } - } - - /** - * implements the visitor to set the state based on the type of method being - * parsed - * - * @param obj the context object of the currently parsed code block - */ - @Override - public void visitCode(Code obj) { - if (collectionFields.isEmpty()) { - return; - } - aliases.clear(); - String methodName = getMethodName(); - if (Values.STATIC_INITIALIZER.equals(methodName)) { - state = State.IN_CLINIT; - } else if (Values.CONSTRUCTOR.equals(methodName)) { - state = State.IN_INIT; - } else { - state = State.IN_METHOD; - } - stack.resetForMethodEntry(this); - super.visitCode(obj); - } - - /** - * implements the visitor to call the approriate visitor based on state - * - * @param seen the opcode of the currently parsed instruction - */ - @Override - public void sawOpcode(int seen) { - switch (state) { - case IN_CLINIT: - sawCLInitOpcode(seen); - break; - - case IN_INIT: - sawInitOpcode(seen); - break; - - case IN_METHOD: - sawMethodOpcode(seen); - break; - } - } - - /** - * - * handle {@code } blocks by looking for putstatic calls referencing - * synchronized collections - * - * @param seen the opcode of the currently parsed instruction - */ - private void sawCLInitOpcode(int seen) { - boolean isSyncCollection = false; - try { - stack.precomputation(this); - - isSyncCollection = isSyncCollectionCreation(seen); - if (seen == PUTSTATIC) { - processCollectionStore(); - } - } finally { - stack.sawOpcode(this, seen); - if (isSyncCollection && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - item.setUserValue(Boolean.TRUE); - } - } - } - - /** - * handle {@code } blocks by looking for putfield calls referencing - * synchronized collections - * - * @param seen the opcode of the currently parsed instruction - */ - private void sawInitOpcode(int seen) { - boolean isSyncCollection = false; - try { - stack.mergeJumps(this); - isSyncCollection = isSyncCollectionCreation(seen); - if (seen == PUTFIELD) { - processCollectionStore(); - } - } finally { - stack.sawOpcode(this, seen); - if (isSyncCollection && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - item.setUserValue(Boolean.TRUE); - } - } - } - - /** - * handles regular methods by looking for methods on collections that are - * modifying and removes those collections from the ones under review - * - * @param seen the opcode of the currently parsed instruction - */ - private void sawMethodOpcode(int seen) { - boolean isSyncCollection = false; - try { - stack.mergeJumps(this); - isSyncCollection = isSyncCollectionCreation(seen); - - switch (seen) { - case INVOKEVIRTUAL: - case INVOKEINTERFACE: - String methodName = getNameConstantOperand(); - if (modifyingMethods.contains(methodName)) { - String signature = getSigConstantOperand(); - int parmCount = SignatureUtils.getNumParameters(signature); - if (stack.getStackDepth() > parmCount) { - OpcodeStack.Item item = stack.getStackItem(parmCount); - XField field = item.getXField(); - if (field != null) { - collectionFields.remove(field.getName()); - } else { - int reg = item.getRegisterNumber(); - if (reg >= 0) { - Integer register = Integer.valueOf(reg); - String fName = aliases.get(register); - if (fName != null) { - collectionFields.remove(fName); - aliases.remove(register); - } - } - } - } - } - removeCollectionParameters(); - break; - - case INVOKESTATIC: - removeCollectionParameters(); - break; - - case ARETURN: - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - XField field = item.getXField(); - if (field != null) { - collectionFields.remove(field.getName()); - } - } - break; - - case PUTFIELD: - case PUTSTATIC: - String fieldName = getNameConstantOperand(); - collectionFields.remove(fieldName); - break; - - case GOTO: - case GOTO_W: - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - XField field = item.getXField(); - if (field != null) { - collectionFields.remove(field.getName()); - } - } - break; - default: - break; - } - } finally { - TernaryPatcher.pre(stack, seen); - stack.sawOpcode(this, seen); - TernaryPatcher.post(stack, seen); - if (isSyncCollection && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - item.setUserValue(Boolean.TRUE); - } - } - - } - - /** - * returns whether this instruction is creating a synchronized collection - * - * @param seen the opcode of the currently parsed instruction - * @return whether a synchronized collection has just been created - */ - private boolean isSyncCollectionCreation(int seen) { - if (seen == INVOKESPECIAL) { - if (Values.CONSTRUCTOR.equals(getNameConstantOperand())) { - return (syncCollections.contains(getClassConstantOperand())); - } - } else if ((seen == INVOKESTATIC) && "java/util/Collections".equals(getClassConstantOperand())) { - String methodName = getNameConstantOperand(); - return ("synchronizedMap".equals(methodName) || "synchronizedSet".equals(methodName)); - } - return false; - } - - /** - * sets the source line annotation of a store to a collection if that collection - * is synchronized. - */ - private void processCollectionStore() { - String fieldClassName = getDottedClassConstantOperand(); - if (fieldClassName.equals(className) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - if (item.getUserValue() != null) { - String fieldName = getNameConstantOperand(); - if (fieldName != null) { - FieldInfo fi = collectionFields.get(fieldName); - if (fi != null) { - fi.getFieldAnnotation().setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this)); - fi.setSynchronized(); - } - } - } - } - } - - /** - * removes collection fields that are passed to other methods as arguments - */ - private void removeCollectionParameters() { - int parmCount = SignatureUtils.getNumParameters(getSigConstantOperand()); - if (stack.getStackDepth() >= parmCount) { - for (int i = 0; i < parmCount; i++) { - OpcodeStack.Item item = stack.getStackItem(i); - XField field = item.getXField(); - if (field != null) { - collectionFields.remove(field.getName()); - } - } - } - } - - /** - * holds information about a field, namely the annotation and whether the - * collection is synchronized. - */ - static class FieldInfo { - private FieldAnnotation fieldAnnotation; - private boolean isSynchronized; - - public FieldInfo(FieldAnnotation fa) { - fieldAnnotation = fa; - isSynchronized = false; - } - - public void setSynchronized() { - isSynchronized = true; - } - - public FieldAnnotation getFieldAnnotation() { - return fieldAnnotation; - } - - public boolean isSynchronized() { - return isSynchronized; - } - } + private static JavaClass collectionClass; + + static { + try { + collectionClass = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_COLLECTION); + } catch (ClassNotFoundException cnfe) { + collectionClass = null; + } + } + + private static JavaClass mapClass; + + static { + try { + mapClass = Repository.lookupClass(Values.SLASHED_JAVA_UTIL_MAP); + } catch (ClassNotFoundException cnfe) { + mapClass = null; + } + } + + private static Set syncCollections = UnmodifiableSet.create("java/util/Vector", "java/util/Hashtable"); + + private static Set modifyingMethods = UnmodifiableSet.create("add", "addAll", "addFirst", "addElement", + "addLast", "clear", "compute", "computeIfAbsent", "computeIfPresent", "insertElementAt", "merge", "put", + "putAll", "putIfAbsent", "remove", "removeAll", "removeAllElements", "removeElement", "removeElementAt", + "removeFirst", "removeLast", "removeRange", "replace", "replaceAll", "retainAll", "set", "setElementAt", + "setSize"); + + private enum State { + IN_METHOD, IN_CLINIT, IN_INIT + }; + + private BugReporter bugReporter; + private Map collectionFields; + private Map aliases; + private OpcodeStack stack; + private State state; + private String className; + + /** + * constructs a NMCS detector given the reporter to report bugs on + * + * @param bugReporter the sync of bug reports + */ + public NeedlessMemberCollectionSynchronization(BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + /** + * implements the visitor to clear the collectionFields and stack and to report + * collections that remain unmodified out of clinit or init + * + * @param classContext the context object of the currently parsed class + */ + @Override + public void visitClassContext(ClassContext classContext) { + try { + if ((collectionClass != null) && (mapClass != null)) { + collectionFields = new HashMap<>(); + aliases = new HashMap<>(); + stack = new OpcodeStack(); + JavaClass cls = classContext.getJavaClass(); + className = cls.getClassName(); + super.visitClassContext(classContext); + for (FieldInfo fi : collectionFields.values()) { + if (fi.isSynchronized()) { + bugReporter.reportBug( + new BugInstance(this, BugType.NMCS_NEEDLESS_MEMBER_COLLECTION_SYNCHRONIZATION.name(), + NORMAL_PRIORITY).addClass(this).addField(fi.getFieldAnnotation())); + } + } + } + } finally { + collectionFields = null; + aliases = null; + stack = null; + } + } + + /** + * implements the visitor to find collection fields + * + * @param obj the context object of the currently parse field + */ + @Override + public void visitField(Field obj) { + if (obj.isPrivate()) { + String signature = obj.getSignature(); + if (signature.startsWith(Values.SIG_QUALIFIED_CLASS_PREFIX)) { + try { + JavaClass cls = Repository.lookupClass(SignatureUtils.stripSignature(signature)); + if (cls.implementationOf(collectionClass) || cls.implementationOf(mapClass)) { + FieldAnnotation fa = FieldAnnotation.fromVisitedField(this); + collectionFields.put(fa.getFieldName(), new FieldInfo(fa)); + } + } catch (ClassNotFoundException cnfe) { + bugReporter.reportMissingClass(cnfe); + } + } + } + } + + /** + * implements the visitor to set the state based on the type of method being + * parsed + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(Code obj) { + if (collectionFields.isEmpty()) { + return; + } + aliases.clear(); + String methodName = getMethodName(); + if (Values.STATIC_INITIALIZER.equals(methodName)) { + state = State.IN_CLINIT; + } else if (Values.CONSTRUCTOR.equals(methodName)) { + state = State.IN_INIT; + } else { + state = State.IN_METHOD; + } + stack.resetForMethodEntry(this); + super.visitCode(obj); + } + + /** + * implements the visitor to call the approriate visitor based on state + * + * @param seen the opcode of the currently parsed instruction + */ + @Override + public void sawOpcode(int seen) { + switch (state) { + case IN_CLINIT: + sawCLInitOpcode(seen); + break; + + case IN_INIT: + sawInitOpcode(seen); + break; + + case IN_METHOD: + sawMethodOpcode(seen); + break; + } + } + + /** + * + * handle {@code } blocks by looking for putstatic calls referencing + * synchronized collections + * + * @param seen the opcode of the currently parsed instruction + */ + private void sawCLInitOpcode(int seen) { + boolean isSyncCollection = false; + try { + stack.precomputation(this); + + isSyncCollection = isSyncCollectionCreation(seen); + if (seen == PUTSTATIC) { + processCollectionStore(); + } + } finally { + stack.sawOpcode(this, seen); + if (isSyncCollection && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + item.setUserValue(Boolean.TRUE); + } + } + } + + /** + * handle {@code } blocks by looking for putfield calls referencing + * synchronized collections + * + * @param seen the opcode of the currently parsed instruction + */ + private void sawInitOpcode(int seen) { + boolean isSyncCollection = false; + try { + stack.mergeJumps(this); + isSyncCollection = isSyncCollectionCreation(seen); + if (seen == PUTFIELD) { + processCollectionStore(); + } + } finally { + stack.sawOpcode(this, seen); + if (isSyncCollection && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + item.setUserValue(Boolean.TRUE); + } + } + } + + /** + * handles regular methods by looking for methods on collections that are + * modifying and removes those collections from the ones under review + * + * @param seen the opcode of the currently parsed instruction + */ + private void sawMethodOpcode(int seen) { + boolean isSyncCollection = false; + try { + stack.mergeJumps(this); + isSyncCollection = isSyncCollectionCreation(seen); + + switch (seen) { + case INVOKEVIRTUAL: + case INVOKEINTERFACE: + String methodName = getNameConstantOperand(); + if (modifyingMethods.contains(methodName)) { + String signature = getSigConstantOperand(); + int parmCount = SignatureUtils.getNumParameters(signature); + if (stack.getStackDepth() > parmCount) { + OpcodeStack.Item item = stack.getStackItem(parmCount); + XField field = item.getXField(); + if (field != null) { + collectionFields.remove(field.getName()); + } else { + int reg = item.getRegisterNumber(); + if (reg >= 0) { + Integer register = Integer.valueOf(reg); + String fName = aliases.get(register); + if (fName != null) { + collectionFields.remove(fName); + aliases.remove(register); + } + } + } + } + } + removeCollectionParameters(); + break; + + case INVOKESTATIC: + removeCollectionParameters(); + break; + + case ARETURN: + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + XField field = item.getXField(); + if (field != null) { + collectionFields.remove(field.getName()); + } + } + break; + + case PUTFIELD: + case PUTSTATIC: + String fieldName = getNameConstantOperand(); + collectionFields.remove(fieldName); + break; + + case GOTO: + case GOTO_W: + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + XField field = item.getXField(); + if (field != null) { + collectionFields.remove(field.getName()); + } + } + break; + default: + break; + } + } finally { + TernaryPatcher.pre(stack, seen); + stack.sawOpcode(this, seen); + TernaryPatcher.post(stack, seen); + if (isSyncCollection && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + item.setUserValue(Boolean.TRUE); + } + } + + } + + /** + * returns whether this instruction is creating a synchronized collection + * + * @param seen the opcode of the currently parsed instruction + * @return whether a synchronized collection has just been created + */ + private boolean isSyncCollectionCreation(int seen) { + if (seen == INVOKESPECIAL) { + if (Values.CONSTRUCTOR.equals(getNameConstantOperand())) { + return (syncCollections.contains(getClassConstantOperand())); + } + } else if ((seen == INVOKESTATIC) && "java/util/Collections".equals(getClassConstantOperand())) { + String methodName = getNameConstantOperand(); + return ("synchronizedMap".equals(methodName) || "synchronizedSet".equals(methodName)); + } + return false; + } + + /** + * sets the source line annotation of a store to a collection if that collection + * is synchronized. + */ + private void processCollectionStore() { + String fieldClassName = getDottedClassConstantOperand(); + if (fieldClassName.equals(className) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + if (item.getUserValue() != null) { + String fieldName = getNameConstantOperand(); + if (fieldName != null) { + FieldInfo fi = collectionFields.get(fieldName); + if (fi != null) { + fi.getFieldAnnotation().setSourceLines(SourceLineAnnotation.fromVisitedInstruction(this)); + fi.setSynchronized(); + } + } + } + } + } + + /** + * removes collection fields that are passed to other methods as arguments + */ + private void removeCollectionParameters() { + int parmCount = SignatureUtils.getNumParameters(getSigConstantOperand()); + if (stack.getStackDepth() >= parmCount) { + for (int i = 0; i < parmCount; i++) { + OpcodeStack.Item item = stack.getStackItem(i); + XField field = item.getXField(); + if (field != null) { + collectionFields.remove(field.getName()); + } + } + } + } + + /** + * holds information about a field, namely the annotation and whether the + * collection is synchronized. + */ + static class FieldInfo { + private FieldAnnotation fieldAnnotation; + private boolean isSynchronized; + + public FieldInfo(FieldAnnotation fa) { + fieldAnnotation = fa; + isSynchronized = false; + } + + public void setSynchronized() { + isSynchronized = true; + } + + public FieldAnnotation getFieldAnnotation() { + return fieldAnnotation; + } + + public boolean isSynchronized() { + return isSynchronized; + } + } } diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/PossiblyRedundantMethodCalls.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/PossiblyRedundantMethodCalls.java index d10601c0..91bed1cf 100755 --- a/src/main/java/com/mebigfatguy/fbcontrib/detect/PossiblyRedundantMethodCalls.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/PossiblyRedundantMethodCalls.java @@ -59,123 +59,123 @@ */ @CustomUserValue public class PossiblyRedundantMethodCalls extends BytecodeScanningDetector { - public static final String PRMC_RISKY_FIELD_USER_KEY = "fbcontrib.PRMC.riskynames"; - public static final String PRMC_RISKY_CLASS_USER_KEY = "fbcontrib.PRMC.riskyclasses"; - public static final String PRMC_HIGH_BYTECOUNT = "fbcontrib.PRMC.highbytecount"; - public static final String PRMC_HIGH_METHODCALLS = "fbcontrib.PRMC.highmethodcalls"; - public static final String PRMC_NORMAL_BYTECOUNT = "fbcontrib.PRMC.normalbytecount"; - public static final String PRMC_NORMAL_METHODCALLS = "fbcontrib.PRMC.normalmethodcalls"; - public static final String PRMC_LOW_BYTECOUNT = "fbcontrib.PRMC.lowbytecount"; - public static final String PRMC_LOW_METHODCALLS = "fbcontrib.PRMC.lowmethodcalls"; - - /** - * a collection of names that are to be checked against a currently parsed - * method, to see if that method is risky to be called redundant. The contents - * are either - *
    - *
  • a simple name that can be found as part of the methodName, like - * "destroy" which would match destroy(), or destroyAll()
    • - *
  • a fully qualified method name that exactly matches a method, like - * "java/lang/String.valueOf"
    • - *
- */ - private static Set riskyMethodNameContents = new HashSet<>(); - private static int highByteCountLimit = 200; - private static int highMethodCallLimit = 10; - private static int normalByteCountLimit = 75; - private static int normalMethodCallLimit = 4; - private static int lowByteCountLimit = 10; - private static int lowMethodCallLimit = 1; - - static { - riskyMethodNameContents.add("next"); - riskyMethodNameContents.add("add"); - riskyMethodNameContents.add("create"); - riskyMethodNameContents.add("append"); - riskyMethodNameContents.add("find"); - riskyMethodNameContents.add("put"); - riskyMethodNameContents.add("remove"); - riskyMethodNameContents.add("read"); - riskyMethodNameContents.add("write"); - riskyMethodNameContents.add("push"); - riskyMethodNameContents.add("pop"); - riskyMethodNameContents.add("scan"); - riskyMethodNameContents.add("skip"); - riskyMethodNameContents.add("clone"); - riskyMethodNameContents.add("close"); - riskyMethodNameContents.add("copy"); - riskyMethodNameContents.add("currentTimeMillis"); - riskyMethodNameContents.add("insert"); - riskyMethodNameContents.add("nanoTime"); - riskyMethodNameContents.add("new"); - riskyMethodNameContents.add("noneOf"); - riskyMethodNameContents.add("now"); - riskyMethodNameContents.add("allOf"); - riskyMethodNameContents.add("random"); - riskyMethodNameContents.add("beep"); - riskyMethodNameContents.add("emptyList"); - riskyMethodNameContents.add("emptySet"); - riskyMethodNameContents.add("emptyMap"); - riskyMethodNameContents.add("generate"); - riskyMethodNameContents.add("stream"); - - String userNameProp = System.getProperty(PRMC_RISKY_FIELD_USER_KEY); - if (userNameProp != null) { - String[] userNames = userNameProp.split(Values.WHITESPACE_COMMA_SPLIT); - for (String name : userNames) { - riskyMethodNameContents.add(name); - } - } - Integer prop = Integer.getInteger(PRMC_HIGH_BYTECOUNT); - if (prop != null) { - highByteCountLimit = prop.intValue(); - } - prop = Integer.getInteger(PRMC_HIGH_METHODCALLS); - if (prop != null) { - highMethodCallLimit = prop.intValue(); - } - prop = Integer.getInteger(PRMC_NORMAL_BYTECOUNT); - if (prop != null) { - normalByteCountLimit = prop.intValue(); - } - prop = Integer.getInteger(PRMC_NORMAL_METHODCALLS); - if (prop != null) { - normalMethodCallLimit = prop.intValue(); - } - prop = Integer.getInteger(PRMC_LOW_BYTECOUNT); - if (prop != null) { - lowByteCountLimit = prop.intValue(); - } - prop = Integer.getInteger(PRMC_LOW_METHODCALLS); - if (prop != null) { - lowMethodCallLimit = prop.intValue(); - } - } - - private static Set riskyClassNames = new HashSet<>(); - - static { - riskyClassNames.add("java/nio/ByteBuffer"); - riskyClassNames.add("java/io/DataInputStream"); - riskyClassNames.add("java/io/ObjectInputStream"); - riskyClassNames.add("java/util/Calendar"); - riskyClassNames.add("java/util/stream/Collectors"); - riskyClassNames.add("com/google/common/collect/Lists"); - riskyClassNames.add("com/google/common/collect/Sets"); - riskyClassNames.add("com/google/common/collect/Maps"); - riskyClassNames.add("com/google/common/collect/Queues"); - - String userNameProp = System.getProperty(PRMC_RISKY_CLASS_USER_KEY); - if (userNameProp != null) { - String[] userNames = userNameProp.split(Values.WHITESPACE_COMMA_SPLIT); - for (String name : userNames) { - riskyClassNames.add(name.replace('.', '.')); - } - } - } - - private static final Set commonMethods = UnmodifiableSet.create( - // @formatter:off + public static final String PRMC_RISKY_FIELD_USER_KEY = "fbcontrib.PRMC.riskynames"; + public static final String PRMC_RISKY_CLASS_USER_KEY = "fbcontrib.PRMC.riskyclasses"; + public static final String PRMC_HIGH_BYTECOUNT = "fbcontrib.PRMC.highbytecount"; + public static final String PRMC_HIGH_METHODCALLS = "fbcontrib.PRMC.highmethodcalls"; + public static final String PRMC_NORMAL_BYTECOUNT = "fbcontrib.PRMC.normalbytecount"; + public static final String PRMC_NORMAL_METHODCALLS = "fbcontrib.PRMC.normalmethodcalls"; + public static final String PRMC_LOW_BYTECOUNT = "fbcontrib.PRMC.lowbytecount"; + public static final String PRMC_LOW_METHODCALLS = "fbcontrib.PRMC.lowmethodcalls"; + + /** + * a collection of names that are to be checked against a currently parsed + * method, to see if that method is risky to be called redundant. The contents + * are either + *
    + *
  • a simple name that can be found as part of the methodName, like + * "destroy" which would match destroy(), or destroyAll()
    • + *
  • a fully qualified method name that exactly matches a method, like + * "java/lang/String.valueOf"
    • + *
+ */ + private static Set riskyMethodNameContents = new HashSet<>(); + private static int highByteCountLimit = 200; + private static int highMethodCallLimit = 10; + private static int normalByteCountLimit = 75; + private static int normalMethodCallLimit = 4; + private static int lowByteCountLimit = 10; + private static int lowMethodCallLimit = 1; + + static { + riskyMethodNameContents.add("next"); + riskyMethodNameContents.add("add"); + riskyMethodNameContents.add("create"); + riskyMethodNameContents.add("append"); + riskyMethodNameContents.add("find"); + riskyMethodNameContents.add("put"); + riskyMethodNameContents.add("remove"); + riskyMethodNameContents.add("read"); + riskyMethodNameContents.add("write"); + riskyMethodNameContents.add("push"); + riskyMethodNameContents.add("pop"); + riskyMethodNameContents.add("scan"); + riskyMethodNameContents.add("skip"); + riskyMethodNameContents.add("clone"); + riskyMethodNameContents.add("close"); + riskyMethodNameContents.add("copy"); + riskyMethodNameContents.add("currentTimeMillis"); + riskyMethodNameContents.add("insert"); + riskyMethodNameContents.add("nanoTime"); + riskyMethodNameContents.add("new"); + riskyMethodNameContents.add("noneOf"); + riskyMethodNameContents.add("now"); + riskyMethodNameContents.add("allOf"); + riskyMethodNameContents.add("random"); + riskyMethodNameContents.add("beep"); + riskyMethodNameContents.add("emptyList"); + riskyMethodNameContents.add("emptySet"); + riskyMethodNameContents.add("emptyMap"); + riskyMethodNameContents.add("generate"); + riskyMethodNameContents.add("stream"); + + String userNameProp = System.getProperty(PRMC_RISKY_FIELD_USER_KEY); + if (userNameProp != null) { + String[] userNames = userNameProp.split(Values.WHITESPACE_COMMA_SPLIT); + for (String name : userNames) { + riskyMethodNameContents.add(name); + } + } + Integer prop = Integer.getInteger(PRMC_HIGH_BYTECOUNT); + if (prop != null) { + highByteCountLimit = prop.intValue(); + } + prop = Integer.getInteger(PRMC_HIGH_METHODCALLS); + if (prop != null) { + highMethodCallLimit = prop.intValue(); + } + prop = Integer.getInteger(PRMC_NORMAL_BYTECOUNT); + if (prop != null) { + normalByteCountLimit = prop.intValue(); + } + prop = Integer.getInteger(PRMC_NORMAL_METHODCALLS); + if (prop != null) { + normalMethodCallLimit = prop.intValue(); + } + prop = Integer.getInteger(PRMC_LOW_BYTECOUNT); + if (prop != null) { + lowByteCountLimit = prop.intValue(); + } + prop = Integer.getInteger(PRMC_LOW_METHODCALLS); + if (prop != null) { + lowMethodCallLimit = prop.intValue(); + } + } + + private static Set riskyClassNames = new HashSet<>(); + + static { + riskyClassNames.add("java/nio/ByteBuffer"); + riskyClassNames.add("java/io/DataInputStream"); + riskyClassNames.add("java/io/ObjectInputStream"); + riskyClassNames.add("java/util/Calendar"); + riskyClassNames.add("java/util/stream/Collectors"); + riskyClassNames.add("com/google/common/collect/Lists"); + riskyClassNames.add("com/google/common/collect/Sets"); + riskyClassNames.add("com/google/common/collect/Maps"); + riskyClassNames.add("com/google/common/collect/Queues"); + + String userNameProp = System.getProperty(PRMC_RISKY_CLASS_USER_KEY); + if (userNameProp != null) { + String[] userNames = userNameProp.split(Values.WHITESPACE_COMMA_SPLIT); + for (String name : userNames) { + riskyClassNames.add(name.replace('.', '.')); + } + } + } + + private static final Set commonMethods = UnmodifiableSet.create( + // @formatter:off new FQMethod("java/lang/Boolean", "valueOf", "(Z)Ljava/lang/Boolean;"), new FQMethod("java/lang/Byte", "valueOf", "(B)Ljava/lang/Byte;"), new FQMethod("java/lang/Character", "valueOf", "(C)Ljava/lang/Character;"), @@ -191,445 +191,445 @@ public class PossiblyRedundantMethodCalls extends BytecodeScanningDetector { new FQMethod("java/lang/Double", "doubleValue", "()D"), new FQMethod("java/util/Optional", "empty", "()Ljava/util/Optional;") // @formatter:on - ); - - private final BugReporter bugReporter; - private OpcodeStack stack = null; - private Map localMethodCalls = null; - private Map fieldMethodCalls = null; - private Map staticMethodCalls = null; - private BitSet branchTargets = null; - - /** - * constructs a PRMC detector given the reporter to report bugs on - * - * @param bugReporter the sync of bug reports - */ - public PossiblyRedundantMethodCalls(BugReporter bugReporter) { - this.bugReporter = bugReporter; - } - - /** - * implements the visitor to create and clear the stack, method call maps, and - * branch targets - * - * @param classContext the context object of the currently visited class - */ - @Override - public void visitClassContext(ClassContext classContext) { - try { - stack = new OpcodeStack(); - localMethodCalls = new HashMap<>(); - fieldMethodCalls = new HashMap<>(); - staticMethodCalls = new HashMap<>(); - branchTargets = new BitSet(); - super.visitClassContext(classContext); - } finally { - stack = null; - localMethodCalls = null; - fieldMethodCalls = null; - staticMethodCalls = null; - branchTargets = null; - } - } - - /** - * implements the visitor to reset the stack, and method call maps for new - * method Note: that when collecting branch targets, it's unfortunately not good - * enough to just collect the handler pcs, as javac plays fast and loose, and - * will sometimes jam code below the end pc and before the first handler pc, - * which gets executed. So we need to clear our maps if we go past the end pc as - * well. - * - * @param obj the context object of the currently parsed code block - */ - @Override - public void visitCode(Code obj) { - stack.resetForMethodEntry(this); - localMethodCalls.clear(); - fieldMethodCalls.clear(); - staticMethodCalls.clear(); - branchTargets.clear(); - CodeException[] codeExceptions = obj.getExceptionTable(); - for (CodeException codeEx : codeExceptions) { - // adding the end pc seems silly, but it is need because javac may repeat - // part of the finally block in the try block, at times. - branchTargets.set(codeEx.getEndPC()); - branchTargets.set(codeEx.getHandlerPC()); - } - super.visitCode(obj); - } - - /** - * implements the visitor to look for repetitive calls to the same method on the - * same object using the same constant parameters. These methods must return a - * value. - * - * @param seen the opcode of the currently parsed instruction - */ - @Override - public void sawOpcode(int seen) { - String userValue = null; - - try { - stack.precomputation(this); - - int pc = getPC(); - if (branchTargets.get(pc)) { - localMethodCalls.clear(); - fieldMethodCalls.clear(); - branchTargets.clear(pc); - } - - if (((seen >= IFEQ) && (seen <= GOTO)) || ((seen >= IFNULL) && (seen <= GOTO_W))) { - branchTargets.set(getBranchTarget()); - } else if ((seen == TABLESWITCH) || (seen == LOOKUPSWITCH)) { - int[] offsets = getSwitchOffsets(); - for (int offset : offsets) { - branchTargets.set(offset + pc); - } - branchTargets.set(getDefaultSwitchOffset() + pc); - } else if (OpcodeUtils.isAStore(seen)) { - localMethodCalls.remove(Integer.valueOf(RegisterUtils.getAStoreReg(this, seen))); - } else if (seen == PUTFIELD) { - String fieldSource = ""; - - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - fieldSource = (String) item.getUserValue(); - if (fieldSource == null) { - fieldSource = ""; - } - } - fieldMethodCalls.remove(new FieldInfo(fieldSource, getNameConstantOperand())); - } else if (seen == GETFIELD) { - if (stack.getStackDepth() > 0) { - OpcodeStack.Item item = stack.getStackItem(0); - userValue = (String) item.getUserValue(); - if (userValue == null) { - int reg = item.getRegisterNumber(); - if (reg >= 0) { - userValue = String.valueOf(reg); - } else { - XField xf = item.getXField(); - if (xf != null) { - userValue = xf.getName(); - } - } - } - } - } else if ((seen == INVOKEVIRTUAL) || (seen == INVOKEINTERFACE) || (seen == INVOKESTATIC)) { - - String className = getClassConstantOperand(); - String methodName = getNameConstantOperand(); - String signature = getSigConstantOperand(); - int parmCount = SignatureUtils.getNumParameters(signature); - - int reg = -1; - XField field = null; - MethodCall mc = null; - String fieldSource = null; - if (seen == INVOKESTATIC) { - XMethod xm = XFactory.createXMethod(getDottedClassConstantOperand(), methodName, signature, true); - String genericSignature = xm.getSourceSignature(); - if ((genericSignature != null) && genericSignature.endsWith(">;")) { - return; - } - } else if (stack.getStackDepth() > parmCount) { - OpcodeStack.Item obj = stack.getStackItem(parmCount); - reg = obj.getRegisterNumber(); - field = obj.getXField(); - - if (reg >= 0) { - mc = localMethodCalls.get(Integer.valueOf(reg)); - MethodInfo mi = Statistics.getStatistics().getMethodStatistics(className, - getNameConstantOperand(), signature); - if ((mi != null) && mi.getModifiesState()) { - clearFieldMethods(String.valueOf(reg)); - return; - } - } else if (field != null) { - fieldSource = (String) obj.getUserValue(); - if (fieldSource == null) { - fieldSource = ""; - } - mc = fieldMethodCalls.get(new FieldInfo(fieldSource, field.getName())); - MethodInfo mi = Statistics.getStatistics().getMethodStatistics(className, - getNameConstantOperand(), signature); - if ((mi != null) && mi.getModifiesState()) { - clearFieldMethods(fieldSource); - return; - } - } - } - - int neededStackSize = parmCount + ((seen == INVOKESTATIC) ? 0 : 1); - if (stack.getStackDepth() >= neededStackSize) { - Object[] parmConstants = new Object[parmCount]; - for (int i = 0; i < parmCount; i++) { - OpcodeStack.Item parm = stack.getStackItem(i); - parmConstants[i] = parm.getConstant(); - if (parm.getSignature().startsWith(Values.SIG_ARRAY_PREFIX)) { - if (!Values.ZERO.equals(parm.getConstant())) { - return; - } - XField f = parm.getXField(); - if (f != null) { - // Two different fields holding a 0 length array should be considered different - parmConstants[i] = f.getName() + ':' + parmConstants[i]; - } - } - - if (parmConstants[i] == null) { - return; - } - } - - if (seen == INVOKESTATIC) { - mc = staticMethodCalls.get(className); - } else if ((reg < 0) && (field == null)) { - return; - } - - if (mc != null) { - if (!signature.endsWith(Values.SIG_VOID) && methodName.equals(mc.getName()) - && signature.equals(mc.getSignature()) && !isRiskyName(className, methodName) - && !commonMethods.contains(new FQMethod(className, methodName, signature))) { - Object[] parms = mc.getParms(); - if (Arrays.equals(parms, parmConstants)) { - int ln = getLineNumber(pc); - - if ((ln != mc.getLineNumber()) || (Math.abs(pc - mc.getPC()) < 10)) { - Statistics statistics = Statistics.getStatistics(); - MethodInfo mi = statistics.getMethodStatistics(className, methodName, signature); - - bugReporter.reportBug( - new BugInstance(this, BugType.PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS.name(), - getBugPriority(methodName, mi)).addClass(this).addMethod(this) - .addSourceLine(this).addString(methodName + signature)); - } - } - } - - if (seen == INVOKESTATIC) { - staticMethodCalls.remove(className); - } else { - if (reg >= 0) { - localMethodCalls.remove(Integer.valueOf(reg)); - } else if (fieldSource != null) { - fieldMethodCalls.remove(new FieldInfo(fieldSource, field.getName())); - } - } - } else { - int ln = getLineNumber(pc); - if (seen == INVOKESTATIC) { - staticMethodCalls.put(className, - new MethodCall(methodName, signature, parmConstants, pc, ln)); - } else { - if (reg >= 0) { - localMethodCalls.put(Integer.valueOf(reg), - new MethodCall(methodName, signature, parmConstants, pc, ln)); - } else if (field != null) { - OpcodeStack.Item obj = stack.getStackItem(parmCount); - fieldSource = (String) obj.getUserValue(); - if (fieldSource == null) { - fieldSource = ""; - } - fieldMethodCalls.put(new FieldInfo(fieldSource, field.getName()), - new MethodCall(methodName, signature, parmConstants, pc, ln)); - } - } - } - } - } else if (OpcodeUtils.isReturn(seen)) { - localMethodCalls.clear(); - fieldMethodCalls.clear(); - branchTargets.clear(pc); - } - } finally { - stack.sawOpcode(this, seen); - if ((userValue != null) && (stack.getStackDepth() > 0)) { - OpcodeStack.Item item = stack.getStackItem(0); - item.setUserValue(userValue); - } - } - } - - private void clearFieldMethods(String fieldSource) { - Iterator it = fieldMethodCalls.keySet().iterator(); - while (it.hasNext()) { - if (it.next().hasFieldSource(fieldSource)) { - it.remove(); - } - } - } - - /** - * returns the bug priority based on metrics about the method - * - * @param methodName TODO - * @param mi metrics about the method - * @return the bug priority - */ - private static int getBugPriority(String methodName, MethodInfo mi) { - if (Values.STATIC_INITIALIZER.equals(methodName)) { - return LOW_PRIORITY; - } - - if ((mi.getNumBytes() >= highByteCountLimit) || (mi.getNumMethodCalls() >= highMethodCallLimit)) { - return HIGH_PRIORITY; - } - - if ((mi.getNumBytes() >= normalByteCountLimit) || (mi.getNumMethodCalls() >= normalMethodCallLimit)) { - return NORMAL_PRIORITY; - } - - if ((mi.getNumBytes() >= lowByteCountLimit) || (mi.getNumMethodCalls() >= lowMethodCallLimit)) { - return LOW_PRIORITY; - } - - return EXP_PRIORITY; - } - - /** - * returns true if the class or method name contains a pattern that is - * considered likely to be this modifying - * - * @param className the class name to check - * @param methodName the method name to check - * @return whether the method sounds like it modifies this - */ - private static boolean isRiskyName(String className, String methodName) { - if (riskyClassNames.contains(className)) { - return true; - } - - String qualifiedMethodName = className + '.' + methodName; - if (riskyMethodNameContents.contains(qualifiedMethodName)) { - return true; - } - - for (String riskyName : riskyMethodNameContents) { - if (methodName.indexOf(riskyName) >= 0) { - return true; - } - } - - return methodName.indexOf(Values.SYNTHETIC_MEMBER_CHAR) >= 0; - } - - /** - * returns the source line number for the pc, or just the pc if the line number - * table doesn't exist - * - * @param pc current pc - * @return the line number - */ - private int getLineNumber(int pc) { - LineNumberTable lnt = getMethod().getLineNumberTable(); - if (lnt == null) { - return pc; - } - - LineNumber[] lns = lnt.getLineNumberTable(); - if (lns == null) { - return pc; - } - - if (pc > lns[lns.length - 1].getStartPC()) { - return lns[lns.length - 1].getLineNumber(); - } - - int lo = 0; - int hi = lns.length - 2; - int mid = 0; - while (lo <= hi) { - mid = (lo + hi) >>> 1; - if (pc < lns[mid].getStartPC()) { - hi = mid - 1; - } else if (pc >= lns[mid + 1].getStartPC()) { - lo = mid + 1; - } else { - break; - } - } - - return lns[mid].getLineNumber(); - } - - /** - * contains information about a field access - */ - static class FieldInfo { - private final String fieldSource; - private final String fieldName; - - public FieldInfo(String source, String name) { - fieldSource = source; - fieldName = name; - } - - public boolean hasFieldSource(String source) { - return fieldSource.equals(source); - } - - @Override - public int hashCode() { - return fieldSource.hashCode() ^ fieldName.hashCode(); - } - - @Override - public boolean equals(Object o) { - if (!(o instanceof FieldInfo)) { - return false; - } - - FieldInfo that = (FieldInfo) o; - return fieldSource.equals(that.fieldSource) && fieldName.equals(that.fieldName); - } - - @Override - public String toString() { - return ToString.build(this); - } - } - - /** - * contains information about a method call - */ - static class MethodCall { - private final String methodName; - private final String methodSignature; - private final Object[] methodParms; - private final int methodPC; - private final int methodLineNumber; - - public MethodCall(String name, String signature, Object[] parms, int pc, int lineNumber) { - methodName = name; - methodSignature = signature; - methodParms = parms; - methodPC = pc; - methodLineNumber = lineNumber; - } - - public String getName() { - return methodName; - } - - public String getSignature() { - return methodSignature; - } - - public Object[] getParms() { - return methodParms; - } - - public int getPC() { - return methodPC; - } - - public int getLineNumber() { - return methodLineNumber; - } - } + ); + + private final BugReporter bugReporter; + private OpcodeStack stack = null; + private Map localMethodCalls = null; + private Map fieldMethodCalls = null; + private Map staticMethodCalls = null; + private BitSet branchTargets = null; + + /** + * constructs a PRMC detector given the reporter to report bugs on + * + * @param bugReporter the sync of bug reports + */ + public PossiblyRedundantMethodCalls(BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + /** + * implements the visitor to create and clear the stack, method call maps, and + * branch targets + * + * @param classContext the context object of the currently visited class + */ + @Override + public void visitClassContext(ClassContext classContext) { + try { + stack = new OpcodeStack(); + localMethodCalls = new HashMap<>(); + fieldMethodCalls = new HashMap<>(); + staticMethodCalls = new HashMap<>(); + branchTargets = new BitSet(); + super.visitClassContext(classContext); + } finally { + stack = null; + localMethodCalls = null; + fieldMethodCalls = null; + staticMethodCalls = null; + branchTargets = null; + } + } + + /** + * implements the visitor to reset the stack, and method call maps for new + * method Note: that when collecting branch targets, it's unfortunately not good + * enough to just collect the handler pcs, as javac plays fast and loose, and + * will sometimes jam code below the end pc and before the first handler pc, + * which gets executed. So we need to clear our maps if we go past the end pc as + * well. + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(Code obj) { + stack.resetForMethodEntry(this); + localMethodCalls.clear(); + fieldMethodCalls.clear(); + staticMethodCalls.clear(); + branchTargets.clear(); + CodeException[] codeExceptions = obj.getExceptionTable(); + for (CodeException codeEx : codeExceptions) { + // adding the end pc seems silly, but it is need because javac may repeat + // part of the finally block in the try block, at times. + branchTargets.set(codeEx.getEndPC()); + branchTargets.set(codeEx.getHandlerPC()); + } + super.visitCode(obj); + } + + /** + * implements the visitor to look for repetitive calls to the same method on the + * same object using the same constant parameters. These methods must return a + * value. + * + * @param seen the opcode of the currently parsed instruction + */ + @Override + public void sawOpcode(int seen) { + String userValue = null; + + try { + stack.precomputation(this); + + int pc = getPC(); + if (branchTargets.get(pc)) { + localMethodCalls.clear(); + fieldMethodCalls.clear(); + branchTargets.clear(pc); + } + + if (((seen >= IFEQ) && (seen <= GOTO)) || ((seen >= IFNULL) && (seen <= GOTO_W))) { + branchTargets.set(getBranchTarget()); + } else if ((seen == TABLESWITCH) || (seen == LOOKUPSWITCH)) { + int[] offsets = getSwitchOffsets(); + for (int offset : offsets) { + branchTargets.set(offset + pc); + } + branchTargets.set(getDefaultSwitchOffset() + pc); + } else if (OpcodeUtils.isAStore(seen)) { + localMethodCalls.remove(Integer.valueOf(RegisterUtils.getAStoreReg(this, seen))); + } else if (seen == PUTFIELD) { + String fieldSource = ""; + + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + fieldSource = (String) item.getUserValue(); + if (fieldSource == null) { + fieldSource = ""; + } + } + fieldMethodCalls.remove(new FieldInfo(fieldSource, getNameConstantOperand())); + } else if (seen == GETFIELD) { + if (stack.getStackDepth() > 0) { + OpcodeStack.Item item = stack.getStackItem(0); + userValue = (String) item.getUserValue(); + if (userValue == null) { + int reg = item.getRegisterNumber(); + if (reg >= 0) { + userValue = String.valueOf(reg); + } else { + XField xf = item.getXField(); + if (xf != null) { + userValue = xf.getName(); + } + } + } + } + } else if ((seen == INVOKEVIRTUAL) || (seen == INVOKEINTERFACE) || (seen == INVOKESTATIC)) { + + String className = getClassConstantOperand(); + String methodName = getNameConstantOperand(); + String signature = getSigConstantOperand(); + int parmCount = SignatureUtils.getNumParameters(signature); + + int reg = -1; + XField field = null; + MethodCall mc = null; + String fieldSource = null; + if (seen == INVOKESTATIC) { + XMethod xm = XFactory.createXMethod(getDottedClassConstantOperand(), methodName, signature, true); + String genericSignature = xm.getSourceSignature(); + if ((genericSignature != null) && genericSignature.endsWith(">;")) { + return; + } + } else if (stack.getStackDepth() > parmCount) { + OpcodeStack.Item obj = stack.getStackItem(parmCount); + reg = obj.getRegisterNumber(); + field = obj.getXField(); + + if (reg >= 0) { + mc = localMethodCalls.get(Integer.valueOf(reg)); + MethodInfo mi = Statistics.getStatistics().getMethodStatistics(className, + getNameConstantOperand(), signature); + if ((mi != null) && mi.getModifiesState()) { + clearFieldMethods(String.valueOf(reg)); + return; + } + } else if (field != null) { + fieldSource = (String) obj.getUserValue(); + if (fieldSource == null) { + fieldSource = ""; + } + mc = fieldMethodCalls.get(new FieldInfo(fieldSource, field.getName())); + MethodInfo mi = Statistics.getStatistics().getMethodStatistics(className, + getNameConstantOperand(), signature); + if ((mi != null) && mi.getModifiesState()) { + clearFieldMethods(fieldSource); + return; + } + } + } + + int neededStackSize = parmCount + ((seen == INVOKESTATIC) ? 0 : 1); + if (stack.getStackDepth() >= neededStackSize) { + Object[] parmConstants = new Object[parmCount]; + for (int i = 0; i < parmCount; i++) { + OpcodeStack.Item parm = stack.getStackItem(i); + parmConstants[i] = parm.getConstant(); + if (parm.getSignature().startsWith(Values.SIG_ARRAY_PREFIX)) { + if (!Values.ZERO.equals(parm.getConstant())) { + return; + } + XField f = parm.getXField(); + if (f != null) { + // Two different fields holding a 0 length array should be considered different + parmConstants[i] = f.getName() + ':' + parmConstants[i]; + } + } + + if (parmConstants[i] == null) { + return; + } + } + + if (seen == INVOKESTATIC) { + mc = staticMethodCalls.get(className); + } else if ((reg < 0) && (field == null)) { + return; + } + + if (mc != null) { + if (!signature.endsWith(Values.SIG_VOID) && methodName.equals(mc.getName()) + && signature.equals(mc.getSignature()) && !isRiskyName(className, methodName) + && !commonMethods.contains(new FQMethod(className, methodName, signature))) { + Object[] parms = mc.getParms(); + if (Arrays.equals(parms, parmConstants)) { + int ln = getLineNumber(pc); + + if ((ln != mc.getLineNumber()) || (Math.abs(pc - mc.getPC()) < 10)) { + Statistics statistics = Statistics.getStatistics(); + MethodInfo mi = statistics.getMethodStatistics(className, methodName, signature); + + bugReporter.reportBug( + new BugInstance(this, BugType.PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS.name(), + getBugPriority(methodName, mi)).addClass(this).addMethod(this) + .addSourceLine(this).addString(methodName + signature)); + } + } + } + + if (seen == INVOKESTATIC) { + staticMethodCalls.remove(className); + } else { + if (reg >= 0) { + localMethodCalls.remove(Integer.valueOf(reg)); + } else if (fieldSource != null) { + fieldMethodCalls.remove(new FieldInfo(fieldSource, field.getName())); + } + } + } else { + int ln = getLineNumber(pc); + if (seen == INVOKESTATIC) { + staticMethodCalls.put(className, + new MethodCall(methodName, signature, parmConstants, pc, ln)); + } else { + if (reg >= 0) { + localMethodCalls.put(Integer.valueOf(reg), + new MethodCall(methodName, signature, parmConstants, pc, ln)); + } else if (field != null) { + OpcodeStack.Item obj = stack.getStackItem(parmCount); + fieldSource = (String) obj.getUserValue(); + if (fieldSource == null) { + fieldSource = ""; + } + fieldMethodCalls.put(new FieldInfo(fieldSource, field.getName()), + new MethodCall(methodName, signature, parmConstants, pc, ln)); + } + } + } + } + } else if (OpcodeUtils.isReturn(seen)) { + localMethodCalls.clear(); + fieldMethodCalls.clear(); + branchTargets.clear(pc); + } + } finally { + stack.sawOpcode(this, seen); + if ((userValue != null) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item item = stack.getStackItem(0); + item.setUserValue(userValue); + } + } + } + + private void clearFieldMethods(String fieldSource) { + Iterator it = fieldMethodCalls.keySet().iterator(); + while (it.hasNext()) { + if (it.next().hasFieldSource(fieldSource)) { + it.remove(); + } + } + } + + /** + * returns the bug priority based on metrics about the method + * + * @param methodName TODO + * @param mi metrics about the method + * @return the bug priority + */ + private static int getBugPriority(String methodName, MethodInfo mi) { + if (Values.STATIC_INITIALIZER.equals(methodName)) { + return LOW_PRIORITY; + } + + if ((mi.getNumBytes() >= highByteCountLimit) || (mi.getNumMethodCalls() >= highMethodCallLimit)) { + return HIGH_PRIORITY; + } + + if ((mi.getNumBytes() >= normalByteCountLimit) || (mi.getNumMethodCalls() >= normalMethodCallLimit)) { + return NORMAL_PRIORITY; + } + + if ((mi.getNumBytes() >= lowByteCountLimit) || (mi.getNumMethodCalls() >= lowMethodCallLimit)) { + return LOW_PRIORITY; + } + + return EXP_PRIORITY; + } + + /** + * returns true if the class or method name contains a pattern that is + * considered likely to be this modifying + * + * @param className the class name to check + * @param methodName the method name to check + * @return whether the method sounds like it modifies this + */ + private static boolean isRiskyName(String className, String methodName) { + if (riskyClassNames.contains(className)) { + return true; + } + + String qualifiedMethodName = className + '.' + methodName; + if (riskyMethodNameContents.contains(qualifiedMethodName)) { + return true; + } + + for (String riskyName : riskyMethodNameContents) { + if (methodName.indexOf(riskyName) >= 0) { + return true; + } + } + + return methodName.indexOf(Values.SYNTHETIC_MEMBER_CHAR) >= 0; + } + + /** + * returns the source line number for the pc, or just the pc if the line number + * table doesn't exist + * + * @param pc current pc + * @return the line number + */ + private int getLineNumber(int pc) { + LineNumberTable lnt = getMethod().getLineNumberTable(); + if (lnt == null) { + return pc; + } + + LineNumber[] lns = lnt.getLineNumberTable(); + if (lns == null) { + return pc; + } + + if (pc > lns[lns.length - 1].getStartPC()) { + return lns[lns.length - 1].getLineNumber(); + } + + int lo = 0; + int hi = lns.length - 2; + int mid = 0; + while (lo <= hi) { + mid = (lo + hi) >>> 1; + if (pc < lns[mid].getStartPC()) { + hi = mid - 1; + } else if (pc >= lns[mid + 1].getStartPC()) { + lo = mid + 1; + } else { + break; + } + } + + return lns[mid].getLineNumber(); + } + + /** + * contains information about a field access + */ + static class FieldInfo { + private final String fieldSource; + private final String fieldName; + + public FieldInfo(String source, String name) { + fieldSource = source; + fieldName = name; + } + + public boolean hasFieldSource(String source) { + return fieldSource.equals(source); + } + + @Override + public int hashCode() { + return fieldSource.hashCode() ^ fieldName.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof FieldInfo)) { + return false; + } + + FieldInfo that = (FieldInfo) o; + return fieldSource.equals(that.fieldSource) && fieldName.equals(that.fieldName); + } + + @Override + public String toString() { + return ToString.build(this); + } + } + + /** + * contains information about a method call + */ + static class MethodCall { + private final String methodName; + private final String methodSignature; + private final Object[] methodParms; + private final int methodPC; + private final int methodLineNumber; + + public MethodCall(String name, String signature, Object[] parms, int pc, int lineNumber) { + methodName = name; + methodSignature = signature; + methodParms = parms; + methodPC = pc; + methodLineNumber = lineNumber; + } + + public String getName() { + return methodName; + } + + public String getSignature() { + return methodSignature; + } + + public Object[] getParms() { + return methodParms; + } + + public int getPC() { + return methodPC; + } + + public int getLineNumber() { + return methodLineNumber; + } + } } diff --git a/src/samples/java/ex/CSBTS_StringToStringSample.java b/src/samples/java/ex/CSBTS_StringToStringSample.java index 05082ec5..3c1d6bf0 100644 --- a/src/samples/java/ex/CSBTS_StringToStringSample.java +++ b/src/samples/java/ex/CSBTS_StringToStringSample.java @@ -3,69 +3,69 @@ import org.apache.commons.lang3.builder.ToStringBuilder; public class CSBTS_StringToStringSample { - static class Person { - String name; - int age; + static class Person { + String name; + int age; - Person(String name, int age) { - this.name = name; - this.age = age; - } + Person(String name, int age) { + this.name = name; + this.age = age; + } - @Override - public String toString() { - // INCORRECT USAGE : The same as invoking Object.toString() - // return new ToStringBuilder(this).toString(); - // Consider using for non final classes to support a - // string representation for derived types - // return ToStringBuilder.reflectionToString(this); - // Use for final classes most efficient solution - return new ToStringBuilder(this).append("name", name).append("age", age).toString(); - } - } + @Override + public String toString() { + // INCORRECT USAGE : The same as invoking Object.toString() + // return new ToStringBuilder(this).toString(); + // Consider using for non final classes to support a + // string representation for derived types + // return ToStringBuilder.reflectionToString(this); + // Use for final classes most efficient solution + return new ToStringBuilder(this).append("name", name).append("age", age).toString(); + } + } - private enum SEX { - Male, Female; - } + private enum SEX { + Male, Female; + } - public final static class GenderPerson extends Person { - private SEX sex; + public final static class GenderPerson extends Person { + private SEX sex; - GenderPerson(String name, int age, SEX sex) { - super(name, age); - this.sex = sex; - } - } + GenderPerson(String name, int age, SEX sex) { + super(name, age); + this.sex = sex; + } + } - public static void main(String[] args) { - Person p = new Person("John Doe", 2); - ToStringBuilder x = new ToStringBuilder(p); - ToStringBuilder y = new ToStringBuilder(p); - // INCORRECT USAGE : The same as invoking Object.toString - System.out.println("P " + new ToStringBuilder(p).toString()); - // Consider using for non final classes to support a string - // representation for derived types - System.out.println("P " + ToStringBuilder.reflectionToString(p)); - GenderPerson p2 = new GenderPerson("Jane Doe", 2, SEX.Female); - System.out.println("GP " + new ToStringBuilder(p2).append("name", p2.name).append("age", p2.age) - .append("sex", p2.sex).toString()); - // Y now has an append - y.append("name", p.name); - System.out.println("P - Once Again " + y.toString()); - System.out.println("P - Again " + x.toString()); - } + public static void main(String[] args) { + Person p = new Person("John Doe", 2); + ToStringBuilder x = new ToStringBuilder(p); + ToStringBuilder y = new ToStringBuilder(p); + // INCORRECT USAGE : The same as invoking Object.toString + System.out.println("P " + new ToStringBuilder(p).toString()); + // Consider using for non final classes to support a string + // representation for derived types + System.out.println("P " + ToStringBuilder.reflectionToString(p)); + GenderPerson p2 = new GenderPerson("Jane Doe", 2, SEX.Female); + System.out.println("GP " + new ToStringBuilder(p2).append("name", p2.name).append("age", p2.age) + .append("sex", p2.sex).toString()); + // Y now has an append + y.append("name", p.name); + System.out.println("P - Once Again " + y.toString()); + System.out.println("P - Again " + x.toString()); + } - public class FPAppend extends ToStringBuilder { + public class FPAppend extends ToStringBuilder { - FPAppend(Object object) { - super(object); - } + FPAppend(Object object) { + super(object); + } - @Override - public ToStringBuilder append(Object obj) { - return super.append(obj); - } + @Override + public ToStringBuilder append(Object obj) { + return super.append(obj); + } - } + } } diff --git a/src/samples/java/ex/MDM_Sample.java b/src/samples/java/ex/MDM_Sample.java index c6a2739a..cb49b983 100755 --- a/src/samples/java/ex/MDM_Sample.java +++ b/src/samples/java/ex/MDM_Sample.java @@ -16,107 +16,107 @@ import javax.net.ssl.SSLServerSocketFactory; public class MDM_Sample implements Runnable { - private ReentrantLock myLock; - private CyclicBarrier barrier; - - public MDM_Sample() throws Exception { - boolean b; - - { // Halt tests - Runtime r = Runtime.getRuntime(); - r.exit(0); // WARNING - r.halt(0); // WARNING - r.runFinalization(); // WARNING - System.runFinalization(); // WARNING - } - - { // equals() tests - BigDecimal bd1 = new BigDecimal(0); - BigDecimal bd2 = new BigDecimal(0); - b = bd1.equals(bd2); // WARNING - } - - { // Socket tests - InetAddress localhost = InetAddress.getLocalHost(); // WARNING - if (localhost == null) { - localhost = InetAddress.getByName("booya"); - } - ServerSocket ss = new ServerSocket(0); - touch(ss); // WARNING - ss = new ServerSocket(0, 0); - touch(ss); // WARNING - ServerSocketFactory ssf = SSLServerSocketFactory.getDefault(); - ss = ssf.createServerSocket(0); - touch(ss); // WARNING - ss = ssf.createServerSocket(0, 0); - touch(ss); // WARNING - } - - { // RNG tests - Random r = new Random();// WARNING - touch(r); - byte[] seed = SecureRandom.getSeed(1); // WARNING (jdk 1.5 or older) - r = new SecureRandom(seed); // WARNING (jdk 1.5 or older) - touch(r); - } - - { // Thread tests - Thread t = new Thread(this); - int priority = t.getPriority(); // WARNING - t.setPriority(priority); // WARNING - t.join(); // WARNING - - Thread.sleep(0); // WARNING - Thread.sleep(0, 0); // WARNING - Thread.yield(); // WARNING - } - - { // Timeout tests - ReentrantLock rl = new ReentrantLock(); - rl.lock(); // WARNING - rl.lockInterruptibly(); // WARNING - b = rl.isHeldByCurrentThread(); // WARNING - b = rl.isLocked(); // WARNING - - Object o = new Object(); - do { - b = rl.tryLock(); // WARNING - o.wait(); // WARNING - } while (b); - - Lock l = rl; - l.lock(); // WARNING - b = l.tryLock(); - touch(b); // WARNING - l.lockInterruptibly(); // WARNING - - Condition c = l.newCondition(); - c.signal(); // WARNING - c.await(); // WARNING - } - - { // String tests - byte[] bytes = "".getBytes(); // WARNING - String s = new String(bytes); // WARNING - bytes = s.getBytes("UTF-8"); - - Locale.setDefault(Locale.ENGLISH); // WARNING - } - } - - @Override - public void run() { - } - - private static void touch(Object o) { - } - - private void fpAssertReentrantLock() { - assert myLock.isHeldByCurrentThread(); - } - - private void fpCyclicBarrier() throws Exception { - barrier.await(1, TimeUnit.MINUTES); - - } + private ReentrantLock myLock; + private CyclicBarrier barrier; + + public MDM_Sample() throws Exception { + boolean b; + + { // Halt tests + Runtime r = Runtime.getRuntime(); + r.exit(0); // WARNING + r.halt(0); // WARNING + r.runFinalization(); // WARNING + System.runFinalization(); // WARNING + } + + { // equals() tests + BigDecimal bd1 = new BigDecimal(0); + BigDecimal bd2 = new BigDecimal(0); + b = bd1.equals(bd2); // WARNING + } + + { // Socket tests + InetAddress localhost = InetAddress.getLocalHost(); // WARNING + if (localhost == null) { + localhost = InetAddress.getByName("booya"); + } + ServerSocket ss = new ServerSocket(0); + touch(ss); // WARNING + ss = new ServerSocket(0, 0); + touch(ss); // WARNING + ServerSocketFactory ssf = SSLServerSocketFactory.getDefault(); + ss = ssf.createServerSocket(0); + touch(ss); // WARNING + ss = ssf.createServerSocket(0, 0); + touch(ss); // WARNING + } + + { // RNG tests + Random r = new Random();// WARNING + touch(r); + byte[] seed = SecureRandom.getSeed(1); // WARNING (jdk 1.5 or older) + r = new SecureRandom(seed); // WARNING (jdk 1.5 or older) + touch(r); + } + + { // Thread tests + Thread t = new Thread(this); + int priority = t.getPriority(); // WARNING + t.setPriority(priority); // WARNING + t.join(); // WARNING + + Thread.sleep(0); // WARNING + Thread.sleep(0, 0); // WARNING + Thread.yield(); // WARNING + } + + { // Timeout tests + ReentrantLock rl = new ReentrantLock(); + rl.lock(); // WARNING + rl.lockInterruptibly(); // WARNING + b = rl.isHeldByCurrentThread(); // WARNING + b = rl.isLocked(); // WARNING + + Object o = new Object(); + do { + b = rl.tryLock(); // WARNING + o.wait(); // WARNING + } while (b); + + Lock l = rl; + l.lock(); // WARNING + b = l.tryLock(); + touch(b); // WARNING + l.lockInterruptibly(); // WARNING + + Condition c = l.newCondition(); + c.signal(); // WARNING + c.await(); // WARNING + } + + { // String tests + byte[] bytes = "".getBytes(); // WARNING + String s = new String(bytes); // WARNING + bytes = s.getBytes("UTF-8"); + + Locale.setDefault(Locale.ENGLISH); // WARNING + } + } + + @Override + public void run() { + } + + private static void touch(Object o) { + } + + private void fpAssertReentrantLock() { + assert myLock.isHeldByCurrentThread(); + } + + private void fpCyclicBarrier() throws Exception { + barrier.await(1, TimeUnit.MINUTES); + + } } diff --git a/src/samples/java/ex/NMCS_Sample.java b/src/samples/java/ex/NMCS_Sample.java index be74cd12..0a6c0188 100755 --- a/src/samples/java/ex/NMCS_Sample.java +++ b/src/samples/java/ex/NMCS_Sample.java @@ -9,52 +9,52 @@ import java.util.Vector; public class NMCS_Sample { - private static List test1 = new Vector(); + private static List test1 = new Vector(); - static { - test1.add("one"); - test1.add("two"); - test1.add("three"); - } + static { + test1.add("one"); + test1.add("two"); + test1.add("three"); + } - private static Map fp = new HashMap<>(); + private static Map fp = new HashMap<>(); - private Map test2 = new Hashtable(); + private Map test2 = new Hashtable(); - private Set test3 = new HashSet(); + private Set test3 = new HashSet(); - private List test4 = new Vector(); + private List test4 = new Vector(); - public String test1() { - StringBuffer sb = new StringBuffer(); - String comma = ""; - for (String s : test1) { - sb.append(comma); - comma = ","; - sb.append(s); - } + public String test1() { + StringBuffer sb = new StringBuffer(); + String comma = ""; + for (String s : test1) { + sb.append(comma); + comma = ","; + sb.append(s); + } - return sb.toString(); - } + return sb.toString(); + } - public String test2(String s) { - test2 = new Hashtable(); + public String test2(String s) { + test2 = new Hashtable(); - return test2.get("foo"); - } + return test2.get("foo"); + } - public Set test3() { - Set temp = test3; - temp.add("Foo"); - return temp; - } + public Set test3() { + Set temp = test3; + temp.add("Foo"); + return temp; + } - public List test4(boolean b1, boolean b2) { - return b1 ? test4 : b2 ? new Vector() : test4; - } + public List test4(boolean b1, boolean b2) { + return b1 ? test4 : b2 ? new Vector() : test4; + } - public void fpComputeIfAbsent(String val) { - fp.computeIfAbsent(val, this::test2); - } + public void fpComputeIfAbsent(String val) { + fp.computeIfAbsent(val, this::test2); + } } diff --git a/src/samples/java/ex/PRMC_Sample.java b/src/samples/java/ex/PRMC_Sample.java index e0ad195d..f63a3908 100755 --- a/src/samples/java/ex/PRMC_Sample.java +++ b/src/samples/java/ex/PRMC_Sample.java @@ -17,223 +17,223 @@ @SuppressWarnings("all") public class PRMC_Sample { - private static PRMC_Sample SAMPLE1; - private static PRMC_Sample SAMPLE2; - String data; - String[] array1; - String[] array2; - - private Integer meh1 = Integer.MIN_VALUE; - private Integer meh2 = Integer.MIN_VALUE; - - public boolean test1(Calendar c) { - Date d = c.getTime(); - long l = d.getTime(); - Date e = c.getTime(); - long j = e.getTime(); - return l == j; - } - - public void rmcFP(ByteBuffer bb) { - int i = bb.getInt(); - int j = bb.getInt(); - } - - @Override - public boolean equals(Object o) { - PRMC_Sample rmc = (PRMC_Sample) o; - if (data.equals("INF") || rmc.data.equals("INF")) { - return false; - } - - return data.equals(rmc.data); - } - - public void staticPRMC() { - Factory.getInstance().fee(); - Factory.getInstance().fi(); - Factory.getInstance().fo(); - Factory.getInstance().fum(); - } - - public void repeatedEmptyArrays() { - System.out.println(Arrays.asList(new Integer[0])); - System.out.println(Arrays.asList(new Integer[0])); - } - - static class Factory { - private static Factory f = new Factory(); - - private Factory() { - } - - public static Factory getInstance() { - return f; - } - - public void fee() { - } - - public void fi() { - } - - public void fo() { - } - - public void fum() { - } - } - - public long fpCurrentTimeMillis(Object o) { - long time = -System.currentTimeMillis(); - o.hashCode(); - time += System.currentTimeMillis(); - - return time; - } - - public void fpEnumToString(FPEnum e) { - Set s = new HashSet<>(); - - s.add(FPEnum.fee.toString()); - s.add(FPEnum.fi.toString()); - s.add(FPEnum.fo.toString()); - s.add(FPEnum.fum.toString()); - } - - public void emptyList() { - List l = Collections.emptyList(); - List o = Collections.emptyList(); - List p = Collections.emptyList(); - } - - public void fpSimpleGetter() { - getData(); - getData(); - } - - public String getData() { - return data; - } - - enum FPEnum { - fee, fi, fo, fum - }; - - public boolean validChainedFields(Chain c1) { - return c1.chainedField.toString().equals(c1.chainedField.toString()); - } - - public boolean fpChainedFieldsOfDiffBases(Chain c1, Chain c2) { - return c1.chainedField.toString().equals(c2.chainedField.toString()); - } - - public void fpMultipleStatics() { - SAMPLE1 = new PRMC_Sample(); - SAMPLE1.setValue(5); - SAMPLE2 = new PRMC_Sample(); - SAMPLE2.setValue(5); - } - - public void fpWithGuava() { - List l = Lists.newArrayList(); - List ll = Lists.newArrayList(); - } - - public void fpAsListLiterals() { - System.out.println(Arrays.asList("foo")); - System.out.println(Arrays.asList("bar")); - } - - public void fpWithFinally() { - Object foo = new Object(); - try { - willThrow(); - System.err.println(foo.toString()); - } finally { - System.out.println(foo.toString()); - } - } - - public void fpWithGenericReturn() { - Set s = ImmutableSet.of(); - Set i = ImmutableSet.of(); - } - - public String fpStream(Collection c) { - String s1 = c.stream().filter((s) -> s.startsWith("A")).findFirst().get(); - String s2 = c.stream().filter((s) -> s.startsWith("B")).findFirst().get(); - return s1 + s2; - } - - void willThrow() { - throw new RuntimeException("kaboom!"); - } - - public void fpIgnoreCommonBoxing(Integer i) { - setTwo(i, i); - setTwo(3.1, 3.1); - } - - public void setTwo(int i, int j) { - } - - public void setTwo(Double d, Double e) { - } - - public void setValue(int i) { - } - - class Chain { - public Chain chainedField; - - @Override - public String toString() { - StringBuilder sb = new StringBuilder(); - sb.append("--"); - sb.append("XX"); - sb.append("--"); - sb.append("XX"); - sb.append("--"); - sb.append("XX"); - sb.append("--"); - sb.append("XX"); - sb.append("--"); - sb.append("XX"); - sb.append("--"); - return sb.toString(); - } - } - - class SFIssue71 { - protected String[] inc = new String[0]; - protected String[] dec = new String[0]; - - public void fplog() { - System.out.println(Arrays.toString(inc)); - System.out.println(Arrays.toString(dec)); - } - } + private static PRMC_Sample SAMPLE1; + private static PRMC_Sample SAMPLE2; + String data; + String[] array1; + String[] array2; + + private Integer meh1 = Integer.MIN_VALUE; + private Integer meh2 = Integer.MIN_VALUE; + + public boolean test1(Calendar c) { + Date d = c.getTime(); + long l = d.getTime(); + Date e = c.getTime(); + long j = e.getTime(); + return l == j; + } + + public void rmcFP(ByteBuffer bb) { + int i = bb.getInt(); + int j = bb.getInt(); + } + + @Override + public boolean equals(Object o) { + PRMC_Sample rmc = (PRMC_Sample) o; + if (data.equals("INF") || rmc.data.equals("INF")) { + return false; + } + + return data.equals(rmc.data); + } + + public void staticPRMC() { + Factory.getInstance().fee(); + Factory.getInstance().fi(); + Factory.getInstance().fo(); + Factory.getInstance().fum(); + } + + public void repeatedEmptyArrays() { + System.out.println(Arrays.asList(new Integer[0])); + System.out.println(Arrays.asList(new Integer[0])); + } + + static class Factory { + private static Factory f = new Factory(); + + private Factory() { + } + + public static Factory getInstance() { + return f; + } + + public void fee() { + } + + public void fi() { + } + + public void fo() { + } + + public void fum() { + } + } + + public long fpCurrentTimeMillis(Object o) { + long time = -System.currentTimeMillis(); + o.hashCode(); + time += System.currentTimeMillis(); + + return time; + } + + public void fpEnumToString(FPEnum e) { + Set s = new HashSet<>(); + + s.add(FPEnum.fee.toString()); + s.add(FPEnum.fi.toString()); + s.add(FPEnum.fo.toString()); + s.add(FPEnum.fum.toString()); + } + + public void emptyList() { + List l = Collections.emptyList(); + List o = Collections.emptyList(); + List p = Collections.emptyList(); + } + + public void fpSimpleGetter() { + getData(); + getData(); + } + + public String getData() { + return data; + } + + enum FPEnum { + fee, fi, fo, fum + }; + + public boolean validChainedFields(Chain c1) { + return c1.chainedField.toString().equals(c1.chainedField.toString()); + } + + public boolean fpChainedFieldsOfDiffBases(Chain c1, Chain c2) { + return c1.chainedField.toString().equals(c2.chainedField.toString()); + } + + public void fpMultipleStatics() { + SAMPLE1 = new PRMC_Sample(); + SAMPLE1.setValue(5); + SAMPLE2 = new PRMC_Sample(); + SAMPLE2.setValue(5); + } + + public void fpWithGuava() { + List l = Lists.newArrayList(); + List ll = Lists.newArrayList(); + } + + public void fpAsListLiterals() { + System.out.println(Arrays.asList("foo")); + System.out.println(Arrays.asList("bar")); + } + + public void fpWithFinally() { + Object foo = new Object(); + try { + willThrow(); + System.err.println(foo.toString()); + } finally { + System.out.println(foo.toString()); + } + } + + public void fpWithGenericReturn() { + Set s = ImmutableSet.of(); + Set i = ImmutableSet.of(); + } + + public String fpStream(Collection c) { + String s1 = c.stream().filter((s) -> s.startsWith("A")).findFirst().get(); + String s2 = c.stream().filter((s) -> s.startsWith("B")).findFirst().get(); + return s1 + s2; + } + + void willThrow() { + throw new RuntimeException("kaboom!"); + } + + public void fpIgnoreCommonBoxing(Integer i) { + setTwo(i, i); + setTwo(3.1, 3.1); + } + + public void setTwo(int i, int j) { + } + + public void setTwo(Double d, Double e) { + } + + public void setValue(int i) { + } + + class Chain { + public Chain chainedField; + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("--"); + sb.append("XX"); + sb.append("--"); + sb.append("XX"); + sb.append("--"); + sb.append("XX"); + sb.append("--"); + sb.append("XX"); + sb.append("--"); + sb.append("XX"); + sb.append("--"); + return sb.toString(); + } + } + + class SFIssue71 { + protected String[] inc = new String[0]; + protected String[] dec = new String[0]; + + public void fplog() { + System.out.println(Arrays.toString(inc)); + System.out.println(Arrays.toString(dec)); + } + } } class PRMCPar { - List c = new ArrayList<>(); + List c = new ArrayList<>(); - public void fpSuperCall() { - c.add("FP"); - } + public void fpSuperCall() { + c.add("FP"); + } } class PRMCChild extends PRMCPar { - @Override - public void fpSuperCall() { + @Override + public void fpSuperCall() { - if (c.isEmpty()) { - super.fpSuperCall(); - if (c.isEmpty()) { - System.out.println("False Positive"); - } - } - } + if (c.isEmpty()) { + super.fpSuperCall(); + if (c.isEmpty()) { + System.out.println("False Positive"); + } + } + } }