From 599da8299817c63466086fec98aa73b106bfd64b Mon Sep 17 00:00:00 2001 From: Martin Mois Date: Tue, 23 Apr 2024 18:37:19 +0200 Subject: [PATCH] #395: checkIfAnnotationsChanged() no longer tracks changes of the annotation class but only of the values --- .../test/annotation/TestAnnotation.java | 1 + .../test/annotation/TestAnnotation.java | 1 + .../japicmp/compat/CompatibilityChanges.java | 399 +++++++++--------- .../japicmp/model/JApiAnnotationElement.java | 3 - .../model/JApiCompatibilityChange.java | 8 + .../model/JApiCompatibilityChangeType.java | 1 - .../compat/CompatibilityChangesTest.java | 37 +- src/site/markdown/MavenPlugin.md | 1 - 8 files changed, 215 insertions(+), 236 deletions(-) diff --git a/japicmp-testbase/japicmp-test-v1/src/main/java/japicmp/test/annotation/TestAnnotation.java b/japicmp-testbase/japicmp-test-v1/src/main/java/japicmp/test/annotation/TestAnnotation.java index fc8fc0bb..80b9086a 100644 --- a/japicmp-testbase/japicmp-test-v1/src/main/java/japicmp/test/annotation/TestAnnotation.java +++ b/japicmp-testbase/japicmp-test-v1/src/main/java/japicmp/test/annotation/TestAnnotation.java @@ -15,6 +15,7 @@ String label(); } + @TestAnnotation(name = "recursion", list = {"r", "r2"}, type = @TestAnnotation.Type(label = "test-recursion")) String name() default "default-name"; String[] list(); Type type(); diff --git a/japicmp-testbase/japicmp-test-v2/src/main/java/japicmp/test/annotation/TestAnnotation.java b/japicmp-testbase/japicmp-test-v2/src/main/java/japicmp/test/annotation/TestAnnotation.java index fc8fc0bb..00873f66 100644 --- a/japicmp-testbase/japicmp-test-v2/src/main/java/japicmp/test/annotation/TestAnnotation.java +++ b/japicmp-testbase/japicmp-test-v2/src/main/java/japicmp/test/annotation/TestAnnotation.java @@ -16,6 +16,7 @@ } String name() default "default-name"; + @TestAnnotation(name = "recursion", list = {"r", "r2"}, type = @TestAnnotation.Type(label = "test-recursion")) String[] list(); Type type(); } diff --git a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java index 8afd2560..d70817b2 100755 --- a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java +++ b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java @@ -116,10 +116,10 @@ private void checkIfFieldsHaveChangedIncompatible(JApiClass jApiClass, Map returnValues = new ArrayList<>(); forAllSuperclasses(jApiClass, classMap, returnValues, (superclass, classMap12, changeStatusOfSuperclass) -> { - int changedIncompatible = 0; - for (JApiField superclassField : superclass.getFields()) { - if (superclassField.getName().equals(field.getName()) && fieldTypeMatches(superclassField, field)) { - boolean superclassFieldIsStatic = false; - boolean subclassFieldIsStatic = false; - boolean accessModifierSubclassLess = false; - if (field.getStaticModifier().getNewModifier().isPresent() && field.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC) { - subclassFieldIsStatic = true; - } - if (superclassField.getStaticModifier().getNewModifier().isPresent() && superclassField.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC && superclassField.getChangeStatus() != JApiChangeStatus.NEW) { - superclassFieldIsStatic = true; - } - if (field.getAccessModifier().getNewModifier().isPresent() && superclassField.getAccessModifier().getNewModifier().isPresent()) { - if (field.getAccessModifier().getNewModifier().get().getLevel() < superclassField.getAccessModifier().getNewModifier().get().getLevel() && superclassField.getChangeStatus() != JApiChangeStatus.NEW) { - accessModifierSubclassLess = true; - } - } - if (superclassFieldIsStatic) { - if (subclassFieldIsStatic) { - changedIncompatible = 1; - } - } - if (accessModifierSubclassLess) { - changedIncompatible = 2; - } - } - } - return changedIncompatible; - }); + int changedIncompatible = 0; + for (JApiField superclassField : superclass.getFields()) { + if (superclassField.getName().equals(field.getName()) && fieldTypeMatches(superclassField, field)) { + boolean superclassFieldIsStatic = false; + boolean subclassFieldIsStatic = false; + boolean accessModifierSubclassLess = false; + if (field.getStaticModifier().getNewModifier().isPresent() && field.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC) { + subclassFieldIsStatic = true; + } + if (superclassField.getStaticModifier().getNewModifier().isPresent() && superclassField.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC && superclassField.getChangeStatus() != JApiChangeStatus.NEW) { + superclassFieldIsStatic = true; + } + if (field.getAccessModifier().getNewModifier().isPresent() && superclassField.getAccessModifier().getNewModifier().isPresent()) { + if (field.getAccessModifier().getNewModifier().get().getLevel() < superclassField.getAccessModifier().getNewModifier().get().getLevel() && superclassField.getChangeStatus() != JApiChangeStatus.NEW) { + accessModifierSubclassLess = true; + } + } + if (superclassFieldIsStatic) { + if (subclassFieldIsStatic) { + changedIncompatible = 1; + } + } + if (accessModifierSubclassLess) { + changedIncompatible = 2; + } + } + } + return changedIncompatible; + }); for (int returnValue : returnValues) { if (returnValue == 1) { addCompatibilityChange(field, JApiCompatibilityChangeType.FIELD_STATIC_AND_OVERRIDES_STATIC); @@ -242,22 +242,26 @@ private JApiClass loadClass(String newSuperclassName, EnumSet classpa ClassPool classPool = this.jarArchiveComparator.getCommonClassPool(); try { oldClassOptional = Optional.of(classPool.get(newSuperclassName)); - } catch (NotFoundException ignored) {} + } catch (NotFoundException ignored) { + } try { newClassOptional = Optional.of(classPool.get(newSuperclassName)); - } catch (NotFoundException ignored) {} + } catch (NotFoundException ignored) { + } } else { if (classpaths.contains(Classpath.OLD_CLASSPATH)) { ClassPool oldClassPool = this.jarArchiveComparator.getOldClassPool(); try { oldClassOptional = Optional.of(oldClassPool.get(newSuperclassName)); - } catch (NotFoundException ignored) {} + } catch (NotFoundException ignored) { + } } if (classpaths.contains(Classpath.NEW_CLASSPATH)) { ClassPool newClassPool = this.jarArchiveComparator.getNewClassPool(); try { newClassOptional = Optional.of(newClassPool.get(newSuperclassName)); - } catch (NotFoundException ignored) {} + } catch (NotFoundException ignored) { + } } } if (!oldClassOptional.isPresent() && !newClassOptional.isPresent()) { @@ -300,7 +304,7 @@ private void checkIfConstructorsHaveChangedIncompatible(JApiClass jApiClass, Map } // section 13.4.7 of "Java Language Specification" SE7 if (hasModifierLevelDecreased(constructor)) { - if (isNotFinal(jApiClass) || constructor.getAccessModifier().hasChangedFrom(AccessModifier.PUBLIC)) { + if (isNotFinal(jApiClass) || constructor.getAccessModifier().hasChangedFrom(AccessModifier.PUBLIC)) { addCompatibilityChange(constructor, JApiCompatibilityChangeType.CONSTRUCTOR_LESS_ACCESSIBLE); } } @@ -324,20 +328,20 @@ private void checkIfMethodsHaveChangedIncompatible(JApiClass jApiClass, Map returnValues = new ArrayList<>(); forAllSuperclasses(jApiClass, classMap, returnValues, (superclass, classMap12, changeStatusOfSuperclass) -> { - for (JApiMethod superMethod : superclass.getMethods()) { - if (superMethod.getName().equals(method.getName()) && superMethod.hasSameSignature(method)) { - return 1; - } - } - return 0; - }); + for (JApiMethod superMethod : superclass.getMethods()) { + if (superMethod.getName().equals(method.getName()) && superMethod.hasSameSignature(method)) { + return 1; + } + } + return 0; + }); checkIfMethodHasBeenPulledUp(jApiClass, classMap, method, returnValues); boolean superclassHasSameMethod = false; for (Integer returnValue : returnValues) { - if (returnValue == 1) { - superclassHasSameMethod = true; - break; - } + if (returnValue == 1) { + superclassHasSameMethod = true; + break; + } } if (!superclassHasSameMethod) { addCompatibilityChange(method, JApiCompatibilityChangeType.METHOD_REMOVED); @@ -356,23 +360,23 @@ private void checkIfMethodsHaveChangedIncompatible(JApiClass jApiClass, Map returnValues = new ArrayList<>(); forAllSuperclasses(jApiClass, classMap, returnValues, (superclass, classMap13, changeStatusOfSuperclass) -> { - for (JApiMethod superMethod : superclass.getMethods()) { - if (superMethod.getName().equals(method.getName()) && superMethod.hasSameSignature(method)) { - if (superMethod.getAccessModifier().getNewModifier().isPresent() && method.getAccessModifier().getNewModifier().isPresent()) { - if (superMethod.getAccessModifier().getNewModifier().get().getLevel() > method.getAccessModifier().getNewModifier().get().getLevel()) { - return 1; - } - } - if (superMethod.getStaticModifier().getNewModifier().isPresent() && method.getStaticModifier().getNewModifier().isPresent()) { - if (superMethod.getStaticModifier().getNewModifier().get() == StaticModifier.NON_STATIC - && method.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC) { - return 2; - } - } - } - } - return 0; - }); + for (JApiMethod superMethod : superclass.getMethods()) { + if (superMethod.getName().equals(method.getName()) && superMethod.hasSameSignature(method)) { + if (superMethod.getAccessModifier().getNewModifier().isPresent() && method.getAccessModifier().getNewModifier().isPresent()) { + if (superMethod.getAccessModifier().getNewModifier().get().getLevel() > method.getAccessModifier().getNewModifier().get().getLevel()) { + return 1; + } + } + if (superMethod.getStaticModifier().getNewModifier().isPresent() && method.getStaticModifier().getNewModifier().isPresent()) { + if (superMethod.getStaticModifier().getNewModifier().get() == StaticModifier.NON_STATIC + && method.getStaticModifier().getNewModifier().get() == StaticModifier.STATIC) { + return 2; + } + } + } + } + return 0; + }); for (Integer returnValue : returnValues) { if (returnValue == 1) { addCompatibilityChange(method, JApiCompatibilityChangeType.METHOD_LESS_ACCESSIBLE_THAN_IN_SUPERCLASS); @@ -399,9 +403,9 @@ private void checkIfMethodsHaveChangedIncompatible(JApiClass jApiClass, Map classMap) { - for (JApiAnnotation annotation : jApiHasAnnotations.getAnnotations()) { - final JApiChangeStatus status = annotation.getChangeStatus(); - if (status == JApiChangeStatus.REMOVED) { - addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_REMOVED); - } else { - boolean isNoAnnotations = this.jarArchiveComparator.getJarArchiveComparatorOptions().isNoAnnotations(); - if (!isNoAnnotations) { + boolean isNoAnnotations = this.jarArchiveComparator.getJarArchiveComparatorOptions().isNoAnnotations(); + if (!isNoAnnotations) { + for (JApiAnnotation annotation : jApiHasAnnotations.getAnnotations()) { + final JApiChangeStatus status = annotation.getChangeStatus(); + if (status == JApiChangeStatus.REMOVED) { + addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_REMOVED); + } else { boolean isDeprecated = annotation.getFullyQualifiedName().equals(Deprecated.class.getName()); if (isDeprecated && status == JApiChangeStatus.NEW) { addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_DEPRECATED_ADDED); } else { JApiClass annotationClass = classMap.computeIfAbsent(annotation.getFullyQualifiedName(), fqn -> loadClass(fqn, EnumSet.allOf(Classpath.class))); - if (annotationClass != null) { - annotation.setJApiClass(annotationClass); - if (status == JApiChangeStatus.NEW) { - addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_ADDED); - } else { - checkIfMethodsHaveChangedIncompatible(annotationClass, classMap); - checkIfFieldsHaveChangedIncompatible(annotationClass, classMap); - if (!annotationClass.isSourceCompatible() || !annotationClass.isBinaryCompatible()) { - addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_MODIFIED_INCOMPATIBLE); - } else { + annotation.setJApiClass(annotationClass); + if (status == JApiChangeStatus.NEW) { + addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_ADDED); + } else { + for (JApiAnnotationElement annotationElement : annotation.getElements()) { + if (annotationElement.getChangeStatus() != JApiChangeStatus.UNCHANGED) { addCompatibilityChange(jApiHasAnnotations, JApiCompatibilityChangeType.ANNOTATION_MODIFIED); } } @@ -583,15 +583,15 @@ private void checkAbstractMethod(JApiClass jApiClass, Map cla private List getOverriddenMethods(final JApiClass jApiClass, final Map classMap, final JApiMethod method) { ArrayList jApiMethods = new ArrayList<>(); forAllSuperclasses(jApiClass, classMap, jApiMethods, (superclass, classMap1, changeStatusOfSuperclass) -> { - for (JApiMethod jApiMethod : superclass.getMethods()) { - if (isAbstract(jApiMethod)) { - if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameSignature(method)) { - return jApiMethod; - } - } - } - return null; - }); + for (JApiMethod jApiMethod : superclass.getMethods()) { + if (isAbstract(jApiMethod)) { + if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameSignature(method)) { + return jApiMethod; + } + } + } + return null; + }); return removeNullValues(jApiMethods); } @@ -608,13 +608,13 @@ private List removeNullValues(ArrayList jApiMethods) { private List getMethodsInImplementedInterfacesWithSameSignature(final JApiClass jApiClass, final Map classMap, final JApiMethod method) { ArrayList jApiMethods = new ArrayList<>(); forAllImplementedInterfaces(jApiClass, classMap, jApiMethods, new ArrayList<>(), (implementedInterface, classMap1) -> { - for (JApiMethod jApiMethod : implementedInterface.getMethods()) { - if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameSignature(method)) { - return jApiMethod; - } - } - return null; - }); + for (JApiMethod jApiMethod : implementedInterface.getMethods()) { + if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameSignature(method)) { + return jApiMethod; + } + } + return null; + }); return removeNullValues(jApiMethods); } @@ -681,25 +681,26 @@ private void forAllImplementedInterfaces(JApiClass jApiClass, Maptrue if it is implemented in a super class */ private boolean isImplemented(JApiMethod jApiMethod, JApiClass aClass) { - while(aClass != null) { - for (JApiMethod method : aClass.getMethods()) { - if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameParameter(method) && - !isAbstract(method) && method.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(method)) { - return true; - } - } + while (aClass != null) { + for (JApiMethod method : aClass.getMethods()) { + if (jApiMethod.getName().equals(method.getName()) && jApiMethod.hasSameParameter(method) && + !isAbstract(method) && method.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(method)) { + return true; + } + } - if(aClass.getSuperclass() != null && aClass.getSuperclass().getJApiClass().isPresent()) { - aClass = aClass.getSuperclass().getJApiClass().get(); - } else { - aClass = null; - } - } - return false; + if (aClass.getSuperclass() != null && aClass.getSuperclass().getJApiClass().isPresent()) { + aClass = aClass.getSuperclass().getJApiClass().get(); + } else { + aClass = null; + } + } + return false; } private void checkIfSuperclassesOrInterfacesChangedIncompatible(final JApiClass jApiClass, Map classMap) { @@ -723,53 +724,53 @@ private void checkIfSuperclassesOrInterfacesChangedIncompatible(final JApiClass } } forAllSuperclasses(jApiClass, classMap, new ArrayList<>(), (superclass1, classMap1, changeStatusOfSuperclass) -> { - for (JApiMethod jApiMethod : superclass1.getMethods()) { - if (!isAbstract(jApiMethod) && jApiMethod.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(jApiMethod)) { - implementedMethods.add(jApiMethod); - } - removedAndNotOverriddenMethods.removeIf(removedAndNotOverriddenMethod -> + for (JApiMethod jApiMethod : superclass1.getMethods()) { + if (!isAbstract(jApiMethod) && jApiMethod.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(jApiMethod)) { + implementedMethods.add(jApiMethod); + } + removedAndNotOverriddenMethods.removeIf(removedAndNotOverriddenMethod -> jApiMethod.getName().equals(removedAndNotOverriddenMethod.getName()) && jApiMethod.hasSameSignature(removedAndNotOverriddenMethod) ); - } - for (JApiField jApiField : superclass1.getFields()) { - if (jApiField.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(jApiField)) { - fields.add(jApiField); - } - removedAndNotOverriddenFields.removeIf(removedAndNotOverriddenField -> + } + for (JApiField jApiField : superclass1.getFields()) { + if (jApiField.getChangeStatus() != JApiChangeStatus.REMOVED && isNotPrivate(jApiField)) { + fields.add(jApiField); + } + removedAndNotOverriddenFields.removeIf(removedAndNotOverriddenField -> jApiField.getName().equals(removedAndNotOverriddenField.getName()) && hasSameType(jApiField, removedAndNotOverriddenField) ); - } - for (JApiMethod jApiMethod : superclass1.getMethods()) { - if (jApiMethod.getChangeStatus() == JApiChangeStatus.REMOVED && !isImplemented(jApiMethod, jApiMethod.getjApiClass())) { - boolean implemented = false; - for (JApiMethod implementedMethod : implementedMethods) { - if (jApiMethod.getName().equals(implementedMethod.getName()) && jApiMethod.hasSameSignature(implementedMethod)) { - implemented = true; - break; - } - } - if (!implemented) { - removedAndNotOverriddenMethods.add(jApiMethod); - } - } - } - for (JApiField jApiField : superclass1.getFields()) { - if (jApiField.getChangeStatus() == JApiChangeStatus.REMOVED) { - boolean overridden = false; - for (JApiField field : fields) { - if (field.getName().equals(jApiField.getName()) && hasSameType(jApiField, field)) { - overridden = true; - } - } - if (!overridden) { - removedAndNotOverriddenFields.add(jApiField); - } - } - } - return 0; - }); + } + for (JApiMethod jApiMethod : superclass1.getMethods()) { + if (jApiMethod.getChangeStatus() == JApiChangeStatus.REMOVED && !isImplemented(jApiMethod, jApiMethod.getjApiClass())) { + boolean implemented = false; + for (JApiMethod implementedMethod : implementedMethods) { + if (jApiMethod.getName().equals(implementedMethod.getName()) && jApiMethod.hasSameSignature(implementedMethod)) { + implemented = true; + break; + } + } + if (!implemented) { + removedAndNotOverriddenMethods.add(jApiMethod); + } + } + } + for (JApiField jApiField : superclass1.getFields()) { + if (jApiField.getChangeStatus() == JApiChangeStatus.REMOVED) { + boolean overridden = false; + for (JApiField field : fields) { + if (field.getName().equals(jApiField.getName()) && hasSameType(jApiField, field)) { + overridden = true; + } + } + if (!overridden) { + removedAndNotOverriddenFields.add(jApiField); + } + } + } + return 0; + }); if (!removedAndNotOverriddenMethods.isEmpty()) { addCompatibilityChange(jApiClass, JApiCompatibilityChangeType.METHOD_REMOVED_IN_SUPERCLASS); } @@ -795,12 +796,12 @@ private void checkIfSuperclassesOrInterfacesChangedIncompatible(final JApiClass List ancestors = new ArrayList<>(); final List matchingAncestors = new ArrayList<>(); forAllSuperclasses(jApiClass, classMap, ancestors, (clazz, classMap12, changeStatusOfSuperclass) -> { - JApiSuperclass ancestor = clazz.getSuperclass(); - if (ancestor.getNewSuperclassName().isPresent() && ancestor.getNewSuperclassName().get().equals(superclass.getOldSuperclassName().get())) { - matchingAncestors.add(ancestor); - } - return ancestor; - }); + JApiSuperclass ancestor = clazz.getSuperclass(); + if (ancestor.getNewSuperclassName().isPresent() && ancestor.getNewSuperclassName().get().equals(superclass.getOldSuperclassName().get())) { + matchingAncestors.add(ancestor); + } + return ancestor; + }); if (matchingAncestors.isEmpty()) { addCompatibilityChange(superclass, JApiCompatibilityChangeType.SUPERCLASS_REMOVED); } else { @@ -896,37 +897,37 @@ private void checkIfAbstractMethodAddedInSuperclass(final JApiClass jApiClass, M } } forAllSuperclasses(jApiClass, classMap, new ArrayList<>(), (superclass, classMap1, changeStatusOfSuperclass) -> { - for (JApiMethod jApiMethod : superclass.getMethods()) { - if (!isAbstract(jApiMethod)) { - implementedMethods.add(jApiMethod); - } - } - for (JApiMethod jApiMethod : superclass.getMethods()) { - if (isAbstract(jApiMethod)) { - boolean isImplemented = false; - for (JApiMethod implementedMethod : implementedMethods) { - if (jApiMethod.getName().equals(implementedMethod.getName()) && jApiMethod.hasSameSignature(implementedMethod)) { - isImplemented = true; - break; - } - } - if (!isImplemented) { - if (jApiMethod.getChangeStatus() == JApiChangeStatus.NEW || changeStatusOfSuperclass == JApiChangeStatus.NEW || changeStatusOfSuperclass == JApiChangeStatus.MODIFIED) { - if (jApiMethod.getAbstractModifier().getNewModifier().isPresent()) { - AbstractModifier abstractModifier = jApiMethod.getAbstractModifier().getNewModifier().get(); - if (abstractModifier == AbstractModifier.ABSTRACT) { - abstractMethods.add(jApiMethod); - } else { - defaultMethods.add(jApiMethod); - } - } - } - } - } - } - implementedInterfaces.addAll(superclass.getInterfaces()); - return 0; - }); + for (JApiMethod jApiMethod : superclass.getMethods()) { + if (!isAbstract(jApiMethod)) { + implementedMethods.add(jApiMethod); + } + } + for (JApiMethod jApiMethod : superclass.getMethods()) { + if (isAbstract(jApiMethod)) { + boolean isImplemented = false; + for (JApiMethod implementedMethod : implementedMethods) { + if (jApiMethod.getName().equals(implementedMethod.getName()) && jApiMethod.hasSameSignature(implementedMethod)) { + isImplemented = true; + break; + } + } + if (!isImplemented) { + if (jApiMethod.getChangeStatus() == JApiChangeStatus.NEW || changeStatusOfSuperclass == JApiChangeStatus.NEW || changeStatusOfSuperclass == JApiChangeStatus.MODIFIED) { + if (jApiMethod.getAbstractModifier().getNewModifier().isPresent()) { + AbstractModifier abstractModifier = jApiMethod.getAbstractModifier().getNewModifier().get(); + if (abstractModifier == AbstractModifier.ABSTRACT) { + abstractMethods.add(jApiMethod); + } else { + defaultMethods.add(jApiMethod); + } + } + } + } + } + } + implementedInterfaces.addAll(superclass.getInterfaces()); + return 0; + }); implementedInterfaces.addAll(jApiClass.getInterfaces()); if (!abstractMethods.isEmpty()) { addCompatibilityChange(jApiClass, JApiCompatibilityChangeType.METHOD_ABSTRACT_ADDED_IN_SUPERCLASS); @@ -962,7 +963,7 @@ private void checkIfAbstractMethodAddedInSuperclass(final JApiClass jApiClass, M final JApiClass interfaceClass = getJApiClass(implementedInterface, classMap); for (JApiMethod defaultMethodCandidate : interfaceClass.getMethods()) { if (!isAbstract(defaultMethodCandidate) - && areMatching(abstractMethod, defaultMethodCandidate)) { + && areMatching(abstractMethod, defaultMethodCandidate)) { // we have a default implementation for this method // double-check that we extend interface that the method comes from for (JApiImplementedInterface extendedInterface : interfaceClass.getInterfaces()) { @@ -989,7 +990,7 @@ && areMatching(abstractMethod, defaultMethodCandidate)) { private static boolean areMatching(JApiMethod left, JApiMethod right) { return left.getName().equals(right.getName()) - && left.hasSameSignature(right); + && left.hasSameSignature(right); } private JApiClass getJApiClass(JApiImplementedInterface implementedInterface, Map classMap) { diff --git a/japicmp/src/main/java/japicmp/model/JApiAnnotationElement.java b/japicmp/src/main/java/japicmp/model/JApiAnnotationElement.java index df313785..64052453 100644 --- a/japicmp/src/main/java/japicmp/model/JApiAnnotationElement.java +++ b/japicmp/src/main/java/japicmp/model/JApiAnnotationElement.java @@ -39,9 +39,6 @@ public String toString() + "]"; } - - - private JApiChangeStatus evaluateChangeStatus(JApiChangeStatus changeStatus) { if (changeStatus == JApiChangeStatus.UNCHANGED) { if (oldValue.isPresent() && newValue.isPresent()) { diff --git a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java index 0b697341..ff345d76 100644 --- a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java +++ b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java @@ -53,4 +53,12 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(type); } + + @Override + public String toString() { + final StringBuffer sb = new StringBuffer("JApiCompatibilityChange{"); + sb.append("type=").append(type); + sb.append('}'); + return sb.toString(); + } } diff --git a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java index 103c10a4..b97ab061 100644 --- a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java +++ b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java @@ -8,7 +8,6 @@ public enum JApiCompatibilityChangeType { ANNOTATION_ADDED(true, true, JApiSemanticVersionLevel.PATCH), ANNOTATION_DEPRECATED_ADDED(true, true, JApiSemanticVersionLevel.MINOR), ANNOTATION_MODIFIED(true, true, JApiSemanticVersionLevel.PATCH), - ANNOTATION_MODIFIED_INCOMPATIBLE(true, true, JApiSemanticVersionLevel.PATCH), ANNOTATION_REMOVED(true, true, JApiSemanticVersionLevel.PATCH), CLASS_REMOVED(false, false, JApiSemanticVersionLevel.MAJOR), CLASS_NOW_ABSTRACT(false, false, JApiSemanticVersionLevel.MAJOR), diff --git a/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java b/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java index e643cd60..687df4dd 100755 --- a/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java +++ b/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java @@ -2615,19 +2615,18 @@ public void testAnnotationOnClassModified() throws Exception { public List createOldClasses(ClassPool classPool) throws Exception { CtClass anAnnotation = CtAnnotationBuilder.create().name("japicmp.MyAnnotation").addToClassPool(classPool); CtMethodBuilder.create().name("foo").returnType(CtClass.intType).publicAccess().addToClass(anAnnotation); - CtClass aClass = CtClassBuilder.create().name("japicmp.Test").withAnnotation("japicmp.MyAnnotation").addToClassPool(classPool); - return Arrays.asList(aClass, anAnnotation); + return Collections.singletonList(anAnnotation); } @Override public List createNewClasses(ClassPool classPool) { CtClass anAnnotation = CtAnnotationBuilder.create().name("japicmp.MyAnnotation").addToClassPool(classPool); - CtClass aClass = CtClassBuilder.create().name("japicmp.Test").withAnnotation("japicmp.MyAnnotation").addToClassPool(classPool); - return Arrays.asList(aClass, anAnnotation); + return Arrays.asList(anAnnotation); } }); - JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.Test"); - assertThat(jApiClass.getCompatibilityChanges(), hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.ANNOTATION_MODIFIED_INCOMPATIBLE))); + JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.MyAnnotation"); + JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "foo"); + assertThat(jApiMethod.getCompatibilityChanges(), hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.METHOD_REMOVED))); } @Test @@ -2677,30 +2676,4 @@ public List createNewClasses(ClassPool classPool) throws Exception { JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "method"); assertThat(jApiMethod.getCompatibilityChanges(), hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.ANNOTATION_REMOVED))); } - - @Test - public void testAnnotationOnMethodModified() throws Exception { - JarArchiveComparatorOptions options = new JarArchiveComparatorOptions(); - List jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() { - @Override - public List createOldClasses(ClassPool classPool) throws Exception { - CtClass anAnnotation = CtAnnotationBuilder.create().name("japicmp.MyAnnotation").addToClassPool(classPool); - CtMethodBuilder.create().name("foo").returnType(CtClass.intType).publicAccess().addToClass(anAnnotation); - CtClass aClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool); - CtMethodBuilder.create().publicAccess().name("method").withAnnotation("japicmp.MyAnnotation").addToClass(aClass); - return Arrays.asList(aClass, anAnnotation); - } - - @Override - public List createNewClasses(ClassPool classPool) throws Exception { - CtClass anAnnotation = CtAnnotationBuilder.create().name("japicmp.MyAnnotation").addToClassPool(classPool); - CtClass aClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool); - CtMethodBuilder.create().publicAccess().name("method").withAnnotation("japicmp.MyAnnotation").addToClass(aClass); - return Arrays.asList(aClass, anAnnotation); - } - }); - JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.Test"); - JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "method"); - assertThat(jApiMethod.getCompatibilityChanges(), hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.ANNOTATION_MODIFIED_INCOMPATIBLE))); - } } diff --git a/src/site/markdown/MavenPlugin.md b/src/site/markdown/MavenPlugin.md index de602a6a..1e8841bd 100644 --- a/src/site/markdown/MavenPlugin.md +++ b/src/site/markdown/MavenPlugin.md @@ -265,7 +265,6 @@ for each check. This allows you to customize the following verifications: | ANNOTATION_ADDED | true | true | PATCH | | ANNOTATION_DEPRECATED_ADDED | true | true | MINOR | | ANNOTATION_MODIFIED | true | true | PATCH | -| ANNOTATION_MODIFIED_INCOMPATIBLE | true | true | PATCH | | ANNOTATION_REMOVED | true | true | PATCH | | CLASS_REMOVED | false | false | MAJOR | | CLASS_NOW_ABSTRACT | false | false | MAJOR |