Merge pull request #37 from timowest/master

Improve access comparison

4 files changed, 44 insertions, 6 deletions

diff --git a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
index 102d9f6..9940255 100644
--- a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
@@ -71,7 +71,7 @@ public class PublicDiffCriteria implements DiffCriteria
      * @return True if the classes differ, false otherwise.
      */
     public boolean differs(ClassInfo oldInfo, ClassInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         // Yes classes can have a null supername, e.g. java.lang.Object !
         if(oldInfo.getSupername() == null) {
@@ -100,7 +100,7 @@ public class PublicDiffCriteria implements DiffCriteria
      * @return True if the methods differ, false otherwise.
      */
     public boolean differs(MethodInfo oldInfo, MethodInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getExceptions() == null
             || newInfo.getExceptions() == null) {
@@ -127,7 +127,7 @@ public class PublicDiffCriteria implements DiffCriteria
      * @return True if the fields differ, false otherwise.
      */
     public boolean differs(FieldInfo oldInfo, FieldInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getValue() == null || newInfo.getValue() == null) {
             if (oldInfo.getValue() != newInfo.getValue())
diff --git a/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java b/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
index 3d9c7ca..5b82266 100644
--- a/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
@@ -71,7 +71,7 @@ public class SimpleDiffCriteria implements DiffCriteria
      * @return True if the classes differ, false otherwise.
      */
     public boolean differs(ClassInfo oldInfo, ClassInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         // Yes classes can have a null supername, e.g. java.lang.Object !
         if(oldInfo.getSupername() == null) {
@@ -100,7 +100,7 @@ public class SimpleDiffCriteria implements DiffCriteria
      * @return True if the methods differ, false otherwise.
      */
     public boolean differs(MethodInfo oldInfo, MethodInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getExceptions() == null
             || newInfo.getExceptions() == null) {
@@ -127,7 +127,7 @@ public class SimpleDiffCriteria implements DiffCriteria
      * @return True if the fields differ, false otherwise.
      */
     public boolean differs(FieldInfo oldInfo, FieldInfo newInfo) {
-        if (oldInfo.getAccess() != newInfo.getAccess())
+        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getValue() == null || newInfo.getValue() == null) {
             if (oldInfo.getValue() != newInfo.getValue())
diff --git a/api/src/main/java/org/osjava/jardiff/Tools.java b/api/src/main/java/org/osjava/jardiff/Tools.java
index 4a32ef7..0a8f5f1 100644
--- a/api/src/main/java/org/osjava/jardiff/Tools.java
+++ b/api/src/main/java/org/osjava/jardiff/Tools.java
@@ -16,6 +16,8 @@
  */
 package org.osjava.jardiff;
 
+import org.objectweb.asm.Opcodes;
+
 /**
  * A set of Tools which do not belong anywhere else in the API at this time.
  * This is nasty, but for now, useful.
@@ -55,4 +57,21 @@ public final class Tools
         }
         return ret.toString();
     }
+
+    /**
+     * Returns whether newAccess is incompatible with oldAccess
+     *
+     * @param oldAccess
+     * @param newAccess
+     * @return
+     */
+    public static boolean isAccessChange(int oldAccess, int newAccess) {
+        if ((oldAccess & Opcodes.ACC_FINAL) == 0 && (newAccess & Opcodes.ACC_FINAL) > 0) {
+            return true;
+        } else {
+            oldAccess = oldAccess & ~Opcodes.ACC_FINAL;
+            newAccess = newAccess & ~Opcodes.ACC_FINAL;
+        }
+        return oldAccess != newAccess;
+    }
 }
diff --git a/api/src/test/java/org/osjava/jardiff/ToolsTest.java b/api/src/test/java/org/osjava/jardiff/ToolsTest.java
new file mode 100644
index 0000000..8066264
--- /dev/null
+++ b/api/src/test/java/org/osjava/jardiff/ToolsTest.java
@@ -0,0 +1,19 @@
+package org.osjava.jardiff;
+
+import org.junit.Test;
+import org.objectweb.asm.Opcodes;
+import static org.junit.Assert.*;
+
+public class ToolsTest {
+
+    @Test
+    public void isAccessChange() {
+        assertTrue(Tools.isAccessChange(0, Opcodes.ACC_FINAL));
+        assertTrue(Tools.isAccessChange(0, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
+        assertTrue(Tools.isAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, 0));
+        assertTrue(Tools.isAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PROTECTED));
+        assertFalse(Tools.isAccessChange(Opcodes.ACC_FINAL, 0));
+        assertFalse(Tools.isAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+    }
+
+}