Merge pull request #43 from sgothel/jogamp

Fix jardiff's Tools.isAccessChange(..): Differentiate between Class, Field and method

4 files changed, 291 insertions, 52 deletions

diff --git a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
index 9940255..90fb382 100644
--- a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
@@ -38,7 +38,7 @@ public class PublicDiffCriteria implements DiffCriteria
     public boolean validClass(ClassInfo info) {
         return !info.isSynthetic() && info.isPublic();
     }
-    
+
     /**
      * Check if a method is valid.
      * If the method is not synthetic and public, return true.
@@ -49,7 +49,7 @@ public class PublicDiffCriteria implements DiffCriteria
     public boolean validMethod(MethodInfo info) {
         return !info.isSynthetic() && info.isPublic();
     }
-    
+
     /**
      * Check if a field is valid.
      * If the method is not synthetic and public, return true.
@@ -60,7 +60,7 @@ public class PublicDiffCriteria implements DiffCriteria
     public boolean validField(FieldInfo info) {
         return !info.isSynthetic() && info.isPublic();
     }
-    
+
     /**
      * Check if there is a change between two versions of a class.
      * Returns true if the access flags differ, or if the superclass differs
@@ -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 (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isClassAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         // Yes classes can have a null supername, e.g. java.lang.Object !
         if(oldInfo.getSupername() == null) {
@@ -81,15 +81,15 @@ public class PublicDiffCriteria implements DiffCriteria
         } else if (!oldInfo.getSupername().equals(newInfo.getSupername())) {
             return true;
         }
-        Set<String> oldInterfaces
+        final Set<String> oldInterfaces
             = new HashSet(Arrays.asList(oldInfo.getInterfaces()));
-        Set<String> newInterfaces
+        final Set<String> newInterfaces
             = new HashSet(Arrays.asList(newInfo.getInterfaces()));
         if (!oldInterfaces.equals(newInterfaces))
             return true;
         return false;
     }
-    
+
     /**
      * Check if there is a change between two versions of a method.
      * Returns true if the access flags differ, or if the thrown
@@ -100,23 +100,23 @@ public class PublicDiffCriteria implements DiffCriteria
      * @return True if the methods differ, false otherwise.
      */
     public boolean differs(MethodInfo oldInfo, MethodInfo newInfo) {
-        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getExceptions() == null
             || newInfo.getExceptions() == null) {
             if (oldInfo.getExceptions() != newInfo.getExceptions())
                 return true;
         } else {
-            Set<String> oldExceptions
+            final Set<String> oldExceptions
                 = new HashSet(Arrays.asList(oldInfo.getExceptions()));
-            Set<String> newExceptions
+            final Set<String> newExceptions
                 = new HashSet(Arrays.asList(newInfo.getExceptions()));
             if (!oldExceptions.equals(newExceptions))
                 return true;
         }
         return false;
     }
-    
+
     /**
      * Check if there is a change between two versions of a field.
      * Returns true if the access flags differ, or if the inital value
@@ -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 (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isFieldAccessChange(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 5b82266..ceaf0d2 100644
--- a/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
@@ -38,7 +38,7 @@ public class SimpleDiffCriteria implements DiffCriteria
     public boolean validClass(ClassInfo info) {
         return !info.isSynthetic() && (info.isPublic() || info.isProtected());
     }
-    
+
     /**
      * Check if a method is valid.
      * If the method is not synthetic and is public or protected, return true.
@@ -49,7 +49,7 @@ public class SimpleDiffCriteria implements DiffCriteria
     public boolean validMethod(MethodInfo info) {
         return !info.isSynthetic() && (info.isPublic() || info.isProtected());
     }
-    
+
     /**
      * Check if a field is valid.
      * If the method is not synthetic and is public or protected, return true.
@@ -60,7 +60,7 @@ public class SimpleDiffCriteria implements DiffCriteria
     public boolean validField(FieldInfo info) {
         return !info.isSynthetic() && (info.isPublic() || info.isProtected());
     }
-    
+
     /**
      * Check if there is a change between two versions of a class.
      * Returns true if the access flags differ, or if the superclass differs
@@ -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 (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isClassAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         // Yes classes can have a null supername, e.g. java.lang.Object !
         if(oldInfo.getSupername() == null) {
@@ -81,15 +81,15 @@ public class SimpleDiffCriteria implements DiffCriteria
         } else if (!oldInfo.getSupername().equals(newInfo.getSupername())) {
             return true;
         }
-        Set<String> oldInterfaces
+        final Set<String> oldInterfaces
             = new HashSet(Arrays.asList(oldInfo.getInterfaces()));
-        Set<String> newInterfaces
+        final Set<String> newInterfaces
             = new HashSet(Arrays.asList(newInfo.getInterfaces()));
         if (!oldInterfaces.equals(newInterfaces))
             return true;
         return false;
     }
-    
+
     /**
      * Check if there is a change between two versions of a method.
      * Returns true if the access flags differ, or if the thrown
@@ -100,23 +100,23 @@ public class SimpleDiffCriteria implements DiffCriteria
      * @return True if the methods differ, false otherwise.
      */
     public boolean differs(MethodInfo oldInfo, MethodInfo newInfo) {
-        if (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
             return true;
         if (oldInfo.getExceptions() == null
             || newInfo.getExceptions() == null) {
             if (oldInfo.getExceptions() != newInfo.getExceptions())
                 return true;
         } else {
-            Set<String> oldExceptions
+            final Set<String> oldExceptions
                 = new HashSet(Arrays.asList(oldInfo.getExceptions()));
-            Set<String> newExceptions
+            final Set<String> newExceptions
                 = new HashSet(Arrays.asList(newInfo.getExceptions()));
             if (!oldExceptions.equals(newExceptions))
                 return true;
         }
         return false;
     }
-    
+
     /**
      * Check if there is a change between two versions of a field.
      * Returns true if the access flags differ, or if the inital value
@@ -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 (Tools.isAccessChange(oldInfo.getAccess(), newInfo.getAccess()))
+        if (Tools.isFieldAccessChange(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 e159541..7ac6a42 100644
--- a/api/src/main/java/org/osjava/jardiff/Tools.java
+++ b/api/src/main/java/org/osjava/jardiff/Tools.java
@@ -32,20 +32,20 @@ public final class Tools
     private Tools() {
         /* empty */
     }
-    
+
     /**
      * Get the java class name given an internal class name.
      * This method currently replaces all instances of $ and / with . this
      * may not be according to the java language spec, and will almost
      * certainly fail for some inner classes.
-     * 
+     *
      * @param internalName The internal name of the class.
      * @return The java class name.
      */
     public static final String getClassName(String internalName) {
-        StringBuffer ret = new StringBuffer(internalName.length());
+        final StringBuffer ret = new StringBuffer(internalName.length());
         for (int i = 0; i < internalName.length(); i++) {
-            char ch = internalName.charAt(i);
+            final char ch = internalName.charAt(i);
             switch (ch) {
             case '$':
             case '/':
@@ -58,22 +58,171 @@ public final class Tools
         return ret.toString();
     }
 
+    private static boolean has(final int value, final int mask) {
+        return (value & mask) != 0;
+    }
+    private static boolean not(final int value, final int mask) {
+        return (value & mask) == 0;
+    }
+
     /**
-     * Returns whether newAccess is incompatible with oldAccess
+     * @deprecated Use {@link #isClassAccessChange(int, int)}.
+     */
+    public static boolean isAccessChange(int oldAccess, int newAccess) {
+        return isClassAccessChange(oldAccess, newAccess);
+    }
+
+    /**
+     * Returns whether a class's newAccess is incompatible with oldAccess
+     * following <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html">Java Language Specification, Java SE 7 Edition</a>:
+     * <ul>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.1">13.4.1 abstract Classes</a><ul>
+     *     <li>If a class that was not declared abstract is changed to be declared abstract,
+     *         then pre-existing binaries that attempt to create new instances of that class
+     *         will throw either an InstantiationError at link time,
+     *         or (if a reflective method is used) an InstantiationException at run time.
+     *         Such changes <b>break backward compatibility</b>!</li>
+     *     <li>Changing a class that is declared abstract to no longer be declared abstract
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.2">13.4.2 final Classes</a><ul>
+     *     <li>If a class that was not declared final is changed to be declared final,
+     *         then a VerifyError is thrown if a binary of a pre-existing subclass of this class is loaded,
+     *         because final classes can have no subclasses.
+     *         Such changes <b>break functional backward compatibility</b>!</li>
+     *     <li>Changing a class that is declared final to no longer be declared final
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *     </ul></li>
+     * </ul>
      *
      * @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 if ((oldAccess & Opcodes.ACC_ABSTRACT) == 0 && (newAccess & Opcodes.ACC_ABSTRACT) > 0) {
-			return true;
+    public static boolean isClassAccessChange(final int oldAccess, final int newAccess) {
+        if ( not(oldAccess, Opcodes.ACC_ABSTRACT) && has(newAccess, Opcodes.ACC_ABSTRACT) ) {
+            return true; // 13.4.1 #1
+        } else  if ( not(oldAccess, Opcodes.ACC_FINAL) && has(newAccess, Opcodes.ACC_FINAL) ) {
+            return true; // 13.4.2 #1
+        } else {
+            final int compatibleChanges = Opcodes.ACC_ABSTRACT |     // 13.4.1 #2
+                                          Opcodes.ACC_FINAL ;        // 13.4.2 #2
+            // FIXME Opcodes.ACC_VOLATILE ?
+            final int oldAccess2 = oldAccess & ~compatibleChanges;
+            final int newAccess2 = newAccess & ~compatibleChanges;
+            return oldAccess2 != newAccess2;
+        }
+    }
+
+    /**
+     * Returns whether a field's newAccess is incompatible with oldAccess
+     * following <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html">Java Language Specification, Java SE 7 Edition</a>:
+     * <ul>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.9">13.4.9 final Fields and Constants</a><ul>
+     *     <li>If a field that was not declared final is changed to be declared final,
+     *         then it <b>can break compatibility</b> with pre-existing binaries that attempt to assign new values to the field.</li>
+     *     <li>Deleting the keyword final or changing the value to which a <i>non-final</i> field is initialized
+     *         <b>does not break compatibility</b> with existing binaries.</li>
+     *     <li>If a field is a constant variable (§4.12.4),
+     *         then deleting the keyword final or changing its value
+     *         will <i>not break compatibility</i> with pre-existing binaries by causing them not to run,
+     *         but they will not see any new value for the usage of the field unless they are recompiled.
+     *         This is true even if the usage itself is not a compile-time constant expression (§15.28).
+     *         Such changes <b>break functional backward compatibility</b>!</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.10">13.4.10 static Fields</a><ul>
+     *     <li>If a field that is not declared private was not declared static
+     *         and is changed to be declared static, or vice versa,
+     *         then a linkage error, specifically an IncompatibleClassChangeError,
+     *         will result if the field is used by a pre-existing binary which expected a field of the other kind.
+     *         Such changes <b>break backward compatibility</b>!</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.11">13.4.11. transient Fields </a><ul>
+     *     <li>Adding or deleting a transient modifier of a field
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *   </ul></li>
+     *   <li><a href="http://www.wsu.edu/UNIX_Systems/java/langspec-1.0/13.doc.html#45194">13.4.11 volatile Fields (JLS 1.0)</a><ul>
+     *     <li>If a field that is not declared private was not declared volatile
+     *         and is changed to be declared volatile, or vice versa, then a linkage time error,
+     *         specifically an IncompatibleClassChangeError, may result if the field is used
+     *         by a preexisting binary that expected a field of the opposite volatility.
+     *         Such changes <b>break backward compatibility</b>!</li>
+     *   </ul></li>
+     * </ul>
+     *
+     * @param oldAccess
+     * @param newAccess
+     * @return
+     */
+    public static boolean isFieldAccessChange(final int oldAccess, final int newAccess) {
+        if ( not(oldAccess, Opcodes.ACC_FINAL) && has(newAccess, Opcodes.ACC_FINAL) ) {
+            return true; // 13.4.9 #1
+        } else {
+            final int compatibleChanges = Opcodes.ACC_FINAL |         // 13.4.9 #2
+                                          Opcodes.ACC_TRANSIENT;      // 13.4.11 #1
+            final int oldAccess2 = oldAccess & ~compatibleChanges;
+            final int newAccess2 = newAccess & ~compatibleChanges;
+            return oldAccess2 != newAccess2;
+        }
+    }
+
+    /**
+     * Returns whether a method's newAccess is incompatible with oldAccess
+     * following <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html">Java Language Specification, Java SE 7 Edition</a>:
+     * <ul>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.16">13.4.16 abstract Methods</a><ul>
+     *     <li>Changing a method that is declared abstract to no longer be declared abstract
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *     <li>Changing a method that is not declared abstract to be declared abstract
+     *         <b>will break compatibility</b> with pre-existing binaries that previously invoked the method, causing an AbstractMethodError.</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.17">13.4.17 final</a><ul>
+     *     <li>Changing a method that is declared final to no longer be declared final
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *     <li>Changing an instance method that is not declared final to be declared final
+     *         <b>may break compatibility</b> with existing binaries that depend on the ability to override the method.</li>
+     *     <li>Changing a class (static) method that is not declared final to be declared final
+     *         <b>does not break compatibility</b> with existing binaries, because the method could not have been overridden.</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.18">13.4.18 native Methods</a><ul>
+     *     <li>Adding or deleting a native modifier of a method
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.19">13.4.19 static Methods</a><ul>
+     *     <li>If a method that is not declared private is also declared static (that is, a class method)
+     *         and is changed to not be declared static (that is, to an instance method), or vice versa,
+     *         then <i>compatibility with pre-existing binaries may be broken</i>, resulting in a linkage time error,
+     *         namely an IncompatibleClassChangeError, if these methods are used by the pre-existing binaries.
+     *         Such changes <b>break functional backward compatibility</b>!</li>
+     *     </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.20">13.4.20 synchronized Methods</a><ul>
+     *     <li>Adding or deleting a synchronized modifier of a method
+     *         <b>does not break compatibility</b> with pre-existing binaries.</li>
+     *   </ul></li>
+     *   <li><a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.21">13.4.21 Method and Constructor Throws</a><ul>
+     *     <li>Changes to the throws clause of methods or constructors
+     *         <b>do not break compatibility</b> with pre-existing binaries; these clauses are checked only at compile time.</li>
+     *   </ul></li>
+     * </ul>
+     *
+     * @param oldAccess
+     * @param newAccess
+     * @return
+     */
+    public static boolean isMethodAccessChange(final int oldAccess, final int newAccess) {
+        if ( not(oldAccess, Opcodes.ACC_ABSTRACT) && has(newAccess, Opcodes.ACC_ABSTRACT) ) {
+            return true; // 13.4.16 #2
+        } else  if ( not(oldAccess, Opcodes.ACC_FINAL) && not(oldAccess, Opcodes.ACC_STATIC) &&
+                     has(newAccess, Opcodes.ACC_FINAL) ) {
+			return true; // 13.4.17 #2 excluding and #3
 		} else {
-			oldAccess = oldAccess & ~Opcodes.ACC_FINAL & ~Opcodes.ACC_ABSTRACT;
-			newAccess = newAccess & ~Opcodes.ACC_FINAL & ~Opcodes.ACC_ABSTRACT;
+		    final int compatibleChanges = Opcodes.ACC_ABSTRACT |     // 13.4.16 #1
+		                                  Opcodes.ACC_FINAL |        // 13.4.17 #1
+		                                  Opcodes.ACC_NATIVE |       // 13.4.18 #1
+		                                  Opcodes.ACC_SYNCHRONIZED;  // 13.4.20 #1
+			final int oldAccess2 = oldAccess & ~compatibleChanges;
+			final int newAccess2 = newAccess & ~compatibleChanges;
+			return oldAccess2 != newAccess2;
 		}
-		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
index 148543a..39683bd 100644
--- a/api/src/test/java/org/osjava/jardiff/ToolsTest.java
+++ b/api/src/test/java/org/osjava/jardiff/ToolsTest.java
@@ -22,26 +22,116 @@ import static org.junit.Assert.*;
 
 public class ToolsTest {
 
+    static class lala {
+        int lala;
+    }
+
 	@Test
-	public void isAccessChange() {
-		// A class or method or field can't become final.
-		assertTrue(Tools.isAccessChange(0, Opcodes.ACC_FINAL));
-		assertTrue(Tools.isAccessChange(0, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
+	public void isClassAccessChange() {
+		// A class can't become final.
+		assertTrue(Tools.isClassAccessChange(0, Opcodes.ACC_FINAL));
+		assertTrue(Tools.isClassAccessChange(0, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
 		// ... but can become non-final.
-		assertFalse(Tools.isAccessChange(Opcodes.ACC_FINAL, 0));
-		assertFalse(Tools.isAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+		assertFalse(Tools.isClassAccessChange(Opcodes.ACC_FINAL, 0));
+		assertFalse(Tools.isClassAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+
 		// No matter the final access, can't become protected or private or
 		// package if it was public.
-		assertTrue(Tools.isAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, 0));
-		assertTrue(Tools.isAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PROTECTED));
-		// A class or method can become concrete.
-		assertFalse(Tools.isAccessChange(Opcodes.ACC_ABSTRACT, 0));
-		assertFalse(Tools.isAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
-		assertFalse(Tools.isAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED));
+		assertTrue(Tools.isClassAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, 0));
+		assertTrue(Tools.isClassAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PROTECTED));
+		// A class can become concrete.
+		assertFalse(Tools.isClassAccessChange(Opcodes.ACC_ABSTRACT, 0));
+		assertFalse(Tools.isClassAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+		assertFalse(Tools.isClassAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED));
 		// ...but can't become abstract
-		assertTrue(Tools.isAccessChange(0, Opcodes.ACC_ABSTRACT));
-		assertTrue(Tools.isAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC + Opcodes.ACC_ABSTRACT));
-		assertTrue(Tools.isAccessChange(Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED + Opcodes.ACC_ABSTRACT));
+		assertTrue(Tools.isClassAccessChange(0, Opcodes.ACC_ABSTRACT));
+		assertTrue(Tools.isClassAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC + Opcodes.ACC_ABSTRACT));
+		assertTrue(Tools.isClassAccessChange(Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED + Opcodes.ACC_ABSTRACT));
 	}
 
+    @Test
+    public void isFieldAccessChange() {
+        // A field can't become final.
+        assertTrue(Tools.isFieldAccessChange(0, Opcodes.ACC_FINAL));
+        assertTrue(Tools.isFieldAccessChange(0, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
+        // ... but can become non-final.
+        assertFalse(Tools.isFieldAccessChange(Opcodes.ACC_FINAL, 0));
+        assertFalse(Tools.isFieldAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+
+        // No matter the final access, can't become protected or private or
+        // package if it was public.
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, 0));
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PROTECTED));
+
+        // A field can't change static
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_STATIC, 0));
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_STATIC + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+
+        // A field can't change volatile
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_VOLATILE, 0));
+        assertTrue(Tools.isFieldAccessChange(Opcodes.ACC_VOLATILE + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+
+        // A field can change transient
+        assertFalse(Tools.isFieldAccessChange(0,
+                                              Opcodes.ACC_TRANSIENT));
+        assertFalse(Tools.isFieldAccessChange(Opcodes.ACC_PUBLIC,
+                                              Opcodes.ACC_PUBLIC + Opcodes.ACC_TRANSIENT));
+        assertFalse(Tools.isFieldAccessChange(Opcodes.ACC_TRANSIENT,
+                                              0));
+        assertFalse(Tools.isFieldAccessChange(Opcodes.ACC_PUBLIC + Opcodes.ACC_TRANSIENT,
+                                              Opcodes.ACC_PUBLIC));
+    }
+
+    @Test
+    public void isMethodAccessChange() {
+        // A non-static method can't become final.
+        assertTrue(Tools.isMethodAccessChange(0, Opcodes.ACC_FINAL));
+        assertTrue(Tools.isMethodAccessChange(0, Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
+        // ... but can become non-final.
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_FINAL, 0));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+        // ... but a static method can become final!
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_STATIC,
+                                               Opcodes.ACC_STATIC + Opcodes.ACC_FINAL));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_STATIC + Opcodes.ACC_PUBLIC,
+                                               Opcodes.ACC_STATIC + Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL));
+
+        // No matter the final access, can't become protected or private or
+        // package if it was public.
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_FINAL + Opcodes.ACC_PUBLIC, 0));
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PROTECTED));
+        // A class or method can become concrete.
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_ABSTRACT, 0));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_ABSTRACT + Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED));
+        // ...but can't become abstract
+        assertTrue(Tools.isMethodAccessChange(0, Opcodes.ACC_ABSTRACT));
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC + Opcodes.ACC_ABSTRACT));
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_PROTECTED, Opcodes.ACC_PROTECTED + Opcodes.ACC_ABSTRACT));
+
+        // A method can't change static
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_STATIC, 0));
+        assertTrue(Tools.isMethodAccessChange(Opcodes.ACC_STATIC + Opcodes.ACC_PUBLIC, Opcodes.ACC_PUBLIC));
+
+        // A method can change synchronized
+        assertFalse(Tools.isMethodAccessChange(0,
+                                               Opcodes.ACC_SYNCHRONIZED));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC,
+                                               Opcodes.ACC_PUBLIC + Opcodes.ACC_SYNCHRONIZED));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_SYNCHRONIZED,
+                                               0));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC + Opcodes.ACC_SYNCHRONIZED,
+                                               Opcodes.ACC_PUBLIC));
+
+        // A method can change native
+        assertFalse(Tools.isMethodAccessChange(0,
+                                               Opcodes.ACC_NATIVE));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC,
+                                               Opcodes.ACC_PUBLIC + Opcodes.ACC_NATIVE));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_NATIVE,
+                                               0));
+        assertFalse(Tools.isMethodAccessChange(Opcodes.ACC_PUBLIC + Opcodes.ACC_NATIVE,
+                                               Opcodes.ACC_PUBLIC));
+    }
+
 }