From a9822b839365976a44a2a0e062fdd34a73125bc1 Mon Sep 17 00:00:00 2001 From: Michael Bien Date: Thu, 3 Apr 2025 20:12:02 +0200 Subject: [PATCH] [NotifyDescriptor] Refactor return values to not rely on Integer identity. - return values must mimic Integer instances and their equals contracts - unit test used the wrong return value (int instead of Object) --- .../nbproject/project.properties | 2 +- .../src/org/openide/NotifyDescriptor.java | 37 ++++++++++++++++--- .../src/org/openide/NotifyDescriptorTest.java | 32 ++++++++++++++++ .../ExternalChangeOfModifiedFileTest.java | 2 +- 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/platform/openide.dialogs/nbproject/project.properties b/platform/openide.dialogs/nbproject/project.properties index 933c72c90abb..09309e766d13 100644 --- a/platform/openide.dialogs/nbproject/project.properties +++ b/platform/openide.dialogs/nbproject/project.properties @@ -17,7 +17,7 @@ is.autoload=true javac.compilerargs=-Xlint:unchecked -javac.source=1.8 +javac.release=17 #javadoc.main.page=org/openide/doc-files/api.html javadoc.arch=${basedir}/arch.xml javadoc.apichanges=${basedir}/apichanges.xml diff --git a/platform/openide.dialogs/src/org/openide/NotifyDescriptor.java b/platform/openide.dialogs/src/org/openide/NotifyDescriptor.java index 263ce9383694..c4c017e66841 100644 --- a/platform/openide.dialogs/src/org/openide/NotifyDescriptor.java +++ b/platform/openide.dialogs/src/org/openide/NotifyDescriptor.java @@ -27,6 +27,7 @@ import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.beans.PropertyChangeSupport; +import java.io.Serializable; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; @@ -129,24 +130,50 @@ public class NotifyDescriptor extends Object { */ public static final String PROP_INFO_NOTIFICATION = "infoNotification"; // NOI18N + /** + * Used to be a new Integer(JOptionPane.FOO_OPTION) instance. + * + * For compatibility reasons the public Object constants must have distinct + * identity, but equals must behave like Integer's equals and compare only + * the wrapped int like a Record. + */ + private record ReturnValue(int option) implements Serializable, Comparable { + + @Override + public String toString() { + return String.valueOf(option); + } + + @Override + public int hashCode() { + return Integer.hashCode(option); + } + + @Override + public int compareTo(ReturnValue other) { + return Integer.compare(option, other.option); + } + + } + // // Return values // /** Return value if YES is chosen. */ - public static final Object YES_OPTION = new Integer(JOptionPane.YES_OPTION); + public static final Object YES_OPTION = new ReturnValue(JOptionPane.YES_OPTION); /** Return value if NO is chosen. */ - public static final Object NO_OPTION = new Integer(JOptionPane.NO_OPTION); + public static final Object NO_OPTION = new ReturnValue(JOptionPane.NO_OPTION); /** Return value if CANCEL is chosen. */ - public static final Object CANCEL_OPTION = new Integer(JOptionPane.CANCEL_OPTION); + public static final Object CANCEL_OPTION = new ReturnValue(JOptionPane.CANCEL_OPTION); /** Return value if OK is chosen. */ - public static final Object OK_OPTION = new Integer(JOptionPane.OK_OPTION); + public static final Object OK_OPTION = new ReturnValue(JOptionPane.OK_OPTION); /** Return value if user closes the window without pressing any button. */ - public static final Object CLOSED_OPTION = new Integer(JOptionPane.CLOSED_OPTION); + public static final Object CLOSED_OPTION = new ReturnValue(JOptionPane.CLOSED_OPTION); // // Option types diff --git a/platform/openide.dialogs/test/unit/src/org/openide/NotifyDescriptorTest.java b/platform/openide.dialogs/test/unit/src/org/openide/NotifyDescriptorTest.java index cdbfe948128d..fd95ec711ea5 100644 --- a/platform/openide.dialogs/test/unit/src/org/openide/NotifyDescriptorTest.java +++ b/platform/openide.dialogs/test/unit/src/org/openide/NotifyDescriptorTest.java @@ -21,7 +21,12 @@ import java.awt.GraphicsEnvironment; import java.beans.PropertyChangeEvent; import java.util.ArrayList; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import javax.swing.*; import javax.swing.text.BadLocationException; import junit.framework.Test; @@ -134,4 +139,31 @@ JTextField createTextField() { events.clear(); } + + // post migration from boxed type constructors + public void testVerifyReturnOptionCompatibility() { + // used to be new Integer(int) + Object YES_OPTION = NotifyDescriptor.YES_OPTION; + Object NO_OPTION = NotifyDescriptor.NO_OPTION; + Object CANCEL_OPTION = NotifyDescriptor.CANCEL_OPTION; + Object OK_OPTION = NotifyDescriptor.OK_OPTION; + Object CLOSED_OPTION = NotifyDescriptor.CLOSED_OPTION; + + // all distinct instances + Set identitySet = Collections.newSetFromMap(new IdentityHashMap<>()); + identitySet.add(YES_OPTION); + identitySet.add(NO_OPTION); + identitySet.add(CANCEL_OPTION); + identitySet.add(OK_OPTION); + identitySet.add(CLOSED_OPTION); + assertEquals(5, identitySet.size()); + + // same int value + assertEquals(YES_OPTION, OK_OPTION); + + // distinct int value + assertNotSame(YES_OPTION, NO_OPTION); + assertNotSame(YES_OPTION, CANCEL_OPTION); + assertNotSame(YES_OPTION, CLOSED_OPTION); + } } diff --git a/platform/openide.loaders/test/unit/src/org/openide/text/ExternalChangeOfModifiedFileTest.java b/platform/openide.loaders/test/unit/src/org/openide/text/ExternalChangeOfModifiedFileTest.java index 018cb6d7a682..1296af6a7568 100644 --- a/platform/openide.loaders/test/unit/src/org/openide/text/ExternalChangeOfModifiedFileTest.java +++ b/platform/openide.loaders/test/unit/src/org/openide/text/ExternalChangeOfModifiedFileTest.java @@ -132,7 +132,7 @@ public void testModifyTheFileAndThenPreventItToBeSavedOnFileDisappear() throws E assertNotNull("Panes are still open", arr); assertTrue("Document is remains modified", edit.isModified()); - DD.toReturn.push(DialogDescriptor.YES_NO_OPTION); + DD.toReturn.push(DialogDescriptor.YES_OPTION); SaveCookie sc = obj.getLookup().lookup(SaveCookie.class); assertNotNull("File is modified and has save cookie", sc);