diff --git a/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualMessageDetector.java b/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualMessageDetector.java index eef6d2ce..4db7f526 100644 --- a/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualMessageDetector.java +++ b/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/ManualMessageDetector.java @@ -1,31 +1,29 @@ package jp.skypencil.findbugs.slf4j; -import static org.apache.bcel.Const.INVOKEVIRTUAL; - import com.google.common.base.Objects; +import jp.skypencil.findbugs.slf4j.parameter.AbstractDetectorForParameterArray2; +import jp.skypencil.findbugs.slf4j.parameter.ArrayData; import edu.umd.cs.findbugs.BugInstance; import edu.umd.cs.findbugs.BugReporter; import edu.umd.cs.findbugs.OpcodeStack.CustomUserValue; import edu.umd.cs.findbugs.OpcodeStack.Item; import javax.annotation.Nullable; import jp.skypencil.findbugs.slf4j.parameter.AbstractDetectorForParameterArray; -import jp.skypencil.findbugs.slf4j.parameter.ArrayData; import jp.skypencil.findbugs.slf4j.parameter.ArrayDataHandler.Strategy; @CustomUserValue public final class ManualMessageDetector extends AbstractDetectorForParameterArray { + @Item.SpecialKind private final int isMessage = Item.defineSpecialKind("message generated by throwable object"); public ManualMessageDetector(BugReporter bugReporter) { - super(bugReporter); + super(bugReporter, "SLF4J_MANUALLY_PROVIDED_MESSAGE"); } @Override protected Strategy createArrayCheckStrategy() { - return new Strategy() { - @Override - public void store(Item storedItem, ArrayData arrayData, int index) { + return (storedItem, arrayData, index) -> { if (arrayData == null) { return; } @@ -39,8 +37,7 @@ public void store(Item storedItem, ArrayData arrayData, int index) { if (index == arrayData.getSize() - 1 && !getThrowableHandler().checkThrowable(storedItem)) { arrayData.mark(false); } - } - }; + }; } @Override @@ -53,6 +50,20 @@ public void afterOpcode(int seen) { } } + + @Override + protected int getIsOfInterestKind() { + return isMessage; + } + + @Override + protected boolean isReallyOfInterest(Item storedItem, ArrayData arrayData, int index) { + // Let developer logs exception message, only when argument does not have throwable instance + // https://github.com/KengoTODA/findbugs-slf4j/issues/31 + return !(index == arrayData.getSize() - 1 + && !getThrowableHandler().checkThrowable(storedItem)); + } + private boolean isInvokingGetMessage(int seen) { return seen == INVOKEVIRTUAL && !stack.isTop() @@ -73,4 +84,13 @@ protected void onLog(@Nullable String format, @Nullable ArrayData arrayData) { .addCalledMethod(this); getBugReporter().reportBug(bugInstance); } + + @Override + protected boolean isWhatWeWantToDetect(int seen) { + return seen == INVOKEVIRTUAL && !stack.isTop() + && getThrowableHandler().checkThrowable(getStack().getStackItem(0)) + && (Objects.equal("getMessage", getNameConstantOperand()) + || Objects.equal("getLocalizedMessage", getNameConstantOperand())); + } + } diff --git a/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/parameter/AbstractDetectorForParameterArray2.java b/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/parameter/AbstractDetectorForParameterArray2.java new file mode 100644 index 00000000..590f7d5c --- /dev/null +++ b/bug-pattern/src/main/java/jp/skypencil/findbugs/slf4j/parameter/AbstractDetectorForParameterArray2.java @@ -0,0 +1,65 @@ +package jp.skypencil.findbugs.slf4j.parameter; + +import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugReporter; +import edu.umd.cs.findbugs.OpcodeStack.Item; +import javax.annotation.Nullable; +import jp.skypencil.findbugs.slf4j.parameter.ArrayDataHandler.Strategy; + +public abstract class AbstractDetectorForParameterArray2 extends AbstractDetectorForParameterArray { + + private final String bugPatternName; + + public AbstractDetectorForParameterArray2(BugReporter bugReporter, String bugPatternName) { + super(bugReporter); + this.bugPatternName = bugPatternName; + } + + @Override + protected final Strategy createArrayCheckStrategy() { + return (storedItem, arrayData, index) -> { + if (arrayData == null) { + return; + } + + if (storedItem.getSpecialKind() == getIsOfInterestKind()) { + arrayData.mark(true); + } + + if (!isReallyOfInterest(storedItem, arrayData, index)) { + arrayData.mark(false); + } + }; + } + + protected boolean isReallyOfInterest(Item storedItem, ArrayData arrayData, int index) { + return true; + } + + @Override + public final void afterOpcode(int seen) { + boolean isInvokingGetMessage = isWhatWeWantToDetect(seen); + super.afterOpcode(seen); + + if (isInvokingGetMessage && !stack.isTop()) { + stack.getStackItem(0).setSpecialKind(getIsOfInterestKind()); + } + } + + @Override + protected final void onLog(@Nullable String format, @Nullable ArrayData arrayData) { + if (arrayData == null || !arrayData.isMarked()) { + return; + } + BugInstance bugInstance = new BugInstance(this, + bugPatternName, NORMAL_PRIORITY) + .addSourceLine(this).addClassAndMethod(this) + .addCalledMethod(this); + getBugReporter().reportBug(bugInstance); + } + + abstract protected boolean isWhatWeWantToDetect(int seen); + + abstract protected int getIsOfInterestKind(); + +}