From b7f165affed6259a40dc7394a19b9e29db501108 Mon Sep 17 00:00:00 2001 From: Timo Westkämper Date: Wed, 30 Jul 2014 22:28:05 +0300 Subject: Improve access comparison --- .../java/org/osjava/jardiff/PublicDiffCriteria.java | 6 +++--- .../java/org/osjava/jardiff/SimpleDiffCriteria.java | 6 +++--- api/src/main/java/org/osjava/jardiff/Tools.java | 19 +++++++++++++++++++ 3 files changed, 25 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; + } } -- cgit v1.2.3 From cd5d3fab37fa4b1b240875ad13bac404a6312bd8 Mon Sep 17 00:00:00 2001 From: Timo Westkämper Date: Wed, 30 Jul 2014 22:29:18 +0300 Subject: Add test --- api/src/test/java/org/osjava/jardiff/ToolsTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 api/src/test/java/org/osjava/jardiff/ToolsTest.java 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)); + } + +} -- cgit v1.2.3