From acb4d4f5909022a36b80169c84d2f3728f53cc57 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Mon, 19 Mar 2018 02:11:45 +0100
Subject: [PATCH] Disambiguate generated nested enumerations

In case we have an enumeration defined directly in a leaf of the same
name as its containing generated type (list, container, etc.), we can
end up violating JLS section 8.1 class naming requirements.

Detect this condition and munge the type name by appending a '$' to
prevent this conflict.

Change-Id: Ia68cac2a96ac0a95fa38a83cb1cdc1c09540533b
JIRA: MDSAL-321
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../generator/impl/AbstractTypeGenerator.java   |  3 +--
 .../yang/types/AbstractTypeProvider.java        |  4 +---
 .../builder/AbstractGeneratedTypeBuilder.java   | 14 +++++++++++---
 .../api/generator/test/CompilationTest.java     |  9 +++++++++
 .../compilation/mdsal321/odl-mdsal321.yang      | 17 +++++++++++++++++
 5 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 binding/mdsal-binding-java-api-generator/src/test/resources/compilation/mdsal321/odl-mdsal321.yang

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
index 3945dc22c3..3c6669ea59 100644
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
@@ -649,8 +649,7 @@ abstract class AbstractTypeGenerator {
     private EnumBuilder resolveInnerEnumFromTypeDefinition(final EnumTypeDefinition enumTypeDef, final QName enumName,
             final GeneratedTypeBuilder typeBuilder, final ModuleContext context) {
         if (enumTypeDef != null && typeBuilder != null && enumTypeDef.getQName().getLocalName() != null) {
-            final String enumerationName = BindingMapping.getClassName(enumName);
-            final EnumBuilder enumBuilder = typeBuilder.addEnumeration(enumerationName);
+            final EnumBuilder enumBuilder = typeBuilder.addEnumeration(BindingMapping.getClassName(enumName));
             typeProvider.addEnumDescription(enumBuilder, enumTypeDef);
             enumBuilder.updateEnumPairsFromEnumTypeDef(enumTypeDef);
             context.addInnerTypedefType(enumTypeDef.getPath(), enumBuilder);
diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/AbstractTypeProvider.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/AbstractTypeProvider.java
index 0f8cea0723..8ece98b359 100644
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/AbstractTypeProvider.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/AbstractTypeProvider.java
@@ -687,9 +687,7 @@ public abstract class AbstractTypeProvider implements TypeProvider {
                 "Local Name in EnumTypeDefinition QName cannot be NULL!");
         Preconditions.checkArgument(typeBuilder != null, "Generated Type Builder reference cannot be NULL!");
 
-        final String enumerationName = BindingMapping.getClassName(enumName);
-
-        final EnumBuilder enumBuilder = typeBuilder.addEnumeration(enumerationName);
+        final EnumBuilder enumBuilder = typeBuilder.addEnumeration(BindingMapping.getClassName(enumName));
 
         addEnumDescription(enumBuilder, enumTypeDef);
         enumBuilder.updateEnumPairsFromEnumTypeDef(enumTypeDef);
diff --git a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/AbstractGeneratedTypeBuilder.java b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/AbstractGeneratedTypeBuilder.java
index e7ac745d10..81fc1f099a 100644
--- a/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/AbstractGeneratedTypeBuilder.java
+++ b/binding/mdsal-binding-generator-util/src/main/java/org/opendaylight/mdsal/binding/model/util/generated/type/builder/AbstractGeneratedTypeBuilder.java
@@ -164,12 +164,20 @@ abstract class AbstractGeneratedTypeBuilder<T extends GeneratedTypeBuilderBase<T
     }
 
     @Override
-    public EnumBuilder addEnumeration(final String name) {
+    public EnumBuilder addEnumeration(String name) {
         Preconditions.checkArgument(name != null, "Name of enumeration cannot be null!");
-        final EnumBuilder builder = newEnumerationBuilder(getIdentifier().createEnclosed(name));
 
+        // 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));
         Preconditions.checkArgument(!this.enumDefinitions.contains(builder),
-            "This generated type already contains equal enumeration.");
+            "Generated type %s already contains an enumeration for %s", this, builder);
         this.enumDefinitions = LazyCollections.lazyAdd(this.enumDefinitions, builder);
         return builder;
     }
diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
index be375a3dc4..172751f570 100644
--- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
+++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
@@ -627,6 +627,15 @@ public class CompilationTest extends BaseCompilationTest {
         CompilationTestUtils.cleanUp(sourcesOutputDir, compiledOutputDir);
     }
 
+    @Test
+    public void innerEnumerationNameCollisionTest() throws Exception {
+        final File sourcesOutputDir = CompilationTestUtils.generatorOutput("mdsal321");
+        final File compiledOutputDir = CompilationTestUtils.compilerOutput("mdsal321");
+        generateTestSources("/compilation/mdsal321", sourcesOutputDir);
+        CompilationTestUtils.testCompilation(sourcesOutputDir, compiledOutputDir);
+        CompilationTestUtils.cleanUp(sourcesOutputDir, compiledOutputDir);
+    }
+
     private void generateTestSources(final String resourceDirPath, final File sourcesOutputDir)
             throws IOException, URISyntaxException {
         final List<File> sourceFiles = CompilationTestUtils.getSourceFiles(resourceDirPath);
diff --git a/binding/mdsal-binding-java-api-generator/src/test/resources/compilation/mdsal321/odl-mdsal321.yang b/binding/mdsal-binding-java-api-generator/src/test/resources/compilation/mdsal321/odl-mdsal321.yang
new file mode 100644
index 0000000000..83ac87a164
--- /dev/null
+++ b/binding/mdsal-binding-java-api-generator/src/test/resources/compilation/mdsal321/odl-mdsal321.yang
@@ -0,0 +1,17 @@
+module odl-mdsal {
+    namespace "urn:odl:mdsal321";
+    prefix odl;
+
+    container foo {
+        leaf foo {
+            type enumeration {
+                enum "foo";
+            }
+            description "Enumeration defined in a leaf. This triggers a nested
+                         class with a name derived from leaf name to be created
+                         in a class from container name. Both names are 'foo'.
+                         Measures have to be taken to rename the class, so it
+                         does not clash with its enclosing class.";
+        }
+    }
+}
-- 
2.36.6