Merge pull request #62 from spotify/krka/fix

Fix issue#55

core/pom.xml

@@ -20,7 +20,7 @@
     <dependency>
       <groupId>org.ow2.asm</groupId>
       <artifactId>asm-tree</artifactId>
-      <version>7.1</version>
+      <version>7.2</version>
     </dependency>
     <dependency>
       <groupId>io.norberg</groupId>

core/src/main/java/com/spotify/missinglink/ClassLoader.java

@@ -32,6 +32,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -57,6 +58,13 @@
  */
 public final class ClassLoader {
 
+  // This is a set of classes that is using @HotSpotIntrinsicCandidate
+  // and thus define native methods that don't actually exist in the class file
+  // This could be removed if we stop loading the full JDK
+  private static final Set<String> BLACKLIST = new HashSet<>(Arrays.asList(
+      "java/lang/invoke/MethodHandle"
+  ));
+
   private ClassLoader() {
     // prevent instantiation
   }
@@ -209,7 +217,7 @@ private static void handleMethodCall(final Set<CalledMethod> thisCalls,
       default:
         throw new RuntimeException("Unexpected method call opcode: " + insn.getOpcode());
     }
-    if (insn.owner.charAt(0) != '[') {
+    if (isArray(insn.owner) && !BLACKLIST.contains(insn.owner)) {
       thisCalls.add(new CalledMethodBuilder()
           .owner(TypeDescriptors.fromClassName(insn.owner))
           .descriptor(MethodDescriptors.fromDesc(insn.desc, insn.name))
@@ -224,7 +232,7 @@ private static void handleMethodCall(final Set<CalledMethod> thisCalls,
   private static void handleFieldAccess(Set<AccessedField> thisFields, int lineNumber,
                                         FieldInsnNode insn,
                                         final List<TryCatchBlockNode> tryCatchBlocksProtecting) {
-    if (insn.owner.charAt(0) != '[') {
+    if (isArray(insn.owner)) {
       thisFields.add(
           new AccessedFieldBuilder()
               .name(insn.name)
@@ -238,6 +246,10 @@ private static void handleFieldAccess(Set<AccessedField> thisFields, int lineNum
     }
   }
 
+  private static boolean isArray(String owner) {
+    return owner.charAt(0) != '[';
+  }
+
   private static void handleLdc(Set<ClassTypeDescriptor> loadedClasses, LdcInsnNode insn) {
     // See http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.ldc
     // if an LDC instruction is emitted with a symbolic reference to a class, that class is

core/src/test/java/com/spotify/missinglink/ClassLoadingUtil.java

@@ -34,15 +34,21 @@ public class ClassLoadingUtil {
       new AtomicReference<>();
 
   public static FileInputStream findClass(Class<?> aClass) throws Exception {
+    final String name = aClass.getName().replace('.', '/') + ".class";
     final File outputDir = FilePathHelper.getPath("target/test-classes");
-    File someClass = Files.walk(outputDir.toPath())
-        .map(Path::toFile)
-        .filter(file -> file.isFile() && file.getName().endsWith(aClass.getSimpleName() + ".class"))
-        .findFirst()
-        .orElseThrow(() -> new IllegalStateException("no file matching " + aClass + " found in " +
-                                                     outputDir + " ?"));
-
-    return new FileInputStream(someClass);
+    List<File> files = Files.walk(outputDir.toPath())
+            .map(Path::toFile)
+            .filter(file -> file.isFile() && file.getAbsolutePath().endsWith(name))
+            .collect(Collectors.toList());
+    if (files.isEmpty()) {
+      throw new IllegalStateException("no file matching " + aClass + " found in "
+              + outputDir + " ?");
+    }
+    if (files.size() >= 2) {
+      throw new IllegalStateException("too many files matching " + aClass + " found in "
+              + outputDir + ": " + files);
+    }
+    return new FileInputStream(files.get(0));
   }
 
   public static List<Artifact> bootstrapArtifacts() {

core/src/test/java/com/spotify/missinglink/FeatureTest.java

@@ -43,6 +43,8 @@
 import com.spotify.missinglink.datamodel.FieldDependencyBuilder;
 import com.spotify.missinglink.datamodel.MethodDependencyBuilder;
 import com.spotify.missinglink.datamodel.TypeDescriptors;
+
+import java.lang.invoke.MethodHandle;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -53,7 +55,7 @@ public class FeatureTest {
 
   private final ConflictChecker conflictChecker = new ConflictChecker();
 
-  @org.junit.Test
+  @Test
   public void testSimpleConflict() throws Exception {
 
     final DeclaredMethod methodOnlyInD1 = newMethod(false, INT, "foo").build();
@@ -86,7 +88,7 @@ public void testSimpleConflict() throws Exception {
         .isEqualTo(Collections.singletonList(expectedConflict));
   }
 
-  @org.junit.Test
+  @Test
   public void testSimpleConflict2() throws Exception {
 
     final DeclaredMethod methodOnlyInD1 = newMethod(false, INT, "foo").build();
@@ -123,7 +125,7 @@ public void testSimpleConflict2() throws Exception {
         .isEqualTo(Collections.singletonList(expectedConflict));
   }
 
-  @org.junit.Test
+  @Test
   public void testMissingField() throws Exception {
 
     final DeclaredClass d2Class = newClass("com/d/Foo").build();
@@ -158,7 +160,7 @@ public void testMissingField() throws Exception {
         .isEqualTo(Collections.singletonList(expectedConflict));
   }
 
-  @org.junit.Test
+  @Test
   public void testNoConflictWithInheritedMethodCall() throws Exception {
     final DeclaredMethod methodOnlyInSuper = newMethod(false, INT, "foo").build();
     final DeclaredClass superClass =
@@ -182,7 +184,7 @@ public void testNoConflictWithInheritedMethodCall() throws Exception {
     )).isEmpty();
   }
 
-  @org.junit.Test
+  @Test
   public void testNoConflictWithCovariantReturnType() throws Exception {
     final DeclaredMethod superMethod = newMethod(false, "Ljava/lang/CharSequence;", "foo").build();
     final DeclaredClass superClass = newClass("com/Super").methods(methodMap(superMethod)).build();
@@ -208,7 +210,7 @@ public void testNoConflictWithCovariantReturnType() throws Exception {
         )).isEmpty();
   }
 
-  @org.junit.Test
+  @Test
   public void testNoConflictWithStaticCall() throws Exception {
     final DeclaredMethod methodOnlyInSuper = newMethod(true, INT, "foo").build();
     final DeclaredClass superClass =
@@ -229,7 +231,7 @@ public void testNoConflictWithStaticCall() throws Exception {
     )).isEmpty();
   }
 
-  @org.junit.Test
+  @Test
   public void testConflictWithStaticToVirtualCall() throws Exception {
     final DeclaredMethod methodOnlyInSuper = newMethod(false, INT, "foo").build();
     final DeclaredClass superClass =
@@ -259,7 +261,7 @@ public void testConflictWithStaticToVirtualCall() throws Exception {
         ));
   }
 
-  @org.junit.Test
+  @Test
   public void testConflictWithVirtualToStaticCall() throws Exception {
     final DeclaredMethod methodOnlyInSuper = newMethod(true, INT, "foo").build();
     final DeclaredClass superClass =
@@ -291,7 +293,7 @@ public void testConflictWithVirtualToStaticCall() throws Exception {
         ));
   }
 
-  @org.junit.Test
+  @Test
   public void testNoConflictWithStaticCallInSuper() throws Exception {
     final DeclaredMethod methodOnlyInSuper = newMethod(true, INT, "foo").build();
     final DeclaredClass superClass =
@@ -316,7 +318,7 @@ public void testNoConflictWithStaticCallInSuper() throws Exception {
         ));
   }
 
-  @org.junit.Test
+  @Test
   public void testNoConflictWithSpecialCallToSuper() throws Exception {
     class SuperDuperClass {
 
@@ -350,6 +352,42 @@ boolean foo(Object o) {
         allArtifacts)).isEmpty();
   }
 
+  @Test
+  public void testNoConflictWithVarargCall() throws Exception {
+    class HasVararg {
+      public final native Object invoke(Object... args);
+    }
+    class MainClass {
+      void main(HasVararg hasVararg) {
+        hasVararg.invoke();
+      }
+    }
+
+    DeclaredClass hasVararg = load(findClass(HasVararg.class));
+    DeclaredClass mainClass = load(findClass(MainClass.class));
+
+    final Artifact art = newArtifact("root", mainClass, hasVararg);
+
+    List<Artifact> allArtifacts = new ArrayList<>(ClassLoadingUtil.bootstrapArtifacts());
+    allArtifacts.add(art);
+
+    assertThat(conflictChecker.check(art,
+        Collections.singletonList(art),
+        allArtifacts)).isEmpty();
+  }
+
+  @Test
+  public void testNoConflictWithMethodHandlerInvoke() throws Exception {
+    final Artifact art = newArtifact("root", load(findClass(ClassWithMethodHandleInvoke.class)));
+
+    List<Artifact> allArtifacts = new ArrayList<>(ClassLoadingUtil.bootstrapArtifacts());
+    allArtifacts.add(art);
+
+    assertThat(conflictChecker.check(art,
+        Collections.singletonList(art),
+        allArtifacts)).isEmpty();
+  }
+
   @Test
   public void shouldReportMissingParent() throws Exception {
     class LostParent {
@@ -491,4 +529,10 @@ private static Dependency dependency(ClassTypeDescriptor className,
         .fieldType(field.descriptor())
         .build();
   }
+
+  static class ClassWithMethodHandleInvoke {
+    static void func(MethodHandle handle) throws Throwable {
+      handle.invoke();
+    }
+  }
 }