From 581a3e890a291b912de5179da0f14628498648f0 Mon Sep 17 00:00:00 2001
From: Vladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Date: Wed, 17 Dec 2014 12:16:32 +0100
Subject: [PATCH] If tag is specified but nameAttribute is not, name attribute
 will not be added

---
 .../intellij/util/xmlb/OptionTagBinding.java  | 23 +++++++++++++------
 .../util/xmlb/annotations/OptionTag.java      |  4 ++++
 .../intellij/util/xmlb/XmlSerializerTest.java | 14 +++++++++++
 .../openapi/vcs/VcsConfiguration.java         |  4 ++--
 4 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/platform/util/src/com/intellij/util/xmlb/OptionTagBinding.java b/platform/util/src/com/intellij/util/xmlb/OptionTagBinding.java
index b11cd341a3da..de45f1b0af91 100644
--- a/platform/util/src/com/intellij/util/xmlb/OptionTagBinding.java
+++ b/platform/util/src/com/intellij/util/xmlb/OptionTagBinding.java
@@ -42,14 +42,15 @@ class OptionTagBinding extends BasePrimitiveBinding {
       myValueAttribute = Constants.VALUE;
     }
     else {
-      myNameAttribute = optionTag.nameAttribute();
       myValueAttribute = optionTag.valueAttribute();
 
       String tagName = optionTag.tag();
-      if (StringUtil.isEmpty(myNameAttribute) && Constants.OPTION.equals(tagName)) {
+      String nameAttribute = optionTag.nameAttribute();
+      if (StringUtil.isEmpty(nameAttribute) && Constants.OPTION.equals(tagName)) {
         tagName = myAccessor.getName();
       }
       myTagName = tagName;
+      myNameAttribute = tagName.equals(Constants.OPTION) || !nameAttribute.equals(Constants.NAME) ? StringUtil.nullize(nameAttribute) : null;
     }
   }
 
@@ -59,7 +60,7 @@ class OptionTagBinding extends BasePrimitiveBinding {
     Object value = myAccessor.read(o);
     Element targetElement = new Element(myTagName);
 
-    if (!StringUtil.isEmpty(myNameAttribute)) {
+    if (myNameAttribute != null) {
       targetElement.setAttribute(myNameAttribute, myName);
     }
 
@@ -119,14 +120,22 @@ class OptionTagBinding extends BasePrimitiveBinding {
 
   @Override
   public boolean isBoundTo(Object node) {
-    if (!(node instanceof Element)) return false;
+    if (!(node instanceof Element)) {
+      return false;
+    }
+
     Element e = (Element)node;
-    if (!e.getName().equals(myTagName)) return false;
+    if (!e.getName().equals(myTagName)) {
+      return false;
+    }
+
     String name = e.getAttributeValue(myNameAttribute);
-    if (StringUtil.isEmpty(myNameAttribute)) {
+    if (myNameAttribute == null) {
       return name == null || name.equals(myName);
     }
-    return name != null && name.equals(myName);
+    else {
+      return myName.equals(name);
+    }
   }
 
   @NonNls
diff --git a/platform/util/src/com/intellij/util/xmlb/annotations/OptionTag.java b/platform/util/src/com/intellij/util/xmlb/annotations/OptionTag.java
index 29eac3a31d13..792d0a5a1534 100644
--- a/platform/util/src/com/intellij/util/xmlb/annotations/OptionTag.java
+++ b/platform/util/src/com/intellij/util/xmlb/annotations/OptionTag.java
@@ -24,9 +24,13 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
+ * Please consider to use annotation parameters only to achieve backward compatibility. Otherwise feel free to file issues about serialization cosmetics.
+ *
  * <p>Store value in tag like {@code <option name="optionName" value="optionValue"/>}</p>
  * <p>nameAttribute can be empty, in which case it is skipped: {@code <option value="optionValue" />}</p>
  *
+ * If {@link #tag} is specified but {@link #nameAttribute} is not, name attribute will not be added: {@code <customTagName value="optionValue" />}
+ *
  * @author nik
  */
 @Retention(RetentionPolicy.RUNTIME)
diff --git a/platform/util/testSrc/com/intellij/util/xmlb/XmlSerializerTest.java b/platform/util/testSrc/com/intellij/util/xmlb/XmlSerializerTest.java
index eb81d4028f2d..7a88fe18db98 100644
--- a/platform/util/testSrc/com/intellij/util/xmlb/XmlSerializerTest.java
+++ b/platform/util/testSrc/com/intellij/util/xmlb/XmlSerializerTest.java
@@ -1245,6 +1245,20 @@ public class XmlSerializerTest extends TestCase {
     checkSmartSerialization(new Bean2(), "<Bean2 ac=\"2\" ab=\"32\" module=\"\" />");
   }
 
+  public void testIgnoreNameAttributeIfTagSpecified() throws IOException, JDOMException {
+    Bean3 bean = new Bean3();
+    bean.MOVE_TO_FAILED_COMMIT_CHANGELIST = "foo";
+    doSerializerTest("<bean>\n" +
+                     "  <confirmMoveToFailedCommit value=\"foo\" />\n" +
+                     "</bean>", bean, new SkipDefaultValuesSerializationFilters());
+  }
+
+  @Tag("bean")
+  static class Bean3 {
+    @OptionTag(tag = "confirmMoveToFailedCommit")
+    public String MOVE_TO_FAILED_COMMIT_CHANGELIST;
+  }
+
   private static void checkSmartSerialization(@NotNull Bean2 bean, @NotNull String serialized) throws IOException, JDOMException {
     SmartSerializer serializer = new SmartSerializer();
     serializer.readExternal(bean, JDOMUtil.loadDocument(serialized).getRootElement());
diff --git a/platform/vcs-api/src/com/intellij/openapi/vcs/VcsConfiguration.java b/platform/vcs-api/src/com/intellij/openapi/vcs/VcsConfiguration.java
index 0d2ad32a66ff..014597f226a9 100644
--- a/platform/vcs-api/src/com/intellij/openapi/vcs/VcsConfiguration.java
+++ b/platform/vcs-api/src/com/intellij/openapi/vcs/VcsConfiguration.java
@@ -63,9 +63,9 @@ public final class VcsConfiguration implements PersistentStateComponent<VcsConfi
   public boolean PERFORM_ADD_REMOVE_IN_BACKGROUND = true;
   public boolean PERFORM_ROLLBACK_IN_BACKGROUND = false;
   public volatile boolean CHECK_LOCALLY_CHANGED_CONFLICTS_IN_BACKGROUND = false;
-  @OptionTag(tag = "confirmMoveToFailedCommit", nameAttribute = "")
+  @OptionTag(tag = "confirmMoveToFailedCommit")
   public VcsShowConfirmationOption.Value MOVE_TO_FAILED_COMMIT_CHANGELIST = VcsShowConfirmationOption.Value.SHOW_CONFIRMATION;
-  @OptionTag(tag = "confirmRemoveEmptyChangelist", nameAttribute = "")
+  @OptionTag(tag = "confirmRemoveEmptyChangelist")
   public VcsShowConfirmationOption.Value REMOVE_EMPTY_INACTIVE_CHANGELISTS = VcsShowConfirmationOption.Value.SHOW_CONFIRMATION;
   public int CHANGED_ON_SERVER_INTERVAL = 60;
   public boolean SHOW_ONLY_CHANGED_IN_SELECTION_DIFF = true;