diff --git a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java index d942fb7ab..fe86c5ac6 100755 --- a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java +++ b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java @@ -9,6 +9,7 @@ import japicmp.util.SignatureParser; import javassist.ClassPool; import javassist.CtClass; +import javassist.CtMethod; import javassist.NotFoundException; import java.util.*; @@ -448,10 +449,14 @@ private void checkIfMethodsHaveChangedIncompatible(JApiClass jApiClass, Map newBridgeModifier = candidate.getBridgeModifier().getNewModifier(); + if (!newBridgeModifier.isPresent() || newBridgeModifier.get() != BridgeModifier.BRIDGE) { + continue; + } + if (!candidate.getReturnType().getNewReturnType().equals(oldReturnType)) { + continue; + } + if (candidate.hasSameParameter(method)) { + bridgeMethodFound = true; + break; + } + } + if (!bridgeMethodFound) { + return false; + } + // Verify the new return type is a subtype of the old return type + try { + CtClass oldReturnCtClass = method.getOldMethod().get().getReturnType(); + CtClass newReturnCtClass = method.getNewMethod().get().getReturnType(); + return newReturnCtClass.subtypeOf(oldReturnCtClass); + } catch (NotFoundException e) { + return true; // trust the bridge method as the primary indicator + } + } + private void checkIfParametersGenericsChanged(JApiBehavior jApiBehavior) { if (jApiBehavior.getChangeStatus() == JApiChangeStatus.MODIFIED || jApiBehavior.getChangeStatus() == JApiChangeStatus.UNCHANGED) { diff --git a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java index f14479a88..8b9f74712 100644 --- a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java +++ b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChangeType.java @@ -31,6 +31,7 @@ public enum JApiCompatibilityChangeType { METHOD_LESS_ACCESSIBLE_THAN_IN_SUPERCLASS(false, false, JApiSemanticVersionLevel.MAJOR), METHOD_IS_STATIC_AND_OVERRIDES_NOT_STATIC(false, false, JApiSemanticVersionLevel.MAJOR), METHOD_RETURN_TYPE_CHANGED(false, false, JApiSemanticVersionLevel.MAJOR), + METHOD_RETURN_TYPE_COVARIANT_CHANGED(true, true, JApiSemanticVersionLevel.MINOR), METHOD_RETURN_TYPE_GENERICS_CHANGED(true, false, JApiSemanticVersionLevel.MINOR), METHOD_PARAMETER_GENERICS_CHANGED(true, false, JApiSemanticVersionLevel.MINOR), METHOD_NOW_ABSTRACT(false, false, JApiSemanticVersionLevel.MAJOR), diff --git a/japicmp/src/test/java/japicmp/compat/MethodCompatibilityTest.java b/japicmp/src/test/java/japicmp/compat/MethodCompatibilityTest.java index 8273c3051..e477aa6a2 100644 --- a/japicmp/src/test/java/japicmp/compat/MethodCompatibilityTest.java +++ b/japicmp/src/test/java/japicmp/compat/MethodCompatibilityTest.java @@ -221,6 +221,45 @@ public List createNewClasses(ClassPool classPool) throws Exception { assertThat(jApiMethod.isSourceCompatible(), is(true)); } + @Test + void testMethodReturnTypeCovariantChange() throws Exception { + JarArchiveComparatorOptions options = new JarArchiveComparatorOptions(); + options.setIncludeSynthetic(true); + List jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() { + @Override + public List createOldClasses(ClassPool classPool) throws Exception { + CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool); + CtMethodBuilder.create().publicAccess().returnType(classPool.get("java.lang.Object")).name("getValue").body("return null;").addToClass(ctClass); + return Collections.singletonList(ctClass); + } + + @Override + public List createNewClasses(ClassPool classPool) throws Exception { + CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool); + // Covariant override — Integer is a subtype of Object + CtMethodBuilder.create().publicAccess().returnType(classPool.get("java.lang.Integer")).name("getValue").body("return Integer.valueOf(0);").addToClass(ctClass); + // Compiler-generated bridge method with original return type + CtMethodBuilder.create().publicAccess().bridgeModifier().syntheticModifier().returnType(classPool.get("java.lang.Object")).name("getValue").body("return null;").addToClass(ctClass); + return Collections.singletonList(ctClass); + } + }); + JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.Test"); + assertThat(jApiClass.getChangeStatus(), is(JApiChangeStatus.MODIFIED)); + JApiMethod nonBridgeMethod = null; + for (JApiMethod m : jApiClass.getMethods()) { + if (m.getName().equals("getValue") && + m.getBridgeModifier().getNewModifier().map(bm -> bm == BridgeModifier.NON_BRIDGE).orElse(false)) { + nonBridgeMethod = m; + break; + } + } + assertThat(nonBridgeMethod, is(notNullValue())); + assertThat(nonBridgeMethod.getCompatibilityChanges(), hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.METHOD_RETURN_TYPE_COVARIANT_CHANGED))); + assertThat(nonBridgeMethod.getCompatibilityChanges(), not(hasItem(new JApiCompatibilityChange(JApiCompatibilityChangeType.METHOD_RETURN_TYPE_CHANGED)))); + assertThat(nonBridgeMethod.isBinaryCompatible(), is(true)); + assertThat(jApiClass.isBinaryCompatible(), is(true)); + } + @Test void testMethodNowAbstract() throws Exception { JarArchiveComparatorOptions options = new JarArchiveComparatorOptions(); diff --git a/japicmp/src/test/java/japicmp/util/CtMethodBuilder.java b/japicmp/src/test/java/japicmp/util/CtMethodBuilder.java index 85062e96a..3031e2976 100644 --- a/japicmp/src/test/java/japicmp/util/CtMethodBuilder.java +++ b/japicmp/src/test/java/japicmp/util/CtMethodBuilder.java @@ -39,6 +39,11 @@ public CtMethodBuilder syntheticModifier() { return this; } + public CtMethodBuilder bridgeModifier() { + this.modifier = this.modifier | ModifierHelper.ACC_BRIDGE; + return this; + } + public CtMethodBuilder parameters(CtClass[] parameters) { return (CtMethodBuilder) super.parameters(parameters); } diff --git a/src/site/markdown/MavenPlugin.md b/src/site/markdown/MavenPlugin.md index 1cbea684c..9b8811285 100644 --- a/src/site/markdown/MavenPlugin.md +++ b/src/site/markdown/MavenPlugin.md @@ -325,6 +325,7 @@ for each check. This allows you to customize the following verifications: | METHOD_REMOVED | false | false | MAJOR | | METHOD_REMOVED_IN_SUPERCLASS | false | false | MAJOR | | METHOD_RETURN_TYPE_CHANGED | false | false | MAJOR | +| METHOD_RETURN_TYPE_COVARIANT_CHANGED | true | true | MINOR | | METHOD_RETURN_TYPE_GENERICS_CHANGED | true | false | MINOR | | METHOD_STATIC_IN_INTERFACE_NO_LONGER_STATIC | false | false | MAJOR | | SUPERCLASS_ADDED | true | true | MINOR |