From 33f4abd70a69e0f3c2d2aa1817f4154847b57494 Mon Sep 17 00:00:00 2001 From: Michael Vorburger Date: Wed, 14 Mar 2018 17:09:39 +0100 Subject: [PATCH] add new SLF4J_GET_STACK_TRACE detector (fixes #70) --- .../slf4j/ManualGetStackTraceDetector.java | 36 +++++++++++++++++++ bug-pattern/src/main/resources/bugrank.txt | 1 + bug-pattern/src/main/resources/findbugs.xml | 3 ++ bug-pattern/src/main/resources/messages.xml | 18 ++++++++++ .../src/main/java/pkg/UsingGetStackTrace.java | 13 +++++++ .../findbugs/slf4j/BugInstanceMatcher.java | 31 ++++++++++++++++ .../slf4j/UsingGetStackTraceTest.java | 25 +++++++++++++ 7 files changed, 127 insertions(+) create mode 100644 bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java create mode 100644 test-case/src/main/java/pkg/UsingGetStackTrace.java create mode 100644 test-case/src/test/java/jp/skypencil/findbugs/slf4j/BugInstanceMatcher.java create mode 100644 test-case/src/test/java/jp/skypencil/findbugs/slf4j/UsingGetStackTraceTest.java diff --git a/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java b/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualGetStackTraceDetector.java new file mode 100644 index 00000000..7189443c --- /dev/null +++ b/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()); + } + +} diff --git a/bug-pattern/src/main/resources/bugrank.txt b/bug-pattern/src/main/resources/bugrank.txt index 0084d2f3..ff0848c4 100644 --- a/bug-pattern/src/main/resources/bugrank.txt +++ b/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 diff --git a/bug-pattern/src/main/resources/findbugs.xml b/bug-pattern/src/main/resources/findbugs.xml index 1b352742..980bf0c1 100644 --- a/bug-pattern/src/main/resources/findbugs.xml +++ b/bug-pattern/src/main/resources/findbugs.xml @@ -11,6 +11,8 @@ + + @@ -20,5 +22,6 @@ + diff --git a/bug-pattern/src/main/resources/messages.xml b/bug-pattern/src/main/resources/messages.xml index ca9e091a..a7eef1a3 100644 --- a/bug-pattern/src/main/resources/messages.xml +++ b/bug-pattern/src/main/resources/messages.xml @@ -43,6 +43,12 @@ + +
+ Finds log which uses Throwable#getStackTrace. +
+ + Count of placeholder is not equal to count of parameter Count of placeholder({5}) is not equal to count of parameter({4}) @@ -146,5 +152,17 @@ You cannot use array which is provided as method argument or returned from other + + Do not use Throwable#getStackTrace. + + Do not have use Throwable#getStackTrace. It is enough to provide throwable instance as the last argument, then binding will log its message. + +
+Do not use Throwable#getStackTrace.

+]]> +
+ + SLF4J diff --git a/test-case/src/main/java/pkg/UsingGetStackTrace.java b/test-case/src/main/java/pkg/UsingGetStackTrace.java new file mode 100644 index 00000000..e0abafc7 --- /dev/null +++ b/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()); + } +} diff --git a/test-case/src/test/java/jp/skypencil/findbugs/slf4j/BugInstanceMatcher.java b/test-case/src/test/java/jp/skypencil/findbugs/slf4j/BugInstanceMatcher.java new file mode 100644 index 00000000..e36aa0c0 --- /dev/null +++ b/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 { + + private final Predicate predicate; + + public BugInstanceMatcher(Predicate 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"); + } + +} diff --git a/test-case/src/test/java/jp/skypencil/findbugs/slf4j/UsingGetStackTraceTest.java b/test-case/src/test/java/jp/skypencil/findbugs/slf4j/UsingGetStackTraceTest.java new file mode 100644 index 00000000..a3f25361 --- /dev/null +++ b/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"))); + } + +}