Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect method varargs changes #331

Merged
merged 1 commit into from Aug 30, 2022
Merged

Detect method varargs changes #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java
View file
Open in desktop
Expand Up @@ -265,6 +265,7 @@ private void checkIfConstructorsHaveChangedIncompatible(JApiClass jApiClass, Map
}
}
checkIfAnnotationDeprecatedAdded(constructor);
checkIfVarargsChanged(constructor);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have also added this check for constructors, though the names METHOD_NOW_VARARGS and METHOD_NO_LONGER_VARARGS might be a bit misleading then. Should I rename them?

}
}

Expand Down Expand Up @@ -367,6 +368,7 @@ public Integer callback(JApiClass superclass, Map<String, JApiClass> classMap, J
checkAbstractMethod(jApiClass, classMap, method);
checkIfExceptionIsNowChecked(method);
checkIfAnnotationDeprecatedAdded(method);
checkIfVarargsChanged(method);
}
}

Expand All @@ -380,6 +382,15 @@ private void checkIfAnnotationDeprecatedAdded(JApiHasAnnotations jApiHasAnnotati
}
}

private void checkIfVarargsChanged(JApiBehavior behavior) {
if (behavior.getVarargsModifier().hasChangedFromTo(VarargsModifier.VARARGS, VarargsModifier.NON_VARARGS)) {
addCompatibilityChange(behavior, JApiCompatibilityChange.METHOD_NO_LONGER_VARARGS);
}
if (behavior.getVarargsModifier().hasChangedFromTo(VarargsModifier.NON_VARARGS, VarargsModifier.VARARGS)) {
addCompatibilityChange(behavior, JApiCompatibilityChange.METHOD_NOW_VARARGS);
}
}

private void checkAbstractMethod(JApiClass jApiClass, Map<String, JApiClass> classMap, JApiMethod method) {
if (isInterface(jApiClass)) {
if (jApiClass.getChangeStatus() != JApiChangeStatus.NEW) {
Expand Down
39 changes: 39 additions & 0 deletions japicmp/src/main/java/japicmp/model/JApiBehavior.java
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ public class JApiBehavior implements JApiHasModifiers, JApiHasChangeStatus, JApi
private final JApiModifier<BridgeModifier> bridgeModifier;
private final JApiModifier<SyntheticModifier> syntheticModifier;
private final JApiAttribute<SyntheticAttribute> syntheticAttribute;
private final JApiModifier<VarargsModifier> varargsModifier;
private final List<JApiException> exceptions;
protected JApiChangeStatus changeStatus;
private final Optional<Integer> oldLineNumber;
Expand All @@ -57,6 +58,7 @@ public JApiBehavior(JApiClass jApiClass, String name, Optional<? extends CtBehav
this.bridgeModifier = extractBridgeModifier(oldBehavior, newBehavior);
this.syntheticModifier = extractSyntheticModifier(oldBehavior, newBehavior);
this.syntheticAttribute = extractSyntheticAttribute(oldBehavior, newBehavior);
this.varargsModifier = extractVarargsModifier(oldBehavior, newBehavior);
this.exceptions = computeExceptionChanges(oldBehavior, newBehavior);
this.changeStatus = evaluateChangeStatus(changeStatus);
this.oldLineNumber = getLineNumber(oldBehavior);
Expand Down Expand Up @@ -181,6 +183,9 @@ private JApiChangeStatus evaluateChangeStatus(JApiChangeStatus changeStatus) {
if (this.syntheticAttribute.getChangeStatus() != JApiChangeStatus.UNCHANGED) {
changeStatus = JApiChangeStatus.MODIFIED;
}
if (this.varargsModifier.getChangeStatus() != JApiChangeStatus.UNCHANGED) {
changeStatus = JApiChangeStatus.MODIFIED;
}
for (JApiException jApiException : exceptions) {
if (jApiException.getChangeStatus() == JApiChangeStatus.NEW || jApiException.getChangeStatus() == JApiChangeStatus.REMOVED) {
changeStatus = JApiChangeStatus.MODIFIED;
Expand Down Expand Up @@ -330,6 +335,25 @@ public SyntheticModifier getModifierForNew(CtBehavior newBehavior) {
});
}

private JApiModifier<VarargsModifier> extractVarargsModifier(Optional<? extends CtBehavior> oldBehaviorOptional, Optional<? extends CtBehavior> newBehaviorOptional) {
return ModifierHelper.extractModifierFromBehavior(oldBehaviorOptional, newBehaviorOptional, new ModifierHelper.ExtractModifierFromBehaviorCallback<VarargsModifier>() {
private VarargsModifier getModifier(CtBehavior behavior) {
return Modifier.isVarArgs(behavior.getModifiers()) ? VarargsModifier.VARARGS : VarargsModifier.NON_VARARGS;
}

@Override
public VarargsModifier getModifierForOld(CtBehavior oldBehavior) {
return getModifier(oldBehavior);
}

@Override
public VarargsModifier getModifierForNew(CtBehavior newBehavior) {
return getModifier(newBehavior);
}
});
}

@Override
@XmlElementWrapper(name = "modifiers")
@XmlElement(name = "modifier")
public List<? extends JApiModifier<? extends Enum<? extends Enum<?>>>> getModifiers() {
Expand All @@ -342,6 +366,7 @@ public String getName() {
}

@XmlAttribute
@Override
public JApiChangeStatus getChangeStatus() {
return changeStatus;
}
Expand All @@ -357,20 +382,24 @@ public void addParameter(JApiParameter jApiParameter) {
}

@XmlTransient
@Override
public JApiModifier<AccessModifier> getAccessModifier() {
return accessModifier;
}

@XmlTransient
@Override
public JApiModifier<FinalModifier> getFinalModifier() {
return finalModifier;
}

@XmlTransient
@Override
public JApiModifier<StaticModifier> getStaticModifier() {
return staticModifier;
}

@Override
public JApiModifier<AbstractModifier> getAbstractModifier() {
return this.abstractModifier;
}
Expand All @@ -384,20 +413,28 @@ public List<JApiAttribute<? extends Enum<?>>> getAttributes() {
}

@XmlTransient
@Override
public JApiModifier<BridgeModifier> getBridgeModifier() {
return this.bridgeModifier;
}

@XmlTransient
@Override
public JApiModifier<SyntheticModifier> getSyntheticModifier() {
return this.syntheticModifier;
}

@XmlTransient
@Override
public JApiAttribute<SyntheticAttribute> getSyntheticAttribute() {
return syntheticAttribute;
}

@XmlTransient
public JApiModifier<VarargsModifier> getVarargsModifier() {
return varargsModifier;
}

@Override
@XmlAttribute
public boolean isBinaryCompatible() {
Expand All @@ -424,12 +461,14 @@ public boolean isSourceCompatible() {

@XmlElementWrapper(name = "compatibilityChanges")
@XmlElement(name = "compatibilityChange")
@Override
public List<JApiCompatibilityChange> getCompatibilityChanges() {
return compatibilityChanges;
}

@XmlElementWrapper(name = "annotations")
@XmlElement(name = "annotation")
@Override
public List<JApiAnnotation> getAnnotations() {
return annotations;
}
Expand Down
2 changes: 2 additions & 0 deletions japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java
View file
Open in desktop
Expand Up @@ -28,6 +28,8 @@ public enum JApiCompatibilityChange {
METHOD_NOW_FINAL(false, false, JApiSemanticVersionLevel.MAJOR),
METHOD_NOW_STATIC(false, false, JApiSemanticVersionLevel.MAJOR),
METHOD_NO_LONGER_STATIC(false, false, JApiSemanticVersionLevel.MAJOR),
METHOD_NOW_VARARGS(true, true, JApiSemanticVersionLevel.MINOR),
METHOD_NO_LONGER_VARARGS(true, false, JApiSemanticVersionLevel.MINOR),
METHOD_ADDED_TO_INTERFACE(true, false, JApiSemanticVersionLevel.MINOR),
METHOD_ADDED_TO_PUBLIC_CLASS(true, true, JApiSemanticVersionLevel.PATCH),
METHOD_NOW_THROWS_CHECKED_EXCEPTION(true, false, JApiSemanticVersionLevel.MINOR),
Expand Down
8 changes: 8 additions & 0 deletions japicmp/src/main/java/japicmp/model/VarargsModifier.java
View file
Open in desktop
@@ -0,0 +1,8 @@
package japicmp.model;

/**
* Represents the varargs modifier of a callable.
*/
public enum VarargsModifier implements JApiModifierBase {
VARARGS, NON_VARARGS
}
1 change: 0 additions & 1 deletion japicmp/src/main/java/japicmp/util/ModifierHelper.java
View file
Open in desktop
@@ -1,6 +1,5 @@
package japicmp.util;

import japicmp.util.Optional;
import japicmp.cmp.JarArchiveComparatorOptions;
import japicmp.config.Options;
import japicmp.model.AccessModifier;
Expand Down
63 changes: 63 additions & 0 deletions japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java
View file
Open in desktop
Expand Up @@ -6,6 +6,8 @@
import japicmp.util.*;
import javassist.ClassPool;
import javassist.CtClass;
import javassist.Modifier;

import org.hamcrest.core.Is;
import org.junit.Test;

Expand All @@ -18,6 +20,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsNot.not;
import static org.junit.Assert.assertEquals;

public class CompatibilityChangesTest {

Expand Down Expand Up @@ -646,6 +649,66 @@ public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
assertThat(jApiMethod.isBinaryCompatible(), is(false));
}

@Test
public void testMethodNowVarargs() throws Exception {
JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
List<JApiClass> jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() {
@Override
public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {
CtClass arrayClass = classPool.getCtClass("[I");

CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
CtMethodBuilder.create().publicAccess().parameter(arrayClass).name("methodNowVarargs").addToClass(ctClass);
return Collections.singletonList(ctClass);
}

@Override
public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
CtClass arrayClass = classPool.getCtClass("[I");

CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
CtMethodBuilder.create().modifier(Modifier.VARARGS).publicAccess().parameter(arrayClass).name("methodNowVarargs").addToClass(ctClass);
return Collections.singletonList(ctClass);
}
});
JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.Test");
assertThat(jApiClass.getChangeStatus(), is(JApiChangeStatus.MODIFIED));
assertThat(jApiClass.isBinaryCompatible(), is(true));
assertThat(jApiClass.isSourceCompatible(), is(true));
JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "methodNowVarargs");
assertEquals(jApiMethod.getCompatibilityChanges(), Collections.singletonList(JApiCompatibilityChange.METHOD_NOW_VARARGS));
}

@Test
public void testMethodNoLongerVarargs() throws Exception {
JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
List<JApiClass> jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() {
@Override
public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {
CtClass arrayClass = classPool.getCtClass("[I");

CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
CtMethodBuilder.create().modifier(Modifier.VARARGS).publicAccess().parameter(arrayClass).name("methodNoLongerVarargs").addToClass(ctClass);
return Collections.singletonList(ctClass);
}

@Override
public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
CtClass arrayClass = classPool.getCtClass("[I");

CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
CtMethodBuilder.create().publicAccess().parameter(arrayClass).name("methodNoLongerVarargs").addToClass(ctClass);
return Collections.singletonList(ctClass);
}
});
JApiClass jApiClass = getJApiClass(jApiClasses, "japicmp.Test");
assertThat(jApiClass.getChangeStatus(), is(JApiChangeStatus.MODIFIED));
assertThat(jApiClass.isBinaryCompatible(), is(true));
assertThat(jApiClass.isSourceCompatible(), is(false));
JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "methodNoLongerVarargs");
assertEquals(jApiMethod.getCompatibilityChanges(), Collections.singletonList(JApiCompatibilityChange.METHOD_NO_LONGER_VARARGS));
}

@Test
public void testFieldStaticOverridesStatic() throws Exception {
JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
Expand Down
2 changes: 2 additions & 0 deletions src/site/markdown/MavenPlugin.md
View file
Open in desktop
Expand Up @@ -281,6 +281,8 @@ for each check. This allows you to customize the following verifications:
| METHOD_NOW_FINAL | false | false | MAJOR |
| METHOD_NOW_STATIC | false | false | MAJOR |
| METHOD_NO_LONGER_STATIC | false | false | MAJOR |
| METHOD_NOW_VARARGS | true | true | MINOR |
| METHOD_NO_LONGER_VARARGS | true | false | MINOR |
| METHOD_ADDED_TO_INTERFACE | true | false | MINOR |
| METHOD_ADDED_TO_PUBLIC_CLASS | true | true | PATCH |
| METHOD_NOW_THROWS_CHECKED_EXCEPTION | true | false | MINOR |
Expand Down