diff options
author | Joerg Kubitz | 2021-11-22 16:41:56 +0000 |
---|---|---|
committer | Sarika Sinha | 2022-01-10 07:26:54 +0000 |
commit | 4873edf0eeadeec0aace36f7c43fc17e762255fb (patch) | |
tree | 14b7ef385917b48b2e25ae0f2be89dc655ac5c22 | |
parent | 6a0eeacff8f748062ff1992334e5c5ad24ff5a70 (diff) | |
download | eclipse.platform.debug-4873edf0eeadeec0aace36f7c43fc17e762255fb.tar.gz eclipse.platform.debug-4873edf0eeadeec0aace36f7c43fc17e762255fb.tar.xz eclipse.platform.debug-4873edf0eeadeec0aace36f7c43fc17e762255fb.zip |
Bug 577400 - fix RefreshScopeComparator Comparator contractY20220111-0600Y20220110-0600I20220112-0210I20220111-2130I20220111-1910I20220111-1800I20220111-0450I20220110-1800I20220110-0550I20220110-0310
RefreshScopeComparator and SourceLocatorMementoComparator where not null
safe and did not yield
compare(x,y)=-compare(y,x) as it never did return 1, but -1.
Change-Id: I6af76fc4abe94fc461f56cdbb05a09431cddd382
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/187986
Tested-by: Platform Bot <platform-bot@eclipse.org>
Reviewed-by: Sarika Sinha <sarika.sinha@in.ibm.com>
3 files changed, 47 insertions, 18 deletions
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/RefreshScopeComparator.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/RefreshScopeComparator.java index a875896d8..3a88dc34e 100644 --- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/RefreshScopeComparator.java +++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/RefreshScopeComparator.java @@ -13,6 +13,7 @@ *******************************************************************************/ package org.eclipse.debug.internal.core; +import java.util.Arrays; import java.util.Comparator; import org.eclipse.core.resources.IResource; @@ -27,26 +28,21 @@ import org.eclipse.debug.core.RefreshUtil; * @since 3.6 */ public class RefreshScopeComparator implements Comparator<String> { - - @Override - public int compare(String o1, String o2) { - String m1 = o1; - String m2 = o2; + private static IResource[] toResources(String memento) { try { - IResource[] r1 = RefreshUtil.toResources(m1); - IResource[] r2 = RefreshUtil.toResources(m2); - if (r1.length == r2.length) { - for (int i = 0; i < r2.length; i++) { - if (!r1[i].equals(r2[i])) { - return -1; - } - } - return 0; - } + return RefreshUtil.toResources(memento); } catch (CoreException e) { - return -1; + return null; } - return -1; + } + + private static final Comparator<IResource> RESOURCE = Comparator.nullsFirst(Comparator.comparing(r -> r.toString())); + private static final Comparator<IResource[]> ARRAY = Comparator.nullsFirst((IResource[] s1, IResource[] s2) -> Arrays.compare(s1, s2, RESOURCE)); + private static final Comparator<String> MEMENTO = Comparator.nullsFirst(Comparator.comparing(m -> toResources(m), ARRAY)); + + @Override + public int compare(String o1, String o2) { + return MEMENTO.compare(o1, o2); } } diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/sourcelookup/SourceLocatorMementoComparator.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/sourcelookup/SourceLocatorMementoComparator.java index 1be76866d..5ec80ab85 100644 --- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/sourcelookup/SourceLocatorMementoComparator.java +++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/sourcelookup/SourceLocatorMementoComparator.java @@ -24,6 +24,15 @@ public class SourceLocatorMementoComparator implements Comparator<String> { @Override public int compare(String o1, String o2) { + if (o1 == null && o2 == null) { + return 0; + } + if (o1 == null) { + return -1; + } + if (o2 == null) { + return 1; + } String m1 = o1; String m2 = o2; int i1 = 0, i2 = 0; @@ -32,7 +41,7 @@ public class SourceLocatorMementoComparator implements Comparator<String> { i2 = skipWhitespace(m2, i2); if (i1 < m1.length() && i2 < m2.length()) { if (m1.charAt(i1) != m2.charAt(i2)) { - return -1; + return m1.charAt(i1) - m2.charAt(i2); } i1++; i2++; @@ -40,6 +49,9 @@ public class SourceLocatorMementoComparator implements Comparator<String> { if (i2 < m2.length()) { return -1; } + if (i1 < m1.length()) { + return 1; + } return 0; } } diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/RefreshTabTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/RefreshTabTests.java index 0c2c14286..669bd0aae 100644 --- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/RefreshTabTests.java +++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/RefreshTabTests.java @@ -27,6 +27,7 @@ import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; import org.eclipse.debug.core.RefreshUtil; import org.eclipse.debug.internal.core.RefreshScopeComparator; +import org.eclipse.debug.internal.core.sourcelookup.SourceLocatorMementoComparator; import org.eclipse.debug.tests.TestsPlugin; import org.eclipse.debug.ui.RefreshTab; import org.eclipse.jface.viewers.ISelectionProvider; @@ -180,11 +181,30 @@ public class RefreshTabTests extends AbstractLaunchTest { * * @throws CoreException */ + @SuppressWarnings("restriction") @Test public void testRefreshScopeComparator() throws CoreException { String oldStyle = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<launchConfigurationWorkingSet factoryID=\"org.eclipse.ui.internal.WorkingSetFactory\" name=\"workingSet\" editPageId=\"org.eclipse.ui.resourceWorkingSetPage\">\n<item factoryID=\"org.eclipse.ui.internal.model.ResourceFactory\" path=\"/RefreshTabTests/some.file\" type=\"1\"/>\n</launchConfigurationWorkingSet>}"; //$NON-NLS-1$ String newStyle = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<resources>\n<item path=\"/RefreshTabTests/some.file\" type=\"1\"/>\n</resources>}"; //$NON-NLS-1$ assertEquals("Comparator should return 0", 0, new RefreshScopeComparator().compare(oldStyle, newStyle)); //$NON-NLS-1$ + String s1 = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<resources>\n<item path=\"/RefreshTabTests/some.file1\" type=\"1\"/>\n</resources>}"; //$NON-NLS-1$ + String s2 = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<resources>\n<item path=\"/RefreshTabTests/some.file2\" type=\"1\"/>\n</resources>}"; //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new RefreshScopeComparator().compare(s1, s1)); //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new RefreshScopeComparator().compare(s2, s2)); //$NON-NLS-1$ + assertEquals("Comparator should return -1", -1, new RefreshScopeComparator().compare(s1, s2)); //$NON-NLS-1$ + assertEquals("Comparator should return 1", 1, new RefreshScopeComparator().compare(s2, s1)); //$NON-NLS-1$ + assertEquals("Comparator should return 1", 1, new RefreshScopeComparator().compare(s1, null)); //$NON-NLS-1$ + assertEquals("Comparator should return -1", -1, new RefreshScopeComparator().compare(null, s1)); //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new RefreshScopeComparator().compare(null, null)); //$NON-NLS-1$ + String o1 = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<launchConfigurationWorkingSet factoryID=\"org.eclipse.ui.internal.WorkingSetFactory\" name=\"workingSet\" editPageId=\"org.eclipse.ui.resourceWorkingSetPage\">\n<item factoryID=\"org.eclipse.ui.internal.model.ResourceFactory\" path=\"/RefreshTabTests/some.file1\" type=\"1\"/>\n</launchConfigurationWorkingSet>}"; //$NON-NLS-1$ + String o2 = "${working_set:<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<launchConfigurationWorkingSet factoryID=\"org.eclipse.ui.internal.WorkingSetFactory\" name=\"workingSet\" editPageId=\"org.eclipse.ui.resourceWorkingSetPage\">\n<item factoryID=\"org.eclipse.ui.internal.model.ResourceFactory\" path=\"/RefreshTabTests/some.file2\" type=\"1\"/>\n</launchConfigurationWorkingSet>}"; //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new SourceLocatorMementoComparator().compare(o1, o1)); //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new SourceLocatorMementoComparator().compare(o2, o2)); //$NON-NLS-1$ + assertEquals("Comparator should return -1", -1, new SourceLocatorMementoComparator().compare(o1, o2)); //$NON-NLS-1$ + assertEquals("Comparator should return 1", 1, new SourceLocatorMementoComparator().compare(o2, o1)); //$NON-NLS-1$ + assertEquals("Comparator should return 1", 1, new SourceLocatorMementoComparator().compare(o1, null)); //$NON-NLS-1$ + assertEquals("Comparator should return -1", -1, new SourceLocatorMementoComparator().compare(null, o1)); //$NON-NLS-1$ + assertEquals("Comparator should return 0", 0, new SourceLocatorMementoComparator().compare(null, null)); //$NON-NLS-1$ } /** @@ -197,6 +217,7 @@ public class RefreshTabTests extends AbstractLaunchTest { IResource[] resources = new IResource[] { getProject(), getProject().getFile("not.exist"), getProject().getFile("some.file") }; //$NON-NLS-1$ //$NON-NLS-2$ String memento = RefreshUtil.toMemento(resources); IResource[] restore = RefreshUtil.toResources(memento); + assertNotNull(resources); assertEquals(resources.length, restore.length); assertEquals(resources[0], restore[0]); assertEquals(resources[1], restore[1]); |