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>

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 3945dc22c3ac5a4a9b3c405a404935e7620fb479..3c6669ea59c0fbd36978abab0f9b9b16188286b8 100644 (file)
--- 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 0f8cea0723816aaef5f7ae19597d13bb7848b9de..8ece98b359855d10cef506e59905f8231e1d0f87 100644 (file)
--- 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 e7ac745d10a3d0d4981ec385eef14e5496c73fb9..81fc1f099ac2860e19d9001f6e8496a73920f404 100644 (file)
--- 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 be375a3dc41c0b30d22a6f9e0082b8c00ff74986..172751f570302c68c5b112ca5b386bf00efc041a 100644 (file)
--- 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 (file)
index 0000000..83ac87a
--- /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.";
+        }
+    }
+}