For static method calls and field accesses do not look into superclas…

…ses (#238)

src/main/java/de/thetaphi/forbiddenapis/ClassScanner.java

@@ -129,7 +129,10 @@ static interface AncestorVisitor {
   String visitAncestors(ClassMetadata cls, AncestorVisitor visitor, boolean visitSelf, boolean visitInterfacesFirst) {
     if (visitSelf) {
       final String result = visitor.visit(cls, cls.className, cls.isInterface, cls.isRuntimeClass);
-      if (result != null && result != AncestorVisitor.STOP) {
+      if (result == AncestorVisitor.STOP) {
+        return null;
+      }
+      if (result != null) {
         return result;
       }
     }
@@ -379,7 +382,7 @@ public MethodVisitor visitMethod(final int access, final String name, final Stri
         }
       }
 
-      private String checkMethodAccess(String owner, final Method method) {
+      private String checkMethodAccess(String owner, final Method method, final boolean callIsVirtual) {
         if  (CLASS_CONSTRUCTOR_METHOD_NAME.equals(method.getName())) {
           // we don't check for violations on class constructors
           return null;
@@ -413,8 +416,10 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
             if (!c.methods.contains(lookupMethod)) {
               return null;
             }
+            // is we have a virtual call, look into superclasses, otherwise stop:
+            final String notFoundRet = callIsVirtual ? null : AncestorVisitor.STOP;
             if (previousInRuntime && c.isNonPortableRuntime) {
-              return null; // something inside the JVM is extending internal class/interface
+              return notFoundRet; // something inside the JVM is extending internal class/interface
             }
             String violation = forbiddenSignatures.checkMethod(c.className, lookupMethod);
             if (violation != null) {
@@ -427,7 +432,7 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
                 return violation;
               }
             }
-            return null;
+            return notFoundRet;
           }
         }, true, false /* JVM spec says: interfaces after superclasses */);
       }
@@ -492,7 +497,8 @@ private String checkHandle(Handle handle, boolean checkLambdaHandle) {
               // so we can assign the called lambda with the same groupId like *this* method:
               lambdas.put(m, currentGroupId);
             }
-            return checkMethodAccess(handle.getOwner(), m);
+            final boolean callIsVirtual = (handle.getTag() == Opcodes.H_INVOKEVIRTUAL) || (handle.getTag() == Opcodes.H_INVOKEINTERFACE);
+            return checkMethodAccess(handle.getOwner(), m, callIsVirtual);
         }
         return null;
       }
@@ -550,7 +556,8 @@ public AnnotationVisitor visitTryCatchAnnotation(int typeRef, TypePath typePath,
 
       @Override
       public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
-        reportMethodViolation(checkMethodAccess(owner, new Method(name, desc)), "method body");
+        final boolean callIsVirtual = (opcode == Opcodes.INVOKEVIRTUAL) || (opcode == Opcodes.INVOKEINTERFACE);
+        reportMethodViolation(checkMethodAccess(owner, new Method(name, desc), callIsVirtual), "method body");
       }
 
       @Override

src/test/antunit/Java7StaticVsVirtual.java

@@ -0,0 +1,65 @@
+/*
+ * (C) Copyright Uwe Schindler (Generics Policeman) and others.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* The binary class file is packaged together with the source distribution.
+ */
+
+import java.util.BitSet;
+
+public class Java7StaticVsVirtual {
+  public static final long[] data = new long[] {1, 2, 3, 4};
+
+  public static void main(String[] args) {
+    X.valueOf(data).toString();  // Line 26 -- the only violation for static BitSet#valueOf(**)
+    Y.valueOf(data).toString();  // Line 27 -- should pass
+    Z.valueOf(data).toString();  // Line 28 -- should pass
+    Integer.toString(Y.a); // Line 29 -- violation (static field access)
+    Integer.toString(Z.a); // Line 30 -- should pass (hidden)
+    new X().get(0);  // Line 31 -- violation (virtual methods detected regardles how they are called)
+    new Y().get(0);  // Line 32 -- violation (virtual methods detected regardles how they are called)
+    new Z().get(0);  // Line 33 -- violation (virtual methods detected regardles how they are called)
+    Integer.toString(new Y().b);  // Line 34 -- violation
+    Integer.toString(new Z().b);  // Line 35 -- should pass (hidden)
+  }
+
+  public static class X extends BitSet { }
+
+  public static class Y extends X {
+    public static int a;
+    public int b;
+
+    public static BitSet valueOf(long[] longs) {
+      return new BitSet();
+    }
+
+    @Override
+    public boolean get(int bit) {
+      return false;
+    }
+  }
+
+  public static class Z extends Y {
+    public static int a; // hides field in superclass
+    public int b; // hides field in superclass
+
+    public String goStatic() {
+      return valueOf(data).toString();  // Line 59 -- should pass
+    }
+    public boolean goVirtual() {
+      return get(0);  // Line 62 -- violation (virtual methods detected regardles how they are called)
+    }
+  }
+}

src/test/antunit/TestStaticVsVirtual.xml

@@ -0,0 +1,61 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ * (C) Copyright Uwe Schindler (Generics Policeman) and others.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+-->
+<project xmlns:au="antlib:org.apache.ant.antunit">
+
+  <target name="testStatic">
+    <au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
+      <forbiddenapis>
+        <fileset file="Java7StaticVsVirtual*.class"/>
+        java.util.BitSet#valueOf(**) @ Forbidden static method
+      </forbiddenapis>
+    </au:expectfailure>
+    <au:assertLogContains level="error" text="Forbidden method invocation: java.util.BitSet#valueOf(**) [Forbidden static method]"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:26)"/>
+    <au:assertLogContains level="error" text=" 1 error(s)"/> 
+  </target>
+
+  <target name="testVirtual">
+    <au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
+      <forbiddenapis>
+        <fileset file="Java7StaticVsVirtual*.class"/>
+        java.util.BitSet#get(int) @ Forbidden virtual method
+      </forbiddenapis>
+    </au:expectfailure>
+    <au:assertLogContains level="error" text="Forbidden method invocation: java.util.BitSet#get(int) [Forbidden virtual method]"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:31)"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:32)"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:33)"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:62)"/>
+    <au:assertLogContains level="error" text=" 4 error(s)"/> 
+  </target>
+
+  <target name="testFields">
+    <au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
+      <forbiddenapis classpath=".">
+        <fileset file="Java7StaticVsVirtual*.class"/>
+        Java7StaticVsVirtual$Y#a @ Forbidden static field
+        Java7StaticVsVirtual$Y#b @ Forbidden instance field        
+      </forbiddenapis>
+    </au:expectfailure>
+    <au:assertLogContains level="error" text="Forbidden field access: Java7StaticVsVirtual$Y#a [Forbidden static field]"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:29)"/> 
+    <au:assertLogContains level="error" text="Forbidden field access: Java7StaticVsVirtual$Y#b [Forbidden instance field]"/>
+    <au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:34)"/> 
+    <au:assertLogContains level="error" text=" 2 error(s)"/> 
+  </target>
+
+</project>