Merge pull request #331 from Marcono1234/varargs

Detect method varargs changes
siom79 · Aug 30, 2022 · 95a4c29 · 95a4c29
2 parents 848bf67 + 453917c
commit 95a4c29
Show file tree

Hide file tree

Showing 7 changed files with 124 additions and 1 deletion.
diff --git a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java
@@ -266,6 +266,7 @@ private void checkIfConstructorsHaveChangedIncompatible(JApiClass jApiClass, Map
 			}
 			checkIfExceptionIsNowChecked(constructor);
 			checkIfAnnotationDeprecatedAdded(constructor);
+			checkIfVarargsChanged(constructor);
 		}
 	}
 
@@ -368,6 +369,7 @@ public Integer callback(JApiClass superclass, Map<String, JApiClass> classMap, J
 			checkAbstractMethod(jApiClass, classMap, method);
 			checkIfExceptionIsNowChecked(method);
 			checkIfAnnotationDeprecatedAdded(method);
+			checkIfVarargsChanged(method);
 		}
 	}
 
@@ -381,6 +383,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) {

diff --git a/japicmp/src/main/java/japicmp/model/JApiBehavior.java b/japicmp/src/main/java/japicmp/model/JApiBehavior.java
@@ -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;
@@ -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);
@@ -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;
@@ -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() {
@@ -342,6 +366,7 @@ public String getName() {
 	}
 
 	@XmlAttribute
+	@Override
 	public JApiChangeStatus getChangeStatus() {
 		return changeStatus;
 	}
@@ -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;
 	}
@@ -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() {
@@ -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;
 	}

diff --git a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java
@@ -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),

diff --git a/japicmp/src/main/java/japicmp/model/VarargsModifier.java b/japicmp/src/main/java/japicmp/model/VarargsModifier.java
@@ -0,0 +1,8 @@
+package japicmp.model;
+
+/**
+ * Represents the varargs modifier of a callable.
+ */
+public enum VarargsModifier implements JApiModifierBase {
+	VARARGS, NON_VARARGS
+}
diff --git a/japicmp/src/main/java/japicmp/util/ModifierHelper.java b/japicmp/src/main/java/japicmp/util/ModifierHelper.java
@@ -1,6 +1,5 @@
 package japicmp.util;
 
-import japicmp.util.Optional;
 import japicmp.cmp.JarArchiveComparatorOptions;
 import japicmp.config.Options;
 import japicmp.model.AccessModifier;

diff --git a/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java b/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java
@@ -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;
 
@@ -647,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();

diff --git a/src/site/markdown/MavenPlugin.md b/src/site/markdown/MavenPlugin.md
@@ -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 |