Do not report any incompatibilities if class becomes public

siom79 · siom79 · commit c0ace886a54e · 2023-11-02T17:05:46.000+01:00
diff --git a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java
@@ -38,6 +38,9 @@ private Map<String, JApiClass> buildClassMap(List<JApiClass> classes) {
 	}
 
 	private void evaluateBinaryCompatibility(JApiClass jApiClass, Map<String, JApiClass> classMap) {
+		if (jApiClass.getAccessModifier().hasChangedTo(AccessModifier.PUBLIC)) {
+			return; // class appears as "new" public class
+		}
 		if (jApiClass.getChangeStatus() == JApiChangeStatus.REMOVED) {
 			addCompatibilityChange(jApiClass, JApiCompatibilityChange.CLASS_REMOVED);
 		} else if (jApiClass.getChangeStatus() == JApiChangeStatus.MODIFIED) {
@@ -583,11 +586,11 @@ public JApiMethod callback(JApiClass implementedInterface, Map<String, JApiClass
 	}
 
 	private boolean isAbstract(JApiHasAbstractModifier jApiHasAbstractModifier) {
-		boolean isAbstract = false;
-		if (jApiHasAbstractModifier.getAbstractModifier().hasChangedTo(AbstractModifier.ABSTRACT)) {
-			isAbstract = true;
+		if (jApiHasAbstractModifier.getAbstractModifier().getNewModifier().isPresent()) {
+			AbstractModifier abstractModifier = jApiHasAbstractModifier.getAbstractModifier().getNewModifier().get();
+			return abstractModifier == AbstractModifier.ABSTRACT;
 		}
-		return isAbstract;
+		return false;
 	}
 
 	private void checkIfExceptionIsNowChecked(JApiBehavior behavior) {
@@ -840,14 +843,6 @@ public JApiSuperclass callback(JApiClass clazz, Map<String, JApiClass> classMap,
 		checkIfAbstractMethodAdded(jApiClass, classMap);
 	}
 
-	private boolean isAbstract(JApiMethod jApiMethod) {
-		if (jApiMethod.getAbstractModifier().getNewModifier().isPresent()) {
-			AbstractModifier abstractModifier = jApiMethod.getAbstractModifier().getNewModifier().get();
-			return abstractModifier == AbstractModifier.ABSTRACT;
-		}
-		return false;
-	}
-
 	private boolean hasSameType(JApiField field, JApiField otherField) {
 		boolean hasSameNewType = false;
 		if (field.getType().getNewTypeOptional().isPresent() && otherField.getType().getNewTypeOptional().isPresent()) {
diff --git a/japicmp/src/main/java/japicmp/model/JApiModifier.java b/japicmp/src/main/java/japicmp/model/JApiModifier.java
@@ -67,9 +67,10 @@ public boolean hasChangedFrom(T oldValue) {
 
 	public boolean hasChangedTo(T value) {
 		boolean hasChangedTo = false;
-		if (newModifier.isPresent()) {
+		if (oldModifier.isPresent() && newModifier.isPresent()) {
 			T newValue = newModifier.get();
-			if (value == newValue) {
+			T oldValue = oldModifier.get();
+			if (value == newValue && oldValue != value) {
 				hasChangedTo = true;
 			}
 		}
diff --git a/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java b/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java
@@ -2214,14 +2214,16 @@ public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {
 			public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
 				CtClass ctInterfaceF = CtInterfaceBuilder.create().name("F").addToClassPool(classPool);
 				CtClass ctClassFImpl = CtClassBuilder.create().name("FImpl").implementsInterface(ctInterfaceF).addToClassPool(classPool);
-				CtClass ctClassC = CtClassBuilder.create().name("C").notPublicModifier().addToClassPool(classPool);
+				CtClass ctClassC = CtClassBuilder.create().name("C").addToClassPool(classPool);
 				CtMethodBuilder.create().publicAccess().staticAccess().name("M").returnType(ctClassFImpl).addToClass(ctClassC);
 				return Arrays.asList(ctInterfaceF, ctClassC);
 			}
 		});
 		JApiClass jApiClass = getJApiClass(jApiClasses, "C");
 		JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "M");
 		assertThat(jApiMethod.getChangeStatus(), is(JApiChangeStatus.MODIFIED));
-		assertThat(jApiMethod.getCompatibilityChanges(), hasItem(JApiCompatibilityChange.METHOD_RETURN_TYPE_CHANGED));
+		assertThat(jApiMethod.isBinaryCompatible(), is(true));
+		assertThat(jApiMethod.isSourceCompatible(), is(true));
+		assertThat(jApiMethod.getCompatibilityChanges().size(), is(0));
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -67,9 +67,10 @@ public boolean hasChangedFrom(T oldValue) {`
`67`	`67`
`68`	`68`	`public boolean hasChangedTo(T value) {`
`69`	`69`	`boolean hasChangedTo = false;`
`70`		`- if (newModifier.isPresent()) {`
	`70`	`+ if (oldModifier.isPresent() && newModifier.isPresent()) {`
`71`	`71`	`T newValue = newModifier.get();`
`72`		`- if (value == newValue) {`
	`72`	`+ T oldValue = oldModifier.get();`
	`73`	`+ if (value == newValue && oldValue != value) {`
`73`	`74`	`hasChangedTo = true;`
`74`	`75`	`}`
`75`	`76`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2214,14 +2214,16 @@ public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {`
`2214`	`2214`	`public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {`
`2215`	`2215`	`CtClass ctInterfaceF = CtInterfaceBuilder.create().name("F").addToClassPool(classPool);`
`2216`	`2216`	`CtClass ctClassFImpl = CtClassBuilder.create().name("FImpl").implementsInterface(ctInterfaceF).addToClassPool(classPool);`
`2217`		`- CtClass ctClassC = CtClassBuilder.create().name("C").notPublicModifier().addToClassPool(classPool);`
	`2217`	`+ CtClass ctClassC = CtClassBuilder.create().name("C").addToClassPool(classPool);`
`2218`	`2218`	`CtMethodBuilder.create().publicAccess().staticAccess().name("M").returnType(ctClassFImpl).addToClass(ctClassC);`
`2219`	`2219`	`return Arrays.asList(ctInterfaceF, ctClassC);`
`2220`	`2220`	`}`
`2221`	`2221`	`});`
`2222`	`2222`	`JApiClass jApiClass = getJApiClass(jApiClasses, "C");`
`2223`	`2223`	`JApiMethod jApiMethod = getJApiMethod(jApiClass.getMethods(), "M");`
`2224`	`2224`	`assertThat(jApiMethod.getChangeStatus(), is(JApiChangeStatus.MODIFIED));`
`2225`		`- assertThat(jApiMethod.getCompatibilityChanges(), hasItem(JApiCompatibilityChange.METHOD_RETURN_TYPE_CHANGED));`
	`2225`	`+ assertThat(jApiMethod.isBinaryCompatible(), is(true));`
	`2226`	`+ assertThat(jApiMethod.isSourceCompatible(), is(true));`
	`2227`	`+ assertThat(jApiMethod.getCompatibilityChanges().size(), is(0));`
`2226`	`2228`	`}`
`2227`	`2229`	`}`