Fix #41: abstract could become concrete.

2 files changed, 31 insertions, 17 deletions

diff --git a/api/src/main/java/org/osjava/jardiff/Tools.java b/api/src/main/java/org/osjava/jardiff/Tools.java
index 0a8f5f1..e159541 100644
--- a/api/src/main/java/org/osjava/jardiff/Tools.java
+++ b/api/src/main/java/org/osjava/jardiff/Tools.java
@@ -66,12 +66,14 @@ public final class Tools
      * @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;
-    }
+		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;
+		} else {
+			oldAccess = oldAccess & ~Opcodes.ACC_FINAL & ~Opcodes.ACC_ABSTRACT;
+			newAccess = newAccess & ~Opcodes.ACC_FINAL & ~Opcodes.ACC_ABSTRACT;
+		}
+		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 0f904ac..148543a 100644
--- a/api/src/test/java/org/osjava/jardiff/ToolsTest.java
+++ b/api/src/test/java/org/osjava/jardiff/ToolsTest.java
@@ -22,14 +22,26 @@ 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));
-    }
+	@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));
+		// ... but can become non-final.
+		assertFalse(Tools.isAccessChange(Opcodes.ACC_FINAL, 0));
+		assertFalse(Tools.isAccessChange(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));
+		// ...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));
+	}
 
 }