Clarifications about access permissions checks

Renamed the Tools method that checks the access permissions /
visibility.
Added comments.

2 files changed, 8 insertions, 3 deletions

diff --git a/api/src/main/java/org/osjava/jardiff/JarDiff.java b/api/src/main/java/org/osjava/jardiff/JarDiff.java
index d391712..24d92ff 100644
--- a/api/src/main/java/org/osjava/jardiff/JarDiff.java
+++ b/api/src/main/java/org/osjava/jardiff/JarDiff.java
@@ -392,7 +392,11 @@ public class JarDiff
                         addedFields.add(entry.getKey());
                 }
 
+                // We add all the old methods that match the criteria
                 changedMethods.addAll(removedMethods);
+                // We keep the intersection of these with all the new methods
+                // to detect as changed a method that no longer match the
+                // criteria (i.e. a method that was public and is now private)
                 changedMethods.retainAll(newMethods.keySet());
                 removedMethods.removeAll(changedMethods);
                 removedMethods.removeAll(extNewMethods.keySet());
diff --git a/api/src/main/java/org/osjava/jardiff/Tools.java b/api/src/main/java/org/osjava/jardiff/Tools.java
index 41ba63c..42ba817 100644
--- a/api/src/main/java/org/osjava/jardiff/Tools.java
+++ b/api/src/main/java/org/osjava/jardiff/Tools.java
@@ -65,7 +65,7 @@ public final class Tools
         return (value & mask) == 0;
     }
 
-    private static boolean isLessAccessPermitted(int oldAccess, int newAccess) {
+    private static boolean isAccessIncompatible(int oldAccess, int newAccess) {
         if (has(newAccess, Opcodes.ACC_PUBLIC)) {
             return false;
         } else if (has(newAccess, Opcodes.ACC_PROTECTED)) {
@@ -73,6 +73,7 @@ public final class Tools
         } else if (has(newAccess, Opcodes.ACC_PRIVATE)) {
             return not(oldAccess, Opcodes.ACC_PRIVATE);
         } else {
+            // new access is package, it is incompatible if old access was public or protected
             return has(oldAccess, Opcodes.ACC_PUBLIC | Opcodes.ACC_PROTECTED);
         }
     }
@@ -175,7 +176,7 @@ public final class Tools
      * @return
      */
     public static boolean isFieldAccessChange(final int oldAccess, final int newAccess) {
-        if (isLessAccessPermitted(oldAccess, newAccess)) {
+        if (isAccessIncompatible(oldAccess, newAccess)) {
             return true; // 13.4.7
         }
         if ( not(oldAccess, Opcodes.ACC_FINAL) && has(newAccess, Opcodes.ACC_FINAL) ) {
@@ -244,7 +245,7 @@ public final class Tools
      * @return
      */
     public static boolean isMethodAccessChange(final int oldAccess, final int newAccess) {
-        if (isLessAccessPermitted(oldAccess, newAccess)) {
+        if (isAccessIncompatible(oldAccess, newAccess)) {
             return true; // 13.4.7
         }
         if ( not(oldAccess, Opcodes.ACC_ABSTRACT) && has(newAccess, Opcodes.ACC_ABSTRACT) ) {