[Fb-contrib-commit] SF.net SVN: fb-contrib:[1610] trunk/fb-contrib
Brought to you by:
dbrosius
Menu
▾
▴
[Fb-contrib-commit] SF.net SVN: fb-contrib:[1610] trunk/fb-contrib
|
From: <dbr...@us...> - 2010-09-11 15:42:02
|
Revision: 1610
http://fb-contrib.svn.sourceforge.net/fb-contrib/?rev=1610&view=rev
Author: dbrosius
Date: 2010-09-11 15:41:55 +0000 (Sat, 11 Sep 2010)
Log Message:
-----------
add LO_STUTTERED_MESSAGE to LoggerOddities
Modified Paths:
--------------
trunk/fb-contrib/etc/findbugs.xml
trunk/fb-contrib/etc/messages.xml
trunk/fb-contrib/samples/LO_Sample.java
trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java
Modified: trunk/fb-contrib/etc/findbugs.xml
===================================================================
--- trunk/fb-contrib/etc/findbugs.xml 2010-09-10 13:02:35 UTC (rev 1609)
+++ trunk/fb-contrib/etc/findbugs.xml 2010-09-11 15:41:55 UTC (rev 1610)
@@ -165,7 +165,7 @@
<Detector class="com.mebigfatguy.fbcontrib.detect.SuspiciousClusteredSessionSupport" speed="fast" reports="SCSS_SUSPICIOUS_CLUSTERED_SESSION_SUPPORT" />
- <Detector class="com.mebigfatguy.fbcontrib.detect.LoggerOddities" speed="fast" reports="LO_SUSPECT_LOG_CLASS,LO_SUSPECT_LOG_PARAMETER" />
+ <Detector class="com.mebigfatguy.fbcontrib.detect.LoggerOddities" speed="fast" reports="LO_SUSPECT_LOG_CLASS,LO_SUSPECT_LOG_PARAMETER,LO_STUTTERED_MESSAGE" />
<Detector class="com.mebigfatguy.fbcontrib.detect.IncorrectInternalClassUse" speed="fast" reports="IICU_INCORRECT_INTERNAL_CLASS_USE" />
@@ -331,6 +331,7 @@
<BugPattern abbrev="SCSS" type="SCSS_SUSPICIOUS_CLUSTERED_SESSION_SUPPORT" category="CORRECTNESS" />
<BugPattern abbrev="LO" type="LO_SUSPECT_LOG_CLASS" category="CORRECTNESS" />
<BugPattern abbrev="LO" type="LO_SUSPECT_LOG_PARAMETER" category="CORRECTNESS" />
+ <BugPattern abbrev="LO" type="LO_STUTTERED_MESSAGE" category="STYLE" />
<BugPattern abbrev="IICU" type="IICU_INCORRECT_INTERNAL_CLASS_USE" category="CORRECTNESS" />
<BugPattern abbrev="DSOC" type="DSOC_DUBIOUS_SET_OF_COLLECTIONS" category="PERFORMANCE" />
<BugPattern abbrev="BED" type="BED_BOGUS_EXCEPTION_DECLARATION" category="CORRECTNESS" />
Modified: trunk/fb-contrib/etc/messages.xml
===================================================================
--- trunk/fb-contrib/etc/messages.xml 2010-09-10 13:02:35 UTC (rev 1609)
+++ trunk/fb-contrib/etc/messages.xml 2010-09-11 15:41:55 UTC (rev 1610)
@@ -882,7 +882,7 @@
<Detector class="com.mebigfatguy.fbcontrib.detect.LoggerOddities">
<Details>
<![CDATA[
- <p>Looks for odd patterns of use of Logger classes from either log4j or slf4j.</p>
+ <p>Looks for odd patterns of use of Logger classes from either log4j, slf4j or commons logging.</p>
<p>It is a fast detector</p>
]]>
</Details>
@@ -2699,6 +2699,21 @@
]]>
</Details>
</BugPattern>
+
+ <BugPattern type="LO_STUTTERED_MESSAGE">
+ <ShortDescription>method stutters exception message in logger</ShortDescription>
+ <LongDescription>method {1} stutters exception message in logger</LongDescription>
+ <Details>
+ <![CDATA[
+ This method uses a logger method that takes an exception, and passes the result of
+ the getMessage() method on the exception that occurred as the log message.
+ Since you are already passing in the exception, that message is already present in the
+ logs, and by passing it in as the message, you are just stuttering information.
+ It would be more helpful to provide a hand written message that describes the error in
+ this method, possibly including the values of key variables.
+ ]]>
+ </Details>
+ </BugPattern>
<BugPattern type="IICU_INCORRECT_INTERNAL_CLASS_USE">
<ShortDescription>class relies on internal api classes</ShortDescription>
Modified: trunk/fb-contrib/samples/LO_Sample.java
===================================================================
--- trunk/fb-contrib/samples/LO_Sample.java 2010-09-10 13:02:35 UTC (rev 1609)
+++ trunk/fb-contrib/samples/LO_Sample.java 2010-09-11 15:41:55 UTC (rev 1610)
@@ -1,3 +1,8 @@
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
import org.apache.log4j.Logger;
@@ -11,4 +16,22 @@
{
}
+
+ public void testStutter() throws IOException
+ {
+ InputStream is = null;
+ try
+ {
+ File f = new File("Foo");
+ is = new FileInputStream(f);
+ }
+ catch (Exception e)
+ {
+ l1.error(e.getMessage(), e);
+ }
+ finally
+ {
+ is.close();
+ }
+ }
}
Modified: trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java
===================================================================
--- trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java 2010-09-10 13:02:35 UTC (rev 1609)
+++ trunk/fb-contrib/src/com/mebigfatguy/fbcontrib/detect/LoggerOddities.java 2010-09-11 15:41:55 UTC (rev 1610)
@@ -18,11 +18,16 @@
*/
package com.mebigfatguy.fbcontrib.detect;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.bcel.Repository;
import org.apache.bcel.classfile.Code;
import org.apache.bcel.classfile.Constant;
import org.apache.bcel.classfile.ConstantClass;
import org.apache.bcel.classfile.ConstantPool;
import org.apache.bcel.classfile.ConstantUtf8;
+import org.apache.bcel.classfile.JavaClass;
import org.apache.bcel.classfile.Method;
import org.apache.bcel.generic.Type;
@@ -33,6 +38,24 @@
import edu.umd.cs.findbugs.ba.ClassContext;
public class LoggerOddities extends BytecodeScanningDetector {
+ private static JavaClass THROWABLE_CLASS;
+ private static Set<String> loggerMethods;
+
+ static {
+ try {
+ THROWABLE_CLASS = Repository.lookupClass("java/lang/Throwable");
+
+ loggerMethods = new HashSet<String>();
+ loggerMethods.add("trace");
+ loggerMethods.add("debug");
+ loggerMethods.add("info");
+ loggerMethods.add("warn");
+ loggerMethods.add("error");
+ loggerMethods.add("fatal");
+ } catch (ClassNotFoundException cnfe) {
+ THROWABLE_CLASS = null;
+ }
+ }
private final BugReporter bugReporter;
private OpcodeStack stack;
private String clsName;
@@ -111,6 +134,8 @@
@Override
public void sawOpcode(int seen) {
String ldcClassName = null;
+ int exMessageReg = -1;
+
try {
if ((seen == LDC) || (seen == LDC_W)) {
Constant c = getConstantRefOperand();
@@ -198,7 +223,42 @@
}
}
}
+ } else if (((seen == INVOKEVIRTUAL) || (seen == INVOKEINTERFACE)) && (THROWABLE_CLASS != null)) {
+ String mthName = getNameConstantOperand();
+ if (mthName.equals("getMessage")) {
+ String callingClsName = getClassConstantOperand();
+ JavaClass cls = Repository.lookupClass(callingClsName);
+ if (cls.instanceOf(THROWABLE_CLASS)) {
+ if (stack.getStackDepth() > 0) {
+ OpcodeStack.Item exItem = stack.getStackItem(0);
+ exMessageReg = exItem.getRegisterNumber();
+ }
+ }
+ } else if (loggerMethods.contains(mthName)) {
+ String callingClsName = getClassConstantOperand();
+ if (callingClsName.endsWith("Log") || (callingClsName.endsWith("Logger"))) {
+ String sig = getSigConstantOperand();
+ if ("(Ljava/lang/String;Ljava/lang/Throwable;)V".equals(sig) || "(Ljava/lang/Object;Ljava/lang/Throwable;)V".equals(sig)) {
+ if (stack.getStackDepth() >= 2) {
+ OpcodeStack.Item exItem = stack.getStackItem(0);
+ OpcodeStack.Item msgItem = stack.getStackItem(1);
+
+ Integer exReg = (Integer)msgItem.getUserValue();
+ if (exReg != null) {
+ if (exReg.intValue() == exItem.getRegisterNumber()) {
+ bugReporter.reportBug(new BugInstance(this, "LO_STUTTERED_MESSAGE", NORMAL_PRIORITY)
+ .addClass(this)
+ .addMethod(this)
+ .addSourceLine(this));
+ }
+ }
+ }
+ }
+ }
+ }
}
+ } catch (ClassNotFoundException cnfe) {
+ bugReporter.reportMissingClass(cnfe);
} finally {
stack.sawOpcode(this, seen);
if (ldcClassName != null) {
@@ -207,6 +267,12 @@
item.setUserValue(ldcClassName);
}
}
+ if (exMessageReg >= 0) {
+ if (stack.getStackDepth() > 0) {
+ OpcodeStack.Item item = stack.getStackItem(0);
+ item.setUserValue(Integer.valueOf(exMessageReg));
+ }
+ }
}
}
}
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|
