IDEA-CR-15578 final fix `rename A to B and B to A`

diff --git a/platform/configuration-store-impl/src/SchemeManagerImpl.kt b/platform/configuration-store-impl/src/SchemeManagerImpl.kt
index f027b546415cef6c5e83a9ca82039a9ae93d22fc..bb4a1eb1da087af436f786878e11945e2a6089fd 100644 (file)
--- a/platform/configuration-store-impl/src/SchemeManagerImpl.kt
+++ b/platform/configuration-store-impl/src/SchemeManagerImpl.kt
@@ -588,19 +588,15 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
     }
 
     val newDigest = element!!.digest()
-    if (externalInfo != null && currentFileNameWithoutExtension === fileNameWithoutExtension && externalInfo.isDigestEquals(newDigest)) {
-      return
-    }
-
-    // save only if scheme differs from bundled
-    if (isEqualToBundledScheme(externalInfo, newDigest, scheme)) {
-      return
-    }
+    when {
+      externalInfo != null && currentFileNameWithoutExtension === fileNameWithoutExtension && externalInfo.isDigestEquals(newDigest) -> return
+      isEqualToBundledScheme(externalInfo, newDigest, scheme) -> return
 
-    // we must check it only here to avoid delete old scheme just because it is empty (old idea save -> new idea delete on open)
-    if (processor is LazySchemeProcessor && processor.isSchemeDefault(scheme, newDigest)) {
-      externalInfo?.scheduleDelete()
-      return
+      // we must check it only here to avoid delete old scheme just because it is empty (old idea save -> new idea delete on open)
+      processor is LazySchemeProcessor && processor.isSchemeDefault(scheme, newDigest) -> {
+        externalInfo?.scheduleDelete()
+        return
+      }
     }
 
     val fileName = fileNameWithoutExtension!! + schemeExtension
@@ -621,8 +617,8 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
       providerPath = null
     }
 
-    // if another new scheme uses old name of this scheme, so, we must not delete it (as part of rename operation)
-    val renamed = externalInfo != null && fileNameWithoutExtension !== currentFileNameWithoutExtension && nameGenerator.value(currentFileNameWithoutExtension)
+    // if another new scheme uses old name of this scheme, we must not delete it (as part of rename operation)
+    val renamed = externalInfo != null && fileNameWithoutExtension !== currentFileNameWithoutExtension && nameGenerator.isUnique(currentFileNameWithoutExtension)
     if (providerPath == null) {
       if (useVfs) {
         var file: VirtualFile? = null
@@ -633,10 +629,15 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
         }
 
         if (renamed) {
-          file = dir.findChild(externalInfo!!.fileName)
-          if (file != null) {
-            runWriteAction {
-              file!!.rename(this, fileName)
+          val oldFile = dir.findChild(externalInfo!!.fileName)
+          oldFile?.let {
+            // VFS doesn't allow to rename to existing file, so, check it
+            if (dir!!.findChild(fileName) == null) {
+              runWriteAction { it.rename(this, fileName) }
+              file = oldFile
+            }
+            else {
+              externalInfo!!.scheduleDelete()
             }
           }
         }
@@ -646,9 +647,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
         }
 
         runWriteAction {
-          file!!.getOutputStream(this).use {
-            byteOut.writeTo(it)
-          }
+          file!!.getOutputStream(this).use { byteOut.writeTo(it) }
         }
       }
       else {

diff --git a/platform/configuration-store-impl/testSrc/SchemeManagerTest.kt b/platform/configuration-store-impl/testSrc/SchemeManagerTest.kt
index c7c2ecc5e8a74eb13678b32408b5995519dcbf0b..ef90a4e2e25efdc1cd13b9bfa5610f6a661efff1 100644 (file)
--- a/platform/configuration-store-impl/testSrc/SchemeManagerTest.kt
+++ b/platform/configuration-store-impl/testSrc/SchemeManagerTest.kt
@@ -15,6 +15,7 @@
  */
 package com.intellij.configurationStore
 
+import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.options.ExternalizableScheme
 import com.intellij.openapi.options.SchemeManagerFactory
 import com.intellij.openapi.util.io.FileUtil
@@ -22,9 +23,11 @@ import com.intellij.openapi.util.text.StringUtil
 import com.intellij.testFramework.PlatformTestUtil
 import com.intellij.testFramework.ProjectRule
 import com.intellij.testFramework.TemporaryDirectory
+import com.intellij.testFramework.runInEdtAndWait
 import com.intellij.util.SmartList
 import com.intellij.util.io.createDirectories
 import com.intellij.util.io.directoryStreamIfExists
+import com.intellij.util.io.readText
 import com.intellij.util.io.write
 import com.intellij.util.lang.CompoundRuntimeException
 import com.intellij.util.loadElement
@@ -310,6 +313,48 @@ internal class SchemeManagerTest {
     assertThat(dir.resolve("s2.xml")).isRegularFile()
   }
 
+  @Test fun `rename A to B and B to A`() {
+    val dir = tempDirManager.newPath()
+    val schemeManager = createSchemeManager(dir)
+
+    val a = TestScheme("a", "a")
+    val b = TestScheme("b", "b")
+    schemeManager.setSchemes(listOf(a, b))
+    schemeManager.save()
+
+    assertThat(dir.resolve("a.xml")).isRegularFile()
+    assertThat(dir.resolve("b.xml")).isRegularFile()
+
+    a.name = "b"
+    b.name = "a"
+
+    schemeManager.save()
+
+    assertThat(dir.resolve("a.xml").readText()).isEqualTo("""<scheme name="a" data="b" />""")
+    assertThat(dir.resolve("b.xml").readText()).isEqualTo("""<scheme name="b" data="a" />""")
+  }
+
+  @Test fun `VFS - rename A to B and B to A`() {
+    val dir = tempDirManager.newPath()
+    val schemeManager = SchemeManagerImpl(FILE_SPEC, TestSchemesProcessor(), null, dir, messageBus = ApplicationManager.getApplication().messageBus)
+
+    val a = TestScheme("a", "a")
+    val b = TestScheme("b", "b")
+    schemeManager.setSchemes(listOf(a, b))
+    runInEdtAndWait { schemeManager.save() }
+
+    assertThat(dir.resolve("a.xml")).isRegularFile()
+    assertThat(dir.resolve("b.xml")).isRegularFile()
+
+    a.name = "b"
+    b.name = "a"
+
+    runInEdtAndWait { schemeManager.save() }
+
+    assertThat(dir.resolve("a.xml").readText()).isEqualTo("""<scheme name="a" data="b" />""")
+    assertThat(dir.resolve("b.xml").readText()).isEqualTo("""<scheme name="b" data="a" />""")
+  }
+
   @Test fun `path must not contains ROOT_CONFIG macro`() {
     assertThatThrownBy({ SchemeManagerFactory.getInstance().create("\$ROOT_CONFIG$/foo", TestSchemesProcessor()) }).hasMessage("Path must not contains ROOT_CONFIG macro, corrected: foo")
   }

diff --git a/platform/lang-impl/src/com/intellij/profile/codeInspection/ui/SingleInspectionProfilePanel.java b/platform/lang-impl/src/com/intellij/profile/codeInspection/ui/SingleInspectionProfilePanel.java
index 0ddf1995966ae3841f6d2e8a386ecc87de7f7a4a..6163296359496d6824806d101e27515c059bea72 100644 (file)
--- a/platform/lang-impl/src/com/intellij/profile/codeInspection/ui/SingleInspectionProfilePanel.java
+++ b/platform/lang-impl/src/com/intellij/profile/codeInspection/ui/SingleInspectionProfilePanel.java
@@ -1113,8 +1113,7 @@ public class SingleInspectionProfilePanel extends JPanel {
     BaseInspectionProfileManager profileManager = selectedProfile.isProjectLevel() ? myProjectProfileManager : (BaseInspectionProfileManager)InspectionProfileManager.getInstance();
     InspectionProfileImpl source = selectedProfile.getSource();
 
-    // delete by instance, only if from another profile manager or has another name (otherwise will be replaced and we don't need to explicitly delete it)
-    if (source.getProfileManager() != profileManager || !source.getName().equals(selectedProfile.getName())) {
+    if (source.getProfileManager() != profileManager) {
       source.getProfileManager().deleteProfile(source);
     }
 

diff --git a/platform/util/src/com/intellij/util/text/UniqueNameGenerator.java b/platform/util/src/com/intellij/util/text/UniqueNameGenerator.java
index 78c519caf08c8a94e762c9940ba12a138371ac95..70b70c485e7b4c221bac1ebd1cf2edbdf5b68434 100644 (file)
--- a/platform/util/src/com/intellij/util/text/UniqueNameGenerator.java
+++ b/platform/util/src/com/intellij/util/text/UniqueNameGenerator.java
@@ -40,7 +40,11 @@ public class UniqueNameGenerator implements Condition<String> {
   }
 
   @Override
-  public final boolean value(final String candidate) {
+  public final boolean value(String candidate) {
+    return isUnique(candidate);
+  }
+
+  public final boolean isUnique(String candidate) {
     return !myExistingNames.contains(candidate);
   }