From 785cb1b76b1f6464420e488424aed98ea8ece34f Mon Sep 17 00:00:00 2001 From: Dmitry Katsubo Date: Mon, 9 Mar 2015 18:13:47 +0100 Subject: [PATCH] If moving of inner class to parent collides with another class, its name is prepended with candidate class name (fixes #39). --- README.md | 4 + .../addon/xew/XmlElementWrapperPlugin.java | 66 +-- .../element_name_collision/ObjectFactory.java | 64 +++ .../element_name_collision/Root.java | 385 ++++++++++++++++++ .../element_name_collision/package-info.java | 2 + .../xew/XmlElementWrapperPluginTest.java | 7 + .../xjc/addon/xew/element-name-collision.xml | 20 + .../xjc/addon/xew/element-name-collision.xsd | 58 +++ 8 files changed, 579 insertions(+), 27 deletions(-) create mode 100644 src/test/generated_resources/element_name_collision/ObjectFactory.java create mode 100644 src/test/generated_resources/element_name_collision/Root.java create mode 100644 src/test/generated_resources/element_name_collision/package-info.java create mode 100644 src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xml create mode 100644 src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xsd diff --git a/README.md b/README.md index fad5259..7e6b633 100644 --- a/README.md +++ b/README.md @@ -351,6 +351,10 @@ You can find more examples of this plugin in [`samples`](samples/) directory (in ## What's new +### v1.5 (future release) + +* Bugs fixed ([#39](https://github.com/dmak/jaxb-xew-plugin/issues/39), [#40](https://github.com/dmak/jaxb-xew-plugin/issues/32), [#33](https://github.com/dmak/jaxb-xew-plugin/issues/40)). + ### v1.4 * Bugs fixed ([#21](https://github.com/dmak/jaxb-xew-plugin/issues/21), [#32](https://github.com/dmak/jaxb-xew-plugin/issues/32), [#33](https://github.com/dmak/jaxb-xew-plugin/issues/33)). diff --git a/src/main/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPlugin.java b/src/main/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPlugin.java index c4813e6..5370e42 100644 --- a/src/main/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPlugin.java +++ b/src/main/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPlugin.java @@ -437,14 +437,14 @@ private void runInternal(Outline outline) throws IOException, ClassNotFoundExcep for (Candidate c : candidatesMap.values()) { // Skip fields with basic types as for example any class can be casted to Object. - if (fieldType.isAssignableFrom(c.getCandidateClass()) && !isHiddenClass(fieldType)) { + if (fieldType.isAssignableFrom(c.getClazz()) && !isHiddenClass(fieldType)) { // If the given field has type T, it cannot be also in the list of parametrisations (e.g. T). candidate = c; break; } // If the candidate T is referred from list of parametrisations (e.g. List), it cannot be removed. // However field substitutions will take place. - else if (isListedAsParametrisation(c.getCandidateClass(), fieldType)) { + else if (isListedAsParametrisation(c.getClazz(), fieldType)) { logger.debug("Candidate " + c.getClassName() + " is listed as parametrisation of " + targetClass.fullName() + "#" + fieldName + " and hence won't be removed."); c.unmarkForRemoval(); @@ -705,11 +705,11 @@ && deleteSettersGetters((JDefinedClass) interfaceClass, propertyName)) { } /** - * If candidate class contains the inner class, which will be referred from collection, then this inner class has to - * be moved to top class. For example from
- * {@code TopClass -> ContainerClass (marked for removal) -> ElementClass}
+ * If candidate class contains the inner class which is collection parametrisation (type), then this inner class has + * to be moved to top class. For example from
+ * {@code TypeClass (is a collection type) -> ContainerClass (marked for removal) -> ElementClass}
* we need to get
- * {@code TopClass (will have a collection) -> ElementClass}.
+ * {@code TypeClass -> ElementClass}.
* Also this move should be reflected on factory method names. */ private boolean moveInnerClassToParent(Outline outline, Candidate candidate) { @@ -720,7 +720,8 @@ private boolean moveInnerClassToParent(Outline outline, Candidate candidate) { JDefinedClass fieldParametrisationImpl = candidate.getFieldParametrisationImpl(); - if (candidate.getCandidateClass() != fieldParametrisationImpl.parentContainer()) { + if (candidate.getClazz() != fieldParametrisationImpl.parentContainer()) { + // Field parametrisation class is not inner class of the candidate: return false; } @@ -975,7 +976,7 @@ private int deleteCandidates(Outline outline, Collection candidates) } // Get the defined class for candidate class. - JDefinedClass candidateClass = candidate.getCandidateClass(); + JDefinedClass candidateClass = candidate.getClazz(); deletionCount += deleteFactoryMethod(candidate.getValueObjectFactoryClass(), candidate); @@ -1038,8 +1039,8 @@ private int deleteFactoryMethod(JDefinedClass factoryClass, Candidate candidate) // * @XmlElementDecl(..., scope = X.class) // public JAXBElement createT...(T value) { return new JAXBElement<...>(QNAME, T.class, X.class, value); } if ((method.type() instanceof JDefinedClass && ((JDefinedClass) method.type()).isAssignableFrom(candidate - .getCandidateClass())) - || isListedAsParametrisation(candidate.getCandidateClass(), method.type()) + .getClazz())) + || isListedAsParametrisation(candidate.getClazz(), method.type()) || candidate.getScopedElementInfos().containsKey(method.name())) { writeSummary("\tRemoving factory method [" + method.type().fullName() + "#" + method.name() + "()] from " + factoryClass.fullName()); @@ -1075,30 +1076,32 @@ private boolean deleteSettersGetters(JDefinedClass clazz, String fieldPublicName } /** - * Move the given class to his grandparent (either class or package). + * Move the given class to his grandparent (either class or package). The given {@code clazz} should be inner class. */ @SuppressWarnings("unchecked") private void moveClassLevelUp(Outline outline, JDefinedClass clazz) { // Modify the container so it now refers the class. Container can be a class or package. - JClassContainer container = clazz.parentContainer().parentContainer(); + JDefinedClass parent = (JDefinedClass) clazz.parentContainer(); + JClassContainer grandParent = parent.parentContainer(); + Map classes; // FIXME: Pending https://java.net/jira/browse/JAXB-957 - if (container.isClass()) { + if (grandParent.isClass()) { // Element class should be added as its container child: - JDefinedClass parentClass = (JDefinedClass) container; + JDefinedClass grandParentClass = (JDefinedClass) grandParent; - writeSummary("\tMoving inner class " + clazz.fullName() + " to class " + parentClass.fullName()); + writeSummary("\tMoving inner class " + clazz.fullName() + " to class " + grandParentClass.fullName()); - ((Map) getPrivateField(parentClass, "classes")).put(clazz.name(), clazz); + classes = ((Map) getPrivateField(grandParentClass, "classes")); } else { - JPackage parentPackage = (JPackage) container; + JPackage grandParentPackage = (JPackage) grandParent; - writeSummary("\tMoving inner class " + clazz.fullName() + " to package " + parentPackage.name()); + writeSummary("\tMoving inner class " + clazz.fullName() + " to package " + grandParentPackage.name()); - ((Map) getPrivateField(parentPackage, "classes")).put(clazz.name(), clazz); + classes = ((Map) getPrivateField(grandParentPackage, "classes")); - // In this scenario class should have "static" modifier reset: + // In this scenario class should have "static" modifier reset otherwise it won't compile: setPrivateField(clazz.mods(), "mods", Integer.valueOf(clazz.mods().getValue() & ~JMod.STATIC)); for (ClassOutline classOutline : outline.getClasses()) { @@ -1109,12 +1112,21 @@ private void moveClassLevelUp(Outline outline, JDefinedClass clazz) { assert (sc instanceof XSDeclaration && ((XSDeclaration) sc).isLocal()); setPrivateField(sc, "anonymous", Boolean.FALSE); + + break; } } } + if (classes.containsKey(clazz.name())) { + writeSummary("\tRenaming class " + clazz.fullName() + " to class " + parent.name() + clazz.name()); + setPrivateField(clazz, "name", parent.name() + clazz.name()); + } + + classes.put(clazz.name(), clazz); + // Finally modify the class so that it refers back the container: - setPrivateField(clazz, "outer", container); + setPrivateField(clazz, "outer", grandParent); } /** @@ -1307,7 +1319,7 @@ private static class Candidate { /** * Container class */ - public JDefinedClass getCandidateClass() { + public JDefinedClass getClazz() { return candidateClass; } @@ -1319,7 +1331,7 @@ public String getClassName() { } /** - * The only field in container class. + * The only field in container class (collection property). */ public JFieldVar getField() { return field; @@ -1333,7 +1345,7 @@ public String getFieldName() { } /** - * The class of the only field in container class. + * The class of the only field in container class (collection interface or concrete implementation). */ public JClass getFieldClass() { return (JClass) field.type(); @@ -1347,15 +1359,15 @@ public String getFieldTargetNamespace() { } /** - * The only parametrisation class of the field. In case of basic parametrisation like {@link List} this - * property is {@code null}. + * The only parametrisation class of the field (collection type). In case of basic parametrisation like + * {@link List} this property is {@code null}. */ public JDefinedClass getFieldParametrisationClass() { return fieldParametrisationClass; } /** - * If {@link #getFieldParametrisationClass()} is an interface, then this holds that same value. Otherwise it + * If {@link #getFieldParametrisationClass()} is an interface, then this holds the same value. Otherwise it * holds the implementation (value object) of {@link #getFieldParametrisationClass()}. In case of basic * parametrisation like {@code List} this property is {@code null}. */ diff --git a/src/test/generated_resources/element_name_collision/ObjectFactory.java b/src/test/generated_resources/element_name_collision/ObjectFactory.java new file mode 100644 index 0000000..074a634 --- /dev/null +++ b/src/test/generated_resources/element_name_collision/ObjectFactory.java @@ -0,0 +1,64 @@ + +package element_name_collision; + +import javax.xml.bind.annotation.XmlRegistry; + + +/** + * This object contains factory methods for each + * Java content interface and Java element interface + * generated in the element_name_collision package. + *

An ObjectFactory allows you to programatically + * construct new instances of the Java representation + * for XML content. The Java representation of XML + * content can consist of schema derived interfaces + * and classes representing the binding of schema + * type definitions, element declarations and model + * groups. Factory methods for each of these are + * provided in this class. + * + */ +@XmlRegistry +public class ObjectFactory { + + + /** + * Create a new ObjectFactory that can be used to create new instances of schema derived classes for package: element_name_collision + * + */ + public ObjectFactory() { + } + + /** + * Create an instance of {@link Root } + * + */ + public Root createRoot() { + return new Root(); + } + + /** + * Create an instance of {@link Root.Action } + * + */ + public Root.Action createRootAction() { + return new Root.Action(); + } + + /** + * Create an instance of {@link Root.Action.AddItem } + * + */ + public Root.Action.AddItem createRootActionAddItem() { + return new Root.Action.AddItem(); + } + + /** + * Create an instance of {@link Root.Action.Item } + * + */ + public Root.Action.Item createRootActionItem() { + return new Root.Action.Item(); + } + +} diff --git a/src/test/generated_resources/element_name_collision/Root.java b/src/test/generated_resources/element_name_collision/Root.java new file mode 100644 index 0000000..781247c --- /dev/null +++ b/src/test/generated_resources/element_name_collision/Root.java @@ -0,0 +1,385 @@ + +package element_name_collision; + +import java.util.ArrayList; +import java.util.List; +import javax.xml.bind.annotation.XmlAccessType; +import javax.xml.bind.annotation.XmlAccessorType; +import javax.xml.bind.annotation.XmlElement; +import javax.xml.bind.annotation.XmlElementWrapper; +import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.bind.annotation.XmlType; + + +/** + *

Java class for anonymous complex type. + * + *

The following schema fragment specifies the expected content contained within this class. + * + * 

+ * <complexType>
+ *   <complexContent>
+ *     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *       <sequence>
+ *         <element name="action">
+ *           <complexType>
+ *             <complexContent>
+ *               <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *                 <choice>
+ *                   <element name="update">
+ *                     <complexType>
+ *                       <complexContent>
+ *                         <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *                           <sequence>
+ *                             <element name="item" maxOccurs="unbounded" minOccurs="0">
+ *                               <complexType>
+ *                                 <complexContent>
+ *                                   <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *                                     <all>
+ *                                       <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+ *                                       <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+ *                                     </all>
+ *                                   </restriction>
+ *                                 </complexContent>
+ *                               </complexType>
+ *                             </element>
+ *                           </sequence>
+ *                         </restriction>
+ *                       </complexContent>
+ *                     </complexType>
+ *                   </element>
+ *                   <element name="add">
+ *                     <complexType>
+ *                       <complexContent>
+ *                         <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *                           <sequence>
+ *                             <element name="item" maxOccurs="unbounded" minOccurs="0">
+ *                               <complexType>
+ *                                 <complexContent>
+ *                                   <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+ *                                     <all>
+ *                                       <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+ *                                       <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+ *                                     </all>
+ *                                   </restriction>
+ *                                 </complexContent>
+ *                               </complexType>
+ *                             </element>
+ *                           </sequence>
+ *                         </restriction>
+ *                       </complexContent>
+ *                     </complexType>
+ *                   </element>
+ *                 </choice>
+ *               </restriction>
+ *             </complexContent>
+ *           </complexType>
+ *         </element>
+ *       </sequence>
+ *     </restriction>
+ *   </complexContent>
+ * </complexType>
+ *
+ * + * + */ +@XmlAccessorType(XmlAccessType.FIELD) +@XmlType(name = "", propOrder = { + "action" +}) +@XmlRootElement(name = "root") +public class Root { + + @XmlElement(required = true) + protected Root.Action action; + + /** + * Gets the value of the action property. + * + * @return + * possible object is + * {@link Root.Action } + * + */ + public Root.Action getAction() { + return action; + } + + /** + * Sets the value of the action property. + * + * @param value + * allowed object is + * {@link Root.Action } + * + */ + public void setAction(Root.Action value) { + this.action = value; + } + + + /** + *

Java class for anonymous complex type. + * + *

The following schema fragment specifies the expected content contained within this class. + * + * 

+     * <complexType>
+     *   <complexContent>
+     *     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+     *       <choice>
+     *         <element name="update">
+     *           <complexType>
+     *             <complexContent>
+     *               <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+     *                 <sequence>
+     *                   <element name="item" maxOccurs="unbounded" minOccurs="0">
+     *                     <complexType>
+     *                       <complexContent>
+     *                         <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+     *                           <all>
+     *                             <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+     *                             <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+     *                           </all>
+     *                         </restriction>
+     *                       </complexContent>
+     *                     </complexType>
+     *                   </element>
+     *                 </sequence>
+     *               </restriction>
+     *             </complexContent>
+     *           </complexType>
+     *         </element>
+     *         <element name="add">
+     *           <complexType>
+     *             <complexContent>
+     *               <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+     *                 <sequence>
+     *                   <element name="item" maxOccurs="unbounded" minOccurs="0">
+     *                     <complexType>
+     *                       <complexContent>
+     *                         <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+     *                           <all>
+     *                             <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+     *                             <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+     *                           </all>
+     *                         </restriction>
+     *                       </complexContent>
+     *                     </complexType>
+     *                   </element>
+     *                 </sequence>
+     *               </restriction>
+     *             </complexContent>
+     *           </complexType>
+     *         </element>
+     *       </choice>
+     *     </restriction>
+     *   </complexContent>
+     * </complexType>
+     *
+ * + * + */ + @XmlAccessorType(XmlAccessType.FIELD) + @XmlType(name = "", propOrder = { + "update", + "add" + }) + public static class Action { + + @XmlElementWrapper + @XmlElement(name = "item", namespace = "http://foo.org/") + protected List update; + @XmlElementWrapper + @XmlElement(name = "item", namespace = "http://foo.org/") + protected List add; + + public List getUpdate() { + if (update == null) { + update = new ArrayList(); + } + return update; + } + + public void setUpdate(List update) { + this.update = update; + } + + public List getAdd() { + if (add == null) { + add = new ArrayList(); + } + return add; + } + + public void setAdd(List add) { + this.add = add; + } + + + /** + *

Java class for anonymous complex type. + * + *

The following schema fragment specifies the expected content contained within this class. + * + * 

+         * <complexType>
+         *   <complexContent>
+         *     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+         *       <all>
+         *         <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+         *         <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+         *       </all>
+         *     </restriction>
+         *   </complexContent>
+         * </complexType>
+         *
+ * + * + */ + @XmlAccessorType(XmlAccessType.FIELD) + @XmlType(name = "", propOrder = { + + }) + public static class AddItem { + + @XmlElement(required = true) + protected String key; + @XmlElement(required = true) + protected String value; + + /** + * Gets the value of the key property. + * + * @return + * possible object is + * {@link String } + * + */ + public String getKey() { + return key; + } + + /** + * Sets the value of the key property. + * + * @param value + * allowed object is + * {@link String } + * + */ + public void setKey(String value) { + this.key = value; + } + + /** + * Gets the value of the value property. + * + * @return + * possible object is + * {@link String } + * + */ + public String getValue() { + return value; + } + + /** + * Sets the value of the value property. + * + * @param value + * allowed object is + * {@link String } + * + */ + public void setValue(String value) { + this.value = value; + } + + } + + + /** + *

Java class for anonymous complex type. + * + *

The following schema fragment specifies the expected content contained within this class. + * + * 

+         * <complexType>
+         *   <complexContent>
+         *     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
+         *       <all>
+         *         <element name="key" type="{http://www.w3.org/2001/XMLSchema}string"/>
+         *         <element name="value" type="{http://www.w3.org/2001/XMLSchema}string"/>
+         *       </all>
+         *     </restriction>
+         *   </complexContent>
+         * </complexType>
+         *
+ * + * + */ + @XmlAccessorType(XmlAccessType.FIELD) + @XmlType(name = "", propOrder = { + + }) + public static class Item { + + @XmlElement(required = true) + protected String key; + @XmlElement(required = true) + protected String value; + + /** + * Gets the value of the key property. + * + * @return + * possible object is + * {@link String } + * + */ + public String getKey() { + return key; + } + + /** + * Sets the value of the key property. + * + * @param value + * allowed object is + * {@link String } + * + */ + public void setKey(String value) { + this.key = value; + } + + /** + * Gets the value of the value property. + * + * @return + * possible object is + * {@link String } + * + */ + public String getValue() { + return value; + } + + /** + * Sets the value of the value property. + * + * @param value + * allowed object is + * {@link String } + * + */ + public void setValue(String value) { + this.value = value; + } + + } + + } + +} diff --git a/src/test/generated_resources/element_name_collision/package-info.java b/src/test/generated_resources/element_name_collision/package-info.java new file mode 100644 index 0000000..9b50ccf --- /dev/null +++ b/src/test/generated_resources/element_name_collision/package-info.java @@ -0,0 +1,2 @@ +@javax.xml.bind.annotation.XmlSchema(namespace = "http://foo.org/", elementFormDefault = javax.xml.bind.annotation.XmlNsForm.QUALIFIED) +package element_name_collision; diff --git a/src/test/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPluginTest.java b/src/test/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPluginTest.java index 7fedf6a..7a028b9 100644 --- a/src/test/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPluginTest.java +++ b/src/test/java/com/sun/tools/xjc/addon/xew/XmlElementWrapperPluginTest.java @@ -195,6 +195,13 @@ public void testElementListExtended() throws Exception { assertXsd("element-list-extended", null, false, "Foo", "package-info"); } + @Test + public void testElementNameCollision() throws Exception { + // Most classes cannot be tested for content + assertXsd("element-name-collision", new String[] { "-debug", "-Xxew:instantiate", "lazy" }, false, "Root", + "package-info"); + } + @Test public void testElementScoped() throws Exception { // Most classes cannot be tested for content diff --git a/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xml b/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xml new file mode 100644 index 0000000..52e5013 --- /dev/null +++ b/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xml @@ -0,0 +1,20 @@ + + + + + + + SpiderMan + Mutant + + + Luke Skywalker + Jedi + + + + diff --git a/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xsd b/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xsd new file mode 100644 index 0000000..250dea4 --- /dev/null +++ b/src/test/resources/com/sun/tools/xjc/addon/xew/element-name-collision.xsd @@ -0,0 +1,58 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file