Add notes of potential references to ReferenceCreator

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/Reference.java

@@ -313,7 +313,6 @@ public class Reference {
 
     @Override
     public int hashCode() {
-      // will cause collisions for overloaded method refs but performance hit should be negligable
       return name.hashCode();
     }
   }

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java

@@ -16,6 +16,10 @@ import net.bytebuddy.jar.asm.MethodVisitor;
 import net.bytebuddy.jar.asm.Opcodes;
 
 /** Visit a class and collect all references made by the visited class. */
+// additional things we could check
+// - annotations on class
+// - outer class
+// - inner class
 public class ReferenceCreator extends ClassVisitor {
   /**
    * Generate all references reachable from a given class.
@@ -97,9 +101,19 @@ public class ReferenceCreator extends ClassVisitor {
       final String superName,
       final String[] interfaces) {
     refSourceClassName = Utils.getClassName(name);
+    // Additional references we could check
+    // - supertype of class and visible from this package
+    // - interfaces of class and visible from this package
     super.visit(version, access, name, signature, superName, interfaces);
   }
 
+  @Override
+  public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) {
+    // Additional references we could check
+    // - type of field + visible from this package
+    return super.visitField(access, name, descriptor, signature, value);
+  }
+
   @Override
   public MethodVisitor visitMethod(
       final int access,
@@ -107,6 +121,8 @@ public class ReferenceCreator extends ClassVisitor {
       final String descriptor,
       final String signature,
       final String[] exceptions) {
+    // Additional references we could check
+    // - Classes in signature (return type, params) and visible from this package
     return new AdviceReferenceMethodVisitor(
         super.visitMethod(access, name, descriptor, signature, exceptions));
   }
@@ -124,6 +140,28 @@ public class ReferenceCreator extends ClassVisitor {
       super.visitLineNumber(line, start);
     }
 
+    @Override
+    public void visitFieldInsn(int opcode, String owner, String name, String descriptor) {
+      // Additional references we could check
+      // * DONE owner class
+      //   * owner class has a field (name)
+      //   * field is static or non-static
+      //   * field's visibility from this point (NON_PRIVATE?)
+      // * owner class's visibility from this point (NON_PRIVATE?)
+      //
+      // * DONE field-source class (descriptor)
+      //   * field-source visibility from this point (PRIVATE?)
+
+      // owning class has a field
+      addReference(new Reference.Builder(owner)
+        .withSource(refSourceClassName, currentLineNumber)
+        .build());
+      addReference(new Reference.Builder(Type.getType(descriptor).getInternalName())
+        .withSource(refSourceClassName, currentLineNumber)
+        .build());
+      super.visitFieldInsn(opcode, owner, name, descriptor);
+    }
+
     @Override
     public void visitMethodInsn(
         final int opcode,
@@ -131,8 +169,20 @@ public class ReferenceCreator extends ClassVisitor {
         final String name,
         final String descriptor,
         final boolean isInterface) {
-      addReference(
+      // Additional references we could check
-          new Reference.Builder(owner).withSource(refSourceClassName, currentLineNumber).build());
+      // * DONE name of method owner's class
+      //   * is the owner an interface?
+      //   * owner's access from here (PRIVATE?)
+      //   * method on the owner class
+      //   * is the method static? Is it visible from here?
+      // * Class names from the method descriptor
+      //   * params classes
+      //   * return type
+
+      addReference(new Reference.Builder(owner)
+        .withSource(refSourceClassName, currentLineNumber)
+        .build());
+      super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
     }
   }
 }

dd-java-agent/testing/src/test/groovy/muzzle/ReferenceCreatorTest.groovy

@@ -0,0 +1,22 @@
+package muzzle
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.tooling.muzzle.Reference
+import datadog.trace.agent.tooling.muzzle.ReferenceCreator
+import static muzzle.TestClasses.*
+
+class ReferenceCreatorTest extends AgentTestRunner {
+  def "method body creates references"() {
+    setup:
+    Map<String, Reference> references = ReferenceCreator.createReferencesFrom(MethodBodyAdvice.getName(), this.getClass().getClassLoader())
+
+    expect:
+    references.get('java.lang.Object') != null
+    references.get('muzzle.TestClasses$MethodBodyAdvice$A') != null
+    references.get('muzzle.TestClasses$MethodBodyAdvice$B') != null
+    references.get('muzzle.TestClasses$MethodBodyAdvice$SomeInterface') != null
+    references.get('muzzle.TestClasses$MethodBodyAdvice$SomeImplementation') != null
+    references.keySet().size() == 5
+  }
+
+}

dd-java-agent/testing/src/test/groovy/muzzle/ReferenceMatcherTest.groovy

@@ -0,0 +1,36 @@
+package muzzle
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.agent.test.TestUtils
+import datadog.trace.agent.tooling.muzzle.Reference
+import datadog.trace.agent.tooling.muzzle.ReferenceCreator
+import datadog.trace.agent.tooling.muzzle.ReferenceMatcher
+import spock.lang.Shared
+
+import static muzzle.TestClasses.*
+
+class ReferenceMatcherTest extends AgentTestRunner {
+
+  @Shared
+  ClassLoader safeClasspath = new URLClassLoader([TestUtils.createJarWithClasses(MethodBodyAdvice.A,
+    MethodBodyAdvice.B,
+    MethodBodyAdvice.SomeInterface,
+    MethodBodyAdvice.SomeImplementation)] as URL[],
+    (ClassLoader) null)
+
+  @Shared
+  ClassLoader unsafeClasspath = new URLClassLoader([TestUtils.createJarWithClasses(MethodBodyAdvice.A,
+    MethodBodyAdvice.SomeInterface,
+    MethodBodyAdvice.SomeImplementation)] as URL[],
+    (ClassLoader) null)
+
+  def "match safe classpaths"() {
+    setup:
+    Reference[] refs = ReferenceCreator.createReferencesFrom(MethodBodyAdvice.getName(), this.getClass().getClassLoader()).values().toArray(new Reference[0])
+    ReferenceMatcher refMatcher = new ReferenceMatcher(refs)
+
+    expect:
+    refMatcher.getMismatchedReferenceSources(safeClasspath).size() == 0
+    refMatcher.getMismatchedReferenceSources(unsafeClasspath).size() == 1
+  }
+}

dd-java-agent/testing/src/test/java/muzzle/AdviceClass.java

@@ -1,24 +0,0 @@
-package muzzle;
-
-import net.bytebuddy.asm.Advice;
-
-public class AdviceClass {
-
-  @Advice.OnMethodEnter
-  public static void advice() {
-    A a = new A();
-    SomeInterface inter = new SomeImplementation();
-    inter.someMethod();
-  }
-
-  public static class A {}
-
-  public interface SomeInterface {
-    void someMethod();
-  }
-
-  public static class SomeImplementation implements SomeInterface {
-    @Override
-    public void someMethod() {}
-  }
-}

dd-java-agent/testing/src/test/java/muzzle/TestClasses.java

@@ -0,0 +1,38 @@
+package muzzle;
+
+import net.bytebuddy.asm.Advice;
+
+public class TestClasses {
+
+  public static class MethodBodyAdvice {
+    @Advice.OnMethodEnter
+    public static void methodBodyAdvice() {
+      A a = new A();
+      SomeInterface inter = new SomeImplementation();
+      inter.someMethod();
+      a.b.toString();
+    }
+
+    public static class A {
+      public B b = new B();
+    }
+
+    public static class B {}
+
+    public interface SomeInterface {
+      void someMethod();
+    }
+
+    public static class SomeImplementation implements SomeInterface {
+      @Override
+      public void someMethod() {}
+    }
+
+    public static class SomeClassWithFields {
+      public int instanceField = 0;
+      public static int staticField = 0;
+      public final int finalField = 0;
+    }
+  }
+
+}