add new SLF4J_GET_STACK_TRACE detector (fixes KengoTODA#70)

Author: vorburger

bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java

@@ -0,0 +1,36 @@
+package jp.skypencil.findbugs.slf4j;
+
+import com.google.common.base.Objects;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.OpcodeStack.Item;
+import jp.skypencil.findbugs.slf4j.parameter.AbstractDetectorForParameterArray2;
+
+/**
+ * FindBugs (now SpotBugs) Detector for (ab)use of {@link Throwable#getStackTrace()} in SFL4j logger.
+ *
+ * @author Michael Vorburger.ch
+ */
+public class ManualGetStackTraceDetector extends AbstractDetectorForParameterArray2 {
+
+    @Item.SpecialKind
+    private final int isGetStackTrace = Item.defineNewSpecialKind("use of throwable getStackTrace");
+
+    public ManualGetStackTraceDetector(BugReporter bugReporter) {
+        super(bugReporter, "SLF4J_GET_STACK_TRACE");
+    }
+
+    @Override
+    protected int getIsOfInterestKind() {
+        return isGetStackTrace;
+    }
+
+    @Override
+    protected boolean isWhatWeWantToDetect(int seen) {
+        // return false;
+        return seen == INVOKEVIRTUAL
+                && !stack.isTop()
+                && getThrowableHandler().checkThrowable(getStack().getStackItem(0))
+                && Objects.equal("getStackTrace", getNameConstantOperand());
+    }
+
+}

bug-pattern/src/main/resources/bugrank.txt

@@ -7,3 +7,4 @@
 0 BugPattern SLF4J_ILLEGAL_PASSED_CLASS
 0 BugPattern SLF4J_SIGN_ONLY_FORMAT
 0 BugPattern SLF4J_MANUALLY_PROVIDED_MESSAGE
+0 BugPattern SLF4J_GET_STACK_TRACE

bug-pattern/src/main/resources/findbugs.xml

@@ -11,6 +11,8 @@
         <Detector class="jp.skypencil.findbugs.slf4j.StaticLoggerDetector" reports="SLF4J_LOGGER_SHOULD_BE_NON_STATIC" speed="fast" />
         <Detector class="jp.skypencil.findbugs.slf4j.IllegalPassedClassDetector" reports="SLF4J_ILLEGAL_PASSED_CLASS" speed="fast" />
         <Detector class="jp.skypencil.findbugs.slf4j.ManualMessageDetector" reports="SLF4J_MANUALLY_PROVIDED_MESSAGE,SLF4J_FORMAT_SHOULD_BE_CONST" speed="fast" />
+        <Detector class="jp.skypencil.findbugs.slf4j.ManualGetStackTraceDetector" reports="SLF4J_GET_STACK_TRACE" speed="fast" />
+
         <BugPattern type="SLF4J_PLACE_HOLDER_MISMATCH" abbrev="SLF4J" category="CORRECTNESS" />
         <BugPattern type="SLF4J_FORMAT_SHOULD_BE_CONST" abbrev="SLF4J" category="CORRECTNESS" />
         <BugPattern type="SLF4J_UNKNOWN_ARRAY" abbrev="SLF4J" category="CORRECTNESS" />
@@ -20,5 +22,6 @@
         <BugPattern type="SLF4J_ILLEGAL_PASSED_CLASS" abbrev="SLF4J" category="CORRECTNESS" />
         <BugPattern type="SLF4J_SIGN_ONLY_FORMAT" abbrev="SLF4J" category="BAD_PRACTICE" />
         <BugPattern type="SLF4J_MANUALLY_PROVIDED_MESSAGE" abbrev="SLF4J" category="BAD_PRACTICE" />
+        <BugPattern type="SLF4J_GET_STACK_TRACE" abbrev="SLF4J" category="BAD_PRACTICE" />
 
 </FindbugsPlugin>

bug-pattern/src/main/resources/messages.xml

@@ -43,6 +43,12 @@
     </Details>
   </Detector>
 
+  <Detector class="jp.skypencil.findbugs.slf4j.ManualGetStackTraceDetector">
+    <Details>
+      Finds log which uses Throwable#getStackTrace.
+    </Details>
+  </Detector>
+
   <BugPattern type="SLF4J_PLACE_HOLDER_MISMATCH">
     <ShortDescription>Count of placeholder is not equal to count of parameter</ShortDescription>
     <LongDescription>Count of placeholder({5}) is not equal to count of parameter({4})</LongDescription>
@@ -146,5 +152,17 @@ You cannot use array which is provided as method argument or returned from other
     </Details>
   </BugPattern>
 
+  <BugPattern type="SLF4J_GET_STACK_TRACE">
+    <ShortDescription>Do not use Throwable#getStackTrace.</ShortDescription>
+    <LongDescription>
+      Do not have use Throwable#getStackTrace. It is enough to provide throwable instance as the last argument, then binding will log its message.
+    </LongDescription>
+    <Details>
+<![CDATA[
+<p>Do not use Throwable#getStackTrace.</p>
+]]>
+    </Details>
+  </BugPattern>
+
   <BugCode abbrev="SLF4J">SLF4J</BugCode>
 </MessageCollection>

test-case/src/main/java/pkg/UsingGetStackTrace.java

@@ -0,0 +1,13 @@
+package pkg;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class UsingGetStackTrace {
+
+    private final Logger logger = LoggerFactory.getLogger(getClass());
+
+    void method(RuntimeException ex) {
+        logger.error("Failed to determine The Answer to Life, The Universe and Everything; {}", 27, ex.getStackTrace());
+    }
+}

test-case/src/test/java/jp/skypencil/findbugs/slf4j/BugInstanceMatcher.java

@@ -0,0 +1,31 @@
+package jp.skypencil.findbugs.slf4j;
+
+import edu.umd.cs.findbugs.BugInstance;
+import java.util.function.Predicate;
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.Description;
+
+/**
+ * Hamcrest matcher for FindBugs (now SpotBugs) {@link BugInstance}.
+ *
+ * @author Michael Vorburger.ch
+ */
+public final class BugInstanceMatcher extends BaseMatcher<BugInstance> {
+
+    private final Predicate<BugInstance> predicate;
+
+    public BugInstanceMatcher(Predicate<BugInstance> predicate) {
+        this.predicate = predicate;
+    }
+
+    @Override
+    public final boolean matches(Object item) {
+        return predicate.test((BugInstance) item);
+    }
+
+    @Override
+    public void describeTo(Description description) {
+        description.appendText("BugInstance must match expected");
+    }
+
+}

test-case/src/test/java/jp/skypencil/findbugs/slf4j/UsingGetStackTraceTest.java

@@ -0,0 +1,25 @@
+package jp.skypencil.findbugs.slf4j;
+
+import com.youdevise.fbplugins.tdd4fb.DetectorAssert;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.Detector;
+import org.junit.jupiter.api.Test;
+import pkg.UsingGetStackTrace;
+
+/**
+ * Test for {@link ManualGetStackTraceDetector}.
+ *
+ * @author Michael Vorburger.ch
+ */
+public class UsingGetStackTraceTest {
+
+    @Test
+    public void testExceptionMethods() throws Exception {
+        BugReporter bugReporter = DetectorAssert.bugReporterForTesting();
+        Detector detector = new ManualGetStackTraceDetector(bugReporter);
+        DetectorAssert.assertNoBugsReported(UsingGetStackTrace.class, detector, bugReporter);
+        DetectorAssert.assertBugReported(UsingGetStackTrace.class, detector, bugReporter, new BugInstanceMatcher(
+                bugInstance -> bugInstance.getBugPattern().getAbbrev().equals("SLF4J_GET_STACK_TRACE")));
+    }
+
+}