Merge pull request #379 from guillermocalvo/volatile-modifier

Take `volatile` modifier into account
siom79 · Feb 18, 2024 · 566598d · 566598d
2 parents af2152a + d5a014d
commit 566598d
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 3 deletions.
diff --git a/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java b/japicmp/src/main/java/japicmp/compat/CompatibilityChanges.java
@@ -95,6 +95,13 @@ private void checkIfGenericTemplatesHaveChanged(JApiHasGenericTemplates jApiHasG
 
 	private void checkIfFieldsHaveChangedIncompatible(JApiClass jApiClass, Map<String, JApiClass> classMap) {
 		for (final JApiField field : jApiClass.getFields()) {
+			// section 8.3.1.4 of "Java Language Specification" SE7
+			if (field.getVolatileModifier().hasChangedFromTo(VolatileModifier.NON_VOLATILE, VolatileModifier.VOLATILE)) {
+				addCompatibilityChange(field, JApiCompatibilityChange.FIELD_NOW_VOLATILE);
+			}
+			if (field.getVolatileModifier().hasChangedFromTo(VolatileModifier.VOLATILE, VolatileModifier.NON_VOLATILE)) {
+				addCompatibilityChange(field, JApiCompatibilityChange.FIELD_NO_LONGER_VOLATILE);
+			}
 			// section 13.4.6 of "Java Language Specification" SE7
 			if (isNotPrivate(field) && field.getChangeStatus() == JApiChangeStatus.REMOVED) {
 				ArrayList<Integer> returnValues = new ArrayList<>();

diff --git a/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java b/japicmp/src/main/java/japicmp/model/JApiCompatibilityChange.java
@@ -52,8 +52,10 @@ public enum JApiCompatibilityChange {
 	FIELD_LESS_ACCESSIBLE_THAN_IN_SUPERCLASS(false, false, JApiSemanticVersionLevel.MAJOR),
 	FIELD_NOW_FINAL(false, false, JApiSemanticVersionLevel.MAJOR),
 	FIELD_NOW_TRANSIENT(true, true, JApiSemanticVersionLevel.PATCH),
+	FIELD_NOW_VOLATILE(true, true, JApiSemanticVersionLevel.PATCH),
 	FIELD_NOW_STATIC(false, false, JApiSemanticVersionLevel.MAJOR),
 	FIELD_NO_LONGER_TRANSIENT(true, true, JApiSemanticVersionLevel.PATCH),
+	FIELD_NO_LONGER_VOLATILE(true, true, JApiSemanticVersionLevel.PATCH),
 	FIELD_NO_LONGER_STATIC(false, false, JApiSemanticVersionLevel.MAJOR),
 	FIELD_TYPE_CHANGED(false, false, JApiSemanticVersionLevel.MAJOR),
 	FIELD_REMOVED(false, false, JApiSemanticVersionLevel.MAJOR),

diff --git a/japicmp/src/main/java/japicmp/model/JApiField.java b/japicmp/src/main/java/japicmp/model/JApiField.java
@@ -16,7 +16,7 @@
 import java.util.List;
 
 public class JApiField implements JApiHasChangeStatus, JApiHasModifiers, JApiHasAccessModifier, JApiHasStaticModifier,
-	JApiHasFinalModifier, JApiHasTransientModifier, JApiCompatibility, JApiHasAnnotations, JApiCanBeSynthetic,
+	JApiHasFinalModifier, JApiHasTransientModifier, JApiHasVolatileModifier, JApiCompatibility, JApiHasAnnotations, JApiCanBeSynthetic,
 	JApiHasGenericTypes {
 	private final JApiChangeStatus changeStatus;
 	private final JApiClass jApiClass;
@@ -27,6 +27,7 @@ public class JApiField implements JApiHasChangeStatus, JApiHasModifiers, JApiHas
 	private final JApiModifier<StaticModifier> staticModifier;
 	private final JApiModifier<FinalModifier> finalModifier;
 	private final JApiModifier<TransientModifier> transientModifier;
+	private final JApiModifier<VolatileModifier> volatileModifier;
 	private final JApiModifier<SyntheticModifier> syntheticModifier;
 	private final JApiAttribute<SyntheticAttribute> syntheticAttribute;
 	private final List<JApiCompatibilityChange> compatibilityChanges = new ArrayList<>();
@@ -43,6 +44,7 @@ public JApiField(JApiClass jApiClass, JApiChangeStatus changeStatus, Optional<Ct
 		this.staticModifier = extractStaticModifier(oldFieldOptional, newFieldOptional);
 		this.finalModifier = extractFinalModifier(oldFieldOptional, newFieldOptional);
 		this.transientModifier = extractTransientModifier(oldFieldOptional, newFieldOptional);
+		this.volatileModifier = extractVolatileModifier(oldFieldOptional, newFieldOptional);
 		this.syntheticModifier = extractSyntheticModifier(oldFieldOptional, newFieldOptional);
 		this.syntheticAttribute = extractSyntheticAttribute(oldFieldOptional, newFieldOptional);
 		this.type = extractType(oldFieldOptional, newFieldOptional);
@@ -107,6 +109,9 @@ private JApiChangeStatus evaluateChangeStatus(JApiChangeStatus changeStatus) {
 			if (this.transientModifier.getChangeStatus() != JApiChangeStatus.UNCHANGED) {
 				changeStatus = JApiChangeStatus.MODIFIED;
 			}
+			if (this.volatileModifier.getChangeStatus() != JApiChangeStatus.UNCHANGED) {
+				changeStatus = JApiChangeStatus.MODIFIED;
+			}
 			if (this.syntheticAttribute.getChangeStatus() != JApiChangeStatus.UNCHANGED) {
 				changeStatus = JApiChangeStatus.MODIFIED;
 			}
@@ -226,6 +231,20 @@ public TransientModifier getModifierForNew(CtField newField) {
 		});
 	}
 
+	private JApiModifier<VolatileModifier> extractVolatileModifier(Optional<CtField> oldFieldOptional, Optional<CtField> newFieldOptional) {
+		return ModifierHelper.extractModifierFromField(oldFieldOptional, newFieldOptional, new ModifierHelper.ExtractModifierFromFieldCallback<VolatileModifier>() {
+			@Override
+			public VolatileModifier getModifierForOld(CtField oldField) {
+				return Modifier.isVolatile(oldField.getModifiers()) ? VolatileModifier.VOLATILE : VolatileModifier.NON_VOLATILE;
+			}
+
+			@Override
+			public VolatileModifier getModifierForNew(CtField newField) {
+				return Modifier.isVolatile(newField.getModifiers()) ? VolatileModifier.VOLATILE : VolatileModifier.NON_VOLATILE;
+			}
+		});
+	}
+
 	private JApiModifier<SyntheticModifier> extractSyntheticModifier(Optional<CtField> oldFieldOptional, Optional<CtField> newFieldOptional) {
 		return ModifierHelper.extractModifierFromField(oldFieldOptional, newFieldOptional, new ModifierHelper.ExtractModifierFromFieldCallback<SyntheticModifier>() {
 			@Override
@@ -270,7 +289,7 @@ public Optional<CtField> getNewFieldOptional() {
 	@XmlElementWrapper(name = "modifiers")
 	@XmlElement(name = "modifier")
 	public List<? extends JApiModifier<? extends Enum<? extends Enum<?>>>> getModifiers() {
-		return Arrays.asList(this.accessModifier, this.staticModifier, this.finalModifier, this.transientModifier, this.syntheticModifier);
+		return Arrays.asList(this.accessModifier, this.staticModifier, this.finalModifier, this.transientModifier, this.volatileModifier, this.syntheticModifier);
 	}
 
 	@XmlTransient
@@ -288,6 +307,11 @@ public JApiModifier<TransientModifier> getTransientModifier() {
 		return transientModifier;
 	}
 
+	@XmlTransient
+	public JApiModifier<VolatileModifier> getVolatileModifier() {
+		return volatileModifier;
+	}
+
 	@XmlTransient
 	public JApiModifier<AccessModifier> getAccessModifier() {
 		return accessModifier;
@@ -377,6 +401,8 @@ public String toString()
 			+ finalModifier
 			+ ", transientModifier="
 			+ transientModifier
+			+ ", volatileModifier="
+			+ volatileModifier
 			+ ", syntheticModifier="
 			+ syntheticModifier
 			+ ", syntheticAttribute="

diff --git a/japicmp/src/main/java/japicmp/model/JApiHasVolatileModifier.java b/japicmp/src/main/java/japicmp/model/JApiHasVolatileModifier.java
@@ -0,0 +1,13 @@
+package japicmp.model;
+
+/**
+ * Implemented by all elements that can have a volatile modifier.
+ */
+public interface JApiHasVolatileModifier {
+	/**
+	 * Returns the volatile modifier.
+	 *
+	 * @return the volatile modifier
+	 */
+	JApiModifier<VolatileModifier> getVolatileModifier();
+}
diff --git a/japicmp/src/main/java/japicmp/model/VolatileModifier.java b/japicmp/src/main/java/japicmp/model/VolatileModifier.java
@@ -0,0 +1,8 @@
+package japicmp.model;
+
+/**
+ * Represents the volatile modifier.
+ */
+public enum VolatileModifier implements JApiModifierBase {
+	VOLATILE, NON_VOLATILE
+}
diff --git a/japicmp/src/main/java/japicmp/output/stdout/StdoutOutputGenerator.java b/japicmp/src/main/java/japicmp/output/stdout/StdoutOutputGenerator.java
@@ -427,7 +427,7 @@ private void processFieldChanges(StringBuilder sb, JApiClass jApiClass) {
 			sb.append(tabs(1)).append(signs(jApiField)).append(" ").append(jApiField.getChangeStatus()).append(" FIELD: ")
 				.append(accessModifierAsString(jApiField)).append(staticModifierAsString(jApiField))
 				.append(finalModifierAsString(jApiField)).append(transientModifierAsString(jApiField))
-				.append(syntheticModifierAsString(jApiField))
+				.append(volatileModifierAsString(jApiField)).append(syntheticModifierAsString(jApiField))
 				.append(fieldTypeChangeAsString(jApiField));
 			appendGenericTypes(sb, jApiField);
 			sb.append(" ").append(jApiField.getName()).append('\n');
@@ -450,6 +450,11 @@ private String transientModifierAsString(JApiHasTransientModifier hasTransientMo
 		return modifierAsString(modifier, TransientModifier.NON_TRANSIENT);
 	}
 
+	private String volatileModifierAsString(JApiHasVolatileModifier hasTransientModifier) {
+		JApiModifier<VolatileModifier> modifier = hasTransientModifier.getVolatileModifier();
+		return modifierAsString(modifier, VolatileModifier.NON_VOLATILE);
+	}
+
 	private String staticModifierAsString(JApiHasStaticModifier hasStaticModifier) {
 		JApiModifier<StaticModifier> modifier = hasStaticModifier.getStaticModifier();
 		return modifierAsString(modifier, StaticModifier.NON_STATIC);

diff --git a/japicmp/src/main/resources/html.xslt b/japicmp/src/main/resources/html.xslt
@@ -806,6 +806,9 @@
 			<xsl:when test="$modifier = 'NON_TRANSIENT'">
 				<xsl:if test="$changeStatus = 'MODIFIED'">not_transient</xsl:if>
 			</xsl:when>
+			<xsl:when test="$modifier = 'NON_VOLATILE'">
+				<xsl:if test="$changeStatus = 'MODIFIED'">not_volatile</xsl:if>
+			</xsl:when>
 			<xsl:when test="$modifier = 'NON_STATIC'">
 				<xsl:if test="$changeStatus = 'MODIFIED'">not_static</xsl:if>
 			</xsl:when>

diff --git a/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java b/japicmp/src/test/java/japicmp/compat/CompatibilityChangesTest.java
@@ -825,6 +825,36 @@ public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
 		assertThat(jApiField.isBinaryCompatible(), is(true));
 	}
 
+	@Test
+	public void testFieldNowVolatile() throws Exception {
+		JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
+		options.setIncludeSynthetic(true);
+		options.setAccessModifier(AccessModifier.PRIVATE);
+		List<JApiClass> jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() {
+			@Override
+			public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {
+				CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
+				CtFieldBuilder.create().type(CtClass.intType).name("field").addToClass(ctClass);
+				return Collections.singletonList(ctClass);
+			}
+
+			@Override
+			public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
+				CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
+				CtFieldBuilder.create().volatileAccess().type(CtClass.intType).name("field").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));
+		JApiField jApiField = getJApiField(jApiClass.getFields(), "field");
+		assertThat(jApiField.getCompatibilityChanges(), hasItem(JApiCompatibilityChange.FIELD_NOW_VOLATILE));
+		assertThat(jApiField.isBinaryCompatible(), is(true));
+		assertThat(jApiField.isBinaryCompatible(), is(true));
+	}
+
 	@Test
 	public void testFieldNowStatic() throws Exception {
 		JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
@@ -883,6 +913,36 @@ public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
 		assertThat(jApiField.isBinaryCompatible(), is(true));
 	}
 
+	@Test
+	public void testFieldNoLongerVolatile() throws Exception {
+		JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();
+		options.setIncludeSynthetic(true);
+		options.setAccessModifier(AccessModifier.PRIVATE);
+		List<JApiClass> jApiClasses = ClassesHelper.compareClasses(options, new ClassesHelper.ClassesGenerator() {
+			@Override
+			public List<CtClass> createOldClasses(ClassPool classPool) throws Exception {
+				CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
+				CtFieldBuilder.create().volatileAccess().type(CtClass.intType).name("field").addToClass(ctClass);
+				return Collections.singletonList(ctClass);
+			}
+
+			@Override
+			public List<CtClass> createNewClasses(ClassPool classPool) throws Exception {
+				CtClass ctClass = CtClassBuilder.create().name("japicmp.Test").addToClassPool(classPool);
+				CtFieldBuilder.create().type(CtClass.intType).name("field").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));
+		JApiField jApiField = getJApiField(jApiClass.getFields(), "field");
+		assertThat(jApiField.getCompatibilityChanges(), hasItem(JApiCompatibilityChange.FIELD_NO_LONGER_VOLATILE));
+		assertThat(jApiField.isBinaryCompatible(), is(true));
+		assertThat(jApiField.isBinaryCompatible(), is(true));
+	}
+
 	@Test
 	public void testFieldNoLongerStatic() throws Exception {
 		JarArchiveComparatorOptions options = new JarArchiveComparatorOptions();

diff --git a/japicmp/src/test/java/japicmp/util/CtFieldBuilder.java b/japicmp/src/test/java/japicmp/util/CtFieldBuilder.java
@@ -99,6 +99,11 @@ public CtFieldBuilder transientAccess() {
 		return this;
 	}
 
+	public CtFieldBuilder volatileAccess() {
+		this.modifier = this.modifier | Modifier.VOLATILE;
+		return this;
+	}
+
 	public CtFieldBuilder finalAccess() {
 		this.modifier = this.modifier | Modifier.FINAL;
 		return this;