Add CompatibilityType.BACKWARD_COMPATIBLE_BINARY: Allowing changes in throw-clause and field-value

6 files changed, 115 insertions, 120 deletions

diff --git a/api/src/main/java/org/osjava/jardiff/JarDiff.java b/api/src/main/java/org/osjava/jardiff/JarDiff.java
index e7d6429..f32982c 100644
--- a/api/src/main/java/org/osjava/jardiff/JarDiff.java
+++ b/api/src/main/java/org/osjava/jardiff/JarDiff.java
@@ -412,16 +412,18 @@ public class JarDiff
                     final String desc = j.next();
                     final MethodInfo oldInfo = oldMethods.get(desc);
                     final MethodInfo newInfo = newMethods.get(desc);
-                    if (!criteria.differs(oldInfo, newInfo))
+                    if (!criteria.differs(oldInfo, newInfo)) {
                         j.remove();
+                    }
                 }
                 j = changedFields.iterator();
                 while (j.hasNext()) {
                     final String desc = j.next();
                     final FieldInfo oldInfo = oldFields.get(desc);
                     final FieldInfo newInfo = newFields.get(desc);
-                    if (!criteria.differs(oldInfo, newInfo))
+                    if (!criteria.differs(oldInfo, newInfo)) {
                         j.remove();
+                    }
                 }
 
                 final boolean classchanged = criteria.differs(oci, nci);
@@ -465,14 +467,12 @@ public class JarDiff
                         final FieldInfo newFieldInfo = newFields.get(field);
                         // Was only deprecated?
                         if (wasDeprecated(oldFieldInfo, newFieldInfo)
-                            && !criteria.differs(
-                                cloneDeprecated(oldFieldInfo),
-                                newFieldInfo)) {
+                            && !criteria.differs(cloneDeprecated(oldFieldInfo), newFieldInfo)) {
                             handler.fieldDeprecated(oldFieldInfo, newFieldInfo);
-                        } else if( !criteria.differsBinary(oldFieldInfo, newFieldInfo)) {
-                            handler.fieldChangedCompat(oldFieldInfo, newFieldInfo);
-                        } else {
+                        } else if( criteria.differsBinary(oldFieldInfo, newFieldInfo)) {
                             handler.fieldChanged(oldFieldInfo, newFieldInfo);
+                        } else {
+                            handler.fieldChangedCompat(oldFieldInfo, newFieldInfo);
                         }
                     }
                     for (final String method : changedMethods) {
@@ -480,15 +480,12 @@ public class JarDiff
                         final MethodInfo newMethodInfo = newMethods.get(method);
                         // Was only deprecated?
                         if (wasDeprecated(oldMethodInfo, newMethodInfo)
-                            && !criteria.differs(
-                                cloneDeprecated(oldMethodInfo),
-                                newMethodInfo)) {
-                            handler.methodDeprecated(oldMethodInfo,
-                                newMethodInfo);
-                        } else if ( !criteria.differsBinary(oldMethodInfo, newMethodInfo) ) {
-                            handler.methodChangedCompat(oldMethodInfo, newMethodInfo);
-                        } else {
+                            && !criteria.differs(cloneDeprecated(oldMethodInfo), newMethodInfo)) {
+                            handler.methodDeprecated(oldMethodInfo, newMethodInfo);
+                        } else if ( criteria.differsBinary(oldMethodInfo, newMethodInfo) ) {
                             handler.methodChanged(oldMethodInfo, newMethodInfo);
+                        } else {
+                            handler.methodChangedCompat(oldMethodInfo, newMethodInfo);
                         }
                     }
                     handler.endChanged();
@@ -518,15 +515,6 @@ public class JarDiff
     }
 
     /**
-     * Determines if an {@link AbstractInfo} was deprecated. (Shortcut to avoid
-     * creating cloned deprecated infos).
-     */
-    private static boolean throwClauseDiffers(final AbstractInfo oldInfo,
-        final AbstractInfo newInfo) {
-    return !oldInfo.isDeprecated() && newInfo.isDeprecated();
-    }
-
-    /**
      * Clones the class info, but changes access, setting deprecated flag.
      *
      * @param classInfo
diff --git a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
index 6cf25d6..c7fea4b 100644
--- a/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/PublicDiffCriteria.java
@@ -102,53 +102,27 @@ public class PublicDiffCriteria implements DiffCriteria
         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
-     * exceptions differ.
-     *
-     * @param oldInfo Info about the old version of the method.
-     * @param newInfo Info about the new version of the method.
-     * @return True if the methods differ, false otherwise.
-     */
+    @Override
     public boolean differs(final MethodInfo oldInfo, final MethodInfo newInfo) {
-        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        if (Tools.isThrowsClauseChange(oldInfo.getExceptions(), newInfo.getExceptions())) {
-            return true;
-        }
-        return false;
+        return // Tools.isDescChange(oldInfo.getDesc(), newInfo.getDesc()) ||
+               Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess()) ||
+               Tools.isThrowsClauseChange(oldInfo.getExceptions(), newInfo.getExceptions());
     }
+    @Override
     public boolean differsBinary(final MethodInfo oldInfo, final MethodInfo newInfo) {
-        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        return false;
+        return // Tools.isDescChange(oldInfo.getDesc(), newInfo.getDesc()) ||
+               Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess());
     }
 
-    /**
-     * Check if there is a change between two versions of a field.
-     * Returns true if the access flags differ, or if the inital value
-     * of the field differs.
-     *
-     * @param oldInfo Info about the old version of the field.
-     * @param newInfo Info about the new version of the field.
-     * @return True if the fields differ, false otherwise.
-     */
+    @Override
     public boolean differs(final FieldInfo oldInfo, final FieldInfo newInfo) {
-        if (Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        if (Tools.isFieldValueChange(oldInfo.getValue(), newInfo.getValue())) {
-            return true;
-        }
-        return false;
+        return Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess()) ||
+               // Tools.isFieldTypeChange(oldInfo.getValue(), newInfo.getValue())     ||
+               Tools.isFieldValueChange(oldInfo.getValue(), newInfo.getValue());
     }
+    @Override
     public boolean differsBinary(final FieldInfo oldInfo, final FieldInfo newInfo) {
-        if (Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        return false;
+        return Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess()); // &&
+               // Tools.isFieldTypeChange(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 578f637..062baca 100644
--- a/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
+++ b/api/src/main/java/org/osjava/jardiff/SimpleDiffCriteria.java
@@ -102,53 +102,27 @@ public class SimpleDiffCriteria implements DiffCriteria
         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
-     * exceptions differ.
-     *
-     * @param oldInfo Info about the old version of the method.
-     * @param newInfo Info about the new version of the method.
-     * @return True if the methods differ, false otherwise.
-     */
+    @Override
     public boolean differs(final MethodInfo oldInfo, final MethodInfo newInfo) {
-        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        if (Tools.isThrowsClauseChange(oldInfo.getExceptions(), newInfo.getExceptions())) {
-            return true;
-        }
-        return false;
+        return // Tools.isDescChange(oldInfo.getDesc(), newInfo.getDesc()) ||
+               Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess()) ||
+               Tools.isThrowsClauseChange(oldInfo.getExceptions(), newInfo.getExceptions());
     }
+    @Override
     public boolean differsBinary(final MethodInfo oldInfo, final MethodInfo newInfo) {
-        if (Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        return false;
+        return // Tools.isDescChange(oldInfo.getDesc(), newInfo.getDesc()) ||
+               Tools.isMethodAccessChange(oldInfo.getAccess(), newInfo.getAccess());
     }
 
-    /**
-     * Check if there is a change between two versions of a field.
-     * Returns true if the access flags differ, or if the inital value
-     * of the field differs.
-     *
-     * @param oldInfo Info about the old version of the field.
-     * @param newInfo Info about the new version of the field.
-     * @return True if the fields differ, false otherwise.
-     */
+    @Override
     public boolean differs(final FieldInfo oldInfo, final FieldInfo newInfo) {
-        if (Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        if (Tools.isFieldValueChange(oldInfo.getValue(), newInfo.getValue())) {
-            return true;
-        }
-        return false;
+        return Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess()) ||
+               // Tools.isFieldTypeChange(oldInfo.getValue(), newInfo.getValue())     ||
+               Tools.isFieldValueChange(oldInfo.getValue(), newInfo.getValue());
     }
+    @Override
     public boolean differsBinary(final FieldInfo oldInfo, final FieldInfo newInfo) {
-        if (Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess())) {
-            return true;
-        }
-        return false;
+        return Tools.isFieldAccessChange(oldInfo.getAccess(), newInfo.getAccess()); // &&
+               // Tools.isFieldTypeChange(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 7fb00ff..da03cd6 100644
--- a/api/src/main/java/org/osjava/jardiff/Tools.java
+++ b/api/src/main/java/org/osjava/jardiff/Tools.java
@@ -38,6 +38,17 @@ public final class Tools
     }
 
     /**
+     * Returns {@code true} if description has changed, i.e. the
+     * {@link MethodInfo#getDesc()} describing the return value.
+     * @param oldDesc
+     * @param newDesc
+     */
+    public static boolean isDescChange(final String oldDesc, final String newDesc) {
+        return null == oldDesc && null != newDesc ||
+               null != oldDesc && !oldDesc.equals(newDesc);
+    }
+
+    /**
      * 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
@@ -300,6 +311,14 @@ public final class Tools
         }
     }
 
+    public static boolean isFieldTypeChange(final Object oldValue, final Object newValue) {
+        if (oldValue == null || newValue == null) {
+            return oldValue != newValue;
+        } else {
+            return !oldValue.getClass().equals(newValue.getClass());
+        }
+    }
+
     /**
      * Returns whether a field's oldValue differs with newValue.
      * <p>
diff --git a/api/src/main/java/org/semver/Delta.java b/api/src/main/java/org/semver/Delta.java
index 6ef79fd..c62ef5a 100755
--- a/api/src/main/java/org/semver/Delta.java
+++ b/api/src/main/java/org/semver/Delta.java
@@ -38,10 +38,27 @@ public final class Delta {
      */
     public enum CompatibilityType {
 
+        /**
+         * No (public) changes.
+         */
         BACKWARD_COMPATIBLE_IMPLEMENTER,
 
+        /**
+         * Only <i>added</i> and <i>deprecated</i> changes,
+         * i.e. source compatible changes.
+         */
         BACKWARD_COMPATIBLE_USER,
 
+        /**
+         * Contains binary compatible changes,
+         * but may not be fully source compatible
+         * and may contain changed values of fields.
+         */
+        BACKWARD_COMPATIBLE_BINARY,
+
+        /**
+         * Contains non binary compatible changes.
+         */
         NON_BACKWARD_COMPATIBLE
     }
 
@@ -174,8 +191,9 @@ public final class Delta {
         if (contains(this.differences, Change.class) ||
             contains(this.differences, Remove.class)) {
             return CompatibilityType.NON_BACKWARD_COMPATIBLE;
+        } else if (contains(this.differences, CompatChange.class)) {
+            return CompatibilityType.BACKWARD_COMPATIBLE_BINARY;
         } else if (contains(this.differences, Add.class) ||
-                   contains(this.differences, CompatChange.class) ||
                    contains(this.differences, Deprecate.class)) {
             return CompatibilityType.BACKWARD_COMPATIBLE_USER;
         } else {
@@ -221,6 +239,7 @@ public final class Delta {
             case BACKWARD_COMPATIBLE_IMPLEMENTER:
                 return version.next(Version.Element.PATCH);
             case BACKWARD_COMPATIBLE_USER:
+            case BACKWARD_COMPATIBLE_BINARY:
                 return version.next(Version.Element.MINOR);
             case NON_BACKWARD_COMPATIBLE:
                 return version.next(Version.Element.MAJOR);
diff --git a/api/src/main/java/org/semver/Dumper.java b/api/src/main/java/org/semver/Dumper.java
index 651d2df..bf2448b 100755
--- a/api/src/main/java/org/semver/Dumper.java
+++ b/api/src/main/java/org/semver/Dumper.java
@@ -58,10 +58,10 @@ public class Dumper {
     protected static String extractDetails(final Difference difference) {
         if (difference instanceof Delta.Change) {
             final Delta.Change change = (Delta.Change) difference;
-            return extractDetails(difference.getInfo())+", "+extractChangeDetails(difference.getInfo(), change.getModifiedInfo());
+            return extractChangeDetails(difference.getInfo(), change.getModifiedInfo());
         } else if (difference instanceof Delta.CompatChange) {
             final Delta.CompatChange change = (Delta.CompatChange) difference;
-            return extractDetails(difference.getInfo())+", "+extractCompatChangeDetails(difference.getInfo(), change.getModifiedInfo());
+            return extractCompatChangeDetails(difference.getInfo(), change.getModifiedInfo());
         } else {
             return extractDetails(difference.getInfo())+", access["+extractAccessDetails(difference.getInfo())+"]";
         }
@@ -71,24 +71,47 @@ public class Dumper {
         final StringBuilder builder = new StringBuilder();
         if (!(info instanceof ClassInfo)) {
             builder.append(info.getName());
-            if( null != info.getSignature() ) {
-                builder.append(", sig ").append(info.getSignature());
-            }
-            if( null != info.getDesc() ) {
-                builder.append(", desc ").append(info.getDesc());
-            }
+            builder.append(", ").append(info.toString());
         }
         return builder.toString();
     }
 
     protected static String extractChangeDetails(final AbstractInfo previousInfo, final AbstractInfo currentInfo) {
         final StringBuilder builder = new StringBuilder();
+        if (!(previousInfo instanceof ClassInfo)) {
+            builder.append(previousInfo.getName());
+            final String pSig = previousInfo.getSignature();
+            final String pDesc = previousInfo.getDesc();
+            final String cSig = currentInfo.getSignature();
+            final String cDesc = currentInfo.getDesc();
+            if( null == pSig && null != cSig ||
+                null != pSig && !pSig.equals(cSig) ) {
+                builder.append(", sig[").append(pSig).append(" -> ").append(cSig).append("]");
+            }
+            if( null == pDesc && null != cDesc ||
+                null != pDesc && !pDesc.equals(cDesc) ) {
+                builder.append(", desc[").append(pDesc).append(" -> ").append(cDesc).append("]");
+            }
+        }
+        if( previousInfo instanceof FieldInfo ) {
+            final FieldInfo fPreInfo = (FieldInfo)previousInfo;
+            final FieldInfo fCurInfo = (FieldInfo)currentInfo;
+            final Object preValue = fPreInfo.getValue();
+            final Object curValue = fCurInfo.getValue();
+            if (Tools.isFieldTypeChange(preValue, curValue)) {
+                builder.append(", type[").append(preValue.getClass())
+                .append(" -> ").append(curValue.getClass()).append("]");
+            }
+        }
         builder.append(", access[");
         return extractAccessDetails(builder, previousInfo, currentInfo).append("]").toString().trim();
     }
 
     protected static String extractCompatChangeDetails(final AbstractInfo previousInfo, final AbstractInfo currentInfo) {
         final StringBuilder builder = new StringBuilder();
+        if (!(previousInfo instanceof ClassInfo)) {
+            builder.append(previousInfo.getName());
+        }
         if( previousInfo instanceof MethodInfo ) {
             final MethodInfo mPreInfo = (MethodInfo)previousInfo;
             final MethodInfo mCurInfo = (MethodInfo)currentInfo;
@@ -107,10 +130,8 @@ public class Dumper {
                 } else {
                     curThrowsSet = new HashSet<String>();
                 }
-                preThrowsSet.removeAll(curThrowsSet);
-                curThrowsSet.removeAll(preThrowsSet);
-                builder.append("throws[removed").append(preThrowsSet.toString())
-                .append(", added").append(curThrowsSet.toString()).append("]");
+                builder.append(", throws[").append(preThrowsSet.toString())
+                .append(" -> ").append(curThrowsSet.toString()).append("]");
             }
         } else if( previousInfo instanceof FieldInfo ) {
             final FieldInfo fPreInfo = (FieldInfo)previousInfo;
@@ -118,8 +139,8 @@ public class Dumper {
             final Object preValue = fPreInfo.getValue();
             final Object curValue = fCurInfo.getValue();
             if (Tools.isFieldValueChange(preValue, curValue)) {
-                builder.append("value[old: ").append(preValue)
-                .append(" != new: ").append(curValue).append("]");
+                builder.append(", value[").append(preValue)
+                .append(" -> ").append(curValue).append("]");
             }
         }
         builder.append(", access[");