diff --git a/etc/findbugs.xml b/etc/findbugs.xml index 6e5fc5692..3b1221d4e 100755 --- a/etc/findbugs.xml +++ b/etc/findbugs.xml @@ -331,6 +331,7 @@ + @@ -341,7 +342,7 @@ - + diff --git a/etc/messages.xml b/etc/messages.xml index d2d2461f2..de4fa2e4a 100755 --- a/etc/messages.xml +++ b/etc/messages.xml @@ -56,6 +56,28 @@ ]]> + + +
+ Looks for appending CharSequence.toString inside of calls to Appendable append.

+
+				StringBuilder sb = new StringBuilder();
+				sb.append(a + b);
+				return sb.toString();
+			
+ You should use the .append method to append values +
+				sb.append(a).append(b);
+			
+

It is a fast detector.

+ ]]> +
+
+ + + +
@@ -6513,6 +6535,22 @@ if (shouldCalcHalting && (calculateHaltingProbability() > 0) { } ]]>
+ + + + Method concatenates the result of a toString() call + Method {1} concatenates the result of a toString() call +
+ This method concatenates the output of a toString() call on a CharSequence into a Appendable. + It is simpler just to pass the CharSequenceobject you want to append to the append call as that gives better performance.

+

+ +

+ ]]> +
+
+ Method calls equals on an enum instance @@ -6551,8 +6589,10 @@ if (shouldCalcHalting && (calculateHaltingProbability() > 0) { } - + + + Inefficient Appendable Appending Inefficient String Buffering Synchronized Collection Iterators Cyclomatic Complexity diff --git a/src/main/java/com/mebigfatguy/fbcontrib/detect/InefficientAppendableAppend.java b/src/main/java/com/mebigfatguy/fbcontrib/detect/InefficientAppendableAppend.java new file mode 100644 index 000000000..1ae180df9 --- /dev/null +++ b/src/main/java/com/mebigfatguy/fbcontrib/detect/InefficientAppendableAppend.java @@ -0,0 +1,205 @@ +/* + * fb-contrib - Auxiliary detectors for Java programs + * Copyright (C) 2005-2019 Dave Brosius + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package com.mebigfatguy.fbcontrib.detect; + +import javax.annotation.Nullable; + +import org.apache.bcel.Const; +import org.apache.bcel.classfile.Code; +import org.apache.bcel.classfile.Constant; +import org.apache.bcel.classfile.ConstantString; +import org.apache.bcel.classfile.JavaClass; + +import com.mebigfatguy.fbcontrib.utils.BugType; +import com.mebigfatguy.fbcontrib.utils.OpcodeUtils; +import com.mebigfatguy.fbcontrib.utils.TernaryPatcher; +import com.mebigfatguy.fbcontrib.utils.Values; + +import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugReporter; +import edu.umd.cs.findbugs.BytecodeScanningDetector; +import edu.umd.cs.findbugs.OpcodeStack; +import edu.umd.cs.findbugs.OpcodeStack.CustomUserValue; +import edu.umd.cs.findbugs.ba.ClassContext; +import edu.umd.cs.findbugs.ba.ch.Subtypes2; +import edu.umd.cs.findbugs.util.ClassName; + +/** + * Looks for following the append methods of interface java.lang.Appendable + * append(CharSeq csq) + * append (CharSeq csq, int start, int end) Reports if + * toString is used on the CharSeq + */ +@CustomUserValue +public class InefficientAppendableAppend extends BytecodeScanningDetector { + + private enum AppendType { + CLEAR, TOSTRING + }; + + private BugReporter bugReporter; + private OpcodeStack stack; + private boolean sawLDCEmpty; + + /** + * constructs a IAA detector given the reporter to report bugs on + * + * @param bugReporter the sync of bug reports + */ + public InefficientAppendableAppend(final BugReporter bugReporter) { + this.bugReporter = bugReporter; + } + + /** + * implements the visitor to create and clear the stack + * + * @param classContext the context object of the currently parsed class + */ + @Override + public void visitClassContext(ClassContext classContext) { + try { + stack = new OpcodeStack(); + super.visitClassContext(classContext); + } finally { + stack = null; + } + } + + /** + * implements the visitor to create and clear the stack + * + * @param obj the context object of the currently parsed code block + */ + @Override + public void visitCode(final Code obj) { + if (obj.getCode() != null) { + stack.resetForMethodEntry(this); + sawLDCEmpty = false; + super.visitCode(obj); + } + } + + @Override + public void sawOpcode(final int seen) { + AppendType userValue = null; + + try { + stack.precomputation(this); + + if (seen == Const.INVOKEVIRTUAL || seen == Const.INVOKEINTERFACE) { + userValue = sawInvokeVirtualOrInterface(); + } else if ((seen == Const.GOTO) || (seen == Const.GOTO_W)) { + int depth = stack.getStackDepth(); + for (int i = 0; i < depth; i++) { + OpcodeStack.Item itm = stack.getStackItem(i); + itm.setUserValue(null); + } + } else if ((seen == Const.LDC) || (seen == Const.LDC_W)) { + Constant c = getConstantRefOperand(); + if (c instanceof ConstantString) { + String s = ((ConstantString) c).getBytes(getConstantPool()); + if (s.length() == 0) { + sawLDCEmpty = true; + } + } + // ((seen >= Const.ALOAD_0) && (seen <= Const.ALOAD_3)) + } else if (OpcodeUtils.isALoad(seen)) { + userValue = AppendType.CLEAR; + } + } finally { + handleOpcode(seen); + if ((userValue != null) && (stack.getStackDepth() > 0)) { + OpcodeStack.Item itm = stack.getStackItem(0); + itm.setUserValue(userValue); + } + } + } + + private void handleOpcode(final int seen) { + TernaryPatcher.pre(stack, seen); + stack.sawOpcode(this, seen); + TernaryPatcher.post(stack, seen); + } + + private AppendType sawInvokeVirtualOrInterface() { + AppendType userValue = null; + String calledClass = getClassConstantOperand(); + if (Subtypes2.instanceOf((ClassName.toDottedClassName(calledClass)), "java.lang.CharSequence") + && Values.TOSTRING.equals(getNameConstantOperand())) { + userValue = AppendType.TOSTRING; + return userValue; + } else if (Subtypes2.instanceOf(ClassName.toDottedClassName(calledClass), "java.lang.Appendable")) { + String methodName = getNameConstantOperand(); + if ("append".equals(methodName)) { + int numArgs = getNumberArguments(getSigConstantOperand()); + OpcodeStack.Item itm = getAppendableItemAt(numArgs); + if (itm != null) { + userValue = (AppendType) itm.getUserValue(); + } + + if (stack.getStackDepth() > 0) { + itm = stack.getStackItem(numArgs - 1); + AppendType uv = (AppendType) itm.getUserValue(); + if (uv != null && uv.equals(AppendType.TOSTRING)) { + bugReporter.reportBug(new BugInstance(this, BugType.IAA_INEFFICIENT_APPENDABLE_APPEND.name(), + Values.TOSTRING.equals(getMethodName()) ? LOW_PRIORITY : NORMAL_PRIORITY).addClass(this) + .addMethod(this).addSourceLine(this)); + } + } + } + } + return userValue; + } + + /* + * @Nullable private OpcodeStack.Item getCharSequenceItemAt(int depth) { if + * (stack.getStackDepth() > depth) { OpcodeStack.Item itm = + * stack.getStackItem(depth); try { JavaClass cls = itm.getJavaClass(); if + * (Subtypes2.instanceOf(cls, "java.lang.CharSequence")) return itm; } catch + * (ClassNotFoundException e) { return null; } + * + * } + * + * return null; } + */ + + @Nullable + private OpcodeStack.Item getAppendableItemAt(int depth) { + if (stack.getStackDepth() > depth) { + OpcodeStack.Item itm = stack.getStackItem(depth); + try { + // This piece of code only required to avoid duplidate reporting by IAA and ISB + // detectors + /* + * String signature = itm.getSignature(); if + * (Values.SIG_JAVA_UTIL_STRINGBUFFER.equals(signature) || + * Values.SIG_JAVA_UTIL_STRINGBUILDER.equals(signature)) { return null; } + */ + JavaClass cls = itm.getJavaClass(); + if (Subtypes2.instanceOf(cls, "java.lang.Appendable")) + return itm; + } catch (ClassNotFoundException e) { + return null; + } + + } + + return null; + } +} diff --git a/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java b/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java index a093fb3ca..8432ba99b 100644 --- a/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java +++ b/src/main/java/com/mebigfatguy/fbcontrib/utils/BugType.java @@ -124,6 +124,7 @@ public enum BugType { IPU_IMPROPER_PROPERTIES_USE_SETPROPERTY, ISB_EMPTY_STRING_APPENDING, ISB_INEFFICIENT_STRING_BUFFERING, + IAA_INEFFICIENT_APPENDABLE_APPEND, ISB_TOSTRING_APPENDING, ITC_INHERITANCE_TYPE_CHECKING, ITU_INAPPROPRIATE_TOSTRING_USE, diff --git a/src/samples/java/ex/IAA_Sample.java b/src/samples/java/ex/IAA_Sample.java new file mode 100644 index 000000000..902a329ef --- /dev/null +++ b/src/samples/java/ex/IAA_Sample.java @@ -0,0 +1,103 @@ +package ex; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileWriter; +import java.io.IOException; +import java.io.PrintWriter; +import java.nio.CharBuffer; + +@SuppressWarnings("all") +public class IAA_Sample { + + public String testIAA1Report() { + Appendable apd = new StringBuilder(); + CharSequence cs = new StringBuilder("foo"); + try { + apd.append(cs.toString()); + } catch (IOException e) { + e.printStackTrace(); + } + return apd.toString(); + } + + public String testIAH2Report() { + Appendable apd = new StringBuilder(); + StringBuilder sb = new StringBuilder("bar"); + try { + apd.append(sb.toString()); + } catch (IOException e) { + e.printStackTrace(); + } + return sb.toString(); + } + + public String testIAH3Report() { + PrintWriter pw = null; + File temp = null; + try { + temp = File.createTempFile("myTempFile", ".txt"); + temp.deleteOnExit(); + pw = new PrintWriter(temp); + CharSequence cs = new StringBuilder("foo"); + pw.append(cs.toString()); + return cs.toString(); + } catch (FileNotFoundException e) { + e.printStackTrace(); + return null; + } catch (IOException e) { + e.printStackTrace(); + return null; + } finally { + if (pw != null) + pw.close(); + } + } + + public String testIAH4Report() { + FileWriter fw = null; + try { + File temp = File.createTempFile("myTempFile", ".txt"); + temp.deleteOnExit(); + fw = new FileWriter(temp); + CharBuffer cb = CharBuffer.allocate(10); + cb.put("foobar"); + fw.append(cb.toString()); + fw.close(); + return cb.toString(); + } catch (FileNotFoundException e) { + e.printStackTrace(); + return null; + } catch (IOException e) { + e.printStackTrace(); + return null; + } + } + + public String testIAH5Report() { + PrintWriter pw = null; + try { + pw = new PrintWriter("file.txt"); + StringBuilder cs = new StringBuilder("foobar"); + pw.append(cs.toString(), 1, 2); + pw.close(); + return cs.toString(); + } catch (FileNotFoundException e) { + e.printStackTrace(); + return null; + } + + } + + public String testIAA6DoNotReport() { + Appendable sb = new StringBuilder(); + int[] intArr = new int[] { 1, 2, 3, 4 }; + try { + sb.append(intArr.toString()); + } catch (IOException e) { + e.printStackTrace(); + } + return sb.toString(); + } + +}