Check order of annotation encoded arrays

Except for system annotations

Change-Id: I726da6a588fd98ea9755959ab3ccc1be8621ea31

12 files changed, 395 insertions, 55 deletions

diff --git a/dexcomparator/src/com/android/jack/DexAnnotationsComparator.java b/dexcomparator/src/com/android/jack/DexAnnotationsComparator.java
index 529db1f..088c0d9 100644
--- a/dexcomparator/src/com/android/jack/DexAnnotationsComparator.java
+++ b/dexcomparator/src/com/android/jack/DexAnnotationsComparator.java
@@ -69,6 +69,8 @@ public class DexAnnotationsComparator {
   @Nonnull
   private static final String MEMBER_CLASSES_DESCRIPTOR = "Ldalvik/annotation/MemberClasses;";
   @Nonnull
+  private static final String THROWS_DESCRIPTOR = "Ldalvik/annotation/Throws;";
+  @Nonnull
   private final Logger logger;
   @Nonnull
   private final DexFile referenceDexFile;
@@ -455,40 +457,24 @@ public class DexAnnotationsComparator {
         } else if (encodedValue instanceof ArrayEncodedSubValue) {
           ArrayEncodedSubValue arrayEncodedValue = (ArrayEncodedSubValue) encodedValue;
           ArrayEncodedSubValue candArrayEncodedValue = (ArrayEncodedSubValue) candEncodedValue;
-          isEqual = true;
-          boolean isMemberClass = type.equals(MEMBER_CLASSES_DESCRIPTOR);
 
           List<EncodedValue> refEncodedValues = Arrays.asList(arrayEncodedValue.values);
           List<EncodedValue> candEncodedValues = Arrays.asList(candArrayEncodedValue.values);
-          Collections.sort(refEncodedValues);
-          Collections.sort(candEncodedValues);
-          Iterator<EncodedValue> refEncodedValuesIterator = refEncodedValues.iterator();
-          Iterator<EncodedValue> candEncodedValuesIterator = candEncodedValues.iterator();
-
-          while (refEncodedValuesIterator.hasNext()) {
-            if (!candEncodedValuesIterator.hasNext()) {
-              isEqual = false;
-              break;
-            }
-            EncodedValue refValue = refEncodedValuesIterator.next();
 
-            // our reference may have synthetic anonymous classes that we should ignore
-            if (isMemberClass) {
-              TypeEncodedValue typeRefValue = (TypeEncodedValue) refValue;
-              if (isAnonymousTypeName(typeRefValue.value.getTypeDescriptor())) {
-                continue;
-              }
-            }
+          // With system annotations we should not take the order of the values in the encoded array
+          // into account because it is synthetic anyway. We only need to avoid "MemberClasses" and
+          // "Throws" because they're the only ones that contain encoded arrays.
+          boolean doNotCompareWithOrder =
+              type.equals(MEMBER_CLASSES_DESCRIPTOR) || type.equals(THROWS_DESCRIPTOR);
 
-            EncodedValue candValue = candEncodedValuesIterator.next();
-            isEqual = compareValues(refValue, candValue, elementString, type, name);
-            if (!isEqual) {
-              break;
-            }
-          }
-          if (candEncodedValuesIterator.hasNext()) {
-            isEqual = false;
+          if (doNotCompareWithOrder) {
+            isEqual = compareUnorderedEncodedValues(refEncodedValues, candEncodedValues,
+                elementString, type, name);
+          } else {
+            isEqual = compareOrderedEncodedValues(refEncodedValues, candEncodedValues,
+                elementString, type, name);
           }
+
         } else {
           isEqual = encodedValue.compareTo(candEncodedValue) == 0;
         }
@@ -517,6 +503,50 @@ public class DexAnnotationsComparator {
       return isEqual;
     }
 
+    private boolean compareUnorderedEncodedValues(List<EncodedValue> refEncodedValues,
+        List<EncodedValue> candEncodedValues, String elementString, String type, String name)
+        throws DifferenceFoundException {
+      Collections.sort(refEncodedValues);
+      Collections.sort(candEncodedValues);
+      return compareOrderedEncodedValues(refEncodedValues, candEncodedValues, elementString, type,
+          name);
+    }
+
+    private boolean compareOrderedEncodedValues(List<EncodedValue> refEncodedValues,
+        List<EncodedValue> candEncodedValues, String elementString, String type, String name)
+        throws DifferenceFoundException {
+      boolean isEqual = true;
+      boolean isMemberClass = type.equals(MEMBER_CLASSES_DESCRIPTOR);
+      Iterator<EncodedValue> refEncodedValuesIterator = refEncodedValues.iterator();
+      Iterator<EncodedValue> candEncodedValuesIterator = candEncodedValues.iterator();
+
+      while (refEncodedValuesIterator.hasNext()) {
+        if (!candEncodedValuesIterator.hasNext()) {
+          isEqual = false;
+          break;
+        }
+        EncodedValue refValue = refEncodedValuesIterator.next();
+
+        // our reference may have additional synthetic anonymous classes that we should ignore
+        if (isMemberClass) {
+          TypeEncodedValue typeRefValue = (TypeEncodedValue) refValue;
+          if (isAnonymousTypeName(typeRefValue.value.getTypeDescriptor())) {
+            continue;
+          }
+        }
+
+        EncodedValue candValue = candEncodedValuesIterator.next();
+        isEqual = compareValues(refValue, candValue, elementString, type, name);
+        if (!isEqual) {
+          break;
+        }
+      }
+      if (candEncodedValuesIterator.hasNext()) {
+        isEqual = false;
+      }
+      return isEqual;
+    }
+
     private boolean compareMethodIds(MethodIdItem value, MethodIdItem value2) {
       boolean isEqual = compareTypeIds(value.getContainingClass(), value2.getContainingClass());
       isEqual &= compareProtoIds(value.getPrototype(), value2.getPrototype());
diff --git a/dexcomparator/testing/com/android/jack/dexcomparator/test/AnnotationComparisonTest.java b/dexcomparator/testing/com/android/jack/dexcomparator/test/AnnotationComparisonTest.java
new file mode 100644
index 0000000..131220a
--- /dev/null
+++ b/dexcomparator/testing/com/android/jack/dexcomparator/test/AnnotationComparisonTest.java
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+import com.android.jack.DexAnnotationsComparator;
+import com.android.jack.DifferenceFoundException;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.annotation.Nonnull;
+
+public class AnnotationComparisonTest {
+
+  @Nonnull
+  private static final File testSource1 = new File("testsource1");
+  @Nonnull
+  private static final File testSource2 = new File("testsource2");
+
+  @Test
+  public void testOutOfOrderMemberClassesAnnotations() throws IOException,
+      DifferenceFoundException {
+    String sourcePath = "com/android/jack/dexcomparator/test/Outer.java";
+    File a1 = new File(testSource1, sourcePath);
+    File a2 = new File(testSource2, sourcePath);
+    File dex1 = File.createTempFile("dex1", ".dex");
+    dex1.deleteOnExit();
+    TestTools.compileToDexWithJack(a1, dex1);
+    File dex2 = File.createTempFile("dex2", ".dex");
+    dex2.deleteOnExit();
+    TestTools.compileToDexWithJack(a2, dex2);
+    new DexAnnotationsComparator(dex1, dex2).compare();
+  }
+
+  @Test
+  public void testOutOfOrderThrowsAnnotations() throws IOException,
+      DifferenceFoundException {
+    String sourcePath = "com/android/jack/dexcomparator/test/Throws.java";
+    File a1 = new File(testSource1, sourcePath);
+    File a2 = new File(testSource2, sourcePath);
+    File dex1 = File.createTempFile("dex1", ".dex");
+    dex1.deleteOnExit();
+    TestTools.compileToDexWithJack(a1, dex1);
+    File dex2 = File.createTempFile("dex2", ".dex");
+    dex2.deleteOnExit();
+    TestTools.compileToDexWithJack(a2, dex2);
+    new DexAnnotationsComparator(dex1, dex2).compare();
+  }
+
+  @Test
+  public void testOutOfOrderCustomAnnotations() throws IOException {
+    String sourcePath1 = "com/android/jack/dexcomparator/test/MyAnnotation.java";
+    String sourcePath2 = "com/android/jack/dexcomparator/test/Annotated.java";
+    File dex1 = File.createTempFile("dex1", ".dex");
+    dex1.deleteOnExit();
+    List<File> sourceList1 = new ArrayList<File>(2);
+    sourceList1.add(new File(testSource1, sourcePath1));
+    sourceList1.add(new File(testSource1, sourcePath2));
+    TestTools.compileToDexWithJack(sourceList1, dex1);
+    File dex2 = File.createTempFile("dex2", ".dex");
+    dex2.deleteOnExit();
+    List<File> sourceList2 = new ArrayList<File>(2);
+    sourceList2.add(new File(testSource2, sourcePath1));
+    sourceList2.add(new File(testSource2, sourcePath2));
+    TestTools.compileToDexWithJack(sourceList2, dex2);
+    try {
+      new DexAnnotationsComparator(dex1, dex2).compare();
+      Assert.fail();
+    } catch (DifferenceFoundException e) {
+    }
+  }
+
+  @Test
+  public void testSameOrderCustomAnnotations() throws IOException,
+      DifferenceFoundException {
+    String sourcePath1 = "com/android/jack/dexcomparator/test/MyAnnotation.java";
+    String sourcePath2 = "com/android/jack/dexcomparator/test/Annotated.java";
+    File dex1 = File.createTempFile("dex1", ".dex");
+    dex1.deleteOnExit();
+    List<File> sourceList1 = new ArrayList<File>(2);
+    sourceList1.add(new File(testSource1, sourcePath1));
+    sourceList1.add(new File(testSource1, sourcePath2));
+    TestTools.compileToDexWithJack(sourceList1, dex1);
+    new DexAnnotationsComparator(dex1, dex1).compare();
+  }
+
+}
diff --git a/dexcomparator/testing/com/android/jack/dexcomparator/test/BinaryCodeComparisonTest.java b/dexcomparator/testing/com/android/jack/dexcomparator/test/BinaryCodeComparisonTest.java
index 863d291..3441168 100644
--- a/dexcomparator/testing/com/android/jack/dexcomparator/test/BinaryCodeComparisonTest.java
+++ b/dexcomparator/testing/com/android/jack/dexcomparator/test/BinaryCodeComparisonTest.java
@@ -18,10 +18,8 @@ package com.android.jack.dexcomparator.test;
 
 import com.android.jack.DexComparator;
 import com.android.jack.DifferenceFoundException;
-import com.android.jack.util.ExecuteFile;
 
 import org.junit.Assert;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.io.File;
@@ -36,15 +34,6 @@ public class BinaryCodeComparisonTest {
   private static final File testSource1 = new File("testsource1");
   @Nonnull
   private static final File testSource2 = new File("testsource2");
-  @Nonnull
-  private static final File jackJar = new File("../jack/dist/jack.jar");
-  @Nonnull
-  private static final File coreStubsMini = new File("../jack/libs/core-stubs-mini.jar");
-
-  @BeforeClass
-  public static void setUpClass() {
-    BinaryCodeComparisonTest.class.getClassLoader().setDefaultAssertionStatus(true);
-  }
 
   @Test
   public void testDifferentBinaryCodeComparison() throws IOException {
@@ -53,10 +42,10 @@ public class BinaryCodeComparisonTest {
     File a2 = new File(testSource2, sourcePath);
     File dex1 = File.createTempFile("dex1", ".dex");
     dex1.deleteOnExit();
-    compileToDexWithJack(a1, dex1);
+    TestTools.compileToDexWithJack(a1, dex1);
     File dex2 = File.createTempFile("dex2", ".dex");
     dex2.deleteOnExit();
-    compileToDexWithJack(a2, dex2);
+    TestTools.compileToDexWithJack(a2, dex2);
     try {
       new DexComparator().compare(dex1, dex2, false /* compareDebugInfo */, true /* strict */,
           false /* compareDebugInfoBinarily */, true /* compareCodeBinarily */);
@@ -77,7 +66,7 @@ public class BinaryCodeComparisonTest {
     File a1 = new File(testSource1, sourcePath);
     File dex1 = File.createTempFile("dex1", ".dex");
     dex1.deleteOnExit();
-    compileToDexWithJack(a1, dex1);
+    TestTools.compileToDexWithJack(a1, dex1);
     try {
       new DexComparator().compare(dex1, dex1, false /* compareDebugInfo */, true /* strict */,
           false /* compareDebugInfoBinarily */, true /* compareCodeBinarily */);
@@ -85,17 +74,4 @@ public class BinaryCodeComparisonTest {
       Assert.fail(e.getMessage());
     }
   }
-
-  private void compileToDexWithJack(File source, File dex) {
-    String[] args = new String[]{"java", "-jar", jackJar.getAbsolutePath(),
-        "-cp", coreStubsMini.getAbsolutePath(),
-        "-o", dex.getAbsolutePath(), "--ecj", source.getAbsolutePath()};
-
-    ExecuteFile execFile = new ExecuteFile(args);
-    if (!execFile.run()) {
-      throw new RuntimeException("Jack exited with an error");
-    }
-
-  }
-
 }
diff --git a/dexcomparator/testing/com/android/jack/dexcomparator/test/TestTools.java b/dexcomparator/testing/com/android/jack/dexcomparator/test/TestTools.java
new file mode 100644
index 0000000..c7aff74
--- /dev/null
+++ b/dexcomparator/testing/com/android/jack/dexcomparator/test/TestTools.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+import com.android.jack.util.ExecuteFile;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import javax.annotation.Nonnull;
+
+public class TestTools {
+
+  @Nonnull
+  private static final File jackJar = new File("../jack/dist/jack.jar");
+  @Nonnull
+  private static final File coreStubsMini = new File("../jack/libs/core-stubs-mini.jar");
+
+  public static void compileToDexWithJack(@Nonnull List<File> sources, @Nonnull File dex) {
+    int size = 8 + sources.size();
+    List<String> argList = new ArrayList<String>(size);
+    argList.add("java");
+    argList.add("-jar");
+    argList.add(jackJar.getAbsolutePath());
+    argList.add("-cp");
+    argList.add(coreStubsMini.getAbsolutePath());
+    argList.add("-o");
+    argList.add(dex.getAbsolutePath());
+    argList.add("--ecj");
+    for (File source : sources) {
+      argList.add(source.getAbsolutePath());
+    }
+
+    ExecuteFile execFile = new ExecuteFile(argList.toArray(new String[size]));
+    if (!execFile.run()) {
+      throw new RuntimeException("Jack exited with an error");
+    }
+  }
+
+  public static void compileToDexWithJack(@Nonnull File source, @Nonnull File dex) {
+    compileToDexWithJack(Collections.singletonList(source), dex);
+  }
+
+}
diff --git a/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Annotated.java b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Annotated.java
new file mode 100644
index 0000000..8304c89
--- /dev/null
+++ b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Annotated.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+@MyAnnotation({"a", "b"})
+public class Annotated {
+
+}
diff --git a/dexcomparator/testsource1/com/android/jack/dexcomparator/test/MyAnnotation.java b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/MyAnnotation.java
new file mode 100644
index 0000000..05dbbeb
--- /dev/null
+++ b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/MyAnnotation.java
@@ -0,0 +1,7 @@
+package com.android.jack.dexcomparator.test;
+
+public @interface MyAnnotation {
+
+  String[] value();
+
+}
diff --git a/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Outer.java b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Outer.java
new file mode 100644
index 0000000..2d3dcee
--- /dev/null
+++ b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Outer.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+public class Outer {
+
+  private static class Inner1 {
+
+  }
+
+  private static class Inner2 {
+
+  }
+}
diff --git a/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Throws.java b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Throws.java
new file mode 100644
index 0000000..4273950
--- /dev/null
+++ b/dexcomparator/testsource1/com/android/jack/dexcomparator/test/Throws.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+import java.io.FileNotFoundException;
+
+public class Throws {
+
+  private int m(int b) throws IllegalArgumentException, FileNotFoundException {
+    return b;
+  }
+
+}
diff --git a/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Annotated.java b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Annotated.java
new file mode 100644
index 0000000..58efcd2
--- /dev/null
+++ b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Annotated.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+@MyAnnotation({"b", "a"})
+public class Annotated {
+
+}
diff --git a/dexcomparator/testsource2/com/android/jack/dexcomparator/test/MyAnnotation.java b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/MyAnnotation.java
new file mode 100644
index 0000000..05dbbeb
--- /dev/null
+++ b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/MyAnnotation.java
@@ -0,0 +1,7 @@
+package com.android.jack.dexcomparator.test;
+
+public @interface MyAnnotation {
+
+  String[] value();
+
+}
diff --git a/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Outer.java b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Outer.java
new file mode 100644
index 0000000..eb1125a
--- /dev/null
+++ b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Outer.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+public class Outer {
+
+  private static class Inner2 {
+
+  }
+
+  private static class Inner1 {
+
+  }
+}
diff --git a/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Throws.java b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Throws.java
new file mode 100644
index 0000000..0d9dbaf
--- /dev/null
+++ b/dexcomparator/testsource2/com/android/jack/dexcomparator/test/Throws.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.jack.dexcomparator.test;
+
+import java.io.FileNotFoundException;
+
+public class Throws {
+
+  private int m(int b) throws FileNotFoundException, IllegalArgumentException {
+    return b;
+  }
+
+}