From e53e0e168d37101f2e7daeecdc53b148d84125b7 Mon Sep 17 00:00:00 2001 From: Kengo TODA Date: Mon, 7 Apr 2014 14:42:19 +0800 Subject: [PATCH] add new detector which uses Guava's @VisibleForTesting. --- README.md | 6 +- .../guava/UnexpectedAccessDetector.java | 109 ++++++++++++++++++ src/main/meta/bugrank.txt | 1 + src/main/meta/findbugs.xml | 5 + src/main/meta/messages.xml | 24 ++++ .../guava/ClassWhichCallsNormalMethod.java | 12 ++ ...lassWhichCallsVisibleMethodForTesting.java | 7 ++ .../guava/MethodWithVisibleForTesting.java | 8 ++ .../guava/MethodWithoutVisibleForTesting.java | 5 + .../guava/UnexpectedAccessDetectorTest.java | 34 ++++++ 10 files changed, 210 insertions(+), 1 deletion(-) create mode 100644 src/main/java/jp/co/worksap/oss/findbugs/guava/UnexpectedAccessDetector.java create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/guava/ClassWhichCallsNormalMethod.java create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/guava/ClassWhichCallsVisibleMethodForTesting.java create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/guava/MethodWithVisibleForTesting.java create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/guava/MethodWithoutVisibleForTesting.java create mode 100644 src/test/java/jp/co/worksap/oss/findbugs/guava/UnexpectedAccessDetectorTest.java diff --git a/README.md b/README.md index c7f400c..27ad839 100755 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ To use this product, please configure your findbugs-maven-plugin like below. jp.co.worksap.oss findbugs-plugin - 0.0.2 + 0.0.3-SNAPSHOT @@ -27,6 +27,10 @@ To use this product, please configure your findbugs-maven-plugin like below. # history +## 0.0.3 + +- added UnexpectedAccessDetector + ## 0.0.2 - added BrokenImmutableClassDetector diff --git a/src/main/java/jp/co/worksap/oss/findbugs/guava/UnexpectedAccessDetector.java b/src/main/java/jp/co/worksap/oss/findbugs/guava/UnexpectedAccessDetector.java new file mode 100644 index 0000000..c78b685 --- /dev/null +++ b/src/main/java/jp/co/worksap/oss/findbugs/guava/UnexpectedAccessDetector.java @@ -0,0 +1,109 @@ +package jp.co.worksap.oss.findbugs.guava; + +import static com.google.common.base.Preconditions.checkNotNull; + +import javax.annotation.Nonnull; + +import org.apache.bcel.Repository; +import org.apache.bcel.classfile.AnnotationEntry; +import org.apache.bcel.classfile.JavaClass; +import org.apache.bcel.classfile.Method; + +import edu.umd.cs.findbugs.BugInstance; +import edu.umd.cs.findbugs.BugReporter; +import edu.umd.cs.findbugs.BytecodeScanningDetector; +import edu.umd.cs.findbugs.bcel.BCELUtil; +import edu.umd.cs.findbugs.classfile.ClassDescriptor; +import edu.umd.cs.findbugs.classfile.MethodDescriptor; + +/** + *

A detector to ensure that implementation (class in src/main/java) doesn't call + * package-private method in other class which is annotated by {@code @VisibleForTesting}.

+ * + * @author Kengo TODA (toda_k@worksap.co.jp) + * @see com.google.common.annotations.VisibleForTesting + */ +public class UnexpectedAccessDetector extends BytecodeScanningDetector { + @Nonnull + private final BugReporter bugReporter; + + public UnexpectedAccessDetector(BugReporter bugReporter) { + this.bugReporter = checkNotNull(bugReporter); + } + + @Override + public void sawOpcode(int opcode) { + if (! isInvoking(opcode)) { + return; + } + + ClassDescriptor currentClass = getClassDescriptor(); + ClassDescriptor invokedClass = getClassDescriptorOperand(); + if (currentClass.equals(invokedClass)) { + // no need to check, because method is called by owner + } else if (! currentClass.getPackageName().equals(invokedClass.getPackageName())) { + // no need to check, because method is called by class in other package + } else { + MethodDescriptor invokedMethod = getMethodDescriptorOperand(); + + try { + verifyVisibility(invokedClass, invokedMethod); + } catch (ClassNotFoundException e) { + throw new IllegalStateException(e); + } + } + } + + /** + *

Report if specified method is package-private and annotated by {@code @VisibleForTesting}.

+ */ + private void verifyVisibility(ClassDescriptor invokedClass, MethodDescriptor invokedMethod) throws ClassNotFoundException { + JavaClass bcelClass = Repository.getRepository().loadClass(invokedClass.getDottedClassName()); + Method bcelMethod = findMethod(bcelClass, invokedMethod); + + if (checkVisibility(bcelMethod) && checkAnnotated(bcelMethod)) { + BugInstance bug = new BugInstance(this, "GUAVA_UNEXPECTED_ACCESS_TO_VISIBLE_FOR_TESTING", HIGH_PRIORITY) + .addCalledMethod(this).addClassAndMethod(this).addSourceLine(this); + bugReporter.reportBug(bug); + } + } + + private Method findMethod(JavaClass bcelClass, MethodDescriptor invokedMethod) { + for (Method bcelMethod : bcelClass.getMethods()) { + MethodDescriptor methodDescriptor = BCELUtil.getMethodDescriptor(bcelClass, bcelMethod); + if (methodDescriptor.equals(invokedMethod)) { + return bcelMethod; + } + } + + throw new IllegalArgumentException("Method not found: " + invokedMethod); + } + + /** + * @return true if visibility of specified method is package-private. + */ + private boolean checkVisibility(Method bcelMethod) { + return ! (bcelMethod.isPrivate() || bcelMethod.isProtected() || bcelMethod.isPublic()); + } + + /** + * @return true if specified method is annotated by {@code VisibleForTesting}. + */ + private boolean checkAnnotated(Method bcelMethod) { + for (AnnotationEntry annotation : bcelMethod.getAnnotationEntries()) { + String type = annotation.getAnnotationType(); + if ("Lcom/google/common/annotations/VisibleForTesting;".equals(type)) { + return true; + } + } + return false; + } + + private boolean isInvoking(int opcode) { + return opcode == INVOKESPECIAL || + opcode == INVOKEINTERFACE || + opcode == INVOKESTATIC || + opcode == INVOKEVIRTUAL; + } + +} diff --git a/src/main/meta/bugrank.txt b/src/main/meta/bugrank.txt index 5bbbeb5..cef2572 100755 --- a/src/main/meta/bugrank.txt +++ b/src/main/meta/bugrank.txt @@ -12,3 +12,4 @@ 0 BugPattern ILLEGAL_LENGTH 0 BugPattern USE_COLUMN_DEFINITION 0 BugPattern NULLABLE_PRIMITIVE +0 BugPattern GUAVA_UNEXPECTED_ACCESS_TO_VISIBLE_FOR_TESTING diff --git a/src/main/meta/findbugs.xml b/src/main/meta/findbugs.xml index 0db4367..a7885f7 100755 --- a/src/main/meta/findbugs.xml +++ b/src/main/meta/findbugs.xml @@ -64,4 +64,9 @@ +