Resolve bits/union nested type naming 62/82762/1
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 25 Jun 2019 13:07:06 +0000 (15:07 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 27 Jun 2019 16:30:41 +0000 (18:30 +0200)
The cases where we allocate an enumeration, bits or union type
for a leaf-like construct (leaf, leaf-list) need to handle an
edge case -- JLS does not allow nested classes to be named the
same as the enclosing class.

Enumeration case was already handled in
AbstractGeneratedTypeBuilder, this patch extends that support
to handle bits and union.

JIRA: MDSAL-458
Change-Id: Idc19b179274c7973ed205ffdae1d2242cbd2dbf6
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 30952238d240260310d357c2e80be55021ccb1fc)

binding/mdsal-binding-generator-api/src/main/java/org/opendaylight/mdsal/binding/model/api/JavaTypeName.java
binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal448Test.java
binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal458Test.java [new file with mode: 0644]
binding/mdsal-binding-generator-impl/src/test/resources/mdsal458.yang [new file with mode: 0644]
binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/AbstractGeneratedTypeBuilder.java

index 555bc426f2f174ccd4ef9a2b0cad0972c9d3cf4b..67aea7e623f060131700862a1c50e09f581127a3 100644 (file)
@@ -20,6 +20,8 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.concepts.Identifier;
 import org.opendaylight.yangtools.concepts.Immutable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * A type name. This class encloses Java type naming rules laid down in
@@ -219,6 +221,7 @@ public abstract class JavaTypeName implements Identifier, Immutable {
         }
     }
 
+    private static final Logger LOG = LoggerFactory.getLogger(JavaTypeName.class);
     private static final long serialVersionUID = 1L;
 
     private final String simpleName;
@@ -278,6 +281,30 @@ public abstract class JavaTypeName implements Identifier, Immutable {
      */
     public abstract JavaTypeName createEnclosed(String simpleName);
 
+    /**
+     * Create a TypeName for a class immediately enclosed by this class, potentially falling back to appending it with
+     * a suffix if a JLS hiding conflict occurs.
+     *
+     * @param simpleName Simple name of the enclosed class
+     * @param fallbackSuffix Suffix to append if the {@code simpleName} is cannot be created due to JLS
+     * @return A new TypeName.
+     * @throws NullPointerException if any argument is null
+     * @throws IllegalArgumentException if simpleName is empty or if both simpleName and fallback both hide any of the
+     *                                  enclosing types
+     * @throws UnsupportedOperationException if this type name does not support nested type
+     */
+    @SuppressWarnings("checkstyle:hiddenField")
+    public final JavaTypeName createEnclosed(final String simpleName, final String fallbackSuffix) {
+        checkArgument(!simpleName.isEmpty());
+        try {
+            return createEnclosed(simpleName);
+        } catch (IllegalArgumentException e) {
+            final String fallback = simpleName + fallbackSuffix;
+            LOG.debug("Failed to create enclosed type '{}', falling back to '{}'", simpleName, fallback, e);
+            return createEnclosed(fallback);
+        }
+    }
+
     /**
      * Create a TypeName for a class that is a sibling of this class. A sibling has the same package name, and the same
      * immediately enclosing class.
index 2d182d1b7aa76452d1c7dd9bb0494769c5615fa9..27d9cf9355844629c837b122a89a5f9b2d7186c7 100644 (file)
@@ -1800,8 +1800,7 @@ abstract class AbstractTypeGenerator {
     private Type addTOToTypeBuilder(final UnionTypeDefinition typeDef,
             final GeneratedTypeBuilder typeBuilder, final DataSchemaNode leaf, final Module parentModule) {
         final List<GeneratedTOBuilder> types = typeProvider.provideGeneratedTOBuildersForUnionTypeDef(
-            typeBuilder.getIdentifier().createEnclosed(BindingMapping.getClassName(leaf.getQName())),
-            typeDef, leaf);
+            allocateNestedType(typeBuilder.getIdentifier(), leaf.getQName()), typeDef, leaf);
 
         checkState(!types.isEmpty(), "No GeneratedTOBuilder objects generated from union %s", typeDef);
         final List<GeneratedTOBuilder> genTOBuilders = new ArrayList<>(types);
@@ -1834,11 +1833,9 @@ abstract class AbstractTypeGenerator {
     private GeneratedTOBuilder addTOToTypeBuilder(final BitsTypeDefinition typeDef,
             final GeneratedTypeBuilder typeBuilder, final DataSchemaNode leaf, final Module parentModule) {
         final GeneratedTOBuilder genTOBuilder = typeProvider.provideGeneratedTOBuilderForBitsTypeDefinition(
-            typeBuilder.getIdentifier().createEnclosed(BindingMapping.getClassName(leaf.getQName())),
-            typeDef, parentModule.getName());
+            allocateNestedType(typeBuilder.getIdentifier(), leaf.getQName()), typeDef, parentModule.getName());
         typeBuilder.addEnclosingTransferObject(genTOBuilder);
         return genTOBuilder;
-
     }
 
     /**
@@ -1895,6 +1892,11 @@ abstract class AbstractTypeGenerator {
         return null;
     }
 
+    private static JavaTypeName allocateNestedType(final JavaTypeName parent, final QName child) {
+        // Single '$' suffix cannot come from user, this mirrors AbstractGeneratedTypeBuilder.addEnumeration()
+        return parent.createEnclosed(BindingMapping.getClassName(child), "$");
+    }
+
     private static void annotateDeprecatedIfNecessary(final Status status, final AnnotableTypeBuilder builder) {
         if (status == Status.DEPRECATED) {
             builder.addAnnotation(DEPRECATED_ANNOTATION);
index 14d66b13dc144b51126eab618f8b4cdd214cb4e2..7010ee44a0db7e7715cffbd107503c4a458be408 100644 (file)
@@ -46,7 +46,7 @@ public class Mdsal448Test {
         assertEquals(9, types.size());
     }
 
-    private static List<GroupingDefinition> sortGroupings(GroupingDefinition... groupings) {
+    private static List<GroupingDefinition> sortGroupings(final GroupingDefinition... groupings) {
         return new GroupingDefinitionDependencySort().sort(Arrays.asList(groupings));
     }
 }
diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal458Test.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal458Test.java
new file mode 100644 (file)
index 0000000..1d826fa
--- /dev/null
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2019 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.mdsal.binding.generator.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import com.google.common.collect.ImmutableSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.opendaylight.mdsal.binding.model.api.JavaTypeName;
+import org.opendaylight.mdsal.binding.model.api.Type;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class Mdsal458Test {
+
+    @Test
+    public void testNestedClassFallback() {
+        final List<Type> types = new BindingGeneratorImpl().generateTypes(
+            YangParserTestUtils.parseYangResource("/mdsal458.yang"));
+        assertNotNull(types);
+        assertEquals(3, types.size());
+
+        final Set<JavaTypeName> typeNames = types.stream().map(Type::getIdentifier).collect(Collectors.toSet());
+        assertEquals(ImmutableSet.of(
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal458.norev", "ExportedTo"),
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal458.norev", "ExportedToExportedTo$Builder"),
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal458.norev", "Mdsal458Data")), typeNames);
+    }
+}
diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/mdsal458.yang b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal458.yang
new file mode 100644 (file)
index 0000000..5738a70
--- /dev/null
@@ -0,0 +1,16 @@
+module mdsal458 {
+  namespace "mdsal458";
+  prefix mdsal458;
+
+  container exported-to {
+    leaf exported-to {
+      type union {
+        type enumeration {
+          enum "netconf";
+          enum "cli";
+        }
+        type string;
+      }
+    }
+  }
+}
index 7b94eaf3868a0ed51de504ba8f090f86ab88212e..30ecc77c82c27e11df898f066c113e0a73366e87 100644 (file)
@@ -165,18 +165,13 @@ abstract class AbstractGeneratedTypeBuilder<T extends GeneratedTypeBuilderBase<T
     }
 
     @Override
-    public EnumBuilder addEnumeration(String name) {
+    public EnumBuilder addEnumeration(final String name) {
         checkArgument(name != null, "Name of enumeration cannot be null!");
 
         // This enumeration may be generated from a leaf, which may end up colliding with its enclosing type
-        // hierarchy. Check it and assign another name if that should be the case.
-        final boolean canCreate = getIdentifier().canCreateEnclosed(name);
-        if (!canCreate) {
-            // Append a single '$' -- it cannot come from the user, hence it marks our namespace.
-            name = name + '$';
-        }
-
-        final EnumBuilder builder = newEnumerationBuilder(getIdentifier().createEnclosed(name));
+        // hierarchy. If that is the case, we use a single '$' suffix to disambiguate -- that cannot come from the user
+        // and hence is marking our namespace
+        final EnumBuilder builder = newEnumerationBuilder(getIdentifier().createEnclosed(name, "$"));
         checkArgument(!enumDefinitions.contains(builder),
             "Generated type %s already contains an enumeration for %s", this, builder);
         this.enumDefinitions = LazyCollections.lazyAdd(this.enumDefinitions, builder);